From 62ff8a949f77ddf5e54eeb2a640de2884a1d15d2 Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Wed, 26 Jun 2024 22:37:55 +1000 Subject: [PATCH] Make redfish firmware update a service step This commit makes changes neccessary for redfish.firmware.update to work as both clean_step and service_step. This is done by adding a service step decorator, adding conditional code to pass the execution to the appropriate subsequent functions and modifying the periodics used to handle async tasks. Change-Id: I20a40127f66f734005a03365b806310a155dc237 --- ironic/drivers/modules/redfish/firmware.py | 21 ++- .../drivers/modules/redfish/test_firmware.py | 127 ++++++++++++++++++ ...-update-service-step-885f47cf051b57ee.yaml | 6 + 3 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/make-redfish-firmware-update-service-step-885f47cf051b57ee.yaml diff --git a/ironic/drivers/modules/redfish/firmware.py b/ironic/drivers/modules/redfish/firmware.py index cd3dcc4866..ed5d96572a 100644 --- a/ironic/drivers/modules/redfish/firmware.py +++ b/ironic/drivers/modules/redfish/firmware.py @@ -144,6 +144,9 @@ class RedfishFirmware(base.FirmwareInterface): @base.clean_step(priority=0, abortable=False, argsinfo=_FW_SETTINGS_ARGSINFO, requires_ramdisk=True) + @base.service_step(priority=0, abortable=False, + argsinfo=_FW_SETTINGS_ARGSINFO, + requires_ramdisk=True) @base.cache_firmware_components def update(self, task, settings): """Update the Firmware on the node using the settings for components. @@ -252,8 +255,13 @@ class RedfishFirmware(base.FirmwareInterface): LOG.info('Firmware updates completed for node %(node)s', {'node': node.uuid}) + if task.node.clean_step: + manager_utils.notify_conductor_resume_clean(task) + elif task.node.service_step: + manager_utils.notify_conductor_resume_service(task) + elif task.node.deploy_step: + manager_utils.notify_conductor_resume_deploy(task) - manager_utils.notify_conductor_resume_clean(task) else: settings.pop(0) self._execute_firmware_update(node, @@ -281,8 +289,8 @@ class RedfishFirmware(base.FirmwareInterface): @periodics.node_periodic( purpose='checking if async update of firmware component failed', spacing=CONF.redfish.firmware_update_fail_interval, - filters={'reserved': False, 'provision_state': states.CLEANFAIL, - 'maintenance': True}, + filters={'reserved': False, 'provision_state_in': [states.CLEANFAIL, + states.DEPLOYFAIL, states.SERVICEFAIL], 'maintenance': True}, predicate_extra_fields=['driver_internal_info'], predicate=lambda n: n.driver_internal_info.get('redfish_fw_updates'), ) @@ -304,7 +312,8 @@ class RedfishFirmware(base.FirmwareInterface): @periodics.node_periodic( purpose='checking async update of firmware component', spacing=CONF.redfish.firmware_update_fail_interval, - filters={'reserved': False, 'provision_state': states.CLEANWAIT}, + filters={'reserved': False, 'provision_state_in': [states.CLEANWAIT, + states.DEPLOYWAIT, states.SERVICEWAIT]}, predicate_extra_fields=['driver_internal_info'], predicate=lambda n: n.driver_internal_info.get('redfish_fw_updates'), ) @@ -407,8 +416,10 @@ class RedfishFirmware(base.FirmwareInterface): self._clear_updates(node) if task.node.clean_step: manager_utils.cleaning_error_handler(task, error_msg) - else: + elif task.node.deploy_step: manager_utils.deploying_error_handler(task, error_msg) + elif task.node.service_step: + manager_utils.servicing_error_handler(task, error_msg) else: LOG.debug('Firmware update in progress for node %(node)s, ' diff --git a/ironic/tests/unit/drivers/modules/redfish/test_firmware.py b/ironic/tests/unit/drivers/modules/redfish/test_firmware.py index 76825b6e88..b6229a5aa1 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_firmware.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_firmware.py @@ -281,6 +281,18 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): task, settings) log_mock.debug.assert_not_called() + @mock.patch.object(redfish_fw, 'LOG', autospec=True) + def _test_invalid_settings_service(self, log_mock): + step = self.node.service_step + settings = step['argsinfo'].get('settings', None) + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises( + exception.InvalidParameterValue, + task.driver.firmware.update, + task, settings) + log_mock.debug.assert_not_called() + def test_invalid_component_in_settings(self): argsinfo = {'settings': [ {'component': 'nic', 'url': 'https://nic-update/v1.1.0'} @@ -291,6 +303,16 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): self.node.save() self._test_invalid_settings() + def test_invalid_component_in_settings_service(self): + argsinfo = {'settings': [ + {'component': 'nic', 'url': 'https://nic-update/v1.1.0'} + ]} + self.node.service_step = {'priority': 100, 'interface': 'firmware', + 'step': 'update', + 'argsinfo': argsinfo} + self.node.save() + self._test_invalid_settings_service() + def test_missing_required_field_in_settings(self): argsinfo = {'settings': [ {'url': 'https://nic-update/v1.1.0'}, @@ -302,6 +324,17 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): self.node.save() self._test_invalid_settings() + def test_missing_required_field_in_settings_service(self): + argsinfo = {'settings': [ + {'url': 'https://nic-update/v1.1.0'}, + {'component': "bmc"} + ]} + self.node.service_step = {'priority': 100, 'interface': 'firmware', + 'step': 'update', + 'argsinfo': argsinfo} + self.node.save() + self._test_invalid_settings_service() + def test_empty_settings(self): argsinfo = {'settings': []} self.node.clean_step = {'priority': 100, 'interface': 'firmware', @@ -310,6 +343,14 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): self.node.save() self._test_invalid_settings() + def test_empty_settings_service(self): + argsinfo = {'settings': []} + self.node.service_step = {'priority': 100, 'interface': 'firmware', + 'step': 'update', + 'argsinfo': argsinfo} + self.node.save() + self._test_invalid_settings_service() + def _generate_new_driver_internal_info(self, components=[], invalid=False, add_wait=False, wait=1): bmc_component = {'component': 'bmc', 'url': 'https://bmc/v1.0.1'} @@ -348,6 +389,45 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): } self.node.save() + def _generate_new_driver_internal_info_service(self, components=[], + invalid=False, + add_wait=False, wait=1): + bmc_component = {'component': 'bmc', 'url': 'https://bmc/v1.0.1'} + bios_component = {'component': 'bios', 'url': 'https://bios/v1.0.1'} + if add_wait: + wait_start_time = timeutils.utcnow() -\ + datetime.timedelta(minutes=1) + bmc_component['wait_start_time'] = wait_start_time.isoformat() + bios_component['wait_start_time'] = wait_start_time.isoformat() + bmc_component['wait'] = wait + bios_component['wait'] = wait + + self.node.service_step = {'priority': 100, 'interface': 'bios', + 'step': 'apply_configuration', + 'argsinfo': {'settings': []}} + + updates = [] + if 'bmc' in components: + self.node.service_step['argsinfo']['settings'].append( + bmc_component) + bmc_component['task_monitor'] = '/task/1' + updates.append(bmc_component) + if 'bios' in components: + self.node.service_step['argsinfo']['settings'].append( + bios_component) + bios_component['task_monitor'] = '/task/2' + updates.append(bios_component) + + if invalid: + self.node.provision_state = states.SERVICING + self.node.driver_internal_info = {'something': 'else'} + else: + self.node.provision_state = states.SERVICING + self.node.driver_internal_info = { + 'redfish_fw_updates': updates, + } + self.node.save() + @mock.patch.object(task_manager, 'acquire', autospec=True) def _test__query_methods(self, acquire_mock): firmware = redfish_fw.RedfishFirmware() @@ -522,6 +602,37 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): cleaning_error_handler_mock.assert_called_once() interface._continue_updates.assert_not_called() + @mock.patch.object(manager_utils, 'servicing_error_handler', + autospec=True) + @mock.patch.object(redfish_utils, 'get_update_service', autospec=True) + @mock.patch.object(redfish_utils, 'get_task_monitor', autospec=True) + def test__check_node_firmware_update_fail_servicing( + self, tm_mock, + get_us_mock, + servicing_error_handler_mock): + + mock_sushy_task = mock.Mock() + mock_sushy_task.task_state = 'exception' + mock_message_unparsed = mock.Mock() + mock_message_unparsed.message = None + message_mock = mock.Mock() + message_mock.message = 'Firmware upgrade failed' + messages = mock.MagicMock(return_value=[[mock_message_unparsed], + [message_mock], + [message_mock]]) + mock_sushy_task.messages = messages + mock_task_monitor = mock.Mock() + mock_task_monitor.is_processing = False + mock_task_monitor.get_task.return_value = mock_sushy_task + tm_mock.return_value = mock_task_monitor + self._generate_new_driver_internal_info_service(['bmc']) + + task, interface = self._test__check_node_redfish_firmware_update() + + task.upgrade_lock.assert_called_once_with() + servicing_error_handler_mock.assert_called_once() + interface._continue_updates.assert_not_called() + @mock.patch.object(redfish_fw, 'LOG', autospec=True) @mock.patch.object(redfish_utils, 'get_update_service', autospec=True) @mock.patch.object(redfish_utils, 'get_task_monitor', autospec=True) @@ -661,6 +772,22 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): ] log_mock.info.assert_has_calls(info_call) + @mock.patch.object(redfish_fw, 'LOG', autospec=True) + @mock.patch.object(manager_utils, 'notify_conductor_resume_service', + autospec=True) + def test_continue_updates_last_service(self, cond_resume_service_mock, + log_mock): + self._generate_new_driver_internal_info_service(['bmc']) + task = self._test_continue_updates() + + cond_resume_service_mock.assert_called_once_with(task) + + info_call = [ + mock.call('Firmware updates completed for node %(node)s', + {'node': self.node.uuid}) + ] + log_mock.info.assert_has_calls(info_call) + @mock.patch.object(redfish_fw, 'LOG', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) def test_continue_updates_more_updates(self, node_power_action_mock, diff --git a/releasenotes/notes/make-redfish-firmware-update-service-step-885f47cf051b57ee.yaml b/releasenotes/notes/make-redfish-firmware-update-service-step-885f47cf051b57ee.yaml new file mode 100644 index 0000000000..d8d3cf2eab --- /dev/null +++ b/releasenotes/notes/make-redfish-firmware-update-service-step-885f47cf051b57ee.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Makes redfish driver firmware update feature a service step, enabling + operators to perform firmware updates on active nodes. +