From bc928a58385245f022cb90779f9249208a9a8312 Mon Sep 17 00:00:00 2001 From: OpenShift Cherrypick Robot Date: Tue, 17 Dec 2024 18:25:46 +0100 Subject: [PATCH] [release-2.12] [ACM-16335] Reconcile MCO only when there is actual change in mch image manifest (#1728) * reconcile MCO only when there is actual chnage in mch image manifest Signed-off-by: Coleen Iona Quadros * reconcile MCO only when there is actual chnage in mch image manifest Signed-off-by: Coleen Iona Quadros * remove unnecessary log and use semantic equality check Signed-off-by: Coleen Iona Quadros * lint Signed-off-by: Coleen Iona Quadros --------- Signed-off-by: Coleen Iona Quadros Co-authored-by: Coleen Iona Quadros --- .../predicate_func.go | 24 +++++++++++++++---- .../placementrule_controller_test.go | 2 +- .../controllers/placementrule/predict_func.go | 21 ++++++++++++---- .../pkg/config/config.go | 24 +++++++++++++++---- .../pkg/config/config_test.go | 2 +- .../pkg/servicemonitor/sm_controller.go | 5 ++-- 6 files changed, 59 insertions(+), 19 deletions(-) diff --git a/operators/multiclusterobservability/controllers/multiclusterobservability/predicate_func.go b/operators/multiclusterobservability/controllers/multiclusterobservability/predicate_func.go index 2b85e5ef0..08d8e5a90 100644 --- 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/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go b/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go index 6bba39945..2bdf6135a 100644 --- a/operators/multiclusterobservability/controllers/placementrule/placementrule_controller_test.go +++ b/operators/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 index 13d0a0a6a..2e3e934ce 100644 --- 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 index c518497ce..f4dc9ced6 100644 --- 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 index 9952b5694..50b24dd98 100644 --- 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 index 62b29109e..adfc4fdd2 100644 --- 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) } }