diff --git a/ironic/cmd/api.py b/ironic/cmd/api.py index fe2a058ebb..e95bd8c516 100644 --- a/ironic/cmd/api.py +++ b/ironic/cmd/api.py @@ -22,6 +22,7 @@ import sys from oslo_config import cfg from ironic.common import service as ironic_service +from ironic.common import wsgi_service from ironic.objects import base from ironic.objects import indirection @@ -38,7 +39,7 @@ def main(): # Build and start the WSGI app launcher = ironic_service.process_launcher() - server = ironic_service.WSGIService('ironic_api', CONF.api.enable_ssl_api) + server = wsgi_service.WSGIService('ironic_api', CONF.api.enable_ssl_api) launcher.launch_service(server, workers=server.workers) launcher.wait() diff --git a/ironic/cmd/conductor.py b/ironic/cmd/conductor.py index 14728a5a25..57dc905678 100644 --- a/ironic/cmd/conductor.py +++ b/ironic/cmd/conductor.py @@ -26,6 +26,7 @@ from oslo_log import log from oslo_service import service from ironic.common.i18n import _LW +from ironic.common import rpc_service from ironic.common import service as ironic_service from ironic.conf import auth @@ -58,12 +59,20 @@ def _check_auth_options(conf): def main(): + # NOTE(lucasagomes): Safeguard to prevent 'ironic.conductor.manager' + # from being imported prior to the configuration options being loaded. + # If this happened, the periodic decorators would always use the + # default values of the options instead of the configured ones. For + # more information see: https://bugs.launchpad.net/ironic/+bug/1562258 + # and https://bugs.launchpad.net/ironic/+bug/1279774. + assert 'ironic.conductor.manager' not in sys.modules + # Parse config file and command line options, then start logging ironic_service.prepare_service(sys.argv) - mgr = ironic_service.RPCService(CONF.host, - 'ironic.conductor.manager', - 'ConductorManager') + mgr = rpc_service.RPCService(CONF.host, + 'ironic.conductor.manager', + 'ConductorManager') _check_auth_options(CONF) diff --git a/ironic/common/rpc_service.py b/ironic/common/rpc_service.py new file mode 100644 index 0000000000..aece81adf5 --- /dev/null +++ b/ironic/common/rpc_service.py @@ -0,0 +1,91 @@ +# -*- encoding: utf-8 -*- +# +# Copyright © 2012 eNovance +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# 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 signal + +from oslo_log import log +import oslo_messaging as messaging +from oslo_service import service +from oslo_utils import importutils + +from ironic.common import context +from ironic.common.i18n import _LE, _LI +from ironic.common import rpc +from ironic.objects import base as objects_base + +LOG = log.getLogger(__name__) + + +class RPCService(service.Service): + + def __init__(self, host, manager_module, manager_class): + super(RPCService, self).__init__() + self.host = host + manager_module = importutils.try_import(manager_module) + manager_class = getattr(manager_module, manager_class) + self.manager = manager_class(host, manager_module.MANAGER_TOPIC) + self.topic = self.manager.topic + self.rpcserver = None + self.deregister = True + + def start(self): + super(RPCService, self).start() + admin_context = context.get_admin_context() + + target = messaging.Target(topic=self.topic, server=self.host) + endpoints = [self.manager] + serializer = objects_base.IronicObjectSerializer() + self.rpcserver = rpc.get_server(target, endpoints, serializer) + self.rpcserver.start() + + self.handle_signal() + self.manager.init_host(admin_context) + + LOG.info(_LI('Created RPC server for service %(service)s on host ' + '%(host)s.'), + {'service': self.topic, 'host': self.host}) + + def stop(self): + try: + self.rpcserver.stop() + self.rpcserver.wait() + except Exception as e: + LOG.exception(_LE('Service error occurred when stopping the ' + 'RPC server. Error: %s'), e) + try: + self.manager.del_host(deregister=self.deregister) + except Exception as e: + LOG.exception(_LE('Service error occurred when cleaning up ' + 'the RPC manager. Error: %s'), e) + + super(RPCService, self).stop(graceful=True) + LOG.info(_LI('Stopped RPC server for service %(service)s on host ' + '%(host)s.'), + {'service': self.topic, 'host': self.host}) + + def _handle_signal(self, signo, frame): + LOG.info(_LI('Got signal SIGUSR1. Not deregistering on next shutdown ' + 'of service %(service)s on host %(host)s.'), + {'service': self.topic, 'host': self.host}) + self.deregister = False + + def handle_signal(self): + """Add a signal handler for SIGUSR1. + + The handler ensures that the manager is not deregistered when it is + shutdown. + """ + signal.signal(signal.SIGUSR1, self._handle_signal) diff --git a/ironic/common/service.py b/ironic/common/service.py index be4ba0fa26..a9d9cc7e54 100644 --- a/ironic/common/service.py +++ b/ironic/common/service.py @@ -14,90 +14,16 @@ # License for the specific language governing permissions and limitations # under the License. -import signal - -from oslo_concurrency import processutils from oslo_log import log -import oslo_messaging as messaging from oslo_service import service -from oslo_service import wsgi -from oslo_utils import importutils -from ironic.api import app from ironic.common import config -from ironic.common import context -from ironic.common import exception -from ironic.common.i18n import _, _LE, _LI -from ironic.common import rpc from ironic.conf import CONF from ironic import objects -from ironic.objects import base as objects_base LOG = log.getLogger(__name__) -class RPCService(service.Service): - - def __init__(self, host, manager_module, manager_class): - super(RPCService, self).__init__() - self.host = host - manager_module = importutils.try_import(manager_module) - manager_class = getattr(manager_module, manager_class) - self.manager = manager_class(host, manager_module.MANAGER_TOPIC) - self.topic = self.manager.topic - self.rpcserver = None - self.deregister = True - - def start(self): - super(RPCService, self).start() - admin_context = context.get_admin_context() - - target = messaging.Target(topic=self.topic, server=self.host) - endpoints = [self.manager] - serializer = objects_base.IronicObjectSerializer() - self.rpcserver = rpc.get_server(target, endpoints, serializer) - self.rpcserver.start() - - self.handle_signal() - self.manager.init_host(admin_context) - - LOG.info(_LI('Created RPC server for service %(service)s on host ' - '%(host)s.'), - {'service': self.topic, 'host': self.host}) - - def stop(self): - try: - self.rpcserver.stop() - self.rpcserver.wait() - except Exception as e: - LOG.exception(_LE('Service error occurred when stopping the ' - 'RPC server. Error: %s'), e) - try: - self.manager.del_host(deregister=self.deregister) - except Exception as e: - LOG.exception(_LE('Service error occurred when cleaning up ' - 'the RPC manager. Error: %s'), e) - - super(RPCService, self).stop(graceful=True) - LOG.info(_LI('Stopped RPC server for service %(service)s on host ' - '%(host)s.'), - {'service': self.topic, 'host': self.host}) - - def _handle_signal(self, signo, frame): - LOG.info(_LI('Got signal SIGUSR1. Not deregistering on next shutdown ' - 'of service %(service)s on host %(host)s.'), - {'service': self.topic, 'host': self.host}) - self.deregister = False - - def handle_signal(self): - """Add a signal handler for SIGUSR1. - - The handler ensures that the manager is not deregistered when it is - shutdown. - """ - signal.signal(signal.SIGUSR1, self._handle_signal) - - def prepare_service(argv=None): argv = [] if argv is None else argv log.register_options(CONF) @@ -124,56 +50,3 @@ def prepare_service(argv=None): def process_launcher(): return service.ProcessLauncher(CONF) - - -class WSGIService(service.ServiceBase): - """Provides ability to launch ironic API from wsgi app.""" - - def __init__(self, name, use_ssl=False): - """Initialize, but do not start the WSGI server. - - :param name: The name of the WSGI server given to the loader. - :param use_ssl: Wraps the socket in an SSL context if True. - :returns: None - """ - self.name = name - self.app = app.VersionSelectorApplication() - self.workers = (CONF.api.api_workers or - processutils.get_worker_count()) - if self.workers and self.workers < 1: - raise exception.ConfigInvalid( - _("api_workers value of %d is invalid, " - "must be greater than 0.") % self.workers) - - self.server = wsgi.Server(CONF, name, self.app, - host=CONF.api.host_ip, - port=CONF.api.port, - use_ssl=use_ssl) - - def start(self): - """Start serving this service using loaded configuration. - - :returns: None - """ - self.server.start() - - def stop(self): - """Stop serving this API. - - :returns: None - """ - self.server.stop() - - def wait(self): - """Wait for the service to stop serving this API. - - :returns: None - """ - self.server.wait() - - def reset(self): - """Reset server greenpool size to default. - - :returns: None - """ - self.server.reset() diff --git a/ironic/common/wsgi_service.py b/ironic/common/wsgi_service.py new file mode 100644 index 0000000000..c2a9f235bc --- /dev/null +++ b/ironic/common/wsgi_service.py @@ -0,0 +1,76 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_concurrency import processutils +from oslo_log import log +from oslo_service import service +from oslo_service import wsgi + +from ironic.api import app +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.conf import CONF + +LOG = log.getLogger(__name__) + + +class WSGIService(service.ServiceBase): + """Provides ability to launch ironic API from wsgi app.""" + + def __init__(self, name, use_ssl=False): + """Initialize, but do not start the WSGI server. + + :param name: The name of the WSGI server given to the loader. + :param use_ssl: Wraps the socket in an SSL context if True. + :returns: None + """ + self.name = name + self.app = app.VersionSelectorApplication() + self.workers = (CONF.api.api_workers or + processutils.get_worker_count()) + if self.workers and self.workers < 1: + raise exception.ConfigInvalid( + _("api_workers value of %d is invalid, " + "must be greater than 0.") % self.workers) + + self.server = wsgi.Server(CONF, name, self.app, + host=CONF.api.host_ip, + port=CONF.api.port, + use_ssl=use_ssl) + + def start(self): + """Start serving this service using loaded configuration. + + :returns: None + """ + self.server.start() + + def stop(self): + """Stop serving this API. + + :returns: None + """ + self.server.stop() + + def wait(self): + """Wait for the service to stop serving this API. + + :returns: None + """ + self.server.wait() + + def reset(self): + """Reset server greenpool size to default. + + :returns: None + """ + self.server.reset() diff --git a/ironic/tests/unit/common/test_rpc_service.py b/ironic/tests/unit/common/test_rpc_service.py new file mode 100644 index 0000000000..b1e0647306 --- /dev/null +++ b/ironic/tests/unit/common/test_rpc_service.py @@ -0,0 +1,53 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# 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 mock +from oslo_config import cfg +import oslo_messaging +from oslo_service import service as base_service + +from ironic.common import context +from ironic.common import rpc +from ironic.common import rpc_service +from ironic.conductor import manager +from ironic.objects import base as objects_base +from ironic.tests import base + +CONF = cfg.CONF + + +@mock.patch.object(base_service.Service, '__init__', lambda *_, **__: None) +class TestRPCService(base.TestCase): + + def setUp(self): + super(TestRPCService, self).setUp() + host = "fake_host" + mgr_module = "ironic.conductor.manager" + mgr_class = "ConductorManager" + self.rpc_svc = rpc_service.RPCService(host, mgr_module, mgr_class) + + @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.return_value.start = mock.MagicMock() + self.rpc_svc.handle_signal = mock.MagicMock() + self.rpc_svc.start() + mock_ctx.assert_called_once_with() + mock_target.assert_called_once_with(topic=self.rpc_svc.topic, + server="fake_host") + mock_ios.assert_called_once_with() + mock_init_method.assert_called_once_with(self.rpc_svc.manager, + mock_ctx.return_value) diff --git a/ironic/tests/unit/common/test_service.py b/ironic/tests/unit/common/test_service.py deleted file mode 100644 index 92bcf8e1f0..0000000000 --- a/ironic/tests/unit/common/test_service.py +++ /dev/null @@ -1,100 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# 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 mock -from oslo_concurrency import processutils -from oslo_config import cfg -import oslo_messaging -from oslo_service import service as base_service - -from ironic.common import context -from ironic.common import exception -from ironic.common import rpc -from ironic.common import service -from ironic.conductor import manager -from ironic.objects import base as objects_base -from ironic.tests import base - -CONF = cfg.CONF - - -@mock.patch.object(base_service.Service, '__init__', lambda *_, **__: None) -class TestRPCService(base.TestCase): - - def setUp(self): - super(TestRPCService, self).setUp() - host = "fake_host" - mgr_module = "ironic.conductor.manager" - mgr_class = "ConductorManager" - self.rpc_svc = service.RPCService(host, mgr_module, mgr_class) - - @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.return_value.start = mock.MagicMock() - self.rpc_svc.handle_signal = mock.MagicMock() - self.rpc_svc.start() - mock_ctx.assert_called_once_with() - mock_target.assert_called_once_with(topic=self.rpc_svc.topic, - server="fake_host") - mock_ios.assert_called_once_with() - mock_init_method.assert_called_once_with(self.rpc_svc.manager, - mock_ctx.return_value) - - -class TestWSGIService(base.TestCase): - @mock.patch.object(service.wsgi, 'Server') - def test_workers_set_default(self, wsgi_server): - service_name = "ironic_api" - test_service = service.WSGIService(service_name) - self.assertEqual(processutils.get_worker_count(), - test_service.workers) - wsgi_server.assert_called_once_with(CONF, service_name, - test_service.app, - host='0.0.0.0', - port=6385, - use_ssl=False) - - @mock.patch.object(service.wsgi, 'Server') - def test_workers_set_correct_setting(self, wsgi_server): - self.config(api_workers=8, group='api') - test_service = service.WSGIService("ironic_api") - self.assertEqual(8, test_service.workers) - - @mock.patch.object(service.wsgi, 'Server') - def test_workers_set_zero_setting(self, wsgi_server): - self.config(api_workers=0, group='api') - test_service = service.WSGIService("ironic_api") - self.assertEqual(processutils.get_worker_count(), test_service.workers) - - @mock.patch.object(service.wsgi, 'Server') - def test_workers_set_negative_setting(self, wsgi_server): - self.config(api_workers=-2, group='api') - self.assertRaises(exception.ConfigInvalid, - service.WSGIService, - 'ironic_api') - self.assertFalse(wsgi_server.called) - - @mock.patch.object(service.wsgi, 'Server') - def test_wsgi_service_with_ssl_enabled(self, wsgi_server): - self.config(enable_ssl_api=True, group='api') - service_name = 'ironic_api' - srv = service.WSGIService('ironic_api', CONF.api.enable_ssl_api) - wsgi_server.assert_called_once_with(CONF, service_name, - srv.app, - host='0.0.0.0', - port=6385, - use_ssl=True) diff --git a/ironic/tests/unit/common/test_wsgi_service.py b/ironic/tests/unit/common/test_wsgi_service.py new file mode 100644 index 0000000000..cfaadef958 --- /dev/null +++ b/ironic/tests/unit/common/test_wsgi_service.py @@ -0,0 +1,66 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# 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 mock +from oslo_concurrency import processutils +from oslo_config import cfg + +from ironic.common import exception +from ironic.common import wsgi_service +from ironic.tests import base + +CONF = cfg.CONF + + +class TestWSGIService(base.TestCase): + @mock.patch.object(wsgi_service.wsgi, 'Server') + def test_workers_set_default(self, mock_server): + service_name = "ironic_api" + test_service = wsgi_service.WSGIService(service_name) + self.assertEqual(processutils.get_worker_count(), + test_service.workers) + mock_server.assert_called_once_with(CONF, service_name, + test_service.app, + host='0.0.0.0', + port=6385, + use_ssl=False) + + @mock.patch.object(wsgi_service.wsgi, 'Server') + def test_workers_set_correct_setting(self, mock_server): + self.config(api_workers=8, group='api') + test_service = wsgi_service.WSGIService("ironic_api") + self.assertEqual(8, test_service.workers) + + @mock.patch.object(wsgi_service.wsgi, 'Server') + def test_workers_set_zero_setting(self, mock_server): + self.config(api_workers=0, group='api') + test_service = wsgi_service.WSGIService("ironic_api") + self.assertEqual(processutils.get_worker_count(), test_service.workers) + + @mock.patch.object(wsgi_service.wsgi, 'Server') + def test_workers_set_negative_setting(self, mock_server): + self.config(api_workers=-2, group='api') + self.assertRaises(exception.ConfigInvalid, + wsgi_service.WSGIService, + 'ironic_api') + self.assertFalse(mock_server.called) + + @mock.patch.object(wsgi_service.wsgi, 'Server') + def test_wsgi_service_with_ssl_enabled(self, mock_server): + self.config(enable_ssl_api=True, group='api') + service_name = 'ironic_api' + srv = wsgi_service.WSGIService('ironic_api', CONF.api.enable_ssl_api) + mock_server.assert_called_once_with(CONF, service_name, + srv.app, + host='0.0.0.0', + port=6385, + use_ssl=True) diff --git a/releasenotes/notes/conductor_early_import-fd29fa8b89089977.yaml b/releasenotes/notes/conductor_early_import-fd29fa8b89089977.yaml new file mode 100644 index 0000000000..a161170514 --- /dev/null +++ b/releasenotes/notes/conductor_early_import-fd29fa8b89089977.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fixes a bug which prevented the ironic-conductor service from + using the interval values from the configuration options, for + the periodic tasks. Instead, the default values had been used.