Merge "Remove locks before RPC bus is started" into stable/queens
This commit is contained in:
commit
918478b4e1
|
@ -44,9 +44,11 @@ class RPCService(service.Service):
|
||||||
super(RPCService, self).start()
|
super(RPCService, self).start()
|
||||||
admin_context = context.get_admin_context()
|
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()
|
||||||
target = messaging.Target(topic=self.topic, server=self.host)
|
target = messaging.Target(topic=self.topic, server=self.host)
|
||||||
endpoints = [self.manager]
|
endpoints = [self.manager]
|
||||||
serializer = objects_base.IronicObjectSerializer(is_server=True)
|
|
||||||
self.rpcserver = rpc.get_server(target, endpoints, serializer)
|
self.rpcserver = rpc.get_server(target, endpoints, serializer)
|
||||||
self.rpcserver.start()
|
self.rpcserver.start()
|
||||||
|
|
||||||
|
|
|
@ -76,6 +76,33 @@ class BaseConductorManager(object):
|
||||||
self.sensors_notifier = rpc.get_sensors_notifier()
|
self.sensors_notifier = rpc.get_sensors_notifier()
|
||||||
self._started = False
|
self._started = False
|
||||||
self._shutdown = None
|
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):
|
def init_host(self, admin_context=None):
|
||||||
"""Initialize the conductor host.
|
"""Initialize the conductor host.
|
||||||
|
@ -95,7 +122,8 @@ class BaseConductorManager(object):
|
||||||
'conductor manager'))
|
'conductor manager'))
|
||||||
self._shutdown = False
|
self._shutdown = False
|
||||||
|
|
||||||
self.dbapi = dbapi.get_instance()
|
if not self.dbapi:
|
||||||
|
self.dbapi = dbapi.get_instance()
|
||||||
|
|
||||||
self._keepalive_evt = threading.Event()
|
self._keepalive_evt = threading.Event()
|
||||||
"""Event for the keepalive thread."""
|
"""Event for the keepalive thread."""
|
||||||
|
@ -163,10 +191,6 @@ class BaseConductorManager(object):
|
||||||
"configured.")
|
"configured.")
|
||||||
raise exception.ConfigInvalid(msg)
|
raise exception.ConfigInvalid(msg)
|
||||||
|
|
||||||
# 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:
|
try:
|
||||||
# Register this conductor with the cluster
|
# Register this conductor with the cluster
|
||||||
self.conductor = objects.Conductor.register(
|
self.conductor = objects.Conductor.register(
|
||||||
|
|
|
@ -35,13 +35,14 @@ class TestRPCService(base.TestCase):
|
||||||
mgr_class = "ConductorManager"
|
mgr_class = "ConductorManager"
|
||||||
self.rpc_svc = rpc_service.RPCService(host, mgr_module, mgr_class)
|
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(oslo_messaging, 'Target', autospec=True)
|
||||||
@mock.patch.object(objects_base, 'IronicObjectSerializer', autospec=True)
|
@mock.patch.object(objects_base, 'IronicObjectSerializer', autospec=True)
|
||||||
@mock.patch.object(rpc, 'get_server', autospec=True)
|
@mock.patch.object(rpc, 'get_server', autospec=True)
|
||||||
@mock.patch.object(manager.ConductorManager, 'init_host', autospec=True)
|
@mock.patch.object(manager.ConductorManager, 'init_host', autospec=True)
|
||||||
@mock.patch.object(context, 'get_admin_context', autospec=True)
|
@mock.patch.object(context, 'get_admin_context', autospec=True)
|
||||||
def test_start(self, mock_ctx, mock_init_method,
|
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()
|
mock_rpc.return_value.start = mock.MagicMock()
|
||||||
self.rpc_svc.handle_signal = mock.MagicMock()
|
self.rpc_svc.handle_signal = mock.MagicMock()
|
||||||
self.rpc_svc.start()
|
self.rpc_svc.start()
|
||||||
|
@ -49,5 +50,6 @@ class TestRPCService(base.TestCase):
|
||||||
mock_target.assert_called_once_with(topic=self.rpc_svc.topic,
|
mock_target.assert_called_once_with(topic=self.rpc_svc.topic,
|
||||||
server="fake_host")
|
server="fake_host")
|
||||||
mock_ios.assert_called_once_with(is_server=True)
|
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_init_method.assert_called_once_with(self.rpc_svc.manager,
|
||||||
mock_ctx.return_value)
|
mock_ctx.return_value)
|
||||||
|
|
|
@ -194,6 +194,7 @@ class ServiceSetUpMixin(object):
|
||||||
self.service.init_host()
|
self.service.init_host()
|
||||||
else:
|
else:
|
||||||
with mock.patch.object(periodics, 'PeriodicWorker', autospec=True):
|
with mock.patch.object(periodics, 'PeriodicWorker', autospec=True):
|
||||||
|
self.service.prepare_host()
|
||||||
self.service.init_host()
|
self.service.init_host()
|
||||||
self.addCleanup(self._stop_service)
|
self.addCleanup(self._stop_service)
|
||||||
|
|
||||||
|
|
|
@ -28,6 +28,7 @@ from ironic.conductor import base_manager
|
||||||
from ironic.conductor import manager
|
from ironic.conductor import manager
|
||||||
from ironic.conductor import notification_utils
|
from ironic.conductor import notification_utils
|
||||||
from ironic.conductor import task_manager
|
from ironic.conductor import task_manager
|
||||||
|
from ironic.db import api as dbapi
|
||||||
from ironic.drivers import fake_hardware
|
from ironic.drivers import fake_hardware
|
||||||
from ironic.drivers import generic
|
from ironic.drivers import generic
|
||||||
from ironic import objects
|
from ironic import objects
|
||||||
|
@ -290,6 +291,15 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
|
||||||
self.service.del_host()
|
self.service.del_host()
|
||||||
self.assertTrue(self.service._shutdown)
|
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):
|
class CheckInterfacesTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
|
||||||
def test__check_enabled_interfaces_success(self):
|
def test__check_enabled_interfaces_success(self):
|
||||||
|
|
|
@ -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.
|
Loading…
Reference in New Issue