From b2da3acce9ca1bf6fcd858acb9f67f2ed0c65b96 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 23 Sep 2026 09:17:58 -0400 Subject: [PATCH] feat: wait for Postgres and Redis before starting work (4295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator, 2026-09-23: *"it's a single container that need to connect successfully to redis and postgres before starting work shouldn't that simply be a check (with retries) at the start of the container."* Yes, and the consolidated layout makes it necessary rather than tidy. Swarm has no ordering primitive — it ignores `depends_on` outright — so every service in a stack starts at once and this container has always raced its own database on a cold deploy. The multi-service stack hid how sharp that is: a `web` task that failed `alembic upgrade head` against a still-initialising Postgres simply died, and Swarm restarted it until it worked. Nobody ever saw a problem worth naming. Consolidation removes that safety net. Each supervisord program gets `startretries=3`, so three quick failures put the program in FATAL and leave it there — supervisord keeps running, the container keeps running, and the application never starts. It would present as a permanently unhealthy container whose image was fine and whose database merely took twenty seconds to initialise, which is a miserable thing to debug on a first deploy. A TCP connect, not a query: the same probe ci.yml's integration lane and the build smoke already use. It answers the question actually being asked — is something listening — and cannot fail for a reason that retrying will never fix. A real query would be a stronger readiness signal and a worse gate, since a wrong password or a missing database is not transient, and a loop waiting for one to heal turns a five-second misconfiguration into a two-minute timeout with a misleading message. Those belong to alembic, which runs seconds later and says exactly what is wrong. Targets are derived from the same env the application reads, so the wait cannot drift from what the app will actually connect to — a gate checking a different host than the app uses is worse than no gate. Bounded at 120s (rule 156), reporting every few attempts so `docker logs` on a waiting container says what it is waiting for. Skipped for `shell`, which exists precisely for when something else is broken and you want a prompt rather than a gate. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- backend/app/scripts/wait_for_deps.py | 146 +++++++++++++++++++++++++++ entrypoint.sh | 22 ++++ tests/test_wait_for_deps.py | 91 +++++++++++++++++ 3 files changed, 259 insertions(+) create mode 100644 backend/app/scripts/wait_for_deps.py create mode 100644 tests/test_wait_for_deps.py diff --git a/backend/app/scripts/wait_for_deps.py b/backend/app/scripts/wait_for_deps.py new file mode 100644 index 0000000..a880e28 --- /dev/null +++ b/backend/app/scripts/wait_for_deps.py @@ -0,0 +1,146 @@ +"""Block until Postgres and Redis accept connections. Exit 0 ready, 1 timed out. + +## Why the container has to do this itself + +Compose has `depends_on: {condition: service_healthy}`, and **Swarm ignores +it**. `docker stack deploy` has no ordering primitive at all: every service in +the stack starts at once, so FabledCurator races Postgres on every cold +deploy and always has. + +The multi-service stack hid how sharp that is. `web` ran `alembic upgrade +head`, failed against a Postgres that was still doing `initdb`, and the task +died — but Swarm restarts a failed task forever, so the service came up a few +seconds later and nobody saw a problem worth naming. + +Consolidation removes that safety net. supervisord gives each program +`startretries=3`, so a web program that fails three times in the first +seconds goes FATAL and **stays** FATAL: supervisord keeps running, the +container keeps running, and the application never starts. The healthcheck +catches it — but as a container that is permanently unhealthy for a reason +that has nothing to do with the image, on a stack whose database simply took +twenty seconds to initialise. + +Operator, 2026-09-23: *"it's a single container that need to connect +successfully to redis and postgres before starting work shouldn't that simply +be a check (with retries) at the start of the container."* Yes. + +## A TCP connect, not a query + +The same probe `ci.yml`'s integration lane and the build smoke already use. +It answers the question that is actually being asked — is something listening +— and it cannot fail for a reason that retrying will never fix. + +A real query would be a stronger readiness signal and a worse gate: a wrong +password or a missing database is not a transient condition, and a loop that +waits for one to heal turns a five-second misconfiguration into a two-minute +timeout with a misleading message. Those belong to alembic, which runs +seconds later and says exactly what is wrong. + +The Postgres image is well behaved here: during `initdb` it serves on a unix +socket only and opens TCP when it is ready for clients, so the connect is a +good proxy for "ready" rather than merely "process exists". +""" + +from __future__ import annotations + +import argparse +import os +import socket +import sys +import time +from urllib.parse import urlparse + +# Long enough for a first-ever `initdb` on a slow disk, which is the worst +# case this exists for and is measured in tens of seconds, not minutes. A +# deploy that is genuinely misconfigured should fail while someone is still +# watching it rather than hold the container open for a quarter of an hour. +DEFAULT_TIMEOUT = 120.0 +CONNECT_TIMEOUT = 2.0 +RETRY_DELAY = 1.0 +# Progress every N attempts. `docker logs` on a container that is waiting must +# say what it is waiting for — silence is indistinguishable from a hang. +REPORT_EVERY = 5 + + +def _target(url: str | None, default_port: int) -> tuple[str, int] | None: + """(host, port) from a connection URL, or None if there is nothing to wait for.""" + if not url: + return None + parsed = urlparse(url) + if not parsed.hostname: + return None + return parsed.hostname, parsed.port or default_port + + +def targets() -> list[tuple[str, tuple[str, int]]]: + """What this container must reach, read from the same env the app reads. + + Derived rather than passed in, so the wait cannot drift from what the + application will actually connect to — a gate that checks a different + host than the app uses is worse than no gate. + """ + out: list[tuple[str, tuple[str, int]]] = [] + + host = os.environ.get("DB_HOST") + if host: + out.append(("postgres", (host, int(os.environ.get("DB_PORT") or 5432)))) + + broker = _target(os.environ.get("CELERY_BROKER_URL"), 6379) + if broker: + out.append(("redis", broker)) + + return out + + +def _accepts(host: str, port: int) -> bool: + try: + with socket.create_connection((host, port), timeout=CONNECT_TIMEOUT): + return True + except OSError: + return False + + +def wait( + name: str, host: str, port: int, deadline: float, now=time.monotonic, +) -> bool: + attempt = 0 + while True: + if _accepts(host, port): + print(f"[wait] {name} at {host}:{port} is accepting connections") + return True + attempt += 1 + if now() >= deadline: + print( + f"[wait] TIMEOUT: {name} at {host}:{port} never accepted a " + f"connection ({attempt} attempts)", + file=sys.stderr, + ) + return False + if attempt % REPORT_EVERY == 0: + left = int(deadline - now()) + print(f"[wait] {name} at {host}:{port} not ready yet, {left}s left") + time.sleep(RETRY_DELAY) + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description="Wait for Postgres and Redis.") + ap.add_argument("--timeout", type=float, default=DEFAULT_TIMEOUT) + args = ap.parse_args(argv) + + wanted = targets() + if not wanted: + # Nothing configured to wait for. Not an error: `shell` and one-off + # runs are legitimate, and refusing to start would make this gate the + # reason a debugging container will not boot. + print("[wait] no database or broker configured; nothing to wait for") + return 0 + + deadline = time.monotonic() + args.timeout + for name, (host, port) in wanted: + if not wait(name, host, port, deadline): + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/entrypoint.sh b/entrypoint.sh index b7c6fa8..0ff86e6 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -51,6 +51,28 @@ if [ -z "${FC_ROLE:-}" ]; then export FC_ROLE="$ROLE" printf '%s\n' "$ROLE" > "${FC_ROLE_FILE:-/tmp/fc-role}" 2>/dev/null || true fi + +# WAIT FOR POSTGRES AND REDIS before doing anything that needs them. +# +# Swarm has no ordering primitive — it ignores `depends_on` entirely — so +# every service in a stack starts at once and this container races its own +# database on every cold deploy. +# +# The multi-service stack hid how sharp that is: a `web` task that failed +# `alembic upgrade head` against a still-initialising Postgres simply died, +# and Swarm restarted it until it worked. Consolidation removes that. Each +# supervisord program gets `startretries=3`, so three quick failures put the +# program in FATAL and leave it there — supervisord keeps running, the +# container keeps running, and the application never starts. It would present +# as a permanently unhealthy container whose image was fine and whose +# database merely took twenty seconds to come up. +# +# Skipped for `shell`, which exists precisely for the case where something +# else is broken and you want a prompt rather than a gate. +case "$ROLE" in + shell|bash) ;; + *) python -m backend.app.scripts.wait_for_deps ;; +esac shift || true case "$ROLE" in diff --git a/tests/test_wait_for_deps.py b/tests/test_wait_for_deps.py new file mode 100644 index 0000000..7090796 --- /dev/null +++ b/tests/test_wait_for_deps.py @@ -0,0 +1,91 @@ +"""The startup gate that stands in for Swarm's missing `depends_on`. + +Swarm ignores `depends_on`, so the container races its own database on every +cold deploy. Under supervisord a program that fails three times goes FATAL and +stays there, so losing that race does not self-heal the way it did when Swarm +restarted a failed task — it leaves a running container with a dead +application inside it. +""" + +from __future__ import annotations + +import pytest + +from backend.app.scripts import wait_for_deps as w + + +def test_it_waits_for_both_postgres_and_redis(monkeypatch): + monkeypatch.setenv("DB_HOST", "postgres") + monkeypatch.setenv("DB_PORT", "5432") + monkeypatch.setenv("CELERY_BROKER_URL", "redis://redis:6379/0") + + assert w.targets() == [ + ("postgres", ("postgres", 5432)), + ("redis", ("redis", 6379)), + ] + + +def test_the_targets_come_from_the_same_env_the_app_reads(monkeypatch): + """A gate that checks a different host than the application connects to is + worse than no gate — it would pass while the app still cannot reach its + database.""" + monkeypatch.setenv("DB_HOST", "10.0.0.5") + monkeypatch.setenv("DB_PORT", "6543") + monkeypatch.setenv("CELERY_BROKER_URL", "redis://broker.internal:6380/2") + + assert w.targets() == [ + ("postgres", ("10.0.0.5", 6543)), + ("redis", ("broker.internal", 6380)), + ] + + +def test_a_broker_url_without_a_port_falls_back_to_the_default(monkeypatch): + monkeypatch.delenv("DB_HOST", raising=False) + monkeypatch.setenv("CELERY_BROKER_URL", "redis://redis/0") + + assert w.targets() == [("redis", ("redis", 6379))] + + +def test_nothing_configured_is_not_an_error(monkeypatch): + """`shell` and one-off runs are legitimate. Refusing to start would make + this gate the reason a debugging container will not boot.""" + monkeypatch.delenv("DB_HOST", raising=False) + monkeypatch.delenv("CELERY_BROKER_URL", raising=False) + + assert w.targets() == [] + assert w.main([]) == 0 + + +def test_it_returns_as_soon_as_the_port_accepts(monkeypatch): + attempts = {"n": 0} + + def accepts(host, port): + attempts["n"] += 1 + return attempts["n"] >= 3 + + monkeypatch.setattr(w, "_accepts", accepts) + monkeypatch.setattr(w.time, "sleep", lambda _: None) + + assert w.wait("postgres", "h", 5432, deadline=float("inf")) is True + assert attempts["n"] == 3 + + +def test_it_gives_up_at_the_deadline_rather_than_hanging(monkeypatch): + """A wait with no deadline is a bug (rule 156). A container that never + starts and never says why is the shape this must not have.""" + monkeypatch.setattr(w, "_accepts", lambda h, p: False) + monkeypatch.setattr(w.time, "sleep", lambda _: None) + + clock = iter([0.0, 0.0, 99.0]) + assert w.wait( + "postgres", "h", 5432, deadline=10.0, now=lambda: next(clock), + ) is False + + +def test_a_dependency_that_never_comes_up_fails_the_boot(monkeypatch): + monkeypatch.setenv("DB_HOST", "postgres") + monkeypatch.delenv("CELERY_BROKER_URL", raising=False) + monkeypatch.setattr(w, "_accepts", lambda h, p: False) + monkeypatch.setattr(w.time, "sleep", lambda _: None) + + assert w.main(["--timeout", "0"]) == 1