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

@@ -41,10 +41,10 @@ REQUIRED_PROPERTIES = {
 }
 
 OPTIONAL_PROPERTIES = {
-    'config_via_floppy': _("Boolean value to indicate whether or not the "
+    'config_via_removable': _("Boolean value to indicate whether or not the "
-                           "driver should use virtual media Floppy device "
+                              "driver should use virtual media USB or floppy "
-                           "for passing configuration information to the "
+                              "device for passing configuration information "
-                           "ramdisk. Defaults to False. Optional."),
+                              "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`,
+    :param boot_device: One or more sushy boot device e.g. `VIRTUAL_MEDIA_CD`,
-        `VIRTUAL_MEDIA_DVD` or `VIRTUAL_MEDIA_FLOPPY`.
+        `VIRTUAL_MEDIA_DVD` or `VIRTUAL_MEDIA_FLOPPY`. Several devices are
+        checked in the given order.
+    :return: The device that could be found or False.
     """
+    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 boot_device in v_media.media_types:
+                if dev in v_media.media_types:
-                return True
+                    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):
+            removable = _has_vmedia_device(
-                # NOTE (etingof): IPA will read the diskette only if
+                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)
+                _eject_vmedia(task, managers, removable)
-                _insert_vmedia(
+                _insert_vmedia(task, managers, floppy_ref, removable)
-                    task, managers, floppy_ref, sushy.VIRTUAL_MEDIA_FLOPPY)
 
-                LOG.debug('Inserted virtual floppy with configuration for '
+                LOG.info('Inserted virtual %(type)s device with configuration'
-                          'node %(node)s', {'node': task.node.uuid})
+                         ' for node %(node)s',
+                         {'node': task.node.uuid, 'type': removable})
 
             else:
-                LOG.warning('Config via floppy is requested, but '
+                LOG.warning('Config via a removable device is requested, but '
-                            'Floppy drive is not available on node '
+                            'virtual USB and floppy devices are not '
-                            '%(node)s', {'node': task.node.uuid})
+                            '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})
-
+        self._eject_all(task)
-        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)
 
     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_removable = (
-        config_via_floppy = d_info.get('config_via_floppy')
+            task.node.driver_info.get('config_via_removable')
-        if config_via_floppy:
+            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')):

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
-
+            task.node.driver_info['config_via_removable'] = True
-            mock__parse_driver_info.return_value = {'config_via_floppy': True}
-            mock__has_vmedia_device.return_value = 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'):
+            if task.node.driver_info.get('config_via_removable'):
-                eject_calls.append(mock.call(task, managers,
+                eject_calls.extend([
-                                             sushy.VIRTUAL_MEDIA_FLOPPY))
+                    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()

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.