Skip to content

feat: check what an upgrade's index builds cost before any pod rolls [ENG-1881] - #72

Draft
twk3 wants to merge 3 commits into
mainfrom
claude/hopeful-shannon-wex8jw
Draft

twk3 wants to merge 3 commits into
mainfrom
claude/hopeful-shannon-wex8jw

Conversation

@twk3

@twk3 twk3 commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Owner: @twk3
Linear: ENG-1881, ENG-1880

Part 2 of ENG-1881. The CLI verdict it calls is currents-dev/currents#4558 — this cannot be released until that is in an image, because the hook runs migrate index-plan --gate from the image being upgraded to.

What and why

A release that declares a new index builds it while the upgrade is in flight, and on a large instances collection that has taken about an hour with the install serving nothing. Nothing tells the operator beforehand: the scheduler logs it once the pods have already rolled.

This adds a pre-upgrade hook Job, currents-index-check, that runs the new image's migrate CLI against the database the old release is still serving, before any pod rolls. index-plan only reads, so running the new code against the old data is safe.

indexes.preUpgrade.mode:

  • gate (default) — read the plan, fail the hook if the estimate is over thresholdSeconds (600). Helm stops the upgrade with the running release untouched.
  • build — build the indexes first and wait. No activeDeadlineSeconds: the point of this mode is to wait.
  • off — render no hook.

Helm does not print a failed hook's log

All the operator sees is:

Error: UPGRADE FAILED: pre-upgrade hooks failed: job currents-index-check failed: BackoffLimitExceeded

The estimate and the collection names are in the pod. That is why the delete policy is before-hook-creation and not hook-failed — hook-failed deletes the pod the moment the Job fails, taking the only copy of the message with it. docs/eks/upgrading.md leads with the kubectl logs line.

ENG-1880 is in here too (second commit)

The gate is only worth having if it can offer something other than a refusal, and the first version of that recipe I wrote was wrong: it ran sync-indexes inside the running scheduler, which doesn't declare the new indexes, so it rebuilds what is already there and the gate blocks again next time.

ENG-1880 planned a separate Job template for this. It doesn't need one. The hook in mode: build already is that Job — same image, same command — and the only thing a standalone pre-build needs that the hook doesn't is a name of its own, which indexes.name supplies:

helm template currents currents \
  --repo https://currents-dev.github.io/helm-charts/ \
  --version <target-chart-version> \
  -f currents-helm-config.yaml \
  --set indexes.preUpgrade.mode=build \
  --set indexes.name=index-build \
  --show-only templates/indexes/pre-upgrade-job.yaml \
  | kubectl apply -n currents -f -

helm template writes nothing to the cluster, so --version is what reaches the next image. The operator picks the window, nothing is down, and the build records a measured rate that makes later estimates trustworthy.

I filed the two tickets together rather than splitting because ENG-1880's whole delivery is the docs section for the feature in commit 1, and separating them would mean shipping a gate that tells operators to do the wrong thing.

Verification

I could not render this: downloading helm and helm-docs was blocked in my container and I didn't work around it. I checked the if/with/end balance by hand, hand-rendered both modes into YAML that yaml.safe_load parses with the right command, deadline and annotations, and derived the 9 docs/configuration.md rows from the # -- annotations using helm-docs' own formatting rules.

That has since been checked properly, and both parts held:

  • CI's docs-check (helm-docs) and lint-test (ct lint, which renders) are green, so the table matches and the template compiles.
  • @twk3 ran the whole thing locally on Docker Desktop Kubernetes against a 4M-document instances, including the pre-upgrade execution that ct install cannot reach: gate blocked with the running release untouched at generation 1, the pre-build recipe built the index and the next upgrade passed with No index builds pending., and build and off behaved as intended.

Still unverified: the --repo/--version form of the pre-build recipe, which needs a published chart, and MongoDB with authentication.

Two things worth a reviewer's eye

  • An image that predates the check blocks every upgrade. If currents.imageTag is pinned to a release older than the one carrying --gate, the hook exits 1 with the migrate usage line. The chart fails closed here on purpose — a gate that silently stops gating is worse than one that fires confusingly — so the fix is legibility, not tolerance: docs/eks/upgrading.md now matches on that usage line and gives mode: off as the way through. Reasoning in full, including why a version floor and a tolerant wrapper are both worse. Worth a CHANGELOG line at release.
  • 600 seconds is a guess. About 18 GB of scan at the built-in rate, about 150 GB at a measured one. A deployment's first sizeable upgrade can trip it on a build that really takes under a minute, because nothing has been measured yet. The Job's log says so explicitly when that is the case.

The container sets HOME=/tmp with a writable emptyDir because global.securityContext sets readOnlyRootFilesystem; both modes ran under that security context in the local run.

No Chart.yaml or CHANGELOG.md bump — the repo does those in separate release: PRs.

🤖 Generated with Claude Code

https://claude.ai/code/session_016uVeGDCC9GMmYpp8gZQSYt

…[ENG-1881]

A release that declares a new index builds it while the upgrade is in
flight, and on a 109 GB instances collection that took about an hour. The
scheduler cannot warn anyone: by the time it logs, the pods have rolled.

A pre-upgrade hook Job runs the new image's `migrate index-plan --gate`
against the database the old release is still serving. It only reads, so
new code against old data is safe. Over the threshold it exits 2 and Helm
stops with the running release untouched.

indexes.preUpgrade.mode is one enum rather than separate acknowledge and
prebuild switches: the three states are exclusive, and booleans leave
combinations with no defined meaning.

Helm prints `BackoffLimitExceeded` and not the pod's log, which is where
the estimate and the collection names are, so the delete policy keeps the
pod and docs/eks/upgrading.md gives the kubectl logs line.

activeDeadlineSeconds on the gate only. Reading the plan is a few
listIndexes and $collStats calls, so overrunning means MongoDB is not
answering, and a hook with no deadline holds helm upgrade open forever. A
build gets no deadline because waiting is the point of that mode.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uVeGDCC9GMmYpp8gZQSYt
The pre-upgrade recipe ran sync-indexes inside a current pod. The running
release does not declare the indexes the next one adds, so that rebuilds the
existing set and the gate blocks again on the next attempt.

Render the hook's own Job at the target version instead, under a name of its
own so it neither collides with the failed check nor deletes its log. No new
template: the only thing the standalone build needs that the hook does not is
a distinct name, and indexes.name already supplies it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uVeGDCC9GMmYpp8gZQSYt
@baz-reviewer

baz-reviewer Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review this PR on Baz

Baz Summary

Add a Helm pre-upgrade hook Job that runs the new image’s migrate CLI against the existing database to estimate pending index-build costs before pods roll. Document gate, build, and off modes so operators can prevent downtime, pre-build indexes, or explicitly bypass the check.

Topics

TopicDetails
Index Upgrade Check Run currents-index-check before upgrades to gate or build pending indexes using configurable thresholds, timeouts, hook behavior, and execution settings.
Modified files (3)
  • charts/currents/templates/_helpers.tpl
  • charts/currents/templates/indexes/pre-upgrade-job.yaml
  • charts/currents/values.yaml
Latest Contributors(2)
UserCommitDate
noreply@anthropic.comfeat: check what an up...October 30, 2026
dj@currents.devrelease: 1.0.0 [] (#64)October 14, 2026
Upgrade Runbook Explain how to inspect failed hook logs and choose pre-build, in-upgrade build, or bypass workflows when an index check stops an upgrade.
Modified files (2)
  • docs/configuration.md
  • docs/eks/upgrading.md
Latest Contributors(2)
UserCommitDate
noreply@anthropic.comdocs: say what to do w...October 30, 2026
dj@currents.devrelease: 1.0.0 [] (#64)October 14, 2026


Skills  Activate Skill Maintainer to keep your skills up to date

Planner  This PR would have been improved with Baz Planner - Try it now

@twk3

twk3 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Local evidence on a065a3f (merged with main for the 1.0.1 base) that the hook works, including the pre-upgrade run that ct install can't exercise.

Setup

  • Docker Desktop Kubernetes, namespace currents-ci, with this repo's .github/ci/dependencies.yaml, the secrets from lint-test.yaml and ci/lint-values.yaml.
  • Chart 1.0.1 installed on 2026-10-14-002, images imported into the node.
  • The scheduler for the upgrade was built locally from currents-dev/currents#4558 at 7db5b36, which adds index-plan --gate. The other services stayed on 2026-10-14-002.
  • 4M instances (2.5 GB), with createdAt_1 dropped to stand in for a release that adds an index.

Rendering, which the description says was never run:

  • helm lint with ci/lint-values.yaml: clean.
  • gate renders the Job with index-plan --gate --gate-seconds "600", activeDeadlineSeconds: 300, backoffLimit: 0, hook-weight: "-10", before-hook-creation.
  • build renders sync-indexes with no deadline. off renders nothing.
  • mode=bogus fails with indexes.preUpgrade.mode must be one of "gate", "build" or "off", got "bogus".
  • scripts/build-docs.sh (helm-docs) leaves docs/configuration.md unchanged, and CI's docs-check agrees.

gate blocks, and the running release is untouched (thresholdSeconds=60):

Error: UPGRADE FAILED: pre-upgrade hooks failed: 1 error occurred:
	* job currents-index-check failed: BackoffLimitExceeded

kubectl logs job/currents-index-check, from the pod that ran currents-local/scheduler:pr4558-7db5b36 and exited 2:

Index build estimated at 84 seconds, which is over the limit of 60 seconds.

  instances: 2.5 GB, about 84 seconds (createdAt_1)
  ...
Estimated at 30.0 MB/s, the built-in default — this deployment has recorded no index builds of its own, so the real build may be several times faster.

Revision 2 was recorded failed. Every deployment stayed on 2026-10-14-002 at generation 1, no pod restarted, and no index was built.

The pre-build steps from docs/eks/upgrading.md, run from the local chart path instead of --repo/--version:

  • helm template … --set indexes.preUpgrade.mode=build --set indexes.name=index-build --show-only templates/indexes/pre-upgrade-job.yaml | kubectl apply -f - created currents-index-build on the new image. It completed in 10 s, built createdAt_1, and recorded the build: 2.6 GB in 5,876 ms.
  • currents-index-check (Failed) stayed next to it, so the log of the blocked run was still readable.
  • After kubectl delete job currents-index-build, the same helm upgrade passed with No index builds pending. (revision 3, deployed in 17 s), and the scheduler rolled to the PR image.

The other modes

  • gate under the default 600 s, with the index pending again: Index build estimated at 6 seconds. The limit is 10 minutes. Estimated at 428.7 MB/s, measured from 1 recorded build on this deployment., and the upgrade went ahead.
  • build: the hook ran sync-indexes (8 s), built the index, then the upgrade went ahead.
  • off: no Job was created; the existing hook Job kept its uid.

Worth deciding before release: an image without --gate blocks every upgrade. I ran the upgrade with --set currents.imageTag=2026-10-14-002. That is an operator pinning images while taking a chart update, or this chart released before an image that has #4558:

Error: UPGRADE FAILED: pre-upgrade hooks failed: … job currents-index-check failed: BackoffLimitExceeded
Usage: node dist/scripts/migrate.js <up|status|sync-indexes|baseline|mark> [--dry-run]

The hook exits 1 and stays failed until the operator finds indexes.preUpgrade.mode=off. Possible answers:

  • say in the upgrade docs and CHANGELOG that a pinned currents.imageTag older than the release carrying --gate needs mode: off;
  • or have the chart require the image to support it.

Not covered

  • --repo/--version against a published chart: this chart isn't released.
  • MongoDB with auth: the CI dependency has none. The chart's default security context was in effect (runAsNonRoot, runAsUser: 1000, readOnlyRootFilesystem: true, all capabilities dropped), and both modes ran under it with HOME=/tmp on the emptyDir.

🤖 Generated with Claude Code

An operator pinning currents.imageTag to a release older than the chart gets
a hook that exits 1 with the migrate usage line, and nothing connects that to
indexes.preUpgrade.mode. The check fails closed on purpose, so the way out is
a documented flag rather than a weaker check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uVeGDCC9GMmYpp8gZQSYt

twk3 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for running the pre-upgrade path — that is the one thing CI structurally cannot do, and it closes the biggest caveat in the description. I will fix the "nothing here has been rendered" paragraph, which your helm lint and build-docs.sh runs have now overtaken.

On the pinned-image question: I think the chart should keep failing closed, and the gap is that the failure is illegible. Pushed as ebce529 — a troubleshooting entry that matches on the usage line you captured, says the image predates the check, and gives mode: off as the way through.

Why not the other two options:

  • Having the chart require a supporting image means comparing currents.imageTag against a floor, and the tags are not reliably ordered — the date tags happen to sort lexically, but staging and your own pr4558-7db5b36 do not. A version check that rejects legitimate custom tags would break more upgrades than it saves.
  • Making the hook tolerant is the tempting one, because index-plan --gate already separates exit 2 (too long) from exit 1 (could not run) specifically so a caller can treat them differently, and the Job currently throws that away. A shell wrapper could pass on exit 1. But the same wrapper would wave the upgrade through when MongoDB is unreachable or the query errors, and a gate that silently stops gating is worse than one that fires confusingly — that is the failure mode the whole phase is meant to avoid. The cost of failing closed is one flag on one upgrade, by someone who has read why. The cost of failing open is the hour of downtime this exists to prevent.

That said, the exit-2/exit-1 split is now unused by its only caller, so if you would rather the hook distinguish them, the mechanism is there and it is a small change to the template. Your call — this is the kind of thing I would rather you decided than have me pick.

Worth adding to the CHANGELOG at release time, in whatever wording you prefer: a pinned currents.imageTag older than the release carrying --gate needs indexes.preUpgrade.mode=off.

On the two things you listed as not covered: --repo/--version against a published chart is also the one path in the pre-build recipe I cannot exercise, so that stays unverified until the chart is released. MongoDB with auth only changes currents.connectionConfigEnv, which the Job takes whole rather than picking MONGODB_URI out of, so I would expect it to behave the same — but I have not run it either.


Generated by Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants