fix(controller): Fix workflows with retryStrategy left Running after …

…completion (#5497) Signed-off-by: Simon Behar <simbeh7@gmail.com>
argoproj · Mar 23, 2021 · dfe6ceb · dfe6ceb
1 parent be44ce9
commit dfe6ceb
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 4 deletions.
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
@@ -1802,7 +1802,7 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string,
 	if retryNodeName != "" {
 		retryNode := woc.wf.GetNodeByName(retryNodeName)
 		if !retryNode.Fulfilled() && node.Fulfilled() { // if the retry child has completed we need to update outself
-			node, err = woc.executeTemplate(ctx, retryNodeName, orgTmpl, tmplCtx, args, opts)
+			retryNode, err = woc.executeTemplate(ctx, retryNodeName, orgTmpl, tmplCtx, args, opts)
 			if err != nil {
 				return woc.markNodeError(node.Name, err), err
 			}

diff --git a/workflow/controller/operator_concurrency_test.go b/workflow/controller/operator_concurrency_test.go
@@ -478,9 +478,7 @@ func TestSynchronizationWithRetry(t *testing.T) {
 		woc = newWorkflowOperationCtx(woc.wf, controller)
 		woc.operate(ctx)
 		// Nobody is waiting for the lock
-		assert.Empty(woc.wf.Status.Synchronization.Semaphore.Waiting)
-		// Nobody is holding the lock
-		assert.Empty(woc.wf.Status.Synchronization.Semaphore.Holding[0].Holders)
+		assert.Nil(woc.wf.Status.Synchronization)
 	})
 }
 

diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
@@ -6314,3 +6314,63 @@ func TestGenerateOutputResultRegex(t *testing.T) {
 	assert.Equal(t, `steps\.template-name\.outputs\.result`, ref)
 	assert.Equal(t, `steps\[['\"]template-name['\"]\]\.outputs.result`, expr)
 }
+
+const rootRetryStrategyCompletes = `apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-world-5bd7v
+spec:
+  activeDeadlineSeconds: 300
+  entrypoint: whalesay
+  retryStrategy:
+    limit: 1
+  templates:
+  - container:
+      args:
+      - hello world
+      command:
+      - cowsay
+      image: docker/whalesay:latest
+    name: whalesay
+  ttlStrategy:
+    secondsAfterCompletion: 600
+status:
+  nodes:
+    hello-world-5bd7v:
+      children:
+      - hello-world-5bd7v-643409622
+      displayName: hello-world-5bd7v
+      finishedAt: "2021-03-23T14:53:45Z"
+      id: hello-world-5bd7v
+      name: hello-world-5bd7v
+      phase: Succeeded
+      startedAt: "2021-03-23T14:53:39Z"
+      templateName: whalesay
+      templateScope: local/hello-world-5bd7v
+      type: Retry
+    hello-world-5bd7v-643409622:
+      displayName: hello-world-5bd7v(0)
+      finishedAt: "2021-03-23T14:53:44Z"
+      hostNodeName: k3d-k3s-default-server-0
+      id: hello-world-5bd7v-643409622
+      name: hello-world-5bd7v(0)
+      phase: Succeeded
+      startedAt: "2021-03-23T14:53:39Z"
+      templateName: whalesay
+      templateScope: local/hello-world-5bd7v
+      type: Pod
+  phase: Running
+  startedAt: "2021-03-23T14:53:39Z"
+`
+
+func TestRootRetryStrategyCompletes(t *testing.T) {
+	wf := unmarshalWF(rootRetryStrategyCompletes)
+	cancel, controller := newController(wf)
+	defer cancel()
+
+	ctx := context.Background()
+	woc := newWorkflowOperationCtx(wf, controller)
+	woc.operate(ctx)
+
+	assert.Equal(t, wfv1.WorkflowSucceeded, woc.wf.Status.Phase)
+}