Merge "Add instance hard delete"

This commit is contained in:
Zuul 2019-04-30 17:39:25 +00:00 committed by Gerrit Code Review
commit 61a3846b61
7 changed files with 226 additions and 52 deletions

View File

@ -748,9 +748,19 @@ def instance_create(context, values):
return IMPL.instance_create(context, values)
def instance_destroy(context, instance_uuid, constraint=None):
"""Destroy the instance or raise if it does not exist."""
return IMPL.instance_destroy(context, instance_uuid, constraint)
def instance_destroy(context, instance_uuid, constraint=None,
hard_delete=False):
"""Destroy the instance or raise if it does not exist.
:param context: request context object
:param instance_uuid: uuid of the instance to delete
:param constraint: a constraint object
:param hard_delete: when set to True, removes all records related to the
instance
"""
return IMPL.instance_destroy(context, instance_uuid,
constraint=constraint,
hard_delete=hard_delete)
def instance_get_by_uuid(context, uuid, columns_to_join=None):

View File

@ -1751,7 +1751,8 @@ def instance_create(context, values):
@require_context
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
@pick_context_manager_writer
def instance_destroy(context, instance_uuid, constraint=None):
def instance_destroy(context, instance_uuid, constraint=None,
hard_delete=False):
if uuidutils.is_uuid_like(instance_uuid):
instance_ref = _instance_get_by_uuid(context, instance_uuid)
else:
@ -1761,37 +1762,31 @@ def instance_destroy(context, instance_uuid, constraint=None):
filter_by(uuid=instance_uuid)
if constraint is not None:
query = constraint.apply(models.Instance, query)
# Either in hard or soft delete, we soft delete the instance first
# to make sure that that the constraints were met.
count = query.soft_delete()
if count == 0:
raise exception.ConstraintNotMet()
model_query(context, models.SecurityGroupInstanceAssociation).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.InstanceInfoCache).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.InstanceMetadata).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.InstanceFault).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.InstanceExtra).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.InstanceSystemMetadata).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.BlockDeviceMapping).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.Migration).\
filter_by(instance_uuid=instance_uuid).\
soft_delete()
model_query(context, models.VirtualInterface).filter_by(
instance_uuid=instance_uuid).soft_delete()
model_query(context, models.InstanceIdMapping).filter_by(
uuid=instance_uuid).soft_delete()
models_to_delete = [
models.SecurityGroupInstanceAssociation, models.InstanceInfoCache,
models.InstanceMetadata, models.InstanceFault, models.InstanceExtra,
models.InstanceSystemMetadata, models.BlockDeviceMapping,
models.Migration, models.VirtualInterface
]
# For most referenced models we filter by the instance_uuid column, but for
# these models we filter by the uuid column.
filtered_by_uuid = [models.InstanceIdMapping]
for model in models_to_delete + filtered_by_uuid:
key = 'instance_uuid' if model not in filtered_by_uuid else 'uuid'
filter_ = {key: instance_uuid}
if hard_delete:
model_query(context, model).filter_by(**filter_).delete()
else:
model_query(context, model).filter_by(**filter_).soft_delete()
# NOTE(snikitin): We can't use model_query here, because there is no
# column 'deleted' in 'tags' or 'console_auth_tokens' tables.
context.session.query(models.Tag).filter_by(
@ -1803,6 +1798,21 @@ def instance_destroy(context, instance_uuid, constraint=None):
# can be used by operators to find out what actions were performed on a
# deleted instance. Both of these tables are special-cased in
# _archive_deleted_rows_for_table().
if hard_delete:
# NOTE(ttsiousts): In case of hard delete, we need to remove the
# instance actions too since instance_uuid is a foreign key and
# for this we need to delete the corresponding InstanceActionEvents
actions = context.session.query(models.InstanceAction).filter_by(
instance_uuid=instance_uuid).all()
for action in actions:
context.session.query(models.InstanceActionEvent).filter_by(
action_id=action.id).delete()
context.session.query(models.InstanceAction).filter_by(
instance_uuid=instance_uuid).delete()
# NOTE(ttsiouts): The instance is the last thing to be deleted in
# order to respect all constraints
context.session.query(models.Instance).filter_by(
uuid=instance_uuid).delete()
return instance_ref

View File

@ -114,7 +114,8 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
# Version 2.2: Added keypairs
# Version 2.3: Added device_metadata
# Version 2.4: Added trusted_certs
VERSION = '2.4'
# Version 2.5: Added hard_delete kwarg in destroy
VERSION = '2.5'
fields = {
'id': fields.IntegerField(),
@ -608,7 +609,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
self.obj_reset_changes(['ec2_ids'])
@base.remotable
def destroy(self):
def destroy(self, hard_delete=False):
if not self.obj_attr_is_set('id'):
raise exception.ObjectActionError(action='destroy',
reason='already destroyed')
@ -627,7 +628,8 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
try:
db_inst = db.instance_destroy(self._context, self.uuid,
constraint=constraint)
constraint=constraint,
hard_delete=hard_delete)
self._from_db_object(self._context, self, db_inst)
except exception.ConstraintNotMet:
raise exception.ObjectActionError(action='destroy',

View File

@ -1189,7 +1189,8 @@ class _ComputeAPIUnitTestMixIn(object):
inst, '%s.end' % delete_type)])
mock_deallocate.assert_called_once_with(self.context, inst)
mock_inst_destroy.assert_called_once_with(
self.context, instance_uuid, constraint=None)
self.context, instance_uuid, constraint=None,
hard_delete=False)
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)
@ -1440,7 +1441,8 @@ class _ComputeAPIUnitTestMixIn(object):
mock_cons.assert_called_once_with(host=mock.ANY)
mock_inst_destroy.assert_called_once_with(
self.context, instance_uuid, constraint='constraint')
self.context, instance_uuid, constraint='constraint',
hard_delete=False)
def _fake_do_delete(context, instance, bdms,
rservations=None, local=False):

View File

@ -137,6 +137,24 @@ def _quota_create(context, project_id, user_id):
user_id=user_id).hard_limit
@sqlalchemy_api.pick_context_manager_reader
def _assert_instance_id_mapping(_ctxt, tc, inst_uuid, expected_existing=False):
# NOTE(mriedem): We can't use ec2_instance_get_by_uuid to assert
# the instance_id_mappings record is gone because it hard-codes
# read_deleted='yes' and will read the soft-deleted record. So we
# do the model_query directly here. See bug 1061166.
inst_id_mapping = sqlalchemy_api.model_query(
_ctxt, models.InstanceIdMapping).filter_by(uuid=inst_uuid).first()
if not expected_existing:
tc.assertFalse(inst_id_mapping,
'instance_id_mapping not deleted for '
'instance: %s' % inst_uuid)
else:
tc.assertTrue(inst_id_mapping,
'instance_id_mapping not found for '
'instance: %s' % inst_uuid)
class DbTestCase(test.TestCase):
def setUp(self):
super(DbTestCase, self).setUp()
@ -2948,19 +2966,7 @@ class InstanceTestCase(test.TestCase, ModelsObjectComparatorMixin):
self.assertEqual([], db.instance_tag_get_by_instance_uuid(
ctxt, inst_uuid))
@sqlalchemy_api.pick_context_manager_reader
def _assert_instance_id_mapping(_ctxt):
# NOTE(mriedem): We can't use ec2_instance_get_by_uuid to assert
# the instance_id_mappings record is gone because it hard-codes
# read_deleted='yes' and will read the soft-deleted record. So we
# do the model_query directly here. See bug 1061166.
inst_id_mapping = sqlalchemy_api.model_query(
_ctxt, models.InstanceIdMapping).filter_by(
uuid=inst_uuid).first()
self.assertFalse(inst_id_mapping,
'instance_id_mapping not deleted for '
'instance: %s' % inst_uuid)
_assert_instance_id_mapping(ctxt)
_assert_instance_id_mapping(ctxt, self, inst_uuid)
ctxt.read_deleted = 'yes'
self.assertEqual(values['system_metadata'],
db.instance_system_metadata_get(ctxt, inst_uuid))
@ -2972,6 +2978,125 @@ class InstanceTestCase(test.TestCase, ModelsObjectComparatorMixin):
self.assertRaises(exception.InstanceNotFound,
db.instance_destroy, ctxt, instance['uuid'])
def test_instance_destroy_hard(self):
ctxt = context.get_admin_context()
instance = self.create_instance_with_args()
uuid = instance['uuid']
utc_now = timeutils.utcnow()
action_values = {
'action': 'run_instance',
'instance_uuid': uuid,
'request_id': ctxt.request_id,
'user_id': ctxt.user_id,
'project_id': ctxt.project_id,
'start_time': utc_now,
'updated_at': utc_now,
'message': 'action-message'
}
action = db.action_start(ctxt, action_values)
action_event_values = {
'event': 'schedule',
'action_id': action['id'],
'instance_uuid': uuid,
'start_time': utc_now,
'request_id': ctxt.request_id,
'host': 'fake-host',
}
db.action_event_start(ctxt, action_event_values)
security_group_values = {
'name': 'fake_sec_group',
'user_id': ctxt.user_id,
'project_id': ctxt.project_id,
'instances': []
}
security_group = db.security_group_create(ctxt, security_group_values)
db.instance_add_security_group(ctxt, uuid, security_group['id'])
instance_fault_values = {
'message': 'message',
'details': 'detail',
'instance_uuid': uuid,
'code': 404,
'host': 'localhost'
}
db.instance_fault_create(ctxt, instance_fault_values)
bdm_values = {
'instance_uuid': uuid,
'device_name': 'fake_device',
'source_type': 'volume',
'destination_type': 'volume',
}
block_dev = block_device.BlockDeviceDict(bdm_values)
db.block_device_mapping_create(self.ctxt, block_dev, legacy=False)
migration_values = {
"status": "finished",
"instance_uuid": uuid,
"dest_compute": "fake_host2"
}
db.migration_create(self.ctxt, migration_values)
db.virtual_interface_create(ctxt, {'instance_uuid': uuid})
pool_values = {
'address': '192.168.10.10',
'username': 'user1',
'password': 'passwd1',
'console_type': 'type1',
'public_hostname': 'public_host1',
'host': 'host1',
'compute_host': 'compute_host1',
}
console_pool = db.console_pool_create(ctxt, pool_values)
console_values = {
'instance_name': instance['name'],
'instance_uuid': uuid,
'password': 'pass',
'port': 7878,
'pool_id': console_pool['id']
}
db.console_create(self.ctxt, console_values)
# Hard delete the instance
db.instance_destroy(ctxt, uuid, hard_delete=True)
# Check that related records are deleted
with utils.temporary_mutation(ctxt, read_deleted="yes"):
# Assert that all information related to the instance is not found
# even using a context that can read soft deleted records.
self.assertEqual(0, len(db.actions_get(ctxt, uuid)))
self.assertEqual(0, len(db.action_events_get(ctxt, action['id'])))
db_sg = db.security_group_get_by_name(
ctxt, ctxt.project_id, security_group_values['name'])
self.assertEqual(0, len(db_sg['instances']))
instance_faults = db.instance_fault_get_by_instance_uuids(
ctxt, [uuid])
self.assertEqual(0, len(instance_faults[uuid]))
inst_bdms = db.block_device_mapping_get_all_by_instance(ctxt, uuid)
self.assertEqual(0, len(inst_bdms))
filters = {"isntance_uuid": uuid}
inst_migrations = db.migration_get_all_by_filters(ctxt, filters)
self.assertEqual(0, len(inst_migrations))
vifs = db.virtual_interface_get_by_instance(ctxt, uuid)
self.assertEqual(0, len(vifs))
self.assertIsNone(db.instance_info_cache_get(ctxt, uuid))
self.assertEqual({}, db.instance_metadata_get(ctxt, uuid))
self.assertIsNone(db.instance_extra_get_by_instance_uuid(
ctxt, uuid))
system_meta = db.instance_system_metadata_get(ctxt, uuid)
self.assertEqual({}, system_meta)
_assert_instance_id_mapping(ctxt, self, uuid)
self.assertRaises(exception.InstanceNotFound,
db.instance_destroy, ctxt, uuid)
# NOTE(ttsiouts): Should these also be valid?
# instance_consoles = db.console_get_all_by_instance(ctxt, uuid)
# self.assertEqual(0, len(instance_consoles))
# Also FixedIp has the instance_uuid as a foreign key
def test_check_instance_exists(self):
instance = self.create_instance_with_args()

View File

@ -13,6 +13,7 @@
# under the License.
import datetime
import six
import mock
import netaddr
@ -1234,7 +1235,8 @@ class _TestInstanceObject(object):
timeutils.normalize_time(inst.deleted_at))
self.assertTrue(inst.deleted)
mock_destroy.assert_called_once_with(self.context, uuids.instance,
constraint=None)
constraint=None,
hard_delete=False)
def test_destroy(self):
values = {'user_id': self.context.user_id,
@ -1281,6 +1283,29 @@ class _TestInstanceObject(object):
inst.destroy()
self.assertFalse(mock_destroy_at_top.called)
def test_destroy_hard(self):
values = {'user_id': self.context.user_id,
'project_id': self.context.project_id}
db_inst = db.instance_create(self.context, values)
inst = objects.Instance(context=self.context, id=db_inst['id'],
uuid=db_inst['uuid'])
inst.destroy(hard_delete=True)
elevated = self.context.elevated(read_deleted="yes")
self.assertRaises(exception.InstanceNotFound,
objects.Instance.get_by_uuid, elevated,
db_inst['uuid'])
def test_destroy_hard_host_constraint(self):
values = {'user_id': self.context.user_id,
'project_id': self.context.project_id,
'host': 'foo'}
db_inst = db.instance_create(self.context, values)
inst = objects.Instance.get_by_uuid(self.context, db_inst['uuid'])
inst.host = None
ex = self.assertRaises(exception.ObjectActionError,
inst.destroy, hard_delete=True)
self.assertIn('host changed', six.text_type(ex))
def test_name_does_not_trigger_lazy_loads(self):
values = {'user_id': self.context.user_id,
'project_id': self.context.project_id,

View File

@ -1072,7 +1072,7 @@ object_data = {
'IDEDeviceBus': '1.0-29d4c9f27ac44197f01b6ac1b7e16502',
'ImageMeta': '1.8-642d1b2eb3e880a367f37d72dd76162d',
'ImageMetaProps': '1.21-f3721d8f744a9507a1966c81c386b532',
'Instance': '2.4-4437eb8b2737c3054ea579b8efe31dc5',
'Instance': '2.5-94e3881f0b9a06c2c3640e44816b51de',
'InstanceAction': '1.1-f9f293e526b66fca0d05c3b3a2d13914',
'InstanceActionEvent': '1.2-b2f368b8a29d8d872b1f6ea841e820a0',
'InstanceActionEventList': '1.1-13d92fb953030cdbfee56481756e02be',