Merge "Call generate_image_url only for legacy notification"

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

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', '')

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')