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 commit14e43f385e) (cherry picked from commit00cba39613)
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user