Fix rebuild of baremetal instance when vm_state is ERROR

Nova allows rebuild of instance when vm_state is ERROR. [1]

The vm_state is restored to ACTIVE only after a successful build.
This means rebuilding a baremetal instance using the Ironic driver
is impossible because wait_for_active fails if vm_state=ERROR is found.

This is a regression introduced in a previous change which added
the ability to delete an instance in spawning state. [2]

This present change will skip the abort installation logic
if task_state is REBUILD_SPAWNING while preserving the previous logic.

[1] https://bugs.launchpad.net/nova/+bug/1183946
[2] https://bugs.launchpad.net/nova/+bug/1455000

Change-Id: I857ad7264f1a7ef1263d8a9d4eca491d6c8dce0f
Closes-bug: #1735009
(cherry picked from commit 1819718)

Author: mgagne

nova/tests/unit/virt/ironic/test_driver.py

@@ -201,6 +201,23 @@ def test__wait_for_active_done(self, fake_validate, fake_refresh):
         fake_validate.assert_called_once_with(instance)
         fake_refresh.assert_called_once_with()
 
+    @mock.patch.object(objects.Instance, 'refresh')
+    @mock.patch.object(ironic_driver.IronicDriver,
+                       '_validate_instance_and_node')
+    def test__wait_for_active_from_error(self, fake_validate, fake_refresh):
+        instance = fake_instance.fake_instance_obj(self.ctx,
+                uuid=uuidutils.generate_uuid(),
+                vm_state=vm_states.ERROR,
+                task_state=task_states.REBUILD_SPAWNING)
+        node = ironic_utils.get_test_node(
+                provision_state=ironic_states.ACTIVE)
+
+        fake_validate.return_value = node
+        self.assertRaises(loopingcall.LoopingCallDone,
+                self.driver._wait_for_active, instance)
+        fake_validate.assert_called_once_with(instance)
+        fake_refresh.assert_called_once_with()
+
     @mock.patch.object(objects.Instance, 'refresh')
     @mock.patch.object(ironic_driver.IronicDriver,
                        '_validate_instance_and_node')
@@ -236,7 +253,8 @@ def test__wait_for_active_abort_deleted(self):
         self._wait_for_active_abort({'vm_state': vm_states.DELETED})
 
     def test__wait_for_active_abort_error(self):
-        self._wait_for_active_abort({'vm_state': vm_states.ERROR})
+        self._wait_for_active_abort({'task_state': task_states.SPAWNING,
+                                     'vm_state': vm_states.ERROR})
 
     @mock.patch.object(ironic_driver.IronicDriver,
                        '_validate_instance_and_node')

nova/virt/ironic/driver.py

@@ -485,8 +485,10 @@ def _cleanup_deploy(self, node, instance, network_info=None,
     def _wait_for_active(self, instance):
         """Wait for the node to be marked as ACTIVE in Ironic."""
         instance.refresh()
-        if (instance.task_state == task_states.DELETING or
-            instance.vm_state in (vm_states.ERROR, vm_states.DELETED)):
+        # Ignore REBUILD_SPAWNING when rebuilding from ERROR state.
+        if (instance.task_state != task_states.REBUILD_SPAWNING and
+                (instance.task_state == task_states.DELETING or
+                 instance.vm_state in (vm_states.ERROR, vm_states.DELETED))):
             raise exception.InstanceDeployFailure(
                 _("Instance %s provisioning was aborted") % instance.uuid)