diff --git a/internal/controller/promotions/promotions.go b/internal/controller/promotions/promotions.go index bfaf35d835..d6a97eba6a 100644 --- a/internal/controller/promotions/promotions.go +++ b/internal/controller/promotions/promotions.go @@ -72,7 +72,9 @@ func SetupReconcilerWithManager( For(&kargoapi.Promotion{}). WithEventFilter(changePredicate). WithEventFilter(shardPredicate). - WithEventFilter(kargo.IgnoreClearRefreshUpdates{}). + WithEventFilter(kargo.IgnoreAnnotationRemoval{ + AnnotationKey: kargoapi.AnnotationKeyRefresh, + }). WithOptions(controller.CommonOptions()). Build(reconciler) if err != nil { diff --git a/internal/controller/stages/stages.go b/internal/controller/stages/stages.go index c77145a0ac..aaaae0be32 100644 --- a/internal/controller/stages/stages.go +++ b/internal/controller/stages/stages.go @@ -305,7 +305,12 @@ func SetupReconcilerWithManager( ), ). WithEventFilter(shardPredicate). - WithEventFilter(kargo.IgnoreClearRefreshUpdates{}). + WithEventFilter(kargo.IgnoreAnnotationRemoval{ + AnnotationKey: kargoapi.AnnotationKeyRefresh, + }). + WithEventFilter(kargo.IgnoreAnnotationRemoval{ + AnnotationKey: kargoapi.AnnotationKeyReconfirm, + }). WithOptions(controller.CommonOptions()). Build( newReconciler( diff --git a/internal/controller/warehouses/warehouses.go b/internal/controller/warehouses/warehouses.go index 9002988043..41eb8e9455 100644 --- a/internal/controller/warehouses/warehouses.go +++ b/internal/controller/warehouses/warehouses.go @@ -123,7 +123,9 @@ func SetupReconcilerWithManager( ), ). WithEventFilter(shardPredicate). - WithEventFilter(kargo.IgnoreClearRefreshUpdates{}). + WithEventFilter(kargo.IgnoreAnnotationRemoval{ + AnnotationKey: kargoapi.AnnotationKeyRefresh, + }). WithOptions(controller.CommonOptions()). Complete(newReconciler(mgr.GetClient(), credentialsDB)); err != nil { return fmt.Errorf("error building Warehouse reconciler: %w", err) diff --git a/internal/kargo/kargo.go b/internal/kargo/kargo.go index b2ac52b6b4..6b681c0a6d 100644 --- a/internal/kargo/kargo.go +++ b/internal/kargo/kargo.go @@ -114,31 +114,31 @@ func (p PromoWentTerminal) Update(e event.UpdateEvent) bool { return false } -// IgnoreClearRefreshUpdates is a predicate that filters out update events if -// the update was only to clear the refresh annotation. This prevents the -// extra reconciliation that happens when reconcilers respond to refresh by -// updating the object's annotations. -type IgnoreClearRefreshUpdates struct { +type IgnoreAnnotationRemoval struct { predicate.Funcs + + AnnotationKey string } -// Update returns false if the update event cleared the refresh annotation. -// NOTE: this predicate assumes that the update was to clear the annotation and nothing else. -// This is safe to assume as long as the ClearXxxxRefresh helpers are always used to clear -// the refresh annotation. -func (i IgnoreClearRefreshUpdates) Update(e event.UpdateEvent) bool { +// Update returns false if the update event removed the AnnotationKey, true +// otherwise. +// +// NOTE: this predicate assumes that the update was to clear the annotation and +// nothing else. This is safe to assume as long as the helpers of the API are +// always used to clear the respective annotation. +func (i IgnoreAnnotationRemoval) Update(e event.UpdateEvent) bool { if e.ObjectOld == nil || e.ObjectNew == nil { return true } - oldHasRefreshAnnotation := false + oldHasAnnotation := false if oldAnnotations := e.ObjectOld.GetAnnotations(); oldAnnotations != nil { - _, oldHasRefreshAnnotation = oldAnnotations[kargoapi.AnnotationKeyRefresh] + _, oldHasAnnotation = oldAnnotations[i.AnnotationKey] } - newHasRefreshAnnotation := false + newHasAnnotation := false if newAnnotations := e.ObjectNew.GetAnnotations(); newAnnotations != nil { - _, newHasRefreshAnnotation = newAnnotations[kargoapi.AnnotationKeyRefresh] + _, newHasAnnotation = newAnnotations[i.AnnotationKey] } - if oldHasRefreshAnnotation && !newHasRefreshAnnotation { + if oldHasAnnotation && !newHasAnnotation { return false } return true diff --git a/internal/kargo/kargo_test.go b/internal/kargo/kargo_test.go index 543013f75a..2aef7c3ef2 100644 --- a/internal/kargo/kargo_test.go +++ b/internal/kargo/kargo_test.go @@ -96,7 +96,7 @@ func TestNewPromotion(t *testing.T) { } } -func TestIgnoreClearRefreshUpdates(t *testing.T) { +func TestIgnoreAnnotationRemovalUpdates(t *testing.T) { testCases := []struct { name string old client.Object @@ -110,7 +110,7 @@ func TestIgnoreClearRefreshUpdates(t *testing.T) { expected: true, }, { - name: "refresh cleared", + name: "annotation removed", old: &kargoapi.Stage{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -126,7 +126,7 @@ func TestIgnoreClearRefreshUpdates(t *testing.T) { expected: false, }, { - name: "refresh set", + name: "annotation set", old: &kargoapi.Stage{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{}, @@ -145,12 +145,14 @@ func TestIgnoreClearRefreshUpdates(t *testing.T) { for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { - icru := IgnoreClearRefreshUpdates{} + p := IgnoreAnnotationRemoval{ + AnnotationKey: kargoapi.AnnotationKeyRefresh, + } e := event.UpdateEvent{ ObjectOld: tc.old, ObjectNew: tc.new, } - require.Equal(t, tc.expected, icru.Update(e)) + require.Equal(t, tc.expected, p.Update(e)) }) } }