diff --git a/internal/jobs/deploy_lifecycle_coverage_test.go b/internal/jobs/deploy_lifecycle_coverage_test.go index 1dd4877..787c949 100644 --- a/internal/jobs/deploy_lifecycle_coverage_test.go +++ b/internal/jobs/deploy_lifecycle_coverage_test.go @@ -53,6 +53,7 @@ import ( "github.com/riverqueue/river" "github.com/riverqueue/river/rivertype" appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -66,16 +67,26 @@ import ( // fakeDeployStatusK8s satisfies deployStatusK8sProvider for the Work // reconciler tests. The map's key is the namespace + "|" + name; missing // entries return apierrors.IsNotFound so the reconciler maps to "stopped". +// +// jobs / jobErrOn are the same shape for the build-Job override path: by +// default GetBuildJob returns NotFound (the existing tests pre-date the +// Job override and expect the legacy Deployment-only behaviour), so the +// override only fires for tests that explicitly populate jobs. type fakeDeployStatusK8s struct { - objs map[string]*appsv1.Deployment - errOn map[string]error - callLog []string + objs map[string]*appsv1.Deployment + errOn map[string]error + jobs map[string]*batchv1.Job + jobErrOn map[string]error + callLog []string + jobCalls []string } func newFakeDeployStatusK8s() *fakeDeployStatusK8s { return &fakeDeployStatusK8s{ - objs: map[string]*appsv1.Deployment{}, - errOn: map[string]error{}, + objs: map[string]*appsv1.Deployment{}, + errOn: map[string]error{}, + jobs: map[string]*batchv1.Job{}, + jobErrOn: map[string]error{}, } } @@ -91,6 +102,18 @@ func (f *fakeDeployStatusK8s) GetDeployment(_ context.Context, ns, name string) return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "deployments"}, name) } +func (f *fakeDeployStatusK8s) GetBuildJob(_ context.Context, ns, name string) (*batchv1.Job, error) { + key := ns + "|" + name + f.jobCalls = append(f.jobCalls, key) + if err, ok := f.jobErrOn[key]; ok { + return nil, err + } + if j, ok := f.jobs[key]; ok { + return j, nil + } + return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "jobs"}, name) +} + // fakeAutopsyK8sCov is a copy of fakeAutopsyK8s from deploy_failure_autopsy_test.go // (kept duplicated so renaming the original doesn't break this file). type fakeAutopsyK8sCov struct { diff --git a/internal/jobs/deploy_status_reconcile.go b/internal/jobs/deploy_status_reconcile.go index e56bd5a..df3146f 100644 --- a/internal/jobs/deploy_status_reconcile.go +++ b/internal/jobs/deploy_status_reconcile.go @@ -82,12 +82,15 @@ import ( "github.com/google/uuid" "github.com/riverqueue/river" appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + + "instant.dev/worker/internal/metrics" ) // Reconciler tunables. The interval matches the periodic-job registration in @@ -147,6 +150,31 @@ const ( // provider. The worker derives the namespace from provider_id rather than // storing it on the deployments row. deployNamespacePrefix = "instant-deploy-" + + // buildJobNamePrefix mirrors the api's k8s.buildImage() jobName format: + // jobName := "build-" + sanitizeName(appID) + // (api/internal/providers/compute/k8s/client.go ~L1390 / L1217). + // + // The build runs as a `batchv1.Job` named `build-` in the same + // per-deployment namespace (`instant-deploy-`) as the runtime + // `appsv1.Deployment`. A Job that hits its BackoffLimit (kaniko Dockerfile + // error) or its ActiveDeadlineSeconds (10 min wall-clock cap) marks itself + // `Failed` in its status BUT the runtime Deployment object is NEVER + // created — buildImage returns an error to runDeploy before the + // apply/rollout step runs. The pre-fix reconciler only queried the + // runtime Deployment, so a build-failed row reconciled to either + // `stopped` (Deployment NotFound — wrong, looks like a teardown) or stayed + // `building` forever (e.g. the api goroutine crashed mid-runDeploy and + // the row's terminal status write never landed). This was the silent- + // deploy-failure bug class (2026-05-30 user incident `truehomie-api-...`). + // + // The fix: after the Deployment query, ALWAYS query the build Job too. A + // Failed Job is authoritative — flip the row to `failed` regardless of + // what the runtime Deployment said. The Job's `TTLSecondsAfterFinished` + // (5 min in the api) keeps the Job object readable for a window even + // after k8s GCs the build pod, giving the reconciler a structured signal + // the pre-fix code missed. + buildJobNamePrefix = "build-" ) // DeployStatusReconcileArgs is the periodic-job payload. Empty — every run is @@ -165,6 +193,13 @@ type deployStatusK8sProvider interface { // when the namespace or Deployment has been deleted. The caller maps NotFound // to status="stopped". GetDeployment(ctx context.Context, namespace, name string) (*appsv1.Deployment, error) + + // GetBuildJob returns the live kaniko build Job, or apierrors.IsNotFound + // when the Job has been GC'd (past its TTLSecondsAfterFinished — 5 min in + // the api) or the namespace has been deleted. The caller inspects + // Status.Failed + JobConditions to detect a terminal build failure that + // the runtime-Deployment query alone cannot see. + GetBuildJob(ctx context.Context, namespace, name string) (*batchv1.Job, error) } // k8sDeployStatusClient is the concrete deployStatusK8sProvider implementation. @@ -179,6 +214,11 @@ func (c *k8sDeployStatusClient) GetDeployment(ctx context.Context, namespace, na return c.cs.AppsV1().Deployments(namespace).Get(ctx, name, metav1.GetOptions{}) } +// GetBuildJob implements deployStatusK8sProvider. +func (c *k8sDeployStatusClient) GetBuildJob(ctx context.Context, namespace, name string) (*batchv1.Job, error) { + return c.cs.BatchV1().Jobs(namespace).Get(ctx, name, metav1.GetOptions{}) +} + // NewK8sDeployStatusClient builds a deployStatusK8sProvider from in-cluster // config, falling back to the default kubeconfig for local dev. Returns nil // (and a non-nil error) when neither is reachable — the caller logs and @@ -434,6 +474,36 @@ var errSkipForeignProviderID = errors.New("provider_id not in app- shape; // and maps the result into the canonical status string set. NotFound (the // namespace or Deployment has been deleted out from under us) maps to // "stopped" — same as the api's k8s.Status() helper. +// +// JOB-FAILED OVERRIDE (silent-deploy-failure fix, 2026-05-30 incident): +// +// After the Deployment query, ALWAYS consult the kaniko build Job too. +// A Job in `Failed` phase (BackoffLimit exhausted, DeadlineExceeded, or any +// `Failed`-type condition) is authoritative — flip the row to `failed` +// regardless of what the runtime Deployment object reports. +// +// The pre-fix code only queried `appsv1.Deployments`; when the build Job +// crashed it did one of two equally-wrong things: +// +// 1. The runtime Deployment was never created (typical: buildImage errored +// before applyDeployment ran) → GetDeployment returned NotFound → mapped +// to `stopped`, a TERMINAL status that looks to the user like the deploy +// was torn down on purpose, with no autopsy and no failure surface. +// +// 2. The api goroutine crashed mid-runDeploy (pod OOM, ctx kill, etc.) and +// the row's terminal `failed` write never landed → the Deployment query +// might return any in-flight state and the row sat at `building` forever. +// +// In both cases the build Job's `Status.Failed > 0` OR a `JobCondition` of +// type `Failed` is the unambiguous evidence that a terminal build failure +// occurred. The Job's `TTLSecondsAfterFinished` (5 min in the api) means we +// can read the Job for a window even after the build pod is GC'd — exactly +// the gap the pre-fix code missed. +// +// The Job's NotFound result is NOT treated as a build success: it just means +// the Job has been reaped or never existed (Deployment-status path remains +// authoritative). Only a `Failed` Job overrides — `Succeeded` and `Active` +// states fall through to the Deployment-based mapping. func (w *DeployStatusReconciler) computeNewStatus(ctx context.Context, providerID string) (string, error) { ns := deployNamespaceFromProviderID(providerID) if ns == "" { @@ -445,20 +515,103 @@ func (w *DeployStatusReconciler) computeNewStatus(ctx context.Context, providerI getCtx, cancel := context.WithTimeout(ctx, k8sGetTimeout) defer cancel() - deploy, err := w.k8s.GetDeployment(getCtx, ns, providerID) - if apierrors.IsNotFound(err) { - // Namespace or Deployment is gone (manual cleanup, expiry sweep, - // teardown). Mark stopped so the row leaves the active set on - // the next sweep. - return deployStatusStopped, nil + deploy, deployErr := w.k8s.GetDeployment(getCtx, ns, providerID) + if deployErr != nil && !apierrors.IsNotFound(deployErr) { + // Transport-level k8s error on the Deployment query. Returning the + // error skips the row this tick — the Job-failed override is not a + // substitute for the row's healthy/deploying transitions, so a + // brownout should bubble up. + return "", deployErr } - if err != nil { - return "", err + + // Job-failed override: query the build Job before deciding the row's + // new status. A terminal Job failure is authoritative over whatever + // the runtime Deployment reports. + jobCtx, jobCancel := context.WithTimeout(ctx, k8sGetTimeout) + defer jobCancel() + appID := strings.TrimPrefix(providerID, providerIDPrefix) + jobName := buildJobNamePrefix + appID + job, jobErr := w.k8s.GetBuildJob(jobCtx, ns, jobName) + if jobErr != nil && !apierrors.IsNotFound(jobErr) { + // Don't fail the whole row on a Job-query brownout — log and fall + // through to the Deployment-based mapping. The next tick retries. + slog.Warn("jobs.deploy_status_reconcile.job_query_failed", + "namespace", ns, "job", jobName, "error", jobErr, + "note", "falling through to Deployment-based status — silent build-failure detection skipped this tick") + } else if jobErr == nil && jobIsFailed(job) { + // Job-failed override — authoritative. + metrics.DeployJobFailedDetectedTotal.WithLabelValues(jobFailureReason(job)).Inc() + return deployStatusFailed, nil + } + + if apierrors.IsNotFound(deployErr) { + // Deployment query: NotFound + Job-not-failed (Job missing, Active, or + // Succeeded). Two distinct cases: + // - Job NotFound + Deployment NotFound → namespace torn down → stopped. + // - Job Active + Deployment NotFound → build still running (pre-apply) + // → keep as "building" — Job-failed override caught the failure case. + if apierrors.IsNotFound(jobErr) { + return deployStatusStopped, nil + } + // Job exists and is not Failed — the build is still in flight or just + // succeeded but the Deployment apply hasn't landed yet. Hold at the + // row's current "building" status (deploymentStatusFromK8s returns + // building for an all-zero status, which is what a missing Deployment + // effectively represents at this stage). + return deployStatusBuilding, nil } return deploymentStatusFromK8s(deploy), nil } +// jobIsFailed reports whether a kaniko build Job has reached a terminal +// failure: either `Status.Failed > 0` (k8s incremented the failed-pod count +// past the BackoffLimit) OR a JobCondition of type `Failed` is present with +// status=True. Either condition is authoritative — the Job will not recover. +func jobIsFailed(job *batchv1.Job) bool { + if job == nil { + return false + } + for _, cond := range job.Status.Conditions { + if cond.Type == batchv1.JobFailed && cond.Status == corev1.ConditionTrue { + return true + } + } + // Only treat Failed>0 as terminal if BackoffLimit has been reached, since + // a transient pod failure during retries also bumps the counter. The + // JobFailed condition above is the primary signal; this is the backstop + // for cluster versions that surface Failed before stamping the condition. + backoffLimit := int32(0) + if job.Spec.BackoffLimit != nil { + backoffLimit = *job.Spec.BackoffLimit + } + return job.Status.Failed > backoffLimit +} + +// jobFailureReason picks a short bounded label for the +// instant_deploy_job_failed_detected_total counter from the Job's `Failed` +// condition. Falls back to "backoff_limit_exceeded" when the condition is +// absent but Status.Failed > BackoffLimit (the cluster-version backstop in +// jobIsFailed). +// +// Cardinality: k8s uses a small, stable set of Reason strings for JobFailed +// ("BackoffLimitExceeded", "DeadlineExceeded", "PodFailurePolicy"). We pass +// them through verbatim plus the fallback bucket. Bounded — safe to label. +func jobFailureReason(job *batchv1.Job) string { + if job == nil { + return "unknown" + } + for _, cond := range job.Status.Conditions { + if cond.Type == batchv1.JobFailed && cond.Status == corev1.ConditionTrue { + if cond.Reason != "" { + return cond.Reason + } + return "failed_no_reason" + } + } + return "backoff_limit_exceeded" +} + // deploymentStatusFromK8s mirrors api/internal/providers/compute/k8s/client.go's // deploymentStatus() helper. Kept verbatim here to guarantee the worker's // state machine matches what runDeploy() would write if it polled longer. diff --git a/internal/jobs/deploy_status_reconcile_job_failed_test.go b/internal/jobs/deploy_status_reconcile_job_failed_test.go new file mode 100644 index 0000000..1b64856 --- /dev/null +++ b/internal/jobs/deploy_status_reconcile_job_failed_test.go @@ -0,0 +1,361 @@ +package jobs + +// deploy_status_reconcile_job_failed_test.go — coverage for the silent-deploy- +// failure fix (2026-05-30 incident: a user's deploy sat at `building` forever +// because the kaniko build Job hit BackoffLimitExceeded and the build pod was +// GC'd before the pre-fix reconciler could observe a runtime Deployment). +// +// Two failure surfaces this file pins: +// +// 1. Reconciler MUST flip to `failed` when the build Job is Failed even +// after the pod is GC'd. The Job object survives its +// TTLSecondsAfterFinished window (5 min in the api) — long enough for +// the 30s reconciler tick to read it. +// +// 2. Reconciler MUST NOT flip prematurely while the Job still has retries +// left (Status.Failed <= BackoffLimit and no JobFailed condition). A +// Failed-pod count during retries is normal — the user incident WAS a +// terminal failure, not a transient retry. +// +// Mirrors fail-open posture: a transport-level Job-query error logs and +// falls through to the Deployment-based mapping, never blocks the row. + +import ( + "context" + "errors" + "testing" + + sqlmock "github.com/DATA-DOG/go-sqlmock" + "github.com/google/uuid" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" +) + +// newHealthyDeployment is a tiny helper that returns an appsv1.Deployment +// in the "healthy" shape (AvailableReplicas=1) used by the +// JobQueryError fall-through test. +func newHealthyDeployment() *appsv1.Deployment { + return &appsv1.Deployment{ + Status: appsv1.DeploymentStatus{AvailableReplicas: 1}, + } +} + +// newFakeClientsetForBuildJob returns a real kubernetes.Interface (the +// k8s fake clientset). Used by TestK8sDeployStatusClient_GetBuildJob_NotFoundPath +// to exercise the production wrapper's BatchV1 dispatch. +func newFakeClientsetForBuildJob() kubernetes.Interface { + return fake.NewSimpleClientset() +} + +// helper: a Job with a Failed condition stamped — the modal BackoffLimit case. +func jobBackoffLimitExceeded() *batchv1.Job { + bl := int32(2) + return &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "build-gced"}, + Spec: batchv1.JobSpec{BackoffLimit: &bl}, + Status: batchv1.JobStatus{ + Failed: 3, // BackoffLimit + 1 — k8s declared the Job dead. + Conditions: []batchv1.JobCondition{{ + Type: batchv1.JobFailed, + Status: corev1.ConditionTrue, + Reason: "BackoffLimitExceeded", + Message: "Job has reached the specified backoff limit", + }}, + }, + } +} + +// helper: a Job actively retrying — Failed count > 0 but BackoffLimit not yet reached. +func jobActiveRetrying() *batchv1.Job { + bl := int32(3) + return &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "build-active"}, + Spec: batchv1.JobSpec{BackoffLimit: &bl}, + Status: batchv1.JobStatus{ + Active: 1, // one pod currently running + Failed: 2, // <= BackoffLimit, still has retries + // No JobFailed condition yet. + }, + } +} + +// TestDeployStatusReconcile_JobFailedAfterPodGC is the PRIMARY guard for the +// silent-deploy-failure bug class. Setup mirrors the user's incident: +// +// - deployments row at status='building' (api goroutine crashed mid-build +// or never got to stamp the terminal status) +// - The runtime Deployment was never created (typical of build-time failure) +// → GetDeployment returns NotFound +// - The build Job's pod has been GC'd by k8s, but the Job object remains +// within its TTLSecondsAfterFinished window with Status.Failed=3 and a +// JobFailed condition stamped +// +// The pre-fix reconciler mapped this to `stopped` (terminal, but looks like a +// teardown). The fixed reconciler MUST flip the row to `failed`, enqueue an +// autopsy upsert (the existing in-sweep capture path), and increment the +// instant_deploy_job_failed_detected_total counter. +func TestDeployStatusReconcile_JobFailedAfterPodGC(t *testing.T) { + db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp)) + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + id := uuid.New() + mock.ExpectQuery(`FROM deployments\s+WHERE status IN`). + WillReturnRows(sqlmock.NewRows([]string{"id", "provider_id", "status"}). + AddRow(id, "app-gced", "building")) + + k8s := newFakeDeployStatusK8s() + // Deployment is missing (build never reached the apply step) — pre-fix + // behaviour mapped this to "stopped". + // Job exists with Failed condition — post-fix behaviour MUST detect + // this as a terminal build failure. + k8s.jobs["instant-deploy-gced|build-gced"] = jobBackoffLimitExceeded() + + // Expectations: autopsy upsert (kind=failure_autopsy) THEN status UPDATE to failed. + mock.ExpectExec(`INSERT INTO deployment_events`). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(`UPDATE deployments\s+SET status = \$1`). + WithArgs(deployStatusFailed, id, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(0, 1)) + + // Pass an empty autopsy stub so the in-sweep capture writes an Unknown- + // reason row (we only care about the status flip + autopsy fired). + w := NewDeployStatusReconciler(db, k8s).WithAutopsyK8s(&fakeAutopsyK8sCov{}) + if err := w.Work(context.Background(), fakeRiverJob[DeployStatusReconcileArgs]()); err != nil { + t.Fatalf("Work: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// TestDeployStatusReconcile_JobActiveAndPodMissing_StaysBuilding is the +// complement: a build still has retries left (Status.Failed <= BackoffLimit, +// no JobFailed condition). The reconciler MUST NOT flip the row to failed — +// the build may yet recover. The runtime Deployment is also missing (the +// apply step hasn't run yet because the build is in flight). Expected +// outcome: row stays at `building`; no UPDATE happens (newStatus == +// currentStatus). +func TestDeployStatusReconcile_JobActiveAndPodMissing_StaysBuilding(t *testing.T) { + db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp)) + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + id := uuid.New() + mock.ExpectQuery(`FROM deployments\s+WHERE status IN`). + WillReturnRows(sqlmock.NewRows([]string{"id", "provider_id", "status"}). + AddRow(id, "app-active", "building")) + + k8s := newFakeDeployStatusK8s() + // Deployment missing — apply step hasn't run yet. + // Job active — Failed=2, BackoffLimit=3 → still retrying. + k8s.jobs["instant-deploy-active|build-active"] = jobActiveRetrying() + + // No autopsy upsert expected. No UPDATE expected (newStatus stays + // "building" == currentStatus, so the sweep loop continues without + // writing). sqlmock will fail if an unexpected exec arrives. + + w := NewDeployStatusReconciler(db, k8s).WithAutopsyK8s(&fakeAutopsyK8sCov{}) + if err := w.Work(context.Background(), fakeRiverJob[DeployStatusReconcileArgs]()); err != nil { + t.Fatalf("Work: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// TestDeployStatusReconcile_BothNotFound_StaysStopped covers the legacy +// path: a row whose Deployment AND build Job have both been reaped (real +// teardown). MUST stay mapped to `stopped`. +func TestDeployStatusReconcile_BothNotFound_StaysStopped(t *testing.T) { + db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp)) + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + id := uuid.New() + mock.ExpectQuery(`FROM deployments\s+WHERE status IN`). + WillReturnRows(sqlmock.NewRows([]string{"id", "provider_id", "status"}). + AddRow(id, "app-gone", "building")) + + // Both Deployment AND Job missing from the fake → NewNotFound errors. + k8s := newFakeDeployStatusK8s() + + mock.ExpectExec(`UPDATE deployments\s+SET status = \$1`). + WithArgs(deployStatusStopped, id, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(0, 1)) + + w := NewDeployStatusReconciler(db, k8s) + if err := w.Work(context.Background(), fakeRiverJob[DeployStatusReconcileArgs]()); err != nil { + t.Fatalf("Work: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// TestDeployStatusReconcile_JobQueryError_FallsThroughToDeployment guards the +// fail-open posture: a transport-level error on the Job query MUST log+continue +// and let the Deployment-based mapping decide the row's status. The row in +// this test has a healthy Deployment so we should observe the legacy healthy +// transition even though the Job query failed. +func TestDeployStatusReconcile_JobQueryError_FallsThroughToDeployment(t *testing.T) { + db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp)) + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + id := uuid.New() + mock.ExpectQuery(`FROM deployments\s+WHERE status IN`). + WillReturnRows(sqlmock.NewRows([]string{"id", "provider_id", "status"}). + AddRow(id, "app-h1", "building")) + + k8s := newFakeDeployStatusK8s() + // Healthy runtime Deployment — Deployment query MUST be authoritative. + k8s.objs["instant-deploy-h1|app-h1"] = newHealthyDeployment() + // Build Job query returns a non-NotFound transport error. + k8s.jobErrOn["instant-deploy-h1|build-h1"] = errors.New("connection refused (mock)") + + mock.ExpectExec(`UPDATE deployments\s+SET status = \$1`). + WithArgs(deployStatusHealthy, id, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(0, 1)) + + w := NewDeployStatusReconciler(db, k8s) + if err := w.Work(context.Background(), fakeRiverJob[DeployStatusReconcileArgs]()); err != nil { + t.Fatalf("Work: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// TestJobIsFailed_Matrix covers the helper's predicate truth-table. We pin +// every branch (no conditions, Failed condition true/false, BackoffLimit +// exceeded via Status.Failed) so a future refactor that loosens the predicate +// (e.g. treats Active Failed-count as terminal) fails CI rather than +// reintroducing flapping false-positive flips. +func TestJobIsFailed_Matrix(t *testing.T) { + one := int32(1) + two := int32(2) + cases := []struct { + name string + job *batchv1.Job + want bool + }{ + {name: "nil job", job: nil, want: false}, + {name: "no conditions, no failed pods", job: &batchv1.Job{}, want: false}, + { + name: "Failed condition stamped True is failed", + job: &batchv1.Job{Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{{Type: batchv1.JobFailed, Status: corev1.ConditionTrue, Reason: "DeadlineExceeded"}}, + }}, + want: true, + }, + { + name: "Failed condition stamped False is not failed", + job: &batchv1.Job{Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{{Type: batchv1.JobFailed, Status: corev1.ConditionFalse}}, + }}, + want: false, + }, + { + name: "Status.Failed > BackoffLimit is failed (cluster-version backstop)", + job: &batchv1.Job{ + Spec: batchv1.JobSpec{BackoffLimit: &one}, + Status: batchv1.JobStatus{Failed: 2}, + }, + want: true, + }, + { + name: "Status.Failed == BackoffLimit is NOT failed (one retry left)", + job: &batchv1.Job{ + Spec: batchv1.JobSpec{BackoffLimit: &two}, + Status: batchv1.JobStatus{Failed: 2}, + }, + want: false, + }, + { + name: "Status.Failed > 0 with nil BackoffLimit is failed (default zero)", + job: &batchv1.Job{ + Status: batchv1.JobStatus{Failed: 1}, + }, + want: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := jobIsFailed(tc.job); got != tc.want { + t.Errorf("jobIsFailed = %v, want %v", got, tc.want) + } + }) + } +} + +// TestJobFailureReason covers the metrics-label helper. Bounded label cardinality +// is enforced by jobFailureReason's small return set; this test pins each branch. +func TestJobFailureReason(t *testing.T) { + cases := []struct { + name string + job *batchv1.Job + want string + }{ + {name: "nil job", job: nil, want: "unknown"}, + { + name: "Failed condition with reason", + job: &batchv1.Job{Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{{Type: batchv1.JobFailed, Status: corev1.ConditionTrue, Reason: "BackoffLimitExceeded"}}, + }}, + want: "BackoffLimitExceeded", + }, + { + name: "Failed condition without reason", + job: &batchv1.Job{Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{{Type: batchv1.JobFailed, Status: corev1.ConditionTrue}}, + }}, + want: "failed_no_reason", + }, + { + name: "no Failed condition (backstop bucket)", + job: &batchv1.Job{}, + want: "backoff_limit_exceeded", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := jobFailureReason(tc.job); got != tc.want { + t.Errorf("jobFailureReason = %q, want %q", got, tc.want) + } + }) + } +} + +// TestK8sDeployStatusClient_GetBuildJob_NotFoundPath proves the production +// wrapper around BatchV1().Jobs().Get() forwards NotFound errors verbatim. +// Uses a fake.Clientset (already imported in deploy_lifecycle_coverage_test.go) +// so we exercise the real BatchV1 dispatch path without a live cluster. +func TestK8sDeployStatusClient_GetBuildJob_NotFoundPath(t *testing.T) { + cs := newFakeClientsetForBuildJob() // empty fake → Get returns NotFound + c := &k8sDeployStatusClient{cs: cs} + _, err := c.GetBuildJob(context.Background(), "instant-deploy-x", "build-x") + if err == nil { + t.Fatal("expected NotFound error from empty fake clientset") + } + if !apierrors.IsNotFound(err) { + t.Errorf("expected apierrors.IsNotFound, got %v", err) + } +} + +// ensure helper imports are used. +var _ = schema.GroupResource{} diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index cdecf36..f45dcfa 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -534,6 +534,46 @@ var ( Help: "Per-component readiness status (1=ok, 0.5=degraded, 0=failed). Set by /readyz on every probe.", }, []string{"service", "check"}) + // ── DeployStatusReconciler — Job-failed override (silent-deploy-failure fix) ─ + // + // Increments every time the reconciler detects a kaniko build Job in the + // `Failed` state (BackoffLimit exhausted, ActiveDeadlineSeconds exceeded, + // or any JobCondition of type Failed). The pre-fix reconciler only + // queried the runtime appsv1.Deployment and missed this whole class of + // failure (2026-05-30 incident: a user's deploy sat at `building` forever + // because the build pod was GC'd and there was no Deployment object to + // query). This counter is the leading indicator that the Job-query + // override is doing its job; pair with `instant_deploy_autopsy_captured_total` + // to see the autopsy follow-through. + // + // Labels: + // reason — the Job's `Failed` condition reason verbatim. k8s uses a + // small, stable set: "BackoffLimitExceeded", "DeadlineExceeded", + // "PodFailurePolicy". Plus two bounded fallbacks set in + // jobFailureReason: "failed_no_reason" (condition present but + // no reason string) and "backoff_limit_exceeded" (cluster- + // version backstop — JobFailed condition not stamped but + // Status.Failed > BackoffLimit). + // + // NR alert (suggested): + // sum(rate(instant_deploy_job_failed_detected_total[15m])) by (reason) > 0.5 + // for 30+ minutes → P2 page. A sustained rate of + // reason="DeadlineExceeded" means the platform's kaniko build slot is + // timing out for many tenants (image bloat or a degraded GHCR push + // path); reason="BackoffLimitExceeded" is the modal Dockerfile-error + // bucket — alert at a higher threshold or visualize on the dashboard + // only. + // + // Catalog row (infra/observability/METRICS-CATALOG.md): + // instant_deploy_job_failed_detected_total | counter | reason | lazy + // (first observation is a real Job-failed detection — does not appear at + // /metrics until then; the test in metrics_test.go forces a label so the + // metric is registered at process start). + DeployJobFailedDetectedTotal = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "instant_deploy_job_failed_detected_total", + Help: "Kaniko build Jobs detected in Failed state by deploy_status_reconcile (silent-deploy-failure fix, 2026-05-30). Labelled by Job Failed-condition reason.", + }, []string{"reason"}) + // ── orphan_sweep_reconciler — reap counters (2026-05-20) ────────────────── // // Every namespace / DB row the orphan-sweep reconciler reaps (or flips to diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 44beefe..0e8c5f2 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -94,6 +94,7 @@ func TestAllMetrics_AreRegistered(t *testing.T) { PropagationUnknownKindTotal.WithLabelValues("unknown").Add(0) OrphanSweepReapedTotal.WithLabelValues("team_tombstoned").Add(0) OrphanSweepReapFailedTotal.WithLabelValues("team_tombstoned").Add(0) + DeployJobFailedDetectedTotal.WithLabelValues("BackoffLimitExceeded").Add(0) // Gauge vecs ResourceDegradedGauge.WithLabelValues("postgres").Set(0)