diff --git a/nova/compute/api.py b/nova/compute/api.py index 3cde93928446..b4f37a237293 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -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, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index a45b3f897369..e529f911daa6 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -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, diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 9bef37e4067f..db5060afdf13 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -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') diff --git a/nova/tests/unit/compute/test_compute_cells.py b/nova/tests/unit/compute/test_compute_cells.py index b8e03ade71a2..0b8111409694 100644 --- a/nova/tests/unit/compute/test_compute_cells.py +++ b/nova/tests/unit/compute/test_compute_cells.py @@ -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()