From 2dd28b7c8eaada043823bc87a69b719a1925c306 Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Tue, 7 Jan 2025 19:21:51 +0100 Subject: [PATCH] GenerationChangedPredicate prevented Cache Sync to Trigger Reconciler --- .../controller/bundledeployment_controller.go | 3 ++ .../gitops/reconciler/gitjob_controller.go | 3 +- .../controller/gitops/reconciler/predicate.go | 51 +++++++++++++++++++ .../helmops/reconciler/helmapp_controller.go | 3 ++ .../reconciler/config_controller.go | 3 ++ .../reconciler/imagescan_controller.go | 3 ++ 6 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 internal/cmd/controller/gitops/reconciler/predicate.go diff --git a/internal/cmd/agent/controller/bundledeployment_controller.go b/internal/cmd/agent/controller/bundledeployment_controller.go index f659f4349f..a2d1762ad9 100644 --- a/internal/cmd/agent/controller/bundledeployment_controller.go +++ b/internal/cmd/agent/controller/bundledeployment_controller.go @@ -59,6 +59,9 @@ func (r *BundleDeploymentReconciler) SetupWithManager(mgr ctrl.Manager) error { WithEventFilter( // we do not trigger for status changes predicate.Or( + // Note: These predicates prevent cache + // syncPeriod from triggering reconcile, since + // cache sync is an Update event. predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.LabelChangedPredicate{}, diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go index 4365d084a1..41d2457e84 100644 --- a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go +++ b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go @@ -103,8 +103,9 @@ func (r *GitJobReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.GitRepo{}, builder.WithPredicates( - // do not trigger for GitRepo status changes (except for commit changes) + // do not trigger for GitRepo status changes (except for commit changes and cache sync) predicate.Or( + TypedResourceVersionUnchangedPredicate[client.Object]{}, predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.LabelChangedPredicate{}, diff --git a/internal/cmd/controller/gitops/reconciler/predicate.go b/internal/cmd/controller/gitops/reconciler/predicate.go new file mode 100644 index 0000000000..864648c965 --- /dev/null +++ b/internal/cmd/controller/gitops/reconciler/predicate.go @@ -0,0 +1,51 @@ +package reconciler + +import ( + "reflect" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +// TypedResourceVersionUnchangedPredicate implements a update predicate to +// allow syncPeriod to trigger the reconciler +type TypedResourceVersionUnchangedPredicate[T metav1.Object] struct { + predicate.TypedFuncs[T] +} + +func isNil(arg any) bool { + if v := reflect.ValueOf(arg); !v.IsValid() || ((v.Kind() == reflect.Ptr || + v.Kind() == reflect.Interface || + v.Kind() == reflect.Slice || + v.Kind() == reflect.Map || + v.Kind() == reflect.Chan || + v.Kind() == reflect.Func) && v.IsNil()) { + return true + } + return false +} + +func (TypedResourceVersionUnchangedPredicate[T]) Create(e event.CreateEvent) bool { + return false +} + +func (TypedResourceVersionUnchangedPredicate[T]) Delete(e event.DeleteEvent) bool { + return false +} + +// Update implements default UpdateEvent filter for validating resource version change. +func (TypedResourceVersionUnchangedPredicate[T]) Update(e event.TypedUpdateEvent[T]) bool { + if isNil(e.ObjectOld) { + return false + } + if isNil(e.ObjectNew) { + return false + } + + return e.ObjectNew.GetResourceVersion() == e.ObjectOld.GetResourceVersion() +} + +func (TypedResourceVersionUnchangedPredicate[T]) Generic(e event.GenericEvent) bool { + return false +} diff --git a/internal/cmd/controller/helmops/reconciler/helmapp_controller.go b/internal/cmd/controller/helmops/reconciler/helmapp_controller.go index 39cb86acda..b9cd20b8f7 100644 --- a/internal/cmd/controller/helmops/reconciler/helmapp_controller.go +++ b/internal/cmd/controller/helmops/reconciler/helmapp_controller.go @@ -51,6 +51,9 @@ func (r *HelmAppReconciler) SetupWithManager(mgr ctrl.Manager) error { For(&fleet.HelmApp{}, builder.WithPredicates( predicate.Or( + // Note: These predicates prevent cache + // syncPeriod from triggering reconcile, since + // cache sync is an Update event. predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.LabelChangedPredicate{}, diff --git a/internal/cmd/controller/reconciler/config_controller.go b/internal/cmd/controller/reconciler/config_controller.go index e1efa47e2e..c7a56ef135 100644 --- a/internal/cmd/controller/reconciler/config_controller.go +++ b/internal/cmd/controller/reconciler/config_controller.go @@ -58,6 +58,9 @@ func (r *ConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { object.GetName() == config.ManagerConfigName }), predicate.Or( + // Note: These predicates prevent cache + // syncPeriod from triggering reconcile, since + // cache sync is an Update event. predicate.ResourceVersionChangedPredicate{}, predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, diff --git a/internal/cmd/controller/reconciler/imagescan_controller.go b/internal/cmd/controller/reconciler/imagescan_controller.go index e99ec053ad..535d1ca2b5 100644 --- a/internal/cmd/controller/reconciler/imagescan_controller.go +++ b/internal/cmd/controller/reconciler/imagescan_controller.go @@ -40,6 +40,9 @@ func (r *ImageScanReconciler) SetupWithManager(mgr ctrl.Manager) error { predicate.And( sharding.FilterByShardID(r.ShardID), predicate.Or( + // Note: These predicates prevent cache + // syncPeriod from triggering reconcile, since + // cache sync is an Update event. predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.LabelChangedPredicate{},