fix(server): exclude uninitialized replicas from readiness

.github/workflows/deploy-azure.yml

@@ -435,42 +435,30 @@ jobs:
           echo "Waiting 30 seconds for Container App to be ready..."
           sleep 30
 
-      - name: Test health endpoint
-        run: |
-          URL="${{ steps.get-url.outputs.url }}"
-          echo "Testing health endpoint: $URL/health"
-
-          for i in {1..5}; do
-            # /health reports but never triggers the lazy DB init on the HTTP
-            # server; any /api/ request does. /health below is the gate.
-            curl -s -o /dev/null --max-time 60 "$URL/api/auth/check-admin" || true
-            # /health returns HTTP 200 even when degraded; the body is the verdict.
-            if RESPONSE=$(curl -fsS --max-time 10 "$URL/health"); then
-              echo "Response: $RESPONSE"
-              if jq -e '.status == "healthy"' > /dev/null <<< "$RESPONSE"; then
-                echo "Health check passed!"
-                exit 0
-              fi
-            fi
-            echo "Attempt $i failed, retrying in 10 seconds..."
-            sleep 10
-          done
-
-          echo "Health check failed after 5 attempts"
-          exit 1
-
-      # Needs the repo for scripts/wait-for-healthy.sh. Rollout race (#513):
-      # a probe can land on the new revision while it is still initializing
-      # (auth_store "not initialized"), so one unhealthy response must not fail
-      # the step. The script requires HEALTH_REQUIRED_STREAK consecutive healthy
-      # responses within HEALTH_MAX_ATTEMPTS rounds and fails with the last body
-      # otherwise.
+      # Needs the repo for scripts/wait-for-healthy.sh, used by both gates below.
       - name: Checkout code
         uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
         with:
           persist-credentials: false
           sparse-checkout: scripts/wait-for-healthy.sh
 
+      # Fast fail gate: the server initializes its database in the background
+      # at startup, so no warm-up request is needed to make progress (#488).
+      # /ready answers 503 while this replica is still initializing, which
+      # curl -f turns into a failed attempt.
+      - name: Test readiness endpoint
+        env:
+          APP_URL: ${{ steps.get-url.outputs.url }}
+          HEALTH_REQUIRED_STREAK: "1"
+          HEALTH_MAX_ATTEMPTS: "5"
+          HEALTH_INTERVAL_SECONDS: "10"
+        run: bash scripts/wait-for-healthy.sh "$APP_URL"
+
+      # Rollout race (#513): a probe can land on the new revision while it is
+      # still initializing (auth_store "not initialized"), so one not-ready
+      # response must not fail the step. The script requires
+      # HEALTH_REQUIRED_STREAK consecutive ready responses within
+      # HEALTH_MAX_ATTEMPTS rounds and fails with the last body otherwise.
       - name: Run smoke tests
         env:
           APP_URL: ${{ steps.get-url.outputs.url }}

internal/api/openapi.yaml

@@ -2114,6 +2114,32 @@ paths:
               schema:
                 $ref: '#/components/schemas/HealthResponse'
 
+  /ready:
+    get:
+      operationId: readinessCheckRoot
+      tags: [Health]
+      summary: Readiness check (root path)
+      description: >-
+        Traffic-admission contract: answers 200 with the same payload as
+        /health once this replica has completed initialization, 503 while any
+        store check is pending or failed or migrations have not run yet. A
+        failed migration run is reported in the body but still answers 200.
+        /health is the liveness counterpart and always answers 200.
+      security: []
+      responses:
+        '200':
+          description: Replica is ready to serve traffic
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/HealthResponse'
+        '503':
+          description: Replica is still initializing or a store check failed
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/HealthResponse'
+
   /api/health:
     get:
       operationId: healthCheck

internal/server/app.go

@@ -86,6 +86,11 @@ type Application struct {
 	migrationsTimeout time.Duration
 	migrationMu       sync.Mutex
 
+	// initRetryDelay is the pause between background database-initialization
+	// attempts (see startBackgroundInit). Zero means backgroundInitRetryDelay.
+	// Only tests set it.
+	initRetryDelay time.Duration
+
 	// encKeySource is the env var name that resolved the credential encryption
 	// key (e.g. "CREDENTIAL_ENCRYPTION_KEY_SECRET_NAME"). Set during
 	// reinitializeAfterConnect; surfaced via /health.
@@ -634,15 +639,22 @@ func (app *Application) ensureDB(ctx context.Context) error {
 	log.Println("PostgreSQL connection established successfully")
 
 	// Run migrations if AutoMigrate is enabled. Failures are non-fatal:
-	// we log, surface via /health's migrations check, and proceed. The app
-	// stays up; handlers that need the missing schema error at query time.
+	// we log, surface via the migrations check in /health and /ready, and
+	// proceed. The app stays up and ready (/ready only waits for the first
+	// attempt to finish); handlers that need the missing schema error at
+	// query time.
 	// See specs/migration-resilience.md (or the plan) for the rationale.
 	if app.dbConfig.AutoMigrate {
 		log.Println("Running database migrations...")
 
 		adminEmail := os.Getenv("ADMIN_EMAIL")
 		adminPassword, err := app.resolveAdminPassword(ctx)
 		if err != nil {
+			// Same cleanup as the reinitializeAfterConnect failure path below:
+			// the retry loop (and repeated lazy attempts) would otherwise leak
+			// a pool per attempt while secret resolution keeps failing.
+			dbConn.Close()
+			app.DB = nil
 			return err // secret-resolution failure is still fatal — env/config, not a migration runtime error
 		}
 

internal/server/health.go

@@ -7,6 +7,10 @@ import (
 	"log"
 	"net/http"
 	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/database"
 )
 
 // HealthStatus represents the overall health of the application.
@@ -23,27 +27,94 @@ type CheckResult struct {
 	Message string `json:"message,omitempty"`
 }
 
-// handleHealthCheck returns the health status of the application.
+// handleHealthCheck returns the liveness status of the application.
+//
+// This endpoint is the LIVENESS contract: it always answers HTTP 200 so
+// orchestrator liveness/startup probes never restart or kill a process that is
+// up but whose dependencies are not connected yet. The verdict is in the JSON
+// body ("healthy" / "degraded"). Traffic admission is a separate question
+// answered by handleReadinessCheck (#488).
 func (app *Application) handleHealthCheck(w http.ResponseWriter, r *http.Request) {
 	ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second)
 	defer cancel()
 
+	writeHealthResponse(w, app.appConfig.CORSAllowedOrigin, http.StatusOK, app.collectHealth(ctx))
+}
+
+// handleReadinessCheck returns the READINESS status of the application.
+//
+// Readiness is the traffic-admission contract: a replica that has not completed
+// required initialization answers 503, so the load balancer stops sending it
+// requests. /health only reported "degraded" in the body while still answering
+// 200, which admitted cold replicas to traffic and broke the deployment smoke
+// gate (#488). Ready means required initialization is done; the body carries
+// the same checks as /health so an operator sees which one is still pending.
+// A failed migration run does not make the replica unready (see ready).
+func (app *Application) handleReadinessCheck(w http.ResponseWriter, r *http.Request) {
+	ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second)
+	defer cancel()
+
+	health := app.collectHealth(ctx)
+	status := http.StatusOK
+	if !health.ready() {
+		status = http.StatusServiceUnavailable
+	}
+	writeHealthResponse(w, app.appConfig.CORSAllowedOrigin, status, health)
+}
+
+// ready reports whether the replica may take traffic: the stores are connected
+// and the first migration attempt has finished. A failed migration run stays
+// "degraded" in the body but is still ready. ensureDB never retries migrations,
+// so treating it as unready would pull every replica out of rotation after one
+// transient error (e.g. a lock timeout) with nothing to bring them back.
+func (h HealthStatus) ready() bool {
+	for name, check := range h.Checks {
+		if name == "migrations" {
+			if check.Status == "pending" {
+				return false
+			}
+			continue
+		}
+		if check.Status != "healthy" {
+			return false
+		}
+	}
+	return true
+}
+
+// collectHealth runs every readiness/health check and folds them into one
+// overall status. Shared by /health and /ready so the two endpoints can never
+// drift on what "initialized" means.
+func (app *Application) collectHealth(ctx context.Context) HealthStatus {
 	health := HealthStatus{
 		Status:    "healthy",
 		Version:   app.Version,
 		Timestamp: time.Now(),
 		Checks:    make(map[string]CheckResult),
 	}
 
-	// Check configuration store
-	health.Checks["config_store"] = app.checkConfigStore(ctx)
-	if health.Checks["config_store"].Status != "healthy" {
-		health.Status = "degraded"
-	}
+	// Check configuration and auth stores. ensureDB rewrites Config/Auth/DB
+	// under dbMu and holds it for the full initialization, migrations
+	// included; these endpoints must never block behind that. Snapshot the
+	// pointers when the mutex is free, and while initialization holds it
+	// report both stores as pending rather than race on half-written state.
+	if app.dbMu.TryLock() {
+		configStore, authService, db := app.Config, app.Auth, app.DB
+		app.dbMu.Unlock()
+
+		health.Checks["config_store"] = app.checkConfigStore(ctx, configStore, db)
+		if health.Checks["config_store"].Status != "healthy" {
+			health.Status = "degraded"
+		}
 
-	// Check auth store
-	health.Checks["auth_store"] = app.checkAuthStore(ctx)
-	if health.Checks["auth_store"].Status != "healthy" {
+		health.Checks["auth_store"] = app.checkAuthStore(ctx, authService)
+		if health.Checks["auth_store"].Status != "healthy" {
+			health.Status = "degraded"
+		}
+	} else {
+		pending := CheckResult{Status: "pending", Message: "database initialization in progress"}
+		health.Checks["config_store"] = pending
+		health.Checks["auth_store"] = pending
 		health.Status = "degraded"
 	}
 
@@ -55,14 +126,14 @@ func (app *Application) handleHealthCheck(w http.ResponseWriter, r *http.Request
 		health.Status = "degraded"
 	}
 
-	// Always return 200 for the health endpoint so startup/liveness probes pass.
-	// The actual health status is in the JSON body. "degraded" means the app is
-	// running but some dependencies (like DB) aren't connected yet - this is
-	// expected during cold starts with lazy DB initialization.
+	return health
+}
 
-	// Write response with security headers and CORS
-	setHealthResponseHeaders(w, app.appConfig.CORSAllowedOrigin)
-	w.WriteHeader(http.StatusOK)
+// writeHealthResponse encodes a health payload with the security headers and
+// CORS both health endpoints share.
+func writeHealthResponse(w http.ResponseWriter, corsOrigin string, status int, health HealthStatus) {
+	setHealthResponseHeaders(w, corsOrigin)
+	w.WriteHeader(status)
 	if err := json.NewEncoder(w).Encode(health); err != nil {
 		// Body write failed after headers are sent; log for parity with
 		// handleScheduledHTTP which already logs its encode error (04-L4).
@@ -98,10 +169,12 @@ func (app *Application) checkMigrations() CheckResult {
 	}
 }
 
-// checkConfigStore checks the health of the configuration store.
-func (app *Application) checkConfigStore(ctx context.Context) CheckResult {
+// checkConfigStore checks the health of the configuration store. The store and
+// connection come in as a snapshot taken under dbMu by collectHealth, so this
+// function never reads pointers ensureDB may be rewriting.
+func (app *Application) checkConfigStore(ctx context.Context, configStore config.StoreInterface, db *database.Connection) CheckResult {
 	// Check if config store exists
-	if app.Config == nil {
+	if configStore == nil {
 		// If using PostgreSQL with lazy initialization, DB might not be connected yet
 		if app.dbConfig != nil {
 			return CheckResult{
@@ -116,8 +189,8 @@ func (app *Application) checkConfigStore(ctx context.Context) CheckResult {
 	}
 
 	// If using PostgreSQL, check database connection health
-	if app.DB != nil {
-		if err := app.DB.HealthCheck(ctx); err != nil {
+	if db != nil {
+		if err := db.HealthCheck(ctx); err != nil {
 			return CheckResult{
 				Status:  "unhealthy",
 				Message: fmt.Sprintf("Database health check failed: %v", err),
@@ -154,17 +227,19 @@ func setHealthResponseHeaders(w http.ResponseWriter, corsOrigin string) {
 	}
 }
 
-// checkAuthStore checks the health of the auth store.
-func (app *Application) checkAuthStore(ctx context.Context) CheckResult {
-	if app.Auth == nil {
+// checkAuthStore checks the health of the auth store. The service comes in as
+// a snapshot taken under dbMu by collectHealth, so this function never reads a
+// pointer ensureDB may be rewriting.
+func (app *Application) checkAuthStore(ctx context.Context, authService *auth.Service) CheckResult {
+	if authService == nil {
 		return CheckResult{
 			Status:  "unhealthy",
 			Message: "Auth service not initialized",
 		}
 	}
 
 	// Ping the database to verify connection is healthy
-	if err := app.Auth.Ping(ctx); err != nil {
+	if err := authService.Ping(ctx); err != nil {
 		return CheckResult{
 			Status:  "unhealthy",
 			Message: fmt.Sprintf("Auth store ping failed: %v", err),

internal/server/health_test.go

@@ -378,7 +378,7 @@ func TestCheckConfigStore(t *testing.T) {
 				tt.setupApp(app)
 			}
 
-			result := app.checkConfigStore(ctx)
+			result := app.checkConfigStore(ctx, app.Config, app.DB)
 			testutil.AssertEqual(t, tt.expectedStatus, result.Status)
 		})
 	}
@@ -414,7 +414,7 @@ func TestCheckAuthStore(t *testing.T) {
 				tt.setupApp(app)
 			}
 
-			result := app.checkAuthStore(ctx)
+			result := app.checkAuthStore(ctx, app.Auth)
 			testutil.AssertEqual(t, tt.expectedStatus, result.Status)
 		})
 	}