From 2dcd8256662115e6528c2b784c08c5724c8227e8 Mon Sep 17 00:00:00 2001 From: Andrew Bogott Date: Thu, 26 Apr 2012 13:54:08 -0500 Subject: [PATCH] Pass context to notification drivers when we can. Note that previously several notification drivers created an admin context and embedded it in their messages. Those drivers now use the passed-in context instead if it's available. For blueprint novaplugins. Change-Id: Icb89030256022d0a68db4a147611fd37fe83316a --- nova/compute/utils.py | 2 +- nova/exception.py | 28 +++++++- nova/log.py | 6 +- nova/notifier/api.py | 19 ++++-- nova/notifier/capacity_notifier.py | 2 +- nova/notifier/list_notifier.py | 6 +- nova/notifier/log_notifier.py | 2 +- nova/notifier/no_op_notifier.py | 2 +- nova/notifier/rabbit_notifier.py | 5 +- nova/notifier/test_notifier.py | 2 +- nova/scheduler/filter_scheduler.py | 6 +- nova/scheduler/manager.py | 2 +- nova/tests/notifier/test_capacity_notifier.py | 8 +-- nova/tests/notifier/test_list_notifier.py | 6 +- nova/tests/test_exception.py | 8 ++- nova/tests/test_notifier.py | 64 ++++++++++++++++--- 16 files changed, 124 insertions(+), 44 deletions(-) diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 9cf890487945..2415abf7ce58 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -239,6 +239,6 @@ def notify_about_instance_usage(context, instance, event_suffix, usage_info = _usage_from_instance( context, instance, network_info=network_info, **extra_usage_info) - notifier_api.notify('compute.%s' % host, + notifier_api.notify(context, 'compute.%s' % host, 'compute.instance.%s' % event_suffix, notifier_api.INFO, usage_info) diff --git a/nova/exception.py b/nova/exception.py index e40fa0ccfdc1..dc1199be3d49 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -25,6 +25,7 @@ SHOULD include dedicated exception logging. """ import functools +import inspect import sys import webob.exc @@ -133,8 +134,12 @@ def wrap_exception(notifier=None, publisher_id=None, event_type=None, # propagated. temp_type = f.__name__ - notifier.notify(publisher_id, temp_type, temp_level, - payload) + context = get_context_from_function_and_args(f, + args, + kw) + + notifier.notify(context, publisher_id, temp_type, + temp_level, payload) # re-raise original exception since it may have been clobbered raise exc_info[0], exc_info[1], exc_info[2] @@ -1060,3 +1065,22 @@ class InvalidInstanceIDMalformed(Invalid): class CouldNotFetchImage(NovaException): message = _("Could not fetch image %(image)s") + + +def get_context_from_function_and_args(function, args, kwargs): + """Find an arg named 'context' and return the value. + + This is useful in a couple of decorators where we don't + know much about the function we're wrapping. + """ + context = None + if 'context' in kwargs.keys(): + context = kwargs['context'] + else: + (iargs, _iva, _ikw, _idf) = inspect.getargspec(function) + if 'context' in iargs: + index = iargs.index('context') + if len(args) > index: + context = args[index] + + return context diff --git a/nova/log.py b/nova/log.py index acd3ba56e064..268f12f334ce 100644 --- a/nova/log.py +++ b/nova/log.py @@ -283,8 +283,10 @@ class PublishErrorsHandler(logging.Handler): if 'list_notifier_drivers' in FLAGS: if 'nova.notifier.log_notifier' in FLAGS.list_notifier_drivers: return - nova.notifier.api.notify('nova.error.publisher', 'error_notification', - nova.notifier.api.ERROR, dict(error=record.msg)) + nova.notifier.api.notify(None, 'nova.error.publisher', + 'error_notification', + nova.notifier.api.ERROR, + dict(error=record.msg)) def handle_exception(type, value, tb): diff --git a/nova/notifier/api.py b/nova/notifier/api.py index 4eff31bc4331..b3bfbf9c462a 100644 --- a/nova/notifier/api.py +++ b/nova/notifier/api.py @@ -13,13 +13,15 @@ # License for the specific language governing permissions and limitations # under the License. +import inspect import uuid +from nova import exception from nova import flags -from nova import utils from nova import log as logging from nova.openstack.common import cfg from nova.openstack.common import importutils +from nova import utils LOG = logging.getLogger(__name__) @@ -65,10 +67,13 @@ def notify_decorator(name, fn): body['args'].append(arg) for key in kwarg: body['kwarg'][key] = kwarg[key] - notify(FLAGS.default_publisher_id, - name, - FLAGS.default_notification_level, - body) + + context = exception.get_context_from_function_and_args(fn, args, kwarg) + notify(context, + FLAGS.default_publisher_id, + name, + FLAGS.default_notification_level, + body) return fn(*args, **kwarg) return wrapped_func @@ -79,7 +84,7 @@ def publisher_id(service, host=None): return "%s.%s" % (service, host) -def notify(publisher_id, event_type, priority, payload): +def notify(context, publisher_id, event_type, priority, payload): """Sends a notification using the specified driver :param publisher_id: the source worker_type.host of the message @@ -126,7 +131,7 @@ def notify(publisher_id, event_type, priority, payload): payload=payload, timestamp=str(utils.utcnow())) try: - driver.notify(msg) + driver.notify(context, msg) except Exception, e: LOG.exception(_("Problem '%(e)s' attempting to " "send to notification system. Payload=%(payload)s") % diff --git a/nova/notifier/capacity_notifier.py b/nova/notifier/capacity_notifier.py index 848e591d5e6b..b8e3a88e288c 100644 --- a/nova/notifier/capacity_notifier.py +++ b/nova/notifier/capacity_notifier.py @@ -21,7 +21,7 @@ from nova import log as logging LOG = logging.getLogger(__name__) -def notify(message): +def notify(_context, message): """Look for specific compute manager events and interprete them so as to keep the Capacity table up to date. diff --git a/nova/notifier/list_notifier.py b/nova/notifier/list_notifier.py index 9f6d29775f33..4e73bdd50802 100644 --- a/nova/notifier/list_notifier.py +++ b/nova/notifier/list_notifier.py @@ -37,7 +37,7 @@ class ImportFailureNotifier(object): def __init__(self, exception): self.exception = exception - def notify(self, message): + def notify(self, context, message): raise self.exception @@ -54,11 +54,11 @@ def _get_drivers(): return drivers -def notify(message): +def notify(context, message): """Passes notification to multiple notifiers in a list.""" for driver in _get_drivers(): try: - driver.notify(message) + driver.notify(context, message) except Exception as e: LOG.exception(_("Problem '%(e)s' attempting to send to " "notification driver %(driver)s."), locals()) diff --git a/nova/notifier/log_notifier.py b/nova/notifier/log_notifier.py index 25dfc693bac9..f13b9813fc7e 100644 --- a/nova/notifier/log_notifier.py +++ b/nova/notifier/log_notifier.py @@ -22,7 +22,7 @@ from nova import log as logging FLAGS = flags.FLAGS -def notify(message): +def notify(_context, message): """Notifies the recipient of the desired event given the model. Log notifications using nova's default logging system""" diff --git a/nova/notifier/no_op_notifier.py b/nova/notifier/no_op_notifier.py index 02971050565d..ee1ddbdcac3a 100644 --- a/nova/notifier/no_op_notifier.py +++ b/nova/notifier/no_op_notifier.py @@ -14,6 +14,6 @@ # under the License. -def notify(message): +def notify(_context, message): """Notifies the recipient of the desired event given the model""" pass diff --git a/nova/notifier/rabbit_notifier.py b/nova/notifier/rabbit_notifier.py index 7543700f73a8..cf39ba9be7aa 100644 --- a/nova/notifier/rabbit_notifier.py +++ b/nova/notifier/rabbit_notifier.py @@ -31,9 +31,10 @@ FLAGS = flags.FLAGS FLAGS.register_opt(notification_topic_opt) -def notify(message): +def notify(context, message): """Sends a notification to the RabbitMQ""" - context = nova.context.get_admin_context() + if not context: + context = nova.context.get_admin_context() priority = message.get('priority', FLAGS.default_notification_level) priority = priority.lower() diff --git a/nova/notifier/test_notifier.py b/nova/notifier/test_notifier.py index 1fa7a66be61b..3dbaa2dbb0a5 100644 --- a/nova/notifier/test_notifier.py +++ b/nova/notifier/test_notifier.py @@ -20,6 +20,6 @@ FLAGS = flags.FLAGS NOTIFICATIONS = [] -def notify(message): +def notify(_context, message): """Test notifier, stores notifications in memory for unittests.""" NOTIFICATIONS.append(message) diff --git a/nova/scheduler/filter_scheduler.py b/nova/scheduler/filter_scheduler.py index 1e7ee16b9071..766f2c369f0d 100644 --- a/nova/scheduler/filter_scheduler.py +++ b/nova/scheduler/filter_scheduler.py @@ -65,7 +65,7 @@ class FilterScheduler(driver.Scheduler): locals()) payload = dict(request_spec=request_spec) - notifier.notify(notifier.publisher_id("scheduler"), + notifier.notify(context, notifier.publisher_id("scheduler"), 'scheduler.run_instance.start', notifier.INFO, payload) weighted_hosts = self._schedule(context, "compute", request_spec, @@ -91,7 +91,7 @@ class FilterScheduler(driver.Scheduler): if instance: instances.append(instance) - notifier.notify(notifier.publisher_id("scheduler"), + notifier.notify(context, notifier.publisher_id("scheduler"), 'scheduler.run_instance.end', notifier.INFO, payload) return instances @@ -125,7 +125,7 @@ class FilterScheduler(driver.Scheduler): payload = dict(request_spec=request_spec, weighted_host=weighted_host.to_dict(), instance_id=instance['uuid']) - notifier.notify(notifier.publisher_id("scheduler"), + notifier.notify(context, notifier.publisher_id("scheduler"), 'scheduler.run_instance.scheduled', notifier.INFO, payload) diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py index 959c915839f9..2823abab3503 100644 --- a/nova/scheduler/manager.py +++ b/nova/scheduler/manager.py @@ -168,7 +168,7 @@ class SchedulerManager(manager.Manager): method=method, reason=ex) - notifier.notify(notifier.publisher_id("scheduler"), + notifier.notify(context, notifier.publisher_id("scheduler"), 'scheduler.' + method, notifier.ERROR, payload) # NOTE (masumotok) : This method should be moved to nova.api.ec2.admin. diff --git a/nova/tests/notifier/test_capacity_notifier.py b/nova/tests/notifier/test_capacity_notifier.py index a34c0ed3796d..e41c52700f18 100644 --- a/nova/tests/notifier/test_capacity_notifier.py +++ b/nova/tests/notifier/test_capacity_notifier.py @@ -34,16 +34,16 @@ class CapacityNotifierTestCase(test.TestCase): def test_event_type(self): msg = self._make_msg("myhost", "mymethod") msg['event_type'] = 'random' - self.assertFalse(cn.notify(msg)) + self.assertFalse(cn.notify(None, msg)) def test_bad_event_suffix(self): msg = self._make_msg("myhost", "mymethod.badsuffix") - self.assertFalse(cn.notify(msg)) + self.assertFalse(cn.notify(None, msg)) def test_bad_publisher_id(self): msg = self._make_msg("myhost", "mymethod.start") msg['publisher_id'] = 'badpublisher' - self.assertFalse(cn.notify(msg)) + self.assertFalse(cn.notify(None, msg)) def test_update_called(self): def _verify_called(host, context, free_ram_mb_delta, @@ -56,4 +56,4 @@ class CapacityNotifierTestCase(test.TestCase): self.stubs.Set(nova.db.api, "compute_node_utilization_update", _verify_called) msg = self._make_msg("myhost", "delete.end") - self.assertTrue(cn.notify(msg)) + self.assertTrue(cn.notify(None, msg)) diff --git a/nova/tests/notifier/test_list_notifier.py b/nova/tests/notifier/test_list_notifier.py index e5ba48b6f28c..b647f501b8df 100644 --- a/nova/tests/notifier/test_list_notifier.py +++ b/nova/tests/notifier/test_list_notifier.py @@ -58,7 +58,7 @@ class NotifierListTestCase(test.TestCase): self.flags(notification_driver='nova.notifier.list_notifier', list_notifier_drivers=['nova.notifier.no_op_notifier', 'nova.notifier.no_op_notifier']) - nova.notifier.api.notify('publisher_id', 'event_type', + nova.notifier.api.notify('contextarg', 'publisher_id', 'event_type', nova.notifier.api.WARN, dict(a=3)) self.assertEqual(self.notify_count, 2) self.assertEqual(self.exception_count, 0) @@ -68,7 +68,7 @@ class NotifierListTestCase(test.TestCase): self.flags(notification_driver='nova.notifier.list_notifier', list_notifier_drivers=['nova.notifier.no_op_notifier', 'nova.notifier.log_notifier']) - nova.notifier.api.notify('publisher_id', + nova.notifier.api.notify('contextarg', 'publisher_id', 'event_type', nova.notifier.api.WARN, dict(a=3)) self.assertEqual(self.notify_count, 1) self.assertEqual(self.exception_count, 1) @@ -78,7 +78,7 @@ class NotifierListTestCase(test.TestCase): list_notifier_drivers=['nova.notifier.no_op_notifier', 'nova.notifier.logo_notifier', 'fdsjgsdfhjkhgsfkj']) - nova.notifier.api.notify('publisher_id', + nova.notifier.api.notify('contextarg', 'publisher_id', 'event_type', nova.notifier.api.WARN, dict(a=3)) self.assertEqual(self.exception_count, 2) self.assertEqual(self.notify_count, 1) diff --git a/nova/tests/test_exception.py b/nova/tests/test_exception.py index acf1c6d69245..34f4f1829049 100644 --- a/nova/tests/test_exception.py +++ b/nova/tests/test_exception.py @@ -44,11 +44,12 @@ class FakeNotifier(object): self.provided_priority = None self.provided_payload = None - def notify(self, publisher, event, priority, payload): + def notify(self, context, publisher, event, priority, payload): self.provided_publisher = publisher self.provided_event = event self.provided_priority = priority self.provided_payload = payload + self.provided_context = context def good_function(): @@ -59,7 +60,7 @@ def bad_function_error(): raise exception.Error() -def bad_function_exception(): +def bad_function_exception(blah="a", boo="b", context=None): raise test.TestingException() @@ -82,10 +83,11 @@ class WrapExceptionTestCase(test.TestCase): wrapped = exception.wrap_exception(notifier, "publisher", "event", "level") self.assertRaises(test.TestingException, - wrapped(bad_function_exception)) + wrapped(bad_function_exception), context="context") self.assertEquals(notifier.provided_publisher, "publisher") self.assertEquals(notifier.provided_event, "event") self.assertEquals(notifier.provided_priority, "level") + self.assertEquals(notifier.provided_context, "context") for key in ['exception', 'args']: self.assertTrue(key in notifier.provided_payload.keys()) diff --git a/nova/tests/test_notifier.py b/nova/tests/test_notifier.py index ff51f6ba246b..c0d49163c72e 100644 --- a/nova/tests/test_notifier.py +++ b/nova/tests/test_notifier.py @@ -36,15 +36,15 @@ class NotifierTestCase(test.TestCase): self.stubs.Set(nova.notifier.no_op_notifier, 'notify', mock_notify) - notifier_api.notify('publisher_id', 'event_type', - nova.notifier.api.WARN, dict(a=3)) + notifier_api.notify('contextarg', 'publisher_id', 'event_type', + nova.notifier.api.WARN, dict(a=3)) self.assertEqual(self.notify_called, True) def test_verify_message_format(self): """A test to ensure changing the message format is prohibitively annoying""" - def message_assert(message): + def message_assert(context, message): fields = [('publisher_id', 'publisher_id'), ('event_type', 'event_type'), ('priority', 'WARN'), @@ -53,11 +53,12 @@ class NotifierTestCase(test.TestCase): self.assertEqual(message[k], v) self.assertTrue(len(message['message_id']) > 0) self.assertTrue(len(message['timestamp']) > 0) + self.assertEqual(context, 'contextarg') self.stubs.Set(nova.notifier.no_op_notifier, 'notify', message_assert) - notifier_api.notify('publisher_id', 'event_type', - nova.notifier.api.WARN, dict(a=3)) + notifier_api.notify('contextarg', 'publisher_id', 'event_type', + nova.notifier.api.WARN, dict(a=3)) def test_send_rabbit_notification(self): self.stubs.Set(nova.flags.FLAGS, 'notification_driver', @@ -68,14 +69,14 @@ class NotifierTestCase(test.TestCase): self.mock_notify = True self.stubs.Set(nova.rpc, 'notify', mock_notify) - notifier_api.notify('publisher_id', 'event_type', - nova.notifier.api.WARN, dict(a=3)) + notifier_api.notify('contextarg', 'publisher_id', 'event_type', + nova.notifier.api.WARN, dict(a=3)) self.assertEqual(self.mock_notify, True) def test_invalid_priority(self): self.assertRaises(nova.notifier.api.BadPriorityException, - notifier_api.notify, 'publisher_id', + notifier_api.notify, 'contextarg', 'publisher_id', 'event_type', 'not a priority', dict(a=3)) def test_rabbit_priority_queue(self): @@ -91,7 +92,8 @@ class NotifierTestCase(test.TestCase): self.test_topic = topic self.stubs.Set(nova.rpc, 'notify', mock_notify) - notifier_api.notify('publisher_id', 'event_type', 'DEBUG', dict(a=3)) + notifier_api.notify('contextarg', 'publisher_id', + 'event_type', 'DEBUG', dict(a=3)) self.assertEqual(self.test_topic, 'testnotify.debug') def test_error_notification(self): @@ -131,3 +133,47 @@ class NotifierTestCase(test.TestCase): self.assertEqual(3, example_api(1, 2)) self.assertEqual(self.notify_called, True) + + def test_decorator_context(self): + """Verify that the notify decorator can extract the 'context' arg.""" + self.notify_called = False + self.context_arg = None + + def example_api(arg1, arg2, context): + return arg1 + arg2 + + def example_api2(arg1, arg2, **kw): + return arg1 + arg2 + + example_api = nova.notifier.api.notify_decorator( + 'example_api', + example_api) + + example_api2 = nova.notifier.api.notify_decorator( + 'example_api2', + example_api2) + + def mock_notify(context, cls, _type, _priority, _payload): + self.notify_called = True + self.context_arg = context + + self.stubs.Set(nova.notifier.api, 'notify', + mock_notify) + + # Test positional context + self.assertEqual(3, example_api(1, 2, "contextname")) + self.assertEqual(self.notify_called, True) + self.assertEqual(self.context_arg, "contextname") + + self.notify_called = False + self.context_arg = None + + # Test named context + self.assertEqual(3, example_api2(1, 2, context="contextname2")) + self.assertEqual(self.notify_called, True) + self.assertEqual(self.context_arg, "contextname2") + + # Test missing context + self.assertEqual(3, example_api2(1, 2, bananas="delicious")) + self.assertEqual(self.notify_called, True) + self.assertEqual(self.context_arg, None)