Call generate_image_url only for legacy notification

The legacy instance.exists notification includes the full url of the glance
image of the given instance. But the versioned notification only includes
the image uuid. Generating the full url can be a costly operation as it
needs to talk to Keystone.

So this patch makes sure that generate_image_url only called when the
generated information will be used.

 Conflicts:
	nova/compute/utils.py
	nova/notifications/base.py
	nova/tests/unit/notifications/test_base.py

NOTE(elod.illes): conflict caused by parameter system_metadata which is not
there on master anymore.

Change-Id: I78c2a34b3d03438457cc968cd0a38b8131e4f6e6
Closes-Bug: #1757407
(cherry picked from commit 93b897348b)

nova/compute/utils.py

@@ -322,7 +322,8 @@ 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, system_metadata, populate_image_ref_url=True,
+            **extra_usage_info)
 
     if fault:
         # NOTE(johngarbutt) mirrors the format in wrap_exception

nova/notifications/base.py

@@ -209,8 +209,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, 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, None,
+                                 populate_image_ref_url=populate_image_ref_url)
 
     # determine how we'll report states
     payload.update(
@@ -380,7 +386,7 @@ def null_safe_isotime(s):
 
 
 def info_from_instance(context, instance, network_info,
-                system_metadata, **kw):
+                system_metadata, populate_image_ref_url=False, **kw):
     """Get detailed instance information for an instance which is common to all
     notifications.
 
@@ -394,21 +400,32 @@ def info_from_instance(context, instance, network_info,
         Currently unused here in trunk, but needed for potential custom
         modifications.
 
+    :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, None,
+            populate_image_ref_url=True)
 
     @mock.patch('nova.image.api.API.generate_image_url',
                 side_effect=ks_exc.EndpointNotFound)
@@ -97,8 +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, system_metadata=None)
+        payload = base.info_from_instance(ctxt, instance, network_info=None,
+            system_metadata=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)
 
@@ -115,9 +118,21 @@ class TestSendInstanceUpdateNotification(test.NoDBTestCase):
         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)
+                          system_metadata=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,
+            system_metadata=None, populate_image_ref_url=False)
+
+        mock_gen_image_url.assert_not_called()
+
 
 class TestBandwidthUsage(test.NoDBTestCase):
     @mock.patch('nova.context.RequestContext.elevated')