Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-2.11] [ACM-16335] Reconcile MCO only when there is actual change in mch image manifest #1731

Open
wants to merge 1 commit into
base: release-2.11
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -139,7 +141,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,
)
Expand All @@ -151,20 +153,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
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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
},
Expand Down
24 changes: 19 additions & 5 deletions operators/multiclusterobservability/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"net/url"
"os"
"strings"
"sync"
"time"

ocinfrav1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -250,6 +251,7 @@ var (
}

multicloudConsoleRouteHost = ""
imageManifestCache sync.Map
)

// GetCrLabelKey returns the key for the CR label injected into the resources created by the operator.
Expand All @@ -267,7 +269,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{
Expand All @@ -282,17 +284,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...
Expand Down Expand Up @@ -819,3 +820,16 @@ func IsAlertingDisabledInSpec(mco *observabilityv1beta2.MultiClusterObservabilit
annotations := mco.GetAnnotations()
return annotations != nil && annotations[AnnotationDisableMCOAlerting] == "true"
}

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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading