From 9dcab12b7c0d57d1f050cd2df58244ce90ce17e3 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Thu, 10 Nov 2016 13:19:32 -0800 Subject: [PATCH] Make RPCFixture support multiple connections For testing cells, we will need to track the driver instances that we give out by url. This normally just works with a conventional oslo.messaging driver, but the fake driver keeps internal data structures for simulating its bus. If we end up with clients creating a new instance of the driver in the rpc switching code, we'll never be able to send messages to services because we'll always have private/separate data structures. So, this makes the fixture wrap the transport creation stuff and unify references by url. In order to make this work, some retooling of rpc.init() is done, which makes it more in line with the recent additions we had for wrapping transport initialization per connection anyway. For now, a lot of our tests can't handle the possibility of multiple RPC connections due to them looking at the global transport_url configuration. So for the moment, even though this makes the fixture support multiple independent connections, we collapse any such attempts down to a single connection to the default broker. Note: this requires a fix in oslo.messaging 5.14.0 Depends-On: I01b6f5a20ea9752da46a546a908bd38cf11da681 Change-Id: Icb63d7dabd17f3c5633387793f68a8ba20863a7e --- nova/rpc.py | 4 +--- nova/tests/fixtures.py | 32 +++++++++++++++++++++++++++++++- nova/tests/unit/test_rpc.py | 34 ++++++++++++++++++---------------- requirements.txt | 2 +- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/nova/rpc.py b/nova/rpc.py index 72be15772b20..8963557952dd 100644 --- a/nova/rpc.py +++ b/nova/rpc.py @@ -71,9 +71,7 @@ TRANSPORT_ALIASES = { def init(conf): global TRANSPORT, NOTIFICATION_TRANSPORT, LEGACY_NOTIFIER, NOTIFIER exmods = get_allowed_exmods() - TRANSPORT = messaging.get_transport(conf, - allowed_remote_exmods=exmods, - aliases=TRANSPORT_ALIASES) + TRANSPORT = create_transport(get_transport_url()) NOTIFICATION_TRANSPORT = messaging.get_notification_transport( conf, allowed_remote_exmods=exmods, aliases=TRANSPORT_ALIASES) serializer = RequestContextSerializer(JsonPayloadSerializer()) diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index fb30db4fc476..6ff0fecfec69 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -27,6 +27,7 @@ import mock from oslo_concurrency import lockutils from oslo_config import cfg from oslo_db.sqlalchemy import enginefacade +import oslo_messaging as messaging from oslo_messaging import conffixture as messaging_conffixture import six @@ -495,6 +496,27 @@ class RPCFixture(fixtures.Fixture): super(RPCFixture, self).__init__() self.exmods = [] self.exmods.extend(exmods) + self._buses = {} + + def _fake_create_transport(self, url): + # FIXME(danms): Right now, collapse all connections + # to a single bus. This is how our tests expect things + # to work. When the tests are fixed, this fixture can + # support simulating multiple independent buses, and this + # hack should be removed. + url = None + + # NOTE(danms): This will be called with a non-None url by + # cells-aware code that is requesting to contact something on + # one of the many transports we're multplexing here. + if url not in self._buses: + exmods = rpc.get_allowed_exmods() + self._buses[url] = messaging.get_transport( + CONF, + url=url, + allowed_remote_exmods=exmods, + aliases=rpc.TRANSPORT_ALIASES) + return self._buses[url] def setUp(self): super(RPCFixture, self).setUp() @@ -504,7 +526,15 @@ class RPCFixture(fixtures.Fixture): self.messaging_conf = messaging_conffixture.ConfFixture(CONF) self.messaging_conf.transport_driver = 'fake' self.useFixture(self.messaging_conf) - rpc.init(CONF) + self.useFixture(fixtures.MonkeyPatch( + 'nova.rpc.create_transport', self._fake_create_transport)) + # NOTE(danms): Execute the init with get_transport_url() as None, + # instead of the parsed TransportURL(None) so that we can cache + # it as it will be called later if the default is requested by + # one of our mq-switching methods. + with mock.patch('nova.rpc.get_transport_url') as mock_gtu: + mock_gtu.return_value = None + rpc.init(CONF) class WarningsFixture(fixtures.Fixture): diff --git a/nova/tests/unit/test_rpc.py b/nova/tests/unit/test_rpc.py index dae202609a74..13afcdad3dbe 100644 --- a/nova/tests/unit/test_rpc.py +++ b/nova/tests/unit/test_rpc.py @@ -56,38 +56,35 @@ class TestRPC(testtools.TestCase): @mock.patch.object(rpc, 'get_allowed_exmods') @mock.patch.object(rpc, 'RequestContextSerializer') - @mock.patch.object(messaging, 'get_transport') @mock.patch.object(messaging, 'get_notification_transport') @mock.patch.object(messaging, 'Notifier') - def test_init_unversioned(self, mock_notif, mock_noti_trans, mock_trans, + def test_init_unversioned(self, mock_notif, mock_noti_trans, mock_ser, mock_exmods): # 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_trans, mock_ser, + self._test_init(mock_notif, mock_noti_trans, mock_ser, mock_exmods, 'unversioned', expected) @mock.patch.object(rpc, 'get_allowed_exmods') @mock.patch.object(rpc, 'RequestContextSerializer') - @mock.patch.object(messaging, 'get_transport') @mock.patch.object(messaging, 'get_notification_transport') @mock.patch.object(messaging, 'Notifier') - def test_init_both(self, mock_notif, mock_noti_trans, mock_trans, + 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_trans, mock_ser, + self._test_init(mock_notif, mock_noti_trans, mock_ser, mock_exmods, 'both', expected) @mock.patch.object(rpc, 'get_allowed_exmods') @mock.patch.object(rpc, 'RequestContextSerializer') - @mock.patch.object(messaging, 'get_transport') @mock.patch.object(messaging, 'get_notification_transport') @mock.patch.object(messaging, 'Notifier') - def test_init_versioned(self, mock_notif, mock_noti_trans, mock_trans, + 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_trans, mock_ser, + self._test_init(mock_notif, mock_noti_trans, mock_ser, mock_exmods, 'versioned', expected) def test_cleanup_transport_null(self): @@ -266,7 +263,7 @@ class TestRPC(testtools.TestCase): allowed_remote_exmods=exmods, aliases=rpc.TRANSPORT_ALIASES) - def _test_init(self, mock_notif, mock_noti_trans, mock_trans, mock_ser, + def _test_init(self, mock_notif, mock_noti_trans, mock_ser, mock_exmods, notif_format, expected_driver_topic_kwargs): legacy_notifier = mock.Mock() notifier = mock.Mock() @@ -275,19 +272,24 @@ class TestRPC(testtools.TestCase): serializer = mock.Mock() conf = mock.Mock() + conf.transport_url = None conf.notification_format = notif_format mock_exmods.return_value = ['foo'] - mock_trans.return_value = transport mock_noti_trans.return_value = notif_transport mock_ser.return_value = serializer mock_notif.side_effect = [legacy_notifier, notifier] - rpc.init(conf) + @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) - mock_exmods.assert_called_once_with() - mock_trans.assert_called_once_with(conf, - allowed_remote_exmods=['foo'], - aliases=rpc.TRANSPORT_ALIASES) + _test() + + self.assertTrue(mock_exmods.called) self.assertIsNotNone(rpc.TRANSPORT) self.assertIsNotNone(rpc.LEGACY_NOTIFIER) self.assertIsNotNone(rpc.NOTIFIER) diff --git a/requirements.txt b/requirements.txt index d58d76c15cab..f41e1911fc26 100644 --- a/requirements.txt +++ b/requirements.txt @@ -43,7 +43,7 @@ oslo.serialization>=1.10.0 # Apache-2.0 oslo.utils>=3.18.0 # Apache-2.0 oslo.db!=4.13.1,!=4.13.2,>=4.11.0 # Apache-2.0 oslo.rootwrap>=5.0.0 # Apache-2.0 -oslo.messaging>=5.2.0 # Apache-2.0 +oslo.messaging>=5.14.0 # Apache-2.0 oslo.policy>=1.17.0 # Apache-2.0 oslo.privsep>=1.9.0 # Apache-2.0 oslo.i18n>=2.1.0 # Apache-2.0