From c0aeb89399052ffd9fbc549e885dd3aaa8ba1b76 Mon Sep 17 00:00:00 2001 From: yellowcooln <12516003+yellowcooln@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:39:42 -0400 Subject: [PATCH] fix(docker): supervise plugin manager lifecycle --- .env.example | 3 + .github/workflows/container-checks.yml | 10 + docker-compose.yml | 4 + docker-entrypoint.sh | 16 +- dockerfile | 3 +- docs/plugins.md | 24 ++- repeater/plugins/container_supervisor.py | 212 ++++++++++++++++++ repeater/plugins/github_releases.py | 13 +- repeater/plugins/manager.py | 5 +- repeater/plugins/runtime.py | 77 ++++++- scripts/docker-plugin-smoke.sh | 248 ++++++++++++++++++++++ tests/test_container_workflow.py | 39 ++++ tests/test_plugin_container_supervisor.py | 105 +++++++++ tests/test_plugin_github_releases.py | 21 ++ tests/test_plugin_manager_lifecycle.py | 103 ++++++++- 15 files changed, 858 insertions(+), 25 deletions(-) create mode 100644 repeater/plugins/container_supervisor.py create mode 100755 scripts/docker-plugin-smoke.sh create mode 100644 tests/test_container_workflow.py create mode 100644 tests/test_plugin_container_supervisor.py diff --git a/.env.example b/.env.example index 02f018d..5127aa8 100644 --- a/.env.example +++ b/.env.example @@ -32,3 +32,6 @@ SPI_GID=989 # image yourself instead of pulling OPENHOP_REPEATER_IMAGE. PUID=15888 PGID=15888 +# Optional fine-grained GitHub token for plugin catalogue release lookups. +# Public-repository read access is sufficient. Leave empty for anonymous access. +OPENHOP_PLUGIN_GITHUB_TOKEN= diff --git a/.github/workflows/container-checks.yml b/.github/workflows/container-checks.yml index 5ae1d7f..c645649 100644 --- a/.github/workflows/container-checks.yml +++ b/.github/workflows/container-checks.yml @@ -8,6 +8,10 @@ on: - docker-compose.yml - docker-compose.build.yml - docker-entrypoint.sh + - repeater/plugins/** + - scripts/docker-plugin-smoke.sh + - tests/test_container_workflow.py + - tests/test_plugin_container_supervisor.py - .github/workflows/container-checks.yml workflow_dispatch: @@ -33,3 +37,9 @@ jobs: - name: Lint Dockerfile run: docker run --rm -i hadolint/hadolint:v2.15.1 hadolint --failure-threshold error - < dockerfile + + - name: Build runtime image + run: docker build --build-arg PACKAGE_VERSION=0.0.0.dev0 -t openhop-repeater:plugin-smoke -f dockerfile . + + - name: Exercise plugin runtime in Docker + run: scripts/docker-plugin-smoke.sh openhop-repeater:plugin-smoke diff --git a/docker-compose.yml b/docker-compose.yml index 6e28463..d9a62bb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -3,6 +3,10 @@ services: image: ${OPENHOP_REPEATER_IMAGE:-${PYMC_REPEATER_IMAGE:-openhop/openhop-repeater:main}} container_name: openhop-repeater restart: unless-stopped + environment: + # Optional fine-grained, read-only token for catalogue release lookups. + # It raises GitHub's API limit; it is never sent to release asset hosts. + OPENHOP_PLUGIN_GITHUB_TOKEN: ${OPENHOP_PLUGIN_GITHUB_TOKEN:-} ports: - 8000:8000 devices: diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index e211dc2..32a3362 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -127,15 +127,7 @@ fi merge_config_from_example "${CONFIG_PATH}" -# Optionally start the plugin manager alongside Repeater (same container). -# Disable with OPENHOP_PLUGIN_MANAGER=0. Failures here must not block Repeater. -PLUGIN_MANAGER_ENABLED="${OPENHOP_PLUGIN_MANAGER:-1}" -if [ "${PLUGIN_MANAGER_ENABLED}" != "0" ] && [ "${PLUGIN_MANAGER_ENABLED}" != "false" ]; then - if command -v openhop-plugin-manager >/dev/null 2>&1; then - openhop-plugin-manager --config "${CONFIG_PATH}" & - elif python3 -c 'import repeater.plugins' >/dev/null 2>&1; then - python3 -m repeater.plugins --config "${CONFIG_PATH}" & - fi -fi - -exec python3 -m repeater.main --config "${CONFIG_PATH}" +# Keep Repeater and its optional plugin manager under one PID 1 supervisor. +# The supervisor forwards shutdown signals to both process groups and restarts +# the plugin manager if it exits while Repeater is still running. +exec python3 -m repeater.plugins.container_supervisor --config "${CONFIG_PATH}" diff --git a/dockerfile b/dockerfile index 28d61c7..f6818cb 100644 --- a/dockerfile +++ b/dockerfile @@ -38,6 +38,7 @@ RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install -y \ libusb-1.0-0 \ swig \ git \ + tini \ build-essential \ python3-dev \ && rm -rf /var/lib/apt/lists/* @@ -105,4 +106,4 @@ USER ${USER} EXPOSE 8000 -ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"] +ENTRYPOINT ["/usr/bin/tini", "-g", "--", "/usr/local/bin/docker-entrypoint.sh"] diff --git a/docs/plugins.md b/docs/plugins.md index d68887b..bd3d91a 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -22,6 +22,8 @@ Default root (under Repeater storage): ``` - Code and virtualenv are **version-specific**. +- The installed wheel is retained with each release so its virtualenv can be + rebuilt automatically if a future container image changes Python versions. - `data/` is **not** version-specific and is never interpreted by the manager. - IPC socket default: `/var/lib/openhop_repeater/plugin-manager.sock`. @@ -84,6 +86,12 @@ The manager starts enabled service plugins on boot and supervises crashes Repeater does **not** require the manager. If the socket is missing, plugin API calls return HTTP 503 and normal Repeater operation continues. +The Docker image runs Repeater and the manager under a small process supervisor, +with `tini` as PID 1 to reap orphaned children. It forwards shutdown signals to +both services and their plugin processes, and restarts the manager if it exits +unexpectedly while Repeater remains running. Set `OPENHOP_PLUGIN_MANAGER=0` to +run Repeater without the manager. + ## Install a local wheel Via API (authenticated): @@ -222,9 +230,13 @@ curl -X DELETE -H "Authorization: Bearer $TOKEN" \ 1. Build the `openhop-nomad-plugin` wheel. 2. Install via `/api/plugins/install`. -3. Write `$OPENHOP_PLUGIN_DATA/config.json` with `nomad_url` / `nomad_model`. -4. Enable the plugin — `meshcore-nomad-bridge` should report `RUNNING`. -5. The bridge talks to the Companion frame server (default `127.0.0.1:5001`). +3. Enable a Repeater Companion frame server reachable from the plugin. In the + standard Docker image, `127.0.0.1:5001` reaches the same container. +4. Write `$OPENHOP_PLUGIN_DATA/config.json` with `nomad_url` / `nomad_model`. +5. Enable the plugin — `meshcore-nomad-bridge` should report `RUNNING`. +6. Confirm its logs show a successful Companion connection, not only a running + retry loop. A `RUNNING` process can still be reconnecting to a disabled frame + server. ## Future work (not in this version) @@ -254,6 +266,12 @@ plugins: The catalogue lists plugin **identity and repository only**. Versions and wheel assets come from GitHub Releases on each plugin repo (draft and prerelease tags are ignored by default). +GitHub permits anonymous lookups, but the anonymous API limit is low and may be +shared by multiple containers behind the same public address. Docker operators +can set `OPENHOP_PLUGIN_GITHUB_TOKEN` in `.env` to a fine-grained token with +read-only access to public repository metadata. The token is used only for +`api.github.com` release lookups and is not sent to release asset URLs. + ```bash # List catalogue (annotated with installed / latest when available) curl -H "Authorization: Bearer $TOKEN" \ diff --git a/repeater/plugins/container_supervisor.py b/repeater/plugins/container_supervisor.py new file mode 100644 index 0000000..527321d --- /dev/null +++ b/repeater/plugins/container_supervisor.py @@ -0,0 +1,212 @@ +"""Container supervision for Repeater and its plugin manager.""" + +from __future__ import annotations + +import argparse +import logging +import os +import signal + +# Subprocess calls below use fixed argument arrays and never shell=True. +import subprocess # nosec B404 +import sys +import time +from collections.abc import Callable, Mapping +from pathlib import Path +from typing import Any + +import yaml + +logger = logging.getLogger("ContainerSupervisor") +_FALSE_VALUES = {"0", "false", "no", "off"} + + +def plugin_manager_enabled( + config_path: Path | str, + environ: Mapping[str, str] | None = None, +) -> bool: + """Return whether the container should run the plugin manager.""" + env = os.environ if environ is None else environ + if str(env.get("OPENHOP_PLUGIN_MANAGER", "1")).strip().lower() in _FALSE_VALUES: + return False + + try: + with open(config_path, encoding="utf-8") as config_file: + config = yaml.safe_load(config_file) or {} + except (OSError, yaml.YAMLError): + return True + + if not isinstance(config, dict): + return True + plugins = config.get("plugins") + return not (isinstance(plugins, dict) and plugins.get("enabled") is False) + + +def _signal_process_group(process: subprocess.Popen, signum: int) -> None: + """Signal a child session, falling back to the direct process.""" + try: + os.killpg(process.pid, signum) + except ProcessLookupError: + return + except OSError: + try: + process.send_signal(signum) + except ProcessLookupError: + return + + +def _process_group_exists(process: subprocess.Popen) -> bool: + """Return whether any process remains in a child session.""" + try: + os.killpg(process.pid, 0) + except ProcessLookupError: + return False + except PermissionError: + return True + return True + + +class ContainerSupervisor: + """Run, restart, and gracefully stop container child processes.""" + + def __init__( + self, + config_path: Path | str, + *, + manager_enabled: bool | None = None, + python_executable: str = "python3", + popen_factory: Callable[..., subprocess.Popen] = subprocess.Popen, + signal_process_group: Callable[[subprocess.Popen, int], None] = _signal_process_group, + process_group_exists: Callable[[subprocess.Popen], bool] = _process_group_exists, + sleep: Callable[[float], None] = time.sleep, + manager_restart_delay: float = 2.0, + shutdown_timeout: float = 8.0, + ): + self.config_path = str(config_path) + self.manager_enabled = ( + plugin_manager_enabled(config_path) if manager_enabled is None else manager_enabled + ) + self.python_executable = python_executable + self._popen = popen_factory + self._signal_process_group = signal_process_group + self._process_group_exists = process_group_exists + self._sleep = sleep + self.manager_restart_delay = manager_restart_delay + self.shutdown_timeout = shutdown_timeout + self.manager_process: subprocess.Popen | None = None + self.repeater_process: subprocess.Popen | None = None + self._stopping = False + self._shutdown_signal = signal.SIGTERM + + def _start(self, module: str) -> subprocess.Popen: + command = [ + self.python_executable, + "-m", + module, + "--config", + self.config_path, + ] + return self._popen(command, start_new_session=True) + + def _start_manager(self) -> None: + logger.info("Starting plugin manager") + self.manager_process = self._start("repeater.plugins") + + def request_shutdown(self, signum: int = signal.SIGTERM) -> None: + if self._stopping: + return + self._stopping = True + self._shutdown_signal = signum + logger.info("Signal %s received; stopping container services", signum) + for process in (self.repeater_process, self.manager_process): + if process is not None: + self._signal_process_group(process, signum) + + def _wait_or_kill(self, process: subprocess.Popen | None) -> None: + if process is None: + return + try: + process.wait(timeout=self.shutdown_timeout) + return + except subprocess.TimeoutExpired: + logger.warning("Process group %s did not stop in time; killing it", process.pid) + self._signal_process_group(process, signal.SIGKILL) + try: + process.wait(timeout=2) + except (subprocess.TimeoutExpired, ChildProcessError): + pass + + def _finish(self) -> None: + if not self._stopping: + self.request_shutdown(signal.SIGTERM) + self._wait_or_kill(self.repeater_process) + self._wait_or_kill(self.manager_process) + + def run(self) -> int: + if self.manager_enabled: + self._start_manager() + else: + logger.info("Plugin manager disabled") + + self.repeater_process = self._start("repeater.main") + + try: + while not self._stopping: + repeater_code = self.repeater_process.poll() + if repeater_code is not None: + return int(repeater_code) + + if self.manager_enabled and self.manager_process is not None: + manager_code = self.manager_process.poll() + if manager_code is not None: + logger.error( + "Plugin manager exited unexpectedly with status %s; restarting", + manager_code, + ) + # Terminate any plugin descendants left in the old process group. + self._signal_process_group(self.manager_process, signal.SIGTERM) + self._sleep(self.manager_restart_delay) + if self._process_group_exists(self.manager_process): + logger.warning( + "Plugin descendants did not stop with manager %s; killing group", + self.manager_process.pid, + ) + self._signal_process_group(self.manager_process, signal.SIGKILL) + if not self._stopping and self.repeater_process.poll() is None: + self._start_manager() + + self._sleep(0.2) + finally: + self._finish() + + code = self.repeater_process.poll() + if code is None: + return 128 + int(self._shutdown_signal) + return int(code) + + +def build_arg_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="openHop Docker process supervisor") + parser.add_argument("--config", required=True, help="Path to Repeater config.yaml") + return parser + + +def main(argv: list[str] | None = None) -> int: + args = build_arg_parser().parse_args(argv) + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s %(levelname)s %(name)s: %(message)s", + ) + supervisor = ContainerSupervisor(args.config) + + def handle_signal(signum: int, _frame: Any) -> None: + supervisor.request_shutdown(signum) + + for signum in (signal.SIGINT, signal.SIGTERM): + signal.signal(signum, handle_signal) + + return supervisor.run() + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/repeater/plugins/github_releases.py b/repeater/plugins/github_releases.py index 0deef90..79c4293 100644 --- a/repeater/plugins/github_releases.py +++ b/repeater/plugins/github_releases.py @@ -125,10 +125,12 @@ class GitHubReleaseClient: cache_ttl: float = DEFAULT_CACHE_TTL_SECONDS, opener: Optional[Callable[..., Any]] = None, user_agent: str = USER_AGENT, + token: str | None = None, ): self.cache_ttl = cache_ttl self._opener = opener or self._default_open self.user_agent = user_agent + self.token = str(token or "").strip() or None # repo -> (monotonic_ts, list[PluginRelease]) self._cache: dict[str, tuple[float, list[PluginRelease]]] = {} @@ -260,12 +262,15 @@ class GitHubReleaseClient: def _fetch_releases_json(self, repo: str) -> list[Any]: url = f"{GITHUB_API_BASE}/repos/{repo}/releases?per_page=30" + headers = { + "User-Agent": self.user_agent, + "Accept": "application/vnd.github+json", + } + if self.token: + headers["Authorization"] = f"Bearer {self.token}" request = urllib.request.Request( url, - headers={ - "User-Agent": self.user_agent, - "Accept": "application/vnd.github+json", - }, + headers=headers, method="GET", ) try: diff --git a/repeater/plugins/manager.py b/repeater/plugins/manager.py index b0d5338..bf29b0a 100644 --- a/repeater/plugins/manager.py +++ b/repeater/plugins/manager.py @@ -4,6 +4,7 @@ from __future__ import annotations import json import logging +import os import threading from pathlib import Path from typing import Any, Optional @@ -48,7 +49,9 @@ class PluginManager: self.runtime = runtime or PluginRuntime(storage) self._lock = threading.RLock() self.catalogue = catalogue_client or CatalogueClient(catalogue_url or DEFAULT_CATALOGUE_URL) - self.github = github_client or GitHubReleaseClient() + self.github = github_client or GitHubReleaseClient( + token=os.environ.get("OPENHOP_PLUGIN_GITHUB_TOKEN") + ) def start(self) -> None: """Load state and start enabled service plugins.""" diff --git a/repeater/plugins/runtime.py b/repeater/plugins/runtime.py index 2758e05..a2827d8 100644 --- a/repeater/plugins/runtime.py +++ b/repeater/plugins/runtime.py @@ -4,6 +4,7 @@ from __future__ import annotations import logging import os +import shutil import signal import subprocess # nosec B404 - argument arrays only, never shell=True import sys @@ -89,15 +90,27 @@ class PluginRuntime: paths = self.storage.ensure_plugin_layout(plugin_id, version) self.storage.write_manifest(plugin_id, version, manifest) + # Keep the exact install artifact with the release. Container data + # volumes outlive the image, so this lets us rebuild the isolated venv + # after a future base-image Python minor-version change. + release_dir = paths.release_dir(version) + archived_wheel = release_dir / wheel_path.name + if archived_wheel.resolve() != wheel_path: + for previous_wheel in release_dir.glob("*.whl"): + previous_wheel.unlink(missing_ok=True) + shutil.copy2(wheel_path, archived_wheel) + else: + archived_wheel = wheel_path + # Extract optional UI assets from the wheel into the release dir - self._extract_ui_assets(wheel_path, paths.release_dir(version), manifest) + self._extract_ui_assets(archived_wheel, release_dir, manifest) # Also extract optional config.default.json next to the manifest if present - self._extract_config_default_file(wheel_path, paths.release_dir(version)) + self._extract_config_default_file(archived_wheel, release_dir) venv_path = paths.venv_dir(version) if manifest.runtime is not None: self._create_venv(venv_path) - self._pip_install(venv_path, wheel_path) + self._pip_install(venv_path, archived_wheel) # Seed data/config.json from defaults when the user has no saved config yet self._seed_config_if_missing(plugin_id, manifest, paths.release_dir(version)) @@ -123,6 +136,58 @@ class PluginRuntime: "data_dir": str(paths.data_dir), } + def ensure_venv_compatible(self, plugin_id: str) -> None: + """Rebuild a persisted plugin venv when Python's minor version changes.""" + state = self.storage.read_state(plugin_id) + manifest = self.storage.load_current_manifest(plugin_id) + if state is None or manifest is None or manifest.runtime is None: + return + + version = str(state.get("version") or manifest.version) + paths = self.storage.paths_for(plugin_id) + venv_path = paths.venv_dir(version) + config_path = venv_path / "pyvenv.cfg" + if not config_path.is_file(): + # Legacy/test venvs have no reliable version marker. Let normal + # entrypoint validation report a useful error instead of deleting. + return + + persisted_version = None + try: + for line in config_path.read_text(encoding="utf-8").splitlines(): + key, separator, value = line.partition("=") + if separator and key.strip().lower() == "version": + parts = value.strip().split(".") + if len(parts) >= 2: + persisted_version = (int(parts[0]), int(parts[1])) + break + except (OSError, ValueError): + return + + current_version = (sys.version_info.major, sys.version_info.minor) + if persisted_version is None or persisted_version == current_version: + return + + wheels = sorted(paths.release_dir(version).glob("*.whl")) + if not wheels: + raise RuntimeError( + f"plugin venv uses Python {persisted_version[0]}.{persisted_version[1]}, " + f"but the container uses {current_version[0]}.{current_version[1]}; " + "reinstall the plugin to rebuild its environment" + ) + + logger.warning( + "Rebuilding %s venv for Python %s.%s (was %s.%s)", + plugin_id, + current_version[0], + current_version[1], + persisted_version[0], + persisted_version[1], + ) + shutil.rmtree(venv_path) + self._create_venv(venv_path) + self._pip_install(venv_path, wheels[0]) + def _create_venv(self, venv_path: Path) -> None: if venv_path.exists(): # Reinstall into existing venv is ok; recreate if broken @@ -411,6 +476,10 @@ class PluginRuntime: # Allow explicit start only when enabled — callers enable first raise RuntimeError(f"plugin is disabled: {plugin_id}") + # Persisted Docker volumes can outlive the image's Python minor + # version. Rebuild before resolving the installed entrypoint. + self.ensure_venv_compatible(plugin_id) + existing = self._handles.get(plugin_id) if existing and existing.process.poll() is None: self._runtime_state[plugin_id] = PluginState.RUNNING @@ -434,6 +503,8 @@ class PluginRuntime: log_fp = open(paths.log_file, "a", encoding="utf-8", buffering=1) env = os.environ.copy() + # Manager-only credentials must never become plugin credentials. + env.pop("OPENHOP_PLUGIN_GITHUB_TOKEN", None) env["OPENHOP_PLUGIN_ID"] = plugin_id env["OPENHOP_PLUGIN_DATA"] = str(paths.data_dir) diff --git a/scripts/docker-plugin-smoke.sh b/scripts/docker-plugin-smoke.sh new file mode 100755 index 0000000..57f74c9 --- /dev/null +++ b/scripts/docker-plugin-smoke.sh @@ -0,0 +1,248 @@ +#!/bin/sh +set -eu + +IMAGE="${1:-openhop-repeater:plugin-smoke}" +SUFFIX="${GITHUB_RUN_ID:-$$}-${GITHUB_RUN_ATTEMPT:-0}" +CONTAINER="openhop-plugin-smoke-${SUFFIX}" +CONFIG_VOLUME="openhop-plugin-smoke-config-${SUFFIX}" +DATA_VOLUME="openhop-plugin-smoke-data-${SUFFIX}" + +cleanup() { + docker rm -f "${CONTAINER}" >/dev/null 2>&1 || true + docker volume rm "${CONFIG_VOLUME}" "${DATA_VOLUME}" >/dev/null 2>&1 || true +} +trap cleanup EXIT HUP INT TERM + +start_container() { + docker run -d \ + --name "${CONTAINER}" \ + -v "${CONFIG_VOLUME}:/etc/openhop_repeater" \ + -v "${DATA_VOLUME}:/var/lib/openhop_repeater" \ + "${IMAGE}" >/dev/null +} + +wait_for_manager() { + attempts=0 + while [ "${attempts}" -lt 60 ]; do + if docker exec "${CONTAINER}" python3 -c \ + 'from repeater.plugins.ipc import PluginIPCClient; PluginIPCClient("/var/lib/openhop_repeater/plugin-manager.sock").call("ping")' \ + >/dev/null 2>&1; then + return 0 + fi + attempts=$((attempts + 1)) + sleep 1 + done + docker logs "${CONTAINER}" >&2 || true + echo "Plugin manager did not become ready" >&2 + return 1 +} + +wait_for_http() { + attempts=0 + while [ "${attempts}" -lt 60 ]; do + if docker exec "${CONTAINER}" python3 -c \ + 'import urllib.request; assert urllib.request.urlopen("http://127.0.0.1:8000/", timeout=2).status == 200' \ + >/dev/null 2>&1; then + return 0 + fi + attempts=$((attempts + 1)) + sleep 1 + done + docker logs "${CONTAINER}" >&2 || true + echo "Repeater HTTP server did not become ready" >&2 + return 1 +} + +docker volume create "${CONFIG_VOLUME}" >/dev/null +docker volume create "${DATA_VOLUME}" >/dev/null +start_container +wait_for_manager +wait_for_http + +echo "Checking local wheel upload" +docker exec -i "${CONTAINER}" python3 - <<'PY' +import json +import urllib.request +import uuid +import zipfile +from pathlib import Path + +wheel_path = Path("/tmp/openhop_smoke_plugin-0.1.0-py3-none-any.whl") +manifest = { + "schema": 1, + "id": "openhop.smoke", + "name": "Container Smoke Plugin", + "version": "0.1.0", + "runtime": {"type": "python", "entrypoint": "openhop-smoke-plugin"}, +} +module = '''import os +import signal +import time +from pathlib import Path + +running = True +lifecycle = Path(os.environ["OPENHOP_PLUGIN_DATA"]) / "lifecycle.log" + +def stop(signum, frame): + global running + with lifecycle.open("a", encoding="utf-8") as handle: + handle.write(f"TERM {os.getpid()}\\n") + running = False + +def main(): + signal.signal(signal.SIGTERM, stop) + signal.signal(signal.SIGINT, stop) + with lifecycle.open("a", encoding="utf-8") as handle: + handle.write(f"START {os.getpid()}\\n") + while running: + time.sleep(0.1) +''' +metadata = "Metadata-Version: 2.1\nName: openhop-smoke-plugin\nVersion: 0.1.0\n" +wheel = "Wheel-Version: 1.0\nGenerator: openhop-container-smoke\nRoot-Is-Purelib: true\nTag: py3-none-any\n" +entries = "[console_scripts]\nopenhop-smoke-plugin = openhop_smoke_plugin:main\n" +record = "\n".join( + [ + "openhop_smoke_plugin.py,,", + "openhop_smoke_plugin-0.1.0.dist-info/METADATA,,", + "openhop_smoke_plugin-0.1.0.dist-info/WHEEL,,", + "openhop_smoke_plugin-0.1.0.dist-info/entry_points.txt,,", + "openhop_smoke_plugin-0.1.0.dist-info/RECORD,,", + "share/openhop/plugins/openhop.smoke/openhop-plugin.json,,", + ] +) + "\n" +with zipfile.ZipFile(wheel_path, "w", compression=zipfile.ZIP_DEFLATED) as archive: + archive.writestr("openhop_smoke_plugin.py", module) + archive.writestr("openhop_smoke_plugin-0.1.0.dist-info/METADATA", metadata) + archive.writestr("openhop_smoke_plugin-0.1.0.dist-info/WHEEL", wheel) + archive.writestr("openhop_smoke_plugin-0.1.0.dist-info/entry_points.txt", entries) + archive.writestr("openhop_smoke_plugin-0.1.0.dist-info/RECORD", record) + archive.writestr( + "share/openhop/plugins/openhop.smoke/openhop-plugin.json", + json.dumps(manifest), + ) + +def request(path, *, body=None, headers=None, method=None): + req = urllib.request.Request( + "http://127.0.0.1:8000" + path, + data=body, + headers=headers or {}, + method=method, + ) + with urllib.request.urlopen(req, timeout=120) as response: + assert response.status == 200 + return json.loads(response.read()) + +login = request( + "/auth/login", + body=json.dumps( + {"username": "admin", "password": "admin123", "client_id": "docker-smoke"} + ).encode(), + headers={"Content-Type": "application/json"}, + method="POST", +) +assert login["success"] is True +headers = {"Authorization": "Bearer " + login["token"]} +boundary = "----openhop-smoke-" + uuid.uuid4().hex +upload = ( + f'--{boundary}\r\nContent-Disposition: form-data; name="wheel"; filename="{wheel_path.name}"\r\n' + "Content-Type: application/octet-stream\r\n\r\n" +).encode() + wheel_path.read_bytes() + f"\r\n--{boundary}--\r\n".encode() +installed = request( + "/api/plugins/install", + body=upload, + headers={**headers, "Content-Type": "multipart/form-data; boundary=" + boundary}, + method="POST", +) +assert installed["success"] is True +assert installed["plugin"]["id"] == "openhop.smoke" +release_dir = Path( + "/var/lib/openhop_repeater/plugins/openhop.smoke/releases/0.1.0" +) +assert any(release_dir.glob("*.whl")), "installed wheel was not archived" +enabled = request( + "/api/plugins/enable", + body=json.dumps({"id": "openhop.smoke"}).encode(), + headers={**headers, "Content-Type": "application/json"}, + method="POST", +) +assert enabled["success"] is True +assert enabled["plugin"]["state"] == "RUNNING" +PY + +echo "Checking plugin-manager crash recovery" +docker exec "${CONTAINER}" python3 -c ' +import os +import signal +from pathlib import Path +needle = b"\x00-m\x00repeater.plugins\x00" +for cmdline in Path("/proc").glob("[0-9]*/cmdline"): + try: + data = cmdline.read_bytes() + except OSError: + continue + if needle in data: + os.kill(int(cmdline.parent.name), signal.SIGKILL) + break +else: + raise SystemExit("plugin manager process not found") +' +wait_for_manager +attempts=0 +while [ "${attempts}" -lt 30 ]; do + starts="$(docker exec "${CONTAINER}" python3 -c ' +from pathlib import Path +path = Path("/var/lib/openhop_repeater/plugins/openhop.smoke/data/lifecycle.log") +print(sum(line.startswith("START ") for line in path.read_text().splitlines())) +' 2>/dev/null || printf '0')" + if [ "${starts}" -ge 2 ]; then + break + fi + attempts=$((attempts + 1)) + sleep 1 +done +[ "${starts}" -ge 2 ] +docker exec "${CONTAINER}" python3 -c ' +from pathlib import Path +zombies = [] +for stat_path in Path("/proc").glob("[0-9]*/stat"): + try: + fields = stat_path.read_text().split() + except OSError: + continue + if len(fields) > 2 and fields[2] == "Z": + zombies.append(stat_path.parent.name) +assert not zombies, f"zombie processes remain after manager restart: {zombies}" +' + +echo "Checking container recreation persistence" +docker stop -t 10 "${CONTAINER}" >/dev/null +docker rm "${CONTAINER}" >/dev/null +start_container +wait_for_manager +wait_for_http +docker exec "${CONTAINER}" python3 -c ' +from repeater.plugins.ipc import PluginIPCClient +client = PluginIPCClient("/var/lib/openhop_repeater/plugin-manager.sock") +status = client.status("openhop.smoke") +assert status["enabled"] is True +assert status["state"] == "RUNNING" +' + +echo "Checking graceful plugin shutdown" +EXPECTED_PLUGIN_PID="$(docker exec "${CONTAINER}" python3 -c ' +from repeater.plugins.ipc import PluginIPCClient +client = PluginIPCClient("/var/lib/openhop_repeater/plugin-manager.sock") +print(client.status("openhop.smoke")["pid"]) +')" +docker stop -t 10 "${CONTAINER}" >/dev/null +docker run --rm --entrypoint python3 -e EXPECTED_PLUGIN_PID="${EXPECTED_PLUGIN_PID}" \ + -v "${DATA_VOLUME}:/var/lib/openhop_repeater" \ + "${IMAGE}" -c ' +import os +from pathlib import Path +lines = Path("/var/lib/openhop_repeater/plugins/openhop.smoke/data/lifecycle.log").read_text().splitlines() +assert sum(line.startswith("START ") for line in lines) >= 3, lines +expected = "TERM " + os.environ["EXPECTED_PLUGIN_PID"] +assert expected in lines, (expected, lines) +print("container plugin smoke test passed") +' diff --git a/tests/test_container_workflow.py b/tests/test_container_workflow.py new file mode 100644 index 0000000..a81a1f6 --- /dev/null +++ b/tests/test_container_workflow.py @@ -0,0 +1,39 @@ +"""Checks for the container runtime quality gate.""" + +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + + +def test_container_workflow_runs_plugin_runtime_smoke_test(): + workflow = (ROOT / ".github/workflows/container-checks.yml").read_text(encoding="utf-8") + + assert "docker build --build-arg PACKAGE_VERSION=0.0.0.dev0" in workflow + assert "scripts/docker-plugin-smoke.sh openhop-repeater:plugin-smoke" in workflow + + +def test_container_smoke_script_checks_install_restart_and_shutdown(): + script = (ROOT / "scripts/docker-plugin-smoke.sh").read_text(encoding="utf-8") + + assert "local wheel upload" in script + assert "container recreation persistence" in script + assert "graceful plugin shutdown" in script + assert "EXPECTED_PLUGIN_PID" in script + + +def test_compose_exposes_service_scoped_github_token(): + compose = (ROOT / "docker-compose.yml").read_text(encoding="utf-8") + docs = (ROOT / "docs/plugins.md").read_text(encoding="utf-8") + + assert "OPENHOP_PLUGIN_GITHUB_TOKEN" in compose + assert "OPENHOP_PLUGIN_GITHUB_TOKEN" in docs + + +def test_image_uses_tini_to_reap_orphaned_plugin_processes(): + dockerfile = (ROOT / "dockerfile").read_text(encoding="utf-8") + + assert " tini \\\n" in dockerfile + assert ( + 'ENTRYPOINT ["/usr/bin/tini", "-g", "--", "/usr/local/bin/docker-entrypoint.sh"]' + in dockerfile + ) diff --git a/tests/test_plugin_container_supervisor.py b/tests/test_plugin_container_supervisor.py new file mode 100644 index 0000000..54ba38d --- /dev/null +++ b/tests/test_plugin_container_supervisor.py @@ -0,0 +1,105 @@ +"""Container process supervision for Repeater and plugin manager.""" + +from __future__ import annotations + +import signal +import subprocess +from pathlib import Path + +from repeater.plugins.container_supervisor import ContainerSupervisor, plugin_manager_enabled + + +class FakeProcess: + def __init__(self, pid: int, polls: list[int | None]): + self.pid = pid + self._polls = list(polls) + self.returncode = None + self.wait_calls = [] + self.killed = False + + def poll(self): + if self._polls: + value = self._polls.pop(0) + if value is not None: + self.returncode = value + return value + return self.returncode + + def wait(self, timeout=None): + self.wait_calls.append(timeout) + if self.returncode is None: + raise subprocess.TimeoutExpired("fake", timeout) + return self.returncode + + def kill(self): + self.killed = True + self.returncode = -signal.SIGKILL + + +def test_plugin_manager_enabled_honours_config_and_environment(tmp_path: Path): + config = tmp_path / "config.yaml" + config.write_text("plugins:\n enabled: false\n", encoding="utf-8") + + assert plugin_manager_enabled(config, {}) is False + assert plugin_manager_enabled(config, {"OPENHOP_PLUGIN_MANAGER": "1"}) is False + + config.write_text("plugins:\n enabled: true\n", encoding="utf-8") + assert plugin_manager_enabled(config, {}) is True + assert plugin_manager_enabled(config, {"OPENHOP_PLUGIN_MANAGER": "false"}) is False + + +def test_supervisor_restarts_failed_manager_and_stops_it_with_repeater(): + manager_one = FakeProcess(101, [1]) + manager_two = FakeProcess(102, [None, None]) + repeater = FakeProcess(201, [None, None, 0]) + processes = [manager_one, repeater, manager_two] + commands = [] + signalled = [] + + def popen(command, **kwargs): + commands.append((command, kwargs)) + return processes.pop(0) + + def signal_group(process, signum): + signalled.append((process.pid, signum)) + process.returncode = -signum + + supervisor = ContainerSupervisor( + "/tmp/config.yaml", + manager_enabled=True, + popen_factory=popen, + signal_process_group=signal_group, + process_group_exists=lambda process: process.pid == manager_one.pid, + sleep=lambda _seconds: None, + manager_restart_delay=0, + ) + + assert supervisor.run() == 0 + assert [command for command, _kwargs in commands] == [ + ["python3", "-m", "repeater.plugins", "--config", "/tmp/config.yaml"], + ["python3", "-m", "repeater.main", "--config", "/tmp/config.yaml"], + ["python3", "-m", "repeater.plugins", "--config", "/tmp/config.yaml"], + ] + assert all(kwargs["start_new_session"] is True for _command, kwargs in commands) + assert (101, signal.SIGTERM) in signalled + assert (101, signal.SIGKILL) in signalled + assert (102, signal.SIGTERM) in signalled + assert len(manager_two.wait_calls) == 1 + + +def test_shutdown_signal_is_forwarded_to_both_process_groups(): + manager = FakeProcess(301, [None]) + repeater = FakeProcess(302, [None]) + signalled = [] + + supervisor = ContainerSupervisor( + "/tmp/config.yaml", + manager_enabled=True, + signal_process_group=lambda process, signum: signalled.append((process.pid, signum)), + ) + supervisor.manager_process = manager + supervisor.repeater_process = repeater + + supervisor.request_shutdown(signal.SIGTERM) + + assert signalled == [(302, signal.SIGTERM), (301, signal.SIGTERM)] diff --git a/tests/test_plugin_github_releases.py b/tests/test_plugin_github_releases.py index 84dd475..df07c3b 100644 --- a/tests/test_plugin_github_releases.py +++ b/tests/test_plugin_github_releases.py @@ -148,6 +148,27 @@ def test_download_wheel(tmp_path: Path): assert path.name.endswith(".whl") +def test_github_token_authenticates_api_but_not_asset_download(tmp_path: Path): + payload = [_release("v0.1.0")] + seen_headers = [] + + def opener(request: Request, timeout: float = 30.0): + seen_headers.append((request.full_url, dict(request.header_items()))) + if "api.github.com" in request.full_url: + return _FakeResp(json.dumps(payload).encode()) + if request.full_url.endswith(".whl"): + return _FakeResp(b"wheel-bytes") + raise AssertionError(request.full_url) + + client = GitHubReleaseClient(opener=opener, token="test-token") + client.download_latest_wheel("org/repo", tmp_path) + + api_headers = next(headers for url, headers in seen_headers if "api.github.com" in url) + asset_headers = next(headers for url, headers in seen_headers if url.endswith(".whl")) + assert api_headers["Authorization"] == "Bearer test-token" + assert "Authorization" not in asset_headers + + def test_rate_limit(): def opener(request: Request, timeout: float = 30.0): headers = {"X-RateLimit-Remaining": "0"} diff --git a/tests/test_plugin_manager_lifecycle.py b/tests/test_plugin_manager_lifecycle.py index a1ddce1..ad8de3f 100644 --- a/tests/test_plugin_manager_lifecycle.py +++ b/tests/test_plugin_manager_lifecycle.py @@ -4,8 +4,10 @@ from __future__ import annotations import json import subprocess +import sys import zipfile from pathlib import Path +from unittest.mock import MagicMock import pytest @@ -50,6 +52,14 @@ def _make_wheel(path: Path, manifest: dict, ui_files: dict[str, str] | None = No return path +def test_default_github_client_uses_service_scoped_token(tmp_path: Path, monkeypatch): + monkeypatch.setenv("OPENHOP_PLUGIN_GITHUB_TOKEN", "service-token") + + manager = PluginManager(PluginStorage(tmp_path / "plugins")) + + assert manager.github.token == "service-token" + + def test_install_creates_layout_and_isolated_venv(tmp_path: Path, monkeypatch): storage = PluginStorage(tmp_path / "plugins") calls = [] @@ -92,6 +102,7 @@ def test_install_creates_layout_and_isolated_venv(tmp_path: Path, monkeypatch): paths = storage.paths_for("openhop.demo") assert paths.data_dir.is_dir() assert paths.venv_dir("0.1.0").is_dir() + assert (paths.release_dir("0.1.0") / wheel.name).is_file() assert any("venv" in c for c in calls) assert any("pip" in c for c in calls) # Ensure we never invoked the system pip against a non-venv target alone @@ -100,13 +111,102 @@ def test_install_creates_layout_and_isolated_venv(tmp_path: Path, monkeypatch): assert str(paths.venv_dir("0.1.0")) in c[0] or "venv" in "".join(c) -def test_enable_start_stop_disable(tmp_path: Path): +def test_start_rebuilds_incompatible_persisted_venv_from_archived_wheel(tmp_path: Path): + storage = PluginStorage(tmp_path / "plugins") + plugin_id = "openhop.demo" + version = "0.1.0" + paths = storage.ensure_plugin_layout(plugin_id, version) + storage.write_state(plugin_id, {"version": version, "enabled": True}) + storage.write_manifest( + plugin_id, + version, + PluginManifest( + schema=1, + id=plugin_id, + name="Demo", + version=version, + runtime=RuntimeSpec(type="python", entrypoint="demo-cli"), + ), + ) + storage.set_current(plugin_id, version) + archived_wheel = paths.release_dir(version) / "demo-0.1.0-py3-none-any.whl" + _make_wheel( + archived_wheel, + { + "schema": 1, + "id": plugin_id, + "name": "Demo", + "version": version, + "runtime": {"type": "python", "entrypoint": "demo-cli"}, + }, + ) + venv = paths.venv_dir(version) + (venv / "bin").mkdir(parents=True) + (venv / "bin" / "python").write_text("old", encoding="utf-8") + (venv / "pyvenv.cfg").write_text("version = 3.11.9\n", encoding="utf-8") + calls = [] + + def fake_run(cmd, **kwargs): + calls.append(list(cmd)) + if "venv" in cmd: + target = Path(cmd[-1]) + (target / "bin").mkdir(parents=True, exist_ok=True) + (target / "bin" / "python").write_text("new", encoding="utf-8") + (target / "pyvenv.cfg").write_text( + f"version = {sys.version_info.major}.{sys.version_info.minor}.0\n", + encoding="utf-8", + ) + if "pip" in cmd: + entrypoint = Path(cmd[0]).parent / "demo-cli" + entrypoint.write_text("new entrypoint", encoding="utf-8") + return subprocess.CompletedProcess(cmd, 0, "", "") + + runtime = PluginRuntime( + storage, + run_factory=fake_run, + popen_factory=lambda *args, **kwargs: FakeProc(), + ) + runtime.start(plugin_id) + + assert any("venv" in call for call in calls) + assert any("pip" in call for call in calls) + assert (venv / "bin" / "python").read_text(encoding="utf-8") == "new" + + +def test_manager_starts_enabled_plugin(tmp_path: Path): + storage = PluginStorage(tmp_path / "plugins") + storage.ensure_plugin_layout("openhop.demo", "0.1.0") + storage.write_state("openhop.demo", {"version": "0.1.0", "enabled": True}) + storage.write_manifest( + "openhop.demo", + "0.1.0", + PluginManifest( + schema=1, + id="openhop.demo", + name="Demo", + version="0.1.0", + runtime=RuntimeSpec(type="python", entrypoint="demo-cli"), + ), + ) + storage.set_current("openhop.demo", "0.1.0") + runtime = MagicMock(spec=PluginRuntime) + manager = PluginManager(storage, runtime) + + manager.start() + + runtime.start.assert_called_once_with("openhop.demo") + + +def test_enable_start_stop_disable(tmp_path: Path, monkeypatch): storage = PluginStorage(tmp_path / "plugins") procs: list[FakeProc] = [] + child_environments = [] + monkeypatch.setenv("OPENHOP_PLUGIN_GITHUB_TOKEN", "must-not-reach-plugin") def fake_popen(cmd, **kwargs): p = FakeProc(pid=1000 + len(procs)) procs.append(p) + child_environments.append(kwargs["env"]) # Write a line to log fd if provided log_fp = kwargs.get("stdout") if log_fp and hasattr(log_fp, "write"): @@ -159,6 +259,7 @@ def test_enable_start_stop_disable(tmp_path: Path): st = manager.disable("openhop.demo") assert st["enabled"] is False assert st["state"] == PluginState.DISABLED.value + assert all("OPENHOP_PLUGIN_GITHUB_TOKEN" not in env for env in child_environments) logs = manager.logs("openhop.demo", tail=10) assert any("hello" in line for line in logs["lines"])