feat: Have inspection hook set up physical_network, etc., for neutron boot

python/ironic-understack/ironic_understack/port_bios_name_hook.py

@@ -5,12 +5,23 @@
 
 LOG = logging.getLogger(__name__)
 
+PXE_BIOS_NAME_PREFIXES = ["NIC.Integrated", "NIC.Slot"]
+
 
 class PortBiosNameHook(base.InspectionHook):
-    """Set port.extra.bios_name and pxe_enabled fields from redfish data."""
+    """Set name, extra.bios_name and pxe_enabled fields from redfish data.
+
+    In addition, this hook ensures that the PXE port has sufficient data to
+    allow neutron to boot a new node for inspection.  If the physical_network
+    and local_link_connections are not populated, we fill them with placeholder
+    data.
 
-    # "ports" creates baremetal ports for each physical NIC, be sure to run this
-    # first because we will only be updating ports that already exist:
+    This is necessary because neutron throws errors if the port doesn't have
+    those fields filled in.
+    """
+
+    # "ports" hook creates baremetal ports for each physical NIC, be sure to run
+    # this first because we will only be updating ports that already exist:
     dependencies = ["ports"]
 
     def __call__(self, task, inventory, plugin_data):
@@ -24,50 +35,91 @@ def __call__(self, task, inventory, plugin_data):
             i["mac_address"].upper(): i["name"] for i in inspected_interfaces
         }
 
-        pxe_interface = _pxe_interface_name(inspected_interfaces)
+        pxe_interface = _pxe_interface_name(inspected_interfaces, task.node.uuid)
 
         for baremetal_port in ironic_ports_for_node(task.context, task.node.id):
             mac = baremetal_port.address.upper()
             required_bios_name = interface_names.get(mac)
-            extra = baremetal_port.extra
-            current_bios_name = extra.get("bios_name")
-
-            if current_bios_name != required_bios_name:
-                LOG.info(
-                    "Port %(mac)s updating bios_name from %(old)s to %(new)s",
-                    {"mac": mac, "old": current_bios_name, "new": required_bios_name},
-                )
+            required_pxe = required_bios_name == pxe_interface
 
-                if required_bios_name:
-                    extra["bios_name"] = required_bios_name
-                else:
-                    extra.pop("bios_name", None)
+            _set_port_extra(baremetal_port, mac, required_bios_name)
+            _set_port_pxe_enabled(baremetal_port, mac, required_pxe)
+            _set_port_name(baremetal_port, mac, required_bios_name, task.node.name)
+            _set_port_physical_network(baremetal_port, mac, required_pxe)
+            _set_port_local_link_connection(baremetal_port, mac, required_pxe)
+
+
+def _set_port_pxe_enabled(baremetal_port, mac, required_pxe):
+    if baremetal_port.pxe_enabled != required_pxe:
+        LOG.info("Port %s changed pxe_enabled to %s", mac, required_pxe)
+        baremetal_port.pxe_enabled = required_pxe
+        baremetal_port.save()
+
+
+def _set_port_extra(baremetal_port, mac, required_bios_name):
+    extra = baremetal_port.extra
+    current_bios_name = extra.get("bios_name")
+    if current_bios_name != required_bios_name:
+        LOG.info(
+            "Port %(mac)s updating bios_name from %(old)s to %(new)s",
+            {"mac": mac, "old": current_bios_name, "new": required_bios_name},
+        )
+
+        if required_bios_name:
+            extra["bios_name"] = required_bios_name
+        else:
+            extra.pop("bios_name", None)
+
+        baremetal_port.extra = extra
+        baremetal_port.save()
+
+
+def _set_port_name(baremetal_port, mac, required_bios_name, node_name):
+    if required_bios_name:
+        required_port_name = node_name + ":" + required_bios_name
+        if baremetal_port.name != required_port_name:
+            LOG.info(
+                "Port %s changing name from %s to %s",
+                mac,
+                baremetal_port.name,
+                required_port_name,
+            )
+            baremetal_port.name = required_port_name
+            baremetal_port.save()
+
+
+def _set_port_physical_network(baremetal_port, mac, required_pxe):
+    if required_pxe and not baremetal_port.physical_network:
+        LOG.info("Port %s changing physical_network from None to 'enrol'", mac)
+        baremetal_port.physical_network = "enrol"
+        baremetal_port.save()
+
+
+def _set_port_local_link_connection(baremetal_port, mac, required_pxe):
+    if required_pxe and not baremetal_port.local_link_connection:
+        baremetal_port.local_link_connection = {
+            "port_id": "None",
+            "switch_id": "00:00:00:00:00:00",
+            "switch_info": "None",
+        }
+        LOG.info(
+            "Port %s changing local_link_connection from None to %s",
+            mac,
+            baremetal_port.local_link_connection,
+        )
+        baremetal_port.save()
 
-                baremetal_port.extra = extra
-                baremetal_port.save()
 
-            required_pxe = required_bios_name == pxe_interface
-            if baremetal_port.pxe_enabled != required_pxe:
-                LOG.info("Port %s changed pxe_enabled to %s", mac, required_pxe)
-                baremetal_port.pxe_enabled = required_pxe
-                baremetal_port.save()
-
-            if required_bios_name:
-                required_port_name = task.node.name + ":" + required_bios_name
-                if baremetal_port.name != required_port_name:
-                    LOG.info(
-                        "Port %s changing name from %s to %s",
-                        mac,
-                        baremetal_port.name,
-                        required_port_name,
-                    )
-                    baremetal_port.name = required_port_name
-
-
-def _pxe_interface_name(inspected_interfaces: list[dict]) -> str:
+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 ["NIC.Integrated", "NIC.Slot"]:
+    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

@@ -26,6 +26,8 @@ def test_adding_bios_name(mocker, caplog):
         node_id=node_uuid,
         address="11:11:11:11:11:11",
         extra={},
+        local_link_connection={},
+        physical_network=None,
     )
     mock_port.name = "some-arbitrary-name"
 
@@ -38,6 +40,54 @@ def test_adding_bios_name(mocker, caplog):
 
     assert mock_port.extra == {"bios_name": "NIC.Integrated.1-1"}
     assert mock_port.name == "Dell-CR1MB0:NIC.Integrated.1-1"
+    assert mock_port.pxe_enabled
+    assert mock_port.physical_network == "enrol"
+    assert mock_port.local_link_connection == {
+        "port_id": "None",
+        "switch_id": "00:00:00:00:00:00",
+        "switch_info": "None",
+    }
+    mock_port.save.assert_called()
+
+
+def test_retaining_physical_network(mocker, caplog):
+    caplog.set_level(logging.DEBUG)
+
+    node_uuid = uuidutils.generate_uuid()
+    mock_context = mocker.Mock()
+    mock_node = mocker.Mock(id=1234)
+    mock_node.name = "Dell-CR1MB0"
+    mock_task = mocker.Mock(node=mock_node, context=mock_context)
+    mock_port = mocker.Mock(
+        uuid=uuidutils.generate_uuid(),
+        node_id=node_uuid,
+        address="11:11:11:11:11:11",
+        extra={},
+        local_link_connection={
+            "port_id": "Ethernet1/19",
+            "switch_id": "00:00:00:00:00:00",
+            "switch_info": "a1-2-3",
+        },
+        physical_network="previous_value",
+    )
+    mock_port.name = "some-arbitrary-name"
+
+    mocker.patch(
+        "ironic_understack.port_bios_name_hook.ironic_ports_for_node",
+        return_value=[mock_port],
+    )
+
+    PortBiosNameHook().__call__(mock_task, _INVENTORY, {})
+
+    assert mock_port.extra == {"bios_name": "NIC.Integrated.1-1"}
+    assert mock_port.name == "Dell-CR1MB0:NIC.Integrated.1-1"
+    assert mock_port.pxe_enabled
+    assert mock_port.physical_network == "previous_value"
+    assert mock_port.local_link_connection == {
+        "port_id": "Ethernet1/19",
+        "switch_id": "00:00:00:00:00:00",
+        "switch_info": "a1-2-3",
+    }
     mock_port.save.assert_called()