batch: resolve secrets once in the parent, not N times in the children - #68
Merged
Conversation
This was the single biggest source of failed hosts across wave 1, and none of it was ever a host problem. Each host runs as its own `reprovision` subprocess, and each independently resolved every secret it needed from 1Password. A 10-host batch therefore fired 10+ `op read` calls within seconds, and 1Password's desktop integration drops under that load. batch-3 mint lost **9 of 10 hosts**: ✗ macmini-m4-247 failed 0:00 couldn't read op://RelOps/RelOps Worker Admin Key/... ✗ macmini-m4-248 failed 0:30 timed out reading op://RelOps/RelOps Worker Admin Key/... ██████ 1 ok · 0 skipped · 9 failed Earlier batches lost preflight and add-to-group runs the same way, and the standing advice had become "prime `op` by hand first", which is a workaround for a design problem: N processes should not each authenticate to a credential store to do one wave. `_resolve()` already prefers a direct env value over the ref, so run_batch now resolves each secret ONCE up front and passes the values to children via REPROVISION_*. The children never invoke `op` at all. Same batch, immediately after: ██████ 10 ok · 0 skipped · 0 failed · 0:05 wall clock Per-host time went 0:30 -> 0:01, because the time was never the work. Best-effort per secret: an action that doesn't need the Taskcluster credential must not fail because that credential wouldn't resolve, and any secret we skip simply falls back to the child resolving it itself. Trade-off, taken deliberately: this puts secret material in the child's environment, readable by other processes of the same user. The alternative is the status quo, where the same material is read N times over a channel that demonstrably fails mid-wave. Single-operator laptop use, and the children already hold these values in memory. Tests get an autouse fixture so the suite never calls 1Password; the two tests of the helper itself opt out with a marker. Without that opt-out they asserted against the stub and passed vacuously. Ref: RELOPS-2515 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The single biggest source of failed hosts across wave 1 — and none of it was ever a host problem.
Each host runs as its own
reprovisionsubprocess, and each independently resolved every secret it needed from 1Password. A 10-host batch fired 10+op readcalls within seconds, and 1Password's desktop integration drops under that load.batch-3 mintlost 9 of 10 hosts:Earlier batches lost
preflightandadd-to-groupruns the same way, and the standing advice had become "primeopby hand first" — a workaround for a design problem. N processes should not each authenticate to a credential store to run one wave.Fix
_resolve()already prefers a direct env value over the ref, sorun_batchresolves each secret once up front and passes the values to children viaREPROVISION_*. The children never invokeopat all.Same batch, immediately after the change:
Per-host time went 0:30 → 0:01, because the time was never the work.
Notes
Best-effort per secret. An action that doesn't need the Taskcluster credential must not fail because that credential wouldn't resolve; any secret skipped here just falls back to the child resolving it itself.
Trade-off, taken deliberately. This puts secret material in the child's environment, readable by other processes of the same user. The alternative is the status quo, where the same material is read N times over a channel that demonstrably fails mid-wave. Single-operator laptop use, and the children already hold these values in memory. Flagging it explicitly so it's a decision rather than an accident.
Testing
219 pass,
ruff check .clean on the pinned 0.15.19.The suite gets an autouse fixture so it never calls 1Password; the two tests of the helper itself opt out via a registered marker. Without that opt-out they asserted against the stub and passed vacuously — worth knowing if you touch them.
Verified live: 10/10 on
mint, then 10/10 onos-update, with all ten downloads confirmed progressing at 205 MB/s aggregate.Ref: RELOPS-2515
🤖 Generated with Claude Code