CLOUDP-359062 - add ownerrefs and only deleteAll in multicluster (#620)

# Summary

fixes: #589 

This pull request introduces important improvements to how Kubernetes
resources are managed and cleaned up for MongoDB and Ops Manager
deployments, especially in multi-cluster versus single-cluster
scenarios. The main focus is on correctly setting owner references for
resources to enable automatic garbage collection by Kubernetes, and
ensuring explicit resource deletion only occurs when necessary (i.e., in
multi-cluster setups where owner references cannot span clusters).

**Owner Reference Management & Resource Cleanup**

* Added missing owner references (`ownerReferences`) to all relevant
ConfigMaps and resources for MongoDB and Ops Manager, ensuring proper
automatic cleanup by Kubernetes in single-cluster deployments.
* Refactored constructors and initialization logic across controllers
(`appdbreplicaset_controller.go`, `mongodbopsmanager_controller.go`,
`mongodbshardedcluster_controller.go`, `state_store.go`) to consistently
pass and set `ownerReferences`.

**Multi-Cluster vs Single-Cluster Resource Deletion Logic**

* Updated deletion logic in controllers so that explicit resource
deletion is only performed in multi-cluster deployments; single-cluster
setups now rely solely on owner references for cleanup.

## Proof of Work

- unit tests
- relying on kubernetes GC working properly

## Checklist

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

Author: nammn

api/v1/mdb/mongodb_types.go

@@ -171,6 +171,13 @@ func (m *MongoDB) GetOwnerLabels() map[string]string {
 	}
 }
 
+// GetKind returns the Kind of the MongoDB resource. This is needed because
+// when objects are retrieved from the Kubernetes API, the TypeMeta
+// (which contains Kind and APIVersion) is not populated.
+func (m *MongoDB) GetKind() string {
+	return "MongoDB"
+}
+
 // GetSecretsMountedIntoDBPod returns a list of all the optional secret names that are used by this resource.
 func (m *MongoDB) GetSecretsMountedIntoDBPod() []string {
 	secrets := []string{}

api/v1/mdbmulti/mongodb_multi_types.go

@@ -180,6 +180,13 @@ func (m *MongoDBMultiCluster) GetOwnerLabels() map[string]string {
 	}
 }
 
+// GetKind returns the Kind of the MongoDBMultiCluster resource. This is needed because
+// when objects are retrieved from the Kubernetes API, the TypeMeta
+// (which contains Kind and APIVersion) is not populated.
+func (m *MongoDBMultiCluster) GetKind() string {
+	return "MongoDBMultiCluster"
+}
+
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 type MongoDBMultiClusterList struct {
 	metav1.TypeMeta `json:",inline"`

api/v1/om/opsmanager_types.go

@@ -349,6 +349,13 @@ func (om *MongoDBOpsManager) GetOwnerLabels() map[string]string {
 	}
 }
 
+// GetKind returns the Kind of the MongoDBOpsManager resource. This is needed because
+// when objects are retrieved from the Kubernetes API, the TypeMeta
+// (which contains Kind and APIVersion) is not populated.
+func (om *MongoDBOpsManager) GetKind() string {
+	return "MongoDBOpsManager"
+}
+
 // MongoDBOpsManagerServiceDefinition struct that defines the mechanism by which this Ops Manager resource
 // is exposed, via a Service, to the outside of the Kubernetes Cluster.
 type MongoDBOpsManagerServiceDefinition struct {

api/v1/owner.go

@@ -14,4 +14,8 @@ type ResourceOwner interface {
 type ObjectOwner interface {
 	ResourceOwner
 	client.Object
+	// GetKind returns the Kind of the resource. This is needed because
+	// when objects are retrieved from the Kubernetes API, the TypeMeta
+	// (which contains Kind and APIVersion) is not populated.
+	GetKind() string
 }

changelog/20251202_fix_add_missing_ownerrefs.md

@@ -0,0 +1,7 @@
+---
+kind: fix
+date: 2025-12-02
+---
+
+* **MongoDB** Adding missing ownerrefs to ensure proper resource deletion by kubernetes.
+* **Single Cluster** Deleting resources created by CRD now only happens on multi-cluster deployments. Single Cluster will solely rely on ownerrefs.

controllers/operator/appdbreplicaset_controller.go

@@ -20,6 +20,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apiErrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	mdbv1 "github.com/mongodb/mongodb-kubernetes/api/v1/mdb"
 	omv1 "github.com/mongodb/mongodb-kubernetes/api/v1/om"
@@ -121,15 +122,18 @@ type ReconcileAppDbReplicaSet struct {
 
 	imageUrls        images.ImageUrls
 	initAppdbVersion string
+
+	ownerReferences []metav1.OwnerReference
 }
 
-func NewAppDBReplicaSetReconciler(ctx context.Context, imageUrls images.ImageUrls, initAppdbVersion string, appDBSpec omv1.AppDBSpec, commonController *ReconcileCommonController, omConnectionFactory om.ConnectionFactory, omAnnotations map[string]string, globalMemberClustersMap map[string]client.Client, log *zap.SugaredLogger) (*ReconcileAppDbReplicaSet, error) {
+func NewAppDBReplicaSetReconciler(ctx context.Context, imageUrls images.ImageUrls, initAppdbVersion string, appDBSpec omv1.AppDBSpec, commonController *ReconcileCommonController, omConnectionFactory om.ConnectionFactory, omAnnotations map[string]string, globalMemberClustersMap map[string]client.Client, log *zap.SugaredLogger, ownerReferences []metav1.OwnerReference) (*ReconcileAppDbReplicaSet, error) {
 	reconciler := &ReconcileAppDbReplicaSet{
 		ReconcileCommonController: commonController,
 		omConnectionFactory:       omConnectionFactory,
 		centralClient:             commonController.client,
 		imageUrls:                 imageUrls,
 		initAppdbVersion:          initAppdbVersion,
+		ownerReferences:           ownerReferences,
 	}
 
 	if err := reconciler.initializeStateStore(ctx, appDBSpec, omAnnotations, log); err != nil {
@@ -148,7 +152,7 @@ func NewAppDBReplicaSetReconciler(ctx context.Context, imageUrls images.ImageUrl
 func (r *ReconcileAppDbReplicaSet) initializeStateStore(ctx context.Context, appDBSpec omv1.AppDBSpec, omAnnotations map[string]string, log *zap.SugaredLogger) error {
 	r.deploymentState = NewAppDBDeploymentState()
 
-	r.stateStore = NewStateStore[AppDBDeploymentState](&appDBSpec, r.centralClient)
+	r.stateStore = NewStateStore[AppDBDeploymentState](&appDBSpec, r.ownerReferences, r.centralClient)
 	if state, err := r.stateStore.ReadState(ctx); err != nil {
 		if apiErrors.IsNotFound(err) {
 			// If the deployment state config map is missing, then it might be either:
@@ -286,6 +290,7 @@ func (r *ReconcileAppDbReplicaSet) writeLegacyStateConfigMaps(ctx context.Contex
 	mappingConfigMap := configmap.Builder().
 		SetName(spec.ClusterMappingConfigMapName()).
 		SetLabels(spec.GetOwnerLabels()).
+		SetOwnerReferences(r.ownerReferences).
 		SetNamespace(spec.Namespace).
 		SetData(mappingConfigMapData).
 		Build()
@@ -302,6 +307,7 @@ func (r *ReconcileAppDbReplicaSet) writeLegacyStateConfigMaps(ctx context.Contex
 	specConfigMap := configmap.Builder().
 		SetName(spec.LastAppliedMemberSpecConfigMapName()).
 		SetLabels(spec.GetOwnerLabels()).
+		SetOwnerReferences(r.ownerReferences).
 		SetNamespace(spec.Namespace).
 		SetData(specConfigMapData).
 		Build()
@@ -1732,6 +1738,7 @@ func (r *ReconcileAppDbReplicaSet) ensureProjectIDConfigMapForCluster(ctx contex
 	cm := configmap.Builder().
 		SetName(opsManager.Spec.AppDB.ProjectIDConfigMapName()).
 		SetLabels(opsManager.GetOwnerLabels()).
+		SetOwnerReferences(r.ownerReferences).
 		SetNamespace(opsManager.Namespace).
 		SetDataField(util.AppDbProjectIdKey, projectID).
 		Build()
@@ -1792,6 +1799,7 @@ func (r *ReconcileAppDbReplicaSet) publishACVersionAsConfigMap(ctx context.Conte
 
 	acVersionConfigMap := configmap.Builder().
 		SetLabels(labels).
+		SetOwnerReferences(r.ownerReferences).
 		SetNamespace(opsManager.Namespace).
 		SetName(cmName).
 		SetDataField(appDBACConfigMapVersionField, fmt.Sprintf("%d", version)).

controllers/operator/appdbreplicaset_controller_test.go

@@ -1352,13 +1352,13 @@ func checkDeploymentEqualToPublished(t *testing.T, expected automationconfig.Aut
 
 func newAppDbReconciler(ctx context.Context, c client.Client, opsManager *omv1.MongoDBOpsManager, omConnectionFactoryFunc om.ConnectionFactory, log *zap.SugaredLogger) (*ReconcileAppDbReplicaSet, error) {
 	commonController := NewReconcileCommonController(ctx, c)
-	return NewAppDBReplicaSetReconciler(ctx, nil, "", opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, nil, zap.S())
+	return NewAppDBReplicaSetReconciler(ctx, nil, "", opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, nil, log, kube.BaseOwnerReference(opsManager))
 }
 
 func newAppDbMultiReconciler(ctx context.Context, c client.Client, opsManager *omv1.MongoDBOpsManager, memberClusterMap map[string]client.Client, log *zap.SugaredLogger, omConnectionFactoryFunc om.ConnectionFactory) (*ReconcileAppDbReplicaSet, error) {
 	_ = c.Update(ctx, opsManager)
 	commonController := NewReconcileCommonController(ctx, c)
-	return NewAppDBReplicaSetReconciler(ctx, nil, "", opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, memberClusterMap, log)
+	return NewAppDBReplicaSetReconciler(ctx, nil, "", opsManager.Spec.AppDB, commonController, omConnectionFactoryFunc, opsManager.Annotations, memberClusterMap, log, kube.BaseOwnerReference(opsManager))
 }
 
 func TestChangingFCVAppDB(t *testing.T) {

controllers/operator/create/create_test.go

@@ -48,7 +48,7 @@ func TestBuildService(t *testing.T) {
 
 	assert.Len(t, svc.OwnerReferences, 1)
 	assert.Equal(t, mdb.Name, svc.OwnerReferences[0].Name)
-	assert.Equal(t, mdb.GetObjectKind().GroupVersionKind().Kind, svc.OwnerReferences[0].Kind)
+	assert.Equal(t, "MongoDB", svc.OwnerReferences[0].Kind)
 	assert.Equal(t, mock.TestNamespace, svc.Namespace)
 	assert.Equal(t, "my-svc", svc.Name)
 	assert.Equal(t, "loadbalancerip", svc.Spec.LoadBalancerIP)
@@ -67,7 +67,7 @@ func TestBuildService(t *testing.T) {
 
 	assert.Len(t, svc.OwnerReferences, 1)
 	assert.Equal(t, mdb.Name, svc.OwnerReferences[0].Name)
-	assert.Equal(t, mdb.GetObjectKind().GroupVersionKind().Kind, svc.OwnerReferences[0].Kind)
+	assert.Equal(t, "MongoDB", svc.OwnerReferences[0].Kind)
 	assert.Equal(t, mock.TestNamespace, svc.Namespace)
 	assert.Equal(t, "my-svc", svc.Name)
 	assert.Equal(t, "loadbalancerip", svc.Spec.LoadBalancerIP)
@@ -201,6 +201,7 @@ func TestOpsManagerInKubernetes_ClusterSpecificExternalConnectivity(t *testing.T
 						OwnerReferences: []metav1.OwnerReference{
 							{
 								APIVersion:         "mongodb.com/v1",
+								Kind:               "MongoDBOpsManager",
 								Name:               "test-om",
 								Controller:         ptr.To(true),
 								BlockOwnerDeletion: ptr.To(true),
@@ -242,6 +243,7 @@ func TestOpsManagerInKubernetes_ClusterSpecificExternalConnectivity(t *testing.T
 						OwnerReferences: []metav1.OwnerReference{
 							{
 								APIVersion:         "mongodb.com/v1",
+								Kind:               "MongoDBOpsManager",
 								Name:               "test-om",
 								Controller:         ptr.To(true),
 								BlockOwnerDeletion: ptr.To(true),
@@ -318,6 +320,7 @@ func TestOpsManagerInKubernetes_ClusterSpecificExternalConnectivity(t *testing.T
 						OwnerReferences: []metav1.OwnerReference{
 							{
 								APIVersion:         "mongodb.com/v1",
+								Kind:               "MongoDBOpsManager",
 								Name:               "test-om",
 								Controller:         ptr.To(true),
 								BlockOwnerDeletion: ptr.To(true),
@@ -359,6 +362,7 @@ func TestOpsManagerInKubernetes_ClusterSpecificExternalConnectivity(t *testing.T
 						OwnerReferences: []metav1.OwnerReference{
 							{
 								APIVersion:         "mongodb.com/v1",
+								Kind:               "MongoDBOpsManager",
 								Name:               "test-om",
 								Controller:         ptr.To(true),
 								BlockOwnerDeletion: ptr.To(true),
@@ -403,6 +407,7 @@ func TestOpsManagerInKubernetes_ClusterSpecificExternalConnectivity(t *testing.T
 						OwnerReferences: []metav1.OwnerReference{
 							{
 								APIVersion:         "mongodb.com/v1",
+								Kind:               "MongoDBOpsManager",
 								Name:               "test-om",
 								Controller:         ptr.To(true),
 								BlockOwnerDeletion: ptr.To(true),

controllers/operator/mongodbopsmanager_controller.go

@@ -197,7 +197,7 @@ func NewOpsManagerReconcilerHelper(ctx context.Context, opsManagerReconciler *Op
 func (r *OpsManagerReconcilerHelper) initializeStateStore(ctx context.Context, reconciler *OpsManagerReconciler, opsManager *omv1.MongoDBOpsManager, globalMemberClustersMap map[string]client.Client, log *zap.SugaredLogger, clusterNamesFromClusterSpecList []string) error {
 	r.deploymentState = NewOMDeploymentState()
 
-	r.stateStore = NewStateStore[OMDeploymentState](opsManager, reconciler.client)
+	r.stateStore = NewStateStore[OMDeploymentState](opsManager, kube.BaseOwnerReference(opsManager), reconciler.client)
 	if err := r.stateStore.read(ctx); err != nil {
 		if apiErrors.IsNotFound(err) {
 			// If the deployment state config map is missing, then it might be either:
@@ -253,6 +253,7 @@ func (r *OpsManagerReconcilerHelper) writeLegacyStateConfigMap(ctx context.Conte
 	mappingConfigMap := configmap.Builder().
 		SetName(spec.ClusterMappingConfigMapName()).
 		SetLabels(spec.GetOwnerLabels()).
+		SetOwnerReferences(kube.BaseOwnerReference(r.opsManager)).
 		SetNamespace(spec.Namespace).
 		SetData(mappingConfigMapData).
 		Build()
@@ -2128,27 +2129,31 @@ func (r *OpsManagerReconciler) OnDelete(ctx context.Context, obj interface{}, lo
 		return
 	}
 
-	// delete the OpsManager resources from each of the member cluster. We need to delete the
-	// resource explicitly in case of multi-cluster because we can't set owner reference cross cluster
-	for _, memberCluster := range helper.getHealthyMemberClusters() {
-		if err := r.deleteClusterResources(ctx, memberCluster.Client, memberCluster.Name, opsManager, log); err != nil {
-			log.Warnf("Failed to delete dependant OpsManager resources in cluster %s: %s", memberCluster.Name, err)
+	// Delete resources explicitly only in multi-cluster mode where we can't set owner references cross cluster.
+	// In single-cluster deployments, OwnerReferences handle cleanup automatically via Kubernetes garbage collection.
+	if opsManager.Spec.IsMultiCluster() {
+		for _, memberCluster := range helper.getHealthyMemberClusters() {
+			if err := r.deleteClusterResources(ctx, memberCluster.Client, memberCluster.Name, opsManager, log); err != nil {
+				log.Warnf("Failed to delete dependant OpsManager resources in cluster %s: %s", memberCluster.Name, err)
+			}
 		}
 	}
 
-	// delete the AppDB resources from each of the member cluster. We need to delete the
-	// resource explicitly in case of multi-cluster because we can't set owner reference cross cluster
-	for _, memberCluster := range appDbReconciler.GetHealthyMemberClusters() {
-		if err := r.deleteClusterResources(ctx, memberCluster.Client, memberCluster.Name, opsManager, log); err != nil {
-			log.Warnf("Failed to delete dependant AppDB resources in cluster %s: %s", memberCluster.Name, err.Error())
+	if opsManager.Spec.AppDB.IsMultiCluster() {
+		for _, memberCluster := range appDbReconciler.GetHealthyMemberClusters() {
+			if err := r.deleteClusterResources(ctx, memberCluster.Client, memberCluster.Name, opsManager, log); err != nil {
+				log.Warnf("Failed to delete dependant AppDB resources in cluster %s: %s", memberCluster.Name, err.Error())
+			}
 		}
 	}
 
+	r.resourceWatcher.RemoveDependentWatchedResources(opsManager.ObjectKey())
+
 	log.Info("Cleaned up Ops Manager related resources.")
 }
 
 func (r *OpsManagerReconciler) createNewAppDBReconciler(ctx context.Context, opsManager *omv1.MongoDBOpsManager, log *zap.SugaredLogger) (*ReconcileAppDbReplicaSet, error) {
-	return NewAppDBReplicaSetReconciler(ctx, r.imageUrls, r.initAppdbVersion, opsManager.Spec.AppDB, r.ReconcileCommonController, r.omConnectionFactory, opsManager.Annotations, r.memberClustersMap, log)
+	return NewAppDBReplicaSetReconciler(ctx, r.imageUrls, r.initAppdbVersion, opsManager.Spec.AppDB, r.ReconcileCommonController, r.omConnectionFactory, opsManager.Annotations, r.memberClustersMap, log, kube.BaseOwnerReference(opsManager))
 }
 
 // getAnnotationsForOpsManagerResource returns all the annotations that should be applied to the resource

controllers/operator/mongodbopsmanager_controller_test.go

@@ -219,6 +219,87 @@ func TestOpsManagerReconciler_removeWatchedResources(t *testing.T) {
 	assert.Zero(t, len(reconciler.resourceWatcher.GetWatchedResources()))
 }
 
+// TestOpsManagerReconciler_OnDeleteClusterResourceCleanup verifies the DeleteAllOf behavior:
+// - In single-cluster mode: DeleteAllOf should NOT be called (OwnerReferences handle cleanup)
+// - In multi-cluster mode: DeleteAllOf SHOULD be called (can't use OwnerReferences cross-cluster)
+func TestOpsManagerReconciler_OnDeleteClusterResourceCleanup(t *testing.T) {
+	multiClusterSpecItems := []omv1.ClusterSpecOMItem{
+		{ClusterName: "cluster-a", Members: 1},
+		{ClusterName: "cluster-b", Members: 1},
+	}
+
+	testCases := []struct {
+		name             string
+		isMultiCluster   bool
+		clusterSpecItems []omv1.ClusterSpecOMItem
+	}{
+		{
+			name:             "SingleCluster_SkipsDeleteAllOf",
+			isMultiCluster:   false,
+			clusterSpecItems: nil,
+		},
+		{
+			name:             "MultiCluster_CallsDeleteAllOf",
+			isMultiCluster:   true,
+			clusterSpecItems: multiClusterSpecItems,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			deleteAllOfCallCount := 0
+			omConnectionFactory := om.NewDefaultCachedOMConnectionFactory()
+
+			omBuilder := DefaultOpsManagerBuilder()
+			var memberClustersMap map[string]client.Client
+
+			// Create fake client with interceptor that tracks DeleteAllOf calls
+			deleteAllOfInterceptor := func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.DeleteAllOfOption) error {
+				deleteAllOfCallCount++
+				return nil
+			}
+
+			if tc.isMultiCluster {
+				omBuilder.SetOpsManagerTopology(omv1.ClusterTopologyMultiCluster).
+					SetOpsManagerClusterSpecList(tc.clusterSpecItems)
+				memberClustersMap = make(map[string]client.Client)
+				for _, clusterSpec := range tc.clusterSpecItems {
+					memberFakeClientBuilder := mock.NewEmptyFakeClientBuilder()
+					memberFakeClientBuilder.WithInterceptorFuncs(interceptor.Funcs{
+						Get:         mock.GetFakeClientInterceptorGetFunc(omConnectionFactory, true, true),
+						DeleteAllOf: deleteAllOfInterceptor,
+					})
+					memberClustersMap[clusterSpec.ClusterName] = memberFakeClientBuilder.Build()
+				}
+			}
+
+			testOm := omBuilder.Build()
+			assert.Equal(t, tc.isMultiCluster, testOm.Spec.IsMultiCluster(), "topology mismatch")
+
+			fakeClientBuilder := mock.NewEmptyFakeClientBuilder()
+			fakeClientBuilder.WithObjects(testOm.DeepCopy())
+			fakeClientBuilder.WithInterceptorFuncs(interceptor.Funcs{
+				Get:         mock.GetFakeClientInterceptorGetFunc(omConnectionFactory, true, true),
+				DeleteAllOf: deleteAllOfInterceptor,
+			})
+			kubeClient := kubernetesClient.NewClient(fakeClientBuilder.Build())
+
+			reconciler := NewOpsManagerReconciler(ctx, kubeClient, memberClustersMap, images.ImageUrls{}, "", "", omConnectionFactory.GetConnectionFunc, &MockedInitializer{expectedOmURL: testOm.CentralURL(), t: t}, func(baseUrl string, user string, publicApiKey string, ca *string) api.OpsManagerAdmin {
+				return api.NewMockedAdminProvider(baseUrl, user, publicApiKey, true).(*api.MockedOmAdmin)
+			})
+
+			reconciler.OnDelete(ctx, testOm, zap.S())
+
+			if tc.isMultiCluster {
+				assert.Greater(t, deleteAllOfCallCount, 0, "DeleteAllOf should be called in multi-cluster mode")
+			} else {
+				assert.Equal(t, 0, deleteAllOfCallCount, "DeleteAllOf should not be called in single-cluster mode; cleanup relies on OwnerReferences")
+			}
+		})
+	}
+}
+
 func TestOpsManagerReconciler_prepareOpsManager(t *testing.T) {
 	ctx := context.Background()
 	testOm := DefaultOpsManagerBuilder().Build()