From c4bd20ebd4b42201bf40e00d4f0a1b2c69d2e931 Mon Sep 17 00:00:00 2001 From: Anton Arefiev Date: Tue, 1 Sep 2015 17:51:37 +0300 Subject: [PATCH] Use wsgi from oslo.service for Ironic API oslo.service provides a wsgi functionality for defining new long-running services using by OpenStack applications. It might usefull for working with SSL for example. Ironic has started migrate to oslo.service, this change continue this work, and consumes wsgi from it. Change-Id: Ic7865709cd87c45e6b7d49f15ce73354aa15401e Closes-Bug: #1484044 --- etc/ironic/ironic.conf.sample | 6 ++ ironic/api/__init__.py | 5 ++ ironic/cmd/api.py | 34 ++---------- ironic/common/service.py | 70 ++++++++++++++++++++++-- ironic/tests/unit/common/test_service.py | 62 +++++++++++++++++++++ 5 files changed, 142 insertions(+), 35 deletions(-) create mode 100644 ironic/tests/unit/common/test_service.py diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 07c34dbb51..c2d0b93e63 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -498,6 +498,12 @@ # (string value) #public_endpoint= +# Number of workers for OpenStack Ironic API service. The +# default is equal to the number of CPUs available if that can +# be determined, else a default worker count of 1 is returned. +# (integer value) +#api_workers= + [cimc] diff --git a/ironic/api/__init__.py b/ironic/api/__init__.py index dfade053c3..aef280261a 100644 --- a/ironic/api/__init__.py +++ b/ironic/api/__init__.py @@ -36,6 +36,11 @@ API_SERVICE_OPTS = [ "host URL. If the API is operating behind a proxy, you " "will want to change this to represent the proxy's URL. " "Defaults to None.")), + cfg.IntOpt('api_workers', + help=_('Number of workers for OpenStack Ironic API service. ' + 'The default is equal to the number of CPUs available ' + 'if that can be determined, else a default worker ' + 'count of 1 is returned.')), ] CONF = cfg.CONF diff --git a/ironic/cmd/api.py b/ironic/cmd/api.py index 337919fdcd..a942ca2d3a 100644 --- a/ironic/cmd/api.py +++ b/ironic/cmd/api.py @@ -17,28 +17,16 @@ """The Ironic Service API.""" -import logging import sys -from wsgiref import simple_server from oslo_config import cfg -from oslo_log import log -from six.moves import socketserver -from ironic.api import app -from ironic.common.i18n import _LI from ironic.common import service as ironic_service from ironic.objects import base CONF = cfg.CONF -class ThreadedSimpleServer(socketserver.ThreadingMixIn, - simple_server.WSGIServer): - """A Mixin class to make the API service greenthread-able.""" - pass - - def main(): # Parse config file and command line options, then start logging ironic_service.prepare_service(sys.argv) @@ -47,24 +35,10 @@ def main(): base.IronicObject.indirection_api = base.IronicObjectIndirectionAPI() # Build and start the WSGI app - host = CONF.api.host_ip - port = CONF.api.port - wsgi = simple_server.make_server( - host, port, - app.VersionSelectorApplication(), - server_class=ThreadedSimpleServer) - - LOG = log.getLogger(__name__) - LOG.info(_LI("Serving on http://%(host)s:%(port)s"), - {'host': host, 'port': port}) - LOG.debug("Configuration:") - CONF.log_opt_values(LOG, logging.DEBUG) - - try: - wsgi.serve_forever() - except KeyboardInterrupt: - pass - + launcher = ironic_service.process_launcher() + server = ironic_service.WSGIService('ironic_api') + launcher.launch_service(server, workers=server.workers) + launcher.wait() if __name__ == '__main__': sys.exit(main()) diff --git a/ironic/common/service.py b/ironic/common/service.py index c12980a835..982a8d3a35 100644 --- a/ironic/common/service.py +++ b/ironic/common/service.py @@ -17,14 +17,18 @@ import signal import socket +from oslo_concurrency import processutils from oslo_config import cfg from oslo_context import context 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 exception from ironic.common.i18n import _ from ironic.common.i18n import _LE from ironic.common.i18n import _LI @@ -45,10 +49,11 @@ service_opts = [ 'hostname, FQDN, or IP address.')), ] -cfg.CONF.register_opts(service_opts) - +CONF = cfg.CONF LOG = log.getLogger(__name__) +CONF.register_opts(service_opts) + class RPCService(service.Service): @@ -76,7 +81,7 @@ class RPCService(service.Service): self.manager.init_host() self.tg.add_dynamic_timer( self.manager.periodic_tasks, - periodic_interval_max=cfg.CONF.periodic_interval, + periodic_interval_max=CONF.periodic_interval, context=admin_context) LOG.info(_LI('Created RPC server for service %(service)s on host ' @@ -117,7 +122,7 @@ class RPCService(service.Service): def prepare_service(argv=[]): - log.register_options(cfg.CONF) + log.register_options(CONF) log.set_defaults(default_log_levels=['amqp=WARNING', 'amqplib=WARNING', 'qpid.messaging=INFO', @@ -135,4 +140,59 @@ def prepare_service(argv=[]): 'urllib3.connectionpool=WARNING', ]) config.parse_args(argv) - log.setup(cfg.CONF, 'ironic') + log.setup(CONF, 'ironic') + + +def process_launcher(): + return service.ProcessLauncher(CONF) + + +class WSGIService(service.ServiceBase): + """Provides ability to launch ironic API from wsgi app.""" + + def __init__(self, name): + """Initialize, but do not start the WSGI server. + + :param name: The name of the WSGI server given to the loader. + :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) + + 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_service.py b/ironic/tests/unit/common/test_service.py new file mode 100644 index 0000000000..b7ae29fa36 --- /dev/null +++ b/ironic/tests/unit/common/test_service.py @@ -0,0 +1,62 @@ +# 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 ironic.common import exception +from ironic.common import service +from ironic.tests import base + + +class TestWSGIService(base.TestCase): + + @mock.patch.object(service.app, 'VersionSelectorApplication') + def test_reset_pool_size_to_default(self, mock_app): + test_service = service.WSGIService("test_service") + test_service.start() + + # Stopping the service, which in turn sets pool size to 0 + test_service.stop() + self.assertEqual(0, test_service.server._pool.size) + + # Resetting pool size to default + test_service.reset() + test_service.start() + self.assertTrue(test_service.server._pool.size > 0) + self.assertTrue(mock_app.called) + + @mock.patch.object(service.wsgi, 'Server') + def test_workers_set_default(self, wsgi_server): + 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_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)