diff --git a/mistral/api/service.py b/mistral/api/service.py index 3d2aeb906..9452cbc60 100644 --- a/mistral/api/service.py +++ b/mistral/api/service.py @@ -12,33 +12,73 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_concurrency import processutils +import os +import threading + +from cheroot.ssl import builtin as cheroot_ssl +from cheroot import wsgi from oslo_config import cfg +from oslo_log import log as logging from oslo_service import service -from oslo_service import wsgi +from oslo_service import sslutils from mistral.api import app from mistral.rpc import clients as rpc_clients +LOG = logging.getLogger(__name__) + + +def validate_cert_paths(cert_file, key_file): + if cert_file and not os.path.exists(cert_file): + raise RuntimeError(_("Unable to find cert_file: %s") % cert_file) + if key_file and not os.path.exists(key_file): + raise RuntimeError(_("Unable to find key_file: %s") % key_file) + if not cert_file or not key_file: + raise RuntimeError(_("When running server in SSL mode, you must " + "specify a valid cert_file and key_file " + "paths in your configuration file")) + + class WSGIService(service.ServiceBase): """Provides ability to launch Mistral API from wsgi app.""" def __init__(self, name): self.name = name self.app = app.setup_app() - self.workers = ( - cfg.CONF.api.api_workers or processutils.get_worker_count() - ) + # NOTE(amorin) since we moved to cheroot, we can't start more than + # one process. + # If you want to use more than one worker, you should start + # mistral-wsgi-api instead + self.workers = 1 + bind_addr = (cfg.CONF.api.host, cfg.CONF.api.port) self.server = wsgi.Server( - cfg.CONF, - name, - self.app, - host=cfg.CONF.api.host, - port=cfg.CONF.api.port, - use_ssl=cfg.CONF.api.enable_ssl_api - ) + bind_addr=bind_addr, + wsgi_app=self.app, + server_name=name) + + if cfg.CONF.api.enable_ssl_api: + # NOTE(amorin) I copy pasted this from ironic code and they + # were warning about this so I kept it + LOG.warning( + "Using deprecated [ssl] group for TLS " + "credentials: the global [ssl] configuration block is " + "deprecated and will be removed in 2026.1" + ) + # Register global SSL config options and validate the + # existence of configured certificate/private key file paths, + # when in secure mode. + sslutils.is_enabled(cfg.CONF) + cert_file = cfg.CONF.ssl.cert_file + key_file = cfg.CONF.ssl.key_file + validate_cert_paths(cert_file, key_file) + self.server.ssl_adapter = cheroot_ssl.BuiltinSSLAdapter( + certificate=cert_file, + private_key=key_file, + ) + + self._thread = None def start(self): # NOTE: When oslo.service creates an API worker it forks a new child @@ -50,15 +90,25 @@ class WSGIService(service.ServiceBase): # generated queue names). rpc_clients.cleanup() - self.server.start() - - print('API server started.') + self.server.prepare() + self._thread = threading.Thread( + target=self.server.serve, + daemon=True + ) + self._thread.start() + LOG.info('API server started with one process. If you want more ' + 'workers, consider switching to a wsgi server using ' + 'mistral-wsgi-api') def stop(self): - self.server.stop() + if self.server: + self.server.stop() + if self._thread: + self._thread.join(timeout=2) def wait(self): - self.server.wait() + if self._thread: + self._thread.join() def reset(self): - self.server.reset() + pass diff --git a/mistral/cmd/__init__.py b/mistral/cmd/__init__.py index e69de29bb..132b3d81a 100644 --- a/mistral/cmd/__init__.py +++ b/mistral/cmd/__init__.py @@ -0,0 +1,19 @@ +# Copyright 2025 - OVHcloud +# +# 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. + +# NOTE(amorin) +# Hardcode the threading backend to avoid using eventlet until this will +# eventually become the default +import oslo_service.backend as service_backend +service_backend.init_backend(service_backend.BackendType.THREADING) diff --git a/mistral/cmd/launch.py b/mistral/cmd/launch.py index 0e34e322d..c83da1073 100644 --- a/mistral/cmd/launch.py +++ b/mistral/cmd/launch.py @@ -16,17 +16,6 @@ import sys -import eventlet - - -eventlet.monkey_patch( - os=True, - select=True, - socket=True, - thread=False if '--use-debugger' in sys.argv else True, - time=True) - - from oslo_config import cfg from oslo_log import log as logging from oslo_service import service @@ -82,7 +71,6 @@ def launch_notifier(): def launch_api(): server = api_service.WSGIService('mistral_api') - launch_process(server, workers=server.workers) @@ -90,12 +78,11 @@ def launch_any(options): for option in options: LAUNCH_OPTIONS[option]() - global SERVER_PROCESS_MANAGER - # Wait for the services to finish now # This main process will do nothing starting from now + global SERVER_PROCESS_MANAGER if SERVER_PROCESS_MANAGER: - SERVER_PROCESS_MANAGER.wait() + sys.exit(SERVER_PROCESS_MANAGER.wait()) # Map cli options to appropriate functions. The cli options are diff --git a/mistral/config.py b/mistral/config.py index 0915fdea9..ae0d254d7 100644 --- a/mistral/config.py +++ b/mistral/config.py @@ -27,7 +27,6 @@ from oslo_config import cfg from oslo_config import types from oslo_log import log from oslo_middleware import cors -from oslo_service import _options as service_opts from osprofiler import opts as profiler from mistral import version @@ -789,7 +788,6 @@ CONF.register_opt(scheduler_type_opt) CONF.register_opt(js_impl_opt) CONF.register_opt(oslo_rpc_executor) CONF.register_opt(expiration_token_duration) -CONF.register_opts(service_opts.service_opts) CONF.register_opts(action_providers_opts, group=ACTION_PROVIDERS_GROUP) CONF.register_opts( diff --git a/mistral/engine/engine_server.py b/mistral/engine/engine_server.py index 39cd53eb6..5e02b735d 100644 --- a/mistral/engine/engine_server.py +++ b/mistral/engine/engine_server.py @@ -92,19 +92,7 @@ class EngineServer(service_base.MistralService): self._notify_started('Engine server started.') def stop(self, graceful=False): - # NOTE(rakhmerov): Unfortunately, oslo.service doesn't pass the - # 'graceful' parameter with a correct value. It's simply ignored - # in the corresponding call chain leading to a concrete service. - # The only workaround for now is to check 'graceful_shutdown_timeout' - # configuration option. If it's not empty (not None or 0) then we - # should treat it a graceful shutdown. - graceful = bool(CONF.graceful_shutdown_timeout) - - LOG.info( - 'Stopping an engine server [graceful=%s, timeout=%s]', - graceful, - CONF.graceful_shutdown_timeout - ) + LOG.info('Stopping engine server') super(EngineServer, self).stop(graceful) diff --git a/mistral/tests/unit/__init__.py b/mistral/tests/unit/__init__.py index 654ea67fc..3968fc415 100644 --- a/mistral/tests/unit/__init__.py +++ b/mistral/tests/unit/__init__.py @@ -12,14 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. -import sys - -import eventlet - -eventlet.monkey_patch( - os=True, - select=True, - socket=True, - thread=False if '--use-debugger' in sys.argv else True, - time=True -) +# NOTE(amorin) +# Hardcode the threading backend to avoid using eventlet until this will +# eventually become the default +import oslo_service.backend as service_backend +service_backend.init_backend(service_backend.BackendType.THREADING) diff --git a/mistral/tests/unit/api/test_service.py b/mistral/tests/unit/api/test_service.py index b20df23f0..c6b1f26e9 100644 --- a/mistral/tests/unit/api/test_service.py +++ b/mistral/tests/unit/api/test_service.py @@ -16,66 +16,83 @@ from unittest import mock from oslo_concurrency import processutils from oslo_config import cfg +from oslo_service import sslutils from mistral.api import service from mistral.tests.unit import base +CONF = cfg.CONF + class TestWSGIService(base.BaseTest): def setUp(self): super(TestWSGIService, self).setUp() - self.override_config('enabled', False, group='cron_trigger') + sslutils.register_opts(CONF) + self.server = mock.Mock() + @mock.patch.object(processutils, 'get_worker_count', lambda: 2) @mock.patch.object(service.wsgi, 'Server') def test_workers_set_default(self, wsgi_server): service_name = "mistral_api" - with mock.patch('mistral.api.app.setup_app'): test_service = service.WSGIService(service_name) - + self.assertEqual(1, test_service.workers) wsgi_server.assert_called_once_with( - cfg.CONF, - service_name, - test_service.app, - host='0.0.0.0', - port=8989, - use_ssl=False + bind_addr=('0.0.0.0', 8989), + wsgi_app=test_service.app, + server_name=service_name, ) def test_workers_set_correct_setting(self): + # NOTE(amorin) since we moved to cheroot, we can't start more than + # one worker, so, no matter what the setting will be set to, + # mistral will start only one worker self.override_config('api_workers', 8, group='api') with mock.patch('mistral.api.app.setup_app'): test_service = service.WSGIService("mistral_api") - self.assertEqual(8, test_service.workers) + self.assertEqual(1, test_service.workers) + @mock.patch.object(processutils, 'get_worker_count', lambda: 3) def test_workers_set_zero_setting(self): self.override_config('api_workers', 0, group='api') with mock.patch('mistral.api.app.setup_app'): test_service = service.WSGIService("mistral_api") - self.assertEqual( - processutils.get_worker_count(), - test_service.workers - ) + self.assertEqual(1, test_service.workers) @mock.patch.object(service.wsgi, 'Server') - def test_wsgi_service_with_ssl_enabled(self, wsgi_server): + @mock.patch('mistral.api.service.cheroot_ssl.BuiltinSSLAdapter', + autospec=True) + @mock.patch('mistral.api.service.validate_cert_paths', + autospec=True) + @mock.patch('oslo_service.sslutils.is_enabled', return_value=True, + autospec=True) + def test_wsgi_service_with_ssl_enabled(self, mock_is_enabled, + mock_validate_tls, + mock_ssl_adapter, + wsgi_server): + wsgi_server.return_value = self.server self.override_config('enable_ssl_api', True, group='api') + self.override_config('cert_file', '/path/to/cert', group='ssl') + self.override_config('key_file', '/path/to/key', group='ssl') service_name = 'mistral_api' with mock.patch('mistral.api.app.setup_app'): - srv = service.WSGIService(service_name) + test_service = service.WSGIService(service_name) wsgi_server.assert_called_once_with( - cfg.CONF, - service_name, - srv.app, - host='0.0.0.0', - port=8989, - use_ssl=True + server_name=service_name, + wsgi_app=test_service.app, + bind_addr=('0.0.0.0', 8989) ) + + mock_ssl_adapter.assert_called_once_with( + certificate='/path/to/cert', + private_key='/path/to/key' + ) + self.assertIsNotNone(self.server.ssl_adapter) diff --git a/mistral/tests/unit/test_launcher.py b/mistral/tests/unit/test_launcher.py index 7b96a6a42..2acf56b62 100644 --- a/mistral/tests/unit/test_launcher.py +++ b/mistral/tests/unit/test_launcher.py @@ -34,20 +34,28 @@ class ServiceLauncherTest(base.DbTestCase): @mock.patch.object(service.ProcessLauncher, 'launch_service') @mock.patch.object(service.ProcessLauncher, 'wait') - def test_launch_one_process(self, mock_wait, mock_launch_service): + @mock.patch('sys.exit') + def test_launch_one_process(self, mock_exit, mock_wait, + mock_launch_service): # Launch API launch.launch_any(['api']) # Make sure we tried to start a service - mock_launch_service.assert_called_once_with(mock.ANY, workers=2) + # NOTE(amorin) despite the fact that the config set 2 workers + # we are hardcoding 1 worker since we use cheroot + mock_launch_service.assert_called_once_with(mock.ANY, workers=1) mock_wait.assert_called_once_with() + mock_exit.assert_called_once() @mock.patch.object(service.ProcessLauncher, 'launch_service') @mock.patch.object(service.ProcessLauncher, 'wait') - def test_launch_multiple_processes(self, mock_wait, mock_launch_service): + @mock.patch('sys.exit') + def test_launch_multiple_processes(self, mock_exit, mock_wait, + mock_launch_service): # Launch API and executor launch.launch_any(['api', 'executor']) # Make sure we tried to start 2 services mock_launch_service.call_count = 2 mock_wait.call_count = 2 + mock_exit.assert_called_once() diff --git a/releasenotes/notes/migrate-api-out-of-eventlet-cheroot-c681999fb16c27d7.yaml b/releasenotes/notes/migrate-api-out-of-eventlet-cheroot-c681999fb16c27d7.yaml new file mode 100644 index 000000000..ba82220d9 --- /dev/null +++ b/releasenotes/notes/migrate-api-out-of-eventlet-cheroot-c681999fb16c27d7.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The Mistral API is now served by ``cheroot.wsgi.Server`` instead of the + deprecated ``oslo_service.wsgi`` / eventlet stack. Behaviour and CLI + commands are unchanged. diff --git a/requirements.txt b/requirements.txt index fcf8a9667..4827e1d3b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,10 +3,10 @@ # you find any incorrect lower bounds, let us know or propose a fix. alembic>=0.9.6 # MIT +cheroot>=10.0.1 # BSD croniter>=0.3.4 # MIT License cachetools>=2.0.0 # MIT License dogpile.cache>=0.6.2 # BSD -eventlet>=0.27.0 # MIT Jinja2>=2.10 # BSD License (3 clause) jsonschema>=3.2.0 # MIT keystonemiddleware>=4.18.0 # Apache-2.0 @@ -23,7 +23,7 @@ oslo.policy>=4.5.0 # Apache-2.0 oslo.utils>=7.0.0 # Apache-2.0 oslo.log>=3.36.0 # Apache-2.0 oslo.serialization>=2.21.1 # Apache-2.0 -oslo.service>=2.1.0 # Apache-2.0 +oslo.service[threading]>=4.2.2 # Apache-2.0 osprofiler>=1.4.0 # Apache-2.0 paramiko>=2.4.1 # LGPLv2.1+ pbr!=2.1.0,>=2.0.0 # Apache-2.0