From bc0cc8a8faf14a163e5751a353e1e1981afe4894 Mon Sep 17 00:00:00 2001 From: Shivanand Tendulker Date: Tue, 14 Jul 2020 03:24:01 -0400 Subject: [PATCH] Adds support SUM based firmware update as deploy step SUM based firmware update exists as an inband clean step. This commit adds it as inband deploy step to `ilo` and `ilo5` hardware types. Change-Id: I2ac03dc2148a56aa23e86c6adb29a16bac443de3 Story: #2007923 Task: #40337 --- driver-requirements.txt | 2 +- ironic/drivers/modules/ilo/management.py | 62 ++- .../drivers/modules/ilo/test_management.py | 363 ++++++++++++++---- ...e-firmware-using-sum-cfee84a19120dd3c.yaml | 11 + 4 files changed, 348 insertions(+), 90 deletions(-) create mode 100644 releasenotes/notes/add-ilo-inband-deploy-step-update-firmware-using-sum-cfee84a19120dd3c.yaml diff --git a/driver-requirements.txt b/driver-requirements.txt index bb016f26ee..539d27ba99 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -4,7 +4,7 @@ # python projects they should package as optional dependencies for Ironic. # These are available on pypi -proliantutils>=2.9.1 +proliantutils>=2.9.5 pysnmp>=4.3.0,<5.0.0 python-scciclient>=0.8.0 python-dracclient>=3.1.0,<5.0.0 diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index 45bac59a12..75b5934ea9 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -524,12 +524,43 @@ class IloManagement(base.ManagementInterface): @base.clean_step(priority=0, abortable=False, argsinfo=_FIRMWARE_UPDATE_SUM_ARGSINFO) def update_firmware_sum(self, task, **kwargs): - """Updates the firmware using Smart Update Manager (SUM). + """Clean step to update the firmware using Smart Update Manager (SUM) :param task: a TaskManager object. :raises: NodeCleaningFailure, on failure to execute of clean step. + :returns: states.CLEANWAIT to signify the step will be completed async + """ + return self._do_update_firmware_sum(task, **kwargs) + + @METRICS.timer('IloManagement.update_firmware_sum') + @base.deploy_step(priority=0, argsinfo=_FIRMWARE_UPDATE_SUM_ARGSINFO) + def flash_firmware_sum(self, task, **kwargs): + """Deploy step to Update the firmware using Smart Update Manager (SUM). + + :param task: a TaskManager object. + :raises: InstanceDeployFailure, on failure to execute of deploy step. + :returns: states.DEPLOYWAIT to signify the step will be completed + async + """ + return self._do_update_firmware_sum(task, **kwargs) + + def _do_update_firmware_sum(self, task, **kwargs): + """Update the firmware using Smart Update Manager (SUM). + + :param task: a TaskManager object. + :raises: NodeCleaningFailure or InstanceDeployFailure, on failure to + execute of clean or deploy step respectively. + :returns: states.CLEANWAIT or states.DEPLOYWAIT to signify the step + will be completed async for clean or deploy step respectively. """ node = task.node + if node.provision_state == states.DEPLOYING: + step = node.deploy_step + step_type = 'deploy' + else: + step = node.clean_step + step_type = 'clean' + # The arguments are validated and sent to the ProliantHardwareManager # to perform SUM based firmware update clean step. firmware_processor.get_and_validate_firmware_image_info(kwargs, @@ -538,24 +569,25 @@ class IloManagement(base.ManagementInterface): url = kwargs['url'] if urlparse.urlparse(url).scheme == 'swift': url = firmware_processor.get_swift_url(urlparse.urlparse(url)) - node.clean_step['args']['url'] = url + step['args']['url'] = url # Insert SPP ISO into virtual media CDROM ilo_common.attach_vmedia(node, 'CDROM', url) - step = node.clean_step - return agent_base.execute_clean_step(task, step) + return agent_base.execute_step(task, step, step_type) @staticmethod + @agent_base.post_deploy_step_hook( + interface='management', step='flash_firmware_sum') @agent_base.post_clean_step_hook( interface='management', step='update_firmware_sum') def _update_firmware_sum_final(task, command): - """Clean step hook after SUM based firmware update operation. + """Deploy/Clean step hook after SUM based firmware update operation. - This method is invoked as a post clean step hook by the Ironic - conductor once firmware update operaion is completed. The clean logs - are collected and stored according to the configured storage backend - when the node is configured to collect the logs. + This method is invoked as a post deploy/clean step hook by the Ironic + conductor once firmware update operaion is completed. The deploy/clean + logs are collected and stored according to the configured storage + backend when the node is configured to collect the logs. :param task: a TaskManager instance. :param command: A command result structure of the SUM based firmware @@ -565,12 +597,16 @@ class IloManagement(base.ManagementInterface): if not _should_collect_logs(command): return + if task.node.provision_state == states.DEPLOYWAIT: + log_data = command['command_result']['deploy_result']['Log Data'] + label = command['command_result']['deploy_step']['step'] + else: + log_data = command['command_result']['clean_result']['Log Data'] + label = command['command_result']['clean_step']['step'] + node = task.node try: - driver_utils.store_ramdisk_logs( - node, - command['command_result']['clean_result']['Log Data'], - label='update_firmware_sum') + driver_utils.store_ramdisk_logs(node, log_data, label=label) except exception.SwiftOperationError as e: LOG.error('Failed to store the logs from the node %(node)s ' 'for "update_firmware_sum" clean step in Swift. ' diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index e7cb060c9f..26b6830581 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -827,177 +827,388 @@ class IloManagementTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) - @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) - def test_update_firmware_sum_mode_with_component( - self, execute_mock, attach_vmedia_mock): + @mock.patch.object(agent_base, 'execute_step', autospec=True) + def _test_write_firmware_sum_mode_with_component( + self, execute_mock, attach_vmedia_mock, step_type='clean'): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - execute_mock.return_value = states.CLEANWAIT # | GIVEN | firmware_update_args = { 'url': 'http://any_url', 'checksum': 'xxxx', 'component': ['CP02345.scexe', 'CP02567.exe']} - clean_step = {'step': 'update_firmware', - 'interface': 'management', - 'args': firmware_update_args} - task.node.clean_step = clean_step + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + task.node.provision_state = states.CLEANING + execute_mock.return_value = states.CLEANWAIT + task.node.clean_step = step + func = task.driver.management.update_firmware_sum + exp_ret_state = states.CLEANWAIT + else: + step['step'] = 'flash_firmware_sum' + task.node.provision_state = states.DEPLOYING + execute_mock.return_value = states.DEPLOYWAIT + task.node.deploy_step = step + func = task.driver.management.flash_firmware_sum + exp_ret_state = states.DEPLOYWAIT # | WHEN | - return_value = task.driver.management.update_firmware_sum( - task, **firmware_update_args) + return_value = func(task, **firmware_update_args) # | THEN | attach_vmedia_mock.assert_any_call( task.node, 'CDROM', 'http://any_url') - self.assertEqual(states.CLEANWAIT, return_value) - execute_mock.assert_called_once_with(task, clean_step) + self.assertEqual(exp_ret_state, return_value) + execute_mock.assert_called_once_with(task, step, step_type) + + def test_update_firmware_sum_mode_with_component(self): + self._test_write_firmware_sum_mode_with_component(step_type='clean') + + def test_flash_firmware_sum_mode_with_component(self): + self._test_write_firmware_sum_mode_with_component(step_type='deploy') @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) @mock.patch.object(ilo_management.firmware_processor, 'get_swift_url', autospec=True) - @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) - def test_update_firmware_sum_mode_swift_url( - self, execute_mock, swift_url_mock, attach_vmedia_mock): + @mock.patch.object(agent_base, 'execute_step', autospec=True) + def _test_write_firmware_sum_mode_swift_url( + self, execute_mock, swift_url_mock, attach_vmedia_mock, + step_type='clean'): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - swift_url_mock.return_value = "http://path-to-file" - execute_mock.return_value = states.CLEANWAIT # | GIVEN | + swift_url_mock.return_value = "http://path-to-file" firmware_update_args = { 'url': 'swift://container/object', 'checksum': 'xxxx', 'components': ['CP02345.scexe', 'CP02567.exe']} - clean_step = {'step': 'update_firmware', - 'interface': 'management', - 'args': firmware_update_args} - task.node.clean_step = clean_step + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + task.node.provision_state = states.CLEANING + execute_mock.return_value = states.CLEANWAIT + step['step'] = 'update_firmware_sum', + task.node.clean_step = step + func = task.driver.management.update_firmware_sum + exp_ret_state = states.CLEANWAIT + args_data = task.node.clean_step['args'] + else: + task.node.provision_state = states.DEPLOYING + execute_mock.return_value = states.DEPLOYWAIT + step['step'] = 'flash_firmware_sum', + task.node.deploy_step = step + func = task.driver.management.flash_firmware_sum + exp_ret_state = states.DEPLOYWAIT + args_data = task.node.deploy_step['args'] # | WHEN | - return_value = task.driver.management.update_firmware_sum( - task, **firmware_update_args) + return_value = func(task, **firmware_update_args) # | THEN | attach_vmedia_mock.assert_any_call( task.node, 'CDROM', 'http://path-to-file') - self.assertEqual(states.CLEANWAIT, return_value) - self.assertEqual(task.node.clean_step['args']['url'], - "http://path-to-file") + self.assertEqual(exp_ret_state, return_value) + self.assertEqual(args_data['url'], "http://path-to-file") + + def test_write_firmware_sum_mode_swift_url_clean(self): + self._test_write_firmware_sum_mode_swift_url(step_type='clean') + + def test_write_firmware_sum_mode_swift_url_deploy(self): + self._test_write_firmware_sum_mode_swift_url(step_type='deploy') @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) - @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) - def test_update_firmware_sum_mode_without_component( - self, execute_mock, attach_vmedia_mock): + @mock.patch.object(agent_base, 'execute_step', autospec=True) + def _test_write_firmware_sum_mode_without_component( + self, execute_mock, attach_vmedia_mock, step_type='clean'): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - execute_mock.return_value = states.CLEANWAIT # | GIVEN | firmware_update_args = { 'url': 'any_valid_url', 'checksum': 'xxxx'} - clean_step = {'step': 'update_firmware', - 'interface': 'management', - 'args': firmware_update_args} - task.node.clean_step = clean_step + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + task.node.provision_state = states.CLEANING + execute_mock.return_value = states.CLEANWAIT + step['step'] = 'update_firmware_sum' + task.node.clean_step = step + func = task.driver.management.update_firmware_sum + exp_ret_state = states.CLEANWAIT + else: + task.node.provision_state = states.DEPLOYING + execute_mock.return_value = states.DEPLOYWAIT + step['step'] = 'flash_firmware_sum' + task.node.deploy_step = step + func = task.driver.management.flash_firmware_sum + exp_ret_state = states.DEPLOYWAIT # | WHEN | - return_value = task.driver.management.update_firmware_sum( - task, **firmware_update_args) + return_value = func(task, **firmware_update_args) # | THEN | attach_vmedia_mock.assert_any_call( task.node, 'CDROM', 'any_valid_url') - self.assertEqual(states.CLEANWAIT, return_value) - execute_mock.assert_called_once_with(task, clean_step) + self.assertEqual(exp_ret_state, return_value) + execute_mock.assert_called_once_with(task, step, step_type) - def test_update_firmware_sum_mode_invalid_component(self): + def test_write_firmware_sum_mode_without_component_clean(self): + self._test_write_firmware_sum_mode_without_component( + step_type='clean') + + def test_write_firmware_sum_mode_without_component_deploy(self): + self._test_write_firmware_sum_mode_without_component( + step_type='deploy') + + def _test_write_firmware_sum_mode_invalid_component(self, + step_type='clean'): + # | GIVEN | + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx', + 'components': ['CP02345']} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - # | GIVEN | - firmware_update_args = { - 'url': 'any_valid_url', - 'checksum': 'xxxx', - 'components': ['CP02345']} # | WHEN & THEN | + if step_type == 'clean': + func = task.driver.management.update_firmware_sum + else: + func = task.driver.management.flash_firmware_sum self.assertRaises(exception.InvalidParameterValue, - task.driver.management.update_firmware_sum, - task, - **firmware_update_args) + func, task, **firmware_update_args) + + def test_write_firmware_sum_mode_invalid_component_clean(self): + self._test_write_firmware_sum_mode_invalid_component( + step_type='clean') + + def test_write_firmware_sum_mode_invalid_component_deploy(self): + self._test_write_firmware_sum_mode_invalid_component( + step_type='deploy') @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_with_logs(self, store_mock): + def _test__write_firmware_sum_final_with_logs(self, store_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + exp_label = 'update_firmware_sum' + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } + exp_label = 'flash_firmware_sum' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) store_mock.assert_called_once_with(task.node, 'aaaabbbbcccdddd', - label='update_firmware_sum') + label=exp_label) + + def test__write_firmware_sum_final_with_logs_clean(self): + self._test__write_firmware_sum_final_with_logs(step_type='clean') + + def test__write_firmware_sum_final_with_logs_deploy(self): + self._test__write_firmware_sum_final_with_logs(step_type='deploy') @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_without_logs(self, store_mock): + def _test__write_firmware_sum_final_without_logs(self, store_mock, + step_type='clean'): self.config(deploy_logs_collect='on_failure', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.management._update_firmware_sum_final( task, command) self.assertFalse(store_mock.called) + def test__write_firmware_sum_final_without_logs_clean(self): + self._test__write_firmware_sum_final_without_logs(step_type='clean') + + def test__write_firmware_sum_final_without_logs_deploy(self): + self._test__write_firmware_sum_final_without_logs(step_type='deploy') + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_swift_error(self, store_mock, - log_mock): + def _test__write_firmware_sum_final_swift_error(self, store_mock, + log_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } store_mock.side_effect = exception.SwiftOperationError('Error') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) self.assertTrue(log_mock.error.called) + def test__write_firmware_sum_final_swift_error_clean(self): + self._test__write_firmware_sum_final_swift_error(step_type='clean') + + def test__write_firmware_sum_final_swift_error_deploy(self): + self._test__write_firmware_sum_final_swift_error(step_type='deploy') + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_environment_error(self, store_mock, - log_mock): + def _test__write_firmware_sum_final_environment_error(self, store_mock, + log_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } store_mock.side_effect = EnvironmentError('Error') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) self.assertTrue(log_mock.exception.called) + def test__write_firmware_sum_final_environment_error_clean(self): + self._test__write_firmware_sum_final_environment_error( + step_type='clean') + + def test__write_firmware_sum_final_environment_error_deploy(self): + self._test__write_firmware_sum_final_environment_error( + step_type='deploy') + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_unknown_exception(self, store_mock, - log_mock): + def _test__write_firmware_sum_final_unknown_exception(self, store_mock, + log_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } store_mock.side_effect = Exception('Error') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) self.assertTrue(log_mock.exception.called) + def test__write_firmware_sum_final_unknown_exception_clean(self): + self._test__write_firmware_sum_final_unknown_exception( + step_type='clean') + + def test__write_firmware_sum_final_unknown_exception_deploy(self): + self._test__write_firmware_sum_final_unknown_exception( + step_type='deploy') + @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True, autospec=True) def test_set_iscsi_boot_target_with_auth(self, get_ilo_object_mock): diff --git a/releasenotes/notes/add-ilo-inband-deploy-step-update-firmware-using-sum-cfee84a19120dd3c.yaml b/releasenotes/notes/add-ilo-inband-deploy-step-update-firmware-using-sum-cfee84a19120dd3c.yaml new file mode 100644 index 0000000000..fe1956e92e --- /dev/null +++ b/releasenotes/notes/add-ilo-inband-deploy-step-update-firmware-using-sum-cfee84a19120dd3c.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Adds inband deploy step ``flash_firmware_sum`` to the ``management`` + interface of the ``ilo`` and ``ilo5`` hardware types. The required + minimum version for the proliantutils library is 2.9.5. +other: + - | + The proliantutils library version 2.9.5 enables ``ssacli`` based + in-band deploy step ``apply_configuration`` of ``agent`` RAID + interface for ``ilo`` and ``ilo5`` hardware types.