From 33bfc1e281bce5c2bcda88433fd79000a8e7b2eb Mon Sep 17 00:00:00 2001 From: Nidhi Rai Date: Mon, 6 Oct 2025 19:16:26 +0530 Subject: [PATCH] Add PCIe function fields to redfish inspection This patch adds support for extracting PCIe function identification fields (device_class, device_id, vendor_id, subsystem_id, subsystem_vendor_id, revision_id) during redfish hardware inspection. The fields are extracted from PCIe functions and stored in a flat structure in the inspection inventory, making them available for inspection rules and hardware identification. Also adds test coverage including edge cases like missing PCIe devices, empty collections, and partial data scenarios. [Removed This changes]Additionally adds system.model field extraction with proper None handling and test coverage. Depends-On: I1ec49e35a53abb8efdae639629cd819ccabbe620 Change-Id: I218c3b3865c07cc2c7fffc21a766cdef36759cd8 Signed-off-by: Nidhi Rai --- ironic/drivers/modules/redfish/inspect.py | 54 ++++++++ .../drivers/modules/redfish/test_inspect.py | 129 ++++++++++++++++++ requirements.txt | 2 +- 3 files changed, 184 insertions(+), 1 deletion(-) diff --git a/ironic/drivers/modules/redfish/inspect.py b/ironic/drivers/modules/redfish/inspect.py index 59c3ce2b0d..3e269a8a9b 100644 --- a/ironic/drivers/modules/redfish/inspect.py +++ b/ironic/drivers/modules/redfish/inspect.py @@ -148,6 +148,10 @@ class RedfishInspect(base.InspectInterface): 'name': eth.identity} inventory['interfaces'].append(iface) + pcie_devices = self._get_pcie_devices(system.pcie_devices) + if pcie_devices: + inventory['pci_devices'] = pcie_devices + system_vendor = {} if system.name: system_vendor['product_name'] = str(system.name) @@ -319,3 +323,53 @@ class RedfishInspect(base.InspectInterface): processor.instruction_set) or '' return cpu + + def _get_pcie_devices(self, pcie_devices_collection): + """Extract PCIe device information from Redfish collection. + + :param pcie_devices_collection: Redfish PCIe devices collection + :returns: List of PCIe device dictionaries + """ + # Return empty list if collection is None + if pcie_devices_collection is None: + return [] + + device_list = [] + + # Process each PCIe device + for pcie_device in pcie_devices_collection.get_members(): + # Skip devices that don't have functions + if (not hasattr(pcie_device, 'pcie_functions') + or not pcie_device.pcie_functions): + continue + + # Process each function on this device + for pcie_function in pcie_device.pcie_functions.get_members(): + function_info = self._extract_function_info(pcie_function) + if function_info: + device_list.append(function_info) + + return device_list + + def _extract_function_info(self, function): + """Extract information from a PCIe function. + + :param function: PCIe function object + :returns: Dictionary with function attributes + """ + info = {} + # Naming them same as in IPA for compatibility + # IPA has extra bus and numa_node_id which BMC doesn't have. + if function.device_class is not None: + info['class'] = str(function.device_class) + if function.device_id is not None: + info['product_id'] = function.device_id + if function.vendor_id is not None: + info['vendor_id'] = function.vendor_id + if function.subsystem_id is not None: + info['subsystem_id'] = function.subsystem_id + if function.subsystem_vendor_id is not None: + info['subsystem_vendor_id'] = function.subsystem_vendor_id + if function.revision_id is not None: + info['revision'] = function.revision_id + return info diff --git a/ironic/tests/unit/drivers/modules/redfish/test_inspect.py b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py index c81fb3934c..b98096091b 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py @@ -133,6 +133,36 @@ class RedfishInspectTestCase(db_base.DbTestCase): } ) + # PCIe device mocks + mock_pcie_function1 = mock.Mock() + mock_pcie_function1.device_class = 'NetworkController' + mock_pcie_function1.device_id = '0x16d7' + mock_pcie_function1.vendor_id = '0x14e4' + mock_pcie_function1.subsystem_id = '0x1402' + mock_pcie_function1.subsystem_vendor_id = '0x14e4' + mock_pcie_function1.revision_id = '0x01' + + mock_pcie_function2 = mock.Mock() + mock_pcie_function2.device_class = 'MassStorageController' + mock_pcie_function2.device_id = '0x1234' + mock_pcie_function2.vendor_id = '0x1000' + mock_pcie_function2.subsystem_id = None + mock_pcie_function2.subsystem_vendor_id = None + mock_pcie_function2.revision_id = '0x02' + + mock_pcie_device1 = mock.Mock() + mock_pcie_device1.pcie_functions.get_members.return_value = [ + mock_pcie_function1] + + mock_pcie_device2 = mock.Mock() + mock_pcie_device2.pcie_functions.get_members.return_value = [ + mock_pcie_function2] + + system_mock.pcie_devices.get_members.return_value = [ + mock_pcie_device1, + mock_pcie_device2 + ] + system_mock.name = 'System1' system_mock.serial_number = '123456' @@ -143,6 +173,8 @@ class RedfishInspectTestCase(db_base.DbTestCase): system_mock.uuid = '12345678-1234-1234-1234-12345' + system_mock.model = 'PowerEdge R1234' + return system_mock def test_get_properties(self): @@ -218,6 +250,21 @@ class RedfishInspectTestCase(db_base.DbTestCase): self.assertEqual(expected_disks, inventory["inventory"]['disks']) + expected_pcie_devices = [ + {'class': 'NetworkController', + 'product_id': '0x16d7', + 'vendor_id': '0x14e4', + 'subsystem_id': '0x1402', + 'subsystem_vendor_id': '0x14e4', + 'revision': '0x01'}, + {'class': 'MassStorageController', + 'product_id': '0x1234', + 'vendor_id': '0x1000', + 'revision': '0x02'} + ] + self.assertEqual(expected_pcie_devices, + inventory['inventory']['pci_devices']) + @mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', @@ -712,6 +759,7 @@ class RedfishInspectTestCase(db_base.DbTestCase): system_mock.manufacturer = None system_mock.sku = None system_mock.uuid = None + system_mock.model = None with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -729,6 +777,87 @@ class RedfishInspectTestCase(db_base.DbTestCase): self.assertEqual(expected_properties, task.driver.inspect._get_pxe_port_macs(task)) + @mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_inspect_hardware_ignore_missing_pcie_devices( + self, mock_get_system, mock_get_enabled_macs): + system_mock = self.init_system_mock(mock_get_system.return_value) + system_mock.pcie_devices = None + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.inspect.inspect_hardware(task) + + inventory = inspect_utils.get_inspection_data(self.node, + self.context) + self.assertNotIn('pci_devices', inventory['inventory']) + + @mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_inspect_hardware_ignore_empty_pcie_devices( + self, mock_get_system, mock_get_enabled_macs): + system_mock = self.init_system_mock(mock_get_system.return_value) + system_mock.pcie_devices.get_members.return_value = [] + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.inspect.inspect_hardware(task) + + inventory = inspect_utils.get_inspection_data(self.node, + self.context) + self.assertNotIn('pci_devices', inventory['inventory']) + + @mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_inspect_hardware_ignore_pcie_device_without_functions( + self, mock_get_system, mock_get_enabled_macs): + system_mock = self.init_system_mock(mock_get_system.return_value) + mock_pcie_device = mock.Mock() + mock_pcie_device.pcie_functions = None + system_mock.pcie_devices.get_members.return_value = [mock_pcie_device] + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.inspect.inspect_hardware(task) + + inventory = inspect_utils.get_inspection_data(self.node, + self.context) + self.assertNotIn('pci_devices', inventory['inventory']) + + @mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_inspect_hardware_pcie_partial_data( + self, mock_get_system, mock_get_enabled_macs): + system_mock = self.init_system_mock(mock_get_system.return_value) + # Create a PCIe function with partial data, some fields None + mock_pcie_function = mock.Mock() + mock_pcie_function.device_class = 'NetworkController' + mock_pcie_function.device_id = '0x16d7' + mock_pcie_function.vendor_id = None + mock_pcie_function.subsystem_id = '0x1402' + mock_pcie_function.subsystem_vendor_id = None + mock_pcie_function.revision_id = '0x01' + + mock_pcie_device = mock.Mock() + mock_pcie_device.pcie_functions.get_members.return_value = [ + mock_pcie_function] + system_mock.pcie_devices.get_members.return_value = [mock_pcie_device] + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.driver.inspect.inspect_hardware(task) + + inventory = inspect_utils.get_inspection_data(self.node, + self.context) + expected_pcie_devices = [ + {'class': 'NetworkController', + 'product_id': '0x16d7', + 'subsystem_id': '0x1402', + 'revision': '0x01'} + ] + self.assertEqual(expected_pcie_devices, + inventory['inventory']['pci_devices']) + class ContinueInspectionTestCase(db_base.DbTestCase): def setUp(self): diff --git a/requirements.txt b/requirements.txt index c4ad3e855b..b838104fb5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -37,7 +37,7 @@ psutil>=3.2.2 # BSD futurist>=3.2.0 # Apache-2.0 tooz>=2.7.0 # Apache-2.0 openstacksdk>=0.99.0 # Apache-2.0 -sushy>=5.7.0 +sushy>=5.9.0 construct>=2.9.39 # MIT netaddr>=0.9.0 # BSD microversion-parse>=1.0.1 # Apache-2.0