From 90387b0248e48be50be972b7e699b6ca647507ac Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Wed, 15 Oct 2025 22:26:50 +0200 Subject: [PATCH 01/25] Refactor state reading --- api/v1/mdb/mongodb_types.go | 3 ++ controllers/om/deployment/testing_utils.go | 2 +- .../operator/mongodbreplicaset_controller.go | 41 +++++++++++++++---- .../mongodbreplicaset_controller_test.go | 8 ++-- 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/api/v1/mdb/mongodb_types.go b/api/v1/mdb/mongodb_types.go index a009deaa3..199146431 100644 --- a/api/v1/mdb/mongodb_types.go +++ b/api/v1/mdb/mongodb_types.go @@ -243,6 +243,9 @@ func GetLastAdditionalMongodConfigByType(lastSpec *MongoDbSpec, configType Addit // GetLastAdditionalMongodConfigByType returns the last successfully achieved AdditionalMongodConfigType for the given component. func (m *MongoDB) GetLastAdditionalMongodConfigByType(configType AdditionalMongodConfigType) (*AdditionalMongodConfig, error) { + if m.Spec.GetResourceType() == ReplicaSet { + panic(errors.Errorf("this method cannot be used from ReplicaSet controller; use non-method GetLastAdditionalMongodConfigByType and pass lastSpec from the deployment state.")) + } if m.Spec.GetResourceType() == ShardedCluster { panic(errors.Errorf("this method cannot be used from ShardedCluster controller; use non-method GetLastAdditionalMongodConfigByType and pass lastSpec from the deployment state.")) } diff --git a/controllers/om/deployment/testing_utils.go b/controllers/om/deployment/testing_utils.go index d62a1aca2..79f0ebf82 100644 --- a/controllers/om/deployment/testing_utils.go +++ b/controllers/om/deployment/testing_utils.go @@ -26,7 +26,7 @@ func CreateFromReplicaSet(mongoDBImage string, forceEnterprise bool, rs *mdb.Mon ), zap.S()) d := om.NewDeployment() - lastConfig, err := rs.GetLastAdditionalMongodConfigByType(mdb.ReplicaSetConfig) + lastConfig, err := mdb.GetLastAdditionalMongodConfigByType(nil, mdb.ReplicaSetConfig) if err != nil { panic(err) } diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index bb30016a9..cd433238a 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -71,13 +71,36 @@ type ReconcileMongoDbReplicaSet struct { imageUrls images.ImageUrls forceEnterprise bool enableClusterMongoDBRoles bool + deploymentState *ReplicaSetDeploymentState initDatabaseNonStaticImageVersion string databaseNonStaticImageVersion string } +type ReplicaSetDeploymentState struct { + LastAchievedSpec *mdbv1.MongoDbSpec `json:"lastAchievedSpec"` +} + +func readState(rs *mdbv1.MongoDB) (*ReplicaSetDeploymentState, error) { + // Try to get the last achieved spec from annotations and store it in state + if lastAchievedSpec, err := rs.GetLastSpec(); err != nil { + return nil, err + } else { + return &ReplicaSetDeploymentState{LastAchievedSpec: lastAchievedSpec}, nil + } +} + var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} +func (r *ReconcileMongoDbReplicaSet) initializeState(rs *mdbv1.MongoDB) error { + if state, err := readState(rs); err == nil { + r.deploymentState = state + return nil + } else { + return err + } +} + func newReplicaSetReconciler(ctx context.Context, kubeClient client.Client, imageUrls images.ImageUrls, initDatabaseNonStaticImageVersion, databaseNonStaticImageVersion string, forceEnterprise bool, enableClusterMongoDBRoles bool, omFunc om.ConnectionFactory) *ReconcileMongoDbReplicaSet { return &ReconcileMongoDbReplicaSet{ ReconcileCommonController: NewReconcileCommonController(ctx, kubeClient), @@ -128,6 +151,10 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco return reconcileResult, err } + if err := r.initializeState(rs); err != nil { + return r.updateStatus(ctx, rs, workflow.Failed(xerrors.Errorf("Failed to initialize replica set state: %w", err)), log) + } + if !architectures.IsRunningStaticArchitecture(rs.Annotations) { agents.UpgradeAllIfNeeded(ctx, agents.ClientSecret{Client: r.client, SecretClient: r.SecretClient}, r.omConnectionFactory, GetWatchedNamespace(), false) } @@ -238,12 +265,8 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco } // 5. Actual reconciliation execution, Ops Manager and kubernetes resources update - lastSpec, err := rs.GetLastSpec() - if err != nil { - lastSpec = &mdbv1.MongoDbSpec{} - } - publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, r.client, *rs, lastSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) + publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, r.client, *rs, r.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, log, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") @@ -580,7 +603,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c caFilePath := fmt.Sprintf("%s/ca-pem", util.TLSCaMountPath) // If current operation is to Disable TLS, then we should the current members of the Replica Set, // this is, do not scale them up or down util TLS disabling has completed. - shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, log, caFilePath, tlsCertPath) + shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, r.deploymentState.LastAchievedSpec, log) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -611,7 +634,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c return status } - lastRsConfig, err := rs.GetLastAdditionalMongodConfigByType(mdbv1.ReplicaSetConfig) + lastRsConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(r.deploymentState.LastAchievedSpec, mdbv1.ReplicaSetConfig) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -671,7 +694,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c // updateOmDeploymentDisableTLSConfiguration checks if TLS configuration needs // to be disabled. In which case it will disable it and inform to the calling // function. -func updateOmDeploymentDisableTLSConfiguration(conn om.Connection, mongoDBImage string, forceEnterprise bool, membersNumberBefore int, rs *mdbv1.MongoDB, log *zap.SugaredLogger, caFilePath, tlsCertPath string) (bool, error) { +func updateOmDeploymentDisableTLSConfiguration(conn om.Connection, mongoDBImage string, forceEnterprise bool, membersNumberBefore int, rs *mdbv1.MongoDB, caFilePath, tlsCertPath string, lastSpec *mdbv1.MongoDbSpec, log *zap.SugaredLogger) (bool, error) { tlsConfigWasDisabled := false err := conn.ReadUpdateDeployment( @@ -687,7 +710,7 @@ func updateOmDeploymentDisableTLSConfiguration(conn om.Connection, mongoDBImage // there's a scale up change at the same time). replicaSet := replicaset.BuildFromMongoDBWithReplicas(mongoDBImage, forceEnterprise, rs, membersNumberBefore, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath) - lastConfig, err := rs.GetLastAdditionalMongodConfigByType(mdbv1.ReplicaSetConfig) + lastConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(lastSpec, mdbv1.ReplicaSetConfig) if err != nil { return err } diff --git a/controllers/operator/mongodbreplicaset_controller_test.go b/controllers/operator/mongodbreplicaset_controller_test.go index 787883ffb..dfda4efc9 100644 --- a/controllers/operator/mongodbreplicaset_controller_test.go +++ b/controllers/operator/mongodbreplicaset_controller_test.go @@ -398,22 +398,22 @@ func TestUpdateDeploymentTLSConfiguration(t *testing.T) { deploymentNoTLS := deployment.CreateFromReplicaSet("fake-mongoDBImage", false, rsNoTLS) // TLS Disabled -> TLS Disabled - shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentNoTLS), "fake-mongoDBImage", false, 3, rsNoTLS, zap.S(), util.CAFilePathInContainer, "") + shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentNoTLS), "fake-mongoDBImage", false, 3, rsNoTLS, util.CAFilePathInContainer, "", nil, zap.S()) assert.NoError(t, err) assert.False(t, shouldLockMembers) // TLS Disabled -> TLS Enabled - shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentNoTLS), "fake-mongoDBImage", false, 3, rsWithTLS, zap.S(), util.CAFilePathInContainer, "") + shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentNoTLS), "fake-mongoDBImage", false, 3, rsWithTLS, util.CAFilePathInContainer, "", nil, zap.S()) assert.NoError(t, err) assert.False(t, shouldLockMembers) // TLS Enabled -> TLS Enabled - shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentWithTLS), "fake-mongoDBImage", false, 3, rsWithTLS, zap.S(), util.CAFilePathInContainer, "") + shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentWithTLS), "fake-mongoDBImage", false, 3, rsWithTLS, util.CAFilePathInContainer, "", nil, zap.S()) assert.NoError(t, err) assert.False(t, shouldLockMembers) // TLS Enabled -> TLS Disabled - shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentWithTLS), "fake-mongoDBImage", false, 3, rsNoTLS, zap.S(), util.CAFilePathInContainer, "") + shouldLockMembers, err = updateOmDeploymentDisableTLSConfiguration(om.NewMockedOmConnection(deploymentWithTLS), "fake-mongoDBImage", false, 3, rsNoTLS, util.CAFilePathInContainer, "", nil, zap.S()) assert.NoError(t, err) assert.True(t, shouldLockMembers) } From bf0ead11a3499c25477b56aa989d6d9cbf688712 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Fri, 17 Oct 2025 21:51:25 +0200 Subject: [PATCH 02/25] Refactor state writing --- .../operator/mongodbreplicaset_controller.go | 82 +++++++++++++------ 1 file changed, 56 insertions(+), 26 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index cd433238a..4e4b010f9 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -81,7 +81,10 @@ type ReplicaSetDeploymentState struct { LastAchievedSpec *mdbv1.MongoDbSpec `json:"lastAchievedSpec"` } -func readState(rs *mdbv1.MongoDB) (*ReplicaSetDeploymentState, error) { +var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} + +// readState abstract reading the state of the resource that we store on the cluster between reconciliations. +func (r *ReconcileMongoDbReplicaSet) readState(rs *mdbv1.MongoDB) (*ReplicaSetDeploymentState, error) { // Try to get the last achieved spec from annotations and store it in state if lastAchievedSpec, err := rs.GetLastSpec(); err != nil { return nil, err @@ -90,10 +93,40 @@ func readState(rs *mdbv1.MongoDB) (*ReplicaSetDeploymentState, error) { } } -var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} +// writeState abstract writing the state of the resource that we store on the cluster between reconciliations. +func (r *ReconcileMongoDbReplicaSet) writeState(ctx context.Context, rs *mdbv1.MongoDB, log *zap.SugaredLogger) error { + // Serialize the state to annotations + annotationsToAdd, err := getAnnotationsForResource(rs) + if err != nil { + return err + } + + // Add vault annotations if needed + if vault.IsVaultSecretBackend() { + secrets := rs.GetSecretsMountedIntoDBPod() + vaultMap := make(map[string]string) + for _, s := range secrets { + path := fmt.Sprintf("%s/%s/%s", r.VaultClient.DatabaseSecretMetadataPath(), rs.Namespace, s) + vaultMap = merge.StringToStringMap(vaultMap, r.VaultClient.GetSecretAnnotation(path)) + } + path := fmt.Sprintf("%s/%s/%s", r.VaultClient.OperatorScretMetadataPath(), rs.Namespace, rs.Spec.Credentials) + vaultMap = merge.StringToStringMap(vaultMap, r.VaultClient.GetSecretAnnotation(path)) + for k, val := range vaultMap { + annotationsToAdd[k] = val + } + } + + // Write annotations back to the resource + if err := annotations.SetAnnotations(ctx, rs, annotationsToAdd, r.client); err != nil { + return err + } + + log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", rs.Namespace, rs.Name) + return nil +} func (r *ReconcileMongoDbReplicaSet) initializeState(rs *mdbv1.MongoDB) error { - if state, err := readState(rs); err == nil { + if state, err := r.readState(rs); err == nil { r.deploymentState = state return nil } else { @@ -101,6 +134,23 @@ func (r *ReconcileMongoDbReplicaSet) initializeState(rs *mdbv1.MongoDB) error { } } +// updateStatus overrides the common controller's updateStatus to ensure that the deployment state +// is written after every status update. This ensures state consistency even on early returns. +// It must be executed only once per reconcile (with a return) +func (r *ReconcileMongoDbReplicaSet) updateStatus(ctx context.Context, resource *mdbv1.MongoDB, status workflow.Status, log *zap.SugaredLogger, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { + result, err := r.ReconcileCommonController.updateStatus(ctx, resource, status, log, statusOptions...) + if err != nil { + return result, err + } + + // Write deployment state after every status update + if err := r.writeState(ctx, resource, log); err != nil { + return r.ReconcileCommonController.updateStatus(ctx, resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), log) + } + + return result, nil +} + func newReplicaSetReconciler(ctx context.Context, kubeClient client.Client, imageUrls images.ImageUrls, initDatabaseNonStaticImageVersion, databaseNonStaticImageVersion string, forceEnterprise bool, enableClusterMongoDBRoles bool, omFunc om.ConnectionFactory) *ReconcileMongoDbReplicaSet { return &ReconcileMongoDbReplicaSet{ ReconcileCommonController: NewReconcileCommonController(ctx, kubeClient), @@ -284,29 +334,6 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco return r.updateStatus(ctx, rs, workflow.Pending("Continuing scaling operation for ReplicaSet %s, desiredMembers=%d, currentMembers=%d", rs.ObjectKey(), rs.DesiredReplicas(), scale.ReplicasThisReconciliation(rs)), log, mdbstatus.MembersOption(rs)) } - annotationsToAdd, err := getAnnotationsForResource(rs) - if err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(err), log) - } - - if vault.IsVaultSecretBackend() { - secrets := rs.GetSecretsMountedIntoDBPod() - vaultMap := make(map[string]string) - for _, s := range secrets { - path := fmt.Sprintf("%s/%s/%s", r.VaultClient.DatabaseSecretMetadataPath(), rs.Namespace, s) - vaultMap = merge.StringToStringMap(vaultMap, r.VaultClient.GetSecretAnnotation(path)) - } - path := fmt.Sprintf("%s/%s/%s", r.VaultClient.OperatorScretMetadataPath(), rs.Namespace, rs.Spec.Credentials) - vaultMap = merge.StringToStringMap(vaultMap, r.VaultClient.GetSecretAnnotation(path)) - for k, val := range vaultMap { - annotationsToAdd[k] = val - } - } - - if err := annotations.SetAnnotations(ctx, rs, annotationsToAdd, r.client); err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(err), log) - } - log.Infof("Finished reconciliation for MongoDbReplicaSet! %s", completionMessage(conn.BaseURL(), conn.GroupID())) return r.updateStatus(ctx, rs, workflow.OK(), log, mdbstatus.NewBaseUrlOption(deployment.Link(conn.BaseURL(), conn.GroupID())), mdbstatus.MembersOption(rs), mdbstatus.NewPVCsStatusOptionEmptyStatus()) } @@ -443,6 +470,8 @@ func (r *ReconcileMongoDbReplicaSet) reconcileStatefulSet(ctx context.Context, r if !workflowStatus.IsOK() { return workflowStatus } + + // TODO: check if updatestatus usage is correct here if workflow.ContainsPVCOption(workflowStatus.StatusOptions()) { _, _ = r.updateStatus(ctx, rs, workflow.Pending(""), log, workflowStatus.StatusOptions()...) } @@ -683,6 +712,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c return workflow.Failed(err) } + //TODO: check if updateStatus usage is correct hee if status := r.ensureBackupConfigurationAndUpdateStatus(ctx, conn, rs, r.SecretClient, log); !status.IsOK() && !isRecovering { return status } From ab6e334cd1b585cd718aa6fbd3c136bec5bf40f5 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Fri, 17 Oct 2025 22:27:37 +0200 Subject: [PATCH 03/25] Refactor to helper pattern --- .../operator/mongodbreplicaset_controller.go | 233 +++++++++++------- 1 file changed, 138 insertions(+), 95 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 4e4b010f9..929af5fcb 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -64,14 +64,15 @@ import ( "github.com/mongodb/mongodb-kubernetes/pkg/vault/vaultwatcher" ) -// ReconcileMongoDbReplicaSet reconciles a MongoDB with a type of ReplicaSet +// ReconcileMongoDbReplicaSet reconciles a MongoDB with a type of ReplicaSet. +// WARNING: do not put any mutable state into this struct. +// Controller runtime uses and shares a single instance of it. type ReconcileMongoDbReplicaSet struct { *ReconcileCommonController omConnectionFactory om.ConnectionFactory imageUrls images.ImageUrls forceEnterprise bool enableClusterMongoDBRoles bool - deploymentState *ReplicaSetDeploymentState initDatabaseNonStaticImageVersion string databaseNonStaticImageVersion string @@ -83,10 +84,37 @@ type ReplicaSetDeploymentState struct { var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} +// ReplicaSetReconcilerHelper contains state and logic for a SINGLE reconcile execution. +// This object is NOT shared between reconcile invocations. +type ReplicaSetReconcilerHelper struct { + resource *mdbv1.MongoDB + deploymentState *ReplicaSetDeploymentState + reconciler *ReconcileMongoDbReplicaSet + log *zap.SugaredLogger +} + +func (r *ReconcileMongoDbReplicaSet) newReconcilerHelper( + ctx context.Context, + rs *mdbv1.MongoDB, + log *zap.SugaredLogger, +) (*ReplicaSetReconcilerHelper, error) { + helper := &ReplicaSetReconcilerHelper{ + resource: rs, + reconciler: r, + log: log, + } + + if err := helper.initialize(ctx); err != nil { + return nil, err + } + + return helper, nil +} + // readState abstract reading the state of the resource that we store on the cluster between reconciliations. -func (r *ReconcileMongoDbReplicaSet) readState(rs *mdbv1.MongoDB) (*ReplicaSetDeploymentState, error) { +func (h *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, error) { // Try to get the last achieved spec from annotations and store it in state - if lastAchievedSpec, err := rs.GetLastSpec(); err != nil { + if lastAchievedSpec, err := h.resource.GetLastSpec(); err != nil { return nil, err } else { return &ReplicaSetDeploymentState{LastAchievedSpec: lastAchievedSpec}, nil @@ -94,116 +122,70 @@ func (r *ReconcileMongoDbReplicaSet) readState(rs *mdbv1.MongoDB) (*ReplicaSetDe } // writeState abstract writing the state of the resource that we store on the cluster between reconciliations. -func (r *ReconcileMongoDbReplicaSet) writeState(ctx context.Context, rs *mdbv1.MongoDB, log *zap.SugaredLogger) error { +func (h *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { // Serialize the state to annotations - annotationsToAdd, err := getAnnotationsForResource(rs) + annotationsToAdd, err := getAnnotationsForResource(h.resource) if err != nil { return err } // Add vault annotations if needed if vault.IsVaultSecretBackend() { - secrets := rs.GetSecretsMountedIntoDBPod() + secrets := h.resource.GetSecretsMountedIntoDBPod() vaultMap := make(map[string]string) for _, s := range secrets { - path := fmt.Sprintf("%s/%s/%s", r.VaultClient.DatabaseSecretMetadataPath(), rs.Namespace, s) - vaultMap = merge.StringToStringMap(vaultMap, r.VaultClient.GetSecretAnnotation(path)) + path := fmt.Sprintf("%s/%s/%s", h.reconciler.VaultClient.DatabaseSecretMetadataPath(), h.resource.Namespace, s) + vaultMap = merge.StringToStringMap(vaultMap, h.reconciler.VaultClient.GetSecretAnnotation(path)) } - path := fmt.Sprintf("%s/%s/%s", r.VaultClient.OperatorScretMetadataPath(), rs.Namespace, rs.Spec.Credentials) - vaultMap = merge.StringToStringMap(vaultMap, r.VaultClient.GetSecretAnnotation(path)) + path := fmt.Sprintf("%s/%s/%s", h.reconciler.VaultClient.OperatorScretMetadataPath(), h.resource.Namespace, h.resource.Spec.Credentials) + vaultMap = merge.StringToStringMap(vaultMap, h.reconciler.VaultClient.GetSecretAnnotation(path)) for k, val := range vaultMap { annotationsToAdd[k] = val } } // Write annotations back to the resource - if err := annotations.SetAnnotations(ctx, rs, annotationsToAdd, r.client); err != nil { + if err := annotations.SetAnnotations(ctx, h.resource, annotationsToAdd, h.reconciler.client); err != nil { return err } - log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", rs.Namespace, rs.Name) + h.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", h.resource.Namespace, h.resource.Name) return nil } -func (r *ReconcileMongoDbReplicaSet) initializeState(rs *mdbv1.MongoDB) error { - if state, err := r.readState(rs); err == nil { - r.deploymentState = state - return nil - } else { - return err +func (h *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { + state, err := h.readState() + if err != nil { + return xerrors.Errorf("Failed to initialize replica set state: %w", err) } + h.deploymentState = state + return nil } // updateStatus overrides the common controller's updateStatus to ensure that the deployment state // is written after every status update. This ensures state consistency even on early returns. // It must be executed only once per reconcile (with a return) -func (r *ReconcileMongoDbReplicaSet) updateStatus(ctx context.Context, resource *mdbv1.MongoDB, status workflow.Status, log *zap.SugaredLogger, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { - result, err := r.ReconcileCommonController.updateStatus(ctx, resource, status, log, statusOptions...) +func (h *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status workflow.Status, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { + result, err := h.reconciler.ReconcileCommonController.updateStatus(ctx, h.resource, status, h.log, statusOptions...) if err != nil { return result, err } // Write deployment state after every status update - if err := r.writeState(ctx, resource, log); err != nil { - return r.ReconcileCommonController.updateStatus(ctx, resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), log) + if err := h.writeState(ctx); err != nil { + return h.reconciler.ReconcileCommonController.updateStatus(ctx, h.resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), h.log) } return result, nil } -func newReplicaSetReconciler(ctx context.Context, kubeClient client.Client, imageUrls images.ImageUrls, initDatabaseNonStaticImageVersion, databaseNonStaticImageVersion string, forceEnterprise bool, enableClusterMongoDBRoles bool, omFunc om.ConnectionFactory) *ReconcileMongoDbReplicaSet { - return &ReconcileMongoDbReplicaSet{ - ReconcileCommonController: NewReconcileCommonController(ctx, kubeClient), - omConnectionFactory: omFunc, - imageUrls: imageUrls, - forceEnterprise: forceEnterprise, - enableClusterMongoDBRoles: enableClusterMongoDBRoles, - - initDatabaseNonStaticImageVersion: initDatabaseNonStaticImageVersion, - databaseNonStaticImageVersion: databaseNonStaticImageVersion, - } -} - -type deploymentOptionsRS struct { - agentCertPath string - agentCertHash string - prometheusCertHash string - currentAgentAuthMode string -} - -// Generic Kubernetes Resources -// +kubebuilder:rbac:groups=core,resources=namespaces,verbs=list;watch,namespace=placeholder -// +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch,namespace=placeholder -// +kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;update,namespace=placeholder -// +kubebuilder:rbac:groups=core,resources={secrets,configmaps},verbs=get;list;watch;create;delete;update,namespace=placeholder -// +kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=create;get;list;watch;delete;update,namespace=placeholder - -// MongoDB Resource -// +kubebuilder:rbac:groups=mongodb.com,resources={mongodb,mongodb/status,mongodb/finalizers},verbs=*,namespace=placeholder - -// Setting up a webhook -// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;create;update;delete - -// Certificate generation -// +kubebuilder:rbac:groups=certificates.k8s.io,resources=certificatesigningrequests,verbs=get;create;list;watch - -// Reconcile reads that state of the cluster for a MongoDbReplicaSet object and makes changes based on the state read -// and what is in the MongoDbReplicaSet.Spec -func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reconcile.Request) (res reconcile.Result, e error) { - // === 1. Initial Checks and setup - log := zap.S().With("ReplicaSet", request.NamespacedName) - rs := &mdbv1.MongoDB{} - - if reconcileResult, err := r.prepareResourceForReconciliation(ctx, request, rs, log); err != nil { - if errors.IsNotFound(err) { - return workflow.Invalid("Object for reconciliation not found").ReconcileResult() - } - return reconcileResult, err - } - - if err := r.initializeState(rs); err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(xerrors.Errorf("Failed to initialize replica set state: %w", err)), log) - } +// Reconcile performs the full reconciliation logic for a replica set. +// This is the main entry point for all reconciliation work and contains all +// state and logic specific to a single reconcile execution. +func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.Result, error) { + rs := h.resource + log := h.log + r := h.reconciler if !architectures.IsRunningStaticArchitecture(rs.Annotations) { agents.UpgradeAllIfNeeded(ctx, agents.ClientSecret{Client: r.client, SecretClient: r.SecretClient}, r.omConnectionFactory, GetWatchedNamespace(), false) @@ -214,36 +196,36 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco log.Infow("ReplicaSet.Status", "status", rs.Status) if err := rs.ProcessValidationsOnReconcile(nil); err != nil { - return r.updateStatus(ctx, rs, workflow.Invalid("%s", err.Error()), log) + return h.updateStatus(ctx, workflow.Invalid("%s", err.Error())) } projectConfig, credsConfig, err := project.ReadConfigAndCredentials(ctx, r.client, r.SecretClient, rs, log) if err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(err), log) + return h.updateStatus(ctx, workflow.Failed(err)) } conn, _, err := connection.PrepareOpsManagerConnection(ctx, r.SecretClient, projectConfig, credsConfig, r.omConnectionFactory, rs.Namespace, log) if err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(xerrors.Errorf("Failed to prepare Ops Manager connection: %w", err)), log) + return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Ops Manager connection: %w", err))) } if status := ensureSupportedOpsManagerVersion(conn); status.Phase() != mdbstatus.PhaseRunning { - return r.updateStatus(ctx, rs, status, log) + return h.updateStatus(ctx, status) } r.SetupCommonWatchers(rs, nil, nil, rs.Name) reconcileResult := checkIfHasExcessProcesses(conn, rs.Name, log) if !reconcileResult.IsOK() { - return r.updateStatus(ctx, rs, reconcileResult, log) + return h.updateStatus(ctx, reconcileResult) } if status := validateMongoDBResource(rs, conn); !status.IsOK() { - return r.updateStatus(ctx, rs, status, log) + return h.updateStatus(ctx, status) } if status := controlledfeature.EnsureFeatureControls(*rs, conn, conn.OpsManagerVersion(), log); !status.IsOK() { - return r.updateStatus(ctx, rs, status, log) + return h.updateStatus(ctx, status) } // === 2. Auth and Certificates @@ -271,18 +253,18 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco prometheusCertHash, err := certs.EnsureTLSCertsForPrometheus(ctx, r.SecretClient, rs.GetNamespace(), rs.GetPrometheus(), certs.Database, log) if err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(xerrors.Errorf("could not generate certificates for Prometheus: %w", err)), log) + return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Could not generate certificates for Prometheus: %w", err))) } currentAgentAuthMode, err := conn.GetAgentAuthMode() if err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(xerrors.Errorf("failed to get agent auth mode: %w", err)), log) + return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to get agent auth mode: %w", err))) } // Check if we need to prepare for scale-down if scale.ReplicasThisReconciliation(rs) < rs.Status.Members { if err := replicaset.PrepareScaleDownFromMongoDB(conn, rs, log); err != nil { - return r.updateStatus(ctx, rs, workflow.Failed(xerrors.Errorf("Failed to prepare Replica Set for scaling down using Ops Manager: %w", err)), log) + return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Replica Set for scaling down using Ops Manager: %w", err))) } } deploymentOpts := deploymentOptionsRS{ @@ -304,7 +286,7 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco // See CLOUDP-189433 and CLOUDP-229222 for more details. if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) { log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition) - automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, log, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, h, log, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") reconcileStatus := r.reconcileMemberResources(ctx, rs, conn, log, projectConfig, deploymentOpts) if !reconcileStatus.IsOK() { log.Errorf("Recovery failed because of reconcile errors, %v", reconcileStatus) @@ -316,26 +298,86 @@ func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reco // 5. Actual reconciliation execution, Ops Manager and kubernetes resources update - publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, r.client, *rs, r.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) + publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, r.client, *rs, h.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { - return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, rs, log, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, h, log, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") }, func() workflow.Status { return r.reconcileMemberResources(ctx, rs, conn, log, projectConfig, deploymentOpts) }) if !status.IsOK() { - return r.updateStatus(ctx, rs, status, log) + return h.updateStatus(ctx, status) } // === 6. Final steps if scale.IsStillScaling(rs) { - return r.updateStatus(ctx, rs, workflow.Pending("Continuing scaling operation for ReplicaSet %s, desiredMembers=%d, currentMembers=%d", rs.ObjectKey(), rs.DesiredReplicas(), scale.ReplicasThisReconciliation(rs)), log, mdbstatus.MembersOption(rs)) + return h.updateStatus(ctx, workflow.Pending("Continuing scaling operation for ReplicaSet %s, desiredMembers=%d, currentMembers=%d", rs.ObjectKey(), rs.DesiredReplicas(), scale.ReplicasThisReconciliation(rs)), mdbstatus.MembersOption(rs)) } log.Infof("Finished reconciliation for MongoDbReplicaSet! %s", completionMessage(conn.BaseURL(), conn.GroupID())) - return r.updateStatus(ctx, rs, workflow.OK(), log, mdbstatus.NewBaseUrlOption(deployment.Link(conn.BaseURL(), conn.GroupID())), mdbstatus.MembersOption(rs), mdbstatus.NewPVCsStatusOptionEmptyStatus()) + return h.updateStatus(ctx, workflow.OK(), mdbstatus.NewBaseUrlOption(deployment.Link(conn.BaseURL(), conn.GroupID())), mdbstatus.MembersOption(rs), mdbstatus.NewPVCsStatusOptionEmptyStatus()) +} + +func newReplicaSetReconciler(ctx context.Context, kubeClient client.Client, imageUrls images.ImageUrls, initDatabaseNonStaticImageVersion, databaseNonStaticImageVersion string, forceEnterprise bool, enableClusterMongoDBRoles bool, omFunc om.ConnectionFactory) *ReconcileMongoDbReplicaSet { + return &ReconcileMongoDbReplicaSet{ + ReconcileCommonController: NewReconcileCommonController(ctx, kubeClient), + omConnectionFactory: omFunc, + imageUrls: imageUrls, + forceEnterprise: forceEnterprise, + enableClusterMongoDBRoles: enableClusterMongoDBRoles, + + initDatabaseNonStaticImageVersion: initDatabaseNonStaticImageVersion, + databaseNonStaticImageVersion: databaseNonStaticImageVersion, + } +} + +type deploymentOptionsRS struct { + agentCertPath string + agentCertHash string + prometheusCertHash string + currentAgentAuthMode string +} + +// Generic Kubernetes Resources +// +kubebuilder:rbac:groups=core,resources=namespaces,verbs=list;watch,namespace=placeholder +// +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch,namespace=placeholder +// +kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;update,namespace=placeholder +// +kubebuilder:rbac:groups=core,resources={secrets,configmaps},verbs=get;list;watch;create;delete;update,namespace=placeholder +// +kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=create;get;list;watch;delete;update,namespace=placeholder + +// MongoDB Resource +// +kubebuilder:rbac:groups=mongodb.com,resources={mongodb,mongodb/status,mongodb/finalizers},verbs=*,namespace=placeholder + +// Setting up a webhook +// +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=get;create;update;delete + +// Certificate generation +// +kubebuilder:rbac:groups=certificates.k8s.io,resources=certificatesigningrequests,verbs=get;create;list;watch + +// Reconcile reads that state of the cluster for a MongoDbReplicaSet object and makes changes based on the state read +// and what is in the MongoDbReplicaSet.Spec +func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reconcile.Request) (res reconcile.Result, e error) { + // === 1. Initial Checks and setup + log := zap.S().With("ReplicaSet", request.NamespacedName) + rs := &mdbv1.MongoDB{} + + if reconcileResult, err := r.prepareResourceForReconciliation(ctx, request, rs, log); err != nil { + if errors.IsNotFound(err) { + return workflow.Invalid("Object for reconciliation not found").ReconcileResult() + } + return reconcileResult, err + } + + // Create helper for THIS reconciliation + helper, err := r.newReconcilerHelper(ctx, rs, log) + if err != nil { + return r.updateStatus(ctx, rs, workflow.Failed(err), log) + } + + // Delegate all reconciliation logic to helper + return helper.Reconcile(ctx) } func publishAutomationConfigFirstRS(ctx context.Context, getter kubernetesClient.Client, mdb mdbv1.MongoDB, lastSpec *mdbv1.MongoDbSpec, currentAgentAuthMode string, sslMMSCAConfigMap string, log *zap.SugaredLogger) bool { @@ -618,7 +660,8 @@ func AddReplicaSetController(ctx context.Context, mgr manager.Manager, imageUrls // updateOmDeploymentRs performs OM registration operation for the replicaset. So the changes will be finally propagated // to automation agents in containers -func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, rs *mdbv1.MongoDB, log *zap.SugaredLogger, tlsCertPath, internalClusterCertPath string, deploymentOptionsRS deploymentOptionsRS, shouldMirrorKeyfileForMongot bool, isRecovering bool) workflow.Status { +func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, helper *ReplicaSetReconcilerHelper, log *zap.SugaredLogger, tlsCertPath, internalClusterCertPath string, deploymentOptionsRS deploymentOptionsRS, shouldMirrorKeyfileForMongot bool, isRecovering bool) workflow.Status { + rs := helper.resource log.Debug("Entering UpdateOMDeployments") // Only "concrete" RS members should be observed // - if scaling down, let's observe only members that will remain after scale-down operation @@ -632,7 +675,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c caFilePath := fmt.Sprintf("%s/ca-pem", util.TLSCaMountPath) // If current operation is to Disable TLS, then we should the current members of the Replica Set, // this is, do not scale them up or down util TLS disabling has completed. - shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, r.deploymentState.LastAchievedSpec, log) + shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, helper.deploymentState.LastAchievedSpec, log) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -663,7 +706,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c return status } - lastRsConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(r.deploymentState.LastAchievedSpec, mdbv1.ReplicaSetConfig) + lastRsConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(helper.deploymentState.LastAchievedSpec, mdbv1.ReplicaSetConfig) if err != nil && !isRecovering { return workflow.Failed(err) } From a3fae90b298db17ff0b088990092cab84c9ecd64 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Mon, 20 Oct 2025 13:41:23 +0200 Subject: [PATCH 04/25] Moved all methods to helper --- .../operator/mongodbreplicaset_controller.go | 82 ++++++++++++------- 1 file changed, 51 insertions(+), 31 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 929af5fcb..ea415e7ff 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -277,7 +277,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // 3. Search Overrides // Apply search overrides early so searchCoordinator role is present before ensureRoles runs // This must happen before the ordering logic to ensure roles are synced regardless of order - shouldMirrorKeyfileForMongot := r.applySearchOverrides(ctx, rs, log) + shouldMirrorKeyfile := h.applySearchOverrides(ctx) // 4. Recovery @@ -286,8 +286,8 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // See CLOUDP-189433 and CLOUDP-229222 for more details. if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) { log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition) - automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, h, log, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") - reconcileStatus := r.reconcileMemberResources(ctx, rs, conn, log, projectConfig, deploymentOpts) + automationConfigStatus := h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfile, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + reconcileStatus := h.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) if !reconcileStatus.IsOK() { log.Errorf("Recovery failed because of reconcile errors, %v", reconcileStatus) } @@ -301,10 +301,10 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, r.client, *rs, h.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { - return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, h, log, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + return h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfile, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") }, func() workflow.Status { - return r.reconcileMemberResources(ctx, rs, conn, log, projectConfig, deploymentOpts) + return h.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) }) if !status.IsOK() { @@ -469,9 +469,11 @@ func (r *ReconcileMongoDbReplicaSet) reconcileHostnameOverrideConfigMap(ctx cont // reconcileMemberResources handles the synchronization of kubernetes resources, which can be statefulsets, services etc. // All the resources required in the k8s cluster (as opposed to the automation config) for creating the replicaset // should be reconciled in this method. -func (r *ReconcileMongoDbReplicaSet) reconcileMemberResources(ctx context.Context, rs *mdbv1.MongoDB, conn om.Connection, - log *zap.SugaredLogger, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS, -) workflow.Status { +func (h *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { + rs := h.resource + r := h.reconciler + log := h.log + // Reconcile hostname override ConfigMap if err := r.reconcileHostnameOverrideConfigMap(ctx, log, r.client, *rs); err != nil { return workflow.Failed(xerrors.Errorf("Failed to reconcileHostnameOverrideConfigMap: %w", err)) @@ -482,12 +484,14 @@ func (r *ReconcileMongoDbReplicaSet) reconcileMemberResources(ctx context.Contex return status } - return r.reconcileStatefulSet(ctx, rs, log, conn, projectConfig, deploymentOptions) + return h.reconcileStatefulSet(ctx, conn, projectConfig, deploymentOptions) } -func (r *ReconcileMongoDbReplicaSet) reconcileStatefulSet(ctx context.Context, rs *mdbv1.MongoDB, - log *zap.SugaredLogger, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS, -) workflow.Status { +func (h *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { + rs := h.resource + r := h.reconciler + log := h.log + certConfigurator := certs.ReplicaSetX509CertConfigurator{MongoDB: rs, SecretClient: r.SecretClient} status := r.ensureX509SecretAndCheckTLSType(ctx, certConfigurator, deploymentOptions.currentAgentAuthMode, log) if !status.IsOK() { @@ -500,7 +504,7 @@ func (r *ReconcileMongoDbReplicaSet) reconcileStatefulSet(ctx context.Context, r } // Build the replica set config - rsConfig, err := r.buildStatefulSetOptions(ctx, rs, conn, projectConfig, deploymentOptions.currentAgentAuthMode, deploymentOptions.prometheusCertHash, deploymentOptions.agentCertHash, log) + rsConfig, err := h.buildStatefulSetOptions(ctx, conn, projectConfig, deploymentOptions) if err != nil { return workflow.Failed(xerrors.Errorf("failed to build StatefulSet options: %w", err)) } @@ -533,7 +537,11 @@ func (r *ReconcileMongoDbReplicaSet) reconcileStatefulSet(ctx context.Context, r } // buildStatefulSetOptions creates the options needed for constructing the StatefulSet -func (r *ReconcileMongoDbReplicaSet) buildStatefulSetOptions(ctx context.Context, rs *mdbv1.MongoDB, conn om.Connection, projectConfig mdbv1.ProjectConfig, currentAgentAuthMode string, prometheusCertHash string, agentCertHash string, log *zap.SugaredLogger) (func(mdb mdbv1.MongoDB) construct.DatabaseStatefulSetOptions, error) { +func (h *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) (func(mdb mdbv1.MongoDB) construct.DatabaseStatefulSetOptions, error) { + rs := h.resource + r := h.reconciler + log := h.log + rsCertsConfig := certs.ReplicaSetConfig(*rs) var vaultConfig vault.VaultConfiguration @@ -562,11 +570,11 @@ func (r *ReconcileMongoDbReplicaSet) buildStatefulSetOptions(ctx context.Context rsConfig := construct.ReplicaSetOptions( PodEnvVars(newPodVars(conn, projectConfig, rs.Spec.LogLevel)), - CurrentAgentAuthMechanism(currentAgentAuthMode), + CurrentAgentAuthMechanism(deploymentOptions.currentAgentAuthMode), CertificateHash(tlsCertHash), - AgentCertHash(agentCertHash), + AgentCertHash(deploymentOptions.agentCertHash), InternalClusterHash(internalClusterCertHash), - PrometheusTLSCertHash(prometheusCertHash), + PrometheusTLSCertHash(deploymentOptions.prometheusCertHash), WithVaultConfig(vaultConfig), WithLabels(rs.Labels), WithAdditionalMongodConfig(rs.Spec.GetAdditionalMongodConfig()), @@ -660,8 +668,10 @@ func AddReplicaSetController(ctx context.Context, mgr manager.Manager, imageUrls // updateOmDeploymentRs performs OM registration operation for the replicaset. So the changes will be finally propagated // to automation agents in containers -func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, helper *ReplicaSetReconcilerHelper, log *zap.SugaredLogger, tlsCertPath, internalClusterCertPath string, deploymentOptionsRS deploymentOptionsRS, shouldMirrorKeyfileForMongot bool, isRecovering bool) workflow.Status { - rs := helper.resource +func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, tlsCertPath, internalClusterCertPath string, deploymentOpts deploymentOptionsRS, shouldMirrorKeyfile bool, isRecovering bool) workflow.Status { + rs := h.resource + log := h.log + r := h.reconciler log.Debug("Entering UpdateOMDeployments") // Only "concrete" RS members should be observed // - if scaling down, let's observe only members that will remain after scale-down operation @@ -675,7 +685,7 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c caFilePath := fmt.Sprintf("%s/ca-pem", util.TLSCaMountPath) // If current operation is to Disable TLS, then we should the current members of the Replica Set, // this is, do not scale them up or down util TLS disabling has completed. - shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, helper.deploymentState.LastAchievedSpec, log) + shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, h.deploymentState.LastAchievedSpec, log) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -701,12 +711,12 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c replicaSet := replicaset.BuildFromMongoDBWithReplicas(r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, rs, updatedMembers, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath) processNames := replicaSet.GetProcessNames() - status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, deploymentOptionsRS.agentCertPath, caFilePath, internalClusterCertPath, isRecovering, log) + status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, deploymentOpts.agentCertPath, caFilePath, internalClusterCertPath, isRecovering, log) if !status.IsOK() && !isRecovering { return status } - lastRsConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(helper.deploymentState.LastAchievedSpec, mdbv1.ReplicaSetConfig) + lastRsConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(h.deploymentState.LastAchievedSpec, mdbv1.ReplicaSetConfig) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -716,13 +726,13 @@ func (r *ReconcileMongoDbReplicaSet) updateOmDeploymentRs(ctx context.Context, c conn: conn, secretsClient: r.SecretClient, namespace: rs.GetNamespace(), - prometheusCertHash: deploymentOptionsRS.prometheusCertHash, + prometheusCertHash: deploymentOpts.prometheusCertHash, } err = conn.ReadUpdateDeployment( func(d om.Deployment) error { - if shouldMirrorKeyfileForMongot { - if err := r.mirrorKeyfileIntoSecretForMongot(ctx, d, rs, log); err != nil { + if shouldMirrorKeyfile { + if err := h.mirrorKeyfileIntoSecretForMongot(ctx, d); err != nil { return err } } @@ -867,8 +877,11 @@ func getAllHostsForReplicas(rs *mdbv1.MongoDB, membersCount int) []string { return hostnames } -func (r *ReconcileMongoDbReplicaSet) applySearchOverrides(ctx context.Context, rs *mdbv1.MongoDB, log *zap.SugaredLogger) bool { - search := r.lookupCorrespondingSearchResource(ctx, rs, log) +func (h *ReplicaSetReconcilerHelper) applySearchOverrides(ctx context.Context) bool { + rs := h.resource + log := h.log + + search := h.lookupCorrespondingSearchResource(ctx) if search == nil { log.Debugf("No MongoDBSearch resource found, skipping search overrides") return false @@ -894,7 +907,11 @@ func (r *ReconcileMongoDbReplicaSet) applySearchOverrides(ctx context.Context, r return true } -func (r *ReconcileMongoDbReplicaSet) mirrorKeyfileIntoSecretForMongot(ctx context.Context, d om.Deployment, rs *mdbv1.MongoDB, log *zap.SugaredLogger) error { +func (h *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx context.Context, d om.Deployment) error { + rs := h.resource + r := h.reconciler + log := h.log + keyfileContents := maputil.ReadMapValueAsString(d, "auth", "key") keyfileSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("%s-%s", rs.Name, searchcontroller.MongotKeyfileFilename), Namespace: rs.Namespace}} @@ -906,12 +923,15 @@ func (r *ReconcileMongoDbReplicaSet) mirrorKeyfileIntoSecretForMongot(ctx contex }) if err != nil { return xerrors.Errorf("Failed to mirror the replicaset's keyfile into a secret: %w", err) - } else { - return nil } + return nil } -func (r *ReconcileMongoDbReplicaSet) lookupCorrespondingSearchResource(ctx context.Context, rs *mdbv1.MongoDB, log *zap.SugaredLogger) *searchv1.MongoDBSearch { +func (h *ReplicaSetReconcilerHelper) lookupCorrespondingSearchResource(ctx context.Context) *searchv1.MongoDBSearch { + rs := h.resource + r := h.reconciler + log := h.log + var search *searchv1.MongoDBSearch searchList := &searchv1.MongoDBSearchList{} if err := r.client.List(ctx, searchList, &client.ListOptions{ From e714e0474f646f945f633a773a615a44e9f20f9d Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 10:14:00 +0200 Subject: [PATCH 05/25] Use helper pattern for reconcileHostnameOverrideConfigMap --- controllers/operator/mongodbreplicaset_controller.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index ea415e7ff..378161e5c 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -195,6 +195,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R log.Infow("ReplicaSet.Spec", "spec", rs.Spec, "desiredReplicas", scale.ReplicasThisReconciliation(rs), "isScaling", scale.IsStillScaling(rs)) log.Infow("ReplicaSet.Status", "status", rs.Status) + // TODO: adapt validations to multi cluster if err := rs.ProcessValidationsOnReconcile(nil); err != nil { return h.updateStatus(ctx, workflow.Invalid("%s", err.Error())) } @@ -431,7 +432,7 @@ func publishAutomationConfigFirstRS(ctx context.Context, getter kubernetesClient return false } -func getHostnameOverrideConfigMapForReplicaset(mdb mdbv1.MongoDB) corev1.ConfigMap { +func getHostnameOverrideConfigMapForReplicaset(mdb *mdbv1.MongoDB) corev1.ConfigMap { data := make(map[string]string) if mdb.Spec.DbCommonSpec.GetExternalDomain() != nil { @@ -451,12 +452,12 @@ func getHostnameOverrideConfigMapForReplicaset(mdb mdbv1.MongoDB) corev1.ConfigM return cm } -func (r *ReconcileMongoDbReplicaSet) reconcileHostnameOverrideConfigMap(ctx context.Context, log *zap.SugaredLogger, getUpdateCreator configmap.GetUpdateCreator, mdb mdbv1.MongoDB) error { - if mdb.Spec.DbCommonSpec.GetExternalDomain() == nil { +func (h *ReplicaSetReconcilerHelper) reconcileHostnameOverrideConfigMap(ctx context.Context, log *zap.SugaredLogger, getUpdateCreator configmap.GetUpdateCreator) error { + if h.resource.Spec.DbCommonSpec.GetExternalDomain() == nil { return nil } - cm := getHostnameOverrideConfigMapForReplicaset(mdb) + cm := getHostnameOverrideConfigMapForReplicaset(h.resource) err := configmap.CreateOrUpdate(ctx, getUpdateCreator, cm) if err != nil && !errors.IsAlreadyExists(err) { return xerrors.Errorf("failed to create configmap: %s, err: %w", cm.Name, err) @@ -475,7 +476,7 @@ func (h *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Contex log := h.log // Reconcile hostname override ConfigMap - if err := r.reconcileHostnameOverrideConfigMap(ctx, log, r.client, *rs); err != nil { + if err := h.reconcileHostnameOverrideConfigMap(ctx, log, h.reconciler.client); err != nil { return workflow.Failed(xerrors.Errorf("Failed to reconcileHostnameOverrideConfigMap: %w", err)) } @@ -808,6 +809,7 @@ func updateOmDeploymentDisableTLSConfiguration(conn om.Connection, mongoDBImage return tlsConfigWasDisabled, err } +// TODO: split into subfunctions, follow helper pattern func (r *ReconcileMongoDbReplicaSet) OnDelete(ctx context.Context, obj runtime.Object, log *zap.SugaredLogger) error { rs := obj.(*mdbv1.MongoDB) From 430cb51cb95c0dc251c9e0cba292e476e01a64b1 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 13:25:19 +0200 Subject: [PATCH 06/25] Lint --- controllers/operator/mongodbreplicaset_controller.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 378161e5c..30142bcff 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -166,14 +166,14 @@ func (h *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { // is written after every status update. This ensures state consistency even on early returns. // It must be executed only once per reconcile (with a return) func (h *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status workflow.Status, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { - result, err := h.reconciler.ReconcileCommonController.updateStatus(ctx, h.resource, status, h.log, statusOptions...) + result, err := h.reconciler.updateStatus(ctx, h.resource, status, h.log, statusOptions...) if err != nil { return result, err } // Write deployment state after every status update if err := h.writeState(ctx); err != nil { - return h.reconciler.ReconcileCommonController.updateStatus(ctx, h.resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), h.log) + return h.reconciler.updateStatus(ctx, h.resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), h.log) } return result, nil @@ -766,7 +766,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c return workflow.Failed(err) } - //TODO: check if updateStatus usage is correct hee + // TODO: check if updateStatus usage is correct hee if status := r.ensureBackupConfigurationAndUpdateStatus(ctx, conn, rs, r.SecretClient, log); !status.IsOK() && !isRecovering { return status } From 894ef002acedbab691177c484e8491a923fc8cb9 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 13:48:27 +0200 Subject: [PATCH 07/25] Rename variables for consistency --- .../operator/mongodbreplicaset_controller.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 30142bcff..bb63d8134 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -278,7 +278,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // 3. Search Overrides // Apply search overrides early so searchCoordinator role is present before ensureRoles runs // This must happen before the ordering logic to ensure roles are synced regardless of order - shouldMirrorKeyfile := h.applySearchOverrides(ctx) + shouldMirrorKeyfileForMongot := h.applySearchOverrides(ctx) // 4. Recovery @@ -287,7 +287,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // See CLOUDP-189433 and CLOUDP-229222 for more details. if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) { log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition) - automationConfigStatus := h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfile, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + automationConfigStatus := h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") reconcileStatus := h.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) if !reconcileStatus.IsOK() { log.Errorf("Recovery failed because of reconcile errors, %v", reconcileStatus) @@ -302,7 +302,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, r.client, *rs, h.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { - return h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfile, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + return h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") }, func() workflow.Status { return h.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) @@ -669,7 +669,7 @@ func AddReplicaSetController(ctx context.Context, mgr manager.Manager, imageUrls // updateOmDeploymentRs performs OM registration operation for the replicaset. So the changes will be finally propagated // to automation agents in containers -func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, tlsCertPath, internalClusterCertPath string, deploymentOpts deploymentOptionsRS, shouldMirrorKeyfile bool, isRecovering bool) workflow.Status { +func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, tlsCertPath, internalClusterCertPath string, deploymentOptions deploymentOptionsRS, shouldMirrorKeyfileForMongot bool, isRecovering bool) workflow.Status { rs := h.resource log := h.log r := h.reconciler @@ -712,7 +712,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c replicaSet := replicaset.BuildFromMongoDBWithReplicas(r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, rs, updatedMembers, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath) processNames := replicaSet.GetProcessNames() - status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, deploymentOpts.agentCertPath, caFilePath, internalClusterCertPath, isRecovering, log) + status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, deploymentOptions.agentCertPath, caFilePath, internalClusterCertPath, isRecovering, log) if !status.IsOK() && !isRecovering { return status } @@ -727,12 +727,12 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c conn: conn, secretsClient: r.SecretClient, namespace: rs.GetNamespace(), - prometheusCertHash: deploymentOpts.prometheusCertHash, + prometheusCertHash: deploymentOptions.prometheusCertHash, } err = conn.ReadUpdateDeployment( func(d om.Deployment) error { - if shouldMirrorKeyfile { + if shouldMirrorKeyfileForMongot { if err := h.mirrorKeyfileIntoSecretForMongot(ctx, d); err != nil { return err } From 4bb174c5483c52762b5f354236b167c1faaddd95 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 13:49:50 +0200 Subject: [PATCH 08/25] typo --- controllers/operator/mongodbreplicaset_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index bb63d8134..73e3c70bb 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -766,7 +766,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c return workflow.Failed(err) } - // TODO: check if updateStatus usage is correct hee + // TODO: check if updateStatus usage is correct here if status := r.ensureBackupConfigurationAndUpdateStatus(ctx, conn, rs, r.SecretClient, log); !status.IsOK() && !isRecovering { return status } From 0ae98c8f7273f5abb73922fbf7e932f9e282d083 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 14:02:37 +0200 Subject: [PATCH 09/25] Rename r := h.reconciler to reconciler --- .../operator/mongodbreplicaset_controller.go | 88 +++++++++---------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 73e3c70bb..d628de60e 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -185,10 +185,10 @@ func (h *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status wo func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.Result, error) { rs := h.resource log := h.log - r := h.reconciler + reconciler := h.reconciler if !architectures.IsRunningStaticArchitecture(rs.Annotations) { - agents.UpgradeAllIfNeeded(ctx, agents.ClientSecret{Client: r.client, SecretClient: r.SecretClient}, r.omConnectionFactory, GetWatchedNamespace(), false) + agents.UpgradeAllIfNeeded(ctx, agents.ClientSecret{Client: reconciler.client, SecretClient: reconciler.SecretClient}, reconciler.omConnectionFactory, GetWatchedNamespace(), false) } log.Info("-> ReplicaSet.Reconcile") @@ -200,12 +200,12 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R return h.updateStatus(ctx, workflow.Invalid("%s", err.Error())) } - projectConfig, credsConfig, err := project.ReadConfigAndCredentials(ctx, r.client, r.SecretClient, rs, log) + projectConfig, credsConfig, err := project.ReadConfigAndCredentials(ctx, reconciler.client, reconciler.SecretClient, rs, log) if err != nil { return h.updateStatus(ctx, workflow.Failed(err)) } - conn, _, err := connection.PrepareOpsManagerConnection(ctx, r.SecretClient, projectConfig, credsConfig, r.omConnectionFactory, rs.Namespace, log) + conn, _, err := connection.PrepareOpsManagerConnection(ctx, reconciler.SecretClient, projectConfig, credsConfig, reconciler.omConnectionFactory, rs.Namespace, log) if err != nil { return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Ops Manager connection: %w", err))) } @@ -214,7 +214,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R return h.updateStatus(ctx, status) } - r.SetupCommonWatchers(rs, nil, nil, rs.Name) + reconciler.SetupCommonWatchers(rs, nil, nil, rs.Name) reconcileResult := checkIfHasExcessProcesses(conn, rs.Name, log) if !reconcileResult.IsOK() { @@ -234,11 +234,11 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // Get certificate paths for later use rsCertsConfig := certs.ReplicaSetConfig(*rs) var databaseSecretPath string - if r.VaultClient != nil { - databaseSecretPath = r.VaultClient.DatabaseSecretPath() + if reconciler.VaultClient != nil { + databaseSecretPath = reconciler.VaultClient.DatabaseSecretPath() } - tlsCertHash := enterprisepem.ReadHashFromSecret(ctx, r.SecretClient, rs.Namespace, rsCertsConfig.CertSecretName, databaseSecretPath, log) - internalClusterCertHash := enterprisepem.ReadHashFromSecret(ctx, r.SecretClient, rs.Namespace, rsCertsConfig.InternalClusterSecretName, databaseSecretPath, log) + tlsCertHash := enterprisepem.ReadHashFromSecret(ctx, reconciler.SecretClient, rs.Namespace, rsCertsConfig.CertSecretName, databaseSecretPath, log) + internalClusterCertHash := enterprisepem.ReadHashFromSecret(ctx, reconciler.SecretClient, rs.Namespace, rsCertsConfig.InternalClusterSecretName, databaseSecretPath, log) tlsCertPath := "" internalClusterCertPath := "" @@ -250,9 +250,9 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R } agentCertSecretName := rs.GetSecurity().AgentClientCertificateSecretName(rs.Name) - agentCertHash, agentCertPath := r.agentCertHashAndPath(ctx, log, rs.Namespace, agentCertSecretName, databaseSecretPath) + agentCertHash, agentCertPath := reconciler.agentCertHashAndPath(ctx, log, rs.Namespace, agentCertSecretName, databaseSecretPath) - prometheusCertHash, err := certs.EnsureTLSCertsForPrometheus(ctx, r.SecretClient, rs.GetNamespace(), rs.GetPrometheus(), certs.Database, log) + prometheusCertHash, err := certs.EnsureTLSCertsForPrometheus(ctx, reconciler.SecretClient, rs.GetNamespace(), rs.GetPrometheus(), certs.Database, log) if err != nil { return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Could not generate certificates for Prometheus: %w", err))) } @@ -299,7 +299,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // 5. Actual reconciliation execution, Ops Manager and kubernetes resources update - publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, r.client, *rs, h.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) + publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, reconciler.client, *rs, h.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { return h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") @@ -472,7 +472,7 @@ func (h *ReplicaSetReconcilerHelper) reconcileHostnameOverrideConfigMap(ctx cont // should be reconciled in this method. func (h *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { rs := h.resource - r := h.reconciler + reconciler := h.reconciler log := h.log // Reconcile hostname override ConfigMap @@ -481,7 +481,7 @@ func (h *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Contex } // Ensure roles are properly configured - if status := r.ensureRoles(ctx, rs.Spec.DbCommonSpec, r.enableClusterMongoDBRoles, conn, kube.ObjectKeyFromApiObject(rs), log); !status.IsOK() { + if status := reconciler.ensureRoles(ctx, rs.Spec.DbCommonSpec, reconciler.enableClusterMongoDBRoles, conn, kube.ObjectKeyFromApiObject(rs), log); !status.IsOK() { return status } @@ -490,16 +490,16 @@ func (h *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Contex func (h *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { rs := h.resource - r := h.reconciler + reconciler := h.reconciler log := h.log - certConfigurator := certs.ReplicaSetX509CertConfigurator{MongoDB: rs, SecretClient: r.SecretClient} - status := r.ensureX509SecretAndCheckTLSType(ctx, certConfigurator, deploymentOptions.currentAgentAuthMode, log) + certConfigurator := certs.ReplicaSetX509CertConfigurator{MongoDB: rs, SecretClient: reconciler.SecretClient} + status := reconciler.ensureX509SecretAndCheckTLSType(ctx, certConfigurator, deploymentOptions.currentAgentAuthMode, log) if !status.IsOK() { return status } - status = certs.EnsureSSLCertsForStatefulSet(ctx, r.SecretClient, r.SecretClient, *rs.Spec.Security, certs.ReplicaSetConfig(*rs), log) + status = certs.EnsureSSLCertsForStatefulSet(ctx, reconciler.SecretClient, reconciler.SecretClient, *rs.Spec.Security, certs.ReplicaSetConfig(*rs), log) if !status.IsOK() { return status } @@ -513,23 +513,23 @@ func (h *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c sts := construct.DatabaseStatefulSet(*rs, rsConfig, log) // Handle PVC resize if needed - workflowStatus := create.HandlePVCResize(ctx, r.client, &sts, log) + workflowStatus := create.HandlePVCResize(ctx, reconciler.client, &sts, log) if !workflowStatus.IsOK() { return workflowStatus } // TODO: check if updatestatus usage is correct here if workflow.ContainsPVCOption(workflowStatus.StatusOptions()) { - _, _ = r.updateStatus(ctx, rs, workflow.Pending(""), log, workflowStatus.StatusOptions()...) + _, _ = reconciler.updateStatus(ctx, rs, workflow.Pending(""), log, workflowStatus.StatusOptions()...) } // Create or update the StatefulSet in Kubernetes - if err := create.DatabaseInKubernetes(ctx, r.client, *rs, sts, rsConfig, log); err != nil { + if err := create.DatabaseInKubernetes(ctx, reconciler.client, *rs, sts, rsConfig, log); err != nil { return workflow.Failed(xerrors.Errorf("Failed to create/update (Kubernetes reconciliation phase): %w", err)) } // Check StatefulSet status - if status := statefulset.GetStatefulSetStatus(ctx, rs.Namespace, rs.Name, r.client); !status.IsOK() { + if status := statefulset.GetStatefulSetStatus(ctx, rs.Namespace, rs.Name, reconciler.client); !status.IsOK() { return status } @@ -540,16 +540,16 @@ func (h *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c // buildStatefulSetOptions creates the options needed for constructing the StatefulSet func (h *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) (func(mdb mdbv1.MongoDB) construct.DatabaseStatefulSetOptions, error) { rs := h.resource - r := h.reconciler + reconciler := h.reconciler log := h.log rsCertsConfig := certs.ReplicaSetConfig(*rs) var vaultConfig vault.VaultConfiguration var databaseSecretPath string - if r.VaultClient != nil { - vaultConfig = r.VaultClient.VaultConfig - databaseSecretPath = r.VaultClient.DatabaseSecretPath() + if reconciler.VaultClient != nil { + vaultConfig = reconciler.VaultClient.VaultConfig + databaseSecretPath = reconciler.VaultClient.DatabaseSecretPath() } // Determine automation agent version for static architecture @@ -559,15 +559,15 @@ func (h *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context // happens after creating the StatefulSet definition. if !rs.IsAgentImageOverridden() { var err error - automationAgentVersion, err = r.getAgentVersion(conn, conn.OpsManagerVersion().VersionString, false, log) + automationAgentVersion, err = reconciler.getAgentVersion(conn, conn.OpsManagerVersion().VersionString, false, log) if err != nil { return nil, xerrors.Errorf("Impossible to get agent version, please override the agent image by providing a pod template: %w", err) } } } - tlsCertHash := enterprisepem.ReadHashFromSecret(ctx, r.SecretClient, rs.Namespace, rsCertsConfig.CertSecretName, databaseSecretPath, log) - internalClusterCertHash := enterprisepem.ReadHashFromSecret(ctx, r.SecretClient, rs.Namespace, rsCertsConfig.InternalClusterSecretName, databaseSecretPath, log) + tlsCertHash := enterprisepem.ReadHashFromSecret(ctx, reconciler.SecretClient, rs.Namespace, rsCertsConfig.CertSecretName, databaseSecretPath, log) + internalClusterCertHash := enterprisepem.ReadHashFromSecret(ctx, reconciler.SecretClient, rs.Namespace, rsCertsConfig.InternalClusterSecretName, databaseSecretPath, log) rsConfig := construct.ReplicaSetOptions( PodEnvVars(newPodVars(conn, projectConfig, rs.Spec.LogLevel)), @@ -579,10 +579,10 @@ func (h *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context WithVaultConfig(vaultConfig), WithLabels(rs.Labels), WithAdditionalMongodConfig(rs.Spec.GetAdditionalMongodConfig()), - WithInitDatabaseNonStaticImage(images.ContainerImage(r.imageUrls, util.InitDatabaseImageUrlEnv, r.initDatabaseNonStaticImageVersion)), - WithDatabaseNonStaticImage(images.ContainerImage(r.imageUrls, util.NonStaticDatabaseEnterpriseImage, r.databaseNonStaticImageVersion)), - WithAgentImage(images.ContainerImage(r.imageUrls, architectures.MdbAgentImageRepo, automationAgentVersion)), - WithMongodbImage(images.GetOfficialImage(r.imageUrls, rs.Spec.Version, rs.GetAnnotations())), + WithInitDatabaseNonStaticImage(images.ContainerImage(reconciler.imageUrls, util.InitDatabaseImageUrlEnv, reconciler.initDatabaseNonStaticImageVersion)), + WithDatabaseNonStaticImage(images.ContainerImage(reconciler.imageUrls, util.NonStaticDatabaseEnterpriseImage, reconciler.databaseNonStaticImageVersion)), + WithAgentImage(images.ContainerImage(reconciler.imageUrls, architectures.MdbAgentImageRepo, automationAgentVersion)), + WithMongodbImage(images.GetOfficialImage(reconciler.imageUrls, rs.Spec.Version, rs.GetAnnotations())), ) return rsConfig, nil @@ -672,7 +672,7 @@ func AddReplicaSetController(ctx context.Context, mgr manager.Manager, imageUrls func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, tlsCertPath, internalClusterCertPath string, deploymentOptions deploymentOptionsRS, shouldMirrorKeyfileForMongot bool, isRecovering bool) workflow.Status { rs := h.resource log := h.log - r := h.reconciler + reconciler := h.reconciler log.Debug("Entering UpdateOMDeployments") // Only "concrete" RS members should be observed // - if scaling down, let's observe only members that will remain after scale-down operation @@ -686,7 +686,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c caFilePath := fmt.Sprintf("%s/ca-pem", util.TLSCaMountPath) // If current operation is to Disable TLS, then we should the current members of the Replica Set, // this is, do not scale them up or down util TLS disabling has completed. - shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, h.deploymentState.LastAchievedSpec, log) + shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, reconciler.imageUrls[mcoConstruct.MongodbImageEnv], reconciler.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, h.deploymentState.LastAchievedSpec, log) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -709,10 +709,10 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c updatedMembers = replicasTarget } - replicaSet := replicaset.BuildFromMongoDBWithReplicas(r.imageUrls[mcoConstruct.MongodbImageEnv], r.forceEnterprise, rs, updatedMembers, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath) + replicaSet := replicaset.BuildFromMongoDBWithReplicas(reconciler.imageUrls[mcoConstruct.MongodbImageEnv], reconciler.forceEnterprise, rs, updatedMembers, rs.CalculateFeatureCompatibilityVersion(), tlsCertPath) processNames := replicaSet.GetProcessNames() - status, additionalReconciliationRequired := r.updateOmAuthentication(ctx, conn, processNames, rs, deploymentOptions.agentCertPath, caFilePath, internalClusterCertPath, isRecovering, log) + status, additionalReconciliationRequired := reconciler.updateOmAuthentication(ctx, conn, processNames, rs, deploymentOptions.agentCertPath, caFilePath, internalClusterCertPath, isRecovering, log) if !status.IsOK() && !isRecovering { return status } @@ -725,7 +725,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c prometheusConfiguration := PrometheusConfiguration{ prometheus: rs.GetPrometheus(), conn: conn, - secretsClient: r.SecretClient, + secretsClient: reconciler.SecretClient, namespace: rs.GetNamespace(), prometheusCertHash: deploymentOptions.prometheusCertHash, } @@ -767,7 +767,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c } // TODO: check if updateStatus usage is correct here - if status := r.ensureBackupConfigurationAndUpdateStatus(ctx, conn, rs, r.SecretClient, log); !status.IsOK() && !isRecovering { + if status := reconciler.ensureBackupConfigurationAndUpdateStatus(ctx, conn, rs, reconciler.SecretClient, log); !status.IsOK() && !isRecovering { return status } @@ -911,7 +911,7 @@ func (h *ReplicaSetReconcilerHelper) applySearchOverrides(ctx context.Context) b func (h *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx context.Context, d om.Deployment) error { rs := h.resource - r := h.reconciler + reconciler := h.reconciler log := h.log keyfileContents := maputil.ReadMapValueAsString(d, "auth", "key") @@ -919,9 +919,9 @@ func (h *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx contex log.Infof("Mirroring the replicaset %s's keyfile into the secret %s", rs.ObjectKey(), kube.ObjectKeyFromApiObject(keyfileSecret)) - _, err := controllerutil.CreateOrUpdate(ctx, r.client, keyfileSecret, func() error { + _, err := controllerutil.CreateOrUpdate(ctx, reconciler.client, keyfileSecret, func() error { keyfileSecret.StringData = map[string]string{searchcontroller.MongotKeyfileFilename: keyfileContents} - return controllerutil.SetOwnerReference(rs, keyfileSecret, r.client.Scheme()) + return controllerutil.SetOwnerReference(rs, keyfileSecret, reconciler.client.Scheme()) }) if err != nil { return xerrors.Errorf("Failed to mirror the replicaset's keyfile into a secret: %w", err) @@ -931,12 +931,12 @@ func (h *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx contex func (h *ReplicaSetReconcilerHelper) lookupCorrespondingSearchResource(ctx context.Context) *searchv1.MongoDBSearch { rs := h.resource - r := h.reconciler + reconciler := h.reconciler log := h.log var search *searchv1.MongoDBSearch searchList := &searchv1.MongoDBSearchList{} - if err := r.client.List(ctx, searchList, &client.ListOptions{ + if err := reconciler.client.List(ctx, searchList, &client.ListOptions{ FieldSelector: fields.OneTermEqualSelector(searchcontroller.MongoDBSearchIndexFieldName, rs.GetNamespace()+"/"+rs.GetName()), }); err != nil { log.Debugf("Failed to list MongoDBSearch resources: %v", err) From 95f0686ba6dfd20c6e63bfebe7a1a85a531678ad Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 14:03:09 +0200 Subject: [PATCH 10/25] Rename helper receivers to r --- .../operator/mongodbreplicaset_controller.go | 156 +++++++++--------- 1 file changed, 78 insertions(+), 78 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index d628de60e..297413d2e 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -112,9 +112,9 @@ func (r *ReconcileMongoDbReplicaSet) newReconcilerHelper( } // readState abstract reading the state of the resource that we store on the cluster between reconciliations. -func (h *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, error) { +func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, error) { // Try to get the last achieved spec from annotations and store it in state - if lastAchievedSpec, err := h.resource.GetLastSpec(); err != nil { + if lastAchievedSpec, err := r.resource.GetLastSpec(); err != nil { return nil, err } else { return &ReplicaSetDeploymentState{LastAchievedSpec: lastAchievedSpec}, nil @@ -122,58 +122,58 @@ func (h *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, er } // writeState abstract writing the state of the resource that we store on the cluster between reconciliations. -func (h *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { +func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { // Serialize the state to annotations - annotationsToAdd, err := getAnnotationsForResource(h.resource) + annotationsToAdd, err := getAnnotationsForResource(r.resource) if err != nil { return err } // Add vault annotations if needed if vault.IsVaultSecretBackend() { - secrets := h.resource.GetSecretsMountedIntoDBPod() + secrets := r.resource.GetSecretsMountedIntoDBPod() vaultMap := make(map[string]string) for _, s := range secrets { - path := fmt.Sprintf("%s/%s/%s", h.reconciler.VaultClient.DatabaseSecretMetadataPath(), h.resource.Namespace, s) - vaultMap = merge.StringToStringMap(vaultMap, h.reconciler.VaultClient.GetSecretAnnotation(path)) + path := fmt.Sprintf("%s/%s/%s", r.reconciler.VaultClient.DatabaseSecretMetadataPath(), r.resource.Namespace, s) + vaultMap = merge.StringToStringMap(vaultMap, r.reconciler.VaultClient.GetSecretAnnotation(path)) } - path := fmt.Sprintf("%s/%s/%s", h.reconciler.VaultClient.OperatorScretMetadataPath(), h.resource.Namespace, h.resource.Spec.Credentials) - vaultMap = merge.StringToStringMap(vaultMap, h.reconciler.VaultClient.GetSecretAnnotation(path)) + path := fmt.Sprintf("%s/%s/%s", r.reconciler.VaultClient.OperatorScretMetadataPath(), r.resource.Namespace, r.resource.Spec.Credentials) + vaultMap = merge.StringToStringMap(vaultMap, r.reconciler.VaultClient.GetSecretAnnotation(path)) for k, val := range vaultMap { annotationsToAdd[k] = val } } // Write annotations back to the resource - if err := annotations.SetAnnotations(ctx, h.resource, annotationsToAdd, h.reconciler.client); err != nil { + if err := annotations.SetAnnotations(ctx, r.resource, annotationsToAdd, r.reconciler.client); err != nil { return err } - h.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", h.resource.Namespace, h.resource.Name) + r.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) return nil } -func (h *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { - state, err := h.readState() +func (r *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { + state, err := r.readState() if err != nil { return xerrors.Errorf("Failed to initialize replica set state: %w", err) } - h.deploymentState = state + r.deploymentState = state return nil } // updateStatus overrides the common controller's updateStatus to ensure that the deployment state // is written after every status update. This ensures state consistency even on early returns. // It must be executed only once per reconcile (with a return) -func (h *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status workflow.Status, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { - result, err := h.reconciler.updateStatus(ctx, h.resource, status, h.log, statusOptions...) +func (r *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status workflow.Status, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { + result, err := r.reconciler.updateStatus(ctx, r.resource, status, r.log, statusOptions...) if err != nil { return result, err } // Write deployment state after every status update - if err := h.writeState(ctx); err != nil { - return h.reconciler.updateStatus(ctx, h.resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), h.log) + if err := r.writeState(ctx); err != nil { + return r.reconciler.updateStatus(ctx, r.resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), r.log) } return result, nil @@ -182,10 +182,10 @@ func (h *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status wo // Reconcile performs the full reconciliation logic for a replica set. // This is the main entry point for all reconciliation work and contains all // state and logic specific to a single reconcile execution. -func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.Result, error) { - rs := h.resource - log := h.log - reconciler := h.reconciler +func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.Result, error) { + rs := r.resource + log := r.log + reconciler := r.reconciler if !architectures.IsRunningStaticArchitecture(rs.Annotations) { agents.UpgradeAllIfNeeded(ctx, agents.ClientSecret{Client: reconciler.client, SecretClient: reconciler.SecretClient}, reconciler.omConnectionFactory, GetWatchedNamespace(), false) @@ -197,36 +197,36 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // TODO: adapt validations to multi cluster if err := rs.ProcessValidationsOnReconcile(nil); err != nil { - return h.updateStatus(ctx, workflow.Invalid("%s", err.Error())) + return r.updateStatus(ctx, workflow.Invalid("%s", err.Error())) } projectConfig, credsConfig, err := project.ReadConfigAndCredentials(ctx, reconciler.client, reconciler.SecretClient, rs, log) if err != nil { - return h.updateStatus(ctx, workflow.Failed(err)) + return r.updateStatus(ctx, workflow.Failed(err)) } conn, _, err := connection.PrepareOpsManagerConnection(ctx, reconciler.SecretClient, projectConfig, credsConfig, reconciler.omConnectionFactory, rs.Namespace, log) if err != nil { - return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Ops Manager connection: %w", err))) + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Ops Manager connection: %w", err))) } if status := ensureSupportedOpsManagerVersion(conn); status.Phase() != mdbstatus.PhaseRunning { - return h.updateStatus(ctx, status) + return r.updateStatus(ctx, status) } reconciler.SetupCommonWatchers(rs, nil, nil, rs.Name) reconcileResult := checkIfHasExcessProcesses(conn, rs.Name, log) if !reconcileResult.IsOK() { - return h.updateStatus(ctx, reconcileResult) + return r.updateStatus(ctx, reconcileResult) } if status := validateMongoDBResource(rs, conn); !status.IsOK() { - return h.updateStatus(ctx, status) + return r.updateStatus(ctx, status) } if status := controlledfeature.EnsureFeatureControls(*rs, conn, conn.OpsManagerVersion(), log); !status.IsOK() { - return h.updateStatus(ctx, status) + return r.updateStatus(ctx, status) } // === 2. Auth and Certificates @@ -254,18 +254,18 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R prometheusCertHash, err := certs.EnsureTLSCertsForPrometheus(ctx, reconciler.SecretClient, rs.GetNamespace(), rs.GetPrometheus(), certs.Database, log) if err != nil { - return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Could not generate certificates for Prometheus: %w", err))) + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Could not generate certificates for Prometheus: %w", err))) } currentAgentAuthMode, err := conn.GetAgentAuthMode() if err != nil { - return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to get agent auth mode: %w", err))) + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to get agent auth mode: %w", err))) } // Check if we need to prepare for scale-down if scale.ReplicasThisReconciliation(rs) < rs.Status.Members { if err := replicaset.PrepareScaleDownFromMongoDB(conn, rs, log); err != nil { - return h.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Replica Set for scaling down using Ops Manager: %w", err))) + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Replica Set for scaling down using Ops Manager: %w", err))) } } deploymentOpts := deploymentOptionsRS{ @@ -278,7 +278,7 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // 3. Search Overrides // Apply search overrides early so searchCoordinator role is present before ensureRoles runs // This must happen before the ordering logic to ensure roles are synced regardless of order - shouldMirrorKeyfileForMongot := h.applySearchOverrides(ctx) + shouldMirrorKeyfileForMongot := r.applySearchOverrides(ctx) // 4. Recovery @@ -287,8 +287,8 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // See CLOUDP-189433 and CLOUDP-229222 for more details. if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) { log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition) - automationConfigStatus := h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") - reconcileStatus := h.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) + automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + reconcileStatus := r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) if !reconcileStatus.IsOK() { log.Errorf("Recovery failed because of reconcile errors, %v", reconcileStatus) } @@ -299,26 +299,26 @@ func (h *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // 5. Actual reconciliation execution, Ops Manager and kubernetes resources update - publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, reconciler.client, *rs, h.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) + publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, reconciler.client, *rs, r.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { - return h.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") }, func() workflow.Status { - return h.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) + return r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) }) if !status.IsOK() { - return h.updateStatus(ctx, status) + return r.updateStatus(ctx, status) } // === 6. Final steps if scale.IsStillScaling(rs) { - return h.updateStatus(ctx, workflow.Pending("Continuing scaling operation for ReplicaSet %s, desiredMembers=%d, currentMembers=%d", rs.ObjectKey(), rs.DesiredReplicas(), scale.ReplicasThisReconciliation(rs)), mdbstatus.MembersOption(rs)) + return r.updateStatus(ctx, workflow.Pending("Continuing scaling operation for ReplicaSet %s, desiredMembers=%d, currentMembers=%d", rs.ObjectKey(), rs.DesiredReplicas(), scale.ReplicasThisReconciliation(rs)), mdbstatus.MembersOption(rs)) } log.Infof("Finished reconciliation for MongoDbReplicaSet! %s", completionMessage(conn.BaseURL(), conn.GroupID())) - return h.updateStatus(ctx, workflow.OK(), mdbstatus.NewBaseUrlOption(deployment.Link(conn.BaseURL(), conn.GroupID())), mdbstatus.MembersOption(rs), mdbstatus.NewPVCsStatusOptionEmptyStatus()) + return r.updateStatus(ctx, workflow.OK(), mdbstatus.NewBaseUrlOption(deployment.Link(conn.BaseURL(), conn.GroupID())), mdbstatus.MembersOption(rs), mdbstatus.NewPVCsStatusOptionEmptyStatus()) } func newReplicaSetReconciler(ctx context.Context, kubeClient client.Client, imageUrls images.ImageUrls, initDatabaseNonStaticImageVersion, databaseNonStaticImageVersion string, forceEnterprise bool, enableClusterMongoDBRoles bool, omFunc om.ConnectionFactory) *ReconcileMongoDbReplicaSet { @@ -452,12 +452,12 @@ func getHostnameOverrideConfigMapForReplicaset(mdb *mdbv1.MongoDB) corev1.Config return cm } -func (h *ReplicaSetReconcilerHelper) reconcileHostnameOverrideConfigMap(ctx context.Context, log *zap.SugaredLogger, getUpdateCreator configmap.GetUpdateCreator) error { - if h.resource.Spec.DbCommonSpec.GetExternalDomain() == nil { +func (r *ReplicaSetReconcilerHelper) reconcileHostnameOverrideConfigMap(ctx context.Context, log *zap.SugaredLogger, getUpdateCreator configmap.GetUpdateCreator) error { + if r.resource.Spec.DbCommonSpec.GetExternalDomain() == nil { return nil } - cm := getHostnameOverrideConfigMapForReplicaset(h.resource) + cm := getHostnameOverrideConfigMapForReplicaset(r.resource) err := configmap.CreateOrUpdate(ctx, getUpdateCreator, cm) if err != nil && !errors.IsAlreadyExists(err) { return xerrors.Errorf("failed to create configmap: %s, err: %w", cm.Name, err) @@ -470,13 +470,13 @@ func (h *ReplicaSetReconcilerHelper) reconcileHostnameOverrideConfigMap(ctx cont // reconcileMemberResources handles the synchronization of kubernetes resources, which can be statefulsets, services etc. // All the resources required in the k8s cluster (as opposed to the automation config) for creating the replicaset // should be reconciled in this method. -func (h *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { - rs := h.resource - reconciler := h.reconciler - log := h.log +func (r *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { + rs := r.resource + reconciler := r.reconciler + log := r.log // Reconcile hostname override ConfigMap - if err := h.reconcileHostnameOverrideConfigMap(ctx, log, h.reconciler.client); err != nil { + if err := r.reconcileHostnameOverrideConfigMap(ctx, log, r.reconciler.client); err != nil { return workflow.Failed(xerrors.Errorf("Failed to reconcileHostnameOverrideConfigMap: %w", err)) } @@ -485,13 +485,13 @@ func (h *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Contex return status } - return h.reconcileStatefulSet(ctx, conn, projectConfig, deploymentOptions) + return r.reconcileStatefulSet(ctx, conn, projectConfig, deploymentOptions) } -func (h *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { - rs := h.resource - reconciler := h.reconciler - log := h.log +func (r *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) workflow.Status { + rs := r.resource + reconciler := r.reconciler + log := r.log certConfigurator := certs.ReplicaSetX509CertConfigurator{MongoDB: rs, SecretClient: reconciler.SecretClient} status := reconciler.ensureX509SecretAndCheckTLSType(ctx, certConfigurator, deploymentOptions.currentAgentAuthMode, log) @@ -505,7 +505,7 @@ func (h *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c } // Build the replica set config - rsConfig, err := h.buildStatefulSetOptions(ctx, conn, projectConfig, deploymentOptions) + rsConfig, err := r.buildStatefulSetOptions(ctx, conn, projectConfig, deploymentOptions) if err != nil { return workflow.Failed(xerrors.Errorf("failed to build StatefulSet options: %w", err)) } @@ -538,10 +538,10 @@ func (h *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c } // buildStatefulSetOptions creates the options needed for constructing the StatefulSet -func (h *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) (func(mdb mdbv1.MongoDB) construct.DatabaseStatefulSetOptions, error) { - rs := h.resource - reconciler := h.reconciler - log := h.log +func (r *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) (func(mdb mdbv1.MongoDB) construct.DatabaseStatefulSetOptions, error) { + rs := r.resource + reconciler := r.reconciler + log := r.log rsCertsConfig := certs.ReplicaSetConfig(*rs) @@ -669,10 +669,10 @@ func AddReplicaSetController(ctx context.Context, mgr manager.Manager, imageUrls // updateOmDeploymentRs performs OM registration operation for the replicaset. So the changes will be finally propagated // to automation agents in containers -func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, tlsCertPath, internalClusterCertPath string, deploymentOptions deploymentOptionsRS, shouldMirrorKeyfileForMongot bool, isRecovering bool) workflow.Status { - rs := h.resource - log := h.log - reconciler := h.reconciler +func (r *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, conn om.Connection, membersNumberBefore int, tlsCertPath, internalClusterCertPath string, deploymentOptions deploymentOptionsRS, shouldMirrorKeyfileForMongot bool, isRecovering bool) workflow.Status { + rs := r.resource + log := r.log + reconciler := r.reconciler log.Debug("Entering UpdateOMDeployments") // Only "concrete" RS members should be observed // - if scaling down, let's observe only members that will remain after scale-down operation @@ -686,7 +686,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c caFilePath := fmt.Sprintf("%s/ca-pem", util.TLSCaMountPath) // If current operation is to Disable TLS, then we should the current members of the Replica Set, // this is, do not scale them up or down util TLS disabling has completed. - shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, reconciler.imageUrls[mcoConstruct.MongodbImageEnv], reconciler.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, h.deploymentState.LastAchievedSpec, log) + shouldLockMembers, err := updateOmDeploymentDisableTLSConfiguration(conn, reconciler.imageUrls[mcoConstruct.MongodbImageEnv], reconciler.forceEnterprise, membersNumberBefore, rs, caFilePath, tlsCertPath, r.deploymentState.LastAchievedSpec, log) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -717,7 +717,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c return status } - lastRsConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(h.deploymentState.LastAchievedSpec, mdbv1.ReplicaSetConfig) + lastRsConfig, err := mdbv1.GetLastAdditionalMongodConfigByType(r.deploymentState.LastAchievedSpec, mdbv1.ReplicaSetConfig) if err != nil && !isRecovering { return workflow.Failed(err) } @@ -733,7 +733,7 @@ func (h *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c err = conn.ReadUpdateDeployment( func(d om.Deployment) error { if shouldMirrorKeyfileForMongot { - if err := h.mirrorKeyfileIntoSecretForMongot(ctx, d); err != nil { + if err := r.mirrorKeyfileIntoSecretForMongot(ctx, d); err != nil { return err } } @@ -879,11 +879,11 @@ func getAllHostsForReplicas(rs *mdbv1.MongoDB, membersCount int) []string { return hostnames } -func (h *ReplicaSetReconcilerHelper) applySearchOverrides(ctx context.Context) bool { - rs := h.resource - log := h.log +func (r *ReplicaSetReconcilerHelper) applySearchOverrides(ctx context.Context) bool { + rs := r.resource + log := r.log - search := h.lookupCorrespondingSearchResource(ctx) + search := r.lookupCorrespondingSearchResource(ctx) if search == nil { log.Debugf("No MongoDBSearch resource found, skipping search overrides") return false @@ -909,10 +909,10 @@ func (h *ReplicaSetReconcilerHelper) applySearchOverrides(ctx context.Context) b return true } -func (h *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx context.Context, d om.Deployment) error { - rs := h.resource - reconciler := h.reconciler - log := h.log +func (r *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx context.Context, d om.Deployment) error { + rs := r.resource + reconciler := r.reconciler + log := r.log keyfileContents := maputil.ReadMapValueAsString(d, "auth", "key") keyfileSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("%s-%s", rs.Name, searchcontroller.MongotKeyfileFilename), Namespace: rs.Namespace}} @@ -929,10 +929,10 @@ func (h *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx contex return nil } -func (h *ReplicaSetReconcilerHelper) lookupCorrespondingSearchResource(ctx context.Context) *searchv1.MongoDBSearch { - rs := h.resource - reconciler := h.reconciler - log := h.log +func (r *ReplicaSetReconcilerHelper) lookupCorrespondingSearchResource(ctx context.Context) *searchv1.MongoDBSearch { + rs := r.resource + reconciler := r.reconciler + log := r.log var search *searchv1.MongoDBSearch searchList := &searchv1.MongoDBSearchList{} From 08ffa3eb7071f5487a654d9ba2d06d47fb26431b Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 14:20:36 +0200 Subject: [PATCH 11/25] Remove TODOs --- .../operator/mongodbreplicaset_controller.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 297413d2e..318b0b5ce 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -195,7 +195,6 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R log.Infow("ReplicaSet.Spec", "spec", rs.Spec, "desiredReplicas", scale.ReplicasThisReconciliation(rs), "isScaling", scale.IsStillScaling(rs)) log.Infow("ReplicaSet.Status", "status", rs.Status) - // TODO: adapt validations to multi cluster if err := rs.ProcessValidationsOnReconcile(nil); err != nil { return r.updateStatus(ctx, workflow.Invalid("%s", err.Error())) } @@ -518,7 +517,6 @@ func (r *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c return workflowStatus } - // TODO: check if updatestatus usage is correct here if workflow.ContainsPVCOption(workflowStatus.StatusOptions()) { _, _ = reconciler.updateStatus(ctx, rs, workflow.Pending(""), log, workflowStatus.StatusOptions()...) } @@ -766,7 +764,6 @@ func (r *ReplicaSetReconcilerHelper) updateOmDeploymentRs(ctx context.Context, c return workflow.Failed(err) } - // TODO: check if updateStatus usage is correct here if status := reconciler.ensureBackupConfigurationAndUpdateStatus(ctx, conn, rs, reconciler.SecretClient, log); !status.IsOK() && !isRecovering { return status } @@ -858,12 +855,10 @@ func (r *ReconcileMongoDbReplicaSet) OnDelete(ctx context.Context, obj runtime.O return err } - if err := r.clearProjectAuthenticationSettings(ctx, conn, rs, processNames, log); err != nil { + if err := r.reconciler.clearProjectAuthenticationSettings(ctx, conn, rs, processNames, log); err != nil { return err } - r.resourceWatcher.RemoveDependentWatchedResources(rs.ObjectKey()) - log.Infow("Clear feature control for group: %s", "groupID", conn.GroupID()) if result := controlledfeature.ClearFeatureControls(conn, conn.OpsManagerVersion(), log); !result.IsOK() { result.Log(log) @@ -874,6 +869,14 @@ func (r *ReconcileMongoDbReplicaSet) OnDelete(ctx context.Context, obj runtime.O return nil } +func (r *ReconcileMongoDbReplicaSet) OnDelete(ctx context.Context, obj runtime.Object, log *zap.SugaredLogger) error { + helper, err := r.newReconcilerHelper(ctx, obj.(*mdbv1.MongoDB), log) + if err != nil { + return err + } + return helper.OnDelete(ctx, obj, log) +} + func getAllHostsForReplicas(rs *mdbv1.MongoDB, membersCount int) []string { hostnames, _ := dns.GetDNSNames(rs.Name, rs.ServiceName(), rs.Namespace, rs.Spec.GetClusterDomain(), membersCount, rs.Spec.DbCommonSpec.GetExternalDomain()) return hostnames From c99c22193757eed5588af77e773d1a6e9655d438 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 14:20:52 +0200 Subject: [PATCH 12/25] Refactor onDelete --- .../operator/mongodbreplicaset_controller.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 318b0b5ce..995681231 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -806,20 +806,30 @@ func updateOmDeploymentDisableTLSConfiguration(conn om.Connection, mongoDBImage return tlsConfigWasDisabled, err } -// TODO: split into subfunctions, follow helper pattern -func (r *ReconcileMongoDbReplicaSet) OnDelete(ctx context.Context, obj runtime.Object, log *zap.SugaredLogger) error { +func (r *ReplicaSetReconcilerHelper) OnDelete(ctx context.Context, obj runtime.Object, log *zap.SugaredLogger) error { rs := obj.(*mdbv1.MongoDB) - projectConfig, credsConfig, err := project.ReadConfigAndCredentials(ctx, r.client, r.SecretClient, rs, log) + if err := r.cleanOpsManagerState(ctx, rs, log); err != nil { + return err + } + + r.reconciler.resourceWatcher.RemoveDependentWatchedResources(rs.ObjectKey()) + + return nil +} + +func (r *ReplicaSetReconcilerHelper) cleanOpsManagerState(ctx context.Context, rs *mdbv1.MongoDB, log *zap.SugaredLogger) error { + projectConfig, credsConfig, err := project.ReadConfigAndCredentials(ctx, r.reconciler.client, r.reconciler.SecretClient, rs, log) if err != nil { return err } log.Infow("Removing replica set from Ops Manager", "config", rs.Spec) - conn, _, err := connection.PrepareOpsManagerConnection(ctx, r.SecretClient, projectConfig, credsConfig, r.omConnectionFactory, rs.Namespace, log) + conn, _, err := connection.PrepareOpsManagerConnection(ctx, r.reconciler.SecretClient, projectConfig, credsConfig, r.reconciler.omConnectionFactory, rs.Namespace, log) if err != nil { return err } + processNames := make([]string, 0) err = conn.ReadUpdateDeployment( func(d om.Deployment) error { From 5e39e4a1f50674e5e0437f3931c35a8e7fd2121f Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 15:33:35 +0200 Subject: [PATCH 13/25] Move comments --- controllers/operator/mongodbreplicaset_controller.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 995681231..d473d7fa9 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -187,6 +187,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R log := r.log reconciler := r.reconciler + // === 1. Initial Checks and setup if !architectures.IsRunningStaticArchitecture(rs.Annotations) { agents.UpgradeAllIfNeeded(ctx, agents.ClientSecret{Client: reconciler.client, SecretClient: reconciler.SecretClient}, reconciler.omConnectionFactory, GetWatchedNamespace(), false) } @@ -229,7 +230,6 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R } // === 2. Auth and Certificates - // Get certificate paths for later use rsCertsConfig := certs.ReplicaSetConfig(*rs) var databaseSecretPath string @@ -280,7 +280,6 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R shouldMirrorKeyfileForMongot := r.applySearchOverrides(ctx) // 4. Recovery - // Recovery prevents some deadlocks that can occur during reconciliation, e.g. the setting of an incorrect automation // configuration and a subsequent attempt to overwrite it later, the operator would be stuck in Pending phase. // See CLOUDP-189433 and CLOUDP-229222 for more details. @@ -297,7 +296,6 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R } // 5. Actual reconciliation execution, Ops Manager and kubernetes resources update - publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, reconciler.client, *rs, r.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { @@ -359,7 +357,6 @@ type deploymentOptionsRS struct { // Reconcile reads that state of the cluster for a MongoDbReplicaSet object and makes changes based on the state read // and what is in the MongoDbReplicaSet.Spec func (r *ReconcileMongoDbReplicaSet) Reconcile(ctx context.Context, request reconcile.Request) (res reconcile.Result, e error) { - // === 1. Initial Checks and setup log := zap.S().With("ReplicaSet", request.NamespacedName) rs := &mdbv1.MongoDB{} From 279a7acc37cba103b7e0179fedc3bf312d43ceff Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 21 Oct 2025 16:24:13 +0200 Subject: [PATCH 14/25] Update vault annotations only when reconciliation is successful --- .../operator/mongodbreplicaset_controller.go | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index d473d7fa9..b0f7329bf 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -121,16 +121,17 @@ func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, er } } -// writeState abstract writing the state of the resource that we store on the cluster between reconciliations. -func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { +// writeState writes the state of the resource that we store on the cluster between reconciliations. +// This includes the lastAchievedSpec annotation (always written) and vault secret version annotations on successful +// reconciliation +func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context, includeVaultAnnotations bool) error { // Serialize the state to annotations annotationsToAdd, err := getAnnotationsForResource(r.resource) if err != nil { return err } - // Add vault annotations if needed - if vault.IsVaultSecretBackend() { + if includeVaultAnnotations && vault.IsVaultSecretBackend() { secrets := r.resource.GetSecretsMountedIntoDBPod() vaultMap := make(map[string]string) for _, s := range secrets { @@ -149,7 +150,11 @@ func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { return err } - r.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) + if includeVaultAnnotations { + r.log.Debugf("Successfully wrote deployment state and vault annotations for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) + } else { + r.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) + } return nil } @@ -164,7 +169,8 @@ func (r *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { // updateStatus overrides the common controller's updateStatus to ensure that the deployment state // is written after every status update. This ensures state consistency even on early returns. -// It must be executed only once per reconcile (with a return) +// Vault annotations are only included when status.IsOK() is true, meaning reconciliation succeeded. +// This function must be executed only once per reconcile (with a return) func (r *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status workflow.Status, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { result, err := r.reconciler.updateStatus(ctx, r.resource, status, r.log, statusOptions...) if err != nil { @@ -172,7 +178,8 @@ func (r *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status wo } // Write deployment state after every status update - if err := r.writeState(ctx); err != nil { + // Include vault annotations only on successful reconciliation + if err := r.writeState(ctx, status.IsOK()); err != nil { return r.reconciler.updateStatus(ctx, r.resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), r.log) } From 5f5cebd1beb9f3855c68f990968d923f86cc399b Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Wed, 22 Oct 2025 14:46:32 +0200 Subject: [PATCH 15/25] Handle PVC Resize in a separate method --- .../operator/mongodbreplicaset_controller.go | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index b0f7329bf..08761ba20 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -516,15 +516,10 @@ func (r *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c sts := construct.DatabaseStatefulSet(*rs, rsConfig, log) // Handle PVC resize if needed - workflowStatus := create.HandlePVCResize(ctx, reconciler.client, &sts, log) - if !workflowStatus.IsOK() { + if workflowStatus := r.handlePVCResize(ctx, &sts); !workflowStatus.IsOK() { return workflowStatus } - if workflow.ContainsPVCOption(workflowStatus.StatusOptions()) { - _, _ = reconciler.updateStatus(ctx, rs, workflow.Pending(""), log, workflowStatus.StatusOptions()...) - } - // Create or update the StatefulSet in Kubernetes if err := create.DatabaseInKubernetes(ctx, reconciler.client, *rs, sts, rsConfig, log); err != nil { return workflow.Failed(xerrors.Errorf("Failed to create/update (Kubernetes reconciliation phase): %w", err)) @@ -539,6 +534,20 @@ func (r *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c return workflow.OK() } +func (r *ReplicaSetReconcilerHelper) handlePVCResize(ctx context.Context, sts *appsv1.StatefulSet) workflow.Status { + workflowStatus := create.HandlePVCResize(ctx, r.reconciler.client, sts, r.log) + if !workflowStatus.IsOK() { + return workflowStatus + } + + if workflow.ContainsPVCOption(workflowStatus.StatusOptions()) { + if _, err := r.reconciler.updateStatus(ctx, r.resource, workflow.Pending(""), r.log, workflowStatus.StatusOptions()...); err != nil { + return workflow.Failed(xerrors.Errorf("error updating status: %w", err)) + } + } + return workflow.OK() +} + // buildStatefulSetOptions creates the options needed for constructing the StatefulSet func (r *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context, conn om.Connection, projectConfig mdbv1.ProjectConfig, deploymentOptions deploymentOptionsRS) (func(mdb mdbv1.MongoDB) construct.DatabaseStatefulSetOptions, error) { rs := r.resource From b831d72cbad4fbb6f0fcf2f395faf9c563f08bc2 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Fri, 24 Oct 2025 15:42:25 +0200 Subject: [PATCH 16/25] Add status member count to the state --- .../operator/mongodbreplicaset_controller.go | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 08761ba20..e41c328a2 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -79,7 +79,8 @@ type ReconcileMongoDbReplicaSet struct { } type ReplicaSetDeploymentState struct { - LastAchievedSpec *mdbv1.MongoDbSpec `json:"lastAchievedSpec"` + LastAchievedSpec *mdbv1.MongoDbSpec `json:"lastAchievedSpec"` + MemberCountBefore int `json:"memberCountBefore"` } var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} @@ -114,11 +115,19 @@ func (r *ReconcileMongoDbReplicaSet) newReconcilerHelper( // readState abstract reading the state of the resource that we store on the cluster between reconciliations. func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, error) { // Try to get the last achieved spec from annotations and store it in state - if lastAchievedSpec, err := r.resource.GetLastSpec(); err != nil { + lastAchievedSpec, err := r.resource.GetLastSpec() + if err != nil { return nil, err - } else { - return &ReplicaSetDeploymentState{LastAchievedSpec: lastAchievedSpec}, nil } + + // Read current member count from Status once at initialization. This provides a stable view throughout + // reconciliation and prepares for eventually storing this in ConfigMap state instead of ephemeral status. + memberCountBefore := r.resource.Status.Members + + return &ReplicaSetDeploymentState{ + LastAchievedSpec: lastAchievedSpec, + MemberCountBefore: memberCountBefore, + }, nil } // writeState writes the state of the resource that we store on the cluster between reconciliations. @@ -269,7 +278,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R } // Check if we need to prepare for scale-down - if scale.ReplicasThisReconciliation(rs) < rs.Status.Members { + if scale.ReplicasThisReconciliation(rs) < r.deploymentState.MemberCountBefore { if err := replicaset.PrepareScaleDownFromMongoDB(conn, rs, log); err != nil { return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Replica Set for scaling down using Ops Manager: %w", err))) } @@ -292,7 +301,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // See CLOUDP-189433 and CLOUDP-229222 for more details. if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) { log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition) - automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") reconcileStatus := r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) if !reconcileStatus.IsOK() { log.Errorf("Recovery failed because of reconcile errors, %v", reconcileStatus) @@ -306,7 +315,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, reconciler.client, *rs, r.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { - return r.updateOmDeploymentRs(ctx, conn, rs.Status.Members, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + return r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") }, func() workflow.Status { return r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) @@ -871,6 +880,8 @@ func (r *ReplicaSetReconcilerHelper) cleanOpsManagerState(ctx context.Context, r } } + // During deletion, calculate the maximum number of hosts that could possibly exist to ensure complete cleanup. + // Reading from Status here is appropriate since this is outside the reconciliation loop. hostsToRemove, _ := dns.GetDNSNames(rs.Name, rs.ServiceName(), rs.Namespace, rs.Spec.GetClusterDomain(), util.MaxInt(rs.Status.Members, rs.Spec.Members), nil) log.Infow("Stop monitoring removed hosts in Ops Manager", "removedHosts", hostsToRemove) From ac8fb339060e6e7705ddd1254b5066060cb142ae Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 28 Oct 2025 10:26:15 +0100 Subject: [PATCH 17/25] Uncapitalize errors --- .../operator/mongodbreplicaset_controller.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index e41c328a2..d6b64cbee 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -170,7 +170,7 @@ func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context, includeVaul func (r *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { state, err := r.readState() if err != nil { - return xerrors.Errorf("Failed to initialize replica set state: %w", err) + return xerrors.Errorf("failed to initialize replica set state: %w", err) } r.deploymentState = state return nil @@ -189,7 +189,7 @@ func (r *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status wo // Write deployment state after every status update // Include vault annotations only on successful reconciliation if err := r.writeState(ctx, status.IsOK()); err != nil { - return r.reconciler.updateStatus(ctx, r.resource, workflow.Failed(xerrors.Errorf("Failed to write deployment state after updating status: %w", err)), r.log) + return r.reconciler.updateStatus(ctx, r.resource, workflow.Failed(xerrors.Errorf("failed to write deployment state after updating status: %w", err)), r.log) } return result, nil @@ -223,7 +223,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R conn, _, err := connection.PrepareOpsManagerConnection(ctx, reconciler.SecretClient, projectConfig, credsConfig, reconciler.omConnectionFactory, rs.Namespace, log) if err != nil { - return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Ops Manager connection: %w", err))) + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to prepare Ops Manager connection: %w", err))) } if status := ensureSupportedOpsManagerVersion(conn); status.Phase() != mdbstatus.PhaseRunning { @@ -269,7 +269,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R prometheusCertHash, err := certs.EnsureTLSCertsForPrometheus(ctx, reconciler.SecretClient, rs.GetNamespace(), rs.GetPrometheus(), certs.Database, log) if err != nil { - return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Could not generate certificates for Prometheus: %w", err))) + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("could not generate certificates for Prometheus: %w", err))) } currentAgentAuthMode, err := conn.GetAgentAuthMode() @@ -280,7 +280,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // Check if we need to prepare for scale-down if scale.ReplicasThisReconciliation(rs) < r.deploymentState.MemberCountBefore { if err := replicaset.PrepareScaleDownFromMongoDB(conn, rs, log); err != nil { - return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("Failed to prepare Replica Set for scaling down using Ops Manager: %w", err))) + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to prepare Replica Set for scaling down using Ops Manager: %w", err))) } } deploymentOpts := deploymentOptionsRS{ @@ -301,7 +301,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // See CLOUDP-189433 and CLOUDP-229222 for more details. if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) { log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition) - automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("failed to create/update (Ops Manager reconciliation phase):") reconcileStatus := r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) if !reconcileStatus.IsOK() { log.Errorf("Recovery failed because of reconcile errors, %v", reconcileStatus) @@ -315,7 +315,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, reconciler.client, *rs, r.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { - return r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("Failed to create/update (Ops Manager reconciliation phase):") + return r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("failed to create/update (Ops Manager reconciliation phase):") }, func() workflow.Status { return r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) @@ -489,7 +489,7 @@ func (r *ReplicaSetReconcilerHelper) reconcileMemberResources(ctx context.Contex // Reconcile hostname override ConfigMap if err := r.reconcileHostnameOverrideConfigMap(ctx, log, r.reconciler.client); err != nil { - return workflow.Failed(xerrors.Errorf("Failed to reconcileHostnameOverrideConfigMap: %w", err)) + return workflow.Failed(xerrors.Errorf("failed to reconcile hostname override ConfigMap: %w", err)) } // Ensure roles are properly configured @@ -531,7 +531,7 @@ func (r *ReplicaSetReconcilerHelper) reconcileStatefulSet(ctx context.Context, c // Create or update the StatefulSet in Kubernetes if err := create.DatabaseInKubernetes(ctx, reconciler.client, *rs, sts, rsConfig, log); err != nil { - return workflow.Failed(xerrors.Errorf("Failed to create/update (Kubernetes reconciliation phase): %w", err)) + return workflow.Failed(xerrors.Errorf("failed to create/update (Kubernetes reconciliation phase): %w", err)) } // Check StatefulSet status @@ -581,7 +581,7 @@ func (r *ReplicaSetReconcilerHelper) buildStatefulSetOptions(ctx context.Context var err error automationAgentVersion, err = reconciler.getAgentVersion(conn, conn.OpsManagerVersion().VersionString, false, log) if err != nil { - return nil, xerrors.Errorf("Impossible to get agent version, please override the agent image by providing a pod template: %w", err) + return nil, xerrors.Errorf("impossible to get agent version, please override the agent image by providing a pod template: %w", err) } } } @@ -961,7 +961,7 @@ func (r *ReplicaSetReconcilerHelper) mirrorKeyfileIntoSecretForMongot(ctx contex return controllerutil.SetOwnerReference(rs, keyfileSecret, reconciler.client.Scheme()) }) if err != nil { - return xerrors.Errorf("Failed to mirror the replicaset's keyfile into a secret: %w", err) + return xerrors.Errorf("failed to mirror the replicaset's keyfile into a secret: %w", err) } return nil } From a777b2f428919b15182e62899afc6e7fddff0087 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 28 Oct 2025 16:31:28 +0100 Subject: [PATCH 18/25] Write lastAchievedSpec only on successful reconciliation, separate vault annotations --- .../operator/mongodbreplicaset_controller.go | 86 ++++++++++--------- 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index d6b64cbee..36af8f6ab 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -130,40 +130,51 @@ func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, er }, nil } -// writeState writes the state of the resource that we store on the cluster between reconciliations. -// This includes the lastAchievedSpec annotation (always written) and vault secret version annotations on successful -// reconciliation -func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context, includeVaultAnnotations bool) error { - // Serialize the state to annotations +// writeState writes the lastAchievedSpec annotation to the resource. +// This should only be called on successful reconciliation. +// The state is currently split between the annotations and the member count in status. Both should be migrated +// to config maps +func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { + // Get lastAchievedSpec annotation annotationsToAdd, err := getAnnotationsForResource(r.resource) if err != nil { return err } - if includeVaultAnnotations && vault.IsVaultSecretBackend() { - secrets := r.resource.GetSecretsMountedIntoDBPod() - vaultMap := make(map[string]string) - for _, s := range secrets { - path := fmt.Sprintf("%s/%s/%s", r.reconciler.VaultClient.DatabaseSecretMetadataPath(), r.resource.Namespace, s) - vaultMap = merge.StringToStringMap(vaultMap, r.reconciler.VaultClient.GetSecretAnnotation(path)) - } - path := fmt.Sprintf("%s/%s/%s", r.reconciler.VaultClient.OperatorScretMetadataPath(), r.resource.Namespace, r.resource.Spec.Credentials) + // Write to CR + if err := annotations.SetAnnotations(ctx, r.resource, annotationsToAdd, r.reconciler.client); err != nil { + return err + } + + r.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) + return nil +} + +// writeVaultAnnotations writes vault secret version annotations to the CR. +// This should only be called on successful reconciliation. +func (r *ReplicaSetReconcilerHelper) writeVaultAnnotations(ctx context.Context) error { + if !vault.IsVaultSecretBackend() { + return nil + } + + vaultMap := make(map[string]string) + secrets := r.resource.GetSecretsMountedIntoDBPod() + + for _, s := range secrets { + path := fmt.Sprintf("%s/%s/%s", r.reconciler.VaultClient.DatabaseSecretMetadataPath(), + r.resource.Namespace, s) vaultMap = merge.StringToStringMap(vaultMap, r.reconciler.VaultClient.GetSecretAnnotation(path)) - for k, val := range vaultMap { - annotationsToAdd[k] = val - } } - // Write annotations back to the resource - if err := annotations.SetAnnotations(ctx, r.resource, annotationsToAdd, r.reconciler.client); err != nil { + path := fmt.Sprintf("%s/%s/%s", r.reconciler.VaultClient.OperatorScretMetadataPath(), + r.resource.Namespace, r.resource.Spec.Credentials) + vaultMap = merge.StringToStringMap(vaultMap, r.reconciler.VaultClient.GetSecretAnnotation(path)) + + if err := annotations.SetAnnotations(ctx, r.resource, vaultMap, r.reconciler.client); err != nil { return err } - if includeVaultAnnotations { - r.log.Debugf("Successfully wrote deployment state and vault annotations for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) - } else { - r.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) - } + r.log.Debugf("Successfully wrote vault annotations for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) return nil } @@ -176,23 +187,11 @@ func (r *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { return nil } -// updateStatus overrides the common controller's updateStatus to ensure that the deployment state -// is written after every status update. This ensures state consistency even on early returns. -// Vault annotations are only included when status.IsOK() is true, meaning reconciliation succeeded. -// This function must be executed only once per reconcile (with a return) +// updateStatus is a pass-through method that calls the reconciler updateStatus. +// In the future (multi-cluster epic), this will be enhanced to write deployment state to ConfigMap after every status +// update (similar to sharded cluster pattern), but for now it just delegates to maintain the same architecture. func (r *ReplicaSetReconcilerHelper) updateStatus(ctx context.Context, status workflow.Status, statusOptions ...mdbstatus.Option) (reconcile.Result, error) { - result, err := r.reconciler.updateStatus(ctx, r.resource, status, r.log, statusOptions...) - if err != nil { - return result, err - } - - // Write deployment state after every status update - // Include vault annotations only on successful reconciliation - if err := r.writeState(ctx, status.IsOK()); err != nil { - return r.reconciler.updateStatus(ctx, r.resource, workflow.Failed(xerrors.Errorf("failed to write deployment state after updating status: %w", err)), r.log) - } - - return result, nil + return r.reconciler.updateStatus(ctx, r.resource, status, r.log, statusOptions...) } // Reconcile performs the full reconciliation logic for a replica set. @@ -330,6 +329,15 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R return r.updateStatus(ctx, workflow.Pending("Continuing scaling operation for ReplicaSet %s, desiredMembers=%d, currentMembers=%d", rs.ObjectKey(), rs.DesiredReplicas(), scale.ReplicasThisReconciliation(rs)), mdbstatus.MembersOption(rs)) } + // Write state and vault annotations on successful reconciliation + if err := r.writeState(ctx); err != nil { + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to write state: %w", err))) + } + + if err := r.writeVaultAnnotations(ctx); err != nil { + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to write vault annotations: %w", err))) + } + log.Infof("Finished reconciliation for MongoDbReplicaSet! %s", completionMessage(conn.BaseURL(), conn.GroupID())) return r.updateStatus(ctx, workflow.OK(), mdbstatus.NewBaseUrlOption(deployment.Link(conn.BaseURL(), conn.GroupID())), mdbstatus.MembersOption(rs), mdbstatus.NewPVCsStatusOptionEmptyStatus()) } From d5bdd182edce0e60f9e9ae910bf9e39d95dcd83e Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Tue, 28 Oct 2025 16:32:31 +0100 Subject: [PATCH 19/25] Rename MemberCountBefore --- .../operator/mongodbreplicaset_controller.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 36af8f6ab..2e57dd4c6 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -79,8 +79,8 @@ type ReconcileMongoDbReplicaSet struct { } type ReplicaSetDeploymentState struct { - LastAchievedSpec *mdbv1.MongoDbSpec `json:"lastAchievedSpec"` - MemberCountBefore int `json:"memberCountBefore"` + LastAchievedSpec *mdbv1.MongoDbSpec `json:"lastAchievedSpec"` + LastReconcileMemberCount int `json:"memberCountBefore"` } var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} @@ -125,8 +125,8 @@ func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, er memberCountBefore := r.resource.Status.Members return &ReplicaSetDeploymentState{ - LastAchievedSpec: lastAchievedSpec, - MemberCountBefore: memberCountBefore, + LastAchievedSpec: lastAchievedSpec, + LastReconcileMemberCount: memberCountBefore, }, nil } @@ -277,7 +277,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R } // Check if we need to prepare for scale-down - if scale.ReplicasThisReconciliation(rs) < r.deploymentState.MemberCountBefore { + if scale.ReplicasThisReconciliation(rs) < r.deploymentState.LastReconcileMemberCount { if err := replicaset.PrepareScaleDownFromMongoDB(conn, rs, log); err != nil { return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to prepare Replica Set for scaling down using Ops Manager: %w", err))) } @@ -300,7 +300,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R // See CLOUDP-189433 and CLOUDP-229222 for more details. if recovery.ShouldTriggerRecovery(rs.Status.Phase != mdbstatus.PhaseRunning, rs.Status.LastTransition) { log.Warnf("Triggering Automatic Recovery. The MongoDB resource %s/%s is in %s state since %s", rs.Namespace, rs.Name, rs.Status.Phase, rs.Status.LastTransition) - automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("failed to create/update (Ops Manager reconciliation phase):") + automationConfigStatus := r.updateOmDeploymentRs(ctx, conn, r.deploymentState.LastReconcileMemberCount, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, true).OnErrorPrepend("failed to create/update (Ops Manager reconciliation phase):") reconcileStatus := r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) if !reconcileStatus.IsOK() { log.Errorf("Recovery failed because of reconcile errors, %v", reconcileStatus) @@ -314,7 +314,7 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R publishAutomationConfigFirst := publishAutomationConfigFirstRS(ctx, reconciler.client, *rs, r.deploymentState.LastAchievedSpec, deploymentOpts.currentAgentAuthMode, projectConfig.SSLMMSCAConfigMap, log) status := workflow.RunInGivenOrder(publishAutomationConfigFirst, func() workflow.Status { - return r.updateOmDeploymentRs(ctx, conn, r.deploymentState.MemberCountBefore, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("failed to create/update (Ops Manager reconciliation phase):") + return r.updateOmDeploymentRs(ctx, conn, r.deploymentState.LastReconcileMemberCount, tlsCertPath, internalClusterCertPath, deploymentOpts, shouldMirrorKeyfileForMongot, false).OnErrorPrepend("failed to create/update (Ops Manager reconciliation phase):") }, func() workflow.Status { return r.reconcileMemberResources(ctx, conn, projectConfig, deploymentOpts) From bbb173b54decb10420a571181b56703a5ceb6a77 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Wed, 29 Oct 2025 16:49:46 +0100 Subject: [PATCH 20/25] Some unit tests for the annotations Some unit tests for the annotations --- .../mongodbreplicaset_controller_test.go | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/controllers/operator/mongodbreplicaset_controller_test.go b/controllers/operator/mongodbreplicaset_controller_test.go index dfda4efc9..9f78fbc2c 100644 --- a/controllers/operator/mongodbreplicaset_controller_test.go +++ b/controllers/operator/mongodbreplicaset_controller_test.go @@ -2,6 +2,7 @@ package operator import ( "context" + "encoding/json" "fmt" "reflect" "testing" @@ -891,6 +892,130 @@ func TestHandlePVCResize(t *testing.T) { testPVCFinishedResizing(t, ctx, memberClient, p, reconciledResource, statefulSet, logger) } +// ===== Test for state and vault annotations handling in replicaset controller ===== + +// TestReplicaSetAnnotations_WrittenOnSuccess verifies that lastAchievedSpec annotation is written after successful +// reconciliation. +func TestReplicaSetAnnotations_WrittenOnSuccess(t *testing.T) { + ctx := context.Background() + rs := DefaultReplicaSetBuilder().Build() + + reconciler, client, _ := defaultReplicaSetReconciler(ctx, nil, "", "", rs) + + checkReconcileSuccessful(ctx, t, reconciler, rs, client) + + err := client.Get(ctx, rs.ObjectKey(), rs) + require.NoError(t, err) + + require.Contains(t, rs.Annotations, util.LastAchievedSpec, + "lastAchievedSpec annotation should be written on successful reconciliation") + + var lastSpec mdbv1.MongoDbSpec + err = json.Unmarshal([]byte(rs.Annotations[util.LastAchievedSpec]), &lastSpec) + require.NoError(t, err) + assert.Equal(t, 3, lastSpec.Members) + assert.Equal(t, "4.0.0", lastSpec.Version) +} + +// TestReplicaSetAnnotations_NotWrittenOnFailure verifies that lastAchievedSpec annotation +// is NOT written when reconciliation fails. +func TestReplicaSetAnnotations_NotWrittenOnFailure(t *testing.T) { + ctx := context.Background() + rs := DefaultReplicaSetBuilder().Build() + + // Setup without credentials secret to cause failure + kubeClient := mock.NewEmptyFakeClientBuilder(). + WithObjects(rs). + WithObjects(mock.GetProjectConfigMap(mock.TestProjectConfigMapName, "testProject", "testOrg")). + Build() + + reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, nil) + + _, err := reconciler.Reconcile(ctx, requestFromObject(rs)) + require.NoError(t, err, "Reconcile should not return error (error captured in status)") + + err = kubeClient.Get(ctx, rs.ObjectKey(), rs) + require.NoError(t, err) + + assert.NotEqual(t, status.PhaseRunning, rs.Status.Phase) + + assert.NotContains(t, rs.Annotations, util.LastAchievedSpec, + "lastAchievedSpec should NOT be written when reconciliation fails") +} + +// TestReplicaSetAnnotations_PreservedOnSubsequentFailure verifies that annotations from a previous successful +// reconciliation are preserved when a later reconciliation fails. +func TestReplicaSetAnnotations_PreservedOnSubsequentFailure(t *testing.T) { + ctx := context.Background() + rs := DefaultReplicaSetBuilder().Build() + + kubeClient, omConnectionFactory := mock.NewDefaultFakeClient(rs) + reconciler := newReplicaSetReconciler(ctx, kubeClient, nil, "", "", false, false, omConnectionFactory.GetConnectionFunc) + + _, err := reconciler.Reconcile(ctx, requestFromObject(rs)) + require.NoError(t, err) + + err = kubeClient.Get(ctx, rs.ObjectKey(), rs) + require.NoError(t, err) + require.Contains(t, rs.Annotations, util.LastAchievedSpec) + + originalLastAchievedSpec := rs.Annotations[util.LastAchievedSpec] + + // Delete credentials to cause failure + credentialsSecret := mock.GetCredentialsSecret("testUser", "testApiKey") + err = kubeClient.Delete(ctx, credentialsSecret) + require.NoError(t, err) + + rs.Spec.Members = 5 + err = kubeClient.Update(ctx, rs) + require.NoError(t, err) + + _, err = reconciler.Reconcile(ctx, requestFromObject(rs)) + require.NoError(t, err) + + err = kubeClient.Get(ctx, rs.ObjectKey(), rs) + require.NoError(t, err) + + assert.Contains(t, rs.Annotations, util.LastAchievedSpec) + assert.NotEqual(t, status.PhaseRunning, rs.Status.Phase) + assert.Equal(t, originalLastAchievedSpec, rs.Annotations[util.LastAchievedSpec], + "lastAchievedSpec should remain unchanged when reconciliation fails") + + var lastSpec mdbv1.MongoDbSpec + err = json.Unmarshal([]byte(rs.Annotations[util.LastAchievedSpec]), &lastSpec) + require.NoError(t, err) + assert.Equal(t, 3, lastSpec.Members, + "Should still reflect previous successful state (3 members, not 5)") +} + +// TestVaultAnnotations_NotWrittenWhenDisabled verifies that vault annotations are NOT +// written when vault backend is disabled. +func TestVaultAnnotations_NotWrittenWhenDisabled(t *testing.T) { + ctx := context.Background() + rs := DefaultReplicaSetBuilder().Build() + + t.Setenv("SECRET_BACKEND", "K8S_SECRET_BACKEND") + + reconciler, client, _ := defaultReplicaSetReconciler(ctx, nil, "", "", rs) + + checkReconcileSuccessful(ctx, t, reconciler, rs, client) + + err := client.Get(ctx, rs.ObjectKey(), rs) + require.NoError(t, err) + + require.Contains(t, rs.Annotations, util.LastAchievedSpec, + "lastAchievedSpec should be written even when vault is disabled") + + // Vault annotations would be simple secret names like "my-secret": "5" + for key := range rs.Annotations { + if key == util.LastAchievedSpec { + continue + } + assert.NotRegexp(t, "^[a-z0-9-]+$", key, + "Should not have simple secret name annotations when vault disabled - found: %s", key) + } +} + func testPVCFinishedResizing(t *testing.T, ctx context.Context, memberClient kubernetesClient.Client, p *corev1.PersistentVolumeClaim, reconciledResource *mdbv1.MongoDB, statefulSet *appsv1.StatefulSet, logger *zap.SugaredLogger) { // Simulate that the PVC has finished resizing setPVCWithUpdatedResource(ctx, t, memberClient, p) From 2b0d2be00448fd28f99eedb09d49420e1a54eccb Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Thu, 30 Oct 2025 10:24:00 +0100 Subject: [PATCH 21/25] lint --- controllers/operator/mongodbreplicaset_controller_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/controllers/operator/mongodbreplicaset_controller_test.go b/controllers/operator/mongodbreplicaset_controller_test.go index 8e8c1dce7..803de4f3b 100644 --- a/controllers/operator/mongodbreplicaset_controller_test.go +++ b/controllers/operator/mongodbreplicaset_controller_test.go @@ -386,7 +386,6 @@ func TestCreateReplicaSet_TLS(t *testing.T) { assert.Equal(t, "OPTIONAL", sslConfig["clientCertificateMode"]) } - // TestCreateDeleteReplicaSet checks that no state is left in OpsManager on removal of the replicaset func TestCreateDeleteReplicaSet(t *testing.T) { ctx := context.Background() From 9ae637a43953eca879acfa5860055f4de683e4a4 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Fri, 31 Oct 2025 11:33:41 +0100 Subject: [PATCH 22/25] Rename variable, remove JSON export --- controllers/operator/mongodbreplicaset_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 003bd1703..92edaa603 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -79,8 +79,8 @@ type ReconcileMongoDbReplicaSet struct { } type ReplicaSetDeploymentState struct { - LastAchievedSpec *mdbv1.MongoDbSpec `json:"lastAchievedSpec"` - LastReconcileMemberCount int `json:"memberCountBefore"` + LastAchievedSpec *mdbv1.MongoDbSpec + LastReconcileMemberCount int } var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} @@ -122,11 +122,11 @@ func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, er // Read current member count from Status once at initialization. This provides a stable view throughout // reconciliation and prepares for eventually storing this in ConfigMap state instead of ephemeral status. - memberCountBefore := r.resource.Status.Members + lastReconcileMemberCount := r.resource.Status.Members return &ReplicaSetDeploymentState{ LastAchievedSpec: lastAchievedSpec, - LastReconcileMemberCount: memberCountBefore, + LastReconcileMemberCount: lastReconcileMemberCount, }, nil } From 370e67b7ba0b95cea4198ca60916bc1b318b6515 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Fri, 31 Oct 2025 11:40:07 +0100 Subject: [PATCH 23/25] PR feedback --- .../operator/mongodbreplicaset_controller.go | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 92edaa603..04a641034 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -134,13 +134,19 @@ func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, er // This should only be called on successful reconciliation. // The state is currently split between the annotations and the member count in status. Both should be migrated // to config maps -func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { +// To avoid posting twice to the API server, we optionally include the vault annotations here, even though they are not +// considered as proper state. The reconciler does not rely on them, they are write-only +func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context, vaultAnnotations map[string]string) error { // Get lastAchievedSpec annotation annotationsToAdd, err := getAnnotationsForResource(r.resource) if err != nil { return err } + for k, val := range vaultAnnotations { + annotationsToAdd[k] = val + } + // Write to CR if err := annotations.SetAnnotations(ctx, r.resource, annotationsToAdd, r.reconciler.client); err != nil { return err @@ -150,9 +156,8 @@ func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context) error { return nil } -// writeVaultAnnotations writes vault secret version annotations to the CR. -// This should only be called on successful reconciliation. -func (r *ReplicaSetReconcilerHelper) writeVaultAnnotations(ctx context.Context) error { +// getVaultAnnotations gets vault secret version annotations to write to the CR. +func (r *ReplicaSetReconcilerHelper) getVaultAnnotations() map[string]string { if !vault.IsVaultSecretBackend() { return nil } @@ -170,12 +175,7 @@ func (r *ReplicaSetReconcilerHelper) writeVaultAnnotations(ctx context.Context) r.resource.Namespace, r.resource.Spec.Credentials) vaultMap = merge.StringToStringMap(vaultMap, r.reconciler.VaultClient.GetSecretAnnotation(path)) - if err := annotations.SetAnnotations(ctx, r.resource, vaultMap, r.reconciler.client); err != nil { - return err - } - - r.log.Debugf("Successfully wrote vault annotations for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) - return nil + return vaultMap } func (r *ReplicaSetReconcilerHelper) initialize(ctx context.Context) error { @@ -330,14 +330,10 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R } // Write state and vault annotations on successful reconciliation - if err := r.writeState(ctx); err != nil { + if err := r.writeState(ctx, r.getVaultAnnotations()); err != nil { return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to write state: %w", err))) } - if err := r.writeVaultAnnotations(ctx); err != nil { - return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to write vault annotations: %w", err))) - } - log.Infof("Finished reconciliation for MongoDbReplicaSet! %s", completionMessage(conn.BaseURL(), conn.GroupID())) return r.updateStatus(ctx, workflow.OK(), mdbstatus.NewBaseUrlOption(deployment.Link(conn.BaseURL(), conn.GroupID())), mdbstatus.MembersOption(rs), mdbstatus.NewPVCsStatusOptionEmptyStatus()) } From 23add7f55702a7f464681ab1107e02ee5e3efff9 Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Fri, 31 Oct 2025 11:54:12 +0100 Subject: [PATCH 24/25] Don't export deployment state struct --- controllers/operator/mongodbreplicaset_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 04a641034..1f3297817 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -78,7 +78,7 @@ type ReconcileMongoDbReplicaSet struct { databaseNonStaticImageVersion string } -type ReplicaSetDeploymentState struct { +type replicaSetDeploymentState struct { LastAchievedSpec *mdbv1.MongoDbSpec LastReconcileMemberCount int } @@ -89,7 +89,7 @@ var _ reconcile.Reconciler = &ReconcileMongoDbReplicaSet{} // This object is NOT shared between reconcile invocations. type ReplicaSetReconcilerHelper struct { resource *mdbv1.MongoDB - deploymentState *ReplicaSetDeploymentState + deploymentState *replicaSetDeploymentState reconciler *ReconcileMongoDbReplicaSet log *zap.SugaredLogger } @@ -113,7 +113,7 @@ func (r *ReconcileMongoDbReplicaSet) newReconcilerHelper( } // readState abstract reading the state of the resource that we store on the cluster between reconciliations. -func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, error) { +func (r *ReplicaSetReconcilerHelper) readState() (*replicaSetDeploymentState, error) { // Try to get the last achieved spec from annotations and store it in state lastAchievedSpec, err := r.resource.GetLastSpec() if err != nil { @@ -124,7 +124,7 @@ func (r *ReplicaSetReconcilerHelper) readState() (*ReplicaSetDeploymentState, er // reconciliation and prepares for eventually storing this in ConfigMap state instead of ephemeral status. lastReconcileMemberCount := r.resource.Status.Members - return &ReplicaSetDeploymentState{ + return &replicaSetDeploymentState{ LastAchievedSpec: lastAchievedSpec, LastReconcileMemberCount: lastReconcileMemberCount, }, nil From ac3d00f3acb2c9d8061459d7200afee8eb902aef Mon Sep 17 00:00:00 2001 From: Julien Benhaim Date: Fri, 31 Oct 2025 13:35:08 +0100 Subject: [PATCH 25/25] Remove writeState method for now --- .../operator/mongodbreplicaset_controller.go | 44 +++++++------------ 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/controllers/operator/mongodbreplicaset_controller.go b/controllers/operator/mongodbreplicaset_controller.go index 1f3297817..4e959ac47 100644 --- a/controllers/operator/mongodbreplicaset_controller.go +++ b/controllers/operator/mongodbreplicaset_controller.go @@ -130,32 +130,6 @@ func (r *ReplicaSetReconcilerHelper) readState() (*replicaSetDeploymentState, er }, nil } -// writeState writes the lastAchievedSpec annotation to the resource. -// This should only be called on successful reconciliation. -// The state is currently split between the annotations and the member count in status. Both should be migrated -// to config maps -// To avoid posting twice to the API server, we optionally include the vault annotations here, even though they are not -// considered as proper state. The reconciler does not rely on them, they are write-only -func (r *ReplicaSetReconcilerHelper) writeState(ctx context.Context, vaultAnnotations map[string]string) error { - // Get lastAchievedSpec annotation - annotationsToAdd, err := getAnnotationsForResource(r.resource) - if err != nil { - return err - } - - for k, val := range vaultAnnotations { - annotationsToAdd[k] = val - } - - // Write to CR - if err := annotations.SetAnnotations(ctx, r.resource, annotationsToAdd, r.reconciler.client); err != nil { - return err - } - - r.log.Debugf("Successfully wrote deployment state for ReplicaSet %s/%s", r.resource.Namespace, r.resource.Name) - return nil -} - // getVaultAnnotations gets vault secret version annotations to write to the CR. func (r *ReplicaSetReconcilerHelper) getVaultAnnotations() map[string]string { if !vault.IsVaultSecretBackend() { @@ -329,9 +303,21 @@ func (r *ReplicaSetReconcilerHelper) Reconcile(ctx context.Context) (reconcile.R return r.updateStatus(ctx, workflow.Pending("Continuing scaling operation for ReplicaSet %s, desiredMembers=%d, currentMembers=%d", rs.ObjectKey(), rs.DesiredReplicas(), scale.ReplicasThisReconciliation(rs)), mdbstatus.MembersOption(rs)) } - // Write state and vault annotations on successful reconciliation - if err := r.writeState(ctx, r.getVaultAnnotations()); err != nil { - return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("failed to write state: %w", err))) + // Get lastspec, vault annotations when needed and write them to the resource. + // These operations should only be performed on successful reconciliations. + // The state of replica sets is currently split between the annotations and the member count in status. Both should + // be migrated to config maps + annotationsToAdd, err := getAnnotationsForResource(r.resource) + if err != nil { + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("could not get resource annotations: %w", err))) + } + + for k, val := range r.getVaultAnnotations() { + annotationsToAdd[k] = val + } + + if err := annotations.SetAnnotations(ctx, r.resource, annotationsToAdd, r.reconciler.client); err != nil { + return r.updateStatus(ctx, workflow.Failed(xerrors.Errorf("could not update resource annotations: %w", err))) } log.Infof("Finished reconciliation for MongoDbReplicaSet! %s", completionMessage(conn.BaseURL(), conn.GroupID()))