Prevent redfish-virtual-media from being used with Dell nodes

Indicate that idrac-redfish-virtual-media must be used instead,
otherwise a confusing failure will happen.

Change-Id: I3b6ced6dcf03580903f5ea7237fc057f372999f9

6 files changed, 103 insertions, 1 deletions

diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst
index 132421bca..f2b6e8f31 100644
--- a/doc/source/admin/drivers/redfish.rst
+++ b/doc/source/admin/drivers/redfish.rst
@@ -178,6 +178,12 @@ 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
+   :doc:`/admin/drivers/idrac` for more details on this hardware type.
+
 If UEFI boot mode is desired, the user should additionally supply EFI
 System Partition image (ESP_), see `Configuring an ESP image`_ for details.
 
diff --git a/ironic/drivers/modules/drac/boot.py b/ironic/drivers/modules/drac/boot.py
index 771dc7f5e..f04ce959c 100644
--- a/ironic/drivers/modules/drac/boot.py
+++ b/ironic/drivers/modules/drac/boot.py
@@ -70,6 +70,9 @@ class DracRedfishVirtualMediaBoot(redfish_boot.RedfishVirtualMediaBoot):
             boot_devices.CDROM: sushy.VIRTUAL_MEDIA_CD
         }
 
+    def _validate_vendor(self, task):
+        pass  # assume people are doing the right thing
+
     @classmethod
     def _set_boot_device(cls, task, device, persistent=False):
         """Set boot device for a node.
diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py
index c40a0adf4..f16f561c2 100644
--- a/ironic/drivers/modules/redfish/boot.py
+++ b/ironic/drivers/modules/redfish/boot.py
@@ -374,6 +374,19 @@ class RedfishVirtualMediaBoot(base.BootInterface):
 
         deploy_utils.validate_image_properties(task.context, d_info, props)
 
+    def _validate_vendor(self, task):
+        vendor = task.node.properties.get('vendor')
+        if not vendor:
+            return
+
+        if 'Dell' in vendor.split():
+            raise exception.InvalidParameterValue(
+                _("The %(iface)s boot interface is not suitable for node "
+                  "%(node)s with vendor %(vendor)s, use "
+                  "idrac-redfish-virtual-media instead")
+                % {'iface': task.node.boot_interface,
+                   'node': task.node.uuid, 'vendor': vendor})
+
     def validate(self, task):
         """Validate the deployment information for the task's node.
 
@@ -385,6 +398,7 @@ 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)
 
         if task.driver.storage.should_write_image(task):
diff --git a/ironic/tests/unit/drivers/modules/drac/test_boot.py b/ironic/tests/unit/drivers/modules/drac/test_boot.py
index d73ef69d7..bed8be2ef 100644
--- a/ironic/tests/unit/drivers/modules/drac/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/drac/test_boot.py
@@ -25,13 +25,15 @@ from oslo_utils import importutils
 from ironic.common import boot_devices
 from ironic.common import exception
 from ironic.conductor import task_manager
+from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.drac import boot as drac_boot
+from ironic.tests.unit.db import utils as db_utils
 from ironic.tests.unit.drivers.modules.drac import utils as test_utils
 from ironic.tests.unit.objects import utils as obj_utils
 
 sushy = importutils.try_import('sushy')
 
-INFO_DICT = test_utils.INFO_DICT
+INFO_DICT = dict(db_utils.get_test_redfish_info(), **test_utils.INFO_DICT)
 
 
 @mock.patch.object(drac_boot, 'redfish_utils', autospec=True)
@@ -42,6 +44,28 @@ class DracBootTestCase(test_utils.BaseDracTest):
         self.node = obj_utils.create_test_node(
             self.context, driver='idrac', driver_info=INFO_DICT)
 
+    @mock.patch.object(deploy_utils, 'validate_image_properties',
+                       autospec=True)
+    def test_validate_correct_vendor(self, mock_redfish_utils,
+                                     mock_validate_image_properties):
+        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(task)
+
     def test__set_boot_device_persistent(self, mock_redfish_utils):
 
         mock_system = mock_redfish_utils.get_system.return_value
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
index 8e1e0664b..6a403a522 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
@@ -295,6 +295,55 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
     @mock.patch.object(redfish_utils, 'parse_driver_info', autospec=True)
     @mock.patch.object(deploy_utils, 'validate_image_properties',
                        autospec=True)
+    def test_validate_correct_vendor(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(
+                {'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'] = "Ironic Co."
+
+            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):
+        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, task)
+
+    @mock.patch.object(redfish_utils, 'parse_driver_info', autospec=True)
+    @mock.patch.object(deploy_utils, 'validate_image_properties',
+                       autospec=True)
     def test_validate_missing(self, mock_validate_image_properties,
                               mock_parse_driver_info):
         with task_manager.acquire(self.context, self.node.uuid,
diff --git a/releasenotes/notes/redfish-vmedia-vendor-fc76086893d99415.yaml b/releasenotes/notes/redfish-vmedia-vendor-fc76086893d99415.yaml
new file mode 100644
index 000000000..d98156530
--- /dev/null
+++ b/releasenotes/notes/redfish-vmedia-vendor-fc76086893d99415.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The ``redfish-virtual-media`` boot interface no longer passes validation
+    for Dell nodes. The ``idrac-redfish-virtual-media`` boot interface must
+    be used for these nodes instead.