Skip to content

Commit f806910

Browse files
authored
CLOUDP-353180: Refactor replica set controller state handling, and use helper pattern (#544)
# 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 - [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
1 parent b478944 commit f806910

File tree

4 files changed

+429
-156
lines changed

4 files changed

+429
-156
lines changed

api/v1/mdb/mongodb_types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,9 @@ func GetLastAdditionalMongodConfigByType(lastSpec *MongoDbSpec, configType Addit
243243

244244
// GetLastAdditionalMongodConfigByType returns the last successfully achieved AdditionalMongodConfigType for the given component.
245245
func (m *MongoDB) GetLastAdditionalMongodConfigByType(configType AdditionalMongodConfigType) (*AdditionalMongodConfig, error) {
246+
if m.Spec.GetResourceType() == ReplicaSet {
247+
panic(errors.Errorf("this method cannot be used from ReplicaSet controller; use non-method GetLastAdditionalMongodConfigByType and pass lastSpec from the deployment state."))
248+
}
246249
if m.Spec.GetResourceType() == ShardedCluster {
247250
panic(errors.Errorf("this method cannot be used from ShardedCluster controller; use non-method GetLastAdditionalMongodConfigByType and pass lastSpec from the deployment state."))
248251
}

controllers/om/deployment/testing_utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func CreateFromReplicaSet(mongoDBImage string, forceEnterprise bool, rs *mdb.Mon
2626
), zap.S())
2727
d := om.NewDeployment()
2828

29-
lastConfig, err := rs.GetLastAdditionalMongodConfigByType(mdb.ReplicaSetConfig)
29+
lastConfig, err := mdb.GetLastAdditionalMongodConfigByType(nil, mdb.ReplicaSetConfig)
3030
if err != nil {
3131
panic(err)
3232
}

0 commit comments

Comments
 (0)