Wrap Flask into oslo.service

This patch is part of inspector HA work, which wraps inspector api into
oslo service.

oslo.service has also provided support to signal processing like SIGHUP or
SIGTERM, so these code were removed in this patch.

Deprecated current SSL cert/key options used by ironic-inspector, code
manually creates ssl context were removed. These options will be fed
from [ssl] section.

Change-Id: Ia5e16fcb9104556d62c90f5507f17b41f73a5208
Story: #2001842
Task: #12609

ironic_inspector/cmd/all.py

@@ -14,16 +14,24 @@
 
 import sys
 
+from oslo_config import cfg
+from oslo_service import service
+
+from ironic_inspector.common.rpc_service import RPCService
 from ironic_inspector.common import service_utils
 from ironic_inspector import wsgi_service
 
+CONF = cfg.CONF
+
 
 def main(args=sys.argv[1:]):
     # Parse config file and command line options, then start logging
     service_utils.prepare_service(args)
 
-    server = wsgi_service.WSGIService()
+    launcher = service.ServiceLauncher(CONF, restart_method='mutate')
-    server.run()
+    launcher.launch_service(wsgi_service.WSGIService())
+    launcher.launch_service(RPCService(CONF.host))
+    launcher.wait()
 
 
 if __name__ == '__main__':

ironic_inspector/common/service_utils.py

@@ -12,6 +12,7 @@
 
 from oslo_config import cfg
 from oslo_log import log
+from oslo_service import sslutils
 
 from ironic_inspector.conf import opts
 
@@ -26,5 +27,10 @@ def prepare_service(args=None):
     opts.parse_args(args)
     log.setup(CONF, 'ironic_inspector')
 
+    # TODO(kaifeng) Remove deprecated options at T* cycle.
+    sslutils.register_opts(CONF)
+    CONF.set_default('cert_file', CONF.ssl_cert_path, group='ssl')
+    CONF.set_default('key_file', CONF.ssl_key_path, group='ssl')
+
     LOG.debug("Configuration:")
     CONF.log_opt_values(LOG, log.DEBUG)

ironic_inspector/conf/default.py

@@ -52,9 +52,15 @@ _OPTS = [
                 help=_('SSL Enabled/Disabled')),
     cfg.StrOpt('ssl_cert_path',
                default='',
+               deprecated_for_removal=True,
+               deprecated_reason=_('This option will be superseded by '
+                                   '[ssl]cert_file.'),
                help=_('Path to SSL certificate')),
     cfg.StrOpt('ssl_key_path',
                default='',
+               deprecated_for_removal=True,
+               deprecated_reason=_('This option will be superseded by '
+                                   '[ssl]key_file.'),
                help=_('Path to SSL key')),
     cfg.IntOpt('max_concurrency',
                default=1000, min=2,
@@ -78,7 +84,7 @@ _OPTS = [
                 help=_('Whether the current installation of ironic-inspector '
                        'can manage PXE booting of nodes. If set to False, '
                        'the API will reject introspection requests with '
-                       'manage_boot missing or set to True.'))
+                       'manage_boot missing or set to True.')),
 ]
 
 

ironic_inspector/test/unit/test_wsgi_service.py

@@ -11,15 +11,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import ssl
-import sys
-import unittest
-
 import eventlet  # noqa
 import fixtures
 import mock
 from oslo_config import cfg
 
+from ironic_inspector.common import service_utils
 from ironic_inspector.test import base as test_base
 from ironic_inspector import wsgi_service
 
@@ -32,9 +29,12 @@ class BaseWSGITest(test_base.BaseTest):
         super(BaseWSGITest, self).setUp()
         self.app = self.useFixture(fixtures.MockPatchObject(
             wsgi_service.app, 'app', autospec=True)).mock
+        self.server = self.useFixture(fixtures.MockPatchObject(
+            wsgi_service.wsgi, 'Server', autospec=True)).mock
         self.mock_log = self.useFixture(fixtures.MockPatchObject(
             wsgi_service, 'LOG')).mock
         self.service = wsgi_service.WSGIService()
+        self.service.server = self.server
 
 
 class TestWSGIServiceInitMiddleware(BaseWSGITest):
@@ -66,174 +66,55 @@ class TestWSGIServiceInitMiddleware(BaseWSGITest):
         self.mock_add_cors_middleware.assert_called_once_with(self.app)
 
 
-class TestWSGIServiceRun(BaseWSGITest):
+class TestWSGIService(BaseWSGITest):
     def setUp(self):
-        super(TestWSGIServiceRun, self).setUp()
+        super(TestWSGIService, self).setUp()
         self.mock__init_middleware = self.useFixture(fixtures.MockPatchObject(
             self.service, '_init_middleware')).mock
-        self.mock__create_ssl_context = self.useFixture(
-            fixtures.MockPatchObject(self.service, '_create_ssl_context')).mock
-        self.mock_shutdown = self.useFixture(fixtures.MockPatchObject(
-            self.service, 'shutdown')).mock
 
         # 'positive' settings
         CONF.set_override('listen_address', '42.42.42.42')
         CONF.set_override('listen_port', 42)
 
-    def test_run(self):
+    def test_start(self):
-        self.service.run()
+        self.service.start()
 
-        self.mock__create_ssl_context.assert_called_once_with()
         self.mock__init_middleware.assert_called_once_with()
-        self.app.run.assert_called_once_with(
+        self.server.start.assert_called_once_with()
-            host=CONF.listen_address, port=CONF.listen_port,
-            ssl_context=self.mock__create_ssl_context.return_value)
-        self.mock_shutdown.assert_called_once_with()
 
-    def test_run_no_ssl_context(self):
+    def test_stop(self):
-        self.mock__create_ssl_context.return_value = None
+        self.service.stop()
+        self.server.stop.assert_called_once_with()
 
-        self.service.run()
+    def test_wait(self):
-        self.mock__create_ssl_context.assert_called_once_with()
+        self.service.wait()
-        self.mock__init_middleware.assert_called_once_with()
+        self.server.wait.assert_called_once_with()
-        self.app.run.assert_called_once_with(
-            host=CONF.listen_address, port=CONF.listen_port)
-        self.mock_shutdown.assert_called_once_with()
 
-    def test_run_app_error(self):
+    def test_reset(self):
-        class MyError(Exception):
+        self.service.reset()
-            pass
+        self.server.reset.assert_called_once_with()
-
-        error = MyError('Oops!')
-        self.app.run.side_effect = error
-        self.service.run()
-
-        self.mock__create_ssl_context.assert_called_once_with()
-        self.mock__init_middleware.assert_called_once_with()
-        self.app.run.assert_called_once_with(
-            host=CONF.listen_address, port=CONF.listen_port,
-            ssl_context=self.mock__create_ssl_context.return_value)
-        self.mock_shutdown.assert_called_once_with(error=str(error))
 
 
-class TestWSGIServiceShutdown(BaseWSGITest):
+@mock.patch.object(service_utils.log, 'register_options', autospec=True)
-    def setUp(self):
+class TestSSLOptions(test_base.BaseTest):
-        super(TestWSGIServiceShutdown, self).setUp()
-        self.service = wsgi_service.WSGIService()
-        self.mock_rpc_service = mock.MagicMock()
-        self.service.rpc_service = self.mock_rpc_service
-        self.mock_exit = self.useFixture(fixtures.MockPatchObject(
-            wsgi_service.sys, 'exit')).mock
 
-    def test_shutdown(self):
+    def test_use_deprecated_options(self, mock_log):
-        class MyError(Exception):
+        CONF.set_override('ssl_cert_path', 'fake_cert_file')
-            pass
+        CONF.set_override('ssl_key_path', 'fake_key_file')
-        error = MyError('Oops!')
 
-        self.service.shutdown(error=error)
+        service_utils.prepare_service()
-        self.mock_rpc_service.stop.assert_called_once_with()
-        self.mock_exit.assert_called_once_with(error)
 
+        self.assertEqual(CONF.ssl.cert_file, 'fake_cert_file')
+        self.assertEqual(CONF.ssl.key_file, 'fake_key_file')
 
-class TestCreateSSLContext(test_base.BaseTest):
+    def test_use_ssl_options(self, mock_log):
-    def setUp(self):
+        CONF.set_override('ssl_cert_path', 'fake_cert_file')
-        super(TestCreateSSLContext, self).setUp()
+        CONF.set_override('ssl_key_path', 'fake_key_file')
-        self.app = mock.Mock()
-        self.service = wsgi_service.WSGIService()
 
-    def test_use_ssl_false(self):
+        service_utils.prepare_service()
-        CONF.set_override('use_ssl', False)
-        con = self.service._create_ssl_context()
-        self.assertIsNone(con)
 
-    @mock.patch.object(sys, 'version_info')
+        CONF.set_override('cert_file', 'fake_new_cert', 'ssl')
-    def test_old_python_returns_none(self, mock_version_info):
+        CONF.set_override('key_file', 'fake_new_key', 'ssl')
-        mock_version_info.__lt__.return_value = True
-        CONF.set_override('use_ssl', True)
-        con = self.service._create_ssl_context()
-        self.assertIsNone(con)
 
-    @unittest.skipIf(sys.version_info[:3] < (2, 7, 9),
+        self.assertEqual(CONF.ssl.cert_file, 'fake_new_cert')
-                     'This feature is unsupported in this version of python '
+        self.assertEqual(CONF.ssl.key_file, 'fake_new_key')
-                     'so the tests will be skipped')
-    @mock.patch.object(ssl, 'create_default_context', autospec=True)
-    def test_use_ssl_true(self, mock_cdc):
-        CONF.set_override('use_ssl', True)
-        m_con = mock_cdc()
-        con = self.service._create_ssl_context()
-        self.assertEqual(m_con, con)
-
-    @unittest.skipIf(sys.version_info[:3] < (2, 7, 9),
-                     'This feature is unsupported in this version of python '
-                     'so the tests will be skipped')
-    @mock.patch.object(ssl, 'create_default_context', autospec=True)
-    def test_only_key_path_provided(self, mock_cdc):
-        CONF.set_override('use_ssl', True)
-        CONF.set_override('ssl_key_path', '/some/fake/path')
-        mock_context = mock_cdc()
-        con = self.service._create_ssl_context()
-        self.assertEqual(mock_context, con)
-        self.assertFalse(mock_context.load_cert_chain.called)
-
-    @unittest.skipIf(sys.version_info[:3] < (2, 7, 9),
-                     'This feature is unsupported in this version of python '
-                     'so the tests will be skipped')
-    @mock.patch.object(ssl, 'create_default_context', autospec=True)
-    def test_only_cert_path_provided(self, mock_cdc):
-        CONF.set_override('use_ssl', True)
-        CONF.set_override('ssl_cert_path', '/some/fake/path')
-        mock_context = mock_cdc()
-        con = self.service._create_ssl_context()
-        self.assertEqual(mock_context, con)
-        self.assertFalse(mock_context.load_cert_chain.called)
-
-    @unittest.skipIf(sys.version_info[:3] < (2, 7, 9),
-                     'This feature is unsupported in this version of python '
-                     'so the tests will be skipped')
-    @mock.patch.object(ssl, 'create_default_context', autospec=True)
-    def test_both_paths_provided(self, mock_cdc):
-        key_path = '/some/fake/path/key'
-        cert_path = '/some/fake/path/cert'
-        CONF.set_override('use_ssl', True)
-        CONF.set_override('ssl_key_path', key_path)
-        CONF.set_override('ssl_cert_path', cert_path)
-        mock_context = mock_cdc()
-        con = self.service._create_ssl_context()
-        self.assertEqual(mock_context, con)
-        mock_context.load_cert_chain.assert_called_once_with(cert_path,
-                                                             key_path)
-
-    @unittest.skipIf(sys.version_info[:3] < (2, 7, 9),
-                     'This feature is unsupported in this version of python '
-                     'so the tests will be skipped')
-    @mock.patch.object(ssl, 'create_default_context', autospec=True)
-    def test_load_cert_chain_fails(self, mock_cdc):
-        CONF.set_override('use_ssl', True)
-        key_path = '/some/fake/path/key'
-        cert_path = '/some/fake/path/cert'
-        CONF.set_override('use_ssl', True)
-        CONF.set_override('ssl_key_path', key_path)
-        CONF.set_override('ssl_cert_path', cert_path)
-        mock_context = mock_cdc()
-        mock_context.load_cert_chain.side_effect = IOError('Boom!')
-        con = self.service._create_ssl_context()
-        self.assertEqual(mock_context, con)
-        mock_context.load_cert_chain.assert_called_once_with(cert_path,
-                                                             key_path)
-
-
-class TestWSGIServiceOnSigHup(BaseWSGITest):
-    def setUp(self):
-        super(TestWSGIServiceOnSigHup, self).setUp()
-        self.mock_spawn = self.useFixture(fixtures.MockPatchObject(
-            wsgi_service.eventlet, 'spawn')).mock
-        self.mock_mutate_conf = self.useFixture(fixtures.MockPatchObject(
-            wsgi_service.CONF, 'mutate_config_files')).mock
-
-    def test_on_sighup(self):
-        self.service._handle_sighup()
-        self.mock_spawn.assert_called_once_with(self.service._handle_sighup_bg)
-
-    def test_on_sighup_bg(self):
-        self.service._handle_sighup_bg()
-        self.mock_mutate_conf.assert_called_once_with()

ironic_inspector/wsgi_service.py

@@ -10,16 +10,11 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import signal
-import ssl
-import sys
-
-import eventlet
 from oslo_config import cfg
 from oslo_log import log
 from oslo_service import service
+from oslo_service import wsgi
 
-from ironic_inspector.common.rpc_service import RPCService
 from ironic_inspector import main as app
 from ironic_inspector import utils
 
@@ -27,21 +22,22 @@ LOG = log.getLogger(__name__)
 CONF = cfg.CONF
 
 
-class WSGIService(object):
+class WSGIService(service.Service):
     """Provides ability to launch API from wsgi app."""
 
     def __init__(self):
         self.app = app.app
-        signal.signal(signal.SIGHUP, self._handle_sighup)
+        self.server = wsgi.Server(CONF, 'ironic_inspector',
-        signal.signal(signal.SIGTERM, self._handle_sigterm)
+                                  self.app,
-        self.rpc_service = RPCService(CONF.host)
+                                  host=CONF.listen_address,
+                                  port=CONF.listen_port,
+                                  use_ssl=CONF.use_ssl)
 
     def _init_middleware(self):
         """Initialize WSGI middleware.
 
         :returns: None
         """
-
         if CONF.auth_strategy != 'noauth':
             utils.add_auth_middleware(self.app)
         else:
@@ -49,80 +45,31 @@ class WSGIService(object):
                         ' configuration')
         utils.add_cors_middleware(self.app)
 
-    def _create_ssl_context(self):
+    def start(self):
-        if not CONF.use_ssl:
+        """Start serving this service using loaded configuration.
-            return
-
-        MIN_VERSION = (2, 7, 9)
-
-        if sys.version_info < MIN_VERSION:
-            LOG.warning(('Unable to use SSL in this version of Python: '
-                         '%(current)s, please ensure your version of Python '
-                         'is greater than %(min)s to enable this feature.'),
-                        {'current': '.'.join(map(str, sys.version_info[:3])),
-                         'min': '.'.join(map(str, MIN_VERSION))})
-            return
-
-        context = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH)
-        if CONF.ssl_cert_path and CONF.ssl_key_path:
-            try:
-                context.load_cert_chain(CONF.ssl_cert_path, CONF.ssl_key_path)
-            except IOError as exc:
-                LOG.warning('Failed to load certificate or key from defined '
-                            'locations: %(cert)s and %(key)s, will continue '
-                            'to run with the default settings: %(exc)s',
-                            {'cert': CONF.ssl_cert_path,
-                             'key': CONF.ssl_key_path,
-                             'exc': exc})
-            except ssl.SSLError as exc:
-                LOG.warning('There was a problem with the loaded certificate '
-                            'and key, will continue to run with the default '
-                            'settings: %s', exc)
-        return context
-
-    def shutdown(self, error=None):
-        """Stop serving API.
 
         :returns: None
         """
-        LOG.debug('Shutting down')
-        self.rpc_service.stop()
-        sys.exit(error)
-
-    def run(self):
-        """Start serving this service using loaded application.
-
-        :returns: None
-        """
-        app_kwargs = {'host': CONF.listen_address,
-                      'port': CONF.listen_port}
-
-        context = self._create_ssl_context()
-        if context:
-            app_kwargs['ssl_context'] = context
-
         self._init_middleware()
+        self.server.start()
 
-        LOG.info('Spawning RPC service')
+    def stop(self):
-        service.launch(CONF, self.rpc_service,
+        """Stop serving this API.
-                       restart_method='mutate')
 
-        try:
+        :returns: None
-            self.app.run(**app_kwargs)
+        """
-        except Exception as e:
+        self.server.stop()
-            self.shutdown(error=str(e))
-        else:
-            self.shutdown()
 
-    def _handle_sighup_bg(self, *args):
+    def wait(self):
-        """Reload config on SIGHUP."""
+        """Wait for the service to stop serving this API.
-        CONF.mutate_config_files()
 
-    def _handle_sighup(self, *args):
+        :returns: None
-        eventlet.spawn(self._handle_sighup_bg, *args)
+        """
+        self.server.wait()
 
-    def _handle_sigterm(self, *args):
+    def reset(self):
-        # This is a workaround to ensure that shutdown() is done when recieving
+        """Reset server greenpool size to default.
-        # SIGTERM. Raising KeyboardIntrerrupt which won't be caught by any
+
-        # 'except Exception' clauses.
+        :returns: None
-        raise KeyboardInterrupt
+        """
+        self.server.reset()

lower-constraints.txt

@@ -73,7 +73,7 @@ oslo.middleware==3.31.0
 oslo.policy==1.30.0
 oslo.rootwrap==5.8.0
 oslo.serialization==2.18.0
-oslo.service==1.30.0
+oslo.service==1.24.0
 oslo.utils==3.33.0
 oslotest==3.2.0
 packaging==17.1

releasenotes/notes/deprecate-ssl-opts-40ce8f4618c786ef.yaml

@@ -0,0 +1,7 @@
+---
+deprecations:
+  - |
+    Configuration options ``[DEFAULT]ssl_cert_path`` and
+    ``[DEFAULT]ssl_key_path`` are deprecated for ironic-inspector now uses
+    oslo.service as underlying HTTP service instead of Werkzeug. Please use
+    ``[ssl]cert_file`` and ``[ssl]key_file``.

requirements.txt

@@ -29,6 +29,7 @@ oslo.middleware>=3.31.0 # Apache-2.0
 oslo.policy>=1.30.0 # Apache-2.0
 oslo.rootwrap>=5.8.0 # Apache-2.0
 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0
+oslo.service!=1.28.1,>=1.24.0 # Apache-2.0
 oslo.utils>=3.33.0 # Apache-2.0
 retrying!=1.3.0,>=1.2.3 # Apache-2.0
 six>=1.10.0 # MIT

tools/config-generator.conf

@@ -4,6 +4,9 @@ namespace = ironic_inspector
 namespace = keystonemiddleware.auth_token
 namespace = oslo.db
 namespace = oslo.log
+namespace = oslo.messaging
 namespace = oslo.middleware.cors
 namespace = oslo.policy
-namespace = oslo.messaging
+namespace = oslo.service.service
+namespace = oslo.service.sslutils
+namespace = oslo.service.wsgi