[release-2.12] [ACM-16335] Reconcile MCO only when there is actual ch…

…ange in mch image manifest (#1728) * reconcile MCO only when there is actual chnage in mch image manifest Signed-off-by: Coleen Iona Quadros <[email protected]> * reconcile MCO only when there is actual chnage in mch image manifest Signed-off-by: Coleen Iona Quadros <[email protected]> * remove unnecessary log and use semantic equality check Signed-off-by: Coleen Iona Quadros <[email protected]> * lint Signed-off-by: Coleen Iona Quadros <[email protected]> --------- Signed-off-by: Coleen Iona Quadros <[email protected]> Co-authored-by: Coleen Iona Quadros <[email protected]>
stolostron · Dec 17, 2024 · bc928a5 · bc928a5
1 parent 64a43b5
commit bc928a5
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 19 deletions.
diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/predicate_func.go b/operators/multiclusterobservability/controllers/multiclusterobservability/predicate_func.go
@@ -5,6 +5,8 @@
 package multiclusterobservability
 
 import (
+	"reflect"
+
 	mcov1beta2 "github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/api/v1beta2"
 	"github.com/stolostron/multicluster-observability-operator/operators/multiclusterobservability/pkg/config"
 	mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1"
@@ -137,7 +139,7 @@ func GetMCHPredicateFunc(c client.Client) predicate.Funcs {
 				e.Object.(*mchv1.MultiClusterHub).Status.DesiredVersion == e.Object.(*mchv1.MultiClusterHub).Status.CurrentVersion {
 				// only read the image manifests configmap and enqueue the request when the MCH is
 				// installed/upgraded successfully
-				ok, err := config.ReadImageManifestConfigMap(
+				_, ok, err := config.ReadImageManifestConfigMap(
 					c,
 					e.Object.(*mchv1.MultiClusterHub).Status.CurrentVersion,
 				)
@@ -149,20 +151,32 @@ func GetMCHPredicateFunc(c client.Client) predicate.Funcs {
 			return false
 		},
 		UpdateFunc: func(e event.UpdateEvent) bool {
+			// Ensure the event pertains to the target namespace and object type
 			if e.ObjectNew.GetNamespace() == config.GetMCONamespace() &&
+				e.ObjectNew.GetResourceVersion() != e.ObjectOld.GetResourceVersion() &&
 				e.ObjectNew.(*mchv1.MultiClusterHub).Status.CurrentVersion != "" &&
 				e.ObjectNew.(*mchv1.MultiClusterHub).Status.DesiredVersion ==
 					e.ObjectNew.(*mchv1.MultiClusterHub).Status.CurrentVersion {
-				// only read the image manifests configmap and enqueue the request when the MCH is
-				// installed/upgraded successfully
-				ok, err := config.ReadImageManifestConfigMap(
+
+				currentData, _, err := config.ReadImageManifestConfigMap(
 					c,
 					e.ObjectNew.(*mchv1.MultiClusterHub).Status.CurrentVersion,
 				)
 				if err != nil {
+					log.Error(err, "Failed to read image manifest ConfigMap")
 					return false
 				}
-				return ok
+
+				previousData, exists := config.GetCachedImageManifestData()
+				if !exists {
+					config.SetCachedImageManifestData(currentData)
+					return true
+				}
+				if !reflect.DeepEqual(currentData, previousData) {
+					config.SetCachedImageManifestData(currentData)
+					return true
+				}
+				return false
 			}
 			return false
 		},

diff --git a/...tors/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go b/...tors/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go
@@ -522,7 +522,7 @@ func TestObservabilityAddonController(t *testing.T) {
 		},
 	}
 
-	ok, err := config.ReadImageManifestConfigMap(c, testMCHInstance.Status.CurrentVersion)
+	_, ok, err := config.ReadImageManifestConfigMap(c, testMCHInstance.Status.CurrentVersion)
 	if err != nil || !ok {
 		t.Fatalf("Failed to read image manifest configmap: (%T,%v)", ok, err)
 	}

diff --git a/operators/multiclusterobservability/controllers/placementrule/predict_func.go b/operators/multiclusterobservability/controllers/placementrule/predict_func.go
@@ -109,7 +109,7 @@ func getMchPred(c client.Client) predicate.Funcs {
 				e.Object.(*mchv1.MultiClusterHub).Status.DesiredVersion == e.Object.(*mchv1.MultiClusterHub).Status.CurrentVersion {
 				// only read the image manifests configmap and enqueue the request when the MCH is
 				// installed/upgraded successfully
-				ok, err := config.ReadImageManifestConfigMap(
+				_, ok, err := config.ReadImageManifestConfigMap(
 					c,
 					e.Object.(*mchv1.MultiClusterHub).Status.CurrentVersion,
 				)
@@ -121,21 +121,32 @@ func getMchPred(c client.Client) predicate.Funcs {
 			return false
 		},
 		UpdateFunc: func(e event.UpdateEvent) bool {
+			// Ensure the event pertains to the target namespace and object type
 			if e.ObjectNew.GetNamespace() == config.GetMCONamespace() &&
 				e.ObjectNew.GetResourceVersion() != e.ObjectOld.GetResourceVersion() &&
 				e.ObjectNew.(*mchv1.MultiClusterHub).Status.CurrentVersion != "" &&
 				e.ObjectNew.(*mchv1.MultiClusterHub).Status.DesiredVersion ==
 					e.ObjectNew.(*mchv1.MultiClusterHub).Status.CurrentVersion {
-				// / only read the image manifests configmap and enqueue the request when the MCH is
-				// installed/upgraded successfully
-				ok, err := config.ReadImageManifestConfigMap(
+
+				currentData, _, err := config.ReadImageManifestConfigMap(
 					c,
 					e.ObjectNew.(*mchv1.MultiClusterHub).Status.CurrentVersion,
 				)
 				if err != nil {
+					log.Error(err, "Failed to read image manifest ConfigMap")
 					return false
 				}
-				return ok
+
+				previousData, exists := config.GetCachedImageManifestData()
+				if !exists {
+					config.SetCachedImageManifestData(currentData)
+					return true
+				}
+				if !reflect.DeepEqual(currentData, previousData) {
+					config.SetCachedImageManifestData(currentData)
+					return true
+				}
+				return false
 			}
 			return false
 		},

diff --git a/operators/multiclusterobservability/pkg/config/config.go b/operators/multiclusterobservability/pkg/config/config.go
@@ -12,6 +12,7 @@ import (
 	"reflect"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	imagev1client "github.com/openshift/client-go/image/clientset/versioned/typed/image/v1"
@@ -275,6 +276,7 @@ var (
 	}
 
 	multicloudConsoleRouteHost = ""
+	imageManifestCache         sync.Map
 )
 
 // GetCrLabelKey returns the key for the CR label injected into the resources created by the operator.
@@ -292,7 +294,7 @@ func GetImageManifestConfigMapName() string {
 }
 
 // ReadImageManifestConfigMap reads configmap with the label ocm-configmap-type=image-manifest.
-func ReadImageManifestConfigMap(c client.Client, version string) (bool, error) {
+func ReadImageManifestConfigMap(c client.Client, version string) (map[string]string, bool, error) {
 	mcoNamespace := GetMCONamespace()
 	// List image manifest configmap with label ocm-configmap-type=image-manifest and ocm-release-version
 	matchLabels := map[string]string{
@@ -307,17 +309,16 @@ func ReadImageManifestConfigMap(c client.Client, version string) (bool, error) {
 	imageCMList := &corev1.ConfigMapList{}
 	err := c.List(context.TODO(), imageCMList, listOpts...)
 	if err != nil {
-		return false, fmt.Errorf("failed to list mch-image-manifest configmaps: %w", err)
+		return nil, false, fmt.Errorf("failed to list mch-image-manifest configmaps: %w", err)
 	}
 
 	if len(imageCMList.Items) != 1 {
 		// there should be only one matched image manifest configmap found
-		return false, nil
+		return nil, false, nil
 	}
 
 	imageManifests = imageCMList.Items[0].Data
-	log.V(1).Info("the length of mch-image-manifest configmap", "imageManifests", len(imageManifests))
-	return true, nil
+	return imageManifests, true, nil
 }
 
 // GetImageManifests...
@@ -889,3 +890,16 @@ func GetMCOASupportedCRDFQDN(name string) string {
 
 	return fmt.Sprintf("%s.%s.%s", parts[0], version, parts[1])
 }
+
+func SetCachedImageManifestData(data map[string]string) {
+	imageManifestCache.Store("mch-image-manifest", data)
+}
+
+func GetCachedImageManifestData() (map[string]string, bool) {
+	if value, ok := imageManifestCache.Load("mch-image-manifest"); ok {
+		if cachedData, valid := value.(map[string]string); valid {
+			return cachedData, true
+		}
+	}
+	return nil, false
+}
diff --git a/operators/multiclusterobservability/pkg/config/config_test.go b/operators/multiclusterobservability/pkg/config/config_test.go
@@ -568,7 +568,7 @@ func TestReadImageManifestConfigMap(t *testing.T) {
 			}
 			client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjs...).Build()
 
-			gotRet, err := ReadImageManifestConfigMap(client, c.version)
+			_, gotRet, err := ReadImageManifestConfigMap(client, c.version)
 			if err != nil {
 				t.Errorf("Failed read image manifest configmap due to %v", err)
 			}

diff --git a/operators/multiclusterobservability/pkg/servicemonitor/sm_controller.go b/operators/multiclusterobservability/pkg/servicemonitor/sm_controller.go
@@ -7,9 +7,10 @@ package servicemonitor
 import (
 	"context"
 	"os"
-	"reflect"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/equality"
+
 	promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	promclientset "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -79,7 +80,7 @@ func onUpdate(promClient promclientset.Interface) func(oldObj interface{}, newOb
 		newSm := newObj.(*promv1.ServiceMonitor)
 		oldSm := oldObj.(*promv1.ServiceMonitor)
 		if newSm.ObjectMeta.OwnerReferences != nil && newSm.ObjectMeta.OwnerReferences[0].Kind == "Observatorium" &&
-			!reflect.DeepEqual(newSm.Spec, oldSm.Spec) {
+			!equality.Semantic.DeepEqual(newSm.Spec, oldSm.Spec) {
 			updateServiceMonitor(promClient, newSm)
 		}
 	}