Avoid unbound instance_uuid var during delete

The patch I03cf285ad83e09d88cdb702a88dfed53c01610f8 fixed most of the
possible cases for this to happen but missed one. An early enough
exception during _delete() can cause that the instance_uuid never gets
defined but then we try to use it during the finally block. This patch
moves the saving of the instance_uuid to the top of the try block to
avoid the issue.

Change-Id: Ib3073d7f595c8927532b7c49fc7e5ffe80d508b9
Closes-Bug: #1940812
Related-Bug: #1914777
(cherry picked from commit 14e43f3)

Author: Balazs Gibizer

nova/compute/api.py

@@ -2281,6 +2281,12 @@ def _delete(self, context, instance, delete_type, cb, **instance_attrs):
         # Normal delete should be attempted.
         may_have_ports_or_volumes = compute_utils.may_have_ports_or_volumes(
             instance)
+
+        # Save a copy of the instance UUID early, in case
+        # _lookup_instance returns instance = None, to pass to
+        # _local_delete_cleanup if needed.
+        instance_uuid = instance.uuid
+
         if not instance.host and not may_have_ports_or_volumes:
             try:
                 if self._delete_while_booting(context, instance):
@@ -2294,10 +2300,6 @@ def _delete(self, context, instance, delete_type, cb, **instance_attrs):
                 # full Instance or None if not found. If not found then it's
                 # acceptable to skip the rest of the delete processing.
 
-                # Save a copy of the instance UUID early, in case
-                # _lookup_instance returns instance = None, to pass to
-                # _local_delete_cleanup if needed.
-                instance_uuid = instance.uuid
                 cell, instance = self._lookup_instance(context, instance.uuid)
                 if cell and instance:
                     try:

nova/tests/unit/compute/test_api.py

@@ -1629,6 +1629,34 @@ def test_delete_instance_from_cell0(self, destroy_mock, notify_mock):
                 self.compute_api.notifier, self.context, instance)
             destroy_mock.assert_called_once_with()
 
+    def test_delete_instance_while_booting_host_changes_lookup_fails(self):
+        """Tests the case where the instance become scheduled while being
+        destroyed but then the final lookup fails.
+        """
+        instance = self._create_instance_obj({'host': None})
+
+        with test.nested(
+            mock.patch.object(
+                self.compute_api, '_delete_while_booting',
+                side_effect=exception.ObjectActionError(
+                    action="delete", reason="reason")),
+            mock.patch.object(
+                self.compute_api, '_lookup_instance',
+                return_value=(None, None)),
+            mock.patch.object(self.compute_api, '_local_delete_cleanup')
+        ) as (
+            _delete_while_booting, _lookup_instance, _local_delete_cleanup
+        ):
+            self.compute_api._delete(
+                self.context, instance, 'delete', mock.NonCallableMock())
+
+            _delete_while_booting.assert_called_once_with(
+                self.context, instance)
+            _lookup_instance.assert_called_once_with(
+                self.context, instance.uuid)
+            _local_delete_cleanup.assert_called_once_with(
+                self.context, instance.uuid)
+
     @mock.patch.object(context, 'target_cell')
     @mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid',
                        side_effect=exception.InstanceMappingNotFound(