From 65e2e190981942b83b3a1eef99183b8128e833a2 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Thu, 8 Apr 2021 14:51:43 +0200
Subject: [PATCH] Fix fast track with redfish-virtual-media
During cleaning prepare_ramdisk is always called, even with fast track.
However, the redfish-virtual-media boot interface ignores fast track and
always reboots the node. This change makes fast track work if we can
detect that a CD device is connected.
Change-Id: I8d13df5db9654b0c2ea24489f7df75cdc5bae0e2
---
ironic/drivers/modules/redfish/boot.py | 31 +++-
.../unit/drivers/modules/redfish/test_boot.py | 142 ++++++++++++++++++
2 files changed, 168 insertions(+), 5 deletions(-)
diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py
index 6297052f87..3fd3f1cd22 100644
--- a/ironic/drivers/modules/redfish/boot.py
+++ b/ironic/drivers/modules/redfish/boot.py
@@ -264,13 +264,15 @@ def eject_vmedia(task, boot_device=None):
_eject_vmedia(task, system.managers, boot_device=boot_device)
-def _has_vmedia_device(managers, boot_device):
+def _has_vmedia_device(managers, boot_device, inserted=None):
"""Indicate if device exists at any of the managers
:param managers: A list of System managers.
: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.
+ :param inserted: If not None, only return a device with a matching
+ inserted status.
:return: The device that could be found or False.
"""
if isinstance(boot_device, str):
@@ -279,8 +281,12 @@ def _has_vmedia_device(managers, 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
+ if dev not in v_media.media_types:
+ continue
+ if (inserted is not None
+ and bool(v_media.inserted) is not inserted):
+ continue
+ return dev
return False
@@ -474,6 +480,18 @@ class RedfishVirtualMediaBoot(base.BootInterface):
return
d_info = _parse_driver_info(node)
+ managers = redfish_utils.get_system(task.node).managers
+
+ if manager_utils.is_fast_track(task):
+ if _has_vmedia_device(managers, sushy.VIRTUAL_MEDIA_CD,
+ inserted=True):
+ LOG.debug('Fast track operation for node %s, not inserting '
+ 'any devices', node.uuid)
+ return
+ else:
+ LOG.warning('Fast track is possible for node %s, but no ISO '
+ 'is currently inserted! Proceeding with '
+ 'normal operation.', node.uuid)
# NOTE(TheJulia): Since we're deploying, cleaning, or rescuing,
# with virtual media boot, we should generate a token!
@@ -494,8 +512,6 @@ class RedfishVirtualMediaBoot(base.BootInterface):
if CONF.debug and 'ipa-debug' not in ramdisk_params:
ramdisk_params['ipa-debug'] = '1'
- managers = redfish_utils.get_system(task.node).managers
-
# NOTE(TheJulia): This is a mandatory setting for virtual media
# based deployment operations.
ramdisk_params['boot_method'] = 'vmedia'
@@ -551,6 +567,11 @@ class RedfishVirtualMediaBoot(base.BootInterface):
:param task: A task from TaskManager.
:returns: None
"""
+ if manager_utils.is_fast_track(task):
+ LOG.debug('Fast track operation for node %s, not ejecting '
+ 'any devices', task.node.uuid)
+ return
+
LOG.debug("Cleaning up deploy boot for "
"%(node)s", {'node': task.node.uuid})
self._eject_all(task)
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
index 782ddfcf52..fd3304c84e 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
@@ -21,6 +21,7 @@ from ironic.common import boot_devices
from ironic.common import exception
from ironic.common import states
from ironic.conductor import task_manager
+from ironic.conductor import utils as manager_utils
from ironic.drivers.modules import boot_mode_utils
from ironic.drivers.modules import deploy_utils
from ironic.drivers.modules import image_utils
@@ -712,6 +713,103 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
self.assertNotIn('agent_secret_token_pregenerated',
task.node.driver_internal_info)
+ @mock.patch.object(manager_utils, 'is_fast_track', lambda task: True)
+ @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device',
+ 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_fast_track(
+ 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_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
+ mock__has_vmedia_device.return_value = sushy.VIRTUAL_MEDIA_CD
+
+ task.driver.boot.prepare_ramdisk(task, {})
+
+ mock__has_vmedia_device.assert_called_once_with(
+ managers, sushy.VIRTUAL_MEDIA_CD, inserted=True)
+
+ mock_node_power_action.assert_not_called()
+ mock__eject_vmedia.assert_not_called()
+ mock__insert_vmedia.assert_not_called()
+ mock_prepare_deploy_iso.assert_not_called()
+ mock_node_set_boot_device.assert_not_called()
+ mock_boot_mode_utils.sync_boot_mode.assert_not_called()
+
+ @mock.patch.object(manager_utils, 'is_fast_track', lambda task: True)
+ @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device',
+ 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_fast_track_impossible(
+ 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_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
+ mock__has_vmedia_device.return_value = False
+
+ mock__parse_driver_info.return_value = {}
+ mock_prepare_deploy_iso.return_value = 'image-url'
+
+ task.driver.boot.prepare_ramdisk(task, {})
+
+ mock__has_vmedia_device.assert_called_once_with(
+ managers, sushy.VIRTUAL_MEDIA_CD, inserted=True)
+
+ mock_node_power_action.assert_called_once_with(
+ task, states.POWER_OFF)
+
+ mock__eject_vmedia.assert_called_once_with(
+ task, managers, sushy.VIRTUAL_MEDIA_CD)
+
+ mock__insert_vmedia.assert_called_once_with(
+ task, managers, 'image-url', sushy.VIRTUAL_MEDIA_CD)
+
+ token = task.node.driver_internal_info['agent_secret_token']
+ self.assertTrue(token)
+
+ expected_params = {
+ 'ipa-agent-token': token,
+ 'ipa-debug': '1',
+ 'boot_method': 'vmedia',
+ }
+
+ mock_prepare_deploy_iso.assert_called_once_with(
+ task, expected_params, 'deploy', {})
+
+ 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)
+
+ self.assertTrue(task.node.driver_internal_info[
+ 'agent_secret_token_pregenerated'])
+
@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)
@@ -1242,3 +1340,47 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
redfish_boot.eject_vmedia(task)
self.assertFalse(mock_vmedia_cd.eject_media.call_count)
+
+ def test__has_vmedia_device(self):
+ mock_vmedia_cd = mock.MagicMock(
+ inserted=False,
+ media_types=[sushy.VIRTUAL_MEDIA_CD])
+ mock_vmedia_floppy = mock.MagicMock(
+ inserted=False,
+ media_types=[sushy.VIRTUAL_MEDIA_FLOPPY])
+
+ mock_manager = mock.MagicMock()
+
+ mock_manager.virtual_media.get_members.return_value = [
+ mock_vmedia_cd, mock_vmedia_floppy]
+
+ self.assertEqual(
+ sushy.VIRTUAL_MEDIA_CD,
+ redfish_boot._has_vmedia_device(
+ [mock_manager], sushy.VIRTUAL_MEDIA_CD))
+
+ self.assertFalse(
+ redfish_boot._has_vmedia_device(
+ [mock_manager], sushy.VIRTUAL_MEDIA_CD, inserted=True))
+
+ self.assertFalse(
+ redfish_boot._has_vmedia_device(
+ [mock_manager], sushy.VIRTUAL_MEDIA_USBSTICK))
+
+ def test__has_vmedia_device_inserted(self):
+ mock_vmedia_cd = mock.MagicMock(
+ inserted=False,
+ media_types=[sushy.VIRTUAL_MEDIA_CD])
+ mock_vmedia_floppy = mock.MagicMock(
+ inserted=True,
+ media_types=[sushy.VIRTUAL_MEDIA_FLOPPY])
+
+ mock_manager = mock.MagicMock()
+
+ mock_manager.virtual_media.get_members.return_value = [
+ mock_vmedia_cd, mock_vmedia_floppy]
+
+ self.assertEqual(
+ sushy.VIRTUAL_MEDIA_FLOPPY,
+ redfish_boot._has_vmedia_device(
+ [mock_manager], sushy.VIRTUAL_MEDIA_FLOPPY, inserted=True))