Fix notification short-circuit

We recently introduced a mechanism to short-circuit notifications when
they were disabled, unfortunately the mechanism was not backward
compatible with deprecated oslo notification setup that defaults to use
the transport mechanism of RPC.

In Mitaka oslo messaging introduced the transport_url specific for
notifications under the oslo_messaging_notifications section, but some
projects still use the default transport_url defined in the DEFAULT
section.

This patch fixes the notification short-circuit, now we don't check the
transport_url but the driver option in the oslo_messaging_notifications
section, which is a more robust way of checking, since it is backward
compatible.

Change-Id: Iccf51756701759e3727d3d989bab08bc05cac9a7
Closes-Bug: #1660928

cinder/rpc.py

@@ -77,7 +77,7 @@ def init(conf):
 
     # get_notification_transport has loaded oslo_messaging_notifications config
     # group, so we can now check if notifications are actually enabled.
-    if conf.oslo_messaging_notifications.transport_url:
+    if utils.notifications_enabled(conf):
         json_serializer = messaging.JsonPayloadSerializer()
         serializer = RequestContextSerializer(json_serializer)
         NOTIFIER = messaging.Notifier(NOTIFICATION_TRANSPORT,

cinder/test.py

@@ -31,6 +31,7 @@ from oslo_concurrency import lockutils
 from oslo_config import cfg
 from oslo_config import fixture as config_fixture
 from oslo_log.fixture import logging_error as log_fixture
+import oslo_messaging
 from oslo_messaging import conffixture as messaging_conffixture
 from oslo_serialization import jsonutils
 from oslo_utils import strutils
@@ -51,7 +52,6 @@ from cinder import service
 from cinder.tests import fixtures as cinder_fixtures
 from cinder.tests.unit import conf_fixture
 from cinder.tests.unit import fake_notifier
-from cinder.tests.unit import utils as test_utils
 from cinder.volume import utils
 
 
@@ -154,6 +154,14 @@ class TestCase(testtools.TestCase):
         self.messaging_conf.transport_driver = 'fake'
         self.messaging_conf.response_timeout = 15
         self.useFixture(self.messaging_conf)
+
+        # Load oslo_messaging_notifications config group so we can set an
+        # override to prevent notifications from being ignored due to the
+        # short-circuit mechanism.
+        oslo_messaging.get_notification_transport(CONF)
+        #  We need to use a valid driver for the notifications, so we use test.
+        self.override_config('driver', ['test'],
+                             group='oslo_messaging_notifications')
         rpc.init(CONF)
 
         # NOTE(geguileo): This is required because _determine_obj_version_cap
@@ -234,8 +242,6 @@ class TestCase(testtools.TestCase):
         coordination.COORDINATOR.start()
         self.addCleanup(coordination.COORDINATOR.stop)
 
-        test_utils.set_normal_rpc_notifier(self)
-
     def _restore_obj_registry(self):
         objects_base.CinderObjectRegistry._registry._obj_classes = \
             self._base_test_obj_backup

cinder/tests/unit/test_rpc.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import ddt
 import mock
 
 from cinder.objects import base
@@ -25,6 +26,7 @@ class FakeAPI(rpc.RPCAPI):
     BINARY = 'cinder-scheduler'
 
 
+@ddt.ddt
 class RPCAPITestCase(test.TestCase):
     """Tests RPCAPI mixin aggregating stuff related to RPC compatibility."""
 
@@ -84,9 +86,11 @@ class RPCAPITestCase(test.TestCase):
         self.assertFalse(get_min_obj.called)
         self.assertFalse(get_min_rpc.called)
 
+    @ddt.data([], ['noop'], ['noop', 'noop'])
     @mock.patch('oslo_messaging.JsonPayloadSerializer', wraps=True)
-    def test_init_no_notifications(self, serializer_mock):
-        self.override_config('transport_url', '',
+    def test_init_no_notifications(self, driver, serializer_mock):
+        """Test short-circuiting notifications with default and noop driver."""
+        self.override_config('driver', driver,
                              group='oslo_messaging_notifications')
         rpc.init(test.CONF)
         self.assertEqual(rpc.utils.DO_NOTHING, rpc.NOTIFIER)

cinder/tests/unit/test_utils.py

@@ -1416,6 +1416,7 @@ class TestValidateInteger(test.TestCase):
                           value, 'limit', min_value=-1, max_value=(2 ** 31))
 
 
+@ddt.ddt
 class TestNotificationShortCircuit(test.TestCase):
     def test_do_nothing_getter(self):
         """Test any attribute will always return the same instance (self)."""
@@ -1441,9 +1442,10 @@ class TestNotificationShortCircuit(test.TestCase):
         result = self._decorated_method()
         self.assertEqual(mock.sentinel.success, result)
 
-    def test_if_notification_enabled_when_disabled(self):
+    @ddt.data([], ['noop'], ['noop', 'noop'])
+    def test_if_notification_enabled_when_disabled(self, driver):
         """Test method is not called when notifications are disabled."""
-        self.override_config('transport_url', '',
+        self.override_config('driver', driver,
                              group='oslo_messaging_notifications')
         result = self._decorated_method()
         self.assertEqual(utils.DO_NOTHING, result)

cinder/tests/unit/utils.py

@@ -28,7 +28,6 @@ from cinder import db
 from cinder import exception
 from cinder import objects
 from cinder.objects import fields
-from cinder import rpc
 from cinder.tests.unit import fake_constants as fake
 
 
@@ -504,13 +503,3 @@ def create_populated_cluster(ctxt, num_services, num_down_svcs=0, **values):
         for i in range(num_services)
     ]
     return cluster, svcs
-
-
-def set_normal_rpc_notifier(test_case):
-    """Instead of shortcircuiting notifications, user Oslo's notifier."""
-    test_case.override_config('transport_url', 'fake_transport',
-                              group='oslo_messaging_notifications')
-    json_serializer = rpc.messaging.JsonPayloadSerializer()
-    serializer = rpc.RequestContextSerializer(json_serializer)
-    rpc.NOTIFIER = rpc.messaging.Notifier(rpc.NOTIFICATION_TRANSPORT,
-                                          serializer=serializer)

cinder/utils.py

@@ -1089,11 +1089,17 @@ class DoNothing(str):
 DO_NOTHING = DoNothing()
 
 
+def notifications_enabled(conf):
+    """Check if oslo notifications are enabled."""
+    notifications_driver = set(conf.oslo_messaging_notifications.driver)
+    return notifications_driver and notifications_driver != {'noop'}
+
+
 def if_notifications_enabled(f):
     """Calls decorated method only if notifications are enabled."""
     @functools.wraps(f)
     def wrapped(*args, **kwargs):
-        if CONF.oslo_messaging_notifications.transport_url:
+        if notifications_enabled(CONF):
             return f(*args, **kwargs)
         return DO_NOTHING
     return wrapped