diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 8c8dd6c807c6..82029d034d14 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -350,7 +350,7 @@ def notify_about_instance_usage(notifier, context, instance, event_suffix, extra_usage_info = {} usage_info = notifications.info_from_instance(context, instance, - network_info, **extra_usage_info) + network_info, populate_image_ref_url=True, **extra_usage_info) if fault: # NOTE(johngarbutt) mirrors the format in wrap_exception diff --git a/nova/notifications/base.py b/nova/notifications/base.py index 0bccc5cd9eb2..1a6f0efac9cc 100644 --- a/nova/notifications/base.py +++ b/nova/notifications/base.py @@ -173,8 +173,14 @@ def send_instance_update_notification(context, instance, old_vm_state=None, """Send 'compute.instance.update' notification to inform observers about instance state changes. """ - - payload = info_from_instance(context, instance, None) + # NOTE(gibi): The image_ref_url is only used in unversioned notifications. + # Calling the generate_image_url() could be costly as it calls + # the Keystone API. So only do the call if the actual value will be + # used. + populate_image_ref_url = (CONF.notifications.notification_format in + ('both', 'unversioned')) + payload = info_from_instance(context, instance, None, + populate_image_ref_url=populate_image_ref_url) # determine how we'll report states payload.update( @@ -344,27 +350,39 @@ def null_safe_isotime(s): return str(s) if s else '' -def info_from_instance(context, instance, network_info, **kw): +def info_from_instance(context, instance, network_info, + populate_image_ref_url=False, **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:populate_image_ref_url: If True then the full URL of the image of + the instance is generated and returned. + This, depending on the configuration, might + mean a call to Keystone. If false, None + value is returned in the dict at the + image_ref_url key. """ - try: - # TODO(mriedem): We can eventually drop this when we no longer support - # legacy notifications since versioned notifications don't use this. - image_ref_url = image_api.API().generate_image_url(instance.image_ref, - context) - except ks_exc.EndpointNotFound: - # We might be running from a periodic task with no auth token and - # CONF.glance.api_servers isn't set, so we can't get the image API - # endpoint URL from the service catalog, therefore just use the image - # id for the URL (yes it's a lie, but it's best effort at this point). - with excutils.save_and_reraise_exception() as exc_ctx: - if context.auth_token is None: - image_ref_url = instance.image_ref - exc_ctx.reraise = False + image_ref_url = None + if populate_image_ref_url: + try: + # NOTE(mriedem): We can eventually drop this when we no longer + # support legacy notifications since versioned notifications don't + # use this. + image_ref_url = image_api.API().generate_image_url( + instance.image_ref, context) + + except ks_exc.EndpointNotFound: + # We might be running from a periodic task with no auth token and + # CONF.glance.api_servers isn't set, so we can't get the image API + # endpoint URL from the service catalog, therefore just use the + # image id for the URL (yes it's a lie, but it's best effort at + # this point). + with excutils.save_and_reraise_exception() as exc_ctx: + if context.auth_token is None: + image_ref_url = instance.image_ref + exc_ctx.reraise = False instance_type = instance.get_flavor() instance_type_name = instance_type.get('name', '') diff --git a/nova/tests/unit/notifications/test_base.py b/nova/tests/unit/notifications/test_base.py index 0e21acb2947e..06f77db15974 100644 --- a/nova/tests/unit/notifications/test_base.py +++ b/nova/tests/unit/notifications/test_base.py @@ -82,6 +82,9 @@ class TestSendInstanceUpdateNotification(test.NoDBTestCase): mock_get_notifier.return_value.info.assert_called_once_with( mock.sentinel.ctxt, 'compute.instance.update', mock.ANY) + mock_info.assert_called_once_with( + mock.sentinel.ctxt, mock.sentinel.instance, None, + populate_image_ref_url=True) @mock.patch('nova.image.api.API.generate_image_url', side_effect=ks_exc.EndpointNotFound) @@ -97,7 +100,8 @@ 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) + payload = base.info_from_instance(ctxt, instance, network_info=None, + populate_image_ref_url=True) self.assertEqual(instance.image_ref, payload['image_ref_url']) mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt) @@ -113,9 +117,22 @@ 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) + ctxt, instance, network_info=None, + populate_image_ref_url=True) mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt) + @mock.patch('nova.image.api.API.generate_image_url') + def test_info_from_instance_not_call_generate_image_url( + self, mock_gen_image_url): + ctxt = nova_context.get_admin_context() + instance = fake_instance.fake_instance_obj(ctxt, image_ref=uuids.image) + instance.system_metadata = {} + instance.metadata = {} + base.info_from_instance(ctxt, instance, network_info=None, + populate_image_ref_url=False) + + mock_gen_image_url.assert_not_called() + class TestBandwidthUsage(test.NoDBTestCase): @mock.patch('nova.context.RequestContext.elevated')