fix: Rework enrol workflow

python/ironic-understack/ironic_understack/port_bios_name_hook.py

@@ -1,27 +1,24 @@
 from typing import ClassVar
 
-from ironic.common import exception
 from ironic.drivers.modules.inspector.hooks import base
-from ironic.objects.bios import BIOSSetting
 from oslo_log import log as logging
 
 from ironic_understack.ironic_wrapper import ironic_ports_for_node
 
 LOG = logging.getLogger(__name__)
 
 PXE_BIOS_NAME_PREFIXES = ["NIC.Integrated", "NIC.Slot"]
-BIOS_SETTING_NAME = "HttpDev1Interface"
 
 
 class PortBiosNameHook(base.InspectionHook):
     """Set bios_name, pxe_enabled, local_link_connection and physical_network.
 
     Populates extra.bios_name and port name from inspection inventory, then
-    determines the PXE port from the BIOS HttpDev1Interface setting (populated
-    during enrolment).  Falls back to a naming-convention heuristic if the
-    BIOS setting is unavailable.
+    determines PXE-enabled ports from node.extra["enrolled_pxe_ports"]
+    (populated during enrolment). If that data is unavailable, all
+    NIC.Integrated.* and NIC.Slot.* ports are treated as PXE-enabled.
 
-    The PXE port gets pxe_enabled=True plus placeholder physical_network and
+    PXE ports get pxe_enabled=True plus placeholder physical_network and
     local_link_connection values that neutron requires.
     """
 
@@ -37,7 +34,7 @@ def __call__(self, task, inventory, plugin_data):
             i["mac_address"].upper(): i["name"] for i in inspected_interfaces
         }
 
-        pxe_nic = _bios_pxe_nic(task)
+        pxe_nics = _enrolled_pxe_nics(task)
 
         for baremetal_port in ironic_ports_for_node(task.context, task.node.id):
             mac = baremetal_port.address.upper()
@@ -46,15 +43,10 @@ def __call__(self, task, inventory, plugin_data):
             _set_port_extra(baremetal_port, mac, bios_name)
             _set_port_name(baremetal_port, mac, bios_name, task.node.name)
 
-            if pxe_nic:
-                is_pxe = bios_name is not None and (
-                    pxe_nic.startswith(bios_name) or bios_name.startswith(pxe_nic)
-                )
-            else:
-                # Fallback: heuristic based on naming convention
-                is_pxe = bios_name == _pxe_interface_name(
-                    inspected_interfaces, task.node.uuid
-                )
+            is_pxe = bios_name is not None and any(
+                pxe_nic.startswith(bios_name) or bios_name.startswith(pxe_nic)
+                for pxe_nic in pxe_nics
+            )
 
             if baremetal_port.pxe_enabled != is_pxe:
                 LOG.info(
@@ -72,45 +64,24 @@ def __call__(self, task, inventory, plugin_data):
                 _set_port_local_link_connection(baremetal_port, mac)
 
 
-def _bios_pxe_nic(task):
-    """Read the BIOS PXE NIC FQDD, or return None if unavailable."""
-    try:
-        task.driver.bios.cache_bios_settings(task)
-    except Exception:
-        LOG.warning(
-            "Cannot cache BIOS settings for node %s, "
-            "falling back to naming heuristic for PXE port.",
-            task.node.uuid,
-        )
-        return None
-
-    try:
-        setting = BIOSSetting.get(task.context, task.node.id, BIOS_SETTING_NAME)
-    except exception.BIOSSettingNotFound:
-        LOG.warning(
-            "BIOS setting %s not found for node %s, "
-            "falling back to naming heuristic for PXE port.",
-            BIOS_SETTING_NAME,
-            task.node.uuid,
-        )
-        return None
-
-    if not setting.value:
+def _enrolled_pxe_nics(task) -> list[str]:
+    """Read enrolled PXE NIC names from node.extra, or use broad prefixes."""
+    enrolled_pxe_nics = task.node.extra.get("enrolled_pxe_ports")
+    if enrolled_pxe_nics is None:
         LOG.warning(
-            "BIOS setting %s is empty for node %s, "
-            "falling back to naming heuristic for PXE port.",
-            BIOS_SETTING_NAME,
+            "Node %s extra.enrolled_pxe_ports is missing, "
+            "setting pxe flag on all interfaces starting %s.",
             task.node.uuid,
+            PXE_BIOS_NAME_PREFIXES,
         )
-        return None
+        return PXE_BIOS_NAME_PREFIXES
 
     LOG.info(
-        "Node %s BIOS %s = %s",
+        "Set node %s pxe flag on interfaces from extra.enrolled_pxe_ports %s",
         task.node.uuid,
-        BIOS_SETTING_NAME,
-        setting.value,
+        enrolled_pxe_nics,
     )
-    return setting.value
+    return enrolled_pxe_nics
 
 
 def _set_port_extra(baremetal_port, mac, required_bios_name):
@@ -165,18 +136,3 @@ def _set_port_local_link_connection(baremetal_port, mac):
             baremetal_port.local_link_connection,
         )
         baremetal_port.save()
-
-
-def _pxe_interface_name(inspected_interfaces: list[dict], node_uuid) -> str | None:
-    """Use a heuristic to determine our default interface for PXE."""
-    names = sorted(i["name"] for i in inspected_interfaces)
-    for prefix in PXE_BIOS_NAME_PREFIXES:
-        for name in names:
-            if name.startswith(prefix):
-                return name
-    LOG.error(
-        "No PXE interface found for node %s.  Expected to find an "
-        "interface whose bios_name starts with one of %s",
-        node_uuid,
-        PXE_BIOS_NAME_PREFIXES,
-    )

python/ironic-understack/ironic_understack/tests/test_port_bios_name_hook.py

@@ -1,6 +1,5 @@
 import logging
 
-from ironic.common import exception
 from oslo_utils import uuidutils
 
 from ironic_understack.port_bios_name_hook import PortBiosNameHook
@@ -14,32 +13,12 @@
 }
 
 
-def _make_task(mocker, bios_value=None, bios_error=None, cache_error=None):
-    mock_node = mocker.Mock(id=1234, uuid=uuidutils.generate_uuid())
+def _make_task(mocker, enrolled_pxe_ports=None):
+    mock_node = mocker.Mock(id=1234, uuid=uuidutils.generate_uuid(), extra={})
     mock_node.name = "Dell-CR1MB0"
-    task = mocker.Mock(node=mock_node, context=mocker.Mock())
-
-    if cache_error:
-        task.driver.bios.cache_bios_settings.side_effect = cache_error
-    elif bios_error:
-        mocker.patch(
-            "ironic_understack.port_bios_name_hook.BIOSSetting.get",
-            side_effect=bios_error,
-        )
-    elif bios_value is not None:
-        setting = mocker.Mock(value=bios_value)
-        mocker.patch(
-            "ironic_understack.port_bios_name_hook.BIOSSetting.get",
-            return_value=setting,
-        )
-    else:
-        setting = mocker.Mock(value=None)
-        mocker.patch(
-            "ironic_understack.port_bios_name_hook.BIOSSetting.get",
-            return_value=setting,
-        )
-
-    return task
+    if enrolled_pxe_ports is not None:
+        mock_node.extra["enrolled_pxe_ports"] = enrolled_pxe_ports
+    return mocker.Mock(node=mock_node, context=mocker.Mock())
 
 
 def _make_port(mocker, mac, **kwargs):
@@ -57,10 +36,10 @@ def _make_port(mocker, mac, **kwargs):
     return port
 
 
-def test_pxe_from_bios_setting(mocker, caplog):
-    """BIOS HttpDev1Interface determines pxe_enabled."""
+def test_pxe_from_enrolled_pxe_ports(mocker, caplog):
+    """node.extra.enrolled_pxe_ports determines pxe_enabled."""
     caplog.set_level(logging.DEBUG)
-    task = _make_task(mocker, bios_value="NIC.Integrated.1-1-1")
+    task = _make_task(mocker, enrolled_pxe_ports=["NIC.Integrated.1-1-1"])
 
     port1 = _make_port(mocker, "11:11:11:11:11:11")
     port2 = _make_port(mocker, "22:22:22:22:22:22")
@@ -79,10 +58,13 @@ def test_pxe_from_bios_setting(mocker, caplog):
     assert port1.physical_network == "enrol"
 
 
-def test_pxe_fallback_when_cache_fails(mocker, caplog):
-    """Falls back to heuristic when BIOS cache fails."""
+def test_pxe_from_enrolled_pxe_ports_enables_multiple_ports(mocker, caplog):
+    """Every port in enrolled_pxe_ports gets pxe_enabled=True."""
     caplog.set_level(logging.DEBUG)
-    task = _make_task(mocker, cache_error=Exception("no BIOS"))
+    task = _make_task(
+        mocker,
+        enrolled_pxe_ports=["NIC.Integrated.1-1-1", "NIC.Integrated.1-2-1"],
+    )
 
     port1 = _make_port(mocker, "11:11:11:11:11:11")
     port2 = _make_port(mocker, "22:22:22:22:22:22")
@@ -94,19 +76,14 @@ def test_pxe_fallback_when_cache_fails(mocker, caplog):
 
     PortBiosNameHook().__call__(task, _INVENTORY, {})
 
-    # Heuristic picks NIC.Integrated.1-1 (first sorted match)
     assert port1.pxe_enabled is True
-    assert port2.pxe_enabled is False
-    assert "falling back to naming heuristic" in caplog.text
+    assert port2.pxe_enabled is True
 
 
-def test_pxe_fallback_when_setting_missing(mocker, caplog):
-    """Falls back to heuristic when BIOS setting not found."""
+def test_pxe_fallback_when_enrolled_pxe_ports_missing(mocker, caplog):
+    """Missing enrolled_pxe_ports enables all matching PXE prefix ports."""
     caplog.set_level(logging.DEBUG)
-    task = _make_task(
-        mocker,
-        bios_error=exception.BIOSSettingNotFound(node="fake", name="HttpDev1Interface"),
-    )
+    task = _make_task(mocker)
 
     port1 = _make_port(mocker, "11:11:11:11:11:11")
     port2 = _make_port(mocker, "22:22:22:22:22:22")
@@ -119,14 +96,43 @@ def test_pxe_fallback_when_setting_missing(mocker, caplog):
     PortBiosNameHook().__call__(task, _INVENTORY, {})
 
     assert port1.pxe_enabled is True
-    assert port2.pxe_enabled is False
-    assert "falling back to naming heuristic" in caplog.text
+    assert port2.pxe_enabled is True
+    assert "setting pxe flag on all interfaces starting" in caplog.text
+
+
+def test_missing_enrolled_pxe_ports_enables_slot_ports_too(mocker, caplog):
+    """Missing enrolled_pxe_ports enables all supported PXE port prefixes."""
+    caplog.set_level(logging.DEBUG)
+    task = _make_task(mocker)
+    inventory = {
+        "memory": {"physical_mb": 98304},
+        "interfaces": [
+            {"mac_address": "11:11:11:11:11:11", "name": "NIC.Integrated.1-1"},
+            {"mac_address": "22:22:22:22:22:22", "name": "NIC.Slot.1-2"},
+            {"mac_address": "33:33:33:33:33:33", "name": "eno1"},
+        ],
+    }
+
+    port1 = _make_port(mocker, "11:11:11:11:11:11")
+    port2 = _make_port(mocker, "22:22:22:22:22:22")
+    port3 = _make_port(mocker, "33:33:33:33:33:33")
+
+    mocker.patch(
+        "ironic_understack.port_bios_name_hook.ironic_ports_for_node",
+        return_value=[port1, port2, port3],
+    )
+
+    PortBiosNameHook().__call__(task, inventory, {})
+
+    assert port1.pxe_enabled is True
+    assert port2.pxe_enabled is True
+    assert port3.pxe_enabled is False
 
 
 def test_retaining_physical_network(mocker, caplog):
     """Existing physical_network and local_link_connection are preserved."""
     caplog.set_level(logging.DEBUG)
-    task = _make_task(mocker, bios_value="NIC.Integrated.1-1-1")
+    task = _make_task(mocker, enrolled_pxe_ports=["NIC.Integrated.1-1-1"])
 
     port = _make_port(
         mocker,
@@ -153,7 +159,7 @@ def test_retaining_physical_network(mocker, caplog):
 def test_clears_pxe_on_previously_enabled_port(mocker, caplog):
     """Port that was pxe_enabled but no longer matches gets cleared."""
     caplog.set_level(logging.DEBUG)
-    task = _make_task(mocker, bios_value="NIC.Integrated.1-2-1")
+    task = _make_task(mocker, enrolled_pxe_ports=["NIC.Integrated.1-2-1"])
 
     port1 = _make_port(mocker, "11:11:11:11:11:11", pxe_enabled=True)
     port2 = _make_port(mocker, "22:22:22:22:22:22")
@@ -172,7 +178,7 @@ def test_clears_pxe_on_previously_enabled_port(mocker, caplog):
 def test_removing_bios_name(mocker, caplog):
     """Port with unknown MAC gets bios_name removed."""
     caplog.set_level(logging.DEBUG)
-    task = _make_task(mocker, bios_value="NIC.Integrated.1-1-1")
+    task = _make_task(mocker, enrolled_pxe_ports=["NIC.Integrated.1-1-1"])
 
     port = _make_port(
         mocker,

python/understack-workflows/pyproject.toml

@@ -30,7 +30,6 @@ dependencies = [
 ]
 
 [project.scripts]
-bios-settings = "understack_workflows.main.bios_settings:main"
 bmc-kube-password = "understack_workflows.main.bmc_display_password:main"
 bmc-password = "understack_workflows.main.print_bmc_password:main"
 enroll-server = "understack_workflows.main.enroll_server:main"