If there is only one wave and no pre/post hooks, we should be synced.… (

argoproj#2217)

controller/sync.go

@@ -267,8 +267,11 @@ func (sc *syncContext) sync() {
  }
  }
 
- // any running tasks, lets wait...
- if tasks.Any(func(t *syncTask) bool { return t.running() }) {
+ // if (a) we are multi-step and we have any running tasks,
+ // or (b) there are any running hooks,
+ // then wait...
+ multiStep := tasks.multiStep()
+ if tasks.Any(func(t *syncTask) bool { return (multiStep || t.isHook()) && t.running() }) {
  sc.setOperationPhase(v1alpha1.OperationRunning, "one or more tasks are running")
  return
  }
@@ -282,9 +285,9 @@ func (sc *syncContext) sync() {
  return
  }
 
- sc.log.WithFields(log.Fields{"tasks": tasks}).Debug("filtering out completed tasks")
+ sc.log.WithFields(log.Fields{"tasks": tasks}).Debug("filtering out non-pending tasks")
  // remove tasks that are completed, we can assume that there are no running tasks
- tasks = tasks.Filter(func(t *syncTask) bool { return !t.completed() })
+ tasks = tasks.Filter(func(t *syncTask) bool { return t.pending() })
 
  // If no sync tasks were generated (e.g., in case all application manifests have been removed),
  // the sync operation is successful.

controller/sync_tasks.go

@@ -165,3 +165,21 @@ func (s syncTasks) wave() int {
  }
  return 0
 }
+
+func (s syncTasks) lastPhase() v1alpha1.SyncPhase {
+ if len(s) > 0 {
+ return s[len(s)-1].phase
+ }
+ return ""
+}
+
+func (s syncTasks) lastWave() int {
+ if len(s) > 0 {
+ return s[len(s)-1].wave()
+ }
+ return 0
+}
+
+func (s syncTasks) multiStep() bool {
+ return s.wave() != s.lastWave() || s.phase() != s.lastPhase()
+}

controller/sync_tasks_test.go

@@ -8,7 +8,9 @@ import (
  apiv1 "k8s.io/api/core/v1"
  "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
+ "github.com/argoproj/argo-cd/common"
  . "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
+ . "github.com/argoproj/argo-cd/test"
 )
 
 func Test_syncTasks_kindOrder(t *testing.T) {
@@ -366,3 +368,25 @@ func TestSyncNamespaceAgainstCRD(t *testing.T) {
 
  assert.Equal(t, syncTasks{namespace, crd}, unsorted)
 }
+
+func Test_syncTasks_multiStep(t *testing.T) {
+ t.Run("Single", func(t *testing.T) {
+ tasks := syncTasks{{liveObj: Annotate(NewPod(), common.AnnotationSyncWave, "-1"), phase: SyncPhaseSync}}
+ assert.Equal(t, SyncPhaseSync, tasks.phase())
+ assert.Equal(t, -1, tasks.wave())
+ assert.Equal(t, SyncPhaseSync, tasks.lastPhase())
+ assert.Equal(t, -1, tasks.lastWave())
+ assert.False(t, tasks.multiStep())
+ })
+ t.Run("Double", func(t *testing.T) {
+ tasks := syncTasks{
+ {liveObj: Annotate(NewPod(), common.AnnotationSyncWave, "-1"), phase: SyncPhasePreSync},
+ {liveObj: Annotate(NewPod(), common.AnnotationSyncWave, "1"), phase: SyncPhasePostSync},
+ }
+ assert.Equal(t, SyncPhasePreSync, tasks.phase())
+ assert.Equal(t, -1, tasks.wave())
+ assert.Equal(t, SyncPhasePostSync, tasks.lastPhase())
+ assert.Equal(t, 1, tasks.lastWave())
+ assert.True(t, tasks.multiStep())
+ })
+}

test/e2e/app_deletion_test.go

@@ -16,13 +16,13 @@ func TestDeletingAppStuckInSync(t *testing.T) {
  Path("hook").
  When().
  PatchFile("hook.yaml", `[{"op": "replace", "path": "/spec/containers/0/command", "value": ["sh", "-c", "until ls /tmp/done; do sleep 0.1; done"]}]`).
+ PatchFile("pod.yaml", `[{"op": "add", "path": "/metadata/annotations", "value": {"argocd.argoproj.io/sync-wave": "1"}}]`).
  Create().
  Sync().
  Then().
  // stuck in running state
  Expect(OperationPhaseIs(OperationRunning)).
- Expect(SyncStatusIs(SyncStatusCodeSynced)).
- Expect(ResourceResultNumbering(2)).
+ Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
  When().
  Delete(true).
  Then().

test/e2e/cli_test.go

@@ -19,11 +19,12 @@ func TestCliAppCommand(t *testing.T) {
  output, err := RunCli("app", "sync", Name(), "--timeout", "90")
  assert.NoError(t, err)
  vars := map[string]interface{}{"Name": Name(), "Namespace": DeploymentNamespace()}
- assert.Contains(t, NormalizeOutput(output), Tmpl(`Pod {{.Namespace}} pod Synced Healthy pod/pod created`, vars))
+ assert.Contains(t, NormalizeOutput(output), Tmpl(`Pod {{.Namespace}} pod Synced Progressing pod/pod created`, vars))
  assert.Contains(t, NormalizeOutput(output), Tmpl(`Pod {{.Namespace}} hook Succeeded Sync pod/hook created`, vars))
  }).
  Then().
  Expect(OperationPhaseIs(OperationSucceeded)).
+ Expect(HealthIs(HealthStatusHealthy)).
  And(func(_ *Application) {
  output, err := RunCli("app", "list")
  assert.NoError(t, err)

test/e2e/hook_test.go

@@ -84,7 +84,7 @@ func TestPreSyncHookFailure(t *testing.T) {
  Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeOutOfSync))
 }
 
-// make sure that if pre-sync fails, we fail the app and we did create the pod
+// make sure that if sync fails, we fail the app and we did create the pod
 func TestSyncHookFailure(t *testing.T) {
  Given(t).
  Path("hook").
@@ -102,6 +102,19 @@ func TestSyncHookFailure(t *testing.T) {
  Expect(ResourceSyncStatusIs("Pod", "pod", SyncStatusCodeSynced))
 }
 
+// make sure that if the deployments fails, we still get success and synced
+func TestSyncHookResourceFailure(t *testing.T) {
+ Given(t).
+ Path("hook-and-deployment").
+ When().
+ Create().
+ Sync().
+ Then().
+ Expect(OperationPhaseIs(OperationSucceeded)).
+ Expect(SyncStatusIs(SyncStatusCodeSynced)).
+ Expect(HealthIs(HealthStatusProgressing))
+}
+
 // make sure that if post-sync fails, we fail the app and we did not create the pod
 func TestPostSyncHookFailure(t *testing.T) {
  Given(t).

test/e2e/testdata/hook-and-deployment/deployment.yaml

@@ -0,0 +1,28 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: my-deployment
+spec:
+ replicas: 1
+ selector:
+ matchLabels:
+ app: my-app
+ template:
+ metadata:
+ labels:
+ app: my-app
+ spec:
+ containers:
+ - name: main
+ image: nginx:latest
+ imagePullPolicy: IfNotPresent
+ readinessProbe:
+ failureThreshold: 3
+ httpGet:
+ path: /does-not-exist
+ port: 8080
+ scheme: HTTP
+ initialDelaySeconds: 5
+ periodSeconds: 5
+ successThreshold: 3
+ timeoutSeconds: 1

test/e2e/testdata/hook-and-deployment/hook.yaml

@@ -0,0 +1,16 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+ name: my-hook
+ annotations:
+ argocd.argoproj.io/hook: Sync
+spec:
+ template:
+ spec:
+ containers:
+ - command:
+ - "true"
+ image: "alpine:latest"
+ imagePullPolicy: IfNotPresent
+ name: main
+ restartPolicy: Never