Repository navigation
fix(hack): keep root monitoring redirects off in the E2E install - #4811
Aleksei Sviridkin (lexfrei) merged 1 commit into
Conversation
The E2E install sets Tenant/root spec.monitoring to false, but left the platform monitoring.rootEnabled at its default of true, so cozystack-basics rendered its vlinsert/vminsert ExternalName Services into cozy-monitoring regardless. That namespace is created only by metrics-server, monitoring-agents and monitoring. A narrow suite selection disables all three, so basics fails with `namespaces "cozy-monitoring" not found` and every package waiting on it never installs. Wide selections passed only because they keep at least one of those packages. Setting rootEnabled to match the root tenant removes the dependency and is what the value's contract asks for anyway. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
github-actions
Bot
added
size/S
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughGenerated platform package values now set ChangesRoot monitoring setting
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change removes unused root monitoring redirects without disabling the packages required by narrow or full installs. No material merge risk is apparent; proceed with ordinary checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM.
Focused test-infra fix and the mechanism checks out. hack/e2e-platform-packages.sh renders the platform Package with variant: isp-full, which inherits monitoring.rootEnabled: true from packages/core/platform/values.yaml. The install sets Tenant/root spec.monitoring: false, so the two disagreed, and cozystack-basics/templates/monitoring-external-services.yaml then rendered the ExternalName Services into cozy-monitoring, a namespace a narrow suite selection never creates. Pinning rootEnabled: false in the generator matches the chart's own documented rule to keep monitoring.rootEnabled in sync with Tenant/root spec.monitoring.
Verified: hack/platform-packages_test.bats passes (all 12) on the head and on the tree merged with current main; the new guard catches the regression (flipping the generator back to rootEnabled: true turns the test red); the cross-reference comment in e2e-install-cozystack.bats is accurate. No blocking findings; change is hack/-only.
2 participants
What this PR does
This fixes the E2E install on pull requests that select only a few suites. Today those runs fail before any suite starts:
cozystack-basicsfails withnamespaces "cozy-monitoring" not found, and every package waiting on basics never installs.The reduced install from #4022 disables every package the selected suites don't need. For narrow selections like
vminstanceor the Kubernetes suites, that list includes metrics-server, monitoring-agents and monitoring, which are the only packages that createcozy-monitoring. Basics still renders three ExternalName redirects into that namespace, becausemonitoring.rootEnabledstays at its default of true. Wide selections keep at least one of those packages, so they pass.The E2E install already sets
Tenant/root spec.monitoringto false, androotEnabledis meant to match it. The harness now setsrootEnabled: false, so basics no longer writes into a namespace the install may not create. No suite uses these redirects. With root monitoring off they pointed at nothing even on full runs.The same failure can happen in a real cluster when a user disables those three packages. That needs a product change and is tracked in #4810.
Screenshots
Downstream repositories
Release note
Summary by CodeRabbit
cozy-monitoringis omitted, preventing monitoring Services from being rendered in narrowly selected test suites.Tenant/rootconfiguration and which setting to change to enable it.