fix(platform): make mongodb-application depend on its operator - #3845
myasnikovdaniil wants to merge 1 commit into
Conversation
The mongodb chart renders kind: PerconaServerMongoDB from psmdb.percona.com/v1, but cozystack.mongodb-application depended only on networking and cozystack-engine. So mongodb-rd registers the ApplicationDefinition as soon as the engine is up, and a tenant can create a MongoDB before the operator exists; its HelmRelease then fails with no matches for kind "PerconaServerMongoDB". This is the same defect #3817 fixed for etcd, found while reviewing it. Every other operator-backed app here already carries the edge. It also closes a test-selection gap: without it cozystack.mongodb-operator reaches no runnable suite, so every change to the operator escalates to the full 21-suite run instead of selecting mongodb. select-install.sh --validate reports the graph still has no cycle. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
github-actions
Bot
added
size/M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe default MongoDB application package now depends on ChangesMongoDB application dependency
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to The MongoDB application now waits for its operator before becoming available, preventing creation before the required CRD exists. The change is localized and covered by a regression test; no actionable merge-blocking risk remains after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
myasnikovdaniil NOT LGTM. The change itself is right, the only blocker is the attribution trailer on the commit.
Business context: a tenant can create a MongoDB before the Percona operator and its CRD exist, because the mongodb application never waited for the mongodb operator.
The edge is still missing on current main, and the branch merges into main without conflicts. It matches postgres, mariadb, kafka, redis and etcd, which all list their operator. Both packages come from the paas bundle under the same condition, and nothing depends on cozystack.mongodb-application. The operator itself depends only on networking, prometheus-operator-crds and cert-manager, so there is no cycle, and hack/select-install.sh --validate agrees. The new test runs under make unit-tests. It goes red when I remove the edge and green when I put it back.
The blocker: commit fcec3b0 carries Assisted-By: Claude <[email protected]>. docs/agents/contributing.md lists this exact line under Review Blockers, and the trailer value must be LLM. CI stayed green only because this base predates the Commit trailers job. Main's hack/check-commit-trailers.sh rejects the commit with "attribution trailer value must be exactly LLM", so it will fail after a rebase. Please reword it to Assisted-by: LLM.
While rewording, I'd also drop "found while reviewing it" from the commit body and "it was found while reviewing that one" from the PR body. The PR body becomes the merge commit message, and how the bug was found is not a reason for the change. Keeping #3817 as the precedent is fine.
E2E on this head failed only in kubernetes-previous and kubernetes-latest, and the mongodb suite passed. A dependsOn edge on the mongodb application doesn't reach the Kamaji control-plane suites, so I don't count that against the PR. The branch is 1289 commits behind main, so the rebase for the reword also gets you a fresh run on current code.
2 participants
The mongodb chart renders
kind: PerconaServerMongoDBfrompsmdb.percona.com/v1, butcozystack.mongodb-applicationdepended only oncozystack.networkingandcozystack.cozystack-engine. Somongodb-rdregisters the ApplicationDefinition as soon as the engine is up, a tenant can create a MongoDB before the operator exists, and its HelmRelease then fails withno matches for kind "PerconaServerMongoDB".Every other operator-backed application here already carries the edge. This is the same defect #3817 fixes for etcd, and it was found while reviewing that one.
It also closes a test-selection gap. Without the edge
cozystack.mongodb-operatorreaches no runnable suite, so every change to the operator escalates to the full 21-suite run instead of selectingmongodb. Seesrc_to_suites()inhack/select-e2e.sh.Checks
packages/core/platform/tests/sources_mongodb_application_dependson_test.yamlpins the edge alongside the two baseline ones, mirroring the etcd guard. Mutation checked: removing the edge turns it red, restoring it turns it green.hack/select-install.sh --validatereports the graph still has no cycle, 99 sources. Platform helm unit tests are 134 across 31 suites.The exposure this adds
An administrator who lists
cozystack.mongodb-operatorinbundles.disabledPackageswhile keeping the application now leaves itDependenciesNotReady. That is the same exposure postgres, mariadb, kafka, redis and etcd already carry, withPackage.spec.ignoreDependenciesas the escape hatch.Release note
Summary by CodeRabbit
Bug Fixes
Tests
Closes #3850