CLOUDP-317886 - block removing cluster from MC Sharded deployment (#495)

# Summary

During investigation of https://jira.mongodb.org/browse/CLOUDP-317886 we
have found that the operator panics, when the cluster is removed from
`clusterSpecList`. Initially we thought about allowing to do that, but
later on I have discussed the issue with @lsierant and the better
approach was chosen:
- when the user wants to remove cluster that already has 0 members, we
allow this
- when the user wants to remove cluster that has more than 0 members, we
block this, because it can lead to unexpected issues. This is described
in the code comment:


https://github.com/mongodb/mongodb-kubernetes/blob/31c346ee4770c3caabb620f3e7ad3f38ebdc4d3c/controllers/operator/mongodbshardedcluster_controller.go#L840-L846

...and in the changelog:

**MultiClusterSharded**: Block removing non-zero member cluster from
MongoDB resource. This prevents from scaling down member cluster without
current configuration available, which can lead to unexpected issues.
Previously operator was crashing in that scenario, after the fix it will
mark reconciliation as `Failed` with appropriate message. Example unsafe
scenario that is now blocked:
* User has 2 member clusters: `main` is used for application traffic,
`read-analytics` is used for read-only analytics
  * `main` cluster has 7 voting members
  * `read-analytics` cluster has 3 non-voting members
* User decides to remove `read-analytics` cluster, by removing the
`clusterSpecItem` completely
* Operator scales down members from `read-analytics` cluster one by one
* Because the configuration does not have voting options specified
anymore and by default `priority` is set to 1, the operator will remove
one member, but the other two members will be reconfigured as voting
members
* `replicaset` contains now 9 voting members, which is not [supported by
MongoDB](https://www.mongodb.com/docs/manual/reference/limits/#mongodb-limit-Number-of-Voting-Members-of-a-Replica-Set)

## Proof of Work

Passing CI + new unit tests for `blockNonEmptyClusterSpecItemRemoval`
function.

## Checklist

- [x] Have you linked a jira ticket and/or is the ticket in the title?
- [x] Have you checked whether your jira ticket required DOCSP changes?
- [x] Have you added changelog file?
    - use `skip-changelog` label if not needed
- refer to [Changelog files and Release
Notes](https://github.com/mongodb/mongodb-kubernetes/blob/master/CONTRIBUTING.md#changelog-files-and-release-notes)
section in CONTRIBUTING.md for more details

---------

Co-authored-by: Vivek Singh <vsingh.ggits.2010@gmail.com>

Author: MaciejKaras

api/v1/mdb/mongodb_types.go

@@ -25,6 +25,7 @@ import (
 	"github.com/mongodb/mongodb-kubernetes/pkg/dns"
 	"github.com/mongodb/mongodb-kubernetes/pkg/fcv"
 	"github.com/mongodb/mongodb-kubernetes/pkg/kube"
+	"github.com/mongodb/mongodb-kubernetes/pkg/multicluster"
 	"github.com/mongodb/mongodb-kubernetes/pkg/util"
 	"github.com/mongodb/mongodb-kubernetes/pkg/util/env"
 	"github.com/mongodb/mongodb-kubernetes/pkg/util/stringutil"
@@ -1665,6 +1666,48 @@ func (m *MongoDbSpec) IsMultiCluster() bool {
 	return m.GetTopology() == ClusterTopologyMultiCluster
 }
 
+func (m *MongoDbSpec) GetShardClusterSpecList() ClusterSpecList {
+	if m.IsMultiCluster() {
+		return m.ShardSpec.ClusterSpecList
+	} else {
+		return ClusterSpecList{
+			{
+				ClusterName:  multicluster.LegacyCentralClusterName,
+				Members:      m.MongodsPerShardCount,
+				MemberConfig: m.MemberConfig,
+			},
+		}
+	}
+}
+
+func (m *MongoDbSpec) GetMongosClusterSpecList() ClusterSpecList {
+	if m.IsMultiCluster() {
+		return m.MongosSpec.ClusterSpecList
+	} else {
+		return ClusterSpecList{
+			{
+				ClusterName:                 multicluster.LegacyCentralClusterName,
+				Members:                     m.MongosCount,
+				ExternalAccessConfiguration: m.ExternalAccessConfiguration,
+			},
+		}
+	}
+}
+
+func (m *MongoDbSpec) GetConfigSrvClusterSpecList() ClusterSpecList {
+	if m.IsMultiCluster() {
+		return m.ConfigSrvSpec.ClusterSpecList
+	} else {
+		return ClusterSpecList{
+			{
+				ClusterName:  multicluster.LegacyCentralClusterName,
+				Members:      m.ConfigServerCount,
+				MemberConfig: m.MemberConfig,
+			},
+		}
+	}
+}
+
 type MongoDBConnectionStringBuilder struct {
 	MongoDB
 	hostnames []string

api/v1/mdb/shardedcluster.go

@@ -91,12 +91,25 @@ func (s *ShardedClusterComponentSpec) GetAgentConfig() *AgentConfig {
 	return &s.Agent
 }
 
+func (s *ShardedClusterComponentSpec) ClusterSpecItemExists(clusterName string) bool {
+	return s.getClusterSpecItemOrNil(clusterName) != nil
+}
+
 func (s *ShardedClusterComponentSpec) GetClusterSpecItem(clusterName string) ClusterSpecItem {
+	if clusterSpecItem := s.getClusterSpecItemOrNil(clusterName); clusterSpecItem != nil {
+		return *clusterSpecItem
+	}
+
+	// it should never occur - we preprocess all clusterSpecLists
+	panic(fmt.Errorf("clusterName %s not found in clusterSpecList", clusterName))
+}
+
+func (s *ShardedClusterComponentSpec) getClusterSpecItemOrNil(clusterName string) *ClusterSpecItem {
 	for i := range s.ClusterSpecList {
 		if s.ClusterSpecList[i].ClusterName == clusterName {
-			return s.ClusterSpecList[i]
+			return &s.ClusterSpecList[i]
 		}
 	}
-	// it should never occur - we preprocess all clusterSpecLists
-	panic(fmt.Errorf("clusterName %s not found in clusterSpecList", clusterName))
+
+	return nil
 }

api/v1/om/appdb_types.go

@@ -494,6 +494,10 @@ func (m *AppDBSpec) GetMemberClusterSpecByName(memberClusterName string) mdbv1.C
 
 	// In case the member cluster is not found in the cluster spec list, we return an empty ClusterSpecItem
 	// with 0 members to handle the case of removing a cluster from the spec list without a panic.
+	//
+	// This is not ideal, because we don't consider other fields that were removed i.e.MemberConfig.
+	// When scaling down we should consider the full spec that was used to create the cluster.
+	// https://jira.mongodb.org/browse/CLOUDP-349925
 	return mdbv1.ClusterSpecItem{
 		ClusterName: memberClusterName,
 		Members:     0,

changelog/20251006_fix_block_removing_non_zero_member_cluster_from.md

@@ -0,0 +1,6 @@
+---
+kind: fix
+date: 2025-10-06
+---
+
+* **MultiClusterSharded**: Blocked removing non-zero member cluster from MongoDB resource. This prevents from scaling down member cluster without current configuration available, which could lead to unexpected issues.

controllers/operator/authentication_test.go

@@ -65,7 +65,7 @@ func TestX509ClusterAuthentication_CanBeEnabled_IfX509AuthenticationIsEnabled_Sh
 	ctx := context.Background()
 	scWithTls := test.DefaultClusterBuilder().EnableTLS().EnableX509().SetName("sc-with-tls").SetTLSCA("custom-ca").Build()
 
-	reconciler, _, client, _, err := defaultClusterReconciler(ctx, nil, "", "", scWithTls, nil)
+	reconciler, _, client, _, err := defaultShardedClusterReconciler(ctx, nil, "", "", scWithTls, nil)
 	require.NoError(t, err)
 	addKubernetesTlsResources(ctx, client, scWithTls)
 
@@ -76,7 +76,7 @@ func TestX509CanBeEnabled_WhenThereAreOnlyTlsDeployments_ShardedCluster(t *testi
 	ctx := context.Background()
 	scWithTls := test.DefaultClusterBuilder().EnableTLS().EnableX509().SetName("sc-with-tls").SetTLSCA("custom-ca").Build()
 
-	reconciler, _, client, _, err := defaultClusterReconciler(ctx, nil, "", "", scWithTls, nil)
+	reconciler, _, client, _, err := defaultShardedClusterReconciler(ctx, nil, "", "", scWithTls, nil)
 	require.NoError(t, err)
 	addKubernetesTlsResources(ctx, client, scWithTls)
 
@@ -333,7 +333,7 @@ func TestX509InternalClusterAuthentication_CanBeEnabledWithScram_ShardedCluster(
 		EnableX509InternalClusterAuth().
 		Build()
 
-	r, _, kubeClient, omConnectionFactory, _ := defaultClusterReconciler(ctx, nil, "", "", sc, nil)
+	r, _, kubeClient, omConnectionFactory, _ := defaultShardedClusterReconciler(ctx, nil, "", "", sc, nil)
 	addKubernetesTlsResources(ctx, r.client, sc)
 	checkReconcileSuccessful(ctx, t, r, sc, kubeClient)
 
@@ -770,15 +770,16 @@ func Test_NoAdditionalDomainsPresent(t *testing.T) {
 	// The default secret we create does not contain additional domains so it will not be valid for this RS
 	rs.Spec.Security.TLSConfig.AdditionalCertificateDomains = []string{"foo"}
 
-	reconciler, _, client, _, err := defaultClusterReconciler(ctx, nil, "", "", rs, nil)
-	require.NoError(t, err)
-	addKubernetesTlsResources(ctx, client, rs)
+	kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs)
+	reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, omConnectionFactory.GetConnectionFunc)
+	addKubernetesTlsResources(ctx, kubeClient, rs)
 
-	secret := &corev1.Secret{}
+	certSecret := &corev1.Secret{}
 
-	_ = client.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s-cert", rs.Name), Namespace: rs.Namespace}, secret)
+	_ = kubeClient.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s-cert", rs.Name), Namespace: rs.Namespace}, certSecret)
 
-	err = certs.VerifyAndEnsureCertificatesForStatefulSet(ctx, reconciler.SecretClient, reconciler.SecretClient, fmt.Sprintf("%s-cert", rs.Name), certs.ReplicaSetConfig(*rs), nil)
+	err := certs.VerifyAndEnsureCertificatesForStatefulSet(ctx, reconciler.SecretClient, reconciler.SecretClient, fmt.Sprintf("%s-cert", rs.Name), certs.ReplicaSetConfig(*rs), nil)
+	require.Error(t, err)
 	for i := 0; i < rs.Spec.Members; i++ {
 		expectedErrorMessage := fmt.Sprintf("domain %s-%d.foo is not contained in the list of DNSNames", rs.Name, i)
 		assert.Contains(t, err.Error(), expectedErrorMessage)