test(controller): stop caching Secrets in the envtest manager - #317
Conversation
The suite's manager cached Secrets while the shipping binary never does (cmd/main.go disables the Secret cache), so controller Secret reads in tests were served by a lazily started cluster-wide informer that lags every write the suite makes. The GitTarget encryption gate reads its age-key Secret by name and parks the target on RequeueSteadyInterval (5 minutes) on any error, so one stale hit put recreation far beyond what a spec waits for — the ~1-in-10 failure of "Should recreate encryption secret when it is deleted while GitTarget still exists". Wire the test manager the way cmd/main.go wires the real one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 52 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe controller suite test imports the Kubernetes Secret type and disables Secret caching in the test manager. Secret reads now bypass the informer cache, matching the production client configuration. ChangesSecret cache test configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This test-only change aligns Secret reads with production behavior and removes a cache-related test flake. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description clearly explains the change, cause, impact, and validation results. However, it omits the required Type of Change, Checklist, Related Issues, Screenshots, and Additional Notes sections, and it does not provide the requested testing checkboxes. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The flake is fixed, so the TODO entry that asked for a fix and the ambient-flake row that told readers not to misattribute it are stale. The rationale that survives lives beside the code: why recreation is polled, and why the spec's budget is a bound. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The controller envtest suite now wires its manager the way
cmd/main.gowires the shipping one:client.CacheOptions{DisableFor: []client.Object{&corev1.Secret{}}}.Why
The suite's manager cached Secrets; the binary never does. Every controller Secret read in tests was therefore served by a lazily started cluster-wide Secret informer whose view lags the writes the specs make — a divergence that tested a controller that does not ship.
The GitTarget encryption gate reads its age-key Secret by name, and on any error there the target is parked on
RequeueSteadyInterval(5 minutes). One stale cache answer — the deleted Secret still present, or a just-created one the informer had not observed — pushed recreation far past the spec's 45-second budget. That is the ~1-in-10 failure of "Should recreate encryption secret when it is deleted while GitTarget still exists" (secrets "recreated-sops-age-key" not found), previously papered over twice by raising that budget.With live reads, the reconcile after the deletion sees
NotFoundon the very nextRequeueStreamSettleIntervaltick, so recreation is bounded by the requeue cadence rather than by informer lag.Validation
task lint— passtask test— pass (coverage 77.4%, baseline held)🤖 Generated with Claude Code
Summary by CodeRabbit