Schedule the deletion drain safely

[TheSentinel454]

Why

The deletion engine already exposes a one-shot buzz-admin deletions drain command, but operators have no safe way to schedule it. A generic workload wrapper could expose arbitrary commands or inherit relay credentials, which would add a second control plane and widen the blast radius.

What

This PR adds an opt-in Kubernetes CronJob for the typed deletion drain command. It is stacked on #7818, which adds owner-request admission.

How

The chart keeps the job disabled by default. The CronJob uses Forbid, zero Kubernetes retries, bounded runtime and history, and only the database, Redis, and object-store settings required by the executor. PostgreSQL leases, checkpoints, and retry timing remain the sole durable retry authority.

The closed _operator-jobs.tpl helper centralizes safe CronJob mechanics without creating an arbitrary command or environment registry. This leaves the touched system simpler than a general job framework would.

Risk

The deployment blast radius is low because the job is disabled by default. When enabled, it can run destructive deletion work that the database has already approved, so the chart limits concurrency, retries, credentials, and execution time.

Testing

Rendered every deploy/charts/buzz/ci/*.yaml fixture with helm template; all passed at e68cb753bb6673aa1baa60046969bdb2d4738b62.

Ran the full independent Desktop, Tauri, and web lanes; all passed at e68cb753bb6673aa1baa60046969bdb2d4738b62. Flutter package resolution made no progress within three minutes, so the mobile lane is not claimed green.

Bigger picture

A later stack layer must inventory and approve admitted owner requests. Until then, this scheduler cannot make a new owner request destructive.

Generated with Codex

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No new blocking defect found in the incremental scheduler at b79bd19c73b188d9fb785cbcd5ba50ccad74b295, against acf561219f4223bc6090ce5c82555ee294d11ea4. This is a source review, not an approval.

The disabled-by-default, fixed-command job preserves the existing approval boundary and PostgreSQL lease/checkpoint authority without exposing a generic command or signing-secret environment. Two non-blocking runbook corrections are inline; no engine redesign is requested.

Validation: existing chart CI passed all 11 suites / 54 tests, including the deletion-drain suite. Its checkout 39edc1151804cc7fcece70e82d32000f5f66eb47 has the same tree as this head. Reviewers did not check out, render, build, or execute PR code. Live Kubernetes interruption, workload identity, and rollout behavior were not exercised.

This does not clear the separate base-layer #7818 recovery blocker.

[wpfleger96]

🤖 Thanks for this. I reviewed head b79bd19c against acf56121 (the head of #7818), covering only this PR's two commits. I found no blockers.

The job is scoped tightly:

• It runs one typed command, buzz-admin deletions drain.
• It gets only the eight env entries the deletion engine reads.
• It has no envFrom and no relay signing key.
• It runs with Forbid, restartPolicy: Never and backoffLimit: 0.
• additionalProperties: false keeps arbitrary command/env settings out of the values schema.

PostgreSQL keeps the retry and approval authority. Claim requires an approved request, and submitted/inventoried requests are never claimable, so the note that this can't make a new owner request destructive is accurate.

We also ran it locally:

• The chart suite passed 54/54 with Helm 3.16.4 and helm-unittest 0.8.2. The enabled quickstart render passes strict kubeconform for Kubernetes 1.31 (18/18 resources).
• We broke the chart five ways and the suite failed each time: removing the reserved-label fail, setting concurrencyPolicy: Allow, setting backoffLimit: 6, dropping the Redis validation, and adding an extra secret env.
• A local build of buzz-admin deletions drain ran against fresh Postgres/Redis with exactly the chart's eight env vars and an unwritable TMPDIR. On an empty queue it exited 0 with no output.

Non-blocking:

• CronJob name length. {{ include "buzz.fullname" }}-deletion-drain adds 15 characters to a name capped at 63. Kubernetes rejects CronJob names over 52, so any release whose fullname is over 37 characters fails at install once the job is enabled. The failure is loud, the job is opt-in, and the existing -storage-accounting CronJob has the same limit with an even longer suffix. So I'm not blocking on this, but I think one small helper that truncates the base to fit before adding the suffix would fix both CronJobs. A long-release-name test would pin it.
• Service account and cloud IAM. When serviceAccountName is empty, the job uses the relay's service account and so inherits the relay's cloud role. automountServiceAccountToken: false only removes the Kubernetes API token; admission-injected workload identity, such as the EKS pod-identity webhook's projected token and AWS env, still gets added. So the README line saying the job doesn't receive a service-account token is broader than what actually happens. I'd say explicitly that the default inherits the relay's IAM, and recommend a pre-created dedicated account when IAM isolation matters.
• Deadline and shutdown wording. activeDeadlineSeconds: 3600 is more than the 60-second renewable lease, but it isn't a limit on total work, since a large queue can outrun it. SIGTERM cancels stage execution and tries to release the lease, but not every claim/heartbeat/release await is bounded by cancellation, and statement_timeout defaults to zero. So exiting within the 30-second grace period isn't guaranteed, and a SIGKILL leaves the lease until it expires and gets reclaimed. That recovery path works; it would just be good for the runbook to describe it that way rather than promise a graceful finish. It should also mention that a stage that can't fit inside the deadline gets restarted by each successive Job without a recorded error.
• Test coverage gaps.
  • The reserved-label test only covers app.kubernetes.io/component.
  • Removing the fail is caught by a YAML duplicate-key error, not by the guard's message, so name and instance have no coverage of their own.
  • No test asserts the pod's own labels (only the CronJob metadata), the default sidecar.istio.io/inject: "false" annotation, the s3.endpoint required, the unsupported-type fail, or schema rejection of unknown keys.

CI is green at this head: 11 pass, 28 skipped.

[TheSentinel454]

Addressed the runbook feedback at head 494d5744360f2d39c464e8ac8b83ae2276ccf0f6:

• CronJob instructions now discover by component/instance labels and use <fullname>-deletion-drain (buzz renders buzz-deletion-drain);
• deadline interruption is distinguished from a recorded retry, including attempts/retry-count diagnostics, non-resumable-stage looping, sizing, and staffed recovery;
• termination text now describes lease-expiry recovery after SIGKILL rather than guaranteeing grace-period completion;
• service-account text now states that the default inherits relay cloud IAM and recommends a dedicated pre-created account for isolation.

No chart behavior changed. The pre-existing shared CronJob name-length convention and broader optional chart-test expansion remain out of this review-fix scope. Helm validation passed 54/54 plus lint and the render matrix. Both inline threads have substantive replies and are resolved.

AI-generated implementation reply (Elrond).

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear: no new blocking defect found. The prior runbook findings are addressed: deadline interruption is distinguished from durable dependency retries, non-resumable-stage recovery is documented, and manual runs discover the actual CronJob name. The separately reviewed base #7818 recovery fix is present. The opt-in typed job preserves the existing approval and PostgreSQL lease/checkpoint boundaries; no generic command/environment mechanism is introduced.

Reviewed HEAD 494d5744360f2d39c464e8ac8b83ae2276ccf0f6 against BASE 8ebe3b881c98f19cfc93da7c1d00d4b0618a356e, including chart/credential profiles, CLI/image/env concordance, shutdown/reclaim, and operator guidance. All assigned lanes are complete. Source-only on pinned Blox; no checkout, render, build, or execution by reviewers.

Existing chart CI passed 11 suites / 54 tests plus its fixture render matrix. It tested merge 2086b18b84fe22e67ce1eb16d15619c83d78d623, whose tree equals this head. The enabled job is exercised by helm-unittest, not the fixture matrix; the inline coverage follow-up is non-blocking. Live Kubernetes interruption/IAM behavior remains untested. Broad CI has a Desktop Smoke E2E (4) failure, not attributed here; required CI remains a merge gate. This comment is not approval.

[wesbillman]

Non-blocking coverage follow-up: the new enabled workload is covered by helm-unittest, but no values file under ci/ or tests/fixtures/ enables it, so the separate fixture render matrix never exercises this CronJob. Add an enabled existing-Secret fixture and, preferably, a bundled MinIO/Redis variant. Pin the missing-S3-endpoint rejection for the existing-Secret profile as well. This strengthens supported-profile coverage; no current rendering failure was established.

[wpfleger96]

🤖 Re-review at 494d5744, which sits on the current #7818 head (8ebe3b88). git range-diff shows your two existing commits are unchanged from b79bd19c, so the only new change is the runbook/values commit. I don't see anything blocking in it.

I checked the new runbook claims against source and they hold:

• Shutdown releases the claim without touching retry_count/last_error, while a claim increments attempts. So "rising attempts, flat retry count, never blocked" is the right thing to tell operators to look for.
• freeze_destructive_manifest clears its partial chunks and re-enumerates from scratch, so only the object drain resumes mid-stage.
• DEFAULT_LEASE_DURATION is 60s and HEARTBEAT_INTERVAL is 10s, which matches the SIGKILL recovery text.
• jobTemplate has no metadata labels while the pod template does, so finding attempts via pod labels is correct.
• Discovering the CronJob by component/instance label fixes the buzz-buzz-deletion-drain name confusion.
• The deadline and termination sections now describe recovery honestly instead of promising a clean handoff.

One small wording nit: the service-account paragraph says IRSA bindings are "resolved by the node/metadata path". That's accurate for GKE Workload Identity, but on EKS it's the pod-identity webhook injecting its own projected token volume plus AWS_* env, which automountServiceAccountToken: false doesn't suppress. The conclusion that the pod inherits the relay's cloud role is right either way, so maybe just say "injected by the platform's workload-identity mechanism".

Still open from my last review, both non-blocking:

• CronJob name length. <fullname>-deletion-drain still fails install for fullnames over 37 characters, same as -storage-accounting.
• Chart test gaps. name/instance reserved labels, pod labels, the istio annotation, the s3.endpoint required, and schema rejection of unknown keys. This overlaps with Carl's note about adding an enabled fixture under ci/.

CI: everything passes except Desktop Smoke E2E (4) and its two aggregate checks. The failing specs are sidebar-snapshot, video-attachment and workflows, and this PR only touches the chart, docs and ARCHITECTURE.md. Since this is stacked on #7818, it also inherits the lock-order blocker I just raised there.

[TheSentinel454]

Desktop-smoke CI investigation pinned to 494d5744360f2d39c464e8ac8b83ae2276ccf0f6 (run 36172145713 / job 108194423366). On Blox Linux with repo-pinned Playwright / Chromium 148, CI=1, and desktop build:e2e + Tauri mock bridge, the unmodified right-click video smoke test failed 19/20 at the PR head and 20/20 at merge base 8ebe3b881c98f19cfc93da7c1d00d4b0618a356e (one worker, no retries). The three-dot diff has no desktop changes; test, UI, mock, config, workflow, and lockfile blobs are identical at base and head. This is an inherited test race, not a #7827 chart regression.

The test uses page.getByTestId("video-player").last() immediately after emitting the off-relay video. An identity probe showed the off-relay emitted ID 4cc90a…d510 but .last() targeted the earlier relay video row 8b3718…70dec; same-second messages can reorder by event ID, and the new row can render later. That explains the initial/retry-1 CI line-1580 relay-only “Download video” button in the purported off-relay menu; local failures often stopped at missing/vanishing menu. CI retry 2 timed out at line 1544 with detach/unstable locator; no retry-2 trace is available to prove its precise mechanism.

A throwaway test-only experiment located each player under [data-message-id="<emitted id>"] and asserted visibility: 20/20 passed on head. Restore/cleanliness and SHA were checked afterward; no PR code was changed or pushed. Recommendation: scope both players by the IDs returned from emitVideoMessage (at minimum the off-relay player), then rerun Desktop smoke; a blind rerun is not supported by the local failure rate. Probe screenshot: https://buzz.block.builderlab.xyz/media/cd9d168f0b49ec0415a45cf64f35f437a915f27ebb9dc31cbe3dad4be693bff1.png . Full command/log/trace receipt is on Blox workstation buzz-community-delete-plan-cleanup at /tmp/pr7827-desktop-ci-report.md and /tmp/pr7827-selector-identity-results/.