Handle EndpointNotFound when building image_ref_url in notifications

Change I4e755b9c66ec8bc3af0393e81cffd91c56064717 made the
[glance]/api_servers option optional. If not set, we attempt
to get the image service endpoint via keystoneauth adapter and
the service catalog via the request context.

Periodic tasks run without an actual token so there is no way
to get the service catalog and the KSA adapter code to get the
endpoint raises EndpointNotFound when trying to build the
"image_ref_url" entry in the legacy instance notification payload.

This change simply handles the EndpointNotFound and sets the
image_ref_url to the instance.image_ref, which for non-volume-backed
instances is the image id (for volume-backed instances it's an empty
string).

This doesn't affect versioned notifications since those don't use the
"image_ref_url" entry in the payload that is created, they just have
an "image_uuid" entry in the versioned notification payload which is
populated via instance.image_ref.

An upgrade impact release note is added in the case that some consuming
service is actually relying on that legacy notification field being
a URL and parsing it as such. The thinking here, however, is this is
better than not sending the field at all, or setting it to None.

Long-term this code all gets replaced with versioned notifications.

Change-Id: Ie23a9c922776b028da0720c939846cba233ac472
Closes-Bug: #1753550
(cherry picked from commit 6482165bb1)

nova/notifications/base.py

@@ -20,6 +20,7 @@ the system.
 
 import datetime
 
+from keystoneauth1 import exceptions as ks_exc
 from oslo_context import context as common_context
 from oslo_log import log
 from oslo_utils import excutils
@@ -394,8 +395,20 @@ def info_from_instance(context, instance, network_info,
         modifications.
 
     """
-    image_ref_url = image_api.API().generate_image_url(instance.image_ref,
-                                                       context)
+    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
 
     instance_type = instance.get_flavor()
     instance_type_name = instance_type.get('name', '')

nova/tests/unit/notifications/test_base.py

@@ -14,11 +14,13 @@
 #    under the License.
 import datetime
 
+from keystoneauth1 import exceptions as ks_exc
 import mock
 
 from nova import context as nova_context
 from nova.notifications import base
 from nova import test
+from nova.tests.unit import fake_instance
 from nova.tests import uuidsentinel as uuids
 from nova import utils
 
@@ -81,6 +83,41 @@ class TestSendInstanceUpdateNotification(test.NoDBTestCase):
         mock_get_notifier.return_value.info.assert_called_once_with(
             mock.sentinel.ctxt, 'compute.instance.update', mock.ANY)
 
+    @mock.patch('nova.image.api.API.generate_image_url',
+                side_effect=ks_exc.EndpointNotFound)
+    def test_info_from_instance_image_api_endpoint_not_found_no_token(
+            self, mock_gen_image_url):
+        """Tests the case that we fail to generate the image ref url because
+        CONF.glance.api_servers isn't set and we have a context without an
+        auth token, like in the case of a periodic task using an admin context.
+        In this case, we expect the payload field 'image_ref_url' to just be
+        the instance.image_ref (image ID for a non-volume-backed server).
+        """
+        ctxt = nova_context.get_admin_context()
+        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)
+        self.assertEqual(instance.image_ref, payload['image_ref_url'])
+        mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt)
+
+    @mock.patch('nova.image.api.API.generate_image_url',
+                side_effect=ks_exc.EndpointNotFound)
+    def test_info_from_instance_image_api_endpoint_not_found_with_token(
+            self, mock_gen_image_url):
+        """Tests the case that we fail to generate the image ref url because
+        an EndpointNotFound error is raised up from the image API but the
+        context does have a token so we pass the error through.
+        """
+        ctxt = nova_context.RequestContext(
+            '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)
+        mock_gen_image_url.assert_called_once_with(instance.image_ref, ctxt)
+
 
 class TestBandwidthUsage(test.NoDBTestCase):
     @mock.patch('nova.context.RequestContext.elevated')

releasenotes/notes/bug-1753550-image-ref-url-notifications-42df5911a46b7de7.yaml

@@ -0,0 +1,9 @@
+---
+upgrade:
+  - |
+    The ``image_ref_url`` entry in legacy instance notification payloads will
+    be just the instance image id if ``[glance]/api_servers`` is not set
+    and the notification is being sent from a periodic task. In this case the
+    periodic task does not have a token to get the image service endpoint URL
+    from the identity service so only the image id is in the payload. This
+    does not affect versioned notifications.