diff options
author | Zuul <zuul@review.opendev.org> | 2020-08-11 12:08:47 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2020-08-11 12:08:47 +0000 |
commit | 5d21afee0aa3acea30afa03f5105502833b7ea44 (patch) | |
tree | 528f9bcaa7bc395b959f540ecda1c15b7e04dc7d | |
parent | b6e74c61652f4369f8e67d716402d5e5e3d464c4 (diff) | |
parent | 5b7ca45ae59d5656870a1a04c34d3774c5bc290d (diff) | |
download | ironic-5d21afee0aa3acea30afa03f5105502833b7ea44.tar.gz |
Merge "Remove locks before RPC bus is started" into stable/stein
-rw-r--r-- | ironic/common/rpc_service.py | 2 | ||||
-rw-r--r-- | ironic/conductor/base_manager.py | 34 | ||||
-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 | ||||
-rw-r--r-- | releasenotes/notes/remove-locks-first-d12ac27106f800f8.yaml | 9 |
6 files changed, 54 insertions, 6 deletions
diff --git a/ironic/common/rpc_service.py b/ironic/common/rpc_service.py index 9df03b917..a38582250 100644 --- a/ironic/common/rpc_service.py +++ b/ironic/common/rpc_service.py @@ -48,6 +48,8 @@ class RPCService(service.Service): admin_context = context.get_admin_context() serializer = objects_base.IronicObjectSerializer(is_server=True) + # Perform preparatory actions before starting the RPC listener + self.manager.prepare_host() if CONF.rpc_transport == 'json-rpc': self.rpcserver = json_rpc.WSGIService(self.manager, serializer) diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index 3960688b1..857be30c3 100644 --- a/ironic/conductor/base_manager.py +++ b/ironic/conductor/base_manager.py @@ -78,6 +78,33 @@ class BaseConductorManager(object): self.sensors_notifier = rpc.get_sensors_notifier() self._started = False self._shutdown = None + self.dbapi = None + + def prepare_host(self): + """Prepares host for initialization + + Removes existing database entries involved with node locking for nodes + in a transitory power state and nodes that are presently locked by + the hostname of this conductor. + + Under normal operation, this is also when the initial database + connectivity is established for the conductor's normal operation. + """ + # NOTE(TheJulia) We need to clear locks early on in the process + # of starting where the database shows we still hold them. + # This must be done before we re-register our existence in the + # conductors table and begin accepting new requests via RPC as + # if we do not then we may squash our *new* locks from new work. + + if not self.dbapi: + LOG.debug('Initializing database client for %s.', self.host) + self.dbapi = dbapi.get_instance() + LOG.debug('Removing stale locks from the database matching ' + 'this conductor\'s hostname: %s', self.host) + # clear all target_power_state with locks by this conductor + self.dbapi.clear_node_target_power_state(self.host) + # clear all locks held by this conductor before registering + self.dbapi.clear_node_reservations_for_conductor(self.host) def init_host(self, admin_context=None): """Initialize the conductor host. @@ -95,7 +122,8 @@ class BaseConductorManager(object): 'conductor manager')) self._shutdown = False - self.dbapi = dbapi.get_instance() + if not self.dbapi: + self.dbapi = dbapi.get_instance() self._keepalive_evt = threading.Event() """Event for the keepalive thread.""" @@ -143,10 +171,6 @@ class BaseConductorManager(object): self._collect_periodic_tasks(admin_context) - # clear all target_power_state with locks by this conductor - self.dbapi.clear_node_target_power_state(self.host) - # clear all locks held by this conductor before registering - self.dbapi.clear_node_reservations_for_conductor(self.host) try: # Register this conductor with the cluster self.conductor = objects.Conductor.register( diff --git a/ironic/tests/unit/common/test_rpc_service.py b/ironic/tests/unit/common/test_rpc_service.py index 755df4f81..b4e857bf8 100644 --- a/ironic/tests/unit/common/test_rpc_service.py +++ b/ironic/tests/unit/common/test_rpc_service.py @@ -35,13 +35,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() @@ -49,5 +50,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 44e492462..4eddeedf2 100644 --- a/ironic/tests/unit/conductor/mgr_utils.py +++ b/ironic/tests/unit/conductor/mgr_utils.py @@ -142,6 +142,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 424dbb231..906e5ead9 100644 --- a/ironic/tests/unit/conductor/test_base_manager.py +++ b/ironic/tests/unit/conductor/test_base_manager.py @@ -30,6 +30,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 fake @@ -248,6 +249,15 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.service.del_host() self.assertTrue(self.service._shutdown) + @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): diff --git a/releasenotes/notes/remove-locks-first-d12ac27106f800f8.yaml b/releasenotes/notes/remove-locks-first-d12ac27106f800f8.yaml new file mode 100644 index 000000000..a7e0cb958 --- /dev/null +++ b/releasenotes/notes/remove-locks-first-d12ac27106f800f8.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixes an issue where ``ironic-conductor`` initialization could return a + ``NodeNotLocked`` error for requests requiring locks when the conductor + was starting. This was due to the conductor removing locks after + beginning accepting new work. The lock removal has been moved to after + the Database connectivity has been established but before the RPC bus + is initialized. |