[release-2.11] Avoid Concurrent writes to ManagedClusterList

operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go

@@ -92,25 +92,6 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	reqLogger := log.WithValues("Request.Namespace", req.Namespace, "Request.Name", req.Name)
 	reqLogger.Info("Reconciling PlacementRule")
 
-	// ACM 8509: Special case for hub/local cluster metrics collection
-	// We want to ensure that the local-cluster is always in the managedClusterList
-	// In the case when hubSelfManagement is enabled, we will delete it from the list and modify the object
-	// to cater to the use case of deploying in open-cluster-management-observability namespace
-	delete(managedClusterList, "local-cluster")
-	if _, ok := managedClusterList["local-cluster"]; !ok {
-		obj := &clusterv1.ManagedCluster{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "local-cluster",
-				Namespace: config.GetDefaultNamespace(),
-				Labels: map[string]string{
-					"openshiftVersion": "mimical",
-				},
-			},
-		}
-		installMetricsWithoutAddon = true
-		updateManagedClusterList(obj)
-	}
-
 	if config.GetMonitoringCRName() == "" {
 		reqLogger.Info("multicluster observability resource is not available")
 		return ctrl.Result{}, nil
@@ -139,6 +120,13 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, nil
 	}
 
+	// ACM 8509: Special case for hub/local cluster metrics collection
+	// We want to ensure that the local-cluster is always in the managedClusterList
+	// In the case when hubSelfManagement is enabled, we will delete it from the list and modify the object
+	// to cater to the use case of deploying in open-cluster-management-observability namespace
+	if req.Name == "local-cluster" {
+		installMetricsWithoutAddon = true
+	}
 	if !deleteAll && !mco.Spec.ObservabilityAddonSpec.EnableMetrics {
 		reqLogger.Info("EnableMetrics is set to false. Delete Observability addons")
 		deleteAll = true
@@ -179,24 +167,12 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 
 	if !deleteAll && installMetricsWithoutAddon {
-		obsAddonList.Items = append(obsAddonList.Items, mcov1beta1.ObservabilityAddon{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      obsAddonName,
-				Namespace: config.GetDefaultNamespace(),
-				Labels: map[string]string{
-					ownerLabelKey: ownerLabelValue,
-				},
-			},
-		})
 		err = deleteObsAddon(r.Client, localClusterName)
 		if err != nil {
 			log.Error(err, "Failed to delete observabilityaddon")
 			return ctrl.Result{}, err
 		}
 	}
-	if operatorconfig.IsMCOTerminating {
-		delete(managedClusterList, "local-cluster")
-	}
 
 	if !deleteAll {
 		if err := createAllRelatedRes(
@@ -238,6 +214,7 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		latestClusters = append(latestClusters, addon.Namespace)
 		staleAddons = append(staleAddons, addon.Namespace)
 	}
+
 	for _, work := range workList.Items {
 		if work.Name != work.Namespace+workNameSuffix {
 			// ACM 8509: Special case for hub metrics collector
@@ -293,7 +270,7 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	if deleteAll {
 		// delete managedclusteraddon for local-cluster
-		err = deleteManagedClusterRes(r.Client, localClusterName)
+		err = deleteManagedClusterRes(r.Client, config.GetDefaultNamespace())
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -450,12 +427,10 @@ func createAllRelatedRes(
 
 	failedDeleteOba := false
 	for _, cluster := range clustersToCleanup {
-		if cluster != config.GetDefaultNamespace() {
-			err = deleteObsAddon(c, cluster)
-			if err != nil {
-				failedDeleteOba = true
-				log.Error(err, "Failed to delete observabilityaddon", "namespace", cluster)
-			}
+		err = deleteObsAddon(c, cluster)
+		if err != nil {
+			failedDeleteOba = true
+			log.Error(err, "Failed to delete observabilityaddon", "namespace", cluster)
 		}
 	}
 
@@ -614,6 +589,12 @@ func areManagedClusterLabelsReady(obj client.Object) bool {
 func updateManagedClusterList(obj client.Object) {
 	managedClusterListMutex.Lock()
 	defer managedClusterListMutex.Unlock()
+	//ACM 8509: Special case for local-cluster, we deploy endpoint and metrics collector in the hub
+	//whether hubSelfManagement is enabled or not
+	if obj.GetName() == localClusterName {
+		managedClusterList[obj.GetName()] = "mimical"
+		return
+	}
 	if version, ok := obj.GetLabels()["openshiftVersion"]; ok {
 		managedClusterList[obj.GetName()] = version
 	} else {

operators/multiclusterobservability/controllers/placementrule/predicate_func.go

@@ -24,13 +24,6 @@ func getClusterPreds() predicate.Funcs {
 	createFunc := func(e event.CreateEvent) bool {
 		log.Info("CreateFunc", "managedCluster", e.Object.GetName())
 
-		//ACM 8509: Special case for local-cluster, we do not react changes to
-		//local-cluster as it is expected to be always present in the managed cluster list
-		//whether hubSelfManagement is enabled or not
-		if e.Object.GetName() == "local-cluster" {
-			return false
-		}
-
 		if isAutomaticAddonInstallationDisabled(e.Object) {
 			return false
 		}
@@ -46,10 +39,6 @@ func getClusterPreds() predicate.Funcs {
 	updateFunc := func(e event.UpdateEvent) bool {
 		log.Info("UpdateFunc", "managedCluster", e.ObjectNew.GetName())
 
-		if e.ObjectNew.GetName() == "local-cluster" {
-			return false
-		}
-
 		if e.ObjectNew.GetResourceVersion() == e.ObjectOld.GetResourceVersion() {
 			return false
 		}
@@ -79,10 +68,6 @@ func getClusterPreds() predicate.Funcs {
 	deleteFunc := func(e event.DeleteEvent) bool {
 		log.Info("DeleteFunc", "managedCluster", e.Object.GetName())
 
-		if e.Object.GetName() == "local-cluster" {
-			return false
-		}
-
 		if isAutomaticAddonInstallationDisabled(e.Object) {
 			return false
 		}