From f0a646e057829b45e82c38233d1fcfd0a9d38bd4 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Sat, 20 Dec 2025 20:30:54 +0000 Subject: [PATCH] Have inspection hook set up physical_network, etc., for neutron boot --- .../ironic_understack/port_bios_name_hook.py | 128 ++++++++++++------ .../tests/test_port_bios_name_hook.py | 50 +++++++ 2 files changed, 140 insertions(+), 38 deletions(-) diff --git a/python/ironic-understack/ironic_understack/port_bios_name_hook.py b/python/ironic-understack/ironic_understack/port_bios_name_hook.py index 570c45889..bcff41cdc 100644 --- a/python/ironic-understack/ironic_understack/port_bios_name_hook.py +++ b/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, + ) diff --git a/python/ironic-understack/ironic_understack/tests/test_port_bios_name_hook.py b/python/ironic-understack/ironic_understack/tests/test_port_bios_name_hook.py index fc79e9949..b86887f20 100644 --- a/python/ironic-understack/ironic_understack/tests/test_port_bios_name_hook.py +++ b/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()