fix(gcp): exclude uninitialized replicas from GKE and Cloud Run readiness

.github/workflows/deploy-gcp.yml

@@ -272,49 +272,39 @@ jobs:
           echo "Waiting 30 seconds for Cloud Run 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, 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 the instance is still initializing, which the
+      # script counts as a failed attempt. The old inline gate warmed up
+      # /api/auth/check-admin and then parsed /health, and the two requests
+      # could land on different instances (#648).
+      - 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 an instance of the new
+      # revision that is still initializing, 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
-        run: |
-          URL="${{ steps.get-url.outputs.url }}"
-
-          echo "Running basic smoke tests..."
-
-          # Test health endpoint multiple times
-          for i in {1..3}; do
-            if RESPONSE=$(curl -fsS --max-time 10 "$URL/health") &&
-              jq -e '.status == "healthy"' > /dev/null <<< "$RESPONSE"; then
-              echo "Health check $i: passed"
-            else
-              echo "Health check $i: failed: ${RESPONSE:-no response}"
-              exit 1
-            fi
-            sleep 2
-          done
-
-          echo "All smoke tests passed!"
+        env:
+          APP_URL: ${{ steps.get-url.outputs.url }}
+          HEALTH_REQUIRED_STREAK: "3"
+          HEALTH_MAX_ATTEMPTS: "30"
+          HEALTH_INTERVAL_SECONDS: "5"
+        run: bash scripts/wait-for-healthy.sh "$APP_URL"
 
   # Post deployment summary
   summary:

terraform/modules/compute/gcp/cloud-run/main.tf

@@ -182,16 +182,24 @@ resource "google_cloud_run_v2_service" "main" {
         }
       }
 
-      # Startup probe
+      # Startup probe. Cloud Run admits an instance to traffic once its
+      # startup probe passes, so this is the readiness gate: /ready answers
+      # 503 until the instance has completed database initialization, so a
+      # cold instance is never admitted (#648, #488). A startup probe that
+      # never passes shuts the container down, so the failure budget must
+      # outlast a slow-but-recovering init: 10 s delay + 24 x 10 s = 250 s,
+      # above the 120 s default migration timeout (CUDLY_MIGRATION_TIMEOUT)
+      # plus database connect retries, and inside Cloud Run's 600 s cap on
+      # failure_threshold x period_seconds. Liveness below stays on /health.
       startup_probe {
         http_get {
-          path = "/health"
+          path = "/ready"
           port = 8080
         }
         initial_delay_seconds = 10
         timeout_seconds       = 3
         period_seconds        = 10
-        failure_threshold     = 3
+        failure_threshold     = 24
       }
 
       # Liveness probe

terraform/modules/compute/gcp/gke/main.tf

@@ -595,9 +595,16 @@ resource "kubernetes_deployment" "app" {
             failure_threshold     = 3
           }
 
+          # Readiness is the traffic-admission signal: /ready answers 503
+          # until this replica has completed database initialization, so a
+          # cold pod stays out of the Service endpoints (and out of the GCE
+          # Ingress NEG, whose health check GKE infers from this probe when
+          # the Ingress is created) (#648, #488). Liveness above stays on
+          # /health, which answers 200 while the process is alive, so a slow
+          # initialization never gets the pod killed.
           readiness_probe {
             http_get {
-              path = var.health_check_path
+              path = var.readiness_probe_path
               port = var.health_check_port
             }
             initial_delay_seconds = var.readiness_probe_initial_delay

terraform/modules/compute/gcp/gke/variables.tf

@@ -216,11 +216,22 @@ variable "deploy_kubernetes_resources" {
 # ==============================================
 
 variable "health_check_path" {
-  description = "HTTP path for health checks"
+  description = "HTTP path for the liveness probe. /health answers 200 while the process is alive, even before initialization completes, so a slow init does not get the pod restarted."
   type        = string
   default     = "/health"
 }
 
+variable "readiness_probe_path" {
+  description = "HTTP path for the readiness probe (traffic admission). /ready answers 503 until the replica has completed initialization, keeping cold pods out of the Service endpoints (#648)."
+  type        = string
+  default     = "/ready"
+
+  validation {
+    condition     = startswith(var.readiness_probe_path, "/")
+    error_message = "readiness_probe_path must be an absolute HTTP path starting with \"/\"."
+  }
+}
+
 variable "health_check_port" {
   description = "Port for health checks"
   type        = number

terraform/modules/compute/gcp/readiness_probe_test.go

@@ -0,0 +1,128 @@
+// Regression test for issue #648: after #488 added /ready (503 until the
+// replica has completed initialization), the GKE readiness probe and the
+// Cloud Run startup probe still gated traffic on /health, which answers 200
+// even while the replica is cold, so uninitialized replicas were admitted to
+// traffic. Liveness must stay on /health so a slow init never gets the
+// container killed.
+package gcp_test
+
+import (
+	"os"
+	"regexp"
+	"strconv"
+	"testing"
+)
+
+func readTF(t *testing.T, path string) string {
+	t.Helper()
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("reading %s: %v", path, err)
+	}
+	return string(data)
+}
+
+// probeBlock returns the body of the first `<name> { ... }` block, matching
+// braces so nested http_get blocks are included.
+func probeBlock(t *testing.T, content, name string) string {
+	t.Helper()
+	loc := regexp.MustCompile(`(?m)^\s*` + name + `\s*\{`).FindStringIndex(content)
+	if loc == nil {
+		t.Fatalf("no %s block found", name)
+	}
+	depth := 0
+	for i := loc[1] - 1; i < len(content); i++ {
+		switch content[i] {
+		case '{':
+			depth++
+		case '}':
+			depth--
+			if depth == 0 {
+				return content[loc[1]:i]
+			}
+		}
+	}
+	t.Fatalf("unterminated %s block", name)
+	return ""
+}
+
+func blockPath(block string) string {
+	m := regexp.MustCompile(`path\s*=\s*(\S+)`).FindStringSubmatch(block)
+	if m == nil {
+		return ""
+	}
+	return m[1]
+}
+
+func blockInt(t *testing.T, block, attr string) int {
+	t.Helper()
+	m := regexp.MustCompile(attr + `\s*=\s*(\d+)`).FindStringSubmatch(block)
+	if m == nil {
+		t.Fatalf("probe block has no numeric %s", attr)
+	}
+	n, err := strconv.Atoi(m[1])
+	if err != nil {
+		t.Fatalf("parsing %s: %v", attr, err)
+	}
+	return n
+}
+
+func variableDefault(t *testing.T, vars, name string) string {
+	t.Helper()
+	block := regexp.MustCompile(`(?s)variable\s+"` + name + `"\s*\{.*?\n\}`).FindString(vars)
+	if block == "" {
+		t.Fatalf("variable %q is not declared", name)
+	}
+	m := regexp.MustCompile(`default\s*=\s*"([^"]*)"`).FindStringSubmatch(block)
+	if m == nil {
+		t.Fatalf("variable %q has no string default", name)
+	}
+	return m[1]
+}
+
+func TestGKEReadinessProbeGatesOnReady(t *testing.T) {
+	main := readTF(t, "gke/main.tf")
+	vars := readTF(t, "gke/variables.tf")
+
+	if got := blockPath(probeBlock(t, main, "readiness_probe")); got != "var.readiness_probe_path" {
+		t.Errorf("GKE readiness_probe path = %s, want var.readiness_probe_path (#648): readiness is the traffic-admission signal and must not share the liveness path", got)
+	}
+	if got := variableDefault(t, vars, "readiness_probe_path"); got != "/ready" {
+		t.Errorf(`var.readiness_probe_path default = %q, want "/ready" (#648); /health answers 200 on a cold replica`, got)
+	}
+
+	if got := blockPath(probeBlock(t, main, "liveness_probe")); got != "var.health_check_path" {
+		t.Errorf("GKE liveness_probe path = %s, want var.health_check_path: liveness must stay on /health so a slow init does not restart the pod", got)
+	}
+	if got := variableDefault(t, vars, "health_check_path"); got != "/health" {
+		t.Errorf(`var.health_check_path default = %q, want "/health": pointing liveness at /ready would kill pods whose init is slow`, got)
+	}
+}
+
+func TestCloudRunStartupProbeGatesOnReady(t *testing.T) {
+	main := readTF(t, "cloud-run/main.tf")
+
+	startup := probeBlock(t, main, "startup_probe")
+	if got := blockPath(startup); got != `"/ready"` {
+		t.Errorf(`Cloud Run startup_probe path = %s, want "/ready" (#648): Cloud Run admits an instance once its startup probe passes`, got)
+	}
+
+	// A startup probe that never passes shuts the container down, so the
+	// window must outlast the 120 s default migration timeout plus connect
+	// retries, and stay inside Cloud Run's 600 s cap on
+	// failure_threshold x period_seconds.
+	window := blockInt(t, startup, "failure_threshold") * blockInt(t, startup, "period_seconds")
+	if window < 180 {
+		t.Errorf("Cloud Run startup probe window = %d s, want >= 180 s so a slow-but-recovering init does not restart-loop the container", window)
+	}
+	if window > 600 {
+		t.Errorf("Cloud Run startup probe window = %d s, exceeds Cloud Run's 600 s limit on failure_threshold x period_seconds", window)
+	}
+	if timeout, period := blockInt(t, startup, "timeout_seconds"), blockInt(t, startup, "period_seconds"); timeout > period {
+		t.Errorf("Cloud Run startup probe timeout_seconds (%d) must not exceed period_seconds (%d)", timeout, period)
+	}
+
+	if got := blockPath(probeBlock(t, main, "liveness_probe")); got != `"/health"` {
+		t.Errorf(`Cloud Run liveness_probe path = %s, want "/health": liveness must not depend on initialization`, got)
+	}
+}