From 1634dca7c8a212874f8c02f724952ee42914910c Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Wed, 26 Jun 2024 12:53:03 +0200 Subject: [PATCH 1/7] synchronisation Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller.go | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index 65739db1f..d6df940d9 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -96,19 +96,23 @@ 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 - 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", + if req.Name == "local-cluster" { + managedClusterListMutex.Lock() + delete(managedClusterList, "local-cluster") + managedClusterListMutex.Unlock() + 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) } - installMetricsWithoutAddon = true - updateManagedClusterList(obj) } if config.GetMonitoringCRName() == "" { From 934ae619185295026ece39c71cb8b0709757c3b3 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Thu, 27 Jun 2024 11:10:03 +0200 Subject: [PATCH 2/7] refactor Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller.go | 34 ++++++++++--------- .../placementrule/predicate_func.go | 16 ++++----- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index d6df940d9..f2e243aa7 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -97,22 +97,20 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques // 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" { - managedClusterListMutex.Lock() - delete(managedClusterList, "local-cluster") - managedClusterListMutex.Unlock() - 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 _, ok := managedClusterList["local-cluster"]; !ok { + // obj := &clusterv1.ManagedCluster{ + // ObjectMeta: metav1.ObjectMeta{ + // Name: "local-cluster", + // Namespace: config.GetDefaultNamespace(), + // Labels: map[string]string{ + // "openshiftVersion": "mimical", + // }, + // }, + // } + // + // updateManagedClusterList(obj) + //} + installMetricsWithoutAddon = true } if config.GetMonitoringCRName() == "" { @@ -618,6 +616,10 @@ func areManagedClusterLabelsReady(obj client.Object) bool { func updateManagedClusterList(obj client.Object) { managedClusterListMutex.Lock() defer managedClusterListMutex.Unlock() + if obj.GetName() == localClusterName { + managedClusterList[obj.GetName()] = "mimical" + return + } if version, ok := obj.GetLabels()["openshiftVersion"]; ok { managedClusterList[obj.GetName()] = version } else { diff --git a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go b/operators/multiclusterobservability/controllers/placementrule/predicate_func.go index 626cea672..44c42eab6 100644 --- a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go +++ b/operators/multiclusterobservability/controllers/placementrule/predicate_func.go @@ -27,9 +27,9 @@ func getClusterPreds() predicate.Funcs { //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 e.Object.GetName() == "local-cluster" { + // return true + //} if isAutomaticAddonInstallationDisabled(e.Object) { return false @@ -46,9 +46,9 @@ 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.GetName() == "local-cluster" { + // return true + //} if e.ObjectNew.GetResourceVersion() == e.ObjectOld.GetResourceVersion() { return false @@ -79,10 +79,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 } From 23421475f1e69ca34928b29b300556e6585842b3 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Thu, 27 Jun 2024 17:10:41 +0200 Subject: [PATCH 3/7] refactor Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller.go | 70 +++++++++---------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index f2e243aa7..81c6d92da 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -92,27 +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 - if req.Name == "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", - // }, - // }, - // } - // - // updateManagedClusterList(obj) - //} - installMetricsWithoutAddon = true - } - if config.GetMonitoringCRName() == "" { reqLogger.Info("multicluster observability resource is not available") return ctrl.Result{}, nil @@ -141,6 +120,27 @@ 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" { + //if _, ok := managedClusterList["local-cluster"]; !ok { + // obj := &clusterv1.ManagedCluster{ + // ObjectMeta: metav1.ObjectMeta{ + // Name: "local-cluster", + // Namespace: config.GetDefaultNamespace(), + // Labels: map[string]string{ + // "openshiftVersion": "mimical", + // }, + // }, + // } + // + // updateManagedClusterList(obj) + //} + reqLogger.Info("Coleen request for local cluster managed cluster") + installMetricsWithoutAddon = true + } if !deleteAll && !mco.Spec.ObservabilityAddonSpec.EnableMetrics { reqLogger.Info("EnableMetrics is set to false. Delete Observability addons") deleteAll = true @@ -181,15 +181,15 @@ 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, - }, - }, - }) + //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") @@ -452,12 +452,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) } } From 16a7f65ae15c6da5d7d4357ed36b5291d4d91321 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Thu, 27 Jun 2024 17:18:59 +0200 Subject: [PATCH 4/7] refactor Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller.go | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index 81c6d92da..c56518029 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -196,9 +196,9 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, err } } - if operatorconfig.IsMCOTerminating { - delete(managedClusterList, "local-cluster") - } + //if operatorconfig.IsMCOTerminating { + // delete(managedClusterList, "local-cluster") + //} if !deleteAll { if err := createAllRelatedRes( @@ -241,7 +241,7 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques staleAddons = append(staleAddons, addon.Namespace) } for _, work := range workList.Items { - if work.Name != work.Namespace+workNameSuffix { + if work.Name != work.Namespace+workNameSuffix || work.Namespace == "local-cluster" { // ACM 8509: Special case for hub metrics collector // In the upgrade case we want to clean up the obs add on and manifest work that was created // for local-cluster before the upgrade that is why we check for the local-cluster namespace @@ -250,15 +250,16 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { return ctrl.Result{}, err } - } - if !slices.Contains(latestClusters, work.Namespace) { - reqLogger.Info("To delete manifestwork", "namespace", work.Namespace) - err = deleteManagedClusterRes(r.Client, work.Namespace) - if err != nil { - return ctrl.Result{}, err - } } else { - staleAddons = commonutil.Remove(staleAddons, work.Namespace) + if !slices.Contains(latestClusters, work.Namespace) { + reqLogger.Info("To delete manifestwork", "namespace", work.Namespace) + err = deleteManagedClusterRes(r.Client, work.Namespace) + if err != nil { + return ctrl.Result{}, err + } + } else { + staleAddons = commonutil.Remove(staleAddons, work.Namespace) + } } } From ca9dccd48496cb8215cb0285bfb2ee2c6c63db6e Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Thu, 27 Jun 2024 17:45:14 +0200 Subject: [PATCH 5/7] refactor Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index c56518029..601739e4a 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -240,8 +240,9 @@ 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 || work.Namespace == "local-cluster" { + if work.Name != work.Namespace+workNameSuffix { // ACM 8509: Special case for hub metrics collector // In the upgrade case we want to clean up the obs add on and manifest work that was created // for local-cluster before the upgrade that is why we check for the local-cluster namespace @@ -250,16 +251,15 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { return ctrl.Result{}, err } - } else { - if !slices.Contains(latestClusters, work.Namespace) { - reqLogger.Info("To delete manifestwork", "namespace", work.Namespace) - err = deleteManagedClusterRes(r.Client, work.Namespace) - if err != nil { - return ctrl.Result{}, err - } - } else { - staleAddons = commonutil.Remove(staleAddons, work.Namespace) + } + if !slices.Contains(latestClusters, work.Namespace) { + reqLogger.Info("To delete manifestwork", "namespace", work.Namespace) + err = deleteManagedClusterRes(r.Client, work.Namespace) + if err != nil { + return ctrl.Result{}, err } + } else { + staleAddons = commonutil.Remove(staleAddons, work.Namespace) } } @@ -296,7 +296,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 } From 6ce4fd0b838594c95f023269a283c65330e56792 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Tue, 13 Aug 2024 10:33:57 +0200 Subject: [PATCH 6/7] cleanup Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller.go | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index 601739e4a..00cee7a85 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -125,19 +125,6 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques // 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" { - //if _, ok := managedClusterList["local-cluster"]; !ok { - // obj := &clusterv1.ManagedCluster{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "local-cluster", - // Namespace: config.GetDefaultNamespace(), - // Labels: map[string]string{ - // "openshiftVersion": "mimical", - // }, - // }, - // } - // - // updateManagedClusterList(obj) - //} reqLogger.Info("Coleen request for local cluster managed cluster") installMetricsWithoutAddon = true } @@ -181,24 +168,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( From a1109af424010872aa5d7b14a44633f8ca1fe833 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Thu, 15 Aug 2024 12:59:19 +0200 Subject: [PATCH 7/7] update Signed-off-by: Coleen Iona Quadros --- .../placementrule/placementrule_controller.go | 3 ++- .../controllers/placementrule/predicate_func.go | 11 ----------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go index 00cee7a85..60702da9c 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go +++ b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller.go @@ -125,7 +125,6 @@ func (r *PlacementRuleReconciler) Reconcile(ctx context.Context, req ctrl.Reques // 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" { - reqLogger.Info("Coleen request for local cluster managed cluster") installMetricsWithoutAddon = true } if !deleteAll && !mco.Spec.ObservabilityAddonSpec.EnableMetrics { @@ -590,6 +589,8 @@ 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 diff --git a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go b/operators/multiclusterobservability/controllers/placementrule/predicate_func.go index 44c42eab6..8e8d5a1a3 100644 --- a/operators/multiclusterobservability/controllers/placementrule/predicate_func.go +++ b/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 true - //} - 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 true - //} - if e.ObjectNew.GetResourceVersion() == e.ObjectOld.GetResourceVersion() { return false }