diff --git a/nova/rpc.py b/nova/rpc.py index 9c92a6b0fee3..0c9b8ad0a0d2 100644 --- a/nova/rpc.py +++ b/nova/rpc.py @@ -47,6 +47,7 @@ CONF = nova.conf.CONF LOG = logging.getLogger(__name__) +# TODO(stephenfin): These should be private TRANSPORT = None LEGACY_NOTIFIER = None NOTIFICATION_TRANSPORT = None diff --git a/nova/tests/unit/test_rpc.py b/nova/tests/unit/test_rpc.py index 6cbccd245f05..59503316b350 100644 --- a/nova/tests/unit/test_rpc.py +++ b/nova/tests/unit/test_rpc.py @@ -11,38 +11,19 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -import copy -import fixtures import mock import oslo_messaging as messaging from oslo_messaging.rpc import dispatcher from oslo_serialization import jsonutils import six +import nova.conf from nova import context from nova import rpc from nova import test - -# Make a class that resets all of the global variables in nova.rpc -class RPCResetFixture(fixtures.Fixture): - def _setUp(self): - self.trans = copy.copy(rpc.TRANSPORT) - self.noti_trans = copy.copy(rpc.NOTIFICATION_TRANSPORT) - 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): - rpc.TRANSPORT = self.trans - rpc.NOTIFICATION_TRANSPORT = self.noti_trans - rpc.NOTIFIER = self.noti - rpc.ALLOWED_EXMODS = self.all_mods - rpc.EXTRA_EXMODS = self.ext_mods - rpc.CONF = self.conf +CONF = nova.conf.CONF class TestRPC(test.NoDBTestCase): @@ -50,93 +31,127 @@ class TestRPC(test.NoDBTestCase): # We're testing the rpc code so we can't use the RPCFixture. STUB_RPC = False - def setUp(self): - super(TestRPC, self).setUp() - self.useFixture(RPCResetFixture()) - + @mock.patch.object(rpc, 'TRANSPORT') + @mock.patch.object(rpc, 'NOTIFICATION_TRANSPORT') + @mock.patch.object(rpc, 'LEGACY_NOTIFIER') + @mock.patch.object(rpc, 'NOTIFIER') @mock.patch.object(rpc, 'get_allowed_exmods') @mock.patch.object(rpc, 'RequestContextSerializer') @mock.patch.object(messaging, 'get_notification_transport') @mock.patch.object(messaging, 'Notifier') - def test_init_unversioned(self, mock_notif, mock_noti_trans, - mock_ser, mock_exmods): + def _test_init(self, notification_format, expected_driver_topic_kwargs, + mock_notif, mock_noti_trans, mock_ser, mock_exmods, + mock_NOTIFIER, mock_LEGACY_NOTIFIER, mock_NOTIFICATION_TRANSPORT, + mock_TRANSPORT, + versioned_notifications_topics=None): + + if not versioned_notifications_topics: + versioned_notifications_topics = ['versioned_notifications'] + + self.flags( + notification_format=notification_format, + versioned_notifications_topics=versioned_notifications_topics, + group='notifications') + + legacy_notifier = mock.Mock() + notifier = mock.Mock() + notif_transport = mock.Mock() + transport = mock.Mock() + serializer = mock.Mock() + + mock_exmods.return_value = ['foo'] + mock_noti_trans.return_value = notif_transport + mock_ser.return_value = serializer + mock_notif.side_effect = [legacy_notifier, notifier] + + with mock.patch.object(rpc, 'create_transport') as create_transport, \ + mock.patch.object(rpc, 'get_transport_url') as get_url: + create_transport.return_value = transport + rpc.init(CONF) + create_transport.assert_called_once_with(get_url.return_value) + + self.assertTrue(mock_exmods.called) + self.assertIsNotNone(mock_TRANSPORT) + self.assertIsNotNone(mock_LEGACY_NOTIFIER) + self.assertIsNotNone(mock_NOTIFIER) + self.assertEqual(legacy_notifier, rpc.LEGACY_NOTIFIER) + self.assertEqual(notifier, rpc.NOTIFIER) + + expected_calls = [] + for kwargs in expected_driver_topic_kwargs: + expected_kwargs = {'serializer': serializer} + expected_kwargs.update(kwargs) + expected_calls.append(((notif_transport,), expected_kwargs)) + + self.assertEqual(expected_calls, mock_notif.call_args_list, + "The calls to messaging.Notifier() did not create " + "the legacy and versioned notifiers properly.") + + def test_init_unversioned(self): # The expected call to get the legacy notifier will require no new # kwargs, and we expect the new notifier will need the noop driver - expected = [{}, {'driver': 'noop'}] - self._test_init(mock_notif, mock_noti_trans, mock_ser, - mock_exmods, 'unversioned', expected) + expected_driver_topic_kwargs = [{}, {'driver': 'noop'}] + self._test_init('unversioned', expected_driver_topic_kwargs) - @mock.patch.object(rpc, 'get_allowed_exmods') - @mock.patch.object(rpc, 'RequestContextSerializer') - @mock.patch.object(messaging, 'get_notification_transport') - @mock.patch.object(messaging, 'Notifier') - def test_init_both(self, mock_notif, mock_noti_trans, - mock_ser, mock_exmods): - expected = [{}, {'topics': ['versioned_notifications']}] - self._test_init(mock_notif, mock_noti_trans, mock_ser, - mock_exmods, 'both', expected) + def test_init_both(self): + expected_driver_topic_kwargs = [ + {}, + {'topics': ['versioned_notifications']}] + self._test_init('both', expected_driver_topic_kwargs) - @mock.patch.object(rpc, 'get_allowed_exmods') - @mock.patch.object(rpc, 'RequestContextSerializer') - @mock.patch.object(messaging, 'get_notification_transport') - @mock.patch.object(messaging, 'Notifier') - def test_init_versioned(self, mock_notif, mock_noti_trans, - mock_ser, mock_exmods): - expected = [{'driver': 'noop'}, - {'topics': ['versioned_notifications']}] - self._test_init(mock_notif, mock_noti_trans, mock_ser, - mock_exmods, 'versioned', expected) + def test_init_versioned(self): + expected_driver_topic_kwargs = [ + {'driver': 'noop'}, + {'topics': ['versioned_notifications']}] + self._test_init('versioned', expected_driver_topic_kwargs) - @mock.patch.object(rpc, 'get_allowed_exmods') - @mock.patch.object(rpc, 'RequestContextSerializer') - @mock.patch.object(messaging, 'get_notification_transport') - @mock.patch.object(messaging, 'Notifier') - def test_init_versioned_with_custom_topics(self, mock_notif, - mock_noti_trans, mock_ser, - mock_exmods): - expected = [{'driver': 'noop'}, - {'topics': ['custom_topic1', 'custom_topic2']}] - self._test_init( - mock_notif, mock_noti_trans, mock_ser, mock_exmods, 'versioned', - expected, versioned_notification_topics=['custom_topic1', - 'custom_topic2']) + def test_init_versioned_with_custom_topics(self): + expected_driver_topic_kwargs = [ + {'driver': 'noop'}, + {'topics': ['custom_topic1', 'custom_topic2']}] + versioned_notifications_topics = ['custom_topic1', 'custom_topic2'] + self._test_init('versioned', expected_driver_topic_kwargs, + versioned_notifications_topics=versioned_notifications_topics) + @mock.patch.object(rpc, 'NOTIFICATION_TRANSPORT', new=mock.Mock()) + @mock.patch.object(rpc, 'LEGACY_NOTIFIER', new=mock.Mock()) + @mock.patch.object(rpc, 'NOTIFIER', new=mock.Mock()) def test_cleanup_transport_null(self): - rpc.NOTIFICATION_TRANSPORT = mock.Mock() - rpc.LEGACY_NOTIFIER = mock.Mock() - rpc.NOTIFIER = mock.Mock() + """Ensure cleanup fails if 'rpc.TRANSPORT' wasn't set.""" self.assertRaises(AssertionError, rpc.cleanup) + @mock.patch.object(rpc, 'TRANSPORT', new=mock.Mock()) + @mock.patch.object(rpc, 'LEGACY_NOTIFIER', new=mock.Mock()) + @mock.patch.object(rpc, 'NOTIFIER', new=mock.Mock()) def test_cleanup_notification_transport_null(self): - rpc.TRANSPORT = mock.Mock() - rpc.NOTIFIER = mock.Mock() + """Ensure cleanup fails if 'rpc.NOTIFICATION_TRANSPORT' wasn't set.""" self.assertRaises(AssertionError, rpc.cleanup) + @mock.patch.object(rpc, 'TRANSPORT', new=mock.Mock()) + @mock.patch.object(rpc, 'NOTIFICATION_TRANSPORT', new=mock.Mock()) + @mock.patch.object(rpc, 'NOTIFIER', new=mock.Mock()) def test_cleanup_legacy_notifier_null(self): - rpc.TRANSPORT = mock.Mock() - rpc.NOTIFICATION_TRANSPORT = mock.Mock() - rpc.NOTIFIER = mock.Mock() - - def test_cleanup_notifier_null(self): - rpc.TRANSPORT = mock.Mock() - rpc.LEGACY_NOTIFIER = mock.Mock() - rpc.NOTIFICATION_TRANSPORT = mock.Mock() + """Ensure cleanup fails if 'rpc.LEGACY_NOTIFIER' wasn't set.""" self.assertRaises(AssertionError, rpc.cleanup) - def test_cleanup(self): - rpc.LEGACY_NOTIFIER = mock.Mock() - rpc.NOTIFIER = mock.Mock() - rpc.NOTIFICATION_TRANSPORT = mock.Mock() - rpc.TRANSPORT = mock.Mock() - trans_cleanup = mock.Mock() - not_trans_cleanup = mock.Mock() - rpc.TRANSPORT.cleanup = trans_cleanup - rpc.NOTIFICATION_TRANSPORT.cleanup = not_trans_cleanup + @mock.patch.object(rpc, 'TRANSPORT', new=mock.Mock()) + @mock.patch.object(rpc, 'NOTIFICATION_TRANSPORT', new=mock.Mock()) + @mock.patch.object(rpc, 'LEGACY_NOTIFIER', new=mock.Mock()) + def test_cleanup_notifier_null(self): + """Ensure cleanup fails if 'rpc.NOTIFIER' wasn't set.""" + self.assertRaises(AssertionError, rpc.cleanup) + @mock.patch.object(rpc, 'TRANSPORT') + @mock.patch.object(rpc, 'NOTIFICATION_TRANSPORT') + @mock.patch.object(rpc, 'LEGACY_NOTIFIER') + @mock.patch.object(rpc, 'NOTIFIER') + def test_cleanup(self, mock_NOTIFIER, mock_LEGACY_NOTIFIER, + mock_NOTIFICATION_TRANSPORT, mock_TRANSPORT): rpc.cleanup() - trans_cleanup.assert_called_once_with() - not_trans_cleanup.assert_called_once_with() + mock_TRANSPORT.cleanup.assert_called_once_with() + mock_NOTIFICATION_TRANSPORT.cleanup.assert_called_once_with() + self.assertIsNone(rpc.TRANSPORT) self.assertIsNone(rpc.NOTIFICATION_TRANSPORT) self.assertIsNone(rpc.LEGACY_NOTIFIER) @@ -151,54 +166,56 @@ class TestRPC(test.NoDBTestCase): mock_set.assert_called_once_with(control_exchange) def test_add_extra_exmods(self): - rpc.EXTRA_EXMODS = [] + extra_exmods = [] - rpc.add_extra_exmods('foo', 'bar') - - self.assertEqual(['foo', 'bar'], rpc.EXTRA_EXMODS) + with mock.patch.object( + rpc, 'EXTRA_EXMODS', extra_exmods) as mock_EXTRA_EXMODS: + rpc.add_extra_exmods('foo', 'bar') + self.assertEqual(['foo', 'bar'], mock_EXTRA_EXMODS) def test_clear_extra_exmods(self): - rpc.EXTRA_EXMODS = ['foo', 'bar'] + extra_exmods = ['foo', 'bar'] - rpc.clear_extra_exmods() - - self.assertEqual(0, len(rpc.EXTRA_EXMODS)) + with mock.patch.object( + rpc, 'EXTRA_EXMODS', extra_exmods) as mock_EXTRA_EXMODS: + rpc.clear_extra_exmods() + self.assertEqual([], mock_EXTRA_EXMODS) def test_get_allowed_exmods(self): - rpc.ALLOWED_EXMODS = ['foo'] - rpc.EXTRA_EXMODS = ['bar'] + allowed_exmods = ['foo'] + extra_exmods = ['bar'] - exmods = rpc.get_allowed_exmods() + with test.nested( + mock.patch.object(rpc, 'EXTRA_EXMODS', extra_exmods), + mock.patch.object(rpc, 'ALLOWED_EXMODS', allowed_exmods) + ) as (mock_EXTRA_EXMODS, mock_ALLOWED_EXMODS): + exmods = rpc.get_allowed_exmods() self.assertEqual(['foo', 'bar'], exmods) @mock.patch.object(messaging, 'TransportURL') def test_get_transport_url(self, mock_url): - conf = mock.Mock() - rpc.CONF = conf mock_url.parse.return_value = 'foo' url = rpc.get_transport_url(url_str='bar') self.assertEqual('foo', url) - mock_url.parse.assert_called_once_with(conf, 'bar') + mock_url.parse.assert_called_once_with(rpc.CONF, 'bar') @mock.patch.object(messaging, 'TransportURL') def test_get_transport_url_null(self, mock_url): - conf = mock.Mock() - rpc.CONF = conf mock_url.parse.return_value = 'foo' url = rpc.get_transport_url() self.assertEqual('foo', url) - mock_url.parse.assert_called_once_with(conf, None) + mock_url.parse.assert_called_once_with(rpc.CONF, None) + @mock.patch.object(rpc, 'TRANSPORT') @mock.patch.object(rpc, 'profiler', None) @mock.patch.object(rpc, 'RequestContextSerializer') @mock.patch.object(messaging, 'RPCClient') - def test_get_client(self, mock_client, mock_ser): - rpc.TRANSPORT = mock.Mock() + def test_get_client(self, mock_client, mock_ser, mock_TRANSPORT): tgt = mock.Mock() ser = mock.Mock() mock_client.return_value = 'client' @@ -207,17 +224,17 @@ class TestRPC(test.NoDBTestCase): client = rpc.get_client(tgt, version_cap='1.0', serializer='foo') mock_ser.assert_called_once_with('foo') - mock_client.assert_called_once_with(rpc.TRANSPORT, + mock_client.assert_called_once_with(mock_TRANSPORT, tgt, version_cap='1.0', call_monitor_timeout=None, serializer=ser) self.assertEqual('client', client) + @mock.patch.object(rpc, 'TRANSPORT') @mock.patch.object(rpc, 'profiler', None) @mock.patch.object(rpc, 'RequestContextSerializer') @mock.patch.object(messaging, 'get_rpc_server') - def test_get_server(self, mock_get, mock_ser): - rpc.TRANSPORT = mock.Mock() + def test_get_server(self, mock_get, mock_ser, mock_TRANSPORT): ser = mock.Mock() tgt = mock.Mock() ends = mock.Mock() @@ -228,16 +245,17 @@ class TestRPC(test.NoDBTestCase): mock_ser.assert_called_once_with('foo') access_policy = dispatcher.DefaultRPCAccessPolicy - mock_get.assert_called_once_with(rpc.TRANSPORT, tgt, ends, + mock_get.assert_called_once_with(mock_TRANSPORT, tgt, ends, executor='eventlet', serializer=ser, access_policy=access_policy) self.assertEqual('server', server) + @mock.patch.object(rpc, 'TRANSPORT') @mock.patch.object(rpc, 'profiler', mock.Mock()) @mock.patch.object(rpc, 'ProfilerRequestContextSerializer') @mock.patch.object(messaging, 'RPCClient') - def test_get_client_profiler_enabled(self, mock_client, mock_ser): - rpc.TRANSPORT = mock.Mock() + def test_get_client_profiler_enabled(self, mock_client, mock_ser, + mock_TRANSPORT): tgt = mock.Mock() ser = mock.Mock() mock_client.return_value = 'client' @@ -246,17 +264,19 @@ class TestRPC(test.NoDBTestCase): client = rpc.get_client(tgt, version_cap='1.0', serializer='foo') mock_ser.assert_called_once_with('foo') - mock_client.assert_called_once_with(rpc.TRANSPORT, + mock_client.assert_called_once_with(mock_TRANSPORT, tgt, version_cap='1.0', call_monitor_timeout=None, serializer=ser) self.assertEqual('client', client) + @mock.patch.object(rpc, 'TRANSPORT') + @mock.patch.object(rpc, 'profiler', mock.Mock()) @mock.patch.object(rpc, 'profiler', mock.Mock()) @mock.patch.object(rpc, 'ProfilerRequestContextSerializer') @mock.patch.object(messaging, 'get_rpc_server') - def test_get_server_profiler_enabled(self, mock_get, mock_ser): - rpc.TRANSPORT = mock.Mock() + def test_get_server_profiler_enabled(self, mock_get, mock_ser, + mock_TRANSPORT): ser = mock.Mock() tgt = mock.Mock() ends = mock.Mock() @@ -267,16 +287,16 @@ class TestRPC(test.NoDBTestCase): mock_ser.assert_called_once_with('foo') access_policy = dispatcher.DefaultRPCAccessPolicy - mock_get.assert_called_once_with(rpc.TRANSPORT, tgt, ends, + mock_get.assert_called_once_with(mock_TRANSPORT, tgt, ends, executor='eventlet', serializer=ser, access_policy=access_policy) self.assertEqual('server', server) - def test_get_notifier(self): - rpc.LEGACY_NOTIFIER = mock.Mock() + @mock.patch.object(rpc, 'LEGACY_NOTIFIER') + def test_get_notifier(self, mock_LEGACY_NOTIFIER): mock_prep = mock.Mock() mock_prep.return_value = 'notifier' - rpc.LEGACY_NOTIFIER.prepare = mock_prep + mock_LEGACY_NOTIFIER.prepare = mock_prep notifier = rpc.get_notifier('service', publisher_id='foo') @@ -284,11 +304,11 @@ class TestRPC(test.NoDBTestCase): self.assertIsInstance(notifier, rpc.LegacyValidatingNotifier) self.assertEqual('notifier', notifier.notifier) - def test_get_notifier_null_publisher(self): - rpc.LEGACY_NOTIFIER = mock.Mock() + @mock.patch.object(rpc, 'LEGACY_NOTIFIER') + def test_get_notifier_null_publisher(self, mock_LEGACY_NOTIFIER): mock_prep = mock.Mock() mock_prep.return_value = 'notifier' - rpc.LEGACY_NOTIFIER.prepare = mock_prep + mock_LEGACY_NOTIFIER.prepare = mock_prep notifier = rpc.get_notifier('service', host='bar') @@ -296,11 +316,11 @@ class TestRPC(test.NoDBTestCase): self.assertIsInstance(notifier, rpc.LegacyValidatingNotifier) self.assertEqual('notifier', notifier.notifier) - def test_get_versioned_notifier(self): - rpc.NOTIFIER = mock.Mock() + @mock.patch.object(rpc, 'NOTIFIER') + def test_get_versioned_notifier(self, mock_NOTIFIER): mock_prep = mock.Mock() mock_prep.return_value = 'notifier' - rpc.NOTIFIER.prepare = mock_prep + mock_NOTIFIER.prepare = mock_prep notifier = rpc.get_versioned_notifier('service.foo') @@ -318,52 +338,6 @@ class TestRPC(test.NoDBTestCase): url=mock.sentinel.url, allowed_remote_exmods=exmods) - def _test_init(self, mock_notif, mock_noti_trans, mock_ser, - mock_exmods, notif_format, expected_driver_topic_kwargs, - versioned_notification_topics=['versioned_notifications']): - legacy_notifier = mock.Mock() - notifier = mock.Mock() - notif_transport = mock.Mock() - transport = mock.Mock() - serializer = mock.Mock() - conf = mock.Mock() - - conf.transport_url = None - conf.notifications.notification_format = notif_format - conf.notifications.versioned_notifications_topics = ( - versioned_notification_topics) - mock_exmods.return_value = ['foo'] - mock_noti_trans.return_value = notif_transport - mock_ser.return_value = serializer - mock_notif.side_effect = [legacy_notifier, notifier] - - @mock.patch.object(rpc, 'CONF', new=conf) - @mock.patch.object(rpc, 'create_transport') - @mock.patch.object(rpc, 'get_transport_url') - def _test(get_url, create_transport): - create_transport.return_value = transport - rpc.init(conf) - create_transport.assert_called_once_with(get_url.return_value) - - _test() - - self.assertTrue(mock_exmods.called) - self.assertIsNotNone(rpc.TRANSPORT) - self.assertIsNotNone(rpc.LEGACY_NOTIFIER) - self.assertIsNotNone(rpc.NOTIFIER) - self.assertEqual(legacy_notifier, rpc.LEGACY_NOTIFIER) - self.assertEqual(notifier, rpc.NOTIFIER) - - expected_calls = [] - for kwargs in expected_driver_topic_kwargs: - expected_kwargs = {'serializer': serializer} - expected_kwargs.update(kwargs) - expected_calls.append(((notif_transport,), expected_kwargs)) - - self.assertEqual(expected_calls, mock_notif.call_args_list, - "The calls to messaging.Notifier() did not create " - "the legacy and versioned notifiers properly.") - class TestJsonPayloadSerializer(test.NoDBTestCase): def test_serialize_entity(self):