diff options
author | Julia Kreger <juliaashleykreger@gmail.com> | 2020-07-09 13:02:35 -0700 |
---|---|---|
committer | Julia Kreger <juliaashleykreger@gmail.com> | 2020-07-28 08:03:21 -0700 |
commit | b8e4aba1ec06885c9b377d5ad659a13fea41049d (patch) | |
tree | 68fe90037203ab77b21441c998bce96b3248360e /ironic/tests/unit | |
parent | a63672f48505e6cb20aaef17e69bf53bc243db2c (diff) | |
download | ironic-b8e4aba1ec06885c9b377d5ad659a13fea41049d.tar.gz |
Remove locks before RPC bus is started
A partner performing some testing recognized a case where if a request
is sent to the Ironic Conductor while it is in the process of starting,
and the request makes it into be processed, yet latter the operation
fails with errors such as NodeNotLocked exception. Notably they were
able to reproduce this by requesting the attachment or detachment of
a VIF at the same time as restarting the conductor.
In part, this condition is due to to the conductor being restarted
where the conductors table includes the node being restarted and
the webserver has not possibly had a chance to observe that the
conductor is in the process of restarting as the hash ring is
still valid.
In short - Incoming RPC requests can come in during the initialization
window and as such we should not remove locks while the conductor could
possibly already be receiving work.
As such, we've added a ``prepare_host`` method which initializes
the conductor database connection and removes the stale locks.
Under normal operating conditions, the database client is reused.
rhbz# 1847305
Change-Id: I8e759168f1dc81cdcf430f3e33be990731595ec3
Diffstat (limited to 'ironic/tests/unit')
-rw-r--r-- | ironic/tests/unit/common/test_rpc_service.py | 4 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/mgr_utils.py | 1 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_base_manager.py | 10 |
3 files changed, 14 insertions, 1 deletions
diff --git a/ironic/tests/unit/common/test_rpc_service.py b/ironic/tests/unit/common/test_rpc_service.py index ec440324c..f187ff7e0 100644 --- a/ironic/tests/unit/common/test_rpc_service.py +++ b/ironic/tests/unit/common/test_rpc_service.py @@ -36,13 +36,14 @@ class TestRPCService(base.TestCase): mgr_class = "ConductorManager" self.rpc_svc = rpc_service.RPCService(host, mgr_module, mgr_class) + @mock.patch.object(manager.ConductorManager, 'prepare_host', autospec=True) @mock.patch.object(oslo_messaging, 'Target', autospec=True) @mock.patch.object(objects_base, 'IronicObjectSerializer', autospec=True) @mock.patch.object(rpc, 'get_server', autospec=True) @mock.patch.object(manager.ConductorManager, 'init_host', autospec=True) @mock.patch.object(context, 'get_admin_context', autospec=True) def test_start(self, mock_ctx, mock_init_method, - mock_rpc, mock_ios, mock_target): + mock_rpc, mock_ios, mock_target, mock_prepare_method): mock_rpc.return_value.start = mock.MagicMock() self.rpc_svc.handle_signal = mock.MagicMock() self.rpc_svc.start() @@ -50,5 +51,6 @@ class TestRPCService(base.TestCase): mock_target.assert_called_once_with(topic=self.rpc_svc.topic, server="fake_host") mock_ios.assert_called_once_with(is_server=True) + mock_prepare_method.assert_called_once_with(self.rpc_svc.manager) mock_init_method.assert_called_once_with(self.rpc_svc.manager, mock_ctx.return_value) diff --git a/ironic/tests/unit/conductor/mgr_utils.py b/ironic/tests/unit/conductor/mgr_utils.py index e5bc43bcb..9baadaf42 100644 --- a/ironic/tests/unit/conductor/mgr_utils.py +++ b/ironic/tests/unit/conductor/mgr_utils.py @@ -143,6 +143,7 @@ class ServiceSetUpMixin(object): self.service.init_host() else: with mock.patch.object(periodics, 'PeriodicWorker', autospec=True): + self.service.prepare_host() self.service.init_host() self.addCleanup(self._stop_service) diff --git a/ironic/tests/unit/conductor/test_base_manager.py b/ironic/tests/unit/conductor/test_base_manager.py index 35d8146ed..7680af153 100644 --- a/ironic/tests/unit/conductor/test_base_manager.py +++ b/ironic/tests/unit/conductor/test_base_manager.py @@ -31,6 +31,7 @@ from ironic.conductor import base_manager from ironic.conductor import manager from ironic.conductor import notification_utils from ironic.conductor import task_manager +from ironic.db import api as dbapi from ironic.drivers import fake_hardware from ironic.drivers import generic from ironic.drivers.modules import deploy_utils @@ -300,6 +301,15 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_zc.close.assert_called_once_with() self.assertIsNone(self.service._zeroconf) + @mock.patch.object(dbapi, 'get_instance', autospec=True) + def test_start_dbapi_single_call(self, mock_dbapi): + self._start_service() + # NOTE(TheJulia): This seems like it should only be 1, but + # the hash ring initailization pulls it's own database connection + # instance, which is likely a good thing, thus this is 2 instead of + # 3 without reuse of the database connection. + self.assertEqual(2, mock_dbapi.call_count) + class CheckInterfacesTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): def test__check_enabled_interfaces_success(self): |