Files
svcforge/services/api/routes/health.py
T
Nguyen Minh Phuc c76154aeaa
ci / lint (push) Successful in 34s
ci / unit (push) Successful in 1m41s
ci / types (push) Successful in 1m41s
ci / dockerfile (push) Successful in 18s
ci / security (push) Successful in 1m27s
ci / chart (push) Failing after 1m11s
ci / integration (push) Successful in 1m10s
ci / image (api) (push) Has been skipped
ci / image (reconciler) (push) Has been skipped
ci / image (worker) (push) Has been skipped
ci / bump (push) Has been skipped
review: fix 26 findings from a 4-agent audit
CORRECTNESS
- lost-lease race: complete()/fail() did not check ownership, so a worker whose
  lease expired could mark a task done while another worker was running it, or
  requeue a task someone else owned. Reproduced, fixed with a CAS on
  (state, locked_by), pinned by two regression tests.
- worker died on report failure: _run_one's docstring claimed no exception
  escapes the TaskGroup; fail()/complete() were outside the guarded block, so a
  DB blip cancelled every sibling provision on the pod.
- claim query used an INNER join, which could strand a just-claimed task and
  report 'queue empty'. LEFT join.
- InstanceRepo.set_error bypassed the state machine and had no callers. Deleted.
- handle_deprovision ignored its CAS result, so a wrong-state instance kept a
  dangling endpoint and got re-provisioned by the drift check 60s later.
- handle_verify re-notified on every retry: five pages for one halt.

DEPLOY-BREAKING
- the migration Job could never succeed: no Dockerfile copied migrations/, and
  migrate.py resolved the path relative to the source tree, which only works for
  an editable install. Added COPY + SVCFORGE_MIGRATIONS_DIR.
- ServiceMonitor selector did not match the Service: API metrics never scraped.
- SvcforgeReconcilerStale fired permanently from every pod, because the gauge is
  module-level and every service exports it as 0. Scoped to the reconciler job.
- SvcforgeTaskFailed latched forever on a monotonic counter. Now increase()[15m].
- the digest guard accepted the all-zeros placeholder.
- worker terminationGracePeriodSeconds was 60s against a 600s helm timeout.

DEAD CODE THAT SHOULD NOT HAVE BEEN
- adapters/k8s.py was never called, so tenant namespaces were never created and
  the first provision for a new team would fail. Wired into handle_provision.
- adapters/redis.py was never imported by any service. Rate limiting is now wired
  into the API, failing open.
- Settings.check_production() had no callers. Given an explicit environment and
  called from every entrypoint.

OBSERVABILITY
- the API never called obs.setup(): no JSON logs, no trace correlation, log_json
  silently inert.
- LogNotifier's structured fields were discarded by the stdlib->structlog bridge.
- bind_task_context cleared the 'service' binding for the life of every task.
- split tasks_failed into task_attempts_failed and tasks_dead_lettered.

SECURITY
- trivy correctly blocked the worker/reconciler images: helm 3.16.2 and kubectl
  1.31.2 carry CRITICAL Go stdlib CVEs. Bumped to helm 3.21.3 and kubectl 1.35.3,
  which also closes a four-minor skew against the v1.35.3 cluster.

TESTS THAT COULD NOT FAIL
- the concurrency cap test passed on a fully serial worker.
- the alert/metric cross-check asserted a hardcoded list instead of reading the
  chart, so it could not catch a rename on the chart side.
- fixed OTel tracer-provider pollution between test files.

DOCS
- ARCHITECTURE.md: mermaid diagrams, user stories, and the helm-vs-ArgoCD
  guarantee (verified with --dry-run=server).
- AGENTS.md + CLAUDE.md.
- prose sweep for back-and-forth phrasing across 19 files.
2026-07-18 12:13:49 +00:00

82 lines
3.6 KiB
Python

"""Liveness, readiness, metrics.
The distinction between the first two is the difference between a 30-second blip and a
fleet-wide outage:
* `/healthz` (liveness) answers "is this process wedged?" A failure here gets the
container KILLED. It must therefore touch NOTHING external. Wire it to the DB and a
20-second Postgres failover restarts every pod at once; they come back, find the DB
still down, and CrashLoopBackOff with exponential restart delays — so the fleet is now
down for minutes after the database recovered.
* `/readyz` (readiness) answers "should this pod get traffic?" A failure here only removes
it from the Service endpoints. It is allowed to check dependencies, and it recovers by
itself the moment the check passes.
"""
from __future__ import annotations
from typing import Any
from fastapi import APIRouter, HTTPException, Request, Response, status
from prometheus_client import REGISTRY
from prometheus_client.exposition import choose_encoder
from services.api.deps import PoolDep
from services.api.models import ErrorBody
router = APIRouter(tags=["ops"])
# No PROMETHEUS_MULTIPROC_DIR here, deliberately: it exists for prefork servers where each
# worker process holds a slice of the counters. One uvicorn process per container means
# the default in-process registry is already correct, and multiproc mode would add a
# shared temp dir, a cleanup obligation, and a class of stale-file bugs for nothing.
@router.get("/healthz", status_code=status.HTTP_200_OK)
async def healthz() -> dict[str, str]:
"""Liveness. No I/O. If the event loop can run this, the process is alive."""
return {"status": "ok"}
@router.get(
"/readyz",
responses={503: {"model": ErrorBody, "description": "A dependency is unavailable"}},
)
async def readyz(pool: PoolDep) -> dict[str, str]:
"""Readiness. Postgres only.
Postgres-only is the rule, and Redis is the temptation. Redis holds derived state —
rate-limit buckets, caches — and everything degrades gracefully without it. Put it in
this check and an Upstash hiccup marks every pod unready, Kubernetes empties the
Service, and a cache outage becomes a total API outage.
"""
try:
async with pool.connection() as conn, conn.cursor() as cur:
await cur.execute("select 1")
row: Any = await cur.fetchone()
if row is None:
raise RuntimeError("select 1 returned no row")
except Exception as exc: # closed pool, timeout, dead DB — all mean the same 'not ready'
raise HTTPException(
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
detail={"code": "not_ready", "message": "database unavailable"},
) from exc
return {"status": "ready"}
@router.get("/metrics", response_class=Response)
async def metrics(request: Request) -> Response:
"""The Prometheus scrape endpoint.
A route rather than `app.mount("/metrics", make_asgi_app())`, for two reasons. A
Starlette `Mount` compiles to `^/metrics(?P<path>/.*)$`, which does not match a bare
`/metrics` — the exact URL every scrape config uses — and a `Mount` is invisible to
OpenAPI, while the deliverable asks for `/metrics` in `openapi.json`.
The encoding is still prometheus_client's: `choose_encoder` reads the Accept header and
picks the exposition format (Prometheus text vs OpenMetrics) with its matching content
type. Hand-rolling either is how you end up serving text/plain that a scraper rejects.
"""
encoder, content_type = choose_encoder(request.headers.get("Accept", ""))
return Response(content=encoder(REGISTRY), media_type=content_type)