From fc20dc2ea23d511a0f741901d7f03135bb269165 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer 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 (cherry picked from commit 14e43f385e6d243b6efd11a777d082e63b66367c) (cherry picked from commit 00cba396134846dbbcf68cc9cf08e80618322bbe) --- 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 7e7012954f0a..efbe8789c42c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2231,6 +2231,12 @@ class API(base.Base): # 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): @@ -2244,10 +2250,6 @@ class API(base.Base): # 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 822bfc1a15be..809ed3b29541 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -1620,6 +1620,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(