fix: controller should not re-trigger auto-sync if sync failed due to…

… comparison error (argoproj#3824)

controller/sync.go

@@ -76,6 +76,9 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
  }
 
  compareResult := m.CompareAppState(app, proj, revision, source, false, syncOp.Manifests)
+ // We now have a concrete commit SHA. Save this in the sync result revision so that we remember
+ // what we should be syncing to when resuming operations.
+ syncRes.Revision = compareResult.syncStatus.Revision
 
  // If there are any comparison or spec errors error conditions do not perform the operation
  if errConditions := app.Status.GetConditions(map[v1alpha1.ApplicationConditionType]bool{
@@ -87,10 +90,6 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
  return
  }
 
- // We now have a concrete commit SHA. Save this in the sync result revision so that we remember
- // what we should be syncing to when resuming operations.
- syncRes.Revision = compareResult.syncStatus.Revision
-
  clst, err := m.db.GetCluster(context.Background(), app.Spec.Destination.Server)
  if err != nil {
  state.Phase = common.OperationError

controller/sync_test.go

@@ -1,6 +1,7 @@
 package controller
 
 import (
+ "os"
  "testing"
 
  "github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -100,3 +101,43 @@ func TestPersistRevisionHistoryRollback(t *testing.T) {
  assert.Equal(t, source, updatedApp.Status.History[0].Source)
  assert.Equal(t, "abc123", updatedApp.Status.History[0].Revision)
 }
+
+func TestSyncComparisonError(t *testing.T) {
+ app := newFakeApp()
+ app.Status.OperationState = nil
+ app.Status.History = nil
+
+ defaultProject := &v1alpha1.AppProject{
+ ObjectMeta: v1.ObjectMeta{
+ Namespace: test.FakeArgoCDNamespace,
+ Name: "default",
+ },
+ Spec: v1alpha1.AppProjectSpec{
+ SignatureKeys: []v1alpha1.SignatureKey{{KeyID: "test"}},
+ },
+ }
+ data := fakeData{
+ apps: []runtime.Object{app, defaultProject},
+ manifestResponse: &apiclient.ManifestResponse{
+ Manifests: []string{},
+ Namespace: test.FakeDestNamespace,
+ Server: test.FakeClusterURL,
+ Revision: "abc123",
+ VerifyResult: "something went wrong",
+ },
+ managedLiveObjs: make(map[kube.ResourceKey]*unstructured.Unstructured),
+ }
+ ctrl := newFakeController(&data)
+
+ // Sync with source unspecified
+ opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{
+ Sync: &v1alpha1.SyncOperation{},
+ }}
+ os.Setenv("ARGOCD_GPG_ENABLED", "true")
+ defer os.Setenv("ARGOCD_GPG_ENABLED", "false")
+ ctrl.appStateManager.SyncAppState(app, opState)
+
+ conditions := app.Status.GetConditions(map[v1alpha1.ApplicationConditionType]bool{v1alpha1.ApplicationConditionComparisonError: true})
+ assert.NotEmpty(t, conditions)
+ assert.Equal(t, "abc123", opState.SyncResult.Revision)
+}