Be more forgiving to empty context in notification
We seem to call rpc.get_notifier('api').error(None, 'api.fault', payload) in notifications.py, where the first parameter is context. This eventually calls context.to_dict() in nova/rpc.py, which breaks for a NoneType object. So we should pick the current context from oslo_context or switch to the admin context. Co-Authored-By: Pranesh Pandurangan <praneshpg@gmail.com> Closes-Bug: 1360911 Change-Id: Ic37e184a56377739cd7cda5ba4bde236990fef9e
This commit is contained in:
parent
6afe84df5a
commit
7135af61c1
|
@ -101,7 +101,10 @@ def send_api_fault(url, status, exception):
|
|||
payload = {'url': url, 'exception': six.text_type(exception),
|
||||
'status': status}
|
||||
|
||||
rpc.get_notifier('api').error(None, 'api.fault', payload)
|
||||
rpc.get_notifier('api').error(common_context.get_current() or
|
||||
nova.context.get_admin_context(),
|
||||
'api.fault',
|
||||
payload)
|
||||
|
||||
|
||||
def send_update(context, old_instance, new_instance, service="compute",
|
||||
|
|
|
@ -29,7 +29,7 @@ def reset():
|
|||
|
||||
FakeMessage = collections.namedtuple('Message',
|
||||
['publisher_id', 'priority',
|
||||
'event_type', 'payload'])
|
||||
'event_type', 'payload', 'context'])
|
||||
|
||||
|
||||
class FakeNotifier(object):
|
||||
|
@ -55,7 +55,8 @@ class FakeNotifier(object):
|
|||
# this permit to raise an exception if something have not
|
||||
# been serialized correctly
|
||||
jsonutils.to_primitive(payload)
|
||||
msg = FakeMessage(self.publisher_id, priority, event_type, payload)
|
||||
msg = FakeMessage(self.publisher_id, priority, event_type,
|
||||
payload, ctxt)
|
||||
NOTIFICATIONS.append(msg)
|
||||
|
||||
|
||||
|
|
|
@ -19,6 +19,8 @@ import copy
|
|||
|
||||
import mock
|
||||
from oslo_config import cfg
|
||||
from oslo_context import context as o_context
|
||||
from oslo_context import fixture as o_fixture
|
||||
|
||||
from nova.compute import flavors
|
||||
from nova.compute import task_states
|
||||
|
@ -40,6 +42,7 @@ class NotificationsTestCase(test.TestCase):
|
|||
|
||||
def setUp(self):
|
||||
super(NotificationsTestCase, self).setUp()
|
||||
self.fixture = self.useFixture(o_fixture.ClearRequestContext())
|
||||
|
||||
self.net_info = fake_network.fake_get_instance_nw_info(self.stubs, 1,
|
||||
1)
|
||||
|
@ -112,6 +115,73 @@ class NotificationsTestCase(test.TestCase):
|
|||
self.assertEqual(n.payload['status'], 500)
|
||||
self.assertIsNotNone(n.payload['exception'])
|
||||
|
||||
def test_send_api_fault_fresh_context(self):
|
||||
self.flags(notify_api_faults=True)
|
||||
exception = None
|
||||
try:
|
||||
# Get a real exception with a call stack.
|
||||
raise test.TestingException("junk")
|
||||
except test.TestingException as e:
|
||||
exception = e
|
||||
|
||||
ctxt = context.RequestContext(overwrite=True)
|
||||
notifications.send_api_fault("http://example.com/foo", 500, exception)
|
||||
|
||||
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
|
||||
n = fake_notifier.NOTIFICATIONS[0]
|
||||
self.assertEqual(n.priority, 'ERROR')
|
||||
self.assertEqual(n.event_type, 'api.fault')
|
||||
self.assertEqual(n.payload['url'], 'http://example.com/foo')
|
||||
self.assertEqual(n.payload['status'], 500)
|
||||
self.assertIsNotNone(n.payload['exception'])
|
||||
self.assertEqual(ctxt, n.context)
|
||||
|
||||
def test_send_api_fault_fake_context(self):
|
||||
self.flags(notify_api_faults=True)
|
||||
exception = None
|
||||
try:
|
||||
# Get a real exception with a call stack.
|
||||
raise test.TestingException("junk")
|
||||
except test.TestingException as e:
|
||||
exception = e
|
||||
|
||||
ctxt = o_context.get_current()
|
||||
self.assertIsNotNone(ctxt)
|
||||
notifications.send_api_fault("http://example.com/foo", 500, exception)
|
||||
|
||||
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
|
||||
n = fake_notifier.NOTIFICATIONS[0]
|
||||
self.assertEqual(n.priority, 'ERROR')
|
||||
self.assertEqual(n.event_type, 'api.fault')
|
||||
self.assertEqual(n.payload['url'], 'http://example.com/foo')
|
||||
self.assertEqual(n.payload['status'], 500)
|
||||
self.assertIsNotNone(n.payload['exception'])
|
||||
self.assertIsNotNone(n.context)
|
||||
self.assertEqual(ctxt, n.context)
|
||||
|
||||
def test_send_api_fault_admin_context(self):
|
||||
self.flags(notify_api_faults=True)
|
||||
exception = None
|
||||
try:
|
||||
# Get a real exception with a call stack.
|
||||
raise test.TestingException("junk")
|
||||
except test.TestingException as e:
|
||||
exception = e
|
||||
|
||||
self.fixture._remove_cached_context()
|
||||
self.assertIsNone(o_context.get_current())
|
||||
notifications.send_api_fault("http://example.com/foo", 500, exception)
|
||||
|
||||
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
|
||||
n = fake_notifier.NOTIFICATIONS[0]
|
||||
self.assertEqual(n.priority, 'ERROR')
|
||||
self.assertEqual(n.event_type, 'api.fault')
|
||||
self.assertEqual(n.payload['url'], 'http://example.com/foo')
|
||||
self.assertEqual(n.payload['status'], 500)
|
||||
self.assertIsNotNone(n.payload['exception'])
|
||||
self.assertIsNotNone(n.context)
|
||||
self.assertTrue(n.context.is_admin)
|
||||
|
||||
def test_notif_disabled(self):
|
||||
|
||||
# test config disable of the notifications
|
||||
|
|
Loading…
Reference in New Issue