From bb807b70dc032b65254e72948cf51649cf0b0f2b Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 2 Dec 2025 12:51:40 +0000 Subject: [PATCH 1/5] Add pytest-mock dep --- python/ironic-understack/pyproject.toml | 5 ++++- python/ironic-understack/uv.lock | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/python/ironic-understack/pyproject.toml b/python/ironic-understack/pyproject.toml index 7624b2b96..6449cea60 100644 --- a/python/ironic-understack/pyproject.toml +++ b/python/ironic-understack/pyproject.toml @@ -18,6 +18,8 @@ dependencies = [ [project.entry-points."ironic.inspection.hooks"] resource-class = "ironic_understack.resource_class:ResourceClassHook" +update-baremetal-port = "ironic_understack.update_baremetal_port:UpdateBaremetalPortsHook" +port-bios-name = "ironic_understack.port_bios_name_hook:PortBiosNameHook" [project.entry-points."ironic.hardware.interfaces.inspect"] redfish-understack = "ironic_understack.redfish_inspect_understack:UnderstackRedfishInspect" @@ -26,8 +28,9 @@ idrac-redfish-understack = "ironic_understack.redfish_inspect_understack:Underst [dependency-groups] test = [ "pytest>=9.0.1,<10", - "pytest-github-actions-annotate-failures", "pytest-cov>=7,<8", + "pytest-github-actions-annotate-failures", + "pytest-mock>=3.15.1", ] [tool.uv] diff --git a/python/ironic-understack/uv.lock b/python/ironic-understack/uv.lock index 79075f895..4e830150a 100644 --- a/python/ironic-understack/uv.lock +++ b/python/ironic-understack/uv.lock @@ -476,6 +476,7 @@ version = "0.0.0" source = { editable = "." } dependencies = [ { name = "ironic" }, + { name = "pytest-mock" }, { name = "pyyaml" }, { name = "understack-flavor-matcher" }, ] @@ -490,6 +491,7 @@ test = [ [package.metadata] requires-dist = [ { name = "ironic", specifier = ">=32.0,<33" }, + { name = "pytest-mock", specifier = ">=3.15.1" }, { name = "pyyaml", specifier = "~=6.0" }, { name = "understack-flavor-matcher", directory = "../understack-flavor-matcher" }, ] @@ -1335,6 +1337,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/6d/73/7b0b15cb8605ee967b34aa1d949737ab664f94e6b0f1534e8339d9e64ab2/pytest_github_actions_annotate_failures-0.3.0-py3-none-any.whl", hash = "sha256:41ea558ba10c332c0bfc053daeee0c85187507b2034e990f21e4f7e5fef044cf", size = 6030, upload-time = "2025-01-17T22:39:31.701Z" }, ] +[[package]] +name = "pytest-mock" +version = "3.15.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/68/14/eb014d26be205d38ad5ad20d9a80f7d201472e08167f0bb4361e251084a9/pytest_mock-3.15.1.tar.gz", hash = "sha256:1849a238f6f396da19762269de72cb1814ab44416fa73a8686deac10b0d87a0f", size = 34036, upload-time = "2025-09-16T16:37:27.081Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/5a/cc/06253936f4a7fa2e0f48dfe6d851d9c56df896a9ab09ac019d70b760619c/pytest_mock-3.15.1-py3-none-any.whl", hash = "sha256:0a25e2eb88fe5168d535041d09a4529a188176ae608a6d249ee65abc0949630d", size = 10095, upload-time = "2025-09-16T16:37:25.734Z" }, +] + [[package]] name = "python-dateutil" version = "2.9.0.post0" From 4579a65bbbd87cea7c630575214abd64f6dcefc4 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 2 Dec 2025 12:52:22 +0000 Subject: [PATCH 2/5] Add a configuration option specifying the vlan group switch name mapping --- .../ironic-understack/ironic_understack/conf.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/python/ironic-understack/ironic_understack/conf.py b/python/ironic-understack/ironic_understack/conf.py index d41bbb234..459d4ff30 100644 --- a/python/ironic-understack/ironic_understack/conf.py +++ b/python/ironic-understack/ironic_understack/conf.py @@ -10,7 +10,22 @@ def setup_conf(): "device_types_dir", help="directory storing Device Type description YAML files", default="/var/lib/understack/device-types", - ) + ), + cfg.DictOpt( + "switch_name_vlan_group_mapping", + help="Dictionary of switch hostname suffix to vlan group name", + default={ + "1": "network", + "2": "network", + "3": "network", + "4": "network", + "1f": "storage", + "2f": "storage", + "3f": "storage-appliance", + "4f": "storage-appliance", + "1d": "bmc", + }, + ), ] cfg.CONF.register_group(grp) cfg.CONF.register_opts(opts, group=grp) From 2050345213368cadbfbca78b33df968240cf4165 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 2 Dec 2025 12:56:41 +0000 Subject: [PATCH 3/5] Implement ironic post-inspection hook to set port bios_name, pxe_enabled --- .../ironic_understack/ironic_wrapper.py | 5 ++ .../ironic_understack/port_bios_name_hook.py | 62 ++++++++++++++++++ .../tests/test_port_bios_name_hook.py | 63 +++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 python/ironic-understack/ironic_understack/ironic_wrapper.py create mode 100644 python/ironic-understack/ironic_understack/port_bios_name_hook.py create mode 100644 python/ironic-understack/ironic_understack/tests/test_port_bios_name_hook.py diff --git a/python/ironic-understack/ironic_understack/ironic_wrapper.py b/python/ironic-understack/ironic_understack/ironic_wrapper.py new file mode 100644 index 000000000..adbbe8002 --- /dev/null +++ b/python/ironic-understack/ironic_understack/ironic_wrapper.py @@ -0,0 +1,5 @@ +import ironic.objects + + +def ironic_ports_for_node(context, node_id: str) -> list: + return ironic.objects.Port.list_by_node_id(context, node_id) diff --git a/python/ironic-understack/ironic_understack/port_bios_name_hook.py b/python/ironic-understack/ironic_understack/port_bios_name_hook.py new file mode 100644 index 000000000..26f580237 --- /dev/null +++ b/python/ironic-understack/ironic_understack/port_bios_name_hook.py @@ -0,0 +1,62 @@ +from ironic.drivers.modules.inspector.hooks import base +from oslo_log import log as logging + +from ironic_understack.ironic_wrapper import ironic_ports_for_node + +LOG = logging.getLogger(__name__) + + +class PortBiosNameHook(base.InspectionHook): + """Set port.extra.bios_name and pxe_enabled fields from redfish 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: + dependencies = ["ports"] + + def __call__(self, task, inventory, plugin_data): + """Populate the baremetal_port.extra.bios_name attribute.""" + inspected_interfaces = inventory.get("interfaces") + if not inspected_interfaces: + LOG.error("No interfaces in inventory for node %s", task.node.uuid) + return + + interface_names = { + i["mac_address"].upper(): i["name"] for i in inspected_interfaces + } + + pxe_interface = _pxe_interface_name(inspected_interfaces) + + 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}, + ) + + if required_bios_name: + extra["bios_name"] = required_bios_name + else: + extra.pop("bios_name", None) + + 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() + + +def _pxe_interface_name(inspected_interfaces: list[dict]) -> str: + """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 name in names: + if name.startswith(prefix): + return name 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 new file mode 100644 index 000000000..49985693e --- /dev/null +++ b/python/ironic-understack/ironic_understack/tests/test_port_bios_name_hook.py @@ -0,0 +1,63 @@ +import logging + +from oslo_utils import uuidutils + +from ironic_understack.port_bios_name_hook import PortBiosNameHook + +_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.Integrated.1-2"}, + ], +} + + +def test_adding_bios_name(mocker, caplog): + caplog.set_level(logging.DEBUG) + + node_uuid = uuidutils.generate_uuid() + mock_context = mocker.Mock() + mock_node = mocker.Mock(id=1234) + 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={}, + ) + + 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"} + mock_port.save.assert_called() + + +def test_removing_bios_name(mocker, caplog): + caplog.set_level(logging.DEBUG) + + node_uuid = uuidutils.generate_uuid() + mock_context = mocker.Mock() + mock_node = mocker.Mock(id=1234) + mock_task = mocker.Mock(node=mock_node, context=mock_context) + mock_port = mocker.Mock( + uuid=uuidutils.generate_uuid(), + node_id=node_uuid, + address="33:33:33:33:33:33", + extra={"bios_name": "old_name_no_longer_valid"}, + ) + + mocker.patch( + "ironic_understack.port_bios_name_hook.ironic_ports_for_node", + return_value=[mock_port], + ) + + PortBiosNameHook().__call__(mock_task, _INVENTORY, {}) + + assert "bios_name" not in mock_port.extra + mock_port.save.assert_called() From 78210f4f0ae6f86cf9e9ec018ef089ce530479ce Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 2 Dec 2025 12:57:36 +0000 Subject: [PATCH 4/5] Implement ironic inspect hook setting port local_link, physical_network --- .../ironic_understack/inspected_port.py | 56 +++++ .../tests/test_update_baremetal_port.py | 108 +++++++++ .../tests/test_vlan_group_name_convention.py | 143 ++++++++++++ .../update_baremetal_port.py | 207 ++++++++++++++++++ .../vlan_group_name_convention.py | 99 +++++++++ 5 files changed, 613 insertions(+) create mode 100644 python/ironic-understack/ironic_understack/inspected_port.py create mode 100644 python/ironic-understack/ironic_understack/tests/test_update_baremetal_port.py create mode 100644 python/ironic-understack/ironic_understack/tests/test_vlan_group_name_convention.py create mode 100644 python/ironic-understack/ironic_understack/update_baremetal_port.py create mode 100644 python/ironic-understack/ironic_understack/vlan_group_name_convention.py diff --git a/python/ironic-understack/ironic_understack/inspected_port.py b/python/ironic-understack/ironic_understack/inspected_port.py new file mode 100644 index 000000000..fedaed395 --- /dev/null +++ b/python/ironic-understack/ironic_understack/inspected_port.py @@ -0,0 +1,56 @@ +from dataclasses import dataclass + + +@dataclass +class InspectedPort: + """Represents a parsed entry from Ironic inspection (inventory) data.""" + + mac_address: str + name: str + switch_system_name: str + switch_port_id: str + switch_chassis_id: str + + @property + def local_link_connection(self) -> dict: + return { + "port_id": self.switch_port_id, + "switch_id": self.switch_chassis_id, + "switch_info": self.switch_system_name, + } + + @property + def parsed_name(self) -> dict[str, str]: + parts = self.switch_system_name.split(".", maxsplit=1) + if len(parts) != 2: + raise ValueError( + "Failed to parse switch hostname - expecting name.dc in %s", self + ) + switch_name, data_center_name = parts + + parts = switch_name.rsplit("-", maxsplit=1) + if len(parts) != 2: + raise ValueError( + f"Unknown switch name format: {switch_name} - this hook requires " + f"that switch names follow the convention -" + ) + + rack_name, switch_suffix = parts + + return { + "rack_name": rack_name, + "switch_suffix": switch_suffix, + "data_center_name": data_center_name, + } + + @property + def rack_name(self) -> str: + return self.parsed_name["rack_name"] + + @property + def switch_suffix(self) -> str: + return self.parsed_name["switch_suffix"] + + @property + def data_center_name(self) -> str: + return self.parsed_name["data_center_name"] diff --git a/python/ironic-understack/ironic_understack/tests/test_update_baremetal_port.py b/python/ironic-understack/ironic_understack/tests/test_update_baremetal_port.py new file mode 100644 index 000000000..640dadb09 --- /dev/null +++ b/python/ironic-understack/ironic_understack/tests/test_update_baremetal_port.py @@ -0,0 +1,108 @@ +import logging + +import ironic.objects +from oslo_utils import uuidutils + +import ironic_understack +from ironic_understack.update_baremetal_port import UpdateBaremetalPortsHook + +# load some metaprgramming normally taken care of during Ironic initialization: +ironic.objects.register_all() + +_INVENTORY = {} +_PLUGIN_DATA = { + "all_interfaces": { + "ex1": { + "name": "ex1", + "mac_address": "11:11:11:11:11:11", + }, + "ex2": { + "name": "ex2", + "mac_address": "22:22:22:22:22:22", + }, + "ex3": { + "name": "ex3", + "mac_address": "33:33:33:33:33:33", + }, + "ex4": { + "name": "ex4", + "mac_address": "44:44:44:44:44:44", + }, + }, + "parsed_lldp": { + "ex1": { + "switch_chassis_id": "88:5a:92:ec:54:59", + "switch_port_id": "Ethernet1/18", + "switch_system_name": "f20-3-1.iad3", + }, + "ex2": { + "switch_chassis_id": "88:5a:92:ec:54:59", + "switch_port_id": "Ethernet1/18", + "switch_system_name": "f20-3-2.iad3", + }, + "ex3": { + "switch_chassis_id": "88:5a:92:ec:54:59", + "switch_port_id": "Ethernet1/18", + "switch_system_name": "f20-3-1f.iad3", + }, + "ex4": { + "switch_chassis_id": "88:5a:92:ec:54:59", + "switch_port_id": "Ethernet1/18", + "switch_system_name": "f20-3-2f.iad3", + }, + }, +} + +MAPPING = { + "1": "network", + "2": "network", + "1f": "storage", + "2f": "storage", + "-1d": "bmc", +} + + +def test_with_valid_data(mocker, caplog): + caplog.set_level(logging.DEBUG) + + node_uuid = uuidutils.generate_uuid() + mock_traits = mocker.Mock() + mock_context = mocker.Mock() + mock_node = mocker.Mock(id=1234, traits=mock_traits) + 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", + local_link_connection={}, + physical_network="original_value", + ) + + mocker.patch( + "ironic_understack.update_baremetal_port.ironic_ports_for_node", + return_value=[mock_port], + ) + mocker.patch( + "ironic_understack.update_baremetal_port.CONF.ironic_understack.switch_name_vlan_group_mapping", + MAPPING, + ) + mocker.patch("ironic_understack.update_baremetal_port.objects.TraitList.create") + + mock_traits.get_trait_names.return_value = ["CUSTOM_BMC_SWITCH", "bar"] + + UpdateBaremetalPortsHook().__call__(mock_task, _INVENTORY, _PLUGIN_DATA) + + assert mock_port.local_link_connection == { + "port_id": "Ethernet1/18", + "switch_id": "88:5a:92:ec:54:59", + "switch_info": "f20-3-1.iad3", + } + assert mock_port.physical_network == "f20-3-network" + mock_port.save.assert_called() + + mock_traits.get_trait_names.assert_called_once() + mock_traits.destroy.assert_called_once_with("CUSTOM_BMC_SWITCH") + ironic_understack.update_baremetal_port.objects.TraitList.create.assert_called_once_with( + mock_context, 1234, ["CUSTOM_NETWORK_SWITCH", "CUSTOM_STORAGE_SWITCH"] + ) + mock_node.save.assert_called_once() diff --git a/python/ironic-understack/ironic_understack/tests/test_vlan_group_name_convention.py b/python/ironic-understack/ironic_understack/tests/test_vlan_group_name_convention.py new file mode 100644 index 000000000..cfd952120 --- /dev/null +++ b/python/ironic-understack/ironic_understack/tests/test_vlan_group_name_convention.py @@ -0,0 +1,143 @@ +import pytest + +from ironic_understack.inspected_port import InspectedPort +from ironic_understack.vlan_group_name_convention import TopologyError +from ironic_understack.vlan_group_name_convention import vlan_group_names + +mapping = { + "1": "network", + "2": "network", + "3": "network", + "4": "network", + "1f": "storage", + "2f": "storage", + "3f": "storage-appliance", + "4f": "storage-appliance", + "1d": "bmc", +} + + +def port(switch: str): + return InspectedPort( + mac_address="", + name="", + switch_system_name=switch, + switch_chassis_id="", + switch_port_id="", + ) + + +def test_vlan_group_name_single_cab(): + assert vlan_group_names( + [ + port("a1-1-1.abc1"), + port("a1-1-2.abc1"), + port("a1-1-1f.abc1"), + port("a1-1-2f.abc1"), + ], + mapping, + ) == { + "a1-1-1.abc1": "a1-1-network", + "a1-1-2.abc1": "a1-1-network", + "a1-1-1f.abc1": "a1-1-storage", + "a1-1-2f.abc1": "a1-1-storage", + } + + +def test_vlan_group_name_pair_cab(): + assert vlan_group_names( + [ + port("a1-1-1.abc1"), + port("a1-2-1.abc1"), + port("a1-1-1f.abc1"), + port("a1-2-1f.abc1"), + ], + mapping, + ) == { + "a1-1-1.abc1": "a1-1/a1-2-network", + "a1-2-1.abc1": "a1-1/a1-2-network", + "a1-1-1f.abc1": "a1-1/a1-2-storage", + "a1-2-1f.abc1": "a1-1/a1-2-storage", + } + + +def test_vlan_group_name_with_domain(): + assert vlan_group_names( + [ + port("a1-1-1.abc1.domain"), + port("a1-1-2.abc1.domain"), + port("a1-1-1f.abc1.domain"), + port("a1-1-2f.abc1.domain"), + ], + mapping, + ) == { + "a1-1-1.abc1.domain": "a1-1-network", + "a1-1-2.abc1.domain": "a1-1-network", + "a1-1-1f.abc1.domain": "a1-1-storage", + "a1-1-2f.abc1.domain": "a1-1-storage", + } + + +def test_vlan_group_name_invalid_format(): + with pytest.raises(ValueError, match="Unknown switch name format"): + vlan_group_names([port("invalid.abc1")], mapping) + + with pytest.raises(ValueError, match="Unknown switch name format"): + vlan_group_names([port(".abc1")], mapping) + + +def test_vlan_group_name_unknown_suffix(): + with pytest.raises(TopologyError, match="suffix a1-1-99.abc1 is not present"): + vlan_group_names([port("a1-1-99.abc1")], mapping) + + with pytest.raises(TopologyError, match="suffix a1-1-5f.abc1 is not present"): + vlan_group_names([port("a1-1-5f.abc1")], mapping) + + with pytest.raises(TopologyError, match="suffix a1-1-xyz.abc1 is not present"): + vlan_group_names([port("a1-1-xyz.abc1")], mapping) + + +def test_vlan_group_name_many_dc(): + with pytest.raises(TopologyError, match="multiple"): + vlan_group_names( + [ + port("a1-1-1.abc1.domain"), + port("a1-1-1.xyz2.domain"), + ], + mapping, + ) + + +def test_vlan_group_name_too_many_racks(): + with pytest.raises(TopologyError, match="more than two racks"): + vlan_group_names( + [ + port("a1-1-1.abc1.domain"), + port("a1-2-1.abc1.domain"), + port("a1-3-1.abc1.domain"), + ], + mapping, + ) + + +def test_vlan_group_name_too_many_switches(): + with pytest.raises(TopologyError, match="exactly two network switches"): + vlan_group_names( + [ + port("a1-1-1.abc1.domain"), + port("a1-1-2.abc1.domain"), + port("a1-1-3.abc1.domain"), + ], + mapping, + ) + + +def test_vlan_group_name_not_enough_switches(): + with pytest.raises(TopologyError, match="exactly two network switches"): + vlan_group_names( + [ + port("a1-1-1.abc1.domain"), + port("a1-1-1.abc1.domain"), + ], + mapping, + ) diff --git a/python/ironic-understack/ironic_understack/update_baremetal_port.py b/python/ironic-understack/ironic_understack/update_baremetal_port.py new file mode 100644 index 000000000..022bdf1eb --- /dev/null +++ b/python/ironic-understack/ironic_understack/update_baremetal_port.py @@ -0,0 +1,207 @@ +from typing import Any + +import openstack +from ironic import objects +from ironic.common import exception +from ironic.drivers.modules.inspector.hooks import base +from oslo_log import log as logging + +import ironic_understack.vlan_group_name_convention +from ironic_understack.conf import CONF +from ironic_understack.inspected_port import InspectedPort +from ironic_understack.ironic_wrapper import ironic_ports_for_node + +LOG = logging.getLogger(__name__) + + +class UpdateBaremetalPortsHook(base.InspectionHook): + """Hook to update ports according to LLDP data.""" + + # "validate-interfaces" provides the all_interfaces field in plugin_data. + # "parse-lldp" provides the parsed_lldp field in plugin_data. + dependencies = ["validate-interfaces", "parse-lldp"] + + def __call__(self, task, inventory, plugin_data): + """Update Ports' local_link_info and physnet based on LLDP data. + + Using the parsed_lldp data as discovered by inspection, validate the + topology and determine the local_link_connection and vlan group names + for each of our connections. + + The ports plugin has already created/deleted the ports as appropriate + and set their "pxe" flag. + + We update attributes of all baremetal ports for this node: + + - local_link_info.port_id (e.g. "Ethernet1/1") + - local_link_info.switch_id (e.g. "aa:bb:cc:dd:ee:ff") + - local_link_info.switch_info (e.g. "a1-1-1.ord1") + - physical_network (e.g. "a1-1-network") + + We also add or remove node "traits" based on the inventory data. We + control the trait "CUSTOM_STORAGE_SWITCH". + """ + node_uuid: str = task.node.uuid + + inspected_ports = _parse_plugin_data(plugin_data) + if not inspected_ports: + LOG.error("No LLDP data for node %s", node_uuid) + return + + ports_by_mac = {p.mac_address: p for p in inspected_ports} + + vlan_groups = ironic_understack.vlan_group_name_convention.vlan_group_names( + inspected_ports, + CONF.ironic_understack.switch_name_vlan_group_mapping, + ) + LOG.debug( + "Node=%(node)s vlan_groups=%(groups)s", + {"node": node_uuid, "groups": vlan_groups}, + ) + + _update_port_attrs(task, ports_by_mac, vlan_groups, node_uuid) + _set_node_traits(task, {x for x in vlan_groups.values() if x}) + + +def _parse_plugin_data(plugin_data: dict) -> list[InspectedPort]: + mac = { + interface["name"]: interface["mac_address"] + for interface in plugin_data["all_interfaces"].values() + } + + return [ + InspectedPort( + mac_address=mac[name], + name=name, + switch_system_name=str(lldp["switch_system_name"]).lower(), + switch_chassis_id=str(lldp["switch_chassis_id"]).lower(), + switch_port_id=str(lldp["switch_port_id"]), + ) + for name, lldp in plugin_data["parsed_lldp"].items() + ] + + +def _update_port_attrs(task, ports_by_mac, vlan_groups, node_uuid): + for baremetal_port in ironic_ports_for_node(task.context, task.node.id): + inspected_port = ports_by_mac.get(baremetal_port.address) + if inspected_port: + LOG.info( + "Port=%(uuid)s Node=%(node)s is connected %(lldp)s", + { + "uuid": baremetal_port.uuid, + "node": node_uuid, + "lldp": inspected_port, + }, + ) + vlan_group = vlan_groups.get(inspected_port.switch_system_name) + if not vlan_group: + LOG.error("Missing VLAN group for %s", inspected_port) + elif vlan_group.endswith("-network"): + _set_port_attributes( + baremetal_port, node_uuid, inspected_port, vlan_group + ) + else: + _clear_port_attributes(baremetal_port, node_uuid) + else: + LOG.info( + "Port=%(uuid)s Node=%(node)s has no LLDP connection", + {"uuid": baremetal_port.uuid, "node": node_uuid}, + ) + _clear_port_attributes(baremetal_port, node_uuid) + + +def _set_port_attributes( + port: Any, node_uuid: str, inspected_port: InspectedPort, physical_network: str +): + try: + if port.local_link_connection != inspected_port.local_link_connection: + LOG.debug( + "Updating node %s port %s local_link_connection %s => %s", + node_uuid, + port.uuid, + port.local_link_connection, + inspected_port.local_link_connection, + ) + port.local_link_connection = inspected_port.local_link_connection + + if port.physical_network != physical_network: + LOG.debug( + "Updating node %s port %s physical_network from %s to %s", + node_uuid, + port.id, + port.physical_network, + physical_network, + ) + port.physical_network = physical_network + + port.save() + except exception.IronicException as e: + LOG.warning( + "Failed to update port %(uuid)s for node %(node)s. Error: %(error)s", + {"uuid": port.id, "node": node_uuid, "error": e}, + ) + + +def _clear_port_attributes(port: Any, node_uuid: str): + try: + port.local_link_connection = {} + port.physical_network = None + port.save() + except exception.IronicException as e: + LOG.warning( + "Failed to clear port %(uuid)s for node %(node)s. Error: %(error)s", + {"uuid": port.id, "node": node_uuid, "error": e}, + ) + + +def _set_node_traits(task, vlan_groups: set[str]): + """Add or remove traits to the node. + + We manage a traits for each type of VLAN Group that can be connected to a + node. + + For example, a connection to VLAN Group whose name ends in "-storage" will + result in a trait being added to the node called "CUSTOM_STORAGE_SWITCH". + + We remove pre-existing traits if the node does not have the required + connections. + """ + node = task.node + all_possible_suffixes = set( + CONF.ironic_understack.switch_name_vlan_group_mapping.values() + ) + our_traits = {_trait_name(x) for x in all_possible_suffixes if x} + required_traits = {_trait_name(x) for x in vlan_groups if x} + existing_traits = set(node.traits.get_trait_names()).intersection(our_traits) + + traits_to_remove = sorted(existing_traits.difference(required_traits)) + traits_to_add = sorted(required_traits.difference(existing_traits)) + + LOG.debug( + "Checking traits for node %s: existing=%s required=%s", + node.uuid, + existing_traits, + required_traits, + ) + + for trait in traits_to_remove: + LOG.debug("Removing trait %s from node %s", trait, node.uuid) + try: + node.traits.destroy(trait) + except openstack.exceptions.NotFoundException: + pass + + if traits_to_add: + LOG.debug("Adding traits %s to node %s", traits_to_add, node.uuid) + + node.traits = objects.TraitList.create( + task.context, node.id, list(traits_to_add) + ) + + if traits_to_add or traits_to_remove: + node.save() + + +def _trait_name(vlan_group_name: str) -> str: + suffix = vlan_group_name.upper().split("-")[-1] + return f"CUSTOM_{suffix}_SWITCH" diff --git a/python/ironic-understack/ironic_understack/vlan_group_name_convention.py b/python/ironic-understack/ironic_understack/vlan_group_name_convention.py new file mode 100644 index 000000000..cf5802fac --- /dev/null +++ b/python/ironic-understack/ironic_understack/vlan_group_name_convention.py @@ -0,0 +1,99 @@ +from collections.abc import Iterable + +from ironic_understack.inspected_port import InspectedPort + + +class TopologyError(Exception): + pass + + +def vlan_group_names( + ports: list[InspectedPort], mapping: dict[str, str] +) -> dict[str, str | None]: + """The VLAN GROUP name is a function of the switch names. + + Given the set of all connections to a single baremetal node, + + Assert that data_center is the same for all switches. + + Assert that the switches are spread across no more than two racks. + + Assert that there are exactly two connections to each "network" switch. + + Use the supplied mapping to categorise the connected switches. + + If both switches are in the same rack, the vlan_group name looks like this: + + ["a11-12-1", "a11-12-2"] => "a11-12-network" + + If those switches are spread across a pair of racks, the VLAN name has both + racks separated by a slash: + + ["a11-12-1", "a11-13-1"] => "a11-12/a11-13-network" + """ + assert_consistent_data_center(ports) + assert_single_or_paired_racks(ports) + assert_switch_names_have_known_suffixes(ports, mapping) + + vlan_groups = group_by_switch_category(ports, mapping) + + assert_redundant_network_connections(vlan_groups) + + vlan_group_names = {} + for switch_category, ports_in_group in vlan_groups.items(): + rack_names = {p.rack_name for p in ports_in_group} + vlan_group_name = "/".join(sorted(rack_names)) + "-" + switch_category + for p in ports_in_group: + vlan_group_names[p.switch_system_name] = vlan_group_name + return vlan_group_names + + +def assert_consistent_data_center(ports: Iterable[InspectedPort]): + data_centers = {p.data_center_name for p in ports} + if len(data_centers) > 1: + raise TopologyError("Connected to switches in multiple data centers: %s", ports) + + +def assert_single_or_paired_racks(ports: Iterable[InspectedPort]): + network_rack_names = {p.rack_name for p in ports} + if len(network_rack_names) > 2: + raise TopologyError("Connected to switches in more than two racks: %s", ports) + + +def assert_switch_names_have_known_suffixes( + ports: Iterable[InspectedPort], mapping: dict +): + for port in ports: + if port.switch_suffix not in mapping: + raise TopologyError( + f"Switch suffix {port.switch_system_name} is not present " + f"in the mapping configured in " + f"ironic_understack.switch_name_vlan_group_mapping. " + f"Recognised suffixes are: {mapping.keys()}" + ) + + +def assert_redundant_network_connections(vlan_groups: dict[str, list[InspectedPort]]): + network_ports = vlan_groups.get("network", []) + switch_names = {p.switch_system_name for p in network_ports} + if len(switch_names) != 2: + raise TopologyError( + "Expected connections to exactly two network switches, got %s", + network_ports, + ) + + +def group_by_switch_category( + ports: list[InspectedPort], mapping: dict[str, str] +) -> dict[str, list[InspectedPort]]: + groups = {} + + for port in ports: + switch_category = mapping[port.switch_suffix] + + if switch_category not in groups: + groups[switch_category] = [] + + groups[switch_category].append(port) + + return groups From bb4bd7a3f0522abc49fe85ce1e2c40b83bffaf57 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 2 Dec 2025 12:58:45 +0000 Subject: [PATCH 5/5] Configure ironic Inspection hooks to manage port and node attributes After out-of-band inspection we run port-bios-name. The interface names returned by the BMC are generally more meaningful to the data center than the linux names we get in the agent inspection data After Agent Inspection we run update-baremetal-port. This consumes the LLDP information, so it must run after agent inspection. Out-of-band inspection doesn't give us the full LLDP data. The default ironic local-link-connection is removed because the new hook updates the same fields. We add "validate-interfaces" because that populates data we consume. --- components/ironic/values.yaml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/components/ironic/values.yaml b/components/ironic/values.yaml index a0935fc8b..2bf379b3c 100644 --- a/components/ironic/values.yaml +++ b/components/ironic/values.yaml @@ -88,11 +88,12 @@ conf: loader_file_paths: "snponly.efi:/usr/lib/ipxe/snponly.efi" inspector: extra_kernel_params: ipa-collect-lldp=1 - # Agent inspection hooks - ports hook removed to prevent port manipulation during agent inspection - # Default hooks include: ramdisk-error,validate-interfaces,ports,architecture - # We override to exclude 'ports' from the default hooks - default_hooks: "ramdisk-error,validate-interfaces,architecture" - hooks: "$default_hooks,pci-devices,parse-lldp,local-link-connection,resource-class" + # Agent inspection hooks run after inspecting in-band using the IPA image: + hooks: "ramdisk-error,validate-interfaces,architecture,pci-devices,validate-interfaces,parse-lldp,resource-class,update-baremetal-port" + redfish: + # Redfish inspection hooks run after inspecting out-of-band using the BMC: + inspection_hooks: "validate-interfaces,ports,port-bios-name,architecture,pci-devices" + add_ports: "all" # enable sensors and metrics for redfish metrics - https://docs.openstack.org/ironic/latest/admin/drivers/redfish/metrics.html sensor_data: send_sensor_data: true