diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index 5a94d1564d..6839e38c6f 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -51,6 +51,16 @@ if sushy: } BOOT_DEVICE_MAP_REV = {v: k for k, v in BOOT_DEVICE_MAP.items()} + # Previously we used sushy constants in driver_internal_info. This mapping + # is provided for backward compatibility, taking into account that sushy + # constants will change from strings to enums. + BOOT_DEVICE_MAP_REV_COMPAT = dict( + BOOT_DEVICE_MAP_REV, + pxe=sushy.BOOT_SOURCE_TARGET_PXE, + hdd=sushy.BOOT_SOURCE_TARGET_HDD, + cd=sushy.BOOT_SOURCE_TARGET_CD, + **{'bios setup': sushy.BOOT_SOURCE_TARGET_BIOS_SETUP} + ) BOOT_MODE_MAP = { sushy.BOOT_SOURCE_MODE_UEFI: boot_modes.UEFI, @@ -109,7 +119,6 @@ def _set_boot_device(task, system, device, persistent=False): # NOTE(etingof): this can be racy, esp if BMC is not RESTful enabled = (desired_enabled if desired_enabled != current_enabled else None) - try: system.set_system_boot_options(device, enabled=enabled) except sushy.exceptions.SushyError as e: @@ -131,7 +140,7 @@ def _set_boot_device(task, system, device, persistent=False): {'dev': device, 'node': task.node.uuid}) utils.set_node_nested_field( task.node, 'driver_internal_info', - 'redfish_boot_device', device) + 'redfish_boot_device', BOOT_DEVICE_MAP[device]) task.node.save() else: raise @@ -193,10 +202,20 @@ class RedfishManagement(base.ManagementInterface): if not device: return + try: + # We used to store Redfish constants, now we're storing Ironic + # values (which is more appropriate). Provide a compatibility layer + # for already deployed nodes. + redfish_device = BOOT_DEVICE_MAP_REV_COMPAT[device.lower()] + except KeyError: + LOG.error('BUG: unexpected redfish_boot_device %(dev)s for node ' + '%(node)s', {'dev': device, 'node': task.node.uuid}) + raise + LOG.debug('Restoring boot device %(dev)s on node %(node)s', {'dev': device, 'node': task.node.uuid}) try: - _set_boot_device(task, system, device) + _set_boot_device(task, system, redfish_device) except sushy.exceptions.SushyError as e: LOG.warning('Unable to recover boot device %(dev)s for node ' '%(node)s, relying on the pre-configured boot order. ' diff --git a/ironic/tests/unit/drivers/modules/redfish/test_inspect.py b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py index 352b1d4e98..dc1ffff4d5 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_inspect.py @@ -54,11 +54,11 @@ class RedfishInspectTestCase(db_base.DbTestCase): system_mock.reset() - system_mock.boot.mode = 'uefi' + system_mock.boot.mode = sushy.BOOT_SOURCE_MODE_UEFI system_mock.memory_summary.size_gib = 2 - system_mock.processors.summary = '8', 'MIPS' + system_mock.processors.summary = '8', sushy.PROCESSOR_ARCH_MIPS system_mock.simple_storage.disks_sizes_bytes = ( 1 * units.Gi, units.Gi * 3, units.Gi * 5) @@ -124,7 +124,6 @@ class RedfishInspectTestCase(db_base.DbTestCase): def test_inspect_hardware_fail_missing_cpu(self, mock_get_system): system_mock = self.init_system_mock(mock_get_system.return_value) system_mock.processors.summary = None, None - system_mock.boot.mode = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -136,7 +135,6 @@ class RedfishInspectTestCase(db_base.DbTestCase): def test_inspect_hardware_ignore_missing_cpu(self, mock_get_system): system_mock = self.init_system_mock(mock_get_system.return_value) system_mock.processors.summary = None, None - system_mock.boot.mode = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -153,7 +151,6 @@ class RedfishInspectTestCase(db_base.DbTestCase): system_mock = self.init_system_mock(mock_get_system.return_value) system_mock.simple_storage.disks_sizes_bytes = None system_mock.storage.volumes_sizes_bytes = None - system_mock.boot.mode = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -169,7 +166,6 @@ class RedfishInspectTestCase(db_base.DbTestCase): def test_inspect_hardware_fail_missing_memory_mb(self, mock_get_system): system_mock = self.init_system_mock(mock_get_system.return_value) system_mock.memory_summary.size_gib = None - system_mock.boot.mode = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -181,7 +177,6 @@ class RedfishInspectTestCase(db_base.DbTestCase): def test_inspect_hardware_ignore_missing_memory_mb(self, mock_get_system): system_mock = self.init_system_mock(mock_get_system.return_value) system_mock.memory_summary.size_gib = None - system_mock.boot.mode = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -200,7 +195,6 @@ class RedfishInspectTestCase(db_base.DbTestCase): self, mock_create_ports_if_not_exist, mock_get_system): system_mock = self.init_system_mock(mock_get_system.return_value) system_mock.ethernet_interfaces.summary = None - system_mock.boot.mode = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -209,8 +203,7 @@ class RedfishInspectTestCase(db_base.DbTestCase): @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_inspect_hardware_preserve_boot_mode(self, mock_get_system): - system_mock = self.init_system_mock(mock_get_system.return_value) - system_mock.boot.mode = 'uefi' + self.init_system_mock(mock_get_system.return_value) with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index f67efa9f07..738bd8802b 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -263,7 +263,7 @@ class RedfishManagementTestCase(db_base.DbTestCase): task.node.refresh() self.assertEqual( - sushy.BOOT_SOURCE_TARGET_PXE, + boot_devices.PXE, task.node.driver_internal_info['redfish_boot_device']) @mock.patch.object(redfish_utils, 'get_system', autospec=True) @@ -305,7 +305,7 @@ class RedfishManagementTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.driver_internal_info['redfish_boot_device'] = ( - sushy.BOOT_SOURCE_TARGET_HDD + boot_devices.DISK ) task.driver.management.restore_boot_device(task, fake_system) @@ -315,7 +315,24 @@ class RedfishManagementTestCase(db_base.DbTestCase): enabled=sushy.BOOT_SOURCE_ENABLED_ONCE) # The stored boot device is kept intact self.assertEqual( + boot_devices.DISK, + task.node.driver_internal_info['redfish_boot_device']) + + def test_restore_boot_device_compat(self): + fake_system = mock.Mock() + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # Previously we used sushy constants + task.node.driver_internal_info['redfish_boot_device'] = "hdd" + + task.driver.management.restore_boot_device(task, fake_system) + + fake_system.set_system_boot_options.assert_called_once_with( sushy.BOOT_SOURCE_TARGET_HDD, + enabled=sushy.BOOT_SOURCE_ENABLED_ONCE) + # The stored boot device is kept intact + self.assertEqual( + "hdd", task.node.driver_internal_info['redfish_boot_device']) def test_restore_boot_device_noop(self): @@ -335,7 +352,7 @@ class RedfishManagementTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.driver_internal_info['redfish_boot_device'] = ( - sushy.BOOT_SOURCE_TARGET_HDD + boot_devices.DISK ) task.driver.management.restore_boot_device(task, fake_system) @@ -346,7 +363,7 @@ class RedfishManagementTestCase(db_base.DbTestCase): self.assertTrue(mock_log.called) # The stored boot device is kept intact self.assertEqual( - sushy.BOOT_SOURCE_TARGET_HDD, + boot_devices.DISK, task.node.driver_internal_info['redfish_boot_device']) @mock.patch.object(redfish_utils, 'get_system', autospec=True) @@ -394,7 +411,7 @@ class RedfishManagementTestCase(db_base.DbTestCase): # Asserts fake_system.set_system_boot_options.assert_called_once_with( - mode=mode) + mode=expected) mock_get_system.assert_called_once_with(task.node) # Reset mocks @@ -419,7 +436,7 @@ class RedfishManagementTestCase(db_base.DbTestCase): exception.RedfishError, 'Setting boot mode', task.driver.management.set_boot_mode, task, boot_modes.UEFI) fake_system.set_system_boot_options.assert_called_once_with( - mode=boot_modes.UEFI) + mode=sushy.BOOT_SOURCE_MODE_UEFI) mock_get_system.assert_called_once_with(task.node) @mock.patch.object(sushy, 'Sushy', autospec=True) @@ -440,7 +457,7 @@ class RedfishManagementTestCase(db_base.DbTestCase): 'does not support set_boot_mode', task.driver.management.set_boot_mode, task, boot_modes.UEFI) fake_system.set_system_boot_options.assert_called_once_with( - mode=boot_modes.UEFI) + mode=sushy.BOOT_SOURCE_MODE_UEFI) mock_get_system.assert_called_once_with(task.node) @mock.patch.object(redfish_utils, 'get_system', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_raid.py b/ironic/tests/unit/drivers/modules/redfish/test_raid.py index 59178570ad..db2d825e3b 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_raid.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_raid.py @@ -90,12 +90,12 @@ class RedfishRAIDTestCase(db_base.DbTestCase): mock_drives.append(_mock_drive( identity=i, block_size_bytes=512, capacity_bytes=899527000000, media_type='HDD', name='Drive', - protocol='Serial Attached SCSI')) + protocol=sushy.PROTOCOL_TYPE_SAS)) for i in [self.drive_id5, self.drive_id6, self.drive_id7]: mock_drives.append(_mock_drive( identity=i, block_size_bytes=512, capacity_bytes=479559942144, media_type='SSD', name='Solid State Drive', - protocol='Serial AT Attachment')) + protocol=sushy.PROTOCOL_TYPE_SATA)) self.mock_storage.drives = mock_drives mock_controller = mock.Mock() mock_controller.raid_types = ['RAID1', 'RAID5', 'RAID10']