feat(controller): ignore removal of annotation

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
akuity · Mar 13, 2024 · 0148e0d · 0148e0d
1 parent 9b44a88
commit 0148e0d
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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))
 		})
 	}
 }