openstack/nova
Code Issues Proposed changes

Merge "Remove vestigial system_metadata param from info_from_instance()"

Browse Source
This commit is contained in:
Zuul 2018-04-24 10:17:27 +00:00 committed by Gerrit Code Review
commit ee1d4e7bb5
8 changed files with 34 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
nova/compute/manager.py
View File

@ -730,24 +730,20 @@ class ComputeManager(manager.Manager):
"""Complete deletion for instances in DELETED status but not marked as
deleted in the DB
"""
system_meta = instance.system_metadata
instance.destroy()
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
context, instance.uuid)
self._complete_deletion(context,
instance,
bdms,
system_meta)
bdms)
def _complete_deletion(self, context, instance, bdms,
system_meta):
def _complete_deletion(self, context, instance, bdms):
self._update_resource_tracker(context, instance)
rt = self._get_resource_tracker()
rt.reportclient.delete_allocation_for_instance(context, instance.uuid)
self._notify_about_instance_usage(context, instance, "delete.end",
system_metadata=system_meta)
self._notify_about_instance_usage(context, instance, "delete.end")
compute_utils.notify_about_instance_action(context, instance,
self.host, action=fields.NotificationAction.DELETE,
phase=fields.NotificationPhase.END, bdms=bdms)
@ -1630,12 +1626,11 @@ class ComputeManager(manager.Manager):
self.scheduler_client.sync_instance_info(context, self.host, uuids)
def _notify_about_instance_usage(self, context, instance, event_suffix,
network_info=None, system_metadata=None,
extra_usage_info=None, fault=None):
network_info=None, extra_usage_info=None,
fault=None):
compute_utils.notify_about_instance_usage(
self.notifier, context, instance, event_suffix,
network_info=network_info,
system_metadata=system_metadata,
extra_usage_info=extra_usage_info, fault=fault)
def _deallocate_network(self, context, instance,
@ -2511,13 +2506,11 @@ class ComputeManager(manager.Manager):
instance.power_state = power_state.NOSTATE
instance.terminated_at = timeutils.utcnow()
instance.save()
system_meta = instance.system_metadata
instance.destroy()
self._complete_deletion(context,
instance,
bdms,
system_meta)
bdms)
@wrap_exception()
@reverts_task_state

27
nova/compute/utils.py
View File

@ -268,16 +268,16 @@ def notify_usage_exists(notifier, context, instance_ref, current_period=False,
usage auditing purposes.
:param notifier: a messaging.Notifier
:param context: request context for the current operation
:param instance_ref: nova.objects.Instance object from which to report
usage
:param current_period: if True, this will generate a usage for the
current usage period; if False, this will generate a usage for the
previous audit period.
:param ignore_missing_network_data: if True, log any exceptions generated
while getting network info; if False, raise the exception.
:param system_metadata: system_metadata DB entries for the instance,
if not None. *NOTE*: Currently unused here in trunk, but needed for
potential custom modifications.
:param system_metadata: system_metadata override for the instance. If
None, the instance_ref.system_metadata will be used.
:param extra_usage_info: Dictionary containing extra values to add or
override in the notification if not None.
"""
@ -301,12 +301,12 @@ def notify_usage_exists(notifier, context, instance_ref, current_period=False,
extra_info.update(extra_usage_info)
notify_about_instance_usage(notifier, context, instance_ref, 'exists',
system_metadata=system_metadata, extra_usage_info=extra_info)
extra_usage_info=extra_info)
def notify_about_instance_usage(notifier, context, instance, event_suffix,
network_info=None, system_metadata=None,
extra_usage_info=None, fault=None):
network_info=None, extra_usage_info=None,
fault=None):
"""Send an unversioned legacy notification about an instance.
All new notifications should use notify_about_instance_action which sends
@ -315,8 +315,6 @@ def notify_about_instance_usage(notifier, context, instance, event_suffix,
:param notifier: a messaging.Notifier
:param event_suffix: Event type like "delete.start" or "exists"
:param network_info: Networking information, if provided.
:param system_metadata: system_metadata DB entries for the instance,
if provided.
:param extra_usage_info: Dictionary containing extra values to add or
override in the notification.
"""
@ -324,7 +322,7 @@ def notify_about_instance_usage(notifier, context, instance, event_suffix,
extra_usage_info = {}
usage_info = notifications.info_from_instance(context, instance,
network_info, system_metadata, **extra_usage_info)
network_info, **extra_usage_info)
if fault:
# NOTE(johngarbutt) mirrors the format in wrap_exception
@ -1067,15 +1065,10 @@ class UnlimitedSemaphore(object):
@contextlib.contextmanager
def notify_about_instance_delete(notifier, context, instance,
delete_type='delete'):
# Pre-load system_metadata because if this context is around an
# instance.destroy(), lazy-loading it later would result in an
# InstanceNotFound error.
system_metadata = instance.system_metadata
try:
notify_about_instance_usage(notifier, context, instance,
"%s.start" % delete_type)
yield
finally:
notify_about_instance_usage(notifier, context, instance,
"%s.end" % delete_type,
system_metadata=system_metadata)
"%s.end" % delete_type)

13
nova/notifications/base.py
View File

@ -210,7 +210,7 @@ def send_instance_update_notification(context, instance, old_vm_state=None,
about instance state changes.
"""
payload = info_from_instance(context, instance, None, None)
payload = info_from_instance(context, instance, None)
# determine how we'll report states
payload.update(
@ -380,21 +380,12 @@ def null_safe_isotime(s):
return str(s) if s else ''
def info_from_instance(context, instance, network_info,
system_metadata, **kw):
def info_from_instance(context, instance, network_info, **kw):
"""Get detailed instance information for an instance which is common to all
notifications.
:param:instance: nova.objects.Instance
:param:network_info: network_info provided if not None
:param:system_metadata: system_metadata DB entries for the instance,
if not None
.. note::
Currently unused here in trunk, but needed for potential custom
modifications.
"""
try:
# TODO(mriedem): We can eventually drop this when we no longer support

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

@ -1035,13 +1035,11 @@ class _ComputeAPIUnitTestMixIn(object):
if 'soft' in delete_type:
compute_utils.notify_about_instance_usage(
self.compute_api.notifier,
self.context, inst, 'delete.end',
system_metadata=inst.system_metadata)
self.context, inst, 'delete.end')
else:
compute_utils.notify_about_instance_usage(
self.compute_api.notifier,
self.context, inst, '%s.end' % delete_type,
system_metadata=inst.system_metadata)
self.context, inst, '%s.end' % delete_type)
cell = objects.CellMapping(uuid=uuids.cell,
transport_url='fake://',
database_connection='fake://')
@ -1343,7 +1341,7 @@ class _ComputeAPIUnitTestMixIn(object):
constraint='constraint').AndReturn(fake_inst)
compute_utils.notify_about_instance_usage(
self.compute_api.notifier, self.context,
inst, 'delete.end', system_metadata={})
inst, 'delete.end')
self.mox.ReplayAll()
@ -1392,8 +1390,7 @@ class _ComputeAPIUnitTestMixIn(object):
inst.destroy()
compute_utils.notify_about_instance_usage(
self.compute_api.notifier, self.context,
inst, 'delete.end',
system_metadata=inst.system_metadata)
inst, 'delete.end')
self.mox.ReplayAll()
self.compute_api._local_delete(self.context, inst, bdms,
@ -1522,40 +1519,6 @@ class _ComputeAPIUnitTestMixIn(object):
do_test(self)
def test_local_delete_without_info_cache(self):
inst = self._create_instance_obj()
with test.nested(
mock.patch.object(inst, 'destroy'),
mock.patch.object(self.context, 'elevated'),
mock.patch.object(self.compute_api.network_api,
'deallocate_for_instance'),
mock.patch.object(db, 'instance_system_metadata_get'),
mock.patch.object(compute_utils,
'notify_about_instance_usage')
) as (
inst_destroy, context_elevated, net_api_deallocate_for_instance,
db_instance_system_metadata_get, notify_about_instance_usage
):
compute_utils.notify_about_instance_usage(
self.compute_api.notifier, self.context,
inst, 'delete.start')
self.context.elevated().MultipleTimes().AndReturn(self.context)
if self.cell_type != 'api':
self.compute_api.network_api.deallocate_for_instance(
self.context, inst)
inst.destroy()
compute_utils.notify_about_instance_usage(
self.compute_api.notifier, self.context,
inst, 'delete.end',
system_metadata=inst.system_metadata)
inst.info_cache = None
self.compute_api._local_delete(self.context, inst, [],
'delete',
self._fake_do_delete)
def test_delete_disabled(self):
inst = self._create_instance_obj()
inst.disable_terminate = True

2
nova/tests/unit/compute/test_compute_utils.py
View File

@ -1080,7 +1080,7 @@ class ComputeUtilsTestCase(test.NoDBTestCase):
mock.call(mock.sentinel.notifier, self.context, instance,
'delete.start'),
mock.call(mock.sentinel.notifier, self.context, instance,
'delete.end', system_metadata=instance.system_metadata)
'delete.end')
]
mock_notify_usage.assert_has_calls(expected_notify_calls)

6
nova/tests/unit/notifications/test_base.py
View File

@ -97,8 +97,7 @@ class TestSendInstanceUpdateNotification(test.NoDBTestCase):
instance = fake_instance.fake_instance_obj(ctxt, image_ref=uuids.image)
instance.system_metadata = {}
instance.metadata = {}
payload = base.info_from_instance(
ctxt, instance, network_info=None, system_metadata=None)
payload = base.info_from_instance(ctxt, instance, network_info=None)
self.assertEqual(instance.image_ref, payload['image_ref_url'])
mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt)
@ -114,8 +113,7 @@ class TestSendInstanceUpdateNotification(test.NoDBTestCase):
'fake-user', 'fake-project', auth_token='fake-token')
instance = fake_instance.fake_instance_obj(ctxt, image_ref=uuids.image)
self.assertRaises(ks_exc.EndpointNotFound, base.info_from_instance,
ctxt, instance, network_info=None,
system_metadata=None)
ctxt, instance, network_info=None)
mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt)

16
nova/tests/unit/test_notifications.py
View File

@ -356,20 +356,20 @@ class NotificationsTestCase(test.TestCase):
def test_payload_has_fixed_ip_labels(self):
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertIn("fixed_ips", info)
self.assertEqual(info["fixed_ips"][0]["label"], "test1")
def test_payload_has_vif_mac_address(self):
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertIn("fixed_ips", info)
self.assertEqual(self.net_info[0]['address'],
info["fixed_ips"][0]["vif_mac"])
def test_payload_has_cell_name_empty(self):
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertIn("cell_name", info)
self.assertIsNone(self.instance.cell_name)
self.assertEqual("", info["cell_name"])
@ -377,13 +377,13 @@ class NotificationsTestCase(test.TestCase):
def test_payload_has_cell_name(self):
self.instance.cell_name = "cell1"
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertIn("cell_name", info)
self.assertEqual("cell1", info["cell_name"])
def test_payload_has_progress_empty(self):
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertIn("progress", info)
self.assertIsNone(self.instance.progress)
self.assertEqual("", info["progress"])
@ -391,7 +391,7 @@ class NotificationsTestCase(test.TestCase):
def test_payload_has_progress(self):
self.instance.progress = 50
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertIn("progress", info)
self.assertEqual(50, info["progress"])
@ -406,7 +406,7 @@ class NotificationsTestCase(test.TestCase):
self.instance.flavor.memory_mb = 30
self.instance.flavor.ephemeral_gb = 40
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertEqual(10, info['vcpus'])
self.assertEqual(20, info['root_gb'])
self.assertEqual(30, info['memory_mb'])
@ -421,7 +421,7 @@ class NotificationsTestCase(test.TestCase):
self.instance.launched_at = time
info = notifications.info_from_instance(self.context, self.instance,
self.net_info, None)
self.net_info)
self.assertEqual('2017-02-02T16:45:00.000000', info['terminated_at'])
self.assertEqual('2017-02-02T16:45:00.000000', info['launched_at'])

3
nova/tests/unit/utils.py
View File

@ -314,8 +314,7 @@ def assert_instance_delete_notification_by_uuid(
mock.call(expected_notifier,
expected_context,
match_by_instance_uuid,
'delete.end',
system_metadata={})])
'delete.end')])
for call in mock_notify.call_args_list:
if expect_targeted_context: