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.

 Conflicts:
       ironic/drivers/modules/redfish/boot.py

Change-Id: I8ead1d24a9bd502b64fe7dd058e77550fcee141c
(cherry picked from commit 73040c88d99f066644277f32e86d196305fc8596)

7 files changed, 76 insertions, 20 deletions

diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst
index 0878b08bf..f4eab73eb 100644
--- a/doc/source/admin/drivers/redfish.rst
+++ b/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::
-   Dell hardware requires a non-standard Redfish call to boot from virtual
-   media, thus you **must** use the ``idrac`` hardware type and the
-   ``idrac-redfish-virtual-media`` boot interface with it instead. See
+.. note::
+   iDRAC firmware before 4.40.10.00 (on Intel systems) and 6.00.00.00
+   (on AMD systems) requires a non-standard Redfish call to boot from virtual
+   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
diff --git a/ironic/drivers/modules/drac/boot.py b/ironic/drivers/modules/drac/boot.py
index 1e2f30dc0..aa3b49708 100644
--- a/ironic/drivers/modules/drac/boot.py
+++ b/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
diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py
index 4572805b4..3c37e5ad2 100644
--- a/ironic/drivers/modules/redfish/boot.py
+++ b/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.boot_interface,
-                   '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)
 
@@ -481,6 +496,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):
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
index e69c29dca..e2685639c 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
@@ -314,12 +314,32 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             task.driver.boot.validate(task)
 
-    @mock.patch.object(redfish_utils, 'parse_driver_info', autospec=True)
-    @mock.patch.object(deploy_utils, 'validate_image_properties',
-                       autospec=True)
-    def test_validate_incompatible_with_idrac(self,
-                                              mock_validate_image_properties,
-                                              mock_parse_driver_info):
+    def test__validate_vendor_incompatible_with_idrac(self):
+        managers = [mock.Mock(firmware_version='5.10.30.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."
+
+            self.assertRaisesRegex(
+                exception.InvalidParameterValue, "with vendor Dell Inc.",
+                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(
@@ -336,9 +356,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             task.node.properties['vendor'] = "Dell Inc."
 
-            self.assertRaisesRegex(exception.InvalidParameterValue,
-                                   "with vendor Dell Inc.",
-                                   task.driver.boot.validate, task)
+            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',
@@ -371,6 +389,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)
@@ -384,7 +404,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,
@@ -396,6 +417,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)
 
diff --git a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py
index 10f76d3a2..f3e8dd314 100644
--- a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py
+++ b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py
@@ -179,6 +179,7 @@ SUSHY_SPEC = (
     'SEVERITY_OK',
     'SEVERITY_WARNING',
     'SEVERITY_CRITICAL',
+    'MANAGER_TYPE_BMC',
 )
 
 SUSHY_AUTH_SPEC = (
diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py
index 985bbc4c3..fd1fb63f1 100644
--- a/ironic/tests/unit/drivers/third_party_driver_mocks.py
+++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py
@@ -246,7 +246,8 @@ if not sushy:
         VOLUME_TYPE_RAW_DEVICE='rawdevice',
         SEVERITY_OK='ok',
         SEVERITY_WARNING='warning',
-        SEVERITY_CRITICAL='critical'
+        SEVERITY_CRITICAL='critical',
+        MANAGER_TYPE_BMC='bmc',
     )
 
     sys.modules['sushy'] = sushy
diff --git a/releasenotes/notes/fix-redfish-vm-boot-idrac-37ec734e6643cbac.yaml b/releasenotes/notes/fix-redfish-vm-boot-idrac-37ec734e6643cbac.yaml
new file mode 100644
index 000000000..837454fcb
--- /dev/null
+++ b/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.