feat(openhands): add budget preflight and reconciliation gate hook jobs

[hieptl]

Description

Adds a budget upgrade preflight and a post-upgrade reconciliation gate to the openhands chart, as two Helm hook Jobs on the enterprise image (OHE-3256):

• <release>-budget-preflight (pre-upgrade, weight 5, default mode acknowledge): read-only, runs the new image against the not-yet-migrated database and reports each enabled organization's budget state (missing baselines, members missing from LiteLLM, unmapped identities, cap drift, over-cap state, snapshot age, last sync) before enforcement changes.
• <release>-budget-reconcile-gate (post-upgrade, weight 5, default mode strict): reconciles every enabled organization's LiteLLM caps in-process and verifies them by readback. In strict a blocking finding exits non-zero, fails helm upgrade, and marks the Replicated version failed; acknowledge records the findings and lets the release proceed.

Both Jobs are gated on .Release.IsUpgrade, so they never render on install or under ArgoCD (which renders with helm template), use backoffLimit: 0 with activeDeadlineSeconds under Helm's per-hook wait (300 s / 540 s against the 600 s Replicated timeout), and keep their last run with hook-delete-policy: before-hook-creation so the JSON artifact in the pod log survives until the next upgrade. Each Job prints one org_budget_preflight artifact line.

Also included: budgetPreflight / budgetReconcileGate values blocks, a render-time guard for invalid modes, two support-bundle logs collectors, a Replicated Config group "Upgrade Checks" (budget_preflight_mode, budget_reconcile_gate_mode) mapped into the chart values, helm-unittest suites for both Jobs and the guard, and an upgrade-rollback runbook section covering artifact capture, strict-gate recovery, manual runs, and restoring LiteLLM-side caps after a rollback.

Depends on OpenHands/enterprise#362, which adds run_budget_preflight.py to the image. This PR must not merge before a release containing that script is the chart's default image.tag; with an older image the pre-upgrade Job would fail on the missing module and block upgrades. Kept as a draft until then.

Validation:

• helm template t charts/openhands --is-upgrade --show-only templates/budget-preflight-job.yaml --show-only templates/budget-reconcile-gate-job.yaml renders both Jobs with the expected hooks and env; without --is-upgrade nothing renders.
• helm unittest charts/openhands: 150 passed in 31 suites.
• helm lint charts/openhands: passed.
• make lint (Replicated): exit 0; only pre-existing warnings on troubleshoot/secrets.yaml.

Helm Chart Checklist

• I have tested the chart upgrade path from the previous version
• I have verified backwards compatibility with existing values.yaml configurations
• I have updated the chart's README.md if there are any breaking changes or new required values

New keys ship with defaults and values.schema.json does not restrict additional top-level keys, so existing values files are unaffected. The README lists no job values, so no README change was needed. A live upgrade on a Replicated test instance has not been run yet.

Additional Notes

• The strict default on the post-upgrade gate is deliberate: it is what makes "failed reconciliation blocks release health" true out of the box. Customers with a known-dirty organization can select Acknowledge in the "Upgrade Checks" Config group before upgrading.
• Mutual exclusion between the hook and the 15-minute budget-maintenance CronJob is tracked separately (OHE-3259); both are idempotent.
• SaaS (ArgoCD) deployments get neither Job; the runbook documents running the preflight by hand there.

[tofarr]

Code Review: PR #1231 — Budget preflight & reconciliation gate hook jobs

Taste Rating: 🟡 Acceptable — well-structured, well-documented, with one default-policy decision worth a second look and a couple of minor DRY/schema gaps.

Summary

Adds two Helm hook Jobs (pre-upgrade preflight, post-upgrade reconcile gate) around helm upgrade of the openhands chart, plus Replicated Config wiring, a render-time mode guard, support-bundle collectors, helm-unittest suites, and a runbook section. Gated on .Release.IsUpgrade so install and ArgoCD (helm template) are excluded. Depends on OpenHands/enterprise#362 (adds run_budget_preflight.py); correctly held as draft.

---

[CRITICAL ISSUES]

None that break correctness.

[IMPROVEMENT OPPORTUNITIES]

1. Default-on strict post-upgrade gate is a behavior change for existing customers — weigh deliberately.
budgetReconcileGate.mode: strict ships enabled-by-default. Every existing customer's next helm upgrade now runs a gate that can mark the Replicated release FAILED on cap_drift, member_missing_from_litellm, last_sync_error, etc. — even though the new Deployments are already serving. The escape hatch (acknowledge in the "Upgrade Checks" Config group) must be set before upgrading; a customer who discovers it post-failure is in a worse spot than before this feature existed.

The author's reasoning ("strict is what makes 'failed reconciliation blocks release health' true out of the box") is sound for the feature's intent. But under the "never break userspace" lens, flipping existing installs from no gate to strict blocking gate in a single release is aggressive. Consider:

• Ship both defaults as acknowledge for one release cycle, then flip the gate to strict once customers have seen the artifacts and opted in; or
• Gate the strict default behind a fresh-install vs. existing-install distinction (hard in Helm, but the Replicated Config default could be acknowledge while the chart value is strict, so only new explicit configs get strict).

This is a product/safety trade-off, not a code bug — but it's the single most important decision in the PR and I'd want a human architect to sign off.

2. Two near-duplicate 74-line templates.
budget-preflight-job.yaml and budget-reconcile-gate-job.yaml are ~90% identical (same pod spec, env, caBundle, affinity, resources). Only the hook type, weight, name, BUDGET_PREFLIGHT_PHASE, and mode source differ. A shared _budget-hook.tpl partial parameterized by phase would eliminate the duplication and the risk of the two files drifting (e.g., one gets a caBundle mount fix and the other doesn't). For Helm, two explicit hook files is an acceptable pattern, so this is "should fix" not "must fix" — but the drift risk is real given how similar they are.

3. New keys absent from values.schema.json.
budgetPreflight / budgetReconcileGate aren't in the schema. The PR description correctly notes the schema doesn't restrict additional top-level keys, and the render-time validations.yaml guard catches invalid modes. But activeDeadlineSeconds and resources are unvalidated — a typo like activeDeadlineSconds: 300 silently falls back to no deadline. Adding the two blocks to values.schema.json would close that gap and is cheap.

[STYLE NOTES]

• Comments are mostly substantive (the hook-delete-policy rationale, the IsUpgrade gating). The two block comments at the top of each job template are genuinely useful. No noise to trim.
• helm.sh/hook-weight: "5" for the post-upgrade gate keeps it ahead of the laminar retention job (weight 10) — verified against laminar-clickhouse-diagnostic-retention-job.yaml. Consistent.

[TESTING GAPS]

• Template-rendering tests are good and real (not mocks): they assert hook annotations, weights, env vars, modes, disabled states, resources, and the render-time guard. 150 passed / 31 suites is credible.
• No test that the support-bundle selectors match the job labels. The collectors select app={{ .Release.Name }}-budget-preflight; the jobs set app: {{ .Release.Name }}-budget-preflight in both metadata.labels and template.metadata.labels. They match today, but a label rename in one file would silently stop collecting logs with no test failure. A helm-unittest asserting the selector string appears in the rendered job labels would pin this.
• No live upgrade evidence. The PR is explicitly draft and notes a live Replicated upgrade hasn't been run. For a chart PR that changes upgrade behavior, an end-to-end upgrade on a test instance (strict-pass and strict-fail paths) is the proof that actually matters. Acceptable to defer given the draft status and the enterprise#362 dependency, but it should be a merge blocker, not just a checklist item.

[RISK ASSESSMENT]

• [Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM
  • The change is opt-out-by-default and can fail production upgrades for existing customers who have never seen this gate. The recovery path is documented and the jobs are idempotent, but the blast radius is "release marked failed mid-upgrade."
  • Correctness of the hook script itself lives in enterprise#362 (not reviewable here); the chart's responsibility is to invoke it with the right phase/mode/env, which it does correctly.
  • Hard dependency on an unreleased image tag is properly tracked and the PR is draft.

[VERDICT:]
✅ Worth merging after (a) the strict-default rollout strategy is confirmed by a human reviewer, (b) a live upgrade test on a Replicated instance exercises both the strict-pass and strict-fail paths, and (c) enterprise#362 is released and is the chart's default image.tag. The code itself is sound.

[KEY INSIGHT:]
The chart wiring is correct and well-tested at the template level; the real risk is operational, not technical — a default-strict gate that can fail existing customers' upgrades on first contact.

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing. See the customization docs.
2. Re-request a review — the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

---

Note: This review was generated by an AI agent (OpenHands) on behalf of the user.

[tofarr]

🍰

[openhands-release-bot]

🚀 Released in openhands/0.70.0.