Skip to content

Move AppDB reconcile logic from the Ops Manager controller into ReconcileAppDbReplicaSet#1387

Open
MaciejKaras wants to merge 1 commit into
masterfrom
appdb-reconciler-helper
Open

Move AppDB reconcile logic from the Ops Manager controller into ReconcileAppDbReplicaSet#1387
MaciejKaras wants to merge 1 commit into
masterfrom
appdb-reconciler-helper

Conversation

@MaciejKaras

Copy link
Copy Markdown
Collaborator

Summary

Moves all AppDB-specific logic out of the Ops Manager controller flow into ReconcileAppDbReplicaSet: the OM controller now interacts with AppDB only through ReconcileAppDB() and BuildAppDBConnectionURL(). This decoupling makes it possible to plug in a different ReconcileAppDbReplicaSet implementation for the External AppDB required by the Ops Manager Backup project.

Additionally, OnDelete was constructing a full-blown AppDB reconciler just to list the healthy member clusters. It now uses the new thin AppDBReconcilerHelper (created via NewReadOnlyAppDBReconcilerHelper), which also skips all state writes during deletion.

Notable changes:

  • ensureResourcesForArchitectureChange moved from the OM controller into ReconcileAppDB (failures now reported on the AppDB status part; runs after shared-resource/watch setup).
  • AppDB connection-string assembly moved behind BuildAppDBConnectionURL.
  • AppDBReconcilerHelper owns the deployment state and member cluster topology; the reconciler holds it as a field.
  • AppDB reconciler construction moved after OM validations; state init happens at construction and is skipped on paths failing earlier.

Proof of Work

Pure refactor — covered by the existing unit test suite (go test ./controllers/operator/..., green) plus new TestNewAppDBReconcilerHelper covering persisting/read-only construction, removed-cluster resurrection, and legacy-state fallback. golangci-lint (pinned v2.1.0) and pre-commit clean.

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you added changelog file?

@MaciejKaras MaciejKaras added the skip-changelog Use this label in Pull Request to not require new changelog entry file label Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

MCK 1.10.0 Release Notes

New Features

  • MongoDBOpsManager, AppDB: The mongodb-agent-monitoring sidecar container has been merged into the main
    mongodb-agent container. Both automation and monitoring now run as a single process in a single container. The
    spec.appDB.monitoringAgent field is now deprecated and has no effect; Monitoring Agent options for the AppDB —
    including log level and rotation — are configured via spec.applicationDatabase.agent and
    spec.applicationDatabase.agent.monitoringAgent, the same fields used by MongoDB resources. Enabling AppDB
    monitoring still results in rolling restarts of AppDB pods.
  • MongoDBSearch: Added support to show cluster level status of Search, Managed LoadBalancer and MetricsForwarder in the MongoDBSearch resource's status.clusters field.

Bug Fixes

  • MongoDBUser: Fixed a bug where the connection string Secret created in the central cluster did not carry a controller owner reference to the MongoDBUser CR. The missing reference prevented Kubernetes garbage collection from cleaning up the Secret when the MongoDBUser was deleted.
  • MongoDBUser: Fixed a bug where the ownership guard on the connection string Secret was always satisfied regardless of the actual owner, allowing the operator to silently overwrite a Secret controlled by a different resource.
  • Fixed a bug where all MongoDBCommunity deployment telemetry rows incorrectly reported IsRunningEnterpriseImage = true. The field was being evaluated against the operator-level enterprise image rather than the image configured in each CR's spec, causing a misleading ~100% enterprise rate for the Community deployment type.
  • IsRunningEnterpriseImage for Community deployments is now derived from the mongod container image override in spec.statefulSet.spec.template.spec.containers, if present. When no override is set, the field correctly defaults to false, reflecting that Community CRs use the community MongoDB server image by default.
  • MongoDBOpsManager, AppDB: Fix for spec.applicationDatabase.agent.monitoringAgent.logRotate field not being
    handled properly. Additionally, added defaults for
    spec.applicationDatabase.agent.monitoringAgent.logRotate.sizeThresholdMB to 1000 and
    spec.applicationDatabase.agent.monitoringAgent.logRotate.timeThresholdHrs to 24, which are same defaults Ops Manager
    would set.
  • MongoDBSearch: The default JVM heap size (half of the memory request) is now capped at 30GB, following the mongot sizing guidance. Heap sizes above ~30GB prevent the JVM from using compressed object pointers and degrade performance. User-provided heap flags are not affected, if more than 30GB heap is required, we recommend using jvmFlags.

@MaciejKaras
MaciejKaras force-pushed the appdb-reconciler-helper branch from 8fd365d to 645c846 Compare July 16, 2026 11:01
@MaciejKaras
MaciejKaras marked this pull request as ready for review July 16, 2026 13:22
@MaciejKaras
MaciejKaras requested a review from a team as a code owner July 16, 2026 13:22
}

// BuildAppDBConnectionURL returns the connection string to the AppDB, ensuring the Ops Manager user password exists.
func (r *ReconcileAppDbReplicaSet) BuildAppDBConnectionURL(ctx context.Context, opsManager *omv1.MongoDBOpsManager, log *zap.SugaredLogger) (string, error) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new BuildAppDBConnectionURL that returns the connection url to ops manager reconciler, so it can construct secret in each cluster where OM is deployed. Connection URL is the single interface between OpsManager and AppDB

return r.updateStatus(ctx, opsManager, workflow.Invalid("%s", err.Error()), log, mdbstatus.NewOMPartOption(part))
}

acClient := appDbReconciler.getMemberCluster(appDbReconciler.getNameOfFirstMemberCluster()).Client

@MaciejKaras MaciejKaras Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this whole section was moved to beginning of appDbReconciler.ReconcileAppDB() method


if opsManager.Spec.AppDB.IsMultiCluster() {
for _, memberCluster := range appDbReconciler.GetHealthyMemberClusters() {
appDbHelper, err := NewReadOnlyAppDBReconcilerHelper(ctx, opsManager, r.ReconcileCommonController, r.memberClustersMap, log)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we use only read only helper to get the healthy appDB clusters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use this label in Pull Request to not require new changelog entry file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant