From 14e43f385e6d243b6efd11a777d082e63b66367c Mon Sep 17 00:00:00 2001
From: Balazs Gibizer <balazs.gibizer@est.tech>
Date: Mon, 23 Aug 2021 10:56:58 +0200
Subject: [PATCH] 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
---
 nova/compute/api.py                 | 10 ++++++----
 nova/tests/unit/compute/test_api.py | 28 ++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/nova/compute/api.py b/nova/compute/api.py
index d41d8b6109..20710dfdff 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2281,6 +2281,12 @@ class API:
         # 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 @@ class API:
                 # 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:
diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py
index 64be53f923..d2ba9855fd 100644
--- a/nova/tests/unit/compute/test_api.py
+++ b/nova/tests/unit/compute/test_api.py
@@ -1629,6 +1629,34 @@ class _ComputeAPIUnitTestMixIn(object):
                 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(