bvandeusenandClaude Opus 5
efde3b188f
CI / extension-version (push) Successful in 4s
CI / lint (push) Successful in 4s
Build images / sign-extension (push) Successful in 5s
Build images / build-agent (push) Successful in 7s
extension / lint (push) Successful in 18s
CI / frontend-build (push) Successful in 24s
CI / backend-lint-and-test (push) Successful in 33s
Build images / build-web (push) Successful in 1m42s
CI / integration (push) Successful in 2m11s
Build images / smoke-web (push) Successful in 57s
Build images / promote (push) Skipped
Operator, 2026-09-23: *"why isn't the healthcheck built into the image or
base on what command runs if one is passed in. why is it manually declared in
the stack here."*
No good reason. The container is the only thing that knows what it was asked
to run, and every compose file, stack file and README had to restate it:
web -> urllib /api/health
worker -> celery inspect ping -d celery@$HOSTNAME
all -> both, for every lane
Three checks written by hand, once per service, in every file anyone ever
wrote — none of them wrong until a role changed, and all of them silently
wrong after. The same duplication the lane table exists to remove one level
down, and I built it without noticing.
`entrypoint.sh` now records the role it started. The Dockerfile declares ONE
`HEALTHCHECK` that reads it and asks the right question: HTTP for web, a
self-addressed celery ping for a worker lane, both-for-every-lane for `all`,
and nothing for shell/alembic, which are one-shot and have no liveness to
probe. `docker-compose.single.yml` and the consolidated stack declare none.
A service that wants something else can still declare its own; docker prefers
it, so the escape hatch is the default docker behaviour rather than a flag.
Two details that are load-bearing:
* The role is written ONCE, by the outermost invocation. `all` starts the
other roles through this same script under supervisord, and a child
overwriting the container's role would turn the composite check into a
web-only one — silently, and only on the consolidated path. FC_ROLE is
exported so a child sees it set and skips.
* The celery ping is addressed to THIS node, not a bare ping. A bare one is
answered by any worker on the broker, so in a stack with replicas a dead
container would report healthy for as long as a sibling lived — the check
would be measuring the cluster rather than the container it is inside.
`healthcheck_all.py` is deleted; its two probes moved into the dispatcher
rather than being a second copy beside it.
An unrecorded role PASSES. The entrypoint always writes the file, so the only
way to miss it is bypassing the entrypoint — a debugging shape, where a check
that cannot tell what it is looking at must not assert the thing is broken
(snippet #3969). Said on stdout rather than assumed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
135 lines
4.4 KiB
Python
135 lines
4.4 KiB
Python
"""The image's healthcheck picks its check from the role it is running.
|
|
|
|
What this protects is the DISPATCH, not the probes. Each probe is a couple of
|
|
lines around a urllib call or a celery ping; the thing that can quietly go
|
|
wrong is a container being asked the wrong question — a consolidated one
|
|
checked as if it were web-only would report healthy with every worker dead,
|
|
which is precisely the failure mode consolidation creates.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
from backend.app.scripts import healthcheck as hc
|
|
|
|
|
|
@pytest.fixture
|
|
def calls(monkeypatch):
|
|
"""Record which probes ran, without running any of them."""
|
|
seen = []
|
|
|
|
def _stub(name, ok=True):
|
|
def run():
|
|
seen.append(name)
|
|
return (ok, "" if ok else f"{name} said no")
|
|
return run
|
|
|
|
monkeypatch.setattr(hc, "_web_ok", _stub("web"))
|
|
monkeypatch.setattr(hc, "_lanes_ok", _stub("lanes"))
|
|
monkeypatch.setattr(hc, "_this_node_ok", _stub("node"))
|
|
return seen
|
|
|
|
|
|
def _role(monkeypatch, value):
|
|
monkeypatch.setattr(hc, "current_role", lambda: value)
|
|
|
|
|
|
def test_the_consolidated_role_checks_both_halves(monkeypatch, calls):
|
|
"""`all` is the reason this file exists. Web alone would pass on a
|
|
container whose every worker had died, and docker can no longer see those
|
|
workers as separate services to notice for us."""
|
|
_role(monkeypatch, "all")
|
|
|
|
assert hc.main() == 0
|
|
assert calls == ["web", "lanes"]
|
|
|
|
|
|
def test_web_is_not_asked_about_lanes(monkeypatch, calls):
|
|
"""A web container runs no lanes, so asking would fail it for the absence
|
|
of something it was never meant to have."""
|
|
_role(monkeypatch, "web")
|
|
|
|
assert hc.main() == 0
|
|
assert calls == ["web"]
|
|
|
|
|
|
@pytest.mark.parametrize("role", ["worker", "scheduler", "ml-worker"])
|
|
def test_a_celery_role_pings_itself_and_nothing_else(monkeypatch, calls, role):
|
|
"""No HTTP — these serve no port, and a web probe would fail every one of
|
|
them."""
|
|
_role(monkeypatch, role)
|
|
|
|
assert hc.main() == 0
|
|
assert calls == ["node"]
|
|
|
|
|
|
@pytest.mark.parametrize("role", ["shell", "bash", "alembic"])
|
|
def test_one_shot_roles_have_nothing_to_check(monkeypatch, calls, role):
|
|
_role(monkeypatch, role)
|
|
|
|
assert hc.main() == 0
|
|
assert calls == []
|
|
|
|
|
|
def test_an_unrecorded_role_passes_rather_than_inventing_a_verdict(
|
|
monkeypatch, calls,
|
|
):
|
|
"""The entrypoint did not run — someone used `--entrypoint` or a bare
|
|
command. A check that cannot tell what it is looking at must not assert
|
|
that the thing is broken (snippet #3969)."""
|
|
_role(monkeypatch, None)
|
|
|
|
assert hc.main() == 0
|
|
assert calls == []
|
|
|
|
|
|
def test_an_unknown_role_passes_too(monkeypatch, calls):
|
|
"""A role added to entrypoint.sh and not here is a gap in this file, not
|
|
evidence the container is unhealthy."""
|
|
_role(monkeypatch, "something-new")
|
|
|
|
assert hc.main() == 0
|
|
assert calls == []
|
|
|
|
|
|
def test_a_failing_probe_fails_the_check(monkeypatch):
|
|
"""The guard has to be able to fail (rule 167) — every test above asserts
|
|
a 0, so one of them has to prove 0 is not all it can return."""
|
|
monkeypatch.setattr(hc, "current_role", lambda: "web")
|
|
monkeypatch.setattr(hc, "_web_ok", lambda: (False, "web unreachable"))
|
|
|
|
assert hc.main() == 1
|
|
|
|
|
|
def test_the_lane_half_can_fail_the_consolidated_role(monkeypatch):
|
|
"""The half that only the consolidated role has, and the half that
|
|
actually broke on run 7319 — web answered while three of four lanes read
|
|
as absent."""
|
|
monkeypatch.setattr(hc, "current_role", lambda: "all")
|
|
monkeypatch.setattr(hc, "_web_ok", lambda: (True, ""))
|
|
monkeypatch.setattr(
|
|
hc, "_lanes_ok", lambda: (False, "lanes not answering: ml, worker"),
|
|
)
|
|
|
|
assert hc.main() == 1
|
|
|
|
|
|
def test_the_role_is_read_from_the_file_the_entrypoint_writes(monkeypatch, tmp_path):
|
|
"""The contract between entrypoint.sh and this module. It is a file rather
|
|
than an env var because the HEALTHCHECK exec inherits the container's
|
|
environment, not whatever the entrypoint set at runtime."""
|
|
path = tmp_path / "fc-role"
|
|
path.write_text("all\n")
|
|
monkeypatch.delenv("FC_ROLE", raising=False)
|
|
monkeypatch.setattr(hc, "ROLE_FILE", str(path))
|
|
|
|
assert hc.current_role() == "all"
|
|
|
|
|
|
def test_a_missing_role_file_reads_as_unknown_not_as_a_crash(monkeypatch, tmp_path):
|
|
monkeypatch.delenv("FC_ROLE", raising=False)
|
|
monkeypatch.setattr(hc, "ROLE_FILE", str(tmp_path / "nope"))
|
|
|
|
assert hc.current_role() is None
|