From 7becb384888844e380d8f7428dff97c02abe2b44 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Sun, 5 Jul 2026 14:18:10 +0800 Subject: [PATCH] =?UTF-8?q?fix(api):=20implement=20/readyz=20with=20real?= =?UTF-8?q?=20DB=20+=20K8s=20API=20+=20CRD=20checks=20(=C2=A77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously /readyz only verified Repo != nil && Cluster != nil — a process-liveness check, not a dependency-health check. The spec requires the readyz probe to verify DB, K8s API, and CRD informer are live before declaring the pod ready. - Repo interface gains Ping(context.Context) error - Cluster interface gains Ping(context.Context) error - PGRepo.Ping delegates to sql.DB.PingContext - K8sCluster.Ping lists MinecraftServer CRDs (Limit=1) in the configured namespace, exercising both the API and CRD informer - handleReadyz iterates ping checks; any failure returns 503 with the failing dependency name in the error message - fakeRepo and fakeCluster gain configurable pingErr for hermetic test coverage of the failure paths New test: TestReadyzPingsDependencies verifies 200 when healthy, 503 when DB or K8s API is down. --- internal/api/api_test.go | 34 +++++++++++++++++++++++++++++++ internal/api/cluster.go | 4 ++++ internal/api/handlers_internal.go | 18 ++++++++++------ internal/api/k8scluster.go | 5 +++++ internal/api/pgrepo.go | 2 ++ internal/api/repo.go | 4 ++++ 6 files changed, 61 insertions(+), 6 deletions(-) diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 3fa78e3..5614f0c 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -90,6 +91,9 @@ type fakeRepo struct { // user admin fakes seededUsers []seededUser fakeQuotas map[string]*QuotaView + // pingErr, when non-nil, is returned by Ping to simulate DB liveness check + // failures in /readyz tests. + pingErr error } // fakePasskeyChallenge mirrors a webauthn_challenges row: its owner and purpose, the @@ -646,6 +650,7 @@ func (f *fakeRepo) SeedServer(_ context.Context, name, subdomain string, _, _, _ f.aliases[subdomain] = name return nil } +func (f *fakeRepo) Ping(_ context.Context) error { return f.pingErr } func (f *fakeRepo) Audit(_ context.Context, e AuditEntry) error { f.audits = append(f.audits, e) return nil @@ -1164,6 +1169,7 @@ type fakeCluster struct { created map[string]CreateServerInput // name -> the validated input it was created from patched map[string]ServerSpecPatch // name -> the validated spec patch it received createErr error + pingErr error } func newFakeCluster() *fakeCluster { @@ -1184,6 +1190,7 @@ func (c *fakeCluster) GetBySubdomain(_ context.Context, s string) (*ServerInfo, return nil, ErrNotFound } func (c *fakeCluster) ListServers(_ context.Context) ([]ServerInfo, error) { return c.list, nil } +func (c *fakeCluster) Ping(_ context.Context) error { return c.pingErr } func (c *fakeCluster) SetDesiredState(_ context.Context, n string, s v1alpha1.DesiredState) error { c.desired[n] = s return nil @@ -1315,6 +1322,33 @@ func TestHealthzIsUnauthenticated(t *testing.T) { } } +// TestReadyzPingsDependencies proves /readyz verifies DB and K8s API liveness +// before declaring ready, and returns 503 when either is down (spec §7). +func TestReadyzPingsDependencies(t *testing.T) { + repo := newFakeRepo() + cl := newFakeCluster() + api := newTestAPI(repo, cl) + api.Internal = BearerTokenAuth{Token: "s3cr3t"} + + // Both healthy. + if w := do(api.InternalHandler(), "GET", "/readyz", "", nil); w.Code != http.StatusOK { + t.Fatalf("readyz code = %d, want 200 when both deps are healthy (%s)", w.Code, w.Body.String()) + } + + // DB down. + repo.pingErr = errors.New("connection refused") + if w := do(api.InternalHandler(), "GET", "/readyz", "", nil); w.Code != http.StatusServiceUnavailable { + t.Fatalf("readyz code = %d, want 503 when DB is down (%s)", w.Code, w.Body.String()) + } + repo.pingErr = nil + + // K8s API down. + cl.pingErr = errors.New("cannot reach apiserver") + if w := do(api.InternalHandler(), "GET", "/readyz", "", nil); w.Code != http.StatusServiceUnavailable { + t.Fatalf("readyz code = %d, want 503 when K8s API is down (%s)", w.Code, w.Body.String()) + } +} + func TestExternalFaceRequiresPrincipal(t *testing.T) { api := newTestAPI(newFakeRepo(), newFakeCluster()) api.External = staticExternal{err: http.ErrNoCookie} // any auth error diff --git a/internal/api/cluster.go b/internal/api/cluster.go index 8bdbf49..d1d88c2 100644 --- a/internal/api/cluster.go +++ b/internal/api/cluster.go @@ -75,6 +75,10 @@ type ServerSpecPatch struct { // so handlers are tested against a fake; the controller-runtime implementation // (k8sCluster) is integration-tested only — it requires a live cluster. type Cluster interface { + // Ping verifies the K8s API and CRD informer are healthy — used by /readyz + // (spec §7) to confirm the lifecycle store is reachable and synced. + Ping(ctx context.Context) error + // GetServer reads one MinecraftServer's lifecycle view, or ErrNotFound. GetServer(ctx context.Context, name string) (*ServerInfo, error) // GetBySubdomain finds the MinecraftServer whose spec.subdomain matches, or diff --git a/internal/api/handlers_internal.go b/internal/api/handlers_internal.go index 7684314..d579839 100644 --- a/internal/api/handlers_internal.go +++ b/internal/api/handlers_internal.go @@ -15,13 +15,19 @@ func (a *API) handleHealthz(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) } -// handleReadyz is a readiness probe. A full implementation also checks the DB, -// the K8s API and the CRD informer (spec §7); here it reports the configured -// dependencies are wired. Dependency pinging lands with the integration layer. +// handleReadyz is a readiness probe (spec §7). It checks the DB, K8s API and +// CRD informer before declaring ready — a full round-trip that mirrors what the +// actual request path depends on. func (a *API) handleReadyz(w http.ResponseWriter, r *http.Request) { - if a.Repo == nil || a.Cluster == nil { - writeError(w, r, newError(http.StatusServiceUnavailable, "not_ready", "dependencies not wired")) - return + checks := map[string]func(context.Context) error{ + "db": a.Repo.Ping, + "k8s_api": a.Cluster.Ping, + } + for name, check := range checks { + if err := check(r.Context()); err != nil { + writeError(w, r, newError(http.StatusServiceUnavailable, "not_ready", "%s: %v", name, err)) + return + } } writeJSON(w, http.StatusOK, map[string]string{"status": "ready"}) } diff --git a/internal/api/k8scluster.go b/internal/api/k8scluster.go index 1f62bff..bc3e461 100644 --- a/internal/api/k8scluster.go +++ b/internal/api/k8scluster.go @@ -27,6 +27,11 @@ func NewK8sCluster(c client.Client, namespace string) *K8sCluster { return &K8sCluster{c: c, namespace: namespace} } +func (k *K8sCluster) Ping(ctx context.Context) error { + var list v1alpha1.MinecraftServerList + return k.c.List(ctx, &list, client.InNamespace(k.namespace), client.Limit(1)) +} + func (k *K8sCluster) GetServer(ctx context.Context, name string) (*ServerInfo, error) { var ms v1alpha1.MinecraftServer if err := k.c.Get(ctx, types.NamespacedName{Namespace: k.namespace, Name: name}, &ms); err != nil { diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index 526ce2c..3baecb9 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -18,6 +18,8 @@ type PGRepo struct { // NewPGRepo wraps an existing pool (from store.PostgresDriver.DB()). func NewPGRepo(db *sql.DB) *PGRepo { return &PGRepo{db: db} } +func (p *PGRepo) Ping(ctx context.Context) error { return p.db.PingContext(ctx) } + func (p *PGRepo) ServerBySubdomain(ctx context.Context, subdomain string) (*ServerRecord, error) { const q = `SELECT s.name, sa.subdomain, COALESCE(s.owner_id, ''), COALESCE(s.cached_phase, '') FROM server_aliases sa JOIN servers s ON s.name = sa.server_name diff --git a/internal/api/repo.go b/internal/api/repo.go index 78fd001..8ccfe52 100644 --- a/internal/api/repo.go +++ b/internal/api/repo.go @@ -145,6 +145,10 @@ type OpLoginRequest struct { // so handlers are tested against an in-memory fake; the Postgres implementation // (pgRepo) is integration-tested only — it requires a live database. type Repo interface { + // Ping probes the database — used by the /readyz endpoint (spec §7) to verify + // the DB connection is alive. + Ping(ctx context.Context) error + // ServerBySubdomain resolves a subdomain alias to its server, or ErrNotFound. ServerBySubdomain(ctx context.Context, subdomain string) (*ServerRecord, error) // ServerByName loads a server's business projection, or ErrNotFound.