CLOUDP-315085: Rename member list cm #66
Merged
Conversation
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
Julien-Ben
requested review from
dan-mckean,
vinilage and
a team
as code owners
mircea-cosbuc
force-pushed
the
rename-member-list-cm
branch
from
02362f4 to
6cb9a32
Compare
Julien-Ben
force-pushed
the
rename-member-list-cm
branch
from
6e0c90b to
cc0d634
Compare
SimonBaeumer
previously requested changes
May 2, 2025
There was a problem hiding this comment.
While I can understand the motivation of this change it introduces potential breaking changes to the customer and modifies behaviors of our Helm chart.
Following questions need clarification for my perspective:
- Is this change documented in an upgrade guide?
- What does this change mean to customers who used the Helm chart to generate this ConfigMap?
- Is it possible to keep backwards-compatibility?
- What happens if the user migrated to MCK and did not updated their ConfigMap?
I am new to the project, so please keep in mind that I miss context on the impact of this ConfigMap but this change seems risky, especially shortly before our release.
SimonBaeumer
dismissed
their stale review
Dismissing review because of Slack chat in Slack.
Julien-Ben
commented
May 2, 2025
nammn
approved these changes
May 2, 2025
mircea-cosbuc
force-pushed
the
rename-member-list-cm
branch
from
cc0d634 to
be9f8a5
Compare
mircea-cosbuc
approved these changes
May 2, 2025
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.
Summary
Why
The multi cluster member-list configmap is created by both the helm chart and kubectl tool.
It should not be created by helm. E.g one drawback is that the configmap can be removed upon release uninstallation, which leads to a panic in the currently running operator.
This is also confusing and it should be the responsibility of a single tool to create that map.
The name of that configmap is
mongodb-enterprise-operator-member-list and we should take the opportunity to rename it to mongodb-kubernetes-operator-member-list, to stay consistent with our new naming conventions.
Customers deploying multi cluster resources must run the kubectl plugin when upgrading, so we can safely rename it.
How
The config map name was mostly defined by constants so it's a small renaming.
Proof of Work
E2E tests pass (including upgrade tests)