From 1abff50dbbd0d899432bb5b395a1637589941c33 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 25 Mar 2021 15:14:45 +0100 Subject: [PATCH] redfish-virtual-media: allow USB devices instead of floppies Support for floppy disks is increasingly harder to find, let us support USB devices as an alternative. Change-Id: Ib02b716cbcf1f7b4ed8e47cf3fcf40872f1dc9a1 --- ironic/drivers/modules/redfish/boot.py | 88 ++++++------ .../unit/drivers/modules/redfish/test_boot.py | 130 ++++++++++++++++-- .../redfish-config-usb-3e9a7543b2912ae7.yaml | 11 ++ 3 files changed, 175 insertions(+), 54 deletions(-) create mode 100644 releasenotes/notes/redfish-config-usb-3e9a7543b2912ae7.yaml diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index 2a6815f531..fc1c8851a5 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -41,10 +41,10 @@ REQUIRED_PROPERTIES = { } OPTIONAL_PROPERTIES = { - 'config_via_floppy': _("Boolean value to indicate whether or not the " - "driver should use virtual media Floppy device " - "for passing configuration information to the " - "ramdisk. Defaults to False. Optional."), + 'config_via_removable': _("Boolean value to indicate whether or not the " + "driver should use virtual media USB or floppy " + "device for passing configuration information " + "to the ramdisk. Defaults to False. Optional."), 'kernel_append_params': _("Additional kernel parameters to pass down to " "instance kernel. These parameters can be " "consumed by the kernel or by the applications " @@ -126,6 +126,12 @@ def _parse_driver_info(node): {option: d_info.get(option, getattr(CONF.conductor, option, None)) for option in OPTIONAL_PROPERTIES}) + if (d_info.get('config_via_removable') is None + and d_info.get('config_via_floppy') is not None): + LOG.warning('The config_via_floppy driver_info option is deprecated, ' + 'use config_via_removable for node %s', node.uuid) + deploy_info['config_via_removable'] = d_info['config_via_floppy'] + deploy_info.update(redfish_utils.parse_driver_info(node)) return deploy_info @@ -255,13 +261,19 @@ def _has_vmedia_device(managers, boot_device): """Indicate if device exists at any of the managers :param managers: A list of System managers. - :param boot_device: sushy boot device e.g. `VIRTUAL_MEDIA_CD`, - `VIRTUAL_MEDIA_DVD` or `VIRTUAL_MEDIA_FLOPPY`. + :param boot_device: One or more sushy boot device e.g. `VIRTUAL_MEDIA_CD`, + `VIRTUAL_MEDIA_DVD` or `VIRTUAL_MEDIA_FLOPPY`. Several devices are + checked in the given order. + :return: The device that could be found or False. """ - for manager in managers: - for v_media in manager.virtual_media.get_members(): - if boot_device in v_media.media_types: - return True + if isinstance(boot_device, str): + boot_device = [boot_device] + + for dev in boot_device: + for manager in managers: + for v_media in manager.virtual_media.get_members(): + if dev in v_media.media_types: + return dev return False @@ -465,7 +477,7 @@ class RedfishVirtualMediaBoot(base.BootInterface): d_info = _parse_driver_info(node) - config_via_floppy = d_info.get('config_via_floppy') + config_via_removable = d_info.get('config_via_removable') deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) if deploy_nic_mac is not None: @@ -475,27 +487,32 @@ class RedfishVirtualMediaBoot(base.BootInterface): managers = redfish_utils.get_system(task.node).managers - if config_via_floppy: + if config_via_removable: - if _has_vmedia_device(managers, sushy.VIRTUAL_MEDIA_FLOPPY): - # NOTE (etingof): IPA will read the diskette only if + removable = _has_vmedia_device( + managers, + # Prefer USB devices since floppies are outdated + [sushy.VIRTUAL_MEDIA_USBSTICK, sushy.VIRTUAL_MEDIA_FLOPPY]) + if removable: + # NOTE (etingof): IPA will read the device only if # we tell it to ramdisk_params['boot_method'] = 'vmedia' floppy_ref = image_utils.prepare_floppy_image( task, params=ramdisk_params) - _eject_vmedia(task, managers, sushy.VIRTUAL_MEDIA_FLOPPY) - _insert_vmedia( - task, managers, floppy_ref, sushy.VIRTUAL_MEDIA_FLOPPY) + _eject_vmedia(task, managers, removable) + _insert_vmedia(task, managers, floppy_ref, removable) - LOG.debug('Inserted virtual floppy with configuration for ' - 'node %(node)s', {'node': task.node.uuid}) + LOG.info('Inserted virtual %(type)s device with configuration' + ' for node %(node)s', + {'node': task.node.uuid, 'type': removable}) else: - LOG.warning('Config via floppy is requested, but ' - 'Floppy drive is not available on node ' - '%(node)s', {'node': task.node.uuid}) + LOG.warning('Config via a removable device is requested, but ' + 'virtual USB and floppy devices are not ' + 'available on node %(node)s', + {'node': task.node.uuid}) mode = deploy_utils.rescue_or_deploy_mode(node) @@ -524,23 +541,9 @@ class RedfishVirtualMediaBoot(base.BootInterface): :param task: A task from TaskManager. :returns: None """ - d_info = _parse_driver_info(task.node) - - config_via_floppy = d_info.get('config_via_floppy') - LOG.debug("Cleaning up deploy boot for " "%(node)s", {'node': task.node.uuid}) - - managers = redfish_utils.get_system(task.node).managers - - _eject_vmedia(task, managers, sushy.VIRTUAL_MEDIA_CD) - image_utils.cleanup_iso_image(task) - - if (config_via_floppy - and _has_vmedia_device(managers, sushy.VIRTUAL_MEDIA_FLOPPY)): - _eject_vmedia(task, managers, sushy.VIRTUAL_MEDIA_FLOPPY) - - image_utils.cleanup_floppy_image(task) + self._eject_all(task) def prepare_instance(self, task): """Prepares the boot of instance over virtual media. @@ -625,11 +628,16 @@ class RedfishVirtualMediaBoot(base.BootInterface): managers = redfish_utils.get_system(task.node).managers _eject_vmedia(task, managers, sushy.VIRTUAL_MEDIA_CD) - d_info = task.node.driver_info - config_via_floppy = d_info.get('config_via_floppy') - if config_via_floppy: + config_via_removable = ( + task.node.driver_info.get('config_via_removable') + or task.node.driver_info.get('config_via_floppy') + ) + if config_via_removable: + _eject_vmedia(task, managers, sushy.VIRTUAL_MEDIA_USBSTICK) _eject_vmedia(task, managers, sushy.VIRTUAL_MEDIA_FLOPPY) + image_utils.cleanup_floppy_image(task) + boot_option = deploy_utils.get_boot_option(task.node) if (boot_option == 'ramdisk' and task.node.instance_info.get('configdrive')): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index e60eafea7c..27ec9c8482 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -83,6 +83,34 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): self.assertEqual('http://boot.iso', actual_driver_info['redfish_deploy_iso']) + def test_parse_driver_info_removable(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.node.driver_info.update( + {'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader', + 'config_via_removable': True} + ) + + actual_driver_info = redfish_boot._parse_driver_info(task.node) + self.assertTrue(actual_driver_info['config_via_removable']) + + @mock.patch.object(redfish_boot.LOG, 'warning', autospec=True) + def test_parse_driver_info_removable_deprecated(self, mock_log): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.node.driver_info.update( + {'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader', + 'config_via_floppy': True} + ) + + actual_driver_info = redfish_boot._parse_driver_info(task.node) + self.assertTrue(actual_driver_info['config_via_removable']) + self.assertTrue(mock_log.called) + def test_parse_driver_info_rescue(self): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -500,12 +528,12 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.node.provision_state = states.DEPLOYING d_info = { - 'config_via_floppy': True + 'config_via_removable': True } mock__parse_driver_info.return_value = d_info - mock__has_vmedia_device.return_value = True + mock__has_vmedia_device.return_value = sushy.VIRTUAL_MEDIA_FLOPPY mock_prepare_floppy_image.return_value = 'floppy-image-url' mock_prepare_deploy_iso.return_value = 'cd-image-url' @@ -515,7 +543,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task, states.POWER_OFF) mock__has_vmedia_device.assert_called_once_with( - managers, sushy.VIRTUAL_MEDIA_FLOPPY) + managers, + [sushy.VIRTUAL_MEDIA_USBSTICK, sushy.VIRTUAL_MEDIA_FLOPPY]) eject_calls = [ mock.call(task, managers, dev) @@ -548,7 +577,79 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) + @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', + autospec=True) + @mock.patch.object(image_utils, 'prepare_floppy_image', autospec=True) + @mock.patch.object(image_utils, 'prepare_deploy_iso', autospec=True) @mock.patch.object(redfish_boot, '_has_vmedia_device', autospec=True) + @mock.patch.object(redfish_boot, '_eject_vmedia', autospec=True) + @mock.patch.object(redfish_boot, '_insert_vmedia', autospec=True) + @mock.patch.object(redfish_boot, '_parse_driver_info', autospec=True) + @mock.patch.object(redfish_boot.manager_utils, 'node_power_action', + autospec=True) + @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_prepare_ramdisk_with_usb( + self, mock_system, mock_boot_mode_utils, mock_node_power_action, + mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, + mock__has_vmedia_device, mock_prepare_deploy_iso, + mock_prepare_floppy_image, mock_node_set_boot_device): + + managers = mock_system.return_value.managers + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.provision_state = states.DEPLOYING + + d_info = { + 'config_via_removable': True + } + + mock__parse_driver_info.return_value = d_info + + mock__has_vmedia_device.return_value = sushy.VIRTUAL_MEDIA_USBSTICK + mock_prepare_floppy_image.return_value = 'floppy-image-url' + mock_prepare_deploy_iso.return_value = 'cd-image-url' + + task.driver.boot.prepare_ramdisk(task, {}) + + mock_node_power_action.assert_called_once_with( + task, states.POWER_OFF) + + mock__has_vmedia_device.assert_called_once_with( + managers, + [sushy.VIRTUAL_MEDIA_USBSTICK, sushy.VIRTUAL_MEDIA_FLOPPY]) + + eject_calls = [ + mock.call(task, managers, dev) + for dev in (sushy.VIRTUAL_MEDIA_USBSTICK, + sushy.VIRTUAL_MEDIA_CD) + ] + + mock__eject_vmedia.assert_has_calls(eject_calls) + + insert_calls = [ + mock.call(task, managers, 'floppy-image-url', + sushy.VIRTUAL_MEDIA_USBSTICK), + mock.call(task, managers, 'cd-image-url', + sushy.VIRTUAL_MEDIA_CD), + ] + + mock__insert_vmedia.assert_has_calls(insert_calls) + + expected_params = { + 'boot_method': 'vmedia', + 'ipa-debug': '1', + 'ipa-agent-token': mock.ANY, + } + + mock_prepare_deploy_iso.assert_called_once_with( + task, expected_params, 'deploy', d_info) + + mock_node_set_boot_device.assert_called_once_with( + task, boot_devices.CDROM, False) + + mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) + @mock.patch.object(redfish_boot, '_eject_vmedia', autospec=True) @mock.patch.object(image_utils, 'cleanup_iso_image', autospec=True) @mock.patch.object(image_utils, 'cleanup_floppy_image', autospec=True) @@ -557,15 +658,13 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_clean_up_ramdisk( self, mock_system, mock__parse_driver_info, mock_cleanup_floppy_image, mock_cleanup_iso_image, - mock__eject_vmedia, mock__has_vmedia_device): + mock__eject_vmedia): managers = mock_system.return_value.managers with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.provision_state = states.DEPLOYING - - mock__parse_driver_info.return_value = {'config_via_floppy': True} - mock__has_vmedia_device.return_value = True + task.node.driver_info['config_via_removable'] = True task.driver.boot.clean_up_ramdisk(task) @@ -573,11 +672,9 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_cleanup_floppy_image.assert_called_once_with(task) - mock__has_vmedia_device.assert_called_once_with( - managers, sushy.VIRTUAL_MEDIA_FLOPPY) - eject_calls = [ mock.call(task, managers, sushy.VIRTUAL_MEDIA_CD), + mock.call(task, managers, sushy.VIRTUAL_MEDIA_USBSTICK), mock.call(task, managers, sushy.VIRTUAL_MEDIA_FLOPPY) ] @@ -837,9 +934,11 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): @mock.patch.object(boot_mode_utils, 'deconfigure_secure_boot_if_needed', autospec=True) @mock.patch.object(redfish_boot, '_eject_vmedia', autospec=True) + @mock.patch.object(image_utils, 'cleanup_floppy_image', autospec=True) @mock.patch.object(image_utils, 'cleanup_iso_image', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def _test_clean_up_instance(self, mock_system, mock_cleanup_iso_image, + mock_cleanup_floppy_image, mock__eject_vmedia, mock_secure_boot): managers = mock_system.return_value.managers with task_manager.acquire(self.context, self.node.uuid, @@ -849,9 +948,12 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_cleanup_iso_image.assert_called_once_with(task) eject_calls = [mock.call(task, managers, sushy.VIRTUAL_MEDIA_CD)] - if task.node.driver_info.get('config_via_floppy'): - eject_calls.append(mock.call(task, managers, - sushy.VIRTUAL_MEDIA_FLOPPY)) + if task.node.driver_info.get('config_via_removable'): + eject_calls.extend([ + mock.call(task, managers, sushy.VIRTUAL_MEDIA_USBSTICK), + mock.call(task, managers, sushy.VIRTUAL_MEDIA_FLOPPY), + ]) + mock_cleanup_floppy_image.assert_called_once_with(task) mock__eject_vmedia.assert_has_calls(eject_calls) mock_secure_boot.assert_called_once_with(task) @@ -861,7 +963,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_clean_up_instance_cdrom_and_floppy(self): driver_info = self.node.driver_info - driver_info['config_via_floppy'] = True + driver_info['config_via_removable'] = True self.node.driver_info = driver_info self.node.save() self._test_clean_up_instance() diff --git a/releasenotes/notes/redfish-config-usb-3e9a7543b2912ae7.yaml b/releasenotes/notes/redfish-config-usb-3e9a7543b2912ae7.yaml new file mode 100644 index 0000000000..8094a77e6d --- /dev/null +++ b/releasenotes/notes/redfish-config-usb-3e9a7543b2912ae7.yaml @@ -0,0 +1,11 @@ +--- +deprecations: + - | + The node's ``driver_info`` parameter ``config_via_floppy`` of the + ``redfish-virtual-media`` boot interface has been renamed to + ``config_via_removable``. The old alias is deprecated. +features: + - | + Supplying configuration to the agent using the ``redfish-virtual-media`` + boot interface now works through USB instead of floppy by default. Modern + hardware (and even virtual machines) has limited support for floppies.