CLOUDP-353180: Refactor replica set controller state handling, and use helper pattern

[Julien-Ben]

Summary

This PR refactors the ReplicaSet controller to use the helper pattern and separate state reading/writing, matching what we already do in the ShardedCluster and OpsManager controllers.

This is a no-op refactor: no behavior changes, just reorganizing code to prepare for multi-cluster support.

What Changed

Added ReplicaSetReconcilerHelper

• New helper struct that holds state for a single reconcile run
• Main reconcile logic moved from the controller to the helper
• Helper gets created fresh for each reconcile
• This is the pattern we used in our other reconcilers

Separated State Operations

• readState() - reads deployment state from annotations
• writeState() - writes deployment state back to annotations ; write vault annotations only on successful reconciliation. In the same way it is done in the sharded controller. (note that vault is not supported for multi replica set at this point)
• initialize() - loads state during helper creation
• This is done to clearly show where we handle data persisted on the cluster, as opposed to reading and writing in multiple places during the reconciliation. This is important for multi-cluster support

Helper's updateStatus() Override

• Writes deployment state after every status update
• Keeps state consistent even when we return early
• Same pattern as ShardedCluster controller

Added ReplicaSetDeploymentState

• Holds the state we persist between reconciles
• Just has LastAchievedSpec and status member count for now
• The status is still read during the reconciliation loop, because of the scaler. Refactoring the scaler is also something that will be done as part of the epic

Not in This PR

These will (potentially) come in follow-up PRs, in the main feature branch for multi cluster support:

• StateStore pattern (like ShardedCluster/OpsManager use)
• Use ConfigMap for state persistence
• State migration logic

Proof of work

• Existing tests pass without changes

Checklist

• Have you linked a jira ticket and/or is the ticket in the title?
• Have you checked whether your jira ticket required DOCSP changes?
• Have you added changelog file?
  • use skip-changelog label if not needed
  • refer to Changelog files and Release Notes section in CONTRIBUTING.md

[github-actions]

⚠️ (this preview might not be accurate if the PR is not rebased on current master branch)

MCK 1.6.0 Release Notes

New Features

• MongoDBCommunity: Added support to configure custom cluster domain via newly introduced spec.clusterDomain resource field. If spec.clusterDomain is not set, environment variable CLUSTER_DOMAIN is used as cluster domain. If the environment variable CLUSTER_DOMAIN is also not set, operator falls back to cluster.local as default cluster domain.
• Helm Chart: Introduced two new helm fields operator.podSecurityContext and operator.securityContext that can be used to configure securityContext for Operator deployment through Helm Chart.

Bug Fixes

• Fixed parsing of the customEnvVars Helm value when values contain = characters.
• ReplicaSet: Blocked disabling TLS and changing member count simultaneously. These operations must now be applied separately to prevent configuration inconsistencies.

Other Changes

• kubectl-mongodb plugin: cosign, the signing tool that is used to sign kubectl-mongodb plugin binaries, has been updated to version 3.0.2. With this change, released binaries will be bundled with .bundle files containing both signature and certificate information. For more information on how to verify signatures using new cosign version please refer to -> https://github.com/sigstore/cosign/blob/v3.0.2/doc/cosign_verify-blob.md

[m1kola]

This is in writeState method. Is vault integration related to deployment state?

[Julien-Ben]

We were storing it in annotations on successful reconciliations previously so I treated it as state:

mongodb-kubernetes/controllers/operator/mongodbreplicaset_controller.go

Line 269 in 582d248

if vault.IsVaultSecretBackend() {

[m1kola]

@Julien-Ben I don't know a lot about Vault integration, but it seems unrelated to the deployment state to me. I also see that a similar piece of code in the sharded controller is is part of ShardedClusterReconcileHelper.Reconcile method:

mongodb-kubernetes/controllers/operator/mongodbshardedcluster_controller.go

Lines 928 to 940 in 582d248

	if vault.IsVaultSecretBackend() {
	secrets := sc.GetSecretsMountedIntoDBPod()
	vaultMap := make(map[string]string)
	for _, s := range secrets {
	path := fmt.Sprintf("%s/%s/%s", r.commonController.VaultClient.DatabaseSecretMetadataPath(), sc.Namespace, s)
	vaultMap = merge.StringToStringMap(vaultMap, r.commonController.VaultClient.GetSecretAnnotation(path))
	}
	path := fmt.Sprintf("%s/%s/%s", r.commonController.VaultClient.OperatorScretMetadataPath(), sc.Namespace, sc.Spec.Credentials)
	vaultMap = merge.StringToStringMap(vaultMap, r.commonController.VaultClient.GetSecretAnnotation(path))
	for k, val := range vaultMap {
	annotationsToAdd[k] = val
	}
	}

Should we follow the same pattern?

[Julien-Ben]

I separated them, you were right.
Vault just needs to read annotations, and they are not state as we don't rely on them ourselves. They should be written only upon successful reconcile

a777b2f

I also changed how we write to lastAchievedSpec
It is actually an annotation we should change only when we achieve Running phase, not on every reconcile.

[Julien-Ben]

@m1kola FYI after your review I added these unit tests

[Julien-Ben]

This function is now used by the standalone controller only.
The reason we don't want it any more is because it acceeds the spec with m.GetLastSpec()

We now do it only once per reconcile

[m1kola]

Looks good. The only concern is an unnecessary write because of two annotations.SetAnnotations calls.