CLOUDP-349087: Block simultaneous TLS disabling and scaling for ReplicaSets #549
Conversation
MCK 1.6.0 Release NotesNew Features
Bug Fixes
|
Julien-Ben
changed the title
| connection := omConnectionFactory.GetConnection() | ||
| connection.(*om.MockedOmConnection).CheckDeployment(t, deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rs), "auth", "ssl") | ||
| connection.(*om.MockedOmConnection).CheckNumberOfUpdateRequests(t, 2) | ||
| connection.(*om.MockedOmConnection).CheckNumberOfUpdateRequests(t, 1) |
There was a problem hiding this comment.
Reduced to one since we removed one update step in updateOmDeploymentRs
|
Assigned |
| --- | ||
| kind: fix | ||
| date: 2025-10-24 | ||
| --- | ||
|
|
||
| * **ReplicaSet**: Blocked disabling TLS and changing member count simultaneously. These operations must now be applied separately to prevent configuration inconsistencies. |
Fix create replica set test Rename fixture Rename file Changelog entry Lint Cleanup TLSConfigurationWillBeDisabled Remove code to handle tls disable + scale Add new scale down step
Fix create replica set test Rename fixture Rename file Changelog entry Lint Cleanup TLSConfigurationWillBeDisabled Remove code to handle tls disable + scale Add new scale down step
m1kola
left a comment
m1kola
left a comment
There was a problem hiding this comment.
I think this is a good candidate for validation on the API server side, but I'm approving because it is consistent with how we do validations for many other things already (unfortunately).
| return v1.ValidationSuccess() | ||
| } | ||
|
|
||
| func noSimultaneousTLSDisablingAndScaling(newObj, oldObj MongoDbSpec) v1.ValidationResult { |
There was a problem hiding this comment.
Looks like a good candidate for CEL validation on the API server so changes like this don't get even admitted to the storage.
There was a problem hiding this comment.
That's definitely something we can add to a Discuss agenda, or at least discuss in the team channel !
MaciejKaras
left a comment
MaciejKaras
left a comment
There was a problem hiding this comment.
Looks good!
One question though: do we have the same logic for multi and multi sharded? Are we also blocking scaling and disabling TLS for other resources?
This has to be checked for other resources, I opened a ticket |
Summary
We have code in the replica set controller dedicated to handling a scenario where the user disable TLS, and ascale their cluster at the same time. The test for this behaviour was broken. Because there were two functions with the name
test_tls_is_disabled_and_scaled_up, so pytest would run only one of them.When I fixed the test, it failed: https://spruce.mongodb.com/task/mongodb_kubernetes_e2e_mdb_kind_ubi_cloudqa_e2e_disable_tls_scale_up_patch_dfb9424e9b34ddd048a725a9988114ca4032f9bf_68de79b2fccf070007a2c51e_25_10_02_13_10_12/tests?execution=4&sorts=STATUS%3AASC
Which means the code to handle it was incorrect. My opinion is that it is better to block this change, rather than introducing complexity to handle it. It is not a common use case.
A changelog entry was added. Question: should it be explicitly mentioned in our public documentation ?
Old related PR (2021): https://github.com/10gen/ops-manager-kubernetes/pull/1444
And ticket: https://jira.mongodb.org/browse/CLOUDP-80768
Proof of Work
Unit and e2e test for blocking the operation should pass.
No regression due to the change in
updateOmDeploymentRsshould be observed.Checklist
skip-changeloglabel if not needed