diff --git a/.github/workflows/run-unit-tests.yaml b/.github/workflows/run-unit-tests.yaml index 5fd749d..379a37d 100644 --- a/.github/workflows/run-unit-tests.yaml +++ b/.github/workflows/run-unit-tests.yaml @@ -26,8 +26,6 @@ jobs: FLIT_ROOT_INSTALL: 1 - name: Run Unit tests run: pytest --cov-branch --cov=lso --cov-report=xml - env: - SETTINGS_FILENAME: dummy.json - name: "Upload coverage to Codecov" uses: codecov/codecov-action@v3 with: diff --git a/README.md b/README.md index 27cc646..de42d86 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,6 @@ services: lso: image: my-lso:latest environment: - SETTINGS_FILENAME: /app/config.json ANSIBLE_ROLES_PATH: /app/lso/ansible_roles volumes: - "/home/user/config.json:/app/config.json:ro" @@ -80,5 +79,5 @@ As an alternative, below are a set of instructions for installing and running LS * Run the app like this (`app.py` starts the server on port 44444): ```bash - SETTINGS_FILENAME=/absolute/path/to/config.json python -m lso.app + python -m lso.app ``` diff --git a/config.json.example b/config.json.example deleted file mode 100644 index 6a599bd..0000000 --- a/config.json.example +++ /dev/null @@ -1,3 +0,0 @@ -{ - "ansible_playbooks_root_dir": "/app/gap/ansible/ansible_collections/geant/gap_ansible/playbooks" -} diff --git a/env.example b/env.example new file mode 100644 index 0000000..2418a9d --- /dev/null +++ b/env.example @@ -0,0 +1,24 @@ +# Environment configuration for LSO application + +# General settings +LSO_ENV_PREFIX=LSO_ + +# Ansible configuration +LSO_ANSIBLE_PLAYBOOKS_ROOT_DIR=/path/to/ansible/playbooks + +# Executor configuration +LSO_EXECUTOR=threadpool # Options: "threadpool", "celery" +LSO_MAX_THREAD_POOL_WORKERS=10 + +# Request settings +LSO_DEFAULT_REQUEST_TIMEOUT_SEC=10 + +# Celery configuration +LSO_CELERY_BROKER_URL=redis://localhost:6379/0 +LSO_CELERY_RESULT_BACKEND=redis://localhost:6379/0 +LSO_CELERY_TIMEZONE=Europe/Amsterdam +LSO_CELERY_ENABLE_UTC=True +LSO_CELERY_RESULT_EXPIRES=3600 + +# Debug/Testing +LSO_TESTING=False diff --git a/lso/__init__.py b/lso/__init__.py index e8af032..d782180 100644 --- a/lso/__init__.py +++ b/lso/__init__.py @@ -20,16 +20,13 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware -from lso import config, environment +from lso import environment from lso.routes.default import router as default_router from lso.routes.playbook import router as playbook_router def create_app() -> FastAPI: - """Override default settings with those found in the file read from environment variable `SETTINGS_FILENAME`. - - :return: a new flask app instance - """ + """Initialise the :term:`LSO` app.""" app = FastAPI() app.add_middleware( @@ -39,9 +36,6 @@ def create_app() -> FastAPI: app.include_router(default_router, prefix="/api") app.include_router(playbook_router, prefix="/api/playbook") - # test that configuration parameters are loaded and available - config.load() - environment.setup_logging() logging.info("FastAPI app initialized") diff --git a/lso/config.py b/lso/config.py index a48d3c6..7bce523 100644 --- a/lso/config.py +++ b/lso/config.py @@ -11,60 +11,45 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""A module for loading configuration data, including a configuration schema that data is validated against. +"""Module for loading and managing configuration settings for the LSO app. -Data is loaded from a file, the location of which may be specified when using :func:`load_from_file`. -Configuration file location can also be loaded from environment variable ``$SETTINGS_FILENAME``, which is default - behaviour in :func:`load`. +Uses `pydantic`'s `BaseSettings` to load settings from environment variables. """ -import json +import logging import os -from pathlib import Path +from enum import Enum -import jsonschema -from pydantic import BaseModel +from pydantic_settings import BaseSettings -CONFIG_SCHEMA = { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": {"ansible_playbooks_root_dir": {"type": "string"}}, - "required": ["ansible_playbooks_root_dir"], - "additionalProperties": False, -} -DEFAULT_REQUEST_TIMEOUT = 10 +logger = logging.getLogger(__name__) -class Config(BaseModel): - """Simple Configuration class. +class ExecutorType(Enum): + """Enum representing the types of executors available for task execution.""" - Contains the root directory at which Ansible playbooks are present. - """ + WORKER = "celery" + THREADPOOL = "threadpool" - ansible_playbooks_root_dir: str +class Config(BaseSettings): + """The set of parameters required for running :term:`LSO`.""" -def load_from_file(file: Path) -> Config: - """Load, validate and return configuration parameters. + TESTING: bool = False + ANSIBLE_PLAYBOOKS_ROOT_DIR: str = "/path/to/ansible/playbooks" + EXECUTOR: ExecutorType = ExecutorType.THREADPOOL + MAX_THREAD_POOL_WORKERS: int = min(32, (os.cpu_count() or 1) + 4) + DEFAULT_REQUEST_TIMEOUT_SEC: int = 10 + CELERY_BROKER_URL: str = "redis://localhost:6379/0" + CELERY_RESULT_BACKEND: str = "redis://localhost:6379/0" + CELERY_TIMEZONE: str = "Europe/Amsterdam" + CELERY_ENABLE_UTC: bool = True + CELERY_RESULT_EXPIRES: int = 3600 - Input is validated against this JSON schema: + class Config: + """Sets the prefix for environment variables used in the main Config class.""" - .. asjson:: lso.config.CONFIG_SCHEMA + env_prefix = os.environ.get("LSO_ENV_PREFIX", "LSO_") - :param file: :class:`Path` object that produces the configuration file. - :return: a dict containing the parsed configuration parameters. - """ - config = json.loads(file.read_text()) - jsonschema.validate(config, CONFIG_SCHEMA) - return Config(**config) - -def load() -> Config: - """Load a configuration file, located at the path specified in the environment variable ``$SETTINGS_FILENAME``. - - Loading and validating the file is performed by :func:`load_from_file`. - - :return: a dict containing the parsed configuration parameters - """ - assert "SETTINGS_FILENAME" in os.environ, "Environment variable SETTINGS_FILENAME not set" # noqa: S101 - return load_from_file(Path(os.environ["SETTINGS_FILENAME"])) +settings = Config() diff --git a/lso/playbook.py b/lso/playbook.py index 6e02251..e0f2930 100644 --- a/lso/playbook.py +++ b/lso/playbook.py @@ -14,27 +14,42 @@ """Module that gathers common API responses and data models.""" import logging -import threading import uuid +from concurrent.futures import ThreadPoolExecutor from pathlib import Path from typing import Any import ansible_runner import requests +from celery import shared_task from fastapi import status from fastapi.responses import JSONResponse from pydantic import HttpUrl -from lso import config -from lso.config import DEFAULT_REQUEST_TIMEOUT +from lso.config import ExecutorType, settings logger = logging.getLogger(__name__) +_lso_executor = None + + +def get_thread_pool() -> ThreadPoolExecutor: + """Get and optionally initialise a ThreadPoolExecutor. + + Returns: + ThreadPoolExecutor + + """ + global _lso_executor # noqa: PLW0603 + if _lso_executor is None: + _lso_executor = ThreadPoolExecutor(max_workers=settings.MAX_THREAD_POOL_WORKERS) + + return _lso_executor + def get_playbook_path(playbook_name: str) -> Path: """Get the path of a playbook on the local filesystem.""" - config_params = config.load() - return Path(config_params.ansible_playbooks_root_dir) / playbook_name + return Path(settings.ANSIBLE_PLAYBOOKS_ROOT_DIR) / playbook_name def playbook_launch_success(job_id: str) -> JSONResponse: @@ -57,7 +72,7 @@ def playbook_launch_error(reason: str, status_code: int = status.HTTP_400_BAD_RE def _run_playbook_proc( - job_id: str, playbook_path: str, extra_vars: dict, inventory: dict[str, Any] | str, callback: str + job_id: str, playbook_path: str, extra_vars: dict, inventory: dict[str, Any] | str, callback: HttpUrl ) -> None: """Run a playbook, internal function. @@ -76,12 +91,28 @@ def _run_playbook_proc( "return_code": int(ansible_playbook_run.rc), } - request_result = requests.post(callback, json=payload, timeout=DEFAULT_REQUEST_TIMEOUT) + request_result = requests.post(str(callback), json=payload, timeout=settings.DEFAULT_REQUEST_TIMEOUT_SEC) if not status.HTTP_200_OK <= request_result.status_code < status.HTTP_300_MULTIPLE_CHOICES: msg = f"Callback failed: {request_result.text}" logger.error(msg) +@shared_task # type: ignore[misc] +def run_playbook_proc_task( + job_id: str, playbook_path: str, extra_vars: dict[str, Any], inventory: dict[str, Any] | str, callback: HttpUrl +) -> None: + """Celery task to run a playbook. + + :param str job_id: Identifier of the job being executed. + :param str playbook_path: Path to the playbook to be executed. + :param dict[str, Any] extra_vars: Extra variables to pass to the playbook. + :param dict[str, Any] | str inventory: Inventory to run the playbook against. + :param HttpUrl callback: Callback URL for status updates. + :return: None + """ + _run_playbook_proc(job_id, playbook_path, extra_vars, inventory, callback) + + def run_playbook( playbook_path: Path, extra_vars: dict[str, Any], @@ -107,16 +138,10 @@ def run_playbook( return playbook_launch_error(reason=msg, status_code=status.HTTP_400_BAD_REQUEST) job_id = str(uuid.uuid4()) - thread = threading.Thread( - target=_run_playbook_proc, - kwargs={ - "job_id": job_id, - "playbook_path": str(playbook_path), - "inventory": inventory, - "extra_vars": extra_vars, - "callback": callback, - }, - ) - thread.start() + if settings.EXECUTOR == ExecutorType.THREADPOOL: + executor = get_thread_pool() + executor.submit(_run_playbook_proc, job_id, str(playbook_path), extra_vars, inventory, callback) + elif settings.EXECUTOR == ExecutorType.WORKER: + run_playbook_proc_task.delay(job_id, str(playbook_path), extra_vars, inventory, callback) return playbook_launch_success(job_id=job_id) diff --git a/lso/routes/playbook.py b/lso/routes/playbook.py index 7ff68db..cb5a560 100644 --- a/lso/routes/playbook.py +++ b/lso/routes/playbook.py @@ -60,7 +60,7 @@ class PlaybookRunParams(BaseModel): """Parameters for executing an Ansible playbook.""" #: The filename of a playbook that's executed. It should be present inside the directory defined in the - #: configuration option ``ansible_playbooks_root_dir``. + #: configuration option ``ANSIBLE_PLAYBOOKS_ROOT_DIR``. playbook_name: str #: The address where LSO should call back to upon completion. callback: HttpUrl diff --git a/lso/worker.py b/lso/worker.py new file mode 100644 index 0000000..da44f94 --- /dev/null +++ b/lso/worker.py @@ -0,0 +1,34 @@ +"""Module that sets up :term:`LSO` as a Celery worker.""" + +from celery import Celery +from celery.signals import worker_shutting_down + +from lso.config import settings + +celery = Celery( + "lso-worker", + broker=settings.CELERY_BROKER_URL, + backend=settings.CELERY_RESULT_BACKEND, + include=[ + "lso.schedules.task_vacuum", + ], +) + +if settings.TESTING: + celery.conf.update(backend=settings.CELERY_RESULT_BACKEND, task_ignore_result=False) +else: + celery.conf.update(task_ignore_result=True) + +celery.conf.update( + result_expires=settings.CELERY_RESULT_EXPIRES, + worker_prefetch_multiplier=1, + worker_send_task_event=True, + task_send_sent_event=True, + redbeat_redis_url=settings.CELERY_BROKER_URL, +) + + +@worker_shutting_down.connect # type: ignore[misc] +def worker_shutting_down_handler(sig, how, exitcode, **kwargs) -> None: # type: ignore[no-untyped-def] # noqa: ARG001 + """Handle the Celery worker shutdown event.""" + celery.close() diff --git a/pyproject.toml b/pyproject.toml index 8aa7e10..ea8fab5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,9 +33,11 @@ dependencies = [ "ansible~=10.5.0", "fastapi~=0.115.2", "httpx~=0.27.0", - "jsonschema~=4.21.1", "uvicorn[standard]~=0.32.0", - "requests~=2.31.0" + "requests~=2.31.0", + "pydantic-settings~=2.5.2", + "celery~=5.3.6", + "redis==5.0.3", ] readme = "README.md" diff --git a/test/conftest.py b/test/conftest.py index 01946f9..d69e925 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -10,11 +10,9 @@ # 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 json import os import tempfile -from collections.abc import Callable, Generator +from collections.abc import Callable from io import StringIO from pathlib import Path from typing import Any @@ -23,7 +21,25 @@ from faker import Faker from fastapi.testclient import TestClient -import lso + +def pytest_configure() -> None: + """Configure environment variables for testing before the session starts.""" + tempdir = tempfile.TemporaryDirectory() + + # Create required YAML files for the unit tests + (Path(tempdir.name) / "placeholder.yaml").touch() + + # Set environment variables for the test session + os.environ["LSO_ANSIBLE_PLAYBOOKS_ROOT_DIR"] = tempdir.name + os.environ["LSO_TESTING"] = "true" + + # Register finalizers to clean up after tests are done + def cleanup() -> None: + tempdir.cleanup() + del os.environ["LSO_ANSIBLE_PLAYBOOKS_ROOT_DIR"] + del os.environ["LSO_TESTING"] + + pytest.session_cleanup = cleanup @pytest.fixture @@ -41,32 +57,11 @@ def run(*args: Any, **kwargs: Any) -> Runner: # noqa: ARG001 @pytest.fixture(scope="session") -def configuration_data() -> dict[str, str]: - """Start the server with valid configuration data.""" - with tempfile.TemporaryDirectory() as tempdir: - # Create required YAML files for the unit tests - (Path(tempdir) / "placeholder.yaml").touch() - - yield {"ansible_playbooks_root_dir": tempdir} - - -@pytest.fixture(scope="session") -def data_config_filename(configuration_data: dict[str, str]) -> Generator[str, Any, None]: - """Fixture that will yield a filename that contains a valid configuration. - - :return: Path to valid configuration file - """ - with tempfile.NamedTemporaryFile(mode="w") as file: - file.write(json.dumps(configuration_data)) - file.flush() - yield file.name - - -@pytest.fixture(scope="session") -def client(data_config_filename: str) -> TestClient: +def client() -> TestClient: """Return a client that can be used to test the server.""" - os.environ["SETTINGS_FILENAME"] = data_config_filename - app = lso.create_app() + from lso import create_app # noqa: PLC0415 + + app = create_app() return TestClient(app) # wait here until calling context ends diff --git a/test/routes/test_playbook.py b/test/routes/test_playbook.py index 3f0e0ce..36990a9 100644 --- a/test/routes/test_playbook.py +++ b/test/routes/test_playbook.py @@ -10,7 +10,6 @@ # 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 re import time from collections.abc import Callable diff --git a/test/test_config.py b/test/test_config.py deleted file mode 100644 index 67f2918..0000000 --- a/test/test_config.py +++ /dev/null @@ -1,44 +0,0 @@ -# Copyright 2023-2024 GÉANT Vereniging. -# 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. - -"""Set of tests that verify correct config is accepted and incorrect config is not.""" - -import json -import os -import tempfile -from pathlib import Path - -import jsonschema -import pytest - -from lso import config - - -def test_validate_testenv_config(data_config_filename: str) -> None: - """Load a configuration from a file. - - :param data_config_filename: Configuration file pytest fixture - """ - os.environ["SETTINGS_FILENAME"] = data_config_filename - params = config.load() - assert params - - -@pytest.mark.parametrize( - "bad_config", [{"name": "bad version", "version": 123}, {"name": "missing version"}, {"version": "missing name"}] -) -def test_bad_config(bad_config: dict) -> None: - with tempfile.NamedTemporaryFile(mode="w") as file: - Path(file.name).write_text(json.dumps(bad_config)) - with pytest.raises(jsonschema.ValidationError): - config.load_from_file(Path(file.name))