merge self heal logic with oss

Author: pasha-codefresh

assets/swagger.json

@@ -9446,13 +9446,6 @@
         "comparedTo": {
           "$ref": "#/definitions/v1alpha1ComparedTo"
         },
-        "manifestsChanged": {
-          "type": "object",
-          "title": "ManifestsChanged indicates whether the manifests have changed base on argocd.argoproj.io/manifest-generate-paths annotation",
-          "additionalProperties": {
-            "type": "boolean"
-          }
-        },
         "revision": {
           "type": "string",
           "title": "Revision contains information about the revision the comparison has been performed to"

controller/appcontroller.go

@@ -8,7 +8,6 @@ import (
 	"math"
 	"math/rand"
 	"net/http"
-	"os"
 	"reflect"
 	"runtime/debug"
 	"sort"
@@ -1623,7 +1622,7 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
 	}
 
 	if project.Spec.SyncWindows.Matches(app).CanSync(false) {
-		syncErrCond, opMS := ctrl.autoSync(app, compareResult.syncStatus, compareResult.resources)
+		syncErrCond, opMS := ctrl.autoSync(app, compareResult.syncStatus, compareResult.resources, compareResult.revisionUpdated)
 		setOpMs = opMS
 		if syncErrCond != nil {
 			app.Status.SetConditions(
@@ -1841,7 +1840,7 @@ func (ctrl *ApplicationController) persistAppStatus(orig *appv1.Application, new
 }
 
 // autoSync will initiate a sync operation for an application configured with automated sync
-func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *appv1.SyncStatus, resources []appv1.ResourceStatus) (*appv1.ApplicationCondition, time.Duration) {
+func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *appv1.SyncStatus, resources []appv1.ResourceStatus, revisionUpdated bool) (*appv1.ApplicationCondition, time.Duration) {
 	if app.Spec.SyncPolicy == nil || app.Spec.SyncPolicy.Automated == nil {
 		return nil, 0
 	}
@@ -1879,7 +1878,7 @@ func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *
 
 	desiredCommitSHA := syncStatus.Revision
 	desiredCommitSHAsMS := syncStatus.Revisions
-	alreadyAttempted, attemptPhase := alreadyAttemptedSync(app, desiredCommitSHA, desiredCommitSHAsMS, app.Spec.HasMultipleSources(), syncStatus.ManifestsChanged)
+	alreadyAttempted, attemptPhase := alreadyAttemptedSync(app, desiredCommitSHA, desiredCommitSHAsMS, app.Spec.HasMultipleSources(), revisionUpdated)
 	selfHeal := app.Spec.SyncPolicy.Automated.SelfHeal
 	op := appv1.Operation{
 		Sync: &appv1.SyncOperation{
@@ -1970,21 +1969,21 @@ func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *
 
 // alreadyAttemptedSync returns whether the most recent sync was performed against the
 // commitSHA and with the same app source config which are currently set in the app
-func alreadyAttemptedSync(app *appv1.Application, commitSHA string, commitSHAsMS []string, hasMultipleSources bool, manifestsChangedMap map[string]bool) (bool, synccommon.OperationPhase) {
+func alreadyAttemptedSync(app *appv1.Application, commitSHA string, commitSHAsMS []string, hasMultipleSources bool, revisionUpdated bool) (bool, synccommon.OperationPhase) {
 	if app.Status.OperationState == nil || app.Status.OperationState.Operation.Sync == nil || app.Status.OperationState.SyncResult == nil {
 		return false, ""
 	}
 	if hasMultipleSources {
-		if !reflect.DeepEqual(app.Status.OperationState.SyncResult.Revisions, commitSHAsMS) {
-			return false, ""
+		if revisionUpdated {
+			if !reflect.DeepEqual(app.Status.OperationState.SyncResult.Revisions, commitSHAsMS) {
+				return false, ""
+			}
+		} else {
+			log.WithField("application", app.Name).Debugf("Skipping auto-sync: commitSHA %s has no changes", commitSHA)
 		}
 	} else {
-		manifestChanged, ok := manifestsChangedMap[commitSHA]
-		featureFlagDisabled := os.Getenv("PERSIST_CHANGE_REVISIONS") != "1"
-		// If record not exists, we need to do sync
-		if featureFlagDisabled || !ok || manifestChanged {
-			log.WithField("application", app.Name).Infof("Executing compare of syncResult.Revision and commitSha because of feature flag disabled or manifest changed: %v", commitSHA)
-			log.WithField("application", app.Name).Infof("Executing compare of syncResult.Revision and commitSha with context, map: %v, flag: %t, record exists: %t", manifestsChangedMap, featureFlagDisabled, ok)
+		if revisionUpdated {
+			log.WithField("application", app.Name).Infof("Executing compare of syncResult.Revision and commitSha because manifest changed: %v", commitSHA)
 			if app.Status.OperationState.SyncResult.Revision != commitSHA {
 				return false, ""
 			}

controller/appcontroller_test.go

@@ -2096,72 +2096,6 @@ func TestAddControllerNamespace(t *testing.T) {
 	})
 }
 
-func TestAlreadyAttemptSync(t *testing.T) {
-	app := newFakeApp()
-	t.Run("same manifest with sync result, with disabled flag", func(t *testing.T) {
-		manifestChangedMap := make(map[string]bool)
-		manifestChangedMap["sha"] = false
-
-		app.Status.Sync.ManifestsChanged = manifestChangedMap
-
-		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, nil)
-		assert.False(t, attempted)
-	})
-
-	t.Run("same manifest with sync result, with enabled flag", func(t *testing.T) {
-		_ = os.Setenv("PERSIST_CHANGE_REVISIONS", "1")
-
-		manifestChangedMap := make(map[string]bool)
-		manifestChangedMap["sha"] = false
-
-		app.Status.Sync.ManifestsChanged = manifestChangedMap
-
-		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, manifestChangedMap)
-		assert.True(t, attempted)
-	})
-
-	t.Run("different manifest with sync result, with disabled flag", func(t *testing.T) {
-		manifestChangedMap := make(map[string]bool)
-		manifestChangedMap["sha"] = true
-
-		app.Status.Sync.ManifestsChanged = manifestChangedMap
-
-		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, nil)
-		assert.False(t, attempted)
-	})
-
-	t.Run("different manifest with sync result, with enabled flag", func(t *testing.T) {
-		_ = os.Setenv("PERSIST_CHANGE_REVISIONS", "1")
-
-		manifestChangedMap := make(map[string]bool)
-		manifestChangedMap["sha"] = true
-
-		app.Status.Sync.ManifestsChanged = manifestChangedMap
-
-		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, manifestChangedMap)
-		assert.False(t, attempted)
-	})
-
-	t.Run("different manifest with sync result, with enabled flag", func(t *testing.T) {
-		_ = os.Setenv("PERSIST_CHANGE_REVISIONS", "1")
-
-		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, nil)
-		assert.False(t, attempted)
-	})
-
-	t.Run("different manifest with sync result, with enabled flag v2", func(t *testing.T) {
-		_ = os.Setenv("PERSIST_CHANGE_REVISIONS", "1")
-
-		manifestChangedMap := make(map[string]bool)
-		manifestChangedMap["sha"] = false
-
-		app.Status.Sync.ManifestsChanged = manifestChangedMap
-
-		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, manifestChangedMap)
-		assert.True(t, attempted)
-	})
-}
-
 func TestHelmValuesObjectHasReplaceStrategy(t *testing.T) {
 	app := v1alpha1.Application{
 		Status: v1alpha1.ApplicationStatus{Sync: v1alpha1.SyncStatus{ComparedTo: v1alpha1.ComparedTo{
@@ -2223,3 +2157,16 @@ func TestAppStatusIsReplaced(t *testing.T) {
 	require.True(t, has)
 	require.Nil(t, val)
 }
+
+func TestAlreadyAttemptSync(t *testing.T) {
+	app := newFakeApp()
+	t.Run("same manifest with sync result", func(t *testing.T) {
+		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, false)
+		assert.True(t, attempted)
+	})
+
+	t.Run("different manifest with sync result", func(t *testing.T) {
+		attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false, true)
+		assert.False(t, attempted)
+	})
+}