fix status update and cm predicate (stolostron#1664)

* fix staus update and cm predicate Signed-off-by: Thibault Mange <[email protected]> * fix invalid update branching Signed-off-by: Thibault Mange <[email protected]> --------- Signed-off-by: Thibault Mange <[email protected]> Signed-off-by: Christian Stark <[email protected]>
ch-stark · Jan 10, 2025 · 8182dc4 · 8182dc4
1 parent 094ade2
commit 8182dc4
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 10 deletions.
diff --git a/operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go b/operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go
@@ -557,7 +557,7 @@ func (r *ObservabilityAddonReconciler) SetupWithManager(mgr ctrl.Manager) error
 		Watches(
 			&corev1.ConfigMap{},
 			&handler.EnqueueRequestForObject{},
-			builder.WithPredicates(getPred(clusterMonitoringConfigName, promNamespace, true, true, true)),
+			builder.WithPredicates(configMapDataChangedPredicate(clusterMonitoringConfigName, promNamespace)),
 		).
 		Watches(
 			&appsv1.Deployment{},

diff --git a/operators/endpointmetrics/controllers/observabilityendpoint/predicate_func.go b/operators/endpointmetrics/controllers/observabilityendpoint/predicate_func.go
@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	v1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
@@ -74,3 +75,29 @@ func getPred(name string, namespace string,
 		DeleteFunc: deleteFunc,
 	}
 }
+
+func configMapDataChangedPredicate(name, namespace string) predicate.Funcs {
+	return predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			oldCM, okOld := e.ObjectOld.(*corev1.ConfigMap)
+			newCM, okNew := e.ObjectNew.(*corev1.ConfigMap)
+			if !okOld || !okNew {
+				return false
+			}
+
+			if newCM.Name != name || newCM.Namespace != namespace {
+				return false
+			}
+
+			return !reflect.DeepEqual(oldCM.Data, newCM.Data)
+		},
+		CreateFunc: func(e event.CreateEvent) bool {
+			cm, ok := e.Object.(*corev1.ConfigMap)
+			return ok && cm.Name == name && cm.Namespace == namespace
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			cm, ok := e.Object.(*corev1.ConfigMap)
+			return ok && cm.Name == name && cm.Namespace == namespace
+		},
+	}
+}
diff --git a/operators/endpointmetrics/pkg/collector/metrics_collector.go b/operators/endpointmetrics/pkg/collector/metrics_collector.go
@@ -73,13 +73,15 @@ type ClusterInfo struct {
 }
 
 type MetricsCollector struct {
-	Client             client.Client
-	ClusterInfo        ClusterInfo
-	HubInfo            *operatorconfig.HubInfo
-	Log                logr.Logger
-	Namespace          string
-	ObsAddon           *oav1beta1.ObservabilityAddon
-	ServiceAccountName string
+	Client                          client.Client
+	ClusterInfo                     ClusterInfo
+	HubInfo                         *operatorconfig.HubInfo
+	Log                             logr.Logger
+	Namespace                       string
+	ObsAddon                        *oav1beta1.ObservabilityAddon
+	ServiceAccountName              string
+	platformCollectorWasUpdated     bool
+	userWorkloadCollectorWasUpdated bool
 }
 
 type proxyConfig struct {
@@ -111,7 +113,9 @@ func (m *MetricsCollector) Update(ctx context.Context, req ctrl.Request) error {
 		return err
 	} else {
 		if m.ObsAddon.Spec.EnableMetrics {
-			m.reportStatus(ctx, status.MetricsCollector, status.UpdateSuccessful, "Metrics collector updated")
+			if m.platformCollectorWasUpdated {
+				m.reportStatus(ctx, status.MetricsCollector, status.UpdateSuccessful, "Metrics collector updated")
+			}
 		} else {
 			m.reportStatus(ctx, status.MetricsCollector, status.Disabled, "Metrics collector disabled")
 		}
@@ -130,7 +134,9 @@ func (m *MetricsCollector) Update(ctx context.Context, req ctrl.Request) error {
 			return err
 		} else {
 			if m.ObsAddon.Spec.EnableMetrics {
-				m.reportStatus(ctx, status.UwlMetricsCollector, status.UpdateSuccessful, "UWL Metrics collector updated")
+				if m.userWorkloadCollectorWasUpdated {
+					m.reportStatus(ctx, status.UwlMetricsCollector, status.UpdateSuccessful, "UWL Metrics collector updated")
+				}
 			} else {
 				m.reportStatus(ctx, status.UwlMetricsCollector, status.Disabled, "UWL Metrics collector disabled")
 			}
@@ -744,6 +750,14 @@ func (m *MetricsCollector) ensureDeployment(ctx context.Context, isUWL bool, dep
 		desiredMetricsCollectorDep.Spec.Template.Spec.Containers[0].Resources = *m.ObsAddon.Spec.Resources
 	}
 
+	wasUpdated := func() {
+		if isUWL {
+			m.userWorkloadCollectorWasUpdated = true
+		} else {
+			m.platformCollectorWasUpdated = true
+		}
+	}
+
 	retryErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 		foundMetricsCollectorDep := &appsv1.Deployment{}
 		err := m.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: m.Namespace}, foundMetricsCollectorDep)
@@ -753,6 +767,7 @@ func (m *MetricsCollector) ensureDeployment(ctx context.Context, isUWL bool, dep
 				return fmt.Errorf("failed to create Deployment %s/%s: %w", m.Namespace, name, err)
 			}
 
+			wasUpdated()
 			return nil
 		}
 		if err != nil {
@@ -773,6 +788,7 @@ func (m *MetricsCollector) ensureDeployment(ctx context.Context, isUWL bool, dep
 				return fmt.Errorf("failed to update Deployment %s/%s: %w", m.Namespace, name, err)
 			}
 
+			wasUpdated()
 			return nil
 		}