feat: Add a hook to manage PCI device capabilities

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

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)

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}"

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"

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"