fix(appset): Reconcile appset only once when appset is refreshed (fix…

… 21171) (argoproj#21172)



Signed-off-by: Philippe Da Costa <[email protected]>

applicationset/controllers/applicationset_controller.go

@@ -531,6 +531,33 @@ func ignoreNotAllowedNamespaces(namespaces []string) predicate.Predicate {
 	}
 }
 
+// ignoreWhenAnnotationApplicationSetRefreshIsRemoved returns a predicate that ignores updates to ApplicationSet resources
+// when the ApplicationSetRefresh annotation is removed
+// First reconciliation is triggered when the annotation is added by [webhook.go#refreshApplicationSet]
+// Using this predicate we avoid a second reconciliation triggered by the controller himself when the annotation is removed.
+func ignoreWhenAnnotationApplicationSetRefreshIsRemoved() predicate.Predicate {
+	return predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			oldAppset, isAppSet := e.ObjectOld.(*argov1alpha1.ApplicationSet)
+			if !isAppSet {
+				return false
+			}
+			newAppset, isAppSet := e.ObjectNew.(*argov1alpha1.ApplicationSet)
+			if !isAppSet {
+				return false
+			}
+
+			_, oldHasRefreshAnnotation := oldAppset.Annotations[common.AnnotationApplicationSetRefresh]
+			_, newHasRefreshAnnotation := newAppset.Annotations[common.AnnotationApplicationSetRefresh]
+
+			if oldHasRefreshAnnotation && !newHasRefreshAnnotation {
+				return false
+			}
+			return true
+		},
+	}
+}
+
 func appControllerIndexer(rawObj client.Object) []string {
 	// grab the job object, extract the owner...
 	app := rawObj.(*argov1alpha1.Application)
@@ -556,7 +583,7 @@ func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager, enableProg
 
 	return ctrl.NewControllerManagedBy(mgr).WithOptions(controller.Options{
 		MaxConcurrentReconciles: maxConcurrentReconciliations,
-	}).For(&argov1alpha1.ApplicationSet{}).
+	}).For(&argov1alpha1.ApplicationSet{}, builder.WithPredicates(ignoreWhenAnnotationApplicationSetRefreshIsRemoved())).
 		Owns(&argov1alpha1.Application{}, builder.WithPredicates(ownsHandler)).
 		WithEventFilter(ignoreNotAllowedNamespaces(r.ApplicationSetNamespaces)).
 		Watches(

applicationset/controllers/applicationset_controller_test.go

@@ -6655,3 +6655,78 @@ func TestMigrateStatus(t *testing.T) {
 		})
 	}
 }
+
+func TestIgnoreWhenAnnotationApplicationSetRefreshIsRemoved(t *testing.T) {
+	buildAppSet := func(annotations map[string]string) *v1alpha1.ApplicationSet {
+		return &v1alpha1.ApplicationSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: annotations,
+			},
+		}
+	}
+
+	tests := []struct {
+		name              string
+		oldAppSet         crtclient.Object
+		newAppSet         crtclient.Object
+		reconcileExpected bool
+	}{
+		{
+			name: "annotation removed",
+			oldAppSet: buildAppSet(map[string]string{
+				argocommon.AnnotationApplicationSetRefresh: "true",
+			}),
+			newAppSet:         buildAppSet(map[string]string{}),
+			reconcileExpected: false,
+		},
+		{
+			name: "annotation not removed",
+			oldAppSet: buildAppSet(map[string]string{
+				argocommon.AnnotationApplicationSetRefresh: "true",
+			}),
+			newAppSet: buildAppSet(map[string]string{
+				argocommon.AnnotationApplicationSetRefresh: "true",
+			}),
+			reconcileExpected: true,
+		},
+		{
+			name:              "annotation never existed",
+			oldAppSet:         buildAppSet(map[string]string{}),
+			newAppSet:         buildAppSet(map[string]string{}),
+			reconcileExpected: true,
+		},
+		{
+			name:      "annotation added",
+			oldAppSet: buildAppSet(map[string]string{}),
+			newAppSet: buildAppSet(map[string]string{
+				argocommon.AnnotationApplicationSetRefresh: "true",
+			}),
+			reconcileExpected: true,
+		},
+		{
+			name:              "old object is not an appset",
+			oldAppSet:         &v1alpha1.Application{},
+			newAppSet:         buildAppSet(map[string]string{}),
+			reconcileExpected: false,
+		},
+		{
+			name:              "new object is not an appset",
+			oldAppSet:         buildAppSet(map[string]string{}),
+			newAppSet:         &v1alpha1.Application{},
+			reconcileExpected: false,
+		},
+	}
+
+	predicate := ignoreWhenAnnotationApplicationSetRefreshIsRemoved()
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := event.UpdateEvent{
+				ObjectOld: tt.oldAppSet,
+				ObjectNew: tt.newAppSet,
+			}
+			result := predicate.Update(e)
+			assert.Equal(t, tt.reconcileExpected, result)
+		})
+	}
+}