openstack
/
nova
Code Issues Proposed changes

Update queued-for-delete from the ComputeAPI during deletion/restoration 

This patch updates queued-for-delete column in the instance_mapping
table from the ComputeAPI during normal delete, local delete (when
the compute service is down or instance is shelved offloaded) and
soft-delete operations. Note that as the name of the column suggests,
it is only "queued" for deletion and does not guarantee successful
deletion of the instance. Hence the value could be stale which is fine,
considering its use is only during down cell (desperate) situation.
This in no way hampers normal delete operations in nova.

This column is also updated from the ComputeAPI during the
nova restore operation.

Related to blueprint handling-down-cell

Change-Id: I3de5c839d212cfab1c69e2c78617c1051b971c38
This commit is contained in:
Surya Seetharaman 2018-05-08 10:25:25 +02:00 committed by Matt Riedemann
parent b2d469e456
commit 2499f61609
4 changed files with 51 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
nova/compute/api.py
View File

@ -2143,6 +2143,20 @@ class API(base.Base):
cb(context, instance, bdms, local=True)
instance.destroy()
@staticmethod
def _update_queued_for_deletion(context, instance, qfd):
# NOTE(tssurya): We query the instance_mapping record of this instance
# and update the queued_for_delete flag to True (or False according to
# the state of the instance). This just means that the instance is
# queued for deletion (or is no longer queued for deletion). It does
# not guarantee its successful deletion (or restoration). Hence the
# value could be stale which is fine, considering its use is only
# during down cell (desperate) situation.
im = objects.InstanceMapping.get_by_instance_uuid(context,
instance.uuid)
im.queued_for_delete = qfd
im.save()
def _do_delete(self, context, instance, bdms, local=False):
if local:
instance.vm_state = vm_states.DELETED
@ -2152,6 +2166,7 @@ class API(base.Base):
else:
self.compute_rpcapi.terminate_instance(context, instance, bdms,
delete_type='delete')
self._update_queued_for_deletion(context, instance, True)
def _do_force_delete(self, context, instance, bdms, local=False):
if local:
@ -2162,6 +2177,7 @@ class API(base.Base):
else:
self.compute_rpcapi.terminate_instance(context, instance, bdms,
delete_type='force_delete')
self._update_queued_for_deletion(context, instance, True)
def _do_soft_delete(self, context, instance, bdms, local=False):
if local:
@ -2171,6 +2187,7 @@ class API(base.Base):
instance.save()
else:
self.compute_rpcapi.soft_delete_instance(context, instance)
self._update_queued_for_deletion(context, instance, True)
# NOTE(maoy): we allow delete to be called no matter what vm_state says.
@check_instance_lock
@ -2225,6 +2242,7 @@ class API(base.Base):
instance.task_state = None
instance.deleted_at = None
instance.save(expected_task_state=[None])
self._update_queued_for_deletion(context, instance, False)
@check_instance_lock
@check_instance_state(task_state=None,

3
nova/tests/unit/compute/test_compute.py
View File

@ -9813,7 +9813,8 @@ class ComputeAPITestCase(BaseTestCase):
i_ref = self._create_fake_instance_obj()
self.assertEqual(i_ref['name'], i_ref['uuid'])
def test_add_remove_fixed_ip(self):
@mock.patch('nova.compute.api.API._update_queued_for_deletion')
def test_add_remove_fixed_ip(self, mock_update_queued_for_delete):
instance = self._create_fake_instance_obj(params={'host': CONF.host})
self.compute_api.add_fixed_ip(self.context, instance, '1')
self.compute_api.remove_fixed_ip(self.context,

30
nova/tests/unit/compute/test_compute_api.py
View File

@ -1018,6 +1018,7 @@ class _ComputeAPIUnitTestMixIn(object):
@mock.patch.object(objects.Migration, 'get_by_instance_and_status')
@mock.patch.object(image_api.API, 'delete')
@mock.patch.object(objects.InstanceMapping, 'save')
@mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid')
@mock.patch.object(consoleauth_rpcapi.ConsoleAuthAPI,
'delete_tokens_for_instance')
@ -1037,7 +1038,7 @@ class _ComputeAPIUnitTestMixIn(object):
def _test_delete(self, delete_type, mock_save, mock_bdm_get, mock_elevated,
mock_get_cn, mock_up, mock_record, mock_inst_update,
mock_deallocate, mock_inst_meta, mock_inst_destroy,
mock_notify, mock_del_token, mock_get_inst,
mock_notify, mock_del_token, mock_get_inst, mock_save_im,
mock_image_delete, mock_mig_get, **attrs):
expected_save_calls = [mock.call()]
expected_record_calls = []
@ -1177,8 +1178,10 @@ class _ComputeAPIUnitTestMixIn(object):
mock_deallocate.assert_called_once_with(self.context, inst)
mock_inst_destroy.assert_called_once_with(
self.context, instance_uuid, constraint=None)
mock_get_inst.assert_called_once_with(self.context,
instance_uuid)
mock_get_inst.assert_called_with(self.context, instance_uuid)
self.assertEqual(2, mock_get_inst.call_count)
self.assertTrue(mock_get_inst.return_value.queued_for_delete)
mock_save_im.assert_called_once_with()
if cast:
if delete_type == 'soft_delete':
@ -4036,7 +4039,8 @@ class _ComputeAPIUnitTestMixIn(object):
@mock.patch('nova.objects.Quotas.limit_check_project_and_user')
@mock.patch('nova.objects.Instance.save')
@mock.patch('nova.objects.InstanceAction.action_start')
def test_restore_by_admin(self, action_start, instance_save,
@mock.patch('nova.compute.api.API._update_queued_for_deletion')
def test_restore_by_admin(self, update_qfd, action_start, instance_save,
quota_check, quota_count):
admin_context = context.RequestContext('admin_user',
'admin_project',
@ -4067,12 +4071,15 @@ class _ComputeAPIUnitTestMixIn(object):
'cores': 1 + instance.flavor.vcpus,
'ram': 512 + instance.flavor.memory_mb},
project_id=instance.project_id, user_id=instance.user_id)
update_qfd.assert_called_once_with(admin_context, instance, False)
@mock.patch('nova.objects.Quotas.count_as_dict')
@mock.patch('nova.objects.Quotas.limit_check_project_and_user')
@mock.patch('nova.objects.Instance.save')
@mock.patch('nova.objects.InstanceAction.action_start')
def test_restore_by_instance_owner(self, action_start, instance_save,
@mock.patch('nova.compute.api.API._update_queued_for_deletion')
def test_restore_by_instance_owner(self, update_qfd, action_start,
instance_save,
quota_check, quota_count):
proj_count = {'instances': 1, 'cores': 1, 'ram': 512}
user_count = proj_count.copy()
@ -4101,6 +4108,7 @@ class _ComputeAPIUnitTestMixIn(object):
'cores': 1 + instance.flavor.vcpus,
'ram': 512 + instance.flavor.memory_mb},
project_id=instance.project_id, user_id=instance.user_id)
update_qfd.assert_called_once_with(self.context, instance, False)
@mock.patch.object(objects.InstanceAction, 'action_start')
def test_external_instance_event(self, mock_action_start):
@ -5472,6 +5480,18 @@ class _ComputeAPIUnitTestMixIn(object):
self.context, requested_networks, 5)
self.assertEqual(4, count)
@mock.patch.object(objects.InstanceMapping, 'save')
@mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid')
def test_update_queued_for_deletion(self, mock_get, mock_save):
uuid = uuids.inst
inst = objects.Instance(uuid=uuid)
im = objects.InstanceMapping(instance_uuid=uuid)
mock_get.return_value = im
self.compute_api._update_queued_for_deletion(self.context, inst, True)
self.assertTrue(im.queued_for_delete)
mock_get.assert_called_once_with(self.context, inst.uuid)
mock_save.assert_called_once_with()
@mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid',
side_effect=exception.InstanceMappingNotFound(uuid='fake'))
@mock.patch.object(objects.BuildRequest, 'get_by_instance_uuid')

8
nova/tests/unit/compute/test_compute_cells.py
View File

@ -288,10 +288,12 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
'instance_delete_everywhere')
@mock.patch.object(compute_api.API, '_lookup_instance',
return_value=(None, inst))
def _test(_mock_lookup_inst, _mock_delete_everywhere):
@mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid')
def _test(mock_get_im, _mock_lookup_inst, _mock_delete_everywhere):
self.assertRaises(exception.ObjectActionError,
self.compute_api.delete, self.context, inst)
inst.destroy.assert_called_once_with()
mock_get_im.assert_called_once_with(self.context, inst.uuid)
_test()
@ -313,12 +315,14 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
'instance_delete_everywhere', side_effect=add_cell_name)
@mock.patch.object(compute_api.API, '_lookup_instance',
return_value=(None, inst))
def _test(_mock_lookup_inst, mock_delete_everywhere,
@mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid')
def _test(mock_get_im, _mock_lookup_inst, mock_delete_everywhere,
mock_compute_delete):
self.compute_api.delete(self.context, inst)
inst.destroy.assert_called_once_with()
mock_compute_delete.assert_called_once_with(self.context, inst)
mock_get_im.assert_called_once_with(self.context, inst.uuid)
_test()