Merge "Revert "Detach volume after deleting instance with no host""
This commit is contained in:
Jenkins
Gerrit Code Review
commit
232ffaea58
|
|
@ -1588,12 +1588,9 @@ class API(base.Base):
|
|||
cb(context, instance, bdms, reservations=None)
|
||||
quotas.commit()
|
||||
return
|
||||
# NOTE(melwitt): We expect instances in certain states to have no
|
||||
# host set but also have resources on a compute host. If this is
|
||||
# the case, we want to try to clean up their resources. Otherwse,
|
||||
# we can destroy the instance here and return.
|
||||
expect_no_instance_host = self._expect_no_host(instance)
|
||||
if not instance.host and not expect_no_instance_host:
|
||||
shelved_offloaded = (instance.vm_state
|
||||
== vm_states.SHELVED_OFFLOADED)
|
||||
if not instance.host and not shelved_offloaded:
|
||||
try:
|
||||
compute_utils.notify_about_instance_usage(
|
||||
self.notifier, context, instance,
|
||||
|
|
@ -1605,17 +1602,7 @@ class API(base.Base):
|
|||
system_metadata=instance.system_metadata)
|
||||
quotas.commit()
|
||||
return
|
||||
except exception.ObjectActionError as ex:
|
||||
# The instance's host likely changed under us as
|
||||
# this instance could be building and has since been
|
||||
# scheduled. Continue with attempts to delete it.
|
||||
# NOTE(melwitt): Often instance.host is None after the
|
||||
# refresh because the claim was aborted on the compute
|
||||
# host. The service up check will raise ComputeHostNotFound
|
||||
# in this case and we will do a local delete with compute
|
||||
# host cleanup
|
||||
LOG.debug('Refreshing instance because: %s', ex,
|
||||
instance=instance)
|
||||
except exception.ObjectActionError:
|
||||
instance.refresh()
|
||||
|
||||
if instance.vm_state == vm_states.RESIZED:
|
||||
|
|
@ -1623,7 +1610,7 @@ class API(base.Base):
|
|||
|
||||
is_local_delete = True
|
||||
try:
|
||||
if not expect_no_instance_host:
|
||||
if not shelved_offloaded:
|
||||
service = objects.Service.get_by_compute_host(
|
||||
context.elevated(), instance.host)
|
||||
is_local_delete = not self.servicegroup_api.service_is_up(
|
||||
|
|
@ -1650,9 +1637,6 @@ class API(base.Base):
|
|||
cb(context, instance, bdms,
|
||||
reservations=quotas.reservations)
|
||||
except exception.ComputeHostNotFound:
|
||||
# NOTE(melwitt): We expect this if instance.host has been
|
||||
# set to None by the compute host during a claim abort
|
||||
# and we pick it up in the instance.refresh()
|
||||
pass
|
||||
|
||||
if is_local_delete:
|
||||
|
|
@ -1671,13 +1655,6 @@ class API(base.Base):
|
|||
if quotas:
|
||||
quotas.rollback()
|
||||
|
||||
def _expect_no_host(self, instance):
|
||||
# NOTE(melwitt): Instances in ERROR state have their host and node
|
||||
# set to None as part of exception handling.
|
||||
if instance.vm_state in (vm_states.SHELVED_OFFLOADED, vm_states.ERROR):
|
||||
return True
|
||||
return False
|
||||
|
||||
def _confirm_resize_on_deleting(self, context, instance):
|
||||
# If in the middle of a resize, use confirm_resize to
|
||||
# ensure the original instance is cleaned up too
|
||||
|
|
@ -1771,22 +1748,6 @@ class API(base.Base):
|
|||
connector)
|
||||
self.volume_api.detach(elevated, bdm.volume_id,
|
||||
instance.uuid)
|
||||
except Exception as exc:
|
||||
err_str = _LW("Ignoring volume cleanup failure due to %s")
|
||||
LOG.warn(err_str % exc, instance=instance)
|
||||
# This block handles the following case:
|
||||
# 1. Instance scheduled to host and fails on the host.
|
||||
# 2. Compute manager's cleanup calls terminate_connection
|
||||
# and detach if the spawn made it that far.
|
||||
# 3. Instance fails to boot on all other reschedule attempts.
|
||||
# 4. Instance is left in error state with no assigned host.
|
||||
# 5. Volumes in the instance's BDMs are left in the available
|
||||
# state.
|
||||
# When this is the case the terminate_connection and detach
|
||||
# calls above will fail. We attempt the volume delete in a
|
||||
# separate try-except to clean up these volume and avoid the
|
||||
# storage leak.
|
||||
try:
|
||||
if bdm.delete_on_termination:
|
||||
self.volume_api.delete(context, bdm.volume_id)
|
||||
except Exception as exc:
|
||||
|
|
|
|||
|
|
@ -1040,7 +1040,7 @@ class _ComputeAPIUnitTestMixIn(object):
|
|||
self._test_delete_resized_part(inst)
|
||||
if inst.vm_state == vm_states.SOFT_DELETED:
|
||||
soft_delete = True
|
||||
if not self.compute_api._expect_no_host(inst):
|
||||
if inst.vm_state != vm_states.SHELVED_OFFLOADED:
|
||||
self.context.elevated().AndReturn(self.context)
|
||||
objects.Service.get_by_compute_host(self.context,
|
||||
inst.host).AndReturn(objects.Service())
|
||||
|
|
@ -1049,7 +1049,8 @@ class _ComputeAPIUnitTestMixIn(object):
|
|||
inst.host != 'down-host')
|
||||
|
||||
if (inst.host == 'down-host' or
|
||||
self.compute_api._expect_no_host(inst)):
|
||||
inst.vm_state == vm_states.SHELVED_OFFLOADED):
|
||||
|
||||
self._test_downed_host_part(inst, updates, delete_time,
|
||||
delete_type)
|
||||
cast = False
|
||||
|
|
@ -3853,54 +3854,6 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
|
|||
self.assertRaises(exception.CannotResizeToSameFlavor,
|
||||
self._test_resize, same_flavor=True)
|
||||
|
||||
@mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid')
|
||||
@mock.patch('nova.context.RequestContext.elevated')
|
||||
@mock.patch.object(objects.Instance, 'save')
|
||||
@mock.patch.object(quota.QUOTAS, 'reserve')
|
||||
@mock.patch.object(compute_utils, 'notify_about_instance_usage')
|
||||
@mock.patch.object(objects.BlockDeviceMapping, 'destroy')
|
||||
@mock.patch.object(objects.Instance, 'destroy')
|
||||
def _test_delete_volume_backed_instance(
|
||||
self, vm_state, mock_instance_destroy, bdm_destroy,
|
||||
notify_about_instance_usage, mock_reserve,
|
||||
mock_save, mock_elevated, bdm_get_by_instance_uuid):
|
||||
volume_id = uuidutils.generate_uuid()
|
||||
bdms = [objects.BlockDeviceMapping(
|
||||
**fake_block_device.FakeDbBlockDeviceDict(
|
||||
{'id': 42, 'volume_id': volume_id,
|
||||
'source_type': 'volume', 'destination_type': 'volume',
|
||||
'delete_on_termination': False}))]
|
||||
reservations = ['fake-resv']
|
||||
|
||||
bdm_get_by_instance_uuid.return_value = bdms
|
||||
mock_reserve.return_value = reservations
|
||||
mock_elevated.return_value = self.context
|
||||
|
||||
params = {'host': None, 'vm_state': vm_state}
|
||||
inst = self._create_instance_obj(params=params)
|
||||
connector = {'ip': '127.0.0.1', 'initiator': 'iqn.fake'}
|
||||
|
||||
with mock.patch.object(self.compute_api.network_api,
|
||||
'deallocate_for_instance') as mock_deallocate, \
|
||||
mock.patch.object(self.compute_api.volume_api,
|
||||
'terminate_connection') as mock_terminate_conn, \
|
||||
mock.patch.object(self.compute_api.volume_api,
|
||||
'detach') as mock_detach:
|
||||
self.compute_api.delete(self.context, inst)
|
||||
|
||||
mock_deallocate.assert_called_once_with(self.context, inst)
|
||||
mock_detach.assert_called_once_with(self.context, volume_id,
|
||||
inst.uuid)
|
||||
mock_terminate_conn.assert_called_once_with(self.context,
|
||||
volume_id, connector)
|
||||
bdm_destroy.assert_called_once_with()
|
||||
|
||||
def test_delete_volume_backed_instance_in_error(self):
|
||||
self._test_delete_volume_backed_instance(vm_states.ERROR)
|
||||
|
||||
def test_delete_volume_backed_instance_in_shelved_offloaded(self):
|
||||
self._test_delete_volume_backed_instance(vm_states.SHELVED_OFFLOADED)
|
||||
|
||||
|
||||
class ComputeAPIAPICellUnitTestCase(_ComputeAPIUnitTestMixIn,
|
||||
test.NoDBTestCase):
|
||||
|
|
|
|||
Loading…
Reference in New Issue