diff --git a/ironic/drivers/modules/redfish/bios.py b/ironic/drivers/modules/redfish/bios.py index 6139f9ff89..f80714aedf 100644 --- a/ironic/drivers/modules/redfish/bios.py +++ b/ironic/drivers/modules/redfish/bios.py @@ -22,6 +22,7 @@ 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 from ironic.drivers.modules.redfish import utils as redfish_utils from ironic import objects @@ -97,19 +98,39 @@ class RedfishBIOS(base.BIOSInterface): :raises: RedfishError on an error from the Sushy library """ system = redfish_utils.get_system(task.node) - LOG.debug('Factory reset BIOS settings for node %(node_uuid)s', - {'node_uuid': task.node.uuid}) try: - system.bios.reset_bios() - except sushy.exceptions.SushyError as e: + bios = system.bios + except sushy.exceptions.MissingAttributeError: error_msg = (_('Redfish BIOS factory reset failed for node ' - '%(node)s. Error: %(error)s') % - {'node': task.node.uuid, 'error': e}) + '%s, because BIOS settings are not supported.') % + task.node.uuid) LOG.error(error_msg) raise exception.RedfishError(error=error_msg) - self.post_reset(task) - self._set_cleaning_reboot(task) + node = task.node + info = node.driver_internal_info + reboot_requested = info.get('post_factory_reset_reboot_requested') + if not reboot_requested: + LOG.debug('Factory reset BIOS configuration for node %(node)s', + {'node': node.uuid}) + try: + bios.reset_bios() + except sushy.exceptions.SushyError as e: + error_msg = (_('Redfish BIOS factory reset failed for node ' + '%(node)s. Error: %(error)s') % + {'node': node.uuid, 'error': e}) + LOG.error(error_msg) + raise exception.RedfishError(error=error_msg) + + self.post_reset(task) + self._set_cleaning_reboot(task) + return states.CLEANWAIT + else: + current_attrs = bios.attributes + LOG.debug('Post factory reset, BIOS configuration for node ' + '%(node_uuid)s: %(attrs)r', + {'node_uuid': node.uuid, 'attrs': current_attrs}) + self._clear_reboot_requested(task) @base.clean_step(priority=0, argsinfo={ 'settings': { @@ -182,6 +203,8 @@ class RedfishBIOS(base.BIOSInterface): :param task: a TaskManager instance containing the node to act on. """ + deploy_opts = deploy_utils.build_agent_options(task.node) + task.driver.boot.prepare_ramdisk(task, deploy_opts) self._reboot(task) def post_configuration(self, task, settings): @@ -195,6 +218,8 @@ 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_opts = deploy_utils.build_agent_options(task.node) + task.driver.boot.prepare_ramdisk(task, deploy_opts) self._reboot(task) def get_properties(self): @@ -252,7 +277,9 @@ class RedfishBIOS(base.BIOSInterface): :param task: a TaskManager instance containing the node to act on. """ info = task.node.driver_internal_info + info['post_factory_reset_reboot_requested'] = True info['cleaning_reboot'] = True + info['skip_current_clean_step'] = False task.node.driver_internal_info = info task.node.save() @@ -276,10 +303,9 @@ class RedfishBIOS(base.BIOSInterface): :param task: a TaskManager instance containing the node to act on. """ info = task.node.driver_internal_info - if 'post_config_reboot_requested' in info: - del info['post_config_reboot_requested'] - if 'requested_bios_attrs' in info: - del info['requested_bios_attrs'] + info.pop('post_config_reboot_requested', None) + info.pop('post_factory_reset_reboot_requested', None) + info.pop('requested_bios_attrs', None) task.node.driver_internal_info = info task.node.save() diff --git a/ironic/tests/unit/drivers/modules/redfish/test_bios.py b/ironic/tests/unit/drivers/modules/redfish/test_bios.py index f60c398425..80f6fc0f50 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_bios.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_bios.py @@ -19,6 +19,8 @@ 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 deploy_utils +from ironic.drivers.modules import pxe as pxe_boot from ironic.drivers.modules.redfish import bios as redfish_bios from ironic.drivers.modules.redfish import utils as redfish_utils from ironic import objects @@ -158,9 +160,14 @@ class RedfishBiosTestCase(db_base.DbTestCase): mock_setting_list.delete.assert_called_once_with( task.context, task.node.id, delete_names) + @mock.patch.object(pxe_boot.PXEBoot, '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_factory_reset(self, mock_power_action, mock_get_system): + def test_factory_reset_step1(self, mock_power_action, mock_get_system, + mock_build_agent_options, mock_prepare): + mock_build_agent_options.return_value = {'a': 'b'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.bios.factory_reset(task) @@ -168,6 +175,26 @@ class RedfishBiosTestCase(db_base.DbTestCase): mock_power_action.assert_called_once_with(task, states.REBOOT) bios = mock_get_system(task.node).bios bios.reset_bios.assert_called_once() + 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 ( + 'post_factory_reset_reboot_requested', 'cleaning_reboot', + 'skip_current_clean_step'))) + + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_factory_reset_step2(self, mock_get_system): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + driver_internal_info = task.node.driver_internal_info + driver_internal_info['post_factory_reset_reboot_requested'] = True + task.node.driver_internal_info = driver_internal_info + task.node.save() + task.driver.bios.factory_reset(task) + mock_get_system.assert_called_with(task.node) + info = task.node.driver_internal_info + self.assertNotIn('post_factory_reset_reboot_requested', info) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_factory_reset_fail(self, mock_get_system): @@ -188,13 +215,19 @@ class RedfishBiosTestCase(db_base.DbTestCase): exception.RedfishError, 'BIOS factory reset failed', task.driver.bios.factory_reset, task) + @mock.patch.object(pxe_boot.PXEBoot, '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_apply_configuration_step1(self, mock_power_action, - mock_get_system): + mock_get_system, + mock_build_agent_options, + mock_prepare): settings = [{'name': 'ProcTurboMode', 'value': 'Disabled'}, {'name': 'NicBoot1', 'value': 'NetworkBoot'}] attributes = {s['name']: s['value'] for s in settings} + mock_build_agent_options.return_value = {'a': 'b'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.bios.apply_configuration(task, settings) @@ -202,6 +235,13 @@ class RedfishBiosTestCase(db_base.DbTestCase): mock_power_action.assert_called_once_with(task, states.REBOOT) bios = mock_get_system(task.node).bios bios.set_attributes.assert_called_once_with(attributes) + 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 ( + 'post_config_reboot_requested', 'cleaning_reboot', + 'skip_current_clean_step'))) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_apply_configuration_step2(self, mock_get_system): @@ -210,15 +250,16 @@ class RedfishBiosTestCase(db_base.DbTestCase): requested_attrs = {'ProcTurboMode': 'Enabled'} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node.driver_internal_info[ - 'post_config_reboot_requested'] = True - task.node.driver_internal_info[ - 'requested_bios_attrs'] = requested_attrs - task.driver.bios._clear_reboot_requested = mock.MagicMock() + driver_internal_info = task.node.driver_internal_info + driver_internal_info['post_config_reboot_requested'] = True + driver_internal_info['requested_bios_attrs'] = requested_attrs + task.node.driver_internal_info = driver_internal_info + task.node.save() task.driver.bios.apply_configuration(task, settings) mock_get_system.assert_called_with(task.node) - task.driver.bios._clear_reboot_requested\ - .assert_called_once_with(task) + info = task.node.driver_internal_info + self.assertNotIn('post_config_reboot_requested', info) + self.assertNotIn('requested_bios_attrs', info) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_apply_configuration_not_supported(self, mock_get_system): diff --git a/releasenotes/notes/story-2006217-redfish-bios-cleaning-fails-fee32f04dd97cbd2.yaml b/releasenotes/notes/story-2006217-redfish-bios-cleaning-fails-fee32f04dd97cbd2.yaml new file mode 100644 index 0000000000..98685a9811 --- /dev/null +++ b/releasenotes/notes/story-2006217-redfish-bios-cleaning-fails-fee32f04dd97cbd2.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where clean steps of ``redfish`` BIOS interface do not + boot up the IPA ramdisk after cleaning reboot. See `story 2006217 + `__ for details.