fix: selfHeal should always be respected with Multi-Source Apps argop…

…roj#19452 (argoproj#19512) * fix(controller): selfHeal respected in multi-source apps when there is object change in cluster Signed-off-by: Eric Lin <[email protected]> * fix: tests for multi source selfheal Signed-off-by: Ezzahhh <[email protected]> --------- Signed-off-by: Eric Lin <[email protected]> Signed-off-by: Ezzahhh <[email protected]>
presidenten · Aug 15, 2024 · 5a7bf2e · 5a7bf2e
1 parent 6aa4de6
commit 5a7bf2e
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 1 deletion.
diff --git a/controller/appcontroller.go b/controller/appcontroller.go
@@ -1957,11 +1957,18 @@ func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *
  }
  }
 
+ selfHeal := app.Spec.SyncPolicy.Automated.SelfHeal
+ // Multi-Source Apps with selfHeal disabled should not trigger an autosync if
+ // the last sync revision and the new sync revision is the same.
+ if app.Spec.HasMultipleSources() && !selfHeal && reflect.DeepEqual(app.Status.Sync.Revisions, syncStatus.Revisions) {
+ logCtx.Infof("Skipping auto-sync: selfHeal disabled and sync caused by object update")
+ return nil, 0
+ }
+
  desiredCommitSHA := syncStatus.Revision
  desiredCommitSHAsMS := syncStatus.Revisions
  alreadyAttempted, attemptPhase := alreadyAttemptedSync(app, desiredCommitSHA, desiredCommitSHAsMS, app.Spec.HasMultipleSources())
  ts.AddCheckpoint("already_attempted_sync_ms")
- selfHeal := app.Spec.SyncPolicy.Automated.SelfHeal
  op := appv1.Operation{
  Sync: &appv1.SyncOperation{
  Revision: desiredCommitSHA,

diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go
@@ -563,6 +563,42 @@ func TestAutoSync(t *testing.T) {
  assert.False(t, app.Operation.Sync.Prune)
 }
 
+func TestMultiSourceSelfHeal(t *testing.T) {
+ // Simulate OutOfSync caused by object change in cluster
+ // So our Sync Revisions and SyncStatus Revisions should deep equal
+ t.Run("ClusterObjectChangeShouldNotTriggerAutoSync", func(t *testing.T) {
+ app := newFakeMultiSourceApp()
+ app.Spec.SyncPolicy.Automated.SelfHeal = false
+ app.Status.Sync.Revisions = []string{"z", "x", "v"}
+ ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil)
+ syncStatus := v1alpha1.SyncStatus{
+ Status: v1alpha1.SyncStatusCodeOutOfSync,
+ Revisions: []string{"z", "x", "v"},
+ }
+ cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook-1", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}})
+ assert.Nil(t, cond)
+ app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
+ require.NoError(t, err)
+ assert.Nil(t, app.Operation)
+ })
+
+ t.Run("NewRevisionChangeShouldTriggerAutoSync", func(t *testing.T) {
+ app := newFakeMultiSourceApp()
+ app.Spec.SyncPolicy.Automated.SelfHeal = false
+ app.Status.Sync.Revisions = []string{"a", "b", "c"}
+ ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil)
+ syncStatus := v1alpha1.SyncStatus{
+ Status: v1alpha1.SyncStatusCodeOutOfSync,
+ Revisions: []string{"z", "x", "v"},
+ }
+ cond, _ := ctrl.autoSync(app, &syncStatus, []v1alpha1.ResourceStatus{{Name: "guestbook-1", Kind: kube.DeploymentKind, Status: v1alpha1.SyncStatusCodeOutOfSync}})
+ assert.Nil(t, cond)
+ app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(test.FakeArgoCDNamespace).Get(context.Background(), "my-app", metav1.GetOptions{})
+ require.NoError(t, err)
+ assert.NotNil(t, app.Operation)
+ })
+}
+
 func TestAutoSyncNotAllowEmpty(t *testing.T) {
  app := newFakeApp()
  app.Spec.SyncPolicy.Automated.Prune = true