From 524f8aa7929bc1692b9e75ed525518e29fe9e37d Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 24 Apr 2020 17:42:57 +0200 Subject: [PATCH] In-band deploy steps: correctly wipe driver_internal_info Also fix a docstring and add a missing TODO. Follow-up to commit 4795c4a8b8896129228fba5a09b87d0551f6e140. Change-Id: I99c28776d135c57311c6a9d4bbd799cc2b642839 Story: #2006963 --- ironic/conductor/deployments.py | 16 +++-------- ironic/conductor/utils.py | 30 ++++++++++++++------- ironic/drivers/modules/agent_base.py | 3 ++- ironic/tests/unit/conductor/test_manager.py | 2 +- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index 3c7eadb2bf..ef53752487 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -87,14 +87,13 @@ def start_deploy(task, manager, configdrive=None, event='deploy'): instance_info.pop('ramdisk', None) node.instance_info = instance_info - driver_internal_info = node.driver_internal_info - driver_internal_info.pop('steps_validated', None) # Infer the image type to make sure the deploy driver # validates only the necessary variables for different # image types. # NOTE(sirushtim): The iwdi variable can be None. It's up to # the deploy driver to validate this. iwdi = images.is_whole_disk_image(task.context, node.instance_info) + driver_internal_info = node.driver_internal_info driver_internal_info['is_whole_disk_image'] = iwdi node.driver_internal_info = driver_internal_info node.save() @@ -128,6 +127,7 @@ def start_deploy(task, manager, configdrive=None, event='deploy'): def do_node_deploy(task, conductor_id=None, configdrive=None): """Prepare the environment and deploy a node.""" node = task.node + utils.wipe_deploy_internal_info(node) utils.del_secret_token(node) try: if configdrive: @@ -308,17 +308,7 @@ def do_next_deploy_step(task, step_index, conductor_id): # Finished executing the steps. Clear deploy_step. node.deploy_step = None - driver_internal_info = node.driver_internal_info - driver_internal_info.pop('agent_secret_token', None) - driver_internal_info.pop('agent_secret_token_pregenerated', None) - driver_internal_info['deploy_steps'] = None - driver_internal_info.pop('deploy_step_index', None) - driver_internal_info.pop('deployment_reboot', None) - driver_internal_info.pop('deployment_polling', None) - driver_internal_info.pop('steps_validated', None) - # Remove the agent_url cached from the deployment. - driver_internal_info.pop('agent_url', None) - node.driver_internal_info = driver_internal_info + utils.wipe_deploy_internal_info(node) node.save() _start_console_in_deploy(task) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index ca399b651b..836409dad4 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -444,6 +444,25 @@ def cleaning_error_handler(task, msg, tear_down_cleaning=True, task.process_event('fail', target_state=target_state) +def wipe_deploy_internal_info(node): + """Remove temporary deployment fields from driver_internal_info.""" + info = node.driver_internal_info + info.pop('agent_secret_token', None) + info.pop('agent_secret_token_pregenerated', None) + # Clear any leftover metadata about deployment. + info['deploy_steps'] = None + info.pop('agent_cached_deploy_steps', None) + info.pop('deploy_step_index', None) + info.pop('deployment_reboot', None) + info.pop('deployment_polling', None) + info.pop('skip_current_deploy_step', None) + info.pop('steps_validated', None) + # Remove agent_url since it will be re-asserted + # upon the next deployment attempt. + info.pop('agent_url', None) + node.driver_internal_info = info + + def deploying_error_handler(task, logmsg, errmsg=None, traceback=False, clean_up=True): """Put a failed node in DEPLOYFAIL. @@ -484,16 +503,7 @@ def deploying_error_handler(task, logmsg, errmsg=None, traceback=False, # Clear deploy step; we leave the list of deploy steps # in node.driver_internal_info for debugging purposes. node.deploy_step = {} - info = node.driver_internal_info - # Clear any leftover metadata about deployment. - info.pop('deploy_step_index', None) - info.pop('deployment_reboot', None) - info.pop('deployment_polling', None) - info.pop('skip_current_deploy_step', None) - # Remove agent_url since it will be re-asserted - # upon the next deployment attempt. - info.pop('agent_url', None) - node.driver_internal_info = info + wipe_deploy_internal_info(node) if cleanup_err: node.last_error = cleanup_err diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index 1b7eeb6252..448a6f4254 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -713,6 +713,7 @@ class AgentDeployMixin(HeartbeatMixin): 'steps': previous_steps}) call = getattr(self._client, 'get_%s_steps' % step_type) + # TODO(dtantsur): remove the error handling in the V release. try: agent_result = call(node, task.ports).get('command_result', {}) except exception.AgentAPIError as exc: @@ -790,7 +791,7 @@ class AgentDeployMixin(HeartbeatMixin): :param task: a TaskManager object containing the node :param step: a deploy step dictionary to execute - :raises: NodeCleaningFailure if the agent does not return a command + :raises: InstanceDeployFailure if the agent does not return a command status :returns: states.DEPLOYWAIT to signify the step will be completed async """ diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index d95aa83d3f..708cf452c2 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -1468,7 +1468,7 @@ class ServiceDoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, mock_iwdi.assert_called_once_with(self.context, node.instance_info) # Verify is_whole_disk_image reflects correct value on rebuild. self.assertTrue(node.driver_internal_info['is_whole_disk_image']) - self.assertEqual(1, len(node.driver_internal_info['deploy_steps'])) + self.assertIsNone(node.driver_internal_info['deploy_steps']) def test_do_node_deploy_rebuild_active_state_waiting(self, mock_iwdi): mock_iwdi.return_value = False