Fix redfish-virtual-media for newer iDRACs

The issue with standard Redfish virtual media boot has
been fixed now. Update to restrict use of redfish-virtual-media
based on iDRAC firmware version.

Change-Id: I8ead1d24a9bd502b64fe7dd058e77550fcee141c

doc/source/admin/drivers/redfish.rst

@@ -178,10 +178,13 @@ BIOS boot mode, it suffice to set ironic boot interface to
 
   baremetal node set --boot-interface redfish-virtual-media node-0
 
-.. warning::
+.. note::
-   Dell hardware requires a non-standard Redfish call to boot from virtual
+   iDRAC firmware before 4.40.10.00 (on Intel systems) and 6.00.00.00
-   media, thus you **must** use the ``idrac`` hardware type and the
+   (on AMD systems) requires a non-standard Redfish call to boot from virtual
-   ``idrac-redfish-virtual-media`` boot interface with it instead. See
+   media. Consider upgrading to 6.00.00.00, otherwise you **must** use
+   the ``idrac`` hardware type and the ``idrac-redfish-virtual-media`` boot
+   interface with older iDRAC firmware instead. For simplicity Ironic restricts
+   both AMD and Intel systems before firmware version 6.00.00.00. See
    :doc:`/admin/drivers/idrac` for more details on this hardware type.
 
 If UEFI boot mode is desired, the user should additionally supply EFI

ironic/drivers/modules/drac/boot.py

@@ -69,7 +69,7 @@ class DracRedfishVirtualMediaBoot(redfish_boot.RedfishVirtualMediaBoot):
             boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD
         }
 
-    def _validate_vendor(self, task):
+    def _validate_vendor(self, task, managers):
         pass  # assume people are doing the right thing
 
     @classmethod

ironic/drivers/modules/redfish/boot.py

@@ -407,18 +407,34 @@ class RedfishVirtualMediaBoot(base.BootInterface):
         d_info = _parse_deploy_info(node)
         deploy_utils.validate_image_properties(task, d_info)
 
-    def _validate_vendor(self, task):
+    def _validate_vendor(self, task, managers):
+        """Validates vendor specific requirements for the task's node.
+
+        :param task: a TaskManager instance containing the node to act on.
+        :param managers: Redfish managers for Redfish system associated
+            with node.
+        :raises: InvalidParameterValue if vendor not supported
+        """
         vendor = task.node.properties.get('vendor')
         if not vendor:
             return
 
         if 'Dell' in vendor.split():
+            # Check if iDRAC fw >= 6.00.00.00 that supports virtual media boot
+            bmc_manager = [m for m in managers
+                           if m.manager_type == sushy.MANAGER_TYPE_BMC]
+            if bmc_manager:
+                fwv = bmc_manager[0].firmware_version.split('.')
+                if int(fwv[0]) >= 6:
+                    return
             raise exception.InvalidParameterValue(
                 _("The %(iface)s boot interface is not suitable for node "
-                  "%(node)s with vendor %(vendor)s, use "
+                  "%(node)s with vendor %(vendor)s and BMC version %(fwv)s, "
+                  "upgrade to 6.00.00.00 or newer or use "
                   "idrac-redfish-virtual-media instead")
                 % {'iface': task.node.get_interface('boot'),
-                   'node': task.node.uuid, 'vendor': vendor})
+                   'node': task.node.uuid, 'vendor': vendor,
+                   'fwv': bmc_manager[0].firmware_version})
 
     def validate(self, task):
         """Validate the deployment information for the task's node.
@@ -431,7 +447,6 @@ class RedfishVirtualMediaBoot(base.BootInterface):
         :raises: InvalidParameterValue on malformed parameter(s)
         :raises: MissingParameterValue on missing parameter(s)
         """
-        self._validate_vendor(task)
         self._validate_driver_info(task)
         self._validate_instance_info(task)
 
@@ -474,6 +489,8 @@ class RedfishVirtualMediaBoot(base.BootInterface):
         d_info = _parse_driver_info(node)
         managers = redfish_utils.get_system(task.node).managers
 
+        self._validate_vendor(task, managers)
+
         if manager_utils.is_fast_track(task):
             if _has_vmedia_device(managers, sushy.VIRTUAL_MEDIA_CD,
                                   inserted=True):

ironic/tests/unit/drivers/modules/redfish/test_boot.py

@@ -324,12 +324,9 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             task.driver.boot.validate(task)
 
-    @mock.patch.object(redfish_utils, 'parse_driver_info', autospec=True)
+    def test__validate_vendor_incompatible_with_idrac(self):
-    @mock.patch.object(deploy_utils, 'validate_image_properties',
+        managers = [mock.Mock(firmware_version='5.10.30.00',
-                       autospec=True)
+                              manager_type=sushy.MANAGER_TYPE_BMC)]
-    def test_validate_incompatible_with_idrac(self,
-                                              mock_validate_image_properties,
-                                              mock_parse_driver_info):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             task.node.instance_info.update(
@@ -346,9 +343,30 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             task.node.properties['vendor'] = "Dell Inc."
 
-            self.assertRaisesRegex(exception.InvalidParameterValue,
+            self.assertRaisesRegex(
-                                   "with vendor Dell Inc.",
+                exception.InvalidParameterValue, "with vendor Dell Inc.",
-                                   task.driver.boot.validate, task)
+                task.driver.boot._validate_vendor, task, managers)
+
+    def test__validate_vendor_compatible_with_idrac(self):
+        managers = [mock.Mock(firmware_version='6.00.00.00',
+                              manager_type=sushy.MANAGER_TYPE_BMC)]
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            task.node.instance_info.update(
+                {'kernel': 'kernel',
+                 'ramdisk': 'ramdisk',
+                 'image_source': 'http://image/source'}
+            )
+
+            task.node.driver_info.update(
+                {'deploy_kernel': 'kernel',
+                 'deploy_ramdisk': 'ramdisk',
+                 'bootloader': 'bootloader'}
+            )
+
+            task.node.properties['vendor'] = "Dell Inc."
+
+            task.driver.boot._validate_vendor(task, managers)
 
     @mock.patch.object(redfish_utils, 'parse_driver_info', autospec=True)
     @mock.patch.object(deploy_utils, 'validate_image_properties',
@@ -381,6 +399,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
             self.assertRaises(exception.UnsupportedDriverExtension,
                               task.driver.boot.validate_inspection, task)
 
+    @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot,
+                       '_validate_vendor', autospec=True)
     @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device',
                        autospec=True)
     @mock.patch.object(image_utils, 'prepare_deploy_iso', autospec=True)
@@ -394,7 +414,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
     def test_prepare_ramdisk_with_params(
             self, mock_system, mock_boot_mode_utils, mock_node_power_action,
             mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia,
-            mock_prepare_deploy_iso, mock_node_set_boot_device):
+            mock_prepare_deploy_iso, mock_node_set_boot_device,
+            mock_validate_vendor):
 
         managers = mock_system.return_value.managers
         with task_manager.acquire(self.context, self.node.uuid,
@@ -406,6 +427,10 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             task.driver.boot.prepare_ramdisk(task, {})
 
+            mock_validate_vendor.assert_called_once_with(
+                task.driver.boot, task, managers
+            )
+
             mock_node_power_action.assert_called_once_with(
                 task, states.POWER_OFF)
 

releasenotes/notes/fix-redfish-vm-boot-idrac-37ec734e6643cbac.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixes ``redfish-virtual-media`` ``boot`` interface to allow it with
+    iDRAC firmware from 6.00.00.00 (released June 2022) as it
+    has virtual media boot issue fixed that prevented iDRAC firmware to
+    work with ``redfish-virtual-media`` before. Consider upgrading iDRAC
+    firmware if not done already, otherwise will still get an error
+    when trying to use ``redfish-virtual-media`` with iDRAC.