Skip to content

Commit 5a7f9ff

Browse files
feat(worker): e2e_cohort_sweep — purge stale is_test_cohort teams
Belt-and-suspenders reaper for STALE synthetic test-cohort teams (teams.is_test_cohort=true, api mig 067) leaked by a CI E2E run that died before deleting its own ephemeral /internal/e2e/account (api #260). - New River periodic job `e2e_cohort_sweep`, hourly, on the reconcile queue, RunOnStart=true. Finds is_test_cohort teams older than a 2h TTL (CI runs are minutes) not already terminal/destroying, capped at 25/run. - Reuses the team-deletion executor's idempotent per-team teardown (the same processTeam the orphan-sweep reconciler reuses): flip to deletion_pending, deprovision resources + delete k8s namespaces + scrub PII + tombstone. No second copy of the purge cascade. - NEVER touches a non-cohort team: the candidate query filters on is_test_cohort=true, AND each candidate is re-checked with isTestCohort before purge, AND the deletion_pending UPDATE re-guards on the flag. Inert in practice — no real team is is_test_cohort=true. - Fail-safe: nil executor → WARN no-op; per-team failure isolated (stays deletion_pending for next tick / orphan-sweep PASS 1); top-level query error returns so River retries. - Observability (rule 25): metric instant_e2e_cohort_swept_total{outcome} (swept/failed/skipped_not_cohort) with NR alert PromQL documented inline; INFO on work, DEBUG on idle tick. - Tests: sqlmock unit tests for every outcome arm (nil-executor, query error, scan error, idle tick, happy-path, recheck-not-cohort blocked, recheck error, 0-row flip, flip error, executor error) + a DB-backed integration test proving ONLY the stale cohort team is swept while a fresh (within-TTL) cohort team AND a normal non-cohort team are left fully intact. New testhelpers (SeedCohortTeamWithAge, ClearPreexistingTestCohortTeams) carry in-package smoke coverage. 100% patch coverage on the new production code; make gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 39ff054 commit 5a7f9ff

9 files changed

Lines changed: 842 additions & 3 deletions

File tree

internal/jobs/e2e_cohort_sweep.go

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
package jobs
2+
3+
// e2e_cohort_sweep.go — belt-and-suspenders reaper for STALE synthetic
4+
// test-cohort teams.
5+
//
6+
// PROBLEM SHAPE
7+
//
8+
// CI's E2E suite mints an ephemeral team via the api's /internal/e2e/account
9+
// endpoint (api PR #260). Every such team is tagged teams.is_test_cohort=true
10+
// (api migration 067). The happy path is that the SAME CI run deletes its
11+
// account at the end of the run — but a CI run that crashes, times out, or is
12+
// hard-cancelled mid-flight dies BEFORE it reaps its account. The leftover
13+
// is_test_cohort team then lingers: it holds real provisioned resources +
14+
// k8s namespaces, and it is a stray row in every operator view.
15+
//
16+
// THIS JOB is the durable backstop. Every cohortSweepInterval it finds
17+
// is_test_cohort teams older than cohortSweepTTL that are not already in a
18+
// terminal/destroying state, and purges each one the SAME way the team-
19+
// deletion executor (team_deletion_executor.go) and the api reap path do:
20+
// flip the team to deletion_pending, then run the executor's idempotent
21+
// per-team teardown (deprovision resources, delete k8s namespaces, scrub
22+
// PII, tombstone). It reuses the executor wholesale — no second copy of the
23+
// purge cascade.
24+
//
25+
// SAFETY — this NEVER touches a non-cohort team
26+
//
27+
// The candidate query filters on `is_test_cohort = true`, so by construction
28+
// only cohort teams are ever selected. As a defence in depth (the WHERE
29+
// clause is the single point of failure for "did we pick the right rows"),
30+
// each candidate is RE-CHECKED with isTestCohort() immediately before the
31+
// purge; a row that is no longer is_test_cohort=true is skipped and counted
32+
// under outcome="skipped_not_cohort" (an alert-able disagreement between the
33+
// candidate query and the guard). No real team is is_test_cohort=true today
34+
// (migration 067 defaults every row to false), so in practice this job is
35+
// inert — it only ever matches a future synthetic/CI cohort row.
36+
//
37+
// SHORT TTL
38+
//
39+
// CI runs complete in minutes, so cohortSweepTTL is deliberately short
40+
// (2h). That is long enough that a still-running CI job's freshly-created
41+
// account is never reaped out from under it, yet short enough that a leaked
42+
// account is reclaimed the same hour. The TTL is a named const so the
43+
// integration test can reason about "stale vs fresh" without magic numbers.
44+
//
45+
// IDEMPOTENCY + FAIL-SAFE
46+
//
47+
// processTeam is fully idempotent (a re-run over an already-tombstoned team
48+
// is a no-op). A per-team failure is isolated: it stays in deletion_pending
49+
// for the next tick (or the orphan-sweep reconciler's PASS 1), is counted
50+
// under outcome="failed", and never stops the sweep from attempting the next
51+
// team. A nil executor disables the job with a single WARN (matching every
52+
// other worker's fail-open posture). A top-level candidate-query failure
53+
// returns an error so River retries the dispatch.
54+
55+
import (
56+
"context"
57+
"database/sql"
58+
"fmt"
59+
"log/slog"
60+
"time"
61+
62+
"github.com/riverqueue/river"
63+
"go.opentelemetry.io/otel"
64+
65+
"instant.dev/worker/internal/metrics"
66+
)
67+
68+
// cohortSweepInterval is how often the job runs. Hourly is the right cadence
69+
// for a CI-account backstop: CI runs finish in minutes, so a leaked account
70+
// older than the (also short) TTL is reclaimed within the hour, while an
71+
// hourly cluster-wide query over the teams table is negligible load.
72+
const cohortSweepInterval = 1 * time.Hour
73+
74+
// cohortSweepTTL is how old an is_test_cohort team must be before this job
75+
// will purge it. Short on purpose — CI runs are minutes, so 2h comfortably
76+
// covers the longest legitimate E2E run without ever reaping an account a
77+
// still-running CI job is actively using.
78+
const cohortSweepTTL = 2 * time.Hour
79+
80+
// cohortSweepBatchLimit caps how many stale cohort teams the job purges per
81+
// run. A healthy steady-state has zero (CI reaps its own accounts); a backlog
82+
// (a CI outage that leaked many runs) is drained a batch per tick rather than
83+
// all at once — the per-team teardown does real deprovision/k8s work.
84+
const cohortSweepBatchLimit = 25
85+
86+
// E2E cohort-sweep outcome labels for instant_e2e_cohort_swept_total. Bounded
87+
// set — every per-team decision picks exactly one. The literal strings are
88+
// part of the metric contract (see metrics.go E2ECohortSweptTotal).
89+
const (
90+
cohortSweepOutcomeSwept = "swept"
91+
cohortSweepOutcomeFailed = "failed"
92+
cohortSweepOutcomeSkippedNotCohort = "skipped_not_cohort"
93+
)
94+
95+
// E2ECohortSweepArgs is the periodic-job payload — empty, every run is a full
96+
// sweep.
97+
type E2ECohortSweepArgs struct{}
98+
99+
// Kind implements river.JobArgs.
100+
func (E2ECohortSweepArgs) Kind() string { return "e2e_cohort_sweep" }
101+
102+
// E2ECohortSweepWorker is the River worker.
103+
type E2ECohortSweepWorker struct {
104+
river.WorkerDefaults[E2ECohortSweepArgs]
105+
db *sql.DB
106+
executor teamTeardownExecutor // nil = job skipped (fail open)
107+
}
108+
109+
// NewE2ECohortSweepWorker constructs the worker. executor may be nil — the
110+
// job then logs a single WARN per tick and reaps nothing, matching the fail-
111+
// open posture of every other worker. In production it is the same
112+
// *TeamDeletionExecutorWorker the orphan-sweep reconciler reuses.
113+
func NewE2ECohortSweepWorker(db *sql.DB, executor teamTeardownExecutor) *E2ECohortSweepWorker {
114+
return &E2ECohortSweepWorker{db: db, executor: executor}
115+
}
116+
117+
// Work runs one full sweep.
118+
func (w *E2ECohortSweepWorker) Work(ctx context.Context, job *river.Job[E2ECohortSweepArgs]) error {
119+
ctx, span := otel.Tracer("instant.dev/worker").Start(ctx, "job.e2e_cohort_sweep")
120+
defer span.End()
121+
122+
start := time.Now()
123+
124+
if w.executor == nil {
125+
slog.Warn("jobs.e2e_cohort_sweep.skipped_no_executor",
126+
"detail", "team-deletion executor unavailable — stale CI cohort teams will not be reaped this tick")
127+
return nil
128+
}
129+
130+
candidates, err := w.fetchStaleCohortTeams(ctx)
131+
if err != nil {
132+
return fmt.Errorf("E2ECohortSweepWorker: candidate query failed: %w", err)
133+
}
134+
135+
var swept, failed, skipped int
136+
for _, c := range candidates {
137+
switch w.purgeTeam(ctx, c) {
138+
case cohortSweepOutcomeSwept:
139+
swept++
140+
case cohortSweepOutcomeFailed:
141+
failed++
142+
case cohortSweepOutcomeSkippedNotCohort:
143+
skipped++
144+
}
145+
}
146+
147+
// Idle-tick noise discipline (worker convention #1): an all-zero sweep is
148+
// the steady state (CI reaps its own accounts) — DEBUG. Any non-zero
149+
// counter is operational signal — INFO.
150+
level := slog.LevelInfo
151+
if swept == 0 && failed == 0 && skipped == 0 {
152+
level = slog.LevelDebug
153+
}
154+
slog.Log(ctx, level, "jobs.e2e_cohort_sweep.completed",
155+
"swept", swept,
156+
"failed", failed,
157+
"skipped_not_cohort", skipped,
158+
"ttl", cohortSweepTTL.String(),
159+
"duration_ms", time.Since(start).Milliseconds(),
160+
"job_id", job.ID,
161+
)
162+
return nil
163+
}
164+
165+
// fetchStaleCohortTeams returns every is_test_cohort team older than the TTL
166+
// that is not already in a terminal/destroying state. Capped by
167+
// cohortSweepBatchLimit.
168+
//
169+
// status filter: a team already 'tombstoned' is done; one in
170+
// 'deletion_pending' is mid-teardown (the orphan-sweep reconciler's PASS 1
171+
// owns it). We sweep everything else — 'active' (the normal leaked-CI-account
172+
// case) and 'deletion_requested' (a CI run that asked to delete but died
173+
// before the 30-day executor ran; for a CI account we skip the grace).
174+
func (w *E2ECohortSweepWorker) fetchStaleCohortTeams(ctx context.Context) ([]teamPendingDeletion, error) {
175+
cutoff := time.Now().Add(-cohortSweepTTL)
176+
rows, err := w.db.QueryContext(ctx, fmt.Sprintf(`
177+
SELECT id, COALESCE(deletion_requested_at, created_at, now())
178+
FROM teams
179+
WHERE is_test_cohort = true
180+
AND created_at < $1
181+
AND status NOT IN ('tombstoned', 'deletion_pending')
182+
ORDER BY created_at ASC
183+
LIMIT %d
184+
`, cohortSweepBatchLimit), cutoff)
185+
if err != nil {
186+
return nil, err
187+
}
188+
defer func() { _ = rows.Close() }()
189+
190+
var out []teamPendingDeletion
191+
for rows.Next() {
192+
var c teamPendingDeletion
193+
if scanErr := rows.Scan(&c.teamID, &c.deletionRequestedAt); scanErr != nil {
194+
return nil, fmt.Errorf("scan stale cohort team: %w", scanErr)
195+
}
196+
out = append(out, c)
197+
}
198+
return out, rows.Err()
199+
}
200+
201+
// purgeTeam runs the per-team purge and returns the outcome label. It first
202+
// re-asserts the team is still is_test_cohort (defence in depth — the WHERE
203+
// clause already filtered, but a flip between the candidate SELECT and here
204+
// must NEVER let the destructive path touch a non-cohort team), then flips the
205+
// team to deletion_pending and runs the executor's idempotent teardown.
206+
func (w *E2ECohortSweepWorker) purgeTeam(ctx context.Context, c teamPendingDeletion) string {
207+
// DEFENSIVE GUARD. isTestCohort fails SAFE-FOR-REAL-TEAMS: on any DB
208+
// error it returns (false, err), and we then SKIP rather than purge — a
209+
// destructive teardown must never run on an unconfirmed row. Only a row
210+
// confirmed is_test_cohort=true proceeds.
211+
flagged, err := isTestCohort(ctx, w.db, c.teamID)
212+
if err != nil {
213+
slog.Warn("jobs.e2e_cohort_sweep.cohort_recheck_failed",
214+
"team_id", c.teamID.String(), "error", err,
215+
"detail", "could not confirm is_test_cohort at purge time — skipping (never purge an unconfirmed team)")
216+
metrics.E2ECohortSweptTotal.WithLabelValues(cohortSweepOutcomeSkippedNotCohort).Inc()
217+
return cohortSweepOutcomeSkippedNotCohort
218+
}
219+
if !flagged {
220+
// The candidate query selected a row the guard says is NOT cohort —
221+
// a disagreement the alert keys on. Skip; never purge a non-cohort team.
222+
slog.Error("jobs.e2e_cohort_sweep.non_cohort_candidate_blocked",
223+
"team_id", c.teamID.String(),
224+
"detail", "candidate query selected a team that is not is_test_cohort — purge blocked by defensive guard; candidate query may have regressed")
225+
metrics.E2ECohortSweptTotal.WithLabelValues(cohortSweepOutcomeSkippedNotCohort).Inc()
226+
return cohortSweepOutcomeSkippedNotCohort
227+
}
228+
229+
// Flip to deletion_pending so processTeam's idempotent teardown tombstones
230+
// it (the executor's step-0 flip only fires from 'deletion_requested'; for
231+
// a CI account we skip the 30-day grace and set the in-flight state here).
232+
// Guarded on the cohort flag a SECOND time inside the UPDATE so even a TOCTOU
233+
// flip between the SELECT/recheck and this write cannot mark a non-cohort
234+
// team for destruction.
235+
res, err := w.db.ExecContext(ctx, `
236+
UPDATE teams
237+
SET status = 'deletion_pending'
238+
WHERE id = $1
239+
AND is_test_cohort = true
240+
AND status NOT IN ('tombstoned', 'deletion_pending')
241+
`, c.teamID)
242+
if err != nil {
243+
slog.Error("jobs.e2e_cohort_sweep.mark_deletion_pending_failed",
244+
"team_id", c.teamID.String(), "error", err)
245+
metrics.E2ECohortSweptTotal.WithLabelValues(cohortSweepOutcomeFailed).Inc()
246+
return cohortSweepOutcomeFailed
247+
}
248+
if n, _ := res.RowsAffected(); n == 0 {
249+
// The row changed state underneath us (already pending/tombstoned, or
250+
// the cohort flag cleared). Re-check guard already passed, so this is a
251+
// benign race — count as skipped, not failed.
252+
slog.Warn("jobs.e2e_cohort_sweep.mark_deletion_pending_noop",
253+
"team_id", c.teamID.String(),
254+
"detail", "team changed state between candidate select and purge — skipping this tick")
255+
metrics.E2ECohortSweptTotal.WithLabelValues(cohortSweepOutcomeSkippedNotCohort).Inc()
256+
return cohortSweepOutcomeSkippedNotCohort
257+
}
258+
259+
if perr := w.executor.processTeam(ctx, c); perr != nil {
260+
slog.Error("jobs.e2e_cohort_sweep.purge_failed",
261+
"team_id", c.teamID.String(), "error", perr,
262+
"detail", "team stays in deletion_pending for next tick / orphan-sweep PASS 1")
263+
metrics.E2ECohortSweptTotal.WithLabelValues(cohortSweepOutcomeFailed).Inc()
264+
return cohortSweepOutcomeFailed
265+
}
266+
267+
slog.Info("jobs.e2e_cohort_sweep.team_swept",
268+
"team_id", c.teamID.String(),
269+
"detail", "stale CI test-cohort team purged + tombstoned (belt-and-suspenders backstop)")
270+
metrics.E2ECohortSweptTotal.WithLabelValues(cohortSweepOutcomeSwept).Inc()
271+
return cohortSweepOutcomeSwept
272+
}

0 commit comments

Comments
 (0)