From 42e24e6e2aa0b64424e28d33519ef542bf3b5670 Mon Sep 17 00:00:00 2001
From: Alexander Matyushentsev <Alexander_Matyushentsev@intuit.com>
Date: Mon, 22 Jun 2020 14:09:22 -0700
Subject: [PATCH] fix: controller should not re-trigger auto-sync if sync
 failed due to comparison error (#3824)

---
 controller/sync.go      |  7 +++----
 controller/sync_test.go | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/controller/sync.go b/controller/sync.go
index 7e90772d0286d..4dccf24e28c90 100644
--- a/controller/sync.go
+++ b/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
diff --git a/controller/sync_test.go b/controller/sync_test.go
index e3618b1895888..697c002701a57 100644
--- a/controller/sync_test.go
+++ b/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)
+}