diff --git a/nova/exception_wrapper.py b/nova/exception_wrapper.py index 644da0163067..7ea70c09c2c1 100644 --- a/nova/exception_wrapper.py +++ b/nova/exception_wrapper.py @@ -20,6 +20,7 @@ import nova.conf from nova.notifications.objects import base from nova.notifications.objects import exception from nova.objects import fields +from nova import rpc from nova import safe_utils CONF = nova.conf.CONF @@ -32,6 +33,7 @@ def _emit_exception_notification(notifier, context, ex, function_name, args, _emit_versioned_exception_notification(context, ex, binary) +@rpc.if_notifications_enabled def _emit_versioned_exception_notification(context, ex, binary): versioned_exception_payload = exception.ExceptionPayload.from_exception(ex) publisher = base.NotificationPublisher(context=context, host=CONF.host, diff --git a/nova/notifications/base.py b/nova/notifications/base.py index 0d8b8b27f383..e321bd5142ff 100644 --- a/nova/notifications/base.py +++ b/nova/notifications/base.py @@ -262,6 +262,7 @@ def _map_service_to_binary(service): return binary +@rpc.if_notifications_enabled def _send_versioned_instance_update(context, instance, payload, host, service): state_update = instance_notification.InstanceStateUpdatePayload( diff --git a/nova/notifications/objects/base.py b/nova/notifications/objects/base.py index 46669a081271..c55dec5d7840 100644 --- a/nova/notifications/objects/base.py +++ b/nova/notifications/objects/base.py @@ -93,6 +93,7 @@ class NotificationPayloadBase(NotificationObject): super(NotificationPayloadBase, self).__init__(**kwargs) self.populated = not self.SCHEMA + @rpc.if_notifications_enabled def populate_schema(self, **kwargs): """Populate the object based on the SCHEMA and the source objects @@ -163,6 +164,7 @@ class NotificationBase(NotificationObject): notify = getattr(notifier, self.priority) notify(context, event_type=event_type, payload=payload) + @rpc.if_notifications_enabled def emit(self, context): """Send the notification.""" assert self.payload.populated diff --git a/nova/rpc.py b/nova/rpc.py index 09ff40ca8ced..d69c5ab53174 100644 --- a/nova/rpc.py +++ b/nova/rpc.py @@ -215,6 +215,19 @@ def get_versioned_notifier(publisher_id): return NOTIFIER.prepare(publisher_id=publisher_id) +def if_notifications_enabled(f): + """Calls decorated method only if versioned notifications are enabled.""" + @functools.wraps(f) + def wrapped(*args, **kwargs): + if (NOTIFIER.is_enabled() and + CONF.notifications.notification_format in ('both', + 'versioned')): + return f(*args, **kwargs) + else: + return None + return wrapped + + def create_transport(url): exmods = get_allowed_exmods() return messaging.get_transport(CONF, diff --git a/nova/test.py b/nova/test.py index c8050e30d965..dc71ccae16fa 100644 --- a/nova/test.py +++ b/nova/test.py @@ -223,6 +223,12 @@ class TestCase(testtools.TestCase): self.useFixture(conf_fixture.ConfFixture(CONF)) self.useFixture(nova_fixtures.RPCFixture('nova.test')) + # we cannot set this in the ConfFixture as oslo only registers the + # notification opts at the first instantiation of a Notifier that + # happens only in the RPCFixture + CONF.set_default('driver', ['test'], + group='oslo_messaging_notifications') + # NOTE(danms): Make sure to reset us back to non-remote objects # for each test to avoid interactions. Also, backup the object # registry. diff --git a/nova/tests/unit/fake_notifier.py b/nova/tests/unit/fake_notifier.py index 5f5979b33f59..1b100e67a883 100644 --- a/nova/tests/unit/fake_notifier.py +++ b/nova/tests/unit/fake_notifier.py @@ -69,6 +69,9 @@ class FakeNotifier(object): payload, ctxt) NOTIFICATIONS.append(msg) + def is_enabled(self): + return True + class FakeVersionedNotifier(FakeNotifier): def _notify(self, priority, ctxt, event_type, payload): diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index 38f574ce7227..a06687a3bee6 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -296,6 +296,54 @@ class TestNotificationBase(test.NoDBTestCase): self.assertIn('test-update-1.json', self.TestNotification.samples) self.assertIn('test-update-2.json', self.TestNotification.samples) + @mock.patch('nova.notifications.objects.base.NotificationBase._emit') + @mock.patch('nova.rpc.NOTIFIER') + def test_payload_is_not_generated_if_notifier_is_not_enabled( + self, mock_notifier, mock_emit): + mock_notifier.is_enabled.return_value = False + + payload = self.TestNotificationPayload( + extra_field='test string') + self.payload.populate_schema(source_field=self.my_obj) + noti = self.TestNotification( + event_type=notification.EventType( + object='test_object', + action=fields.NotificationAction.UPDATE), + publisher=notification.NotificationPublisher.from_service_obj( + self.service_obj), + priority=fields.NotificationPriority.INFO, + payload=payload) + + mock_context = mock.Mock() + + noti.emit(mock_context) + + self.assertFalse(payload.populated) + self.assertFalse(mock_emit.called) + + @mock.patch('nova.notifications.objects.base.NotificationBase._emit') + def test_payload_is_not_generated_if_notification_format_is_unversioned( + self, mock_emit): + self.flags(notification_format='unversioned', group='notifications') + + payload = self.TestNotificationPayload( + extra_field='test string') + self.payload.populate_schema(source_field=self.my_obj) + noti = self.TestNotification( + event_type=notification.EventType( + object='test_object', + action=fields.NotificationAction.UPDATE), + publisher=notification.NotificationPublisher.from_service_obj( + self.service_obj), + priority=fields.NotificationPriority.INFO, + payload=payload) + + mock_context = mock.Mock() + + noti.emit(mock_context) + + self.assertFalse(payload.populated) + self.assertFalse(mock_emit.called) notification_object_data = { 'AggregateNotification': '1.0-a73147b93b520ff0061865849d3dfa56', @@ -381,38 +429,79 @@ def get_extra_data(obj_class): class TestInstanceNotification(test.NoDBTestCase): - @mock.patch('nova.notifications.objects.instance.' - 'InstanceUpdateNotification._emit') - def test_send_version_instance_update_uses_flavor(self, mock_emit): - # Make sure that the notification payload chooses the values in - # instance.flavor.$value instead of instance.$value - test_keys = ['memory_mb', 'vcpus', 'root_gb', 'ephemeral_gb', 'swap'] - flavor_values = {k: 123 for k in test_keys} - instance_values = {k: 456 for k in test_keys} + def setUp(self): + super(TestInstanceNotification, self).setUp() + self.test_keys = ['memory_mb', 'vcpus', 'root_gb', 'ephemeral_gb', + 'swap'] + self.flavor_values = {k: 123 for k in self.test_keys} + instance_values = {k: 456 for k in self.test_keys} flavor = objects.Flavor(flavorid='test-flavor', name='test-flavor', disabled=False, projects=[], is_public=True, - extra_specs = {}, **flavor_values) + extra_specs={}, **self.flavor_values) info_cache = objects.InstanceInfoCache( network_info=network_model.NetworkInfo()) - instance = objects.Instance( + self.instance = objects.Instance( flavor=flavor, info_cache=info_cache, metadata={}, uuid=uuids.instance1, locked=False, **instance_values) - payload = { + self.payload = { 'bandwidth': {}, 'audit_period_ending': timeutils.utcnow(), 'audit_period_beginning': timeutils.utcnow(), } + + @mock.patch('nova.notifications.objects.instance.' + 'InstanceUpdateNotification._emit') + def test_send_version_instance_update_uses_flavor(self, mock_emit): + # Make sure that the notification payload chooses the values in + # instance.flavor.$value instead of instance.$value notification_base._send_versioned_instance_update( mock.MagicMock(), - instance, - payload, + self.instance, + self.payload, 'host', 'compute') payload = mock_emit.call_args_list[0][1]['payload']['nova_object.data'] flavor_payload = payload['flavor']['nova_object.data'] - data = {k: flavor_payload[k] for k in test_keys} - self.assertEqual(flavor_values, data) + data = {k: flavor_payload[k] for k in self.test_keys} + self.assertEqual(self.flavor_values, data) + + @mock.patch('nova.rpc.NOTIFIER') + @mock.patch('nova.notifications.objects.instance.' + 'InstanceUpdatePayload.__init__', return_value=None) + @mock.patch('nova.notifications.objects.instance.' + 'InstanceUpdateNotification.__init__', return_value=None) + def test_send_versioned_instance_notification_is_not_called_disabled( + self, mock_notification, mock_payload, mock_notifier): + mock_notifier.is_enabled.return_value = False + + notification_base._send_versioned_instance_update( + mock.MagicMock(), + self.instance, + self.payload, + 'host', + 'compute') + + self.assertFalse(mock_payload.called) + self.assertFalse(mock_notification.called) + + @mock.patch('nova.notifications.objects.instance.' + 'InstanceUpdatePayload.__init__', return_value=None) + @mock.patch('nova.notifications.objects.instance.' + 'InstanceUpdateNotification.__init__', return_value=None) + def test_send_versioned_instance_notification_is_not_called_unversioned( + self, mock_notification, mock_payload): + self.flags(notification_format='unversioned', group='notifications') + + notification_base._send_versioned_instance_update( + mock.MagicMock(), + self.instance, + self.payload, + 'host', + 'compute') + + self.assertFalse(mock_payload.called) + self.assertFalse(mock_notification.called) diff --git a/nova/tests/unit/test_exception.py b/nova/tests/unit/test_exception.py index d3ae76161627..816bf06b01f3 100644 --- a/nova/tests/unit/test_exception.py +++ b/nova/tests/unit/test_exception.py @@ -16,6 +16,7 @@ import inspect +import mock import six from webob.util import status_reasons @@ -110,6 +111,33 @@ class WrapExceptionTestCase(test.NoDBTestCase): self.assertEqual('nova.tests.unit.test_exception', payload['module_name']) + @mock.patch('nova.rpc.NOTIFIER') + @mock.patch('nova.notifications.objects.exception.' + 'ExceptionNotification.__init__') + def test_wrap_exception_notification_not_emitted_if_disabled( + self, mock_notification, mock_notifier): + mock_notifier.is_enabled.return_value = False + + wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'), + binary='fake-binary') + ctxt = context.get_admin_context() + self.assertRaises(test.TestingException, + wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) + self.assertFalse(mock_notification.called) + + @mock.patch('nova.notifications.objects.exception.' + 'ExceptionNotification.__init__') + def test_wrap_exception_notification_not_emitted_if_unversioned( + self, mock_notifier): + self.flags(notification_format='unversioned', group='notifications') + + wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'), + binary='fake-binary') + ctxt = context.get_admin_context() + self.assertRaises(test.TestingException, + wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) + self.assertFalse(mock_notifier.called) + class NovaExceptionTestCase(test.NoDBTestCase): def test_default_error_msg(self): diff --git a/nova/tests/unit/test_rpc.py b/nova/tests/unit/test_rpc.py index 051ea8e7913c..f97f04081c63 100644 --- a/nova/tests/unit/test_rpc.py +++ b/nova/tests/unit/test_rpc.py @@ -35,6 +35,7 @@ class RPCResetFixture(fixtures.Fixture): self.noti = copy.copy(rpc.NOTIFIER) self.all_mods = copy.copy(rpc.ALLOWED_EXMODS) self.ext_mods = copy.copy(rpc.EXTRA_EXMODS) + self.conf = copy.copy(rpc.CONF) self.addCleanup(self._reset_everything) def _reset_everything(self): @@ -43,6 +44,7 @@ class RPCResetFixture(fixtures.Fixture): rpc.NOTIFIER = self.noti rpc.ALLOWED_EXMODS = self.all_mods rpc.EXTRA_EXMODS = self.ext_mods + rpc.CONF = self.conf # We can't import nova.test.TestCase because that sets up an RPCFixture @@ -525,3 +527,26 @@ class TestClientRouter(test.NoDBTestCase): self.assertEqual(router.default_client, client) self.assertFalse(mock_rpcclient.called) + + +class TestIsNotificationsEnabledDecorator(test.NoDBTestCase): + + def setUp(self): + super(TestIsNotificationsEnabledDecorator, self).setUp() + self.f = mock.Mock() + self.f.__name__ = 'f' + self.decorated = rpc.if_notifications_enabled(self.f) + + def test_call_func_if_needed(self): + self.decorated() + self.f.assert_called_once_with() + + @mock.patch('nova.rpc.NOTIFIER.is_enabled', return_value=False) + def test_not_call_func_if_notifier_disabled(self, mock_is_enabled): + self.decorated() + self.assertEqual(0, len(self.f.mock_calls)) + + def test_not_call_func_if_only_unversioned_notifications_requested(self): + self.flags(notification_format='unversioned', group='notifications') + self.decorated() + self.assertEqual(0, len(self.f.mock_calls))