stop cmo update if not changed (#1625)

Signed-off-by: Thibault Mange <[email protected]>
stolostron · Sep 13, 2024 · dc5f925 · dc5f925
1 parent 54cdb1c
commit dc5f925
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 12 deletions.
diff --git a/operators/endpointmetrics/controllers/observabilityendpoint/ocp_monitoring_config.go b/operators/endpointmetrics/controllers/observabilityendpoint/ocp_monitoring_config.go
@@ -11,6 +11,7 @@ import (
 	"reflect"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -382,42 +383,51 @@ func createOrUpdateClusterMonitoringConfig(
 		return fmt.Errorf("failed to unmarshal the cluster monitoring config: %w", err)
 	}
 
-	if foundClusterMonitoringConfiguration.PrometheusK8sConfig != nil {
+	newCMOCfg := &cmomanifests.ClusterMonitoringConfiguration{}
+	if err := yaml.Unmarshal([]byte(foundClusterMonitoringConfigurationYAMLString), newCMOCfg); err != nil {
+		return fmt.Errorf("failed to unmarshal the cluster monitoring config: %w", err)
+	}
+
+	if newCMOCfg.PrometheusK8sConfig != nil {
 
 		// check if externalLabels exists
-		if foundClusterMonitoringConfiguration.PrometheusK8sConfig.ExternalLabels != nil {
-			foundClusterMonitoringConfiguration.PrometheusK8sConfig.ExternalLabels[operatorconfig.ClusterLabelKeyForAlerts] = clusterID
+		if newCMOCfg.PrometheusK8sConfig.ExternalLabels != nil {
+			newCMOCfg.PrometheusK8sConfig.ExternalLabels[operatorconfig.ClusterLabelKeyForAlerts] = clusterID
 		} else {
-			foundClusterMonitoringConfiguration.PrometheusK8sConfig.ExternalLabels = newExternalLabels
+			newCMOCfg.PrometheusK8sConfig.ExternalLabels = newExternalLabels
 		}
 
 		// check if alertmanagerConfigs exists
-		if foundClusterMonitoringConfiguration.PrometheusK8sConfig.AlertmanagerConfigs != nil {
+		if newCMOCfg.PrometheusK8sConfig.AlertmanagerConfigs != nil {
 			additionalAlertmanagerConfigExists := false
 			var atIndex int
-			for i, v := range foundClusterMonitoringConfiguration.PrometheusK8sConfig.AlertmanagerConfigs {
+			for i, v := range newCMOCfg.PrometheusK8sConfig.AlertmanagerConfigs {
 				if isManaged(v) {
 					additionalAlertmanagerConfigExists = true
 					atIndex = i
 					break
 				}
 			}
 			if !additionalAlertmanagerConfigExists {
-				foundClusterMonitoringConfiguration.PrometheusK8sConfig.AlertmanagerConfigs = append(
-					foundClusterMonitoringConfiguration.PrometheusK8sConfig.AlertmanagerConfigs,
+				newCMOCfg.PrometheusK8sConfig.AlertmanagerConfigs = append(
+					newCMOCfg.PrometheusK8sConfig.AlertmanagerConfigs,
 					newAdditionalAlertmanagerConfig(hubInfo))
 			} else {
-				foundClusterMonitoringConfiguration.PrometheusK8sConfig.AlertmanagerConfigs[atIndex] = newAdditionalAlertmanagerConfig(hubInfo)
+				newCMOCfg.PrometheusK8sConfig.AlertmanagerConfigs[atIndex] = newAdditionalAlertmanagerConfig(hubInfo)
 			}
 		} else {
-			foundClusterMonitoringConfiguration.PrometheusK8sConfig.AlertmanagerConfigs = newAlertmanagerConfigs
+			newCMOCfg.PrometheusK8sConfig.AlertmanagerConfigs = newAlertmanagerConfigs
 		}
 
 	} else {
-		foundClusterMonitoringConfiguration.PrometheusK8sConfig = newPmK8sConfig
+		newCMOCfg.PrometheusK8sConfig = newPmK8sConfig
 	}
 
-	updatedClusterMonitoringConfigurationYAMLBytes, err := yaml.Marshal(foundClusterMonitoringConfiguration)
+	if equality.Semantic.DeepEqual(*foundClusterMonitoringConfiguration, *newCMOCfg) {
+		return nil
+	}
+
+	updatedClusterMonitoringConfigurationYAMLBytes, err := yaml.Marshal(newCMOCfg)
 	if err != nil {
 		return fmt.Errorf("failed to marshal the cluster monitoring config to yaml: %w", err)
 	}

diff --git a/operators/endpointmetrics/controllers/observabilityendpoint/ocp_monitoring_config_test.go b/operators/endpointmetrics/controllers/observabilityendpoint/ocp_monitoring_config_test.go
@@ -152,6 +152,44 @@ prometheusK8s:
 	}
 }
 
+// When the cluster-monitoring-config is unchanged, no need to update it
+func TestClusterMonitoringConfigUnchanged(t *testing.T) {
+	testNamespace := "test-ns"
+	amAccessSrt := newAMAccessorSecret(testNamespace, "test-token")
+	hubInfo := &operatorconfig.HubInfo{
+		ClusterName:              "test-cluster",
+		ObservatoriumAPIEndpoint: "http://test-endpoint",
+		AlertmanagerEndpoint:     "http://test-alertamanger-endpoint",
+	}
+	cmoCfg := newClusterMonitoringConfigCM(clusterMonitoringConfigDataYaml, endpointMonitoringOperatorMgr)
+	client := fake.NewClientBuilder().WithRuntimeObjects(newHubInfoSecret([]byte(hubInfoYAML), testNamespace), cmoCfg, amAccessSrt).Build()
+	err := createOrUpdateClusterMonitoringConfig(context.Background(), hubInfo, testClusterID, client, false, testNamespace)
+	if err != nil {
+		t.Fatalf("Failed to create or update the cluster-monitoring-config configmap: (%v)", err)
+	}
+
+	cmoCfgBeforeUpdate := &corev1.ConfigMap{}
+	err = client.Get(context.Background(), types.NamespacedName{Name: clusterMonitoringConfigName, Namespace: promNamespace}, cmoCfgBeforeUpdate)
+	if err != nil {
+		t.Fatalf("failed to check configmap %s: %v", clusterMonitoringConfigName, err)
+	}
+
+	err = createOrUpdateClusterMonitoringConfig(context.Background(), hubInfo, testClusterID, client, false, testNamespace)
+	if err != nil {
+		t.Fatalf("Failed to create or update the cluster-monitoring-config configmap: (%v)", err)
+	}
+
+	cmoCfgAfterUpdate := &corev1.ConfigMap{}
+	err = client.Get(context.Background(), types.NamespacedName{Name: clusterMonitoringConfigName, Namespace: promNamespace}, cmoCfgAfterUpdate)
+	if err != nil {
+		t.Fatalf("failed to check configmap %s: %v", clusterMonitoringConfigName, err)
+	}
+
+	if cmoCfgBeforeUpdate.ResourceVersion != cmoCfgAfterUpdate.ResourceVersion {
+		t.Fatalf("The cluster-monitoring-config configmap should not be updated")
+	}
+}
+
 func TestClusterMonitoringConfigAlertsDisabled(t *testing.T) {
 	testNamespace := "test-ns"
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true})))