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
This commit is contained in:
parent
5e08298249
commit
6482165bb1
@ -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', '')
|
||||
|
@ -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')
|
||||
|
@ -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.
|
Loading…
Reference in New Issue
Block a user