Merge "Always read-deleted=yes on lazy-load"
This commit is contained in:
commit
3a8dd02c81
|
@ -664,14 +664,9 @@ class ComputeManager(manager.Manager):
|
|||
return
|
||||
evacuations = {mig.instance_uuid: mig for mig in evacuations}
|
||||
|
||||
# The instances might be deleted in which case we need to avoid
|
||||
# InstanceNotFound being raised from lazy-loading fields on the
|
||||
# instances while cleaning up this host.
|
||||
read_deleted_context = context.elevated(read_deleted='yes')
|
||||
# TODO(mriedem): We could optimize by pre-loading the joined fields
|
||||
# we know we'll use, like info_cache and flavor. We can also replace
|
||||
# this with a generic solution: https://review.openstack.org/575190/
|
||||
local_instances = self._get_instances_on_driver(read_deleted_context)
|
||||
# we know we'll use, like info_cache and flavor.
|
||||
local_instances = self._get_instances_on_driver(context)
|
||||
evacuated = [inst for inst in local_instances
|
||||
if inst.uuid in evacuations]
|
||||
|
||||
|
|
|
@ -899,10 +899,9 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
self.obj_reset_changes()
|
||||
|
||||
def _load_generic(self, attrname):
|
||||
with utils.temporary_mutation(self._context, read_deleted='yes'):
|
||||
instance = self.__class__.get_by_uuid(self._context,
|
||||
uuid=self.uuid,
|
||||
expected_attrs=[attrname])
|
||||
instance = self.__class__.get_by_uuid(self._context,
|
||||
uuid=self.uuid,
|
||||
expected_attrs=[attrname])
|
||||
|
||||
if attrname not in instance:
|
||||
# NOTE(danms): Never allow us to recursively-load
|
||||
|
@ -1111,6 +1110,24 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
|
|||
'uuid': self.uuid,
|
||||
})
|
||||
|
||||
with utils.temporary_mutation(self._context, read_deleted='yes'):
|
||||
self._obj_load_attr(attrname)
|
||||
|
||||
def _obj_load_attr(self, attrname):
|
||||
"""Internal method for loading attributes from instances.
|
||||
|
||||
NOTE: Do not use this directly.
|
||||
|
||||
This method contains the implementation of lazy-loading attributes
|
||||
from Instance object, minus some massaging of the context and
|
||||
error-checking. This should always be called with the object-local
|
||||
context set for reading deleted instances and with uuid set. All
|
||||
of the code below depends on those two things. Thus, this should
|
||||
only be called from obj_load_attr() itself.
|
||||
|
||||
:param attrname: The name of the attribute to be loaded
|
||||
"""
|
||||
|
||||
# NOTE(danms): We handle some fields differently here so that we
|
||||
# can be more efficient
|
||||
if attrname == 'fault':
|
||||
|
|
|
@ -7464,7 +7464,6 @@ class ComputeTestCase(BaseTestCase,
|
|||
instance = self._create_fake_instance_obj({'host': 'someotherhost'})
|
||||
self.compute._instance_update(self.context, instance, vcpus=4)
|
||||
|
||||
@mock.patch('nova.context.RequestContext.elevated')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'remove_provider_tree_from_instance_allocation')
|
||||
@mock.patch.object(compute_manager.ComputeManager,
|
||||
|
@ -7478,10 +7477,8 @@ class ComputeTestCase(BaseTestCase,
|
|||
@mock.patch('nova.objects.Migration.save')
|
||||
def test_destroy_evacuated_instance_on_shared_storage(self, mock_save,
|
||||
mock_get_filter, mock_destroy, mock_is_inst, mock_get_blk,
|
||||
mock_get_inst, mock_remove_allocs, mock_elevated):
|
||||
mock_get_inst, mock_remove_allocs):
|
||||
fake_context = context.get_admin_context()
|
||||
read_deleted_context = fake_context.elevated(read_only='yes')
|
||||
mock_elevated.return_value = read_deleted_context
|
||||
|
||||
# instances in central db
|
||||
instances = [
|
||||
|
@ -7518,7 +7515,7 @@ class ComputeTestCase(BaseTestCase,
|
|||
'pre-migrating',
|
||||
'done'],
|
||||
'migration_type': 'evacuation'})
|
||||
mock_get_inst.assert_called_once_with(read_deleted_context)
|
||||
mock_get_inst.assert_called_once_with(fake_context)
|
||||
mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)
|
||||
mock_get_blk.assert_called_once_with(fake_context, evacuated_instance)
|
||||
mock_is_inst.assert_called_once_with(fake_context, evacuated_instance)
|
||||
|
@ -7529,7 +7526,6 @@ class ComputeTestCase(BaseTestCase,
|
|||
fake_context, evacuated_instance.uuid,
|
||||
self.rt.compute_nodes[NODENAME].uuid)
|
||||
|
||||
@mock.patch('nova.context.RequestContext.elevated')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'remove_provider_tree_from_instance_allocation')
|
||||
@mock.patch.object(compute_manager.ComputeManager,
|
||||
|
@ -7547,11 +7543,8 @@ class ComputeTestCase(BaseTestCase,
|
|||
@mock.patch('nova.objects.Migration.save')
|
||||
def test_destroy_evacuated_instance_with_disks(self, mock_save,
|
||||
mock_get_filter, mock_destroy, mock_check_clean, mock_check,
|
||||
mock_check_local, mock_get_blk, mock_get_drv, mock_remove_allocs,
|
||||
mock_elevated):
|
||||
mock_check_local, mock_get_blk, mock_get_drv, mock_remove_allocs):
|
||||
fake_context = context.get_admin_context()
|
||||
read_deleted_context = fake_context.elevated(read_deleted='yes')
|
||||
mock_elevated.return_value = read_deleted_context
|
||||
|
||||
# instances in central db
|
||||
instances = [
|
||||
|
@ -7582,7 +7575,7 @@ class ComputeTestCase(BaseTestCase,
|
|||
return_value='fake_network_info') as mock_get_nw:
|
||||
self.compute._destroy_evacuated_instances(fake_context)
|
||||
|
||||
mock_get_drv.assert_called_once_with(read_deleted_context)
|
||||
mock_get_drv.assert_called_once_with(fake_context)
|
||||
mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)
|
||||
mock_get_blk.assert_called_once_with(fake_context, evacuated_instance)
|
||||
mock_check_local.assert_called_once_with(fake_context,
|
||||
|
@ -7599,7 +7592,6 @@ class ComputeTestCase(BaseTestCase,
|
|||
fake_context, evacuated_instance.uuid,
|
||||
self.rt.compute_nodes[NODENAME].uuid)
|
||||
|
||||
@mock.patch('nova.context.RequestContext.elevated')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'remove_provider_tree_from_instance_allocation')
|
||||
@mock.patch.object(compute_manager.ComputeManager,
|
||||
|
@ -7616,11 +7608,8 @@ class ComputeTestCase(BaseTestCase,
|
|||
@mock.patch('nova.objects.Migration.save')
|
||||
def test_destroy_evacuated_instance_not_implemented(self, mock_save,
|
||||
mock_get_filter, mock_destroy, mock_check_clean, mock_check,
|
||||
mock_check_local, mock_get_blk, mock_get_inst, mock_remove_allocs,
|
||||
mock_elevated):
|
||||
mock_check_local, mock_get_blk, mock_get_inst, mock_remove_allocs):
|
||||
fake_context = context.get_admin_context()
|
||||
read_deleted_context = fake_context.elevated(read_deleted='yes')
|
||||
mock_elevated.return_value = read_deleted_context
|
||||
|
||||
# instances in central db
|
||||
instances = [
|
||||
|
@ -7650,7 +7639,7 @@ class ComputeTestCase(BaseTestCase,
|
|||
return_value='fake_network_info') as mock_get_nw:
|
||||
self.compute._destroy_evacuated_instances(fake_context)
|
||||
|
||||
mock_get_inst.assert_called_once_with(read_deleted_context)
|
||||
mock_get_inst.assert_called_once_with(fake_context)
|
||||
mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)
|
||||
mock_get_blk.assert_called_once_with(fake_context, evacuated_instance)
|
||||
mock_check_local.assert_called_once_with(fake_context,
|
||||
|
|
|
@ -848,7 +848,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
|||
self.compute.init_virt_events()
|
||||
self.assertFalse(mock_register.called)
|
||||
|
||||
@mock.patch('nova.context.RequestContext.elevated')
|
||||
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||
@mock.patch('nova.scheduler.utils.resources_from_flavor')
|
||||
@mock.patch.object(manager.ComputeManager, '_get_instances_on_driver')
|
||||
|
@ -863,12 +862,10 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
|||
def test_init_host_with_evacuated_instance(self, mock_save, mock_mig_get,
|
||||
mock_temp_mut, mock_init_host, mock_destroy, mock_host_get,
|
||||
mock_admin_ctxt, mock_init_virt, mock_get_inst, mock_resources,
|
||||
mock_get_node, mock_elevated):
|
||||
mock_get_node):
|
||||
our_host = self.compute.host
|
||||
not_our_host = 'not-' + our_host
|
||||
|
||||
read_deleted_context = self.context.elevated(read_deleted='yes')
|
||||
mock_elevated.return_value = read_deleted_context
|
||||
deleted_instance = fake_instance.fake_instance_obj(
|
||||
self.context, host=not_our_host, uuid=uuids.deleted_instance)
|
||||
migration = objects.Migration(instance_uuid=deleted_instance.uuid)
|
||||
|
@ -902,7 +899,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
|||
expected_attrs=['info_cache', 'metadata'])
|
||||
mock_init_virt.assert_called_once_with()
|
||||
mock_temp_mut.assert_called_once_with(self.context, read_deleted='yes')
|
||||
mock_get_inst.assert_called_once_with(read_deleted_context)
|
||||
mock_get_inst.assert_called_once_with(self.context)
|
||||
mock_get_net.assert_called_once_with(self.context, deleted_instance)
|
||||
|
||||
# ensure driver.destroy is called so that driver may
|
||||
|
|
|
@ -242,6 +242,24 @@ class _TestInstanceObject(object):
|
|||
self.assertNotIn('system_metadata', instance)
|
||||
self.assertEqual(0, len(instance.system_metadata))
|
||||
|
||||
def test_lazy_load_flavor_on_deleted_instance(self):
|
||||
# For something like a flavor, we should be reading from the DB
|
||||
# with read_deleted='yes'
|
||||
flavor = objects.Flavor(name='testflavor')
|
||||
instance = objects.Instance(self.context, uuid=uuids.instance,
|
||||
flavor=flavor,
|
||||
user_id=self.context.user_id,
|
||||
project_id=self.context.project_id)
|
||||
instance.create()
|
||||
instance.destroy()
|
||||
# Re-create our local object to make sure it doesn't have sysmeta
|
||||
# filled in by create()
|
||||
instance = objects.Instance(self.context, uuid=uuids.instance,
|
||||
user_id=self.context.user_id,
|
||||
project_id=self.context.project_id)
|
||||
self.assertNotIn('flavor', instance)
|
||||
self.assertEqual('testflavor', instance.flavor.name)
|
||||
|
||||
def test_lazy_load_tags(self):
|
||||
instance = objects.Instance(self.context, uuid=uuids.instance,
|
||||
user_id=self.context.user_id,
|
||||
|
|
Loading…
Reference in New Issue