From 89f421b166915a22626dcb919382ce13f4588973 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 15 Aug 2022 16:42:19 +0200 Subject: [PATCH] Do not reboot into nowhere after BIOS settings with fast-track Currently, the prepare_ramdisk implementation of the redfish-virtual-media boot interface skips configuring an ISO in fast-track mode. When rebooting after BIOS/RAID settings changes, we need to enforce the correct ISO configuration. Change-Id: Ibdfe0775065f3b27478305dd18929a291df3ee3c --- ironic/drivers/modules/deploy_utils.py | 5 +++ ironic/drivers/modules/redfish/bios.py | 24 +++----------- .../unit/drivers/modules/redfish/test_bios.py | 32 ++++++++++++++++--- .../fast-track-bios-fa9ae685c151dd24.yaml | 6 ++++ 4 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/fast-track-bios-fa9ae685c151dd24.yaml diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 399dfa68f3..13f91e9cd1 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -1458,7 +1458,12 @@ def reboot_to_finish_step(task): disable_ramdisk = task.node.driver_internal_info.get( 'cleaning_disable_ramdisk') if not disable_ramdisk: + if manager_utils.is_fast_track(task): + LOG.debug('Forcing power off on node %s for a clean reboot into ' + 'the agent image', task.node) + manager_utils.node_power_action(task, states.POWER_OFF) prepare_agent_boot(task) + manager_utils.node_power_action(task, states.REBOOT) return get_async_step_return_state(task.node) diff --git a/ironic/drivers/modules/redfish/bios.py b/ironic/drivers/modules/redfish/bios.py index c2eb8fcbcf..bee868e385 100644 --- a/ironic/drivers/modules/redfish/bios.py +++ b/ironic/drivers/modules/redfish/bios.py @@ -19,7 +19,6 @@ from oslo_utils import importutils from ironic.common import exception from ironic.common.i18n import _ from ironic.common import states -from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers import base from ironic.drivers.modules import deploy_utils @@ -185,9 +184,8 @@ class RedfishBIOS(base.BIOSInterface): LOG.error(error_msg) raise exception.RedfishError(error=error_msg) - self.post_reset(task) self._set_reboot(task) - return deploy_utils.get_async_step_return_state(task.node) + return self.post_reset(task) else: current_attrs = bios.attributes LOG.debug('Post factory reset, BIOS configuration for node ' @@ -244,9 +242,8 @@ class RedfishBIOS(base.BIOSInterface): LOG.error(error_msg) raise exception.RedfishError(error=error_msg) - self.post_configuration(task, settings) self._set_reboot_requested(task, attributes) - return deploy_utils.get_async_step_return_state(task.node) + return self.post_configuration(task, settings) else: # Step 2: Verify requested BIOS settings applied requested_attrs = info.get('requested_bios_attrs') @@ -267,8 +264,7 @@ class RedfishBIOS(base.BIOSInterface): :param task: a TaskManager instance containing the node to act on. """ - deploy_utils.prepare_agent_boot(task) - self._reboot(task) + return deploy_utils.reboot_to_finish_step(task) def post_configuration(self, task, settings): """Perform post configuration action to store the BIOS settings. @@ -281,8 +277,7 @@ class RedfishBIOS(base.BIOSInterface): :param task: a TaskManager instance containing the node to act on. :param settings: a list of BIOS settings to be updated. """ - deploy_utils.prepare_agent_boot(task) - self._reboot(task) + return deploy_utils.reboot_to_finish_step(task) def get_properties(self): """Return the properties of the interface. @@ -322,17 +317,6 @@ class RedfishBIOS(base.BIOSInterface): LOG.debug('Verification of BIOS settings for node %(node_uuid)s ' 'successful.', {'node_uuid': task.node.uuid}) - @task_manager.require_exclusive_lock - def _reboot(self, task): - """Reboot the target Redfish service. - - :param task: a TaskManager instance containing the node to act on. - :raises: InvalidParameterValue when the wrong state is specified - or the wrong driver info is specified. - :raises: RedfishError on an error from the Sushy library - """ - manager_utils.node_power_action(task, states.REBOOT) - def _set_reboot(self, task): """Set driver_internal_info flags for deployment or cleaning reboot. diff --git a/ironic/tests/unit/drivers/modules/redfish/test_bios.py b/ironic/tests/unit/drivers/modules/redfish/test_bios.py index cd6f9be5ff..f9146f3b67 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_bios.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_bios.py @@ -165,13 +165,16 @@ class RedfishBiosTestCase(db_base.DbTestCase): mock_setting_list.delete.assert_called_once_with( task.context, task.node.id, delete_names) + @mock.patch.object(manager_utils, 'is_fast_track', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk', spec_set=True, autospec=True) @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) def _test_step_pre_reboot(self, mock_power_action, mock_get_system, - mock_build_agent_options, mock_prepare): + mock_build_agent_options, mock_prepare, + mock_fast_track, fast_track=False): + mock_fast_track.return_value = fast_track if self.node.clean_step: step_data = self.node.clean_step check_fields = ['cleaning_reboot', 'skip_current_clean_step'] @@ -197,7 +200,13 @@ class RedfishBiosTestCase(db_base.DbTestCase): bios.supported_apply_times = [] ret = task.driver.bios.apply_configuration(task, data) mock_get_system.assert_called_with(task.node) - mock_power_action.assert_called_once_with(task, states.REBOOT) + if fast_track: + mock_power_action.assert_has_calls([ + mock.call(task, states.POWER_OFF), + mock.call(task, states.REBOOT), + ]) + else: + mock_power_action.assert_called_once_with(task, states.REBOOT) if step == 'factory_reset': bios.reset_bios.assert_called_once() if step == 'apply_configuration': @@ -205,8 +214,8 @@ class RedfishBiosTestCase(db_base.DbTestCase): attributes, apply_time=None) mock_build_agent_options.assert_called_once_with(task.node) mock_prepare.assert_called_once_with(mock.ANY, task, {'a': 'b'}) - info = task.node.driver_internal_info - self.assertTrue(all(x in info for x in check_fields)) + for field in check_fields: + self.assertIn(field, task.node.driver_internal_info) self.assertEqual(expected_ret, ret) def test_factory_reset_step_pre_reboot_cleaning(self): @@ -221,6 +230,12 @@ class RedfishBiosTestCase(db_base.DbTestCase): self.node.save() self._test_step_pre_reboot() + def test_factory_reset_step_pre_reboot_fast_track(self): + self.node.clean_step = {'priority': 100, 'interface': 'bios', + 'step': 'factory_reset', 'argsinfo': {}} + self.node.save() + self._test_step_pre_reboot(fast_track=True) + def test_apply_conf_step_pre_reboot_cleaning(self): data = [{'name': 'ProcTurboMode', 'value': 'Disabled'}, {'name': 'NicBoot1', 'value': 'NetworkBoot'}] @@ -239,6 +254,15 @@ class RedfishBiosTestCase(db_base.DbTestCase): self.node.save() self._test_step_pre_reboot() + def test_apply_conf_step_pre_reboot_fast_track(self): + data = [{'name': 'ProcTurboMode', 'value': 'Disabled'}, + {'name': 'NicBoot1', 'value': 'NetworkBoot'}] + self.node.clean_step = {'priority': 100, 'interface': 'bios', + 'step': 'apply_configuration', + 'argsinfo': {'settings': data}} + self.node.save() + self._test_step_pre_reboot(fast_track=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) def _test_step_post_reboot(self, mock_get_system, attributes_after_reboot=None): diff --git a/releasenotes/notes/fast-track-bios-fa9ae685c151dd24.yaml b/releasenotes/notes/fast-track-bios-fa9ae685c151dd24.yaml new file mode 100644 index 0000000000..b4a8004f72 --- /dev/null +++ b/releasenotes/notes/fast-track-bios-fa9ae685c151dd24.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes rebooting into the agent after changing BIOS settings in fast-track + mode with the ``redfish-virtual-media`` boot interface. Previously, the ISO + would not be configured.