Merge "Distinguish between prepare + deploy errors"

ironic/conductor/manager.py

@@ -1361,16 +1361,30 @@ def do_node_deploy(task, conductor_id):
     """Prepare the environment and deploy a node."""
     node = task.node
     try:
-        task.driver.deploy.prepare(task)
-        new_state = task.driver.deploy.deploy(task)
-    except Exception as e:
-        # NOTE(deva): there is no need to clear conductor_affinity
-        with excutils.save_and_reraise_exception():
+        def handle_failure(e, task, logmsg, errmsg):
+            # NOTE(deva): there is no need to clear conductor_affinity
             task.process_event('fail')
-            LOG.warning(_LW('Error in deploy of node %(node)s: %(err)s'),
-                        {'node': task.node.uuid, 'err': e})
-            node.last_error = _("Failed to deploy. Error: %s") % e
-    else:
+            args = {'node': task.node.uuid, 'err': e}
+            LOG.warning(logmsg, args)
+            node.last_error = errmsg % e
+
+        try:
+            task.driver.deploy.prepare(task)
+        except Exception as e:
+            with excutils.save_and_reraise_exception():
+                handle_failure(e, task,
+                    _LW('Error while preparing to deploy to node %(node)s: '
+                        '%(err)s'),
+                    _("Failed to prepare to deploy. Error: %s"))
+
+        try:
+            new_state = task.driver.deploy.deploy(task)
+        except Exception as e:
+            with excutils.save_and_reraise_exception():
+                handle_failure(e, task,
+                    _LW('Error in deploy of node %(node)s: %(err)s'),
+                    _("Failed to deploy. Error: %s"))
+
         # Update conductor_affinity to reference this conductor's ID
         # since there may be local persistent state
         node.conductor_affinity = conductor_id

ironic/tests/conductor/test_manager.py

@@ -930,6 +930,31 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
     def test_do_node_deploy_power_validate_fail(self, mock_validate):
         self._test_do_node_deploy_validate_fail(mock_validate)
 
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy')
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.prepare')
+    def test__do_node_deploy_driver_raises_prepare_error(self, mock_prepare,
+                                                         mock_deploy):
+        self._start_service()
+        # test when driver.deploy.prepare raises an exception
+        mock_prepare.side_effect = exception.InstanceDeployFailure('test')
+        node = obj_utils.create_test_node(self.context, driver='fake',
+                                          provision_state=states.DEPLOYING,
+                                          target_provision_state=states.ACTIVE)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        self.assertRaises(exception.InstanceDeployFailure,
+                          manager.do_node_deploy, task,
+                          self.service.conductor.id)
+        node.refresh()
+        self.assertEqual(states.DEPLOYFAIL, node.provision_state)
+        # NOTE(deva): failing a deploy does not clear the target state
+        #             any longer. Instead, it is cleared when the instance
+        #             is deleted.
+        self.assertEqual(states.ACTIVE, node.target_provision_state)
+        self.assertIsNotNone(node.last_error)
+        self.assertTrue(mock_prepare.called)
+        self.assertFalse(mock_deploy.called)
+
     @mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy')
     def test__do_node_deploy_driver_raises_error(self, mock_deploy):
         self._start_service()