From 98604b356d98f27f8695f3a5952f1488e713b108 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Mon, 26 Oct 2026 16:06:16 +0000 Subject: [PATCH 1/2] Add a hook to manage PCI device capabilities This is similar to the ironic built-in pci-device hook except that it prefixes the capability names, which allows it to clean up old capabilities. Also, this looks for PCI device data in inventory, whereas the ironic hook looks for it in plugin_data. --- .../ironic_understack/conf/__init__.py | 5 + .../inspect_hook_pci_device_capabilities.py | 105 +++++++++ ...st_inspect_hook_pci_device_capabilities.py | 220 ++++++++++++++++++ 3 files changed, 330 insertions(+) create mode 100644 python/ironic-understack/ironic_understack/hooks/inspect_hook_pci_device_capabilities.py create mode 100644 python/ironic-understack/ironic_understack/tests/test_inspect_hook_pci_device_capabilities.py diff --git a/python/ironic-understack/ironic_understack/conf/__init__.py b/python/ironic-understack/ironic_understack/conf/__init__.py index b8eba00fb..8c90a0404 100644 --- a/python/ironic-understack/ironic_understack/conf/__init__.py +++ b/python/ironic-understack/ironic_understack/conf/__init__.py @@ -26,6 +26,11 @@ def setup_conf(): "1d": "bmc", }, ), + cfg.DictOpt( + "pci_device_capabilities", + help="Dictionary of pci device IDs (xxxx:xxxx) to capability name", + default={}, + ), ] cfg.CONF.register_group(grp) cfg.CONF.register_opts(opts, group=grp) diff --git a/python/ironic-understack/ironic_understack/hooks/inspect_hook_pci_device_capabilities.py b/python/ironic-understack/ironic_understack/hooks/inspect_hook_pci_device_capabilities.py new file mode 100644 index 000000000..64220b4d8 --- /dev/null +++ b/python/ironic-understack/ironic_understack/hooks/inspect_hook_pci_device_capabilities.py @@ -0,0 +1,105 @@ +import collections + +from ironic.drivers.modules.inspector.hooks import base +from oslo_log import log as logging + +from ironic_understack.conf import CONF + +LOG = logging.getLogger(__name__) + +CAP_PREFIX = "pci_device_" + + +class PciDeviceCapabilitiesHook(base.InspectionHook): + """Control a node's PCI device capabilities based on PCI device IDs. + + When this hook is configured with config in the config file that looks like: + + ironic_understack: + pci_device_capabilities: + 1002:75a8: "amd_gpu_mi350p" + 8086:2002: "intel_duperkard_2" + + Upon inspection, a `capability` will be added to the node for each matching + PCI device that is present in the inventory. + + Given the example above, if the node has a PCI card with vendor_id=0x1002 + product_id=0x75a8 then this hook will add a capability of + "pci_device_amd_gpu_mi350p:1". + + The number is a count of those cards. If two such cards are present in the + server then the capability would read "pci_device_amd_gpu_mi350p:2". + + All capabilities starting with "pci_device_" are owned by this hook. This + hook will clean up capabilities for PCI devices that are missing from + inventory (or from the configuration!). Capabilities that don't start with + "pci_device_" are left untouched. + """ + + def __call__(self, task, inventory, plugin_data): + """Set node capabilities for each applicable PCI device.""" + capability_names: dict[str, str] = ( + CONF.ironic_understack.pci_device_capabilities + ) + + if capability_names is None: + LOG.debug("ironic_understack.pci_device_capabilities not configured.") + return + + pci_devices = inventory.get("pci_devices") + if not pci_devices: + LOG.warning("No PCI device data is present in inventory") + return + + current_capabilities = task.node.properties.get("capabilities", "") + current_capabilities = ( + current_capabilities.split(",") if current_capabilities else [] + ) + + required_capabilities = pci_device_capabilities(pci_devices, capability_names) + + new_capabilities = [ + k + for k in current_capabilities + if k in required_capabilities or not k.startswith(CAP_PREFIX) + ] + [k for k in required_capabilities if k not in current_capabilities] + + task.node.set_property("capabilities", ",".join(new_capabilities)) + task.node.save() + + +def pci_device_capabilities( + pci_devices: list[dict[str, str]], + capability_names: dict[str, str], +) -> set[str]: + """Return capability strings such as ``pci_device_acme_coolkard:3``. + + This has the configured device name along with the number of such devices in + this box. We only include devices that appear in the capability_names + dict. + + pci_devices is expected to be a list of devices like: + + [{"vendor_id": "0x1002", "product_id": "0x75a8", ...}, ...] + + capability_names is a dictionary mapping device ids to their names, like: + + {"1002:75a8": "amd_gpu_mi350p", ...} + """ + pci_device_number_slugs = [pci_device_slug(dev) for dev in pci_devices] + pci_device_names = [ + capability_names[key] + for key in pci_device_number_slugs + if key in capability_names + ] + return { + f"{CAP_PREFIX}{name}:{count}" + for name, count in collections.Counter(pci_device_names).items() + } + + +def pci_device_slug(pci_device: dict[str, str]) -> str: + """Return device ID in a format like "8086:4002".""" + vendor_id = pci_device["vendor_id"].removeprefix("0x") + product_id = pci_device["product_id"].removeprefix("0x") + return f"{vendor_id}:{product_id}" diff --git a/python/ironic-understack/ironic_understack/tests/test_inspect_hook_pci_device_capabilities.py b/python/ironic-understack/ironic_understack/tests/test_inspect_hook_pci_device_capabilities.py new file mode 100644 index 000000000..ef6c40fea --- /dev/null +++ b/python/ironic-understack/ironic_understack/tests/test_inspect_hook_pci_device_capabilities.py @@ -0,0 +1,220 @@ +from ironic_understack.hooks.inspect_hook_pci_device_capabilities import ( + PciDeviceCapabilitiesHook, +) +from ironic_understack.hooks.inspect_hook_pci_device_capabilities import ( + pci_device_capabilities, +) +from ironic_understack.hooks.inspect_hook_pci_device_capabilities import pci_device_slug + + +def test_adds_pci_capability_to_node_without_capabilities(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + {"1002:75a8": "amd_gpu_mi350p"}, + ) + inventory = { + "pci_devices": [ + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + ] + } + node = mocker.Mock(properties={}) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_called_once_with( + "capabilities", "pci_device_amd_gpu_mi350p:2" + ) + node.save.assert_called_once_with() + + +def test_preserves_non_pci_capabilities(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + {"1002:75a8": "amd_gpu_mi350p"}, + ) + inventory = { + "pci_devices": [ + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + ] + } + node = mocker.Mock(properties={"capabilities": "boot_mode:uefi,profile:compute"}) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_called_once_with( + "capabilities", + "boot_mode:uefi,profile:compute,pci_device_amd_gpu_mi350p:2", + ) + node.save.assert_called_once_with() + + +def test_replaces_stale_pci_capability(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + {"1002:75a8": "amd_gpu_mi350p"}, + ) + inventory = { + "pci_devices": [ + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + ] + } + node = mocker.Mock( + properties={"capabilities": "boot_mode:uefi,pci_device_amd_gpu_mi350p:1"} + ) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_called_once_with( + "capabilities", "boot_mode:uefi,pci_device_amd_gpu_mi350p:2" + ) + node.save.assert_called_once_with() + + +def test_does_not_duplicate_current_pci_capability(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + {"1002:75a8": "amd_gpu_mi350p"}, + ) + inventory = { + "pci_devices": [ + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + ] + } + node = mocker.Mock(properties={"capabilities": "pci_device_amd_gpu_mi350p:2"}) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_called_once_with( + "capabilities", "pci_device_amd_gpu_mi350p:2" + ) + node.save.assert_called_once_with() + + +def test_removes_pci_capability_when_device_is_no_longer_present(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + {"1002:75a8": "amd_gpu_mi350p"}, + ) + inventory = { + "pci_devices": [ + {"vendor_id": "0x1022", "product_id": "0x7901"}, + ] + } + node = mocker.Mock( + properties={"capabilities": "boot_mode:uefi,pci_device_amd_gpu_mi350p:2"} + ) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_called_once_with("capabilities", "boot_mode:uefi") + node.save.assert_called_once_with() + + +def test_does_nothing_when_pci_capabilities_are_not_configured(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + None, + ) + inventory = { + "pci_devices": [ + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + ] + } + node = mocker.Mock(properties={}) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_not_called() + node.save.assert_not_called() + + +def test_does_nothing_when_inventory_has_no_pci_devices(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + {"1002:75a8": "amd_gpu_mi350p"}, + ) + inventory = {} + node = mocker.Mock(properties={}) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_not_called() + node.save.assert_not_called() + + +def test_does_nothing_when_pci_device_list_is_empty(mocker): + mocker.patch( + "ironic_understack.hooks.inspect_hook_pci_device_capabilities." + "CONF.ironic_understack.pci_device_capabilities", + {"1002:75a8": "amd_gpu_mi350p"}, + ) + inventory = {"pci_devices": []} + node = mocker.Mock(properties={}) + task = mocker.Mock(node=node) + + PciDeviceCapabilitiesHook()(task, inventory, {}) + + node.set_property.assert_not_called() + node.save.assert_not_called() + + +def test_counts_configured_pci_devices_and_ignores_other_devices(): + pci_devices = [ + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + {"vendor_id": "0x1022", "product_id": "0x7901"}, + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + ] + capability_names = {"1002:75a8": "amd_gpu_mi350p"} + + capabilities = pci_device_capabilities(pci_devices, capability_names) + + assert capabilities == {"pci_device_amd_gpu_mi350p:2"} + + +def test_counts_device_ids_with_the_same_configured_name_together(): + pci_devices = [ + {"vendor_id": "0x1002", "product_id": "0x75a8"}, + {"vendor_id": "0x1002", "product_id": "0x75b0"}, + ] + capability_names = { + "1002:75a8": "amd_gpu", + "1002:75b0": "amd_gpu", + } + + capabilities = pci_device_capabilities(pci_devices, capability_names) + + assert capabilities == {"pci_device_amd_gpu:2"} + + +def test_pci_device_slug_removes_hexadecimal_prefixes(): + pci_device = {"vendor_id": "0x1002", "product_id": "0x75a8"} + + slug = pci_device_slug(pci_device) + + assert slug == "1002:75a8" + + +def test_pci_device_slug_accepts_ids_without_hexadecimal_prefixes(): + pci_device = {"vendor_id": "1002", "product_id": "75a8"} + + slug = pci_device_slug(pci_device) + + assert slug == "1002:75a8" From 39688ab3bf43eb60071eff2c42e9f40bccb60d6a Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Mon, 26 Oct 2026 17:53:35 +0000 Subject: [PATCH 2/2] ironic: enable the pci-device-capabilities hook to annotate nodes --- components/ironic/values.yaml | 6 +++++- python/ironic-understack/pyproject.toml | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/components/ironic/values.yaml b/components/ironic/values.yaml index c28e48562..50ee4edc8 100644 --- a/components/ironic/values.yaml +++ b/components/ironic/values.yaml @@ -127,9 +127,13 @@ conf: # Agent inspection hooks run after inspecting in-band using the IPA image: hooks: "ramdisk-error,validate-interfaces,architecture,pci-devices,parse-lldp,update-baremetal-port,chassis_model" keep_ports: "present" + pci_device_alias: + type: multistring + values: + - '{"vendor_id": "0x1002", "product_id": "0x75a8", "name": "gpu_amd_mi350p"}' redfish: # Redfish inspection hooks run after inspecting out-of-band using the BMC: - inspection_hooks: "validate-interfaces,ports,port-bios-name,architecture,pci-devices,resource-class,chassis_model" + inspection_hooks: "validate-interfaces,ports,port-bios-name,architecture,pci-devices,resource-class,chassis_model,pci-device-capabilities" # enable sensors and metrics for redfish metrics - https://docs.openstack.org/ironic/latest/admin/drivers/redfish/metrics.html sensor_data: send_sensor_data: true diff --git a/python/ironic-understack/pyproject.toml b/python/ironic-understack/pyproject.toml index 2ffb93e82..67ac70d4b 100644 --- a/python/ironic-understack/pyproject.toml +++ b/python/ironic-understack/pyproject.toml @@ -26,6 +26,7 @@ update-baremetal-port = "ironic_understack.hooks.inspect_hook_update_baremetal_p port-bios-name = "ironic_understack.hooks.port_bios_name_hook:PortBiosNameHook" node-name-check = "ironic_understack.hooks.inspect_hook_node_name_check:InspectHookNodeNameCheck" chassis_model = "ironic_understack.hooks.inspect_hook_chassis_model:InspectHookChassisModel" +pci-device-capabilities = "ironic_understack.hooks.inspect_hook_pci_device_capabilities:PciDeviceCapabilitiesHook" [project.entry-points."ironic.hardware.types"] netdev = "ironic_understack.drivers.netdev_hardware:NetdevHardware"