fix: inject artificial delay between sync waves to better support hea…

…lth assessments (#4715)

Signed-off-by: Jesse Suen <jesse_suen@intuit.com>

Makefile

@@ -383,7 +383,7 @@ start-e2e-local:
 	ARGOCD_ZJWT_FEATURE_FLAG=always \
 	ARGOCD_IN_CI=$(ARGOCD_IN_CI) \
 	ARGOCD_E2E_TEST=true \
-		goreman -f $(ARGOCD_PROCFILE) start
+		goreman -f $(ARGOCD_PROCFILE) start ${ARGOCD_START}
 
 # Cleans VSCode debug.test files from sub-dirs to prevent them from being included in packr boxes
 .PHONY: clean-debug

controller/sync.go

@@ -3,6 +3,8 @@ package controller
 import (
 	"context"
 	"fmt"
+	"os"
+	"strconv"
 	"sync/atomic"
 	"time"
 
@@ -26,6 +28,12 @@ import (
 
 var syncIdPrefix uint64 = 0
 
+const (
+	// EnvVarSyncWaveDelay is an environment variable which controls the delay in seconds between
+	// each sync-wave
+	EnvVarSyncWaveDelay = "ARGOCD_SYNC_WAVE_DELAY"
+)
+
 func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState) {
 	// Sync requests might be requested with ambiguous revisions (e.g. master, HEAD, v1.2.3).
 	// This can change meaning when resuming operations (e.g a hook sync). After calculating a
@@ -158,6 +166,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
 			}
 			return false
 		}),
+		sync.WithSyncWaveHook(delayBetweenSyncWaves),
 	)
 
 	if err != nil {
@@ -200,3 +209,25 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
 		}
 	}
 }
+
+// delayBetweenSyncWaves is a gitops-engine SyncWaveHook which introduces an artificial delay
+// between each sync wave. We introduce an artificial delay in order give other controllers a
+// _chance_ to react to the spec change that we just applied. This is important because without
+// this, Argo CD will likely assess resource health too quickly (against the stale object), causing
+// hooks to fire prematurely. See: https://github.com/argoproj/argo-cd/issues/4669.
+// Note, this is not foolproof, since a proper fix would require the CRD record
+// status.observedGeneration coupled with a health.lua that verifies
+// status.observedGeneration == metadata.generation
+func delayBetweenSyncWaves(phase common.SyncPhase, wave int, finalWave bool) error {
+	if !finalWave {
+		delaySec := 2
+		if delaySecStr := os.Getenv(EnvVarSyncWaveDelay); delaySecStr != "" {
+			if val, err := strconv.Atoi(delaySecStr); err == nil {
+				delaySec = val
+			}
+		}
+		duration := time.Duration(delaySec) * time.Second
+		time.Sleep(duration)
+	}
+	return nil
+}

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/TomOnTime/utfutil v0.0.0-20180511104225-09c41003ee1d
 	github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6 // indirect
 	github.com/alicebob/miniredis v2.5.0+incompatible
-	github.com/argoproj/gitops-engine v0.1.3-0.20201027224148-eb76c93f0a2e
+	github.com/argoproj/gitops-engine v0.1.3-0.20201030194627-cfdefa46b20c
 	github.com/argoproj/pkg v0.2.0
 	github.com/bombsimon/logrusr v0.0.0-20200131103305-03a291ce59b4
 	github.com/casbin/casbin v1.9.1