Stop relying on explicit values of Redfish constants

There are a few places in unit tests where we rely on Redfish constants
having well-known values. Use constants themselves instead.

An even more problematic place is _set_boot_device: it relies on storing
Redfish constants in driver_internal_info. With the future switch to
enums, it will no longer be possible. Use Ironic constants and provide a
compatibility layer.

Change-Id: I8e075f808e013afd778dd7cdc0927fe339309b70

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. '

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:

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)

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']