diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index 088c1dfd1..93eebc5bd 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -61,8 +61,7 @@ var ( clusterAddon = &addonv1alpha1.ClusterManagementAddOn{} defaultAddonDeploymentConfig = &addonv1alpha1.AddOnDeploymentConfig{} isplacementControllerRunnning = false - managedClusterList = map[string]string{} - managedClusterListMutex = &sync.RWMutex{} + managedClusterList = sync.Map{} installMetricsWithoutAddon = false ) @@ -110,7 +109,7 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { if k8serrors.IsNotFound(err) { deleteAll = true - delete(managedClusterList, "local-cluster") + managedClusterList.Delete("local-cluster") } else { // Error reading the object - requeue the request. return ctrl.Result{}, err @@ -127,9 +126,21 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques // 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" { + managedClusterList.Delete("local-cluster") + if _, ok := managedClusterList.Load("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 !deleteAll && !mco.Spec.ObservabilityAddonSpec.EnableMetrics { reqLogger.Info("EnableMetrics is set to false. Delete Observability addons") deleteAll = true @@ -375,8 +386,9 @@ func createAllRelatedRes( } failedCreateManagedClusterRes := false - managedClusterListMutex.RLock() - for managedCluster, openshiftVersion := range managedClusterList { + managedClusterList.Range(func(key, value interface{}) bool { + managedCluster := key.(string) + openshiftVersion := value.(string) if isReconcileRequired(request, managedCluster) { log.Info( "Monitoring operator should be installed in cluster", @@ -417,22 +429,21 @@ func createAllRelatedRes( log.Error(err, "Failed to create managedcluster resources", "namespace", managedCluster) } if request.Namespace == managedCluster { - break + return false } } - } + return true + }) // Look through the obsAddonList items and find clusters // which are no longer to be managed and therefore needs deletion clustersToCleanup := []string{} for _, ep := range obsAddonList.Items { - if _, ok := managedClusterList[ep.Namespace]; !ok { + if _, ok := managedClusterList.Load(ep.Namespace); !ok { clustersToCleanup = append(clustersToCleanup, ep.Namespace) } } - managedClusterListMutex.RUnlock() - failedDeleteOba := false for _, cluster := range clustersToCleanup { err = deleteObsAddon(c, cluster) @@ -595,18 +606,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 + managedClusterList.Store(obj.GetName(), version) } else { - managedClusterList[obj.GetName()] = nonOCP + managedClusterList.Store(obj.GetName(), nonOCP) } } diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go index 80207e4a7..6bba39945 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go @@ -205,10 +205,9 @@ func TestObservabilityAddonController(t *testing.T) { }, } - managedClusterList = map[string]string{ - namespace: "4", - namespace2: "4", - } + managedClusterList.Store(namespace, "4") + managedClusterList.Store(namespace2, "4") + _, err := r.Reconcile(context.TODO(), req) if err != nil { t.Fatalf("reconcile: (%v)", err) @@ -228,7 +227,11 @@ func TestObservabilityAddonController(t *testing.T) { t.Fatalf("Deprecated role not removed") } - managedClusterList = map[string]string{namespace: "4"} + managedClusterList.Range(func(key, value interface{}) bool { + managedClusterList.Delete(key) + return true + }) + managedClusterList.Store(namespace, "4") _, err = r.Reconcile(context.TODO(), req) if err != nil { t.Fatalf("reconcile: (%v)", err) diff --git a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go b/operators/multiclusterobservability/controllers/placementrule/predicate_func.go index 8e8d5a1a3..61027c878 100644 --- a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go +++ b/operators/multiclusterobservability/controllers/placementrule/predicate_func.go @@ -9,6 +9,8 @@ import ( "reflect" "strings" + clusterv1 "open-cluster-management.io/api/cluster/v1" + "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config" operatorconfig "github.com/stolostron/multicluster-observability-operator/operators/pkg/config" @@ -31,7 +33,9 @@ func getClusterPreds() predicate.Funcs { if !areManagedClusterLabelsReady(e.Object) { return false } - updateManagedClusterList(e.Object) + if e.Object.GetName() != localClusterName { + updateManagedClusterList(e.Object) + } return true } @@ -48,9 +52,7 @@ func getClusterPreds() predicate.Funcs { if e.ObjectNew.GetDeletionTimestamp() != nil { log.Info("managedcluster is in terminating state", "managedCluster", e.ObjectNew.GetName()) - managedClusterListMutex.Lock() - delete(managedClusterList, e.ObjectNew.GetName()) - managedClusterListMutex.Unlock() + managedClusterList.Delete(e.ObjectNew.GetName()) managedClusterImageRegistryMutex.Lock() delete(managedClusterImageRegistry, e.ObjectNew.GetName()) managedClusterImageRegistryMutex.Unlock() @@ -59,7 +61,15 @@ func getClusterPreds() predicate.Funcs { if !areManagedClusterLabelsReady(e.ObjectNew) { return false } - updateManagedClusterList(e.ObjectNew) + if e.ObjectNew.GetName() != localClusterName { + updateManagedClusterList(e.ObjectNew) + } + + } + //log the diff in managedccluster object + if !reflect.DeepEqual(e.ObjectNew.(*clusterv1.ManagedCluster), e.ObjectOld.(*clusterv1.ManagedCluster)) { + log.Info("managedcluster object New diff", "managedCluster", e.ObjectNew.GetName(), "diff", fmt.Sprintf("%+v", e.ObjectNew.(*clusterv1.ManagedCluster))) + log.Info("managedcluster object Old diff", "managedCluster", e.ObjectOld.GetName(), "diff", fmt.Sprintf("%+v", e.ObjectOld.(*clusterv1.ManagedCluster))) } return true @@ -72,9 +82,9 @@ func getClusterPreds() predicate.Funcs { return false } - managedClusterListMutex.Lock() - delete(managedClusterList, e.Object.GetName()) - managedClusterListMutex.Unlock() + if e.Object.GetName() != localClusterName { + managedClusterList.Delete(e.Object.GetName()) + } managedClusterImageRegistryMutex.Lock() delete(managedClusterImageRegistry, e.Object.GetName()) managedClusterImageRegistryMutex.Unlock() diff --git a/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go b/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go index 9be504e58..e8160d8dc 100644 --- a/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go +++ b/operators/multiclusterobservability/controllers/placementrule/predicate_func_test.go @@ -8,9 +8,10 @@ import ( "testing" "time" + clusterv1 "open-cluster-management.io/api/cluster/v1" + addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" - appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/event" ) @@ -66,15 +67,16 @@ func TestClusterPred(t *testing.T) { t.Run(c.caseName, func(t *testing.T) { pred := getClusterPreds() create_event := event.CreateEvent{ - Object: &appsv1.Deployment{ + Object: &clusterv1.ManagedCluster{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: c.namespace, - Annotations: c.annotations, - Labels: map[string]string{"vendor": "OpenShift", "openshiftVersion": "4.6.0"}, + Name: name, + Namespace: c.namespace, + Annotations: c.annotations, + Labels: map[string]string{"vendor": "OpenShift", "openshiftVersion": "4.6.0"}, + ResourceVersion: "1", }, - Spec: appsv1.DeploymentSpec{ - Replicas: int32Ptr(2), + Spec: clusterv1.ManagedClusterSpec{ + HubAcceptsClient: true, }, }, } @@ -90,7 +92,7 @@ func TestClusterPred(t *testing.T) { } update_event := event.UpdateEvent{ - ObjectNew: &appsv1.Deployment{ + ObjectNew: &clusterv1.ManagedCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: c.namespace, @@ -100,7 +102,7 @@ func TestClusterPred(t *testing.T) { Labels: map[string]string{"vendor": "OpenShift", "openshiftVersion": "4.6.0"}, }, }, - ObjectOld: &appsv1.Deployment{ + ObjectOld: &clusterv1.ManagedCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: c.namespace, @@ -125,14 +127,14 @@ func TestClusterPred(t *testing.T) { } delete_event := event.DeleteEvent{ - Object: &appsv1.Deployment{ + Object: &clusterv1.ManagedCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: c.namespace, Annotations: c.annotations, }, - Spec: appsv1.DeploymentSpec{ - Replicas: int32Ptr(2), + Spec: clusterv1.ManagedClusterSpec{ + HubAcceptsClient: true, }, }, } @@ -196,19 +198,18 @@ func TestManagedClusterLabelReady(t *testing.T) { t.Run(c.caseName, func(t *testing.T) { pred := getClusterPreds() create_event := event.CreateEvent{ - Object: &appsv1.Deployment{ + Object: &clusterv1.ManagedCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: c.namespace, Annotations: c.annotations, Labels: c.labels, }, - Spec: appsv1.DeploymentSpec{ - Replicas: int32Ptr(2), + Spec: clusterv1.ManagedClusterSpec{ + HubAcceptsClient: true, }, }, } - if c.expectedCreate { if !pred.CreateFunc(create_event) { t.Fatalf("pre func return false on applied createevent in case: (%v)", c.caseName) @@ -218,8 +219,9 @@ func TestManagedClusterLabelReady(t *testing.T) { t.Fatalf("pre func return true on non-applied createevent in case: (%v)", c.caseName) } } + update_event := event.UpdateEvent{ - ObjectNew: &appsv1.Deployment{ + ObjectNew: &clusterv1.ManagedCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: c.namespace, @@ -229,7 +231,8 @@ func TestManagedClusterLabelReady(t *testing.T) { Labels: c.labels, }, }, - ObjectOld: &appsv1.Deployment{ + + ObjectOld: &clusterv1.ManagedCluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: c.namespace, @@ -237,7 +240,6 @@ func TestManagedClusterLabelReady(t *testing.T) { }, }, } - if c.expectedUpdate { if !pred.UpdateFunc(update_event) { t.Fatalf("pre func return false on applied update event in case: (%v)", c.caseName)