From cf22604c5863920b56e0b16dbf30894cbb544130 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 20 Jan 2021 11:58:44 +0100 Subject: [PATCH] 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 --- doc/source/admin/drivers/redfish.rst | 6 +++ ironic/drivers/modules/drac/boot.py | 3 ++ ironic/drivers/modules/redfish/boot.py | 14 ++++++ .../unit/drivers/modules/drac/test_boot.py | 26 +++++++++- .../unit/drivers/modules/redfish/test_boot.py | 49 +++++++++++++++++++ ...edfish-vmedia-vendor-fc76086893d99415.yaml | 6 +++ 6 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/redfish-vmedia-vendor-fc76086893d99415.yaml diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst index 132421bcaa..f2b6e8f314 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 771dc7f5ec..f04ce959c9 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 c40a0adf47..f16f561c22 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 d73ef69d7e..bed8be2ef2 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 8e1e0664bb..6a403a5229 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -292,6 +292,55 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_validate_image_properties.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY) + @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) diff --git a/releasenotes/notes/redfish-vmedia-vendor-fc76086893d99415.yaml b/releasenotes/notes/redfish-vmedia-vendor-fc76086893d99415.yaml new file mode 100644 index 0000000000..d98156530b --- /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.