diff --git a/readme.md b/readme.md index cea2f7eb..a9f63abe 100644 --- a/readme.md +++ b/readme.md @@ -288,6 +288,8 @@ healthcheck: retries: 3 ``` +The endpoint answers 503 when the download coordinator, the part that picks queued downloads up, is not running. Without it every download stays queued, so a container that reports unhealthy for this reason needs a restart. + ## Logging Logs are available via: diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index c0964f89..5ed2bc97 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -1344,6 +1344,16 @@ _coordinator_thread: threading.Thread | None = None _coordinator_lock = Lock() +def coordinator_running() -> bool: + """Whether the download coordinator thread is alive. + + The coordinator is the only thing that picks queued downloads up. If it dies, every task + stays "Queued" while the rest of the app looks healthy, so /api/health reports this. + """ + with _coordinator_lock: + return _coordinator_thread is not None and _coordinator_thread.is_alive() + + def start() -> None: """Start the download coordinator thread. Safe to call multiple times.""" global _coordinator_thread diff --git a/shelfmark/main.py b/shelfmark/main.py index 2bb39f8f..7b3757e9 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -1506,7 +1506,9 @@ def api_health() -> Response | tuple[Response, int]: No authentication required. Returns: - flask.Response: JSON with status "ok" and optional degraded features. + flask.Response: JSON with status "ok" and optional degraded features. Answers 503 with + status "error" when the download coordinator is not running, since nothing is + downloaded without it. """ response: dict[str, object] = {"status": "ok"} @@ -1515,6 +1517,12 @@ def api_health() -> Response | tuple[Response, int]: if not backend.WEBSOCKET_AVAILABLE: response["degraded"] = {"websocket": "WebSocket unavailable - real-time updates disabled"} + coordinator_up = backend.coordinator_running() + response["coordinator"] = "running" if coordinator_up else "stopped" + if not coordinator_up: + response["status"] = "error" + return jsonify(response), 503 + return jsonify(response) diff --git a/tests/core/test_api_key_env.py b/tests/core/test_api_key_env.py index 031da2a7..cf19cbf3 100644 --- a/tests/core/test_api_key_env.py +++ b/tests/core/test_api_key_env.py @@ -307,7 +307,9 @@ class TestMismatchFallsThrough: class TestScopeAndModes: def test_health_and_auth_paths_and_root_ignore_key(self, wired): client = wired.app.test_client() - assert client.get("/api/health", headers=_bearer("s3cret")).status_code == 200 + # The test app never starts the coordinator, which /api/health reports. + with patch.object(wired.backend, "coordinator_running", return_value=True): + assert client.get("/api/health", headers=_bearer("s3cret")).status_code == 200 assert ( client.get("/api/auth/check", headers=_bearer("s3cret")).get_json()["authenticated"] is False diff --git a/tests/core/test_health_endpoint.py b/tests/core/test_health_endpoint.py new file mode 100644 index 00000000..83f0cb92 --- /dev/null +++ b/tests/core/test_health_endpoint.py @@ -0,0 +1,78 @@ +"""/api/health must say so when the download coordinator is not running.""" + +from __future__ import annotations + +import importlib +import threading +from unittest.mock import patch + +import pytest + +from shelfmark.download import orchestrator + + +@pytest.fixture(scope="module") +def main_module(): + """Import `shelfmark.main` with background startup disabled.""" + with patch("shelfmark.download.orchestrator.start"): + import shelfmark.main as main + + importlib.reload(main) + return main + + +@pytest.fixture +def client(main_module): + return main_module.app.test_client() + + +def test_health_is_ok_while_the_coordinator_runs(main_module, client): + with patch.object(main_module.backend, "coordinator_running", return_value=True): + resp = client.get("/api/health") + + assert resp.status_code == 200 + assert resp.get_json()["status"] == "ok" + assert resp.get_json()["coordinator"] == "running" + + +def test_health_fails_when_the_coordinator_is_gone(main_module, client): + with patch.object(main_module.backend, "coordinator_running", return_value=False): + resp = client.get("/api/health") + + assert resp.status_code == 503 + assert resp.get_json()["status"] == "error" + assert resp.get_json()["coordinator"] == "stopped" + + +def test_health_needs_no_login(main_module, client): + with ( + patch.object(main_module, "get_auth_mode", return_value="builtin"), + patch.object(main_module.backend, "coordinator_running", return_value=True), + ): + resp = client.get("/api/health") + + assert resp.status_code == 200 + + +class TestCoordinatorRunning: + def test_false_before_it_was_ever_started(self, monkeypatch): + monkeypatch.setattr(orchestrator, "_coordinator_thread", None) + assert orchestrator.coordinator_running() is False + + def test_true_while_the_thread_is_alive(self, monkeypatch): + release = threading.Event() + thread = threading.Thread(target=release.wait, daemon=True) + thread.start() + monkeypatch.setattr(orchestrator, "_coordinator_thread", thread) + try: + assert orchestrator.coordinator_running() is True + finally: + release.set() + thread.join(timeout=2) + + def test_false_once_the_thread_has_died(self, monkeypatch): + thread = threading.Thread(target=lambda: None, daemon=True) + thread.start() + thread.join(timeout=2) + monkeypatch.setattr(orchestrator, "_coordinator_thread", thread) + assert orchestrator.coordinator_running() is False