In-band deploy steps: correctly wipe driver_internal_info

Also fix a docstring and add a missing TODO.
Follow-up to commit 4795c4a8b8.

Change-Id: I99c28776d135c57311c6a9d4bbd799cc2b642839
Story: #2006963

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)

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

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
         """

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