openstack/ironic
Code Issues Proposed changes

fix redfish processor inspection

Browse Source 
The processor inspection tests actually are looking at the instruction
set value and expecting it to be x86_64 while the processor architecture
value is x86 which we map to i686. The current tests confuse this by
mixing the two different return values from the get_members() mock and
the summary mock so it doesn't break things but later usage of redfish
inspection breaks the existing hook tests so fix up the mocks to be
consistent. The _get_processor_info() function additionally operates on
an out-parameter which is a bit awkward in Python so use the more
idiomatic return value to set the expected data. Lastly the cpus field
on the node properties is not a typical field populated by other
inspection methods so drop it entirely. Then match the behavior of
ironic-inspector and ironic-python-agent by ensuring the architecture
and count fields are always set in the cpu inventory data.

Change-Id: Id99d6948f8bef73302281e84411b2263716a278f
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
This commit is contained in:
Doug Goldstein
2025-07-21 15:46:05 -05:00
parent 382a9f5c80
commit 5a8312fefa
3 changed files with 64 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
ironic/drivers/modules/redfish/inspect.py
View File

@@ -106,7 +106,16 @@ class RedfishInspect(base.InspectInterface):
inspected_properties['memory_mb'] = memory
inventory['memory'] = {'physical_mb': memory}
self._get_processor_info(task, system, inspected_properties, inventory)
# match the inventory data of ironic-inspector / ironic-python-agent
# to make existing inspection hooks and rules work by defaulting
# the values
inventory['cpu'] = {
'count': 0,
'architecture': '',
}
proc_info = self._get_processor_info(task, system,
inspected_properties)
inventory['cpu'].update(proc_info)
# TODO(etingof): should we respect root device hints here?
local_gb = self._detect_local_gb(task, system)
@@ -296,20 +305,17 @@ class RedfishInspect(base.InspectInterface):
"""
return None
def _get_processor_info(self, task, system, inspected_properties,
inventory):
def _get_processor_info(self, task, system, inspected_properties):
# NOTE(JayF): Checking truthiness here is better than checking for None
# because if we have an empty list, we'll raise a
# ValueError.
if not system.processors:
return
cpu = {}
if not system.processors:
return cpu
if system.processors.summary:
cpus, arch = system.processors.summary
if cpus:
inspected_properties['cpus'] = cpus
cpu['count'] = cpus
cpu['count'], arch = system.processors.summary
if arch:
try:
inspected_properties['cpu_arch'] = CPU_ARCH_MAP[arch]
@@ -324,8 +330,7 @@ class RedfishInspect(base.InspectInterface):
cpu['model_name'] = str(processor.model)
if processor.max_speed_mhz is not None:
cpu['frequency'] = processor.max_speed_mhz
if processor.instruction_set is not None:
cpu['architecture'] = PROCESSOR_INSTRUCTION_SET_MAP[
processor.instruction_set]
cpu['architecture'] = PROCESSOR_INSTRUCTION_SET_MAP.get(
processor.instruction_set) or ''
inventory['cpu'] = cpu
return cpu

57
ironic/tests/unit/drivers/modules/redfish/test_inspect.py
View File

@@ -56,13 +56,24 @@ class RedfishInspectTestCase(db_base.DbTestCase):
system_mock.memory_summary.size_gib = 2
mock_processor = mock.Mock()
mock_processor = mock.Mock(
spec=sushy.resources.system.processor.Processor)
mock_processor.model = 'test'
mock_processor.instruction_set = sushy.InstructionSet.X86
mock_processor.processor_architecture = sushy.PROCESSOR_ARCH_x86
mock_processor.instruction_set = sushy.InstructionSet.X86_64
mock_processor.max_speed_mhz = 1234
mock_processor.total_threads = 8
system_mock.processors.get_members.return_value = [mock_processor]
system_mock.processors.summary = '8', sushy.PROCESSOR_ARCH_x86
# make the summary follow the data above by making it
# a property like sushy and returning the same data
type(system_mock.processors).summary = mock.PropertyMock(
side_effect=lambda:
sushy.resources.system.processor.ProcessorSummary(
count=mock_processor.total_threads,
architecture=mock_processor.processor_architecture
)
)
mock_storage_drive = mock.Mock(
spec=sushy.resources.system.storage.drive.Drive)
@@ -151,7 +162,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
mock_get_enabled_macs):
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': 2048,
}
self.init_system_mock(mock_get_system.return_value)
@@ -184,8 +195,8 @@ class RedfishInspectTestCase(db_base.DbTestCase):
self.assertEqual(expected_interfaces,
inventory['inventory']['interfaces'])
expected_cpu = {'count': '8', 'model_name': 'test',
'frequency': 1234, 'architecture': 'i686'}
expected_cpu = {'count': 8, 'model_name': 'test',
'frequency': 1234, 'architecture': 'x86_64'}
self.assertEqual(expected_cpu,
inventory['inventory']['cpu'])
@@ -212,7 +223,8 @@ class RedfishInspectTestCase(db_base.DbTestCase):
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
def test_inspect_hardware_fail_missing_cpu_arch(self, mock_get_system):
system_mock = self.init_system_mock(mock_get_system.return_value)
system_mock.processors.summary = None, None
mock_processor = system_mock.processors.get_members.return_value[0]
mock_processor.processor_architecture = None
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
@@ -225,7 +237,8 @@ class RedfishInspectTestCase(db_base.DbTestCase):
def test_inspect_hardware_ignore_missing_cpu_count(self, mock_get_system,
mock_get_enabled_macs):
system_mock = self.init_system_mock(mock_get_system.return_value)
system_mock.processors.summary = None, None
mock_processor = system_mock.processors.get_members.return_value[0]
mock_processor.total_threads = 0
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
@@ -238,7 +251,8 @@ class RedfishInspectTestCase(db_base.DbTestCase):
inventory = inspect_utils.get_inspection_data(task.node,
self.context)
self.assertNotIn('count', inventory['inventory']['cpu'])
self.assertIn('count', inventory['inventory']['cpu'])
self.assertEqual(0, inventory['inventory']['cpu']['count'])
@mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
@@ -252,7 +266,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)
@@ -274,7 +288,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)
@@ -296,7 +310,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)
@@ -304,7 +318,8 @@ class RedfishInspectTestCase(db_base.DbTestCase):
inventory = inspect_utils.get_inspection_data(task.node,
self.context)
self.assertNotIn('architecture', inventory['inventory']['cpu'])
self.assertIn('architecture', inventory['inventory']['cpu'])
self.assertEqual('', inventory['inventory']['cpu']['architecture'])
@mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
@@ -318,7 +333,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '0', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)
@@ -336,7 +351,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '0', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)
@@ -357,7 +372,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)
@@ -378,7 +393,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '4', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)
@@ -410,7 +425,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
shared=True) as task:
expected_properties = {
'capabilities': 'boot_mode:uefi',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': '4096'
}
task.driver.inspect.inspect_hardware(task)
@@ -449,7 +464,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
task.driver.inspect.inspect_hardware(task)
inventory = inspect_utils.get_inspection_data(task.node,
self.context)
self.assertNotIn('cpu', inventory['inventory'])
self.assertIn('cpu', inventory['inventory'])
@mock.patch.object(redfish_utils, 'get_enabled_macs', autospec=True)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
@@ -464,7 +479,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
}
expected_properties = {
'capabilities': 'boot_mode:bios',
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': 2048
}
@@ -487,7 +502,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
expected_properties = {
'cpu_arch': 'x86_64', 'cpus': '8',
'cpu_arch': 'x86_64',
'local_gb': '3', 'memory_mb': 2048
}
task.driver.inspect.inspect_hardware(task)

9
releasenotes/notes/redfish-inspect-processor-2467118e869a2915.yaml Normal file
View File

@@ -0,0 +1,9 @@
---
upgrade:
- |
When using the redfish inspection method, the node property ``cpus`` will no
longer be populated. No other inspection method populated this field.
fixes:
- |
When using the redfish inspection method, the node property ``cpu_arch`` is
now populated correctly with ``i686`` or ``x86_64`` for the processor.