diff --git a/collectoss/api/gunicorn_conf.py b/collectoss/api/gunicorn_conf.py index ee7797471..69bc811b3 100644 --- a/collectoss/api/gunicorn_conf.py +++ b/collectoss/api/gunicorn_conf.py @@ -42,11 +42,11 @@ logs_directory = get_value('Logging', 'logs_directory') # this syntax satisfies the type checker -is_docker = SystemEnv.get_bool("AUGUR_DOCKER_DEPLOY", False) +is_docker = SystemEnv.get_bool("COLLECTOSS_DOCKER_DEPLOY", False) accesslog = f"{logs_directory}/gunicorn.log" errorlog = f"{logs_directory}/gunicorn.log" -# If deploying via docker, include gunicorn error logs in the docker log stream by sending it to stdout +# If deploying via Docker, include Gunicorn error logs in the Docker log stream. if is_docker: errorlog = '-' @@ -69,4 +69,3 @@ def worker_exit(server, worker): print("Stopping gunicorn worker process") dispose_database_engine() - diff --git a/collectoss/application/cli/_gunicorn.py b/collectoss/application/cli/_gunicorn.py new file mode 100644 index 000000000..0f06b4ab7 --- /dev/null +++ b/collectoss/application/cli/_gunicorn.py @@ -0,0 +1,46 @@ +# SPDX-License-Identifier: MIT +""" +Helpers for starting Gunicorn from CollectOSS CLI commands. +""" +from __future__ import annotations + +import os +from typing import Optional + +from collectoss.application.environment import SystemEnv + + +def is_docker_deploy() -> bool: + return SystemEnv.get_bool("COLLECTOSS_DOCKER_DEPLOY", False) + + +def build_gunicorn_command( + gunicorn_config: str, + host: str, + port: str, + app: str = "collectoss.api.server:app", + log_file: Optional[str] = None, + docker_deploy: Optional[bool] = None, +) -> list[str]: + """ + Build a Gunicorn command while preserving Docker stderr logging. + + In Docker, Gunicorn's config sends error logs to "-" so failures are visible + through container logs. Passing --log-file would override that setting. + """ + command = [ + "gunicorn", + "-c", + gunicorn_config, + "-b", + f"{host}:{port}", + app, + ] + + if docker_deploy is None: + docker_deploy = is_docker_deploy() + + if log_file and not docker_deploy: + command.extend(["--log-file", os.fspath(log_file)]) + + return command diff --git a/collectoss/application/cli/api.py b/collectoss/application/cli/api.py index 0c567c590..6b43867e7 100644 --- a/collectoss/application/cli/api.py +++ b/collectoss/application/cli/api.py @@ -16,6 +16,7 @@ from collectoss.application.logs import SystemLogger from collectoss.application.cli import test_connection, test_db_connection, with_database, DatabaseContext from collectoss.application.cli._cli_util import _broadcast_signal_to_processes, raise_open_file_limit, clear_redis_caches, clear_rabbitmq_messages +from collectoss.application.cli._gunicorn import build_gunicorn_command from collectoss.application.db.lib import get_value from collectoss.application.environment import SystemEnv @@ -61,8 +62,8 @@ def start(ctx, development, port): if not port: port = get_value("Server", "port") - gunicorn_command = f"gunicorn -c {gunicorn_location} -b {host}:{port} collectoss.api.server:app --log-file gunicorn.log" - server = subprocess.Popen(gunicorn_command.split(" ")) + gunicorn_command = build_gunicorn_command(gunicorn_location, host, str(port), log_file="gunicorn.log") + server = subprocess.Popen(gunicorn_command) time.sleep(3) logger.info('Gunicorn webserver started...') @@ -154,5 +155,3 @@ def is_api_process(process): return True return False - - diff --git a/collectoss/application/cli/backend.py b/collectoss/application/cli/backend.py index 3526a3c2c..b60d02586 100644 --- a/collectoss/application/cli/backend.py +++ b/collectoss/application/cli/backend.py @@ -28,6 +28,7 @@ from collectoss.application.service_manager import SystemServiceManager, cleanup_collection_status_and_rabbit, clean_collection_status from collectoss.application.db.lib import get_value from collectoss.application.cli import test_connection, test_db_connection, with_database, DatabaseContext +from collectoss.application.cli._gunicorn import build_gunicorn_command import sqlalchemy as s from keyman.KeyClient import KeyClient, KeyPublisher @@ -104,8 +105,8 @@ def start(ctx, disable_collection, development, pidfile, port): log_dir = get_value("Logging", "logs_directory") or "." gunicorn_log_file = os.path.join(log_dir, "gunicorn.log") - gunicorn_command = f"gunicorn -c {gunicorn_location} -b {host}:{port} collectoss.api.server:app --log-file {gunicorn_log_file}" - server = subprocess.Popen(gunicorn_command.split(" ")) + gunicorn_command = build_gunicorn_command(gunicorn_location, host, str(port), log_file=gunicorn_log_file) + server = subprocess.Popen(gunicorn_command) manager.server = server logger.info("awaiting Gunicorn start") diff --git a/tests/test_application/test_cli/test_gunicorn_command.py b/tests/test_application/test_cli/test_gunicorn_command.py new file mode 100644 index 000000000..f8b978c14 --- /dev/null +++ b/tests/test_application/test_cli/test_gunicorn_command.py @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: MIT +"""Tests for Gunicorn CLI command construction.""" + +from collectoss.application.cli._gunicorn import build_gunicorn_command + + +def test_gunicorn_command_uses_log_file_outside_docker(): + command = build_gunicorn_command( + "/collectoss/collectoss/api/gunicorn_conf.py", + "0.0.0.0", + "5000", + log_file="/collectoss/logs/gunicorn.log", + docker_deploy=False, + ) + + assert command == [ + "gunicorn", + "-c", + "/collectoss/collectoss/api/gunicorn_conf.py", + "-b", + "0.0.0.0:5000", + "collectoss.api.server:app", + "--log-file", + "/collectoss/logs/gunicorn.log", + ] + + +def test_gunicorn_command_does_not_override_docker_errorlog(): + command = build_gunicorn_command( + "/collectoss/collectoss/api/gunicorn_conf.py", + "0.0.0.0", + "5000", + log_file="/collectoss/logs/gunicorn.log", + docker_deploy=True, + ) + + assert "--log-file" not in command + assert command == [ + "gunicorn", + "-c", + "/collectoss/collectoss/api/gunicorn_conf.py", + "-b", + "0.0.0.0:5000", + "collectoss.api.server:app", + ]