Repository: argoproj/argo
Pull request #3435 (2020-07-09 16:52:40)

fix: Ensure task dependencies run after onExit handler is fulfilled
Fixes: https://github.com/argoproj/argo/issues/3431
Fixes: https://github.com/argoproj/argo/issues/3432

~**Not ready for review**.~

~Tests are not expected to pass~ Tests should now pass
~Still need to write new tests~ Added tests


Patch:
diff --git a/.github/workflows/docker-image.yml b/.github/workflows/docker-image.yml
new file mode 100644
index 000000000..478ccec15
--- /dev/null
+++ b/.github/workflows/docker-image.yml
@@ -0,0 +1,106 @@
+name: Docker Image CI
+
+on:
+  push:
+    tags:
+      - v*
+    branches:
+      - master
+
+defaults:
+  run:
+    shell: bash
+
+jobs:
+  build-linux:
+    name: Build & Push Linux Docker Images
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Docker Login
+        uses: Azure/docker-login@v1
+        with:
+          username: ${{ secrets.DOCKERIO_USERNAME }}
+          password: ${{ secrets.DOCKERIO_PASSWORD }}
+      - name: Build & Push Linux Docker Images
+        env:
+          DOCKERIO_USERNAME: ${{ secrets.DOCKERIO_USERNAME }}
+        run: |
+          docker_org=$DOCKERIO_USERNAME
+
+          tag=$(basename $GITHUB_REF)
+          if [ $tag = "master" ]; then
+            tag="latest"
+          fi
+
+          targets="workflow-controller argoexec argocli"
+          for target in $targets; do
+            image_name="${docker_org}/${target}:${tag}-linux"
+            docker build --target $target -t $image_name .
+            docker push $image_name
+          done
+
+  build-windows:
+    name: Build & Push Windows Docker Images
+    runs-on: windows-2019
+    steps:
+      - uses: actions/checkout@v2
+      - name: Docker Login
+        uses: Azure/docker-login@v1
+        with:
+          username: ${{ secrets.DOCKERIO_USERNAME }}
+          password: ${{ secrets.DOCKERIO_PASSWORD }}
+      - name: Build & Push Windows Docker Images
+        env:
+          DOCKERIO_USERNAME: ${{ secrets.DOCKERIO_USERNAME }}
+        run: |
+          docker_org=$DOCKERIO_USERNAME
+
+          tag=$(basename $GITHUB_REF)
+          if [ $tag = "master" ]; then
+            tag="latest"
+          fi
+
+          targets="argoexec"
+          for target in $targets; do
+            image_name="${docker_org}/${target}:${tag}-windows"
+            docker build --target $target -t $image_name -f Dockerfile.windows .
+            docker push $image_name
+          done
+
+  push-images:
+    name: Push Multiarch Image
+    runs-on: ubuntu-latest
+    needs: [build-linux, build-windows]
+    steps:
+      - uses: actions/checkout@v2
+      - name: Docker Login
+        uses: Azure/docker-login@v1
+        with:
+          username: ${{ secrets.DOCKERIO_USERNAME }}
+          password: ${{ secrets.DOCKERIO_PASSWORD }}
+      - name: Push Multiarch Image
+        env:
+          DOCKERIO_USERNAME: ${{ secrets.DOCKERIO_USERNAME }}
+        run: |
+          echo $(jq -c '. + { "experimental": "enabled" }' ${DOCKER_CONFIG}/config.json) > ${DOCKER_CONFIG}/config.json
+
+          docker_org=$DOCKERIO_USERNAME
+
+          tag=$(basename $GITHUB_REF)
+          if [ $tag = "master" ]; then
+            tag="latest"
+          fi
+
+          targets="workflow-controller argoexec argocli"
+          for target in $targets; do
+            image_name="${docker_org}/${target}:${tag}"
+
+            if [ $target = "argoexec" ]; then
+              docker manifest create $image_name ${image_name}-linux ${image_name}-windows
+            else
+              docker manifest create $image_name ${image_name}-linux
+            fi
+
+            docker manifest push $image_name
+          done
\ No newline at end of file
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
index f2d2a2b48..6309f06ed 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -1812,6 +1812,15 @@ func (wf *Workflow) GetTemplateByName(name string) *Template {
 	return nil
 }
 
+func (wf *Workflow) GetNodeByName(nodeName string) *NodeStatus {
+	nodeID := wf.NodeID(nodeName)
+	node, ok := wf.Status.Nodes[nodeID]
+	if !ok {
+		return nil
+	}
+	return &node
+}
+
 // GetResourceScope returns the template scope of workflow.
 func (wf *Workflow) GetResourceScope() ResourceScope {
 	return ResourceScopeLocal
diff --git a/workflow/common/util.go b/workflow/common/util.go
index 79a87e804..6998dc3ca 100644
--- a/workflow/common/util.go
+++ b/workflow/common/util.go
@@ -694,3 +694,7 @@ func SplitClusterWorkflowTemplateYAMLFile(body []byte, strict bool) ([]wfv1.Clus
 	}
 	return manifests, nil
 }
+
+func GenerateOnExitNodeName(parentDisplayName string) string {
+	return fmt.Sprintf("%s.onExit", parentDisplayName)
+}
diff --git a/workflow/controller/dag.go b/workflow/controller/dag.go
index bfb9a0129..7436fc7df 100644
--- a/workflow/controller/dag.go
+++ b/workflow/controller/dag.go
@@ -193,7 +193,7 @@ func (d *dagContext) assessDAGPhase(targetTasks []string, nodes wfv1.Nodes) wfv1
 }
 
 func (woc *wfOperationCtx) executeDAG(nodeName string, tmplCtx *templateresolution.Context, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	if node == nil {
 		node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypeDAG, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodeRunning)
 	}
@@ -261,7 +261,7 @@ func (woc *wfOperationCtx) executeDAG(nodeName string, tmplCtx *templateresoluti
 		return node, err
 	}
 	if outputs != nil {
-		node = woc.getNodeByName(nodeName)
+		node = woc.wf.GetNodeByName(nodeName)
 		node.Outputs = outputs
 		woc.wf.Status.Nodes[node.ID] = *node
 	}
@@ -283,7 +283,7 @@ func (woc *wfOperationCtx) updateOutboundNodesForTargetTasks(dagCtx *dagContext,
 		outboundNodeIDs := woc.getOutboundNodes(depNode.ID)
 		outbound = append(outbound, outboundNodeIDs...)
 	}
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	node.OutboundNodes = outbound
 	woc.wf.Status.Nodes[node.ID] = *node
 	woc.log.Infof("Outbound nodes of %s set to %s", node.ID, outbound)
@@ -317,7 +317,7 @@ func (woc *wfOperationCtx) executeDAGTask(dagCtx *dagContext, taskName string) {
 	nodeName := dagCtx.taskNodeName(taskName)
 	taskDependencies := dagCtx.GetTaskDependencies(taskName)
 
-	taskGroupNode := woc.getNodeByName(nodeName)
+	taskGroupNode := woc.wf.GetNodeByName(nodeName)
 	if taskGroupNode != nil && taskGroupNode.Type != wfv1.NodeTypeTaskGroup {
 		taskGroupNode = nil
 	}
@@ -345,16 +345,16 @@ func (woc *wfOperationCtx) executeDAGTask(dagCtx *dagContext, taskName string) {
 	}
 
 	if dagCtx.GetTaskDependsLogic(taskName) != "" {
+		// Recurse into all of this node's dependencies
+		for _, dep := range taskDependencies {
+			woc.executeDAGTask(dagCtx, dep)
+		}
 		execute, proceed, err := dagCtx.evaluateDependsLogic(taskName)
 		if err != nil {
 			woc.initializeNode(nodeName, wfv1.NodeTypeSkipped, dagTemplateScope, task, dagCtx.boundaryID, wfv1.NodeError, err.Error())
 			connectDependencies(nodeName)
 			return
 		}
-		// Recurse into all of this node's dependencies
-		for _, dep := range taskDependencies {
-			woc.executeDAGTask(dagCtx, dep)
-		}
 		if !proceed {
 			// This node's dependencies are not completed yet, return
 			return
@@ -624,6 +624,13 @@ func (d *dagContext) evaluateDependsLogic(taskName string) (bool, bool, error) {
 			return false, false, nil
 		}
 
+		// If a task happens to have an onExit node, don't proceed until the onExit node is fulfilled
+		if onExitNode := d.wf.GetNodeByName(common.GenerateOnExitNodeName(taskName)); onExitNode != nil {
+			if !onExitNode.Fulfilled() {
+				return false, false, nil
+			}
+		}
+
 		evalTaskName := strings.Replace(taskName, "-", "_", -1)
 		if _, ok := evalScope[evalTaskName]; ok {
 			continue
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
index dcf98a622..37948af01 100644
--- a/workflow/controller/operator.go
+++ b/workflow/controller/operator.go
@@ -338,7 +338,7 @@ func (woc *wfOperationCtx) operate() {
 		woc.globalParams[common.GlobalVarWorkflowFailures] = strconv.Quote(string(failedNodeBytes))
 
 		woc.log.Infof("Running OnExit handler: %s", woc.wfSpec.OnExit)
-		onExitNodeName := woc.wf.ObjectMeta.Name + ".onExit"
+		onExitNodeName := common.GenerateOnExitNodeName(woc.wf.ObjectMeta.Name)
 		onExitNode, err = woc.executeTemplate(onExitNodeName, &wfv1.WorkflowStep{Template: woc.wfSpec.OnExit}, tmplCtx, woc.wfSpec.Arguments, &executeTemplateOpts{onExitTemplate: true})
 		if err != nil {
 			// the error are handled in the callee so just log it.
@@ -451,15 +451,6 @@ func (woc *wfOperationCtx) setGlobalParameters(executionParameters wfv1.Argument
 	}
 }
 
-func (woc *wfOperationCtx) getNodeByName(nodeName string) *wfv1.NodeStatus {
-	nodeID := woc.wf.NodeID(nodeName)
-	node, ok := woc.wf.Status.Nodes[nodeID]
-	if !ok {
-		return nil
-	}
-	return &node
-}
-
 // persistUpdates will update a workflow with any updates made during workflow operation.
 // It also labels any pods as completed if we have extracted everything we need from it.
 // NOTE: a previous implementation used Patch instead of Update, but Patch does not work with
@@ -1379,7 +1370,7 @@ type executeTemplateOpts struct {
 func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.TemplateReferenceHolder, tmplCtx *templateresolution.Context, args wfv1.Arguments, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
 	woc.log.Debugf("Evaluating node %s: template: %s, boundaryID: %s", nodeName, common.GetTemplateHolderString(orgTmpl), opts.boundaryID)
 
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 
 	// Set templateScope from which the template resolution starts.
 	templateScope := tmplCtx.GetTemplateScope()
@@ -1541,11 +1532,11 @@ func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.Templat
 		}
 	}
 
-	node = woc.getNodeByName(node.Name)
+	node = woc.wf.GetNodeByName(node.Name)
 
 	// Swap the node back to retry node
 	if retryNodeName != "" {
-		retryNode := woc.getNodeByName(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(retryNodeName, orgTmpl, tmplCtx, args, opts)
 			if err != nil {
@@ -1723,7 +1714,7 @@ func (woc *wfOperationCtx) initializeNode(nodeName string, nodeType wfv1.NodeTyp
 
 // markNodePhase marks a node with the given phase, creating the node if necessary and handles timestamps
 func (woc *wfOperationCtx) markNodePhase(nodeName string, phase wfv1.NodePhase, message ...string) *wfv1.NodeStatus {
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	if node == nil {
 		panic(fmt.Sprintf("workflow '%s' node '%s' uninitialized when marking as %v: %s", woc.wf.Name, nodeName, phase, message))
 	}
@@ -1784,7 +1775,7 @@ func (woc *wfOperationCtx) markNodeError(nodeName string, err error) *wfv1.NodeS
 // markNodePending is a convenience method to mark a node and set the message from the error
 func (woc *wfOperationCtx) markNodePending(nodeName string, err error) *wfv1.NodeStatus {
 	woc.log.Infof("Mark node %s as Pending, due to: %+v", nodeName, err)
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	return woc.markNodePhase(nodeName, wfv1.NodePending, fmt.Sprintf("Pending %s", time.Since(node.StartedAt.Time)))
 }
 
@@ -1840,7 +1831,7 @@ func (woc *wfOperationCtx) checkParallelism(tmpl *wfv1.Template, node *wfv1.Node
 }
 
 func (woc *wfOperationCtx) executeContainer(nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	if node == nil {
 		node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypePod, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodePending)
 	} else if !isResubmitAllowed(tmpl) || !node.Pending() {
@@ -1982,7 +1973,7 @@ func getStepOrDAGTaskName(nodeName string) string {
 }
 
 func (woc *wfOperationCtx) executeScript(nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	if node != nil {
 		return node, nil
 	}
@@ -2247,7 +2238,7 @@ func (woc *wfOperationCtx) addChildNode(parent string, child string) {
 
 // executeResource is runs a kubectl command against a manifest
 func (woc *wfOperationCtx) executeResource(nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	if node != nil {
 		return node, nil
 	}
@@ -2283,7 +2274,7 @@ func (woc *wfOperationCtx) executeResource(nodeName string, templateScope string
 }
 
 func (woc *wfOperationCtx) executeSuspend(nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	if node == nil {
 		node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypeSuspend, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodePending)
 	}
@@ -2294,7 +2285,7 @@ func (woc *wfOperationCtx) executeSuspend(nodeName string, templateScope string,
 	var requeueTime *time.Time
 
 	if tmpl.Suspend.Duration != "" {
-		node := woc.getNodeByName(nodeName)
+		node := woc.wf.GetNodeByName(nodeName)
 		suspendDuration, err := parseStringToDuration(tmpl.Suspend.Duration)
 		if err != nil {
 			return node, err
@@ -2497,7 +2488,7 @@ func (woc *wfOperationCtx) createTemplateContext(scope wfv1.ResourceScope, resou
 func (woc *wfOperationCtx) runOnExitNode(templateRef, parentDisplayName, parentNodeName, boundaryID string, tmplCtx *templateresolution.Context) (bool, *wfv1.NodeStatus, error) {
 	if templateRef != "" && woc.wf.Spec.Shutdown.ShouldExecute(true) {
 		woc.log.Infof("Running OnExit handler: %s", templateRef)
-		onExitNodeName := parentDisplayName + ".onExit"
+		onExitNodeName := common.GenerateOnExitNodeName(parentDisplayName)
 		onExitNode, err := woc.executeTemplate(onExitNodeName, &wfv1.WorkflowStep{Template: templateRef}, tmplCtx, woc.wfSpec.Arguments, &executeTemplateOpts{
 			boundaryID:     boundaryID,
 			onExitTemplate: true,
diff --git a/workflow/controller/steps.go b/workflow/controller/steps.go
index 786cf97fa..06e057a56 100644
--- a/workflow/controller/steps.go
+++ b/workflow/controller/steps.go
@@ -32,7 +32,7 @@ type stepsContext struct {
 }
 
 func (woc *wfOperationCtx) executeSteps(nodeName string, tmplCtx *templateresolution.Context, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	if node == nil {
 		node = woc.initializeExecutableNode(nodeName, wfv1.NodeTypeSteps, templateScope, tmpl, orgTmpl, opts.boundaryID, wfv1.NodeRunning)
 	}
@@ -59,7 +59,7 @@ func (woc *wfOperationCtx) executeSteps(nodeName string, tmplCtx *templateresolu
 
 	for i, stepGroup := range tmpl.Steps {
 		sgNodeName := fmt.Sprintf("%s[%d]", nodeName, i)
-		if woc.getNodeByName(sgNodeName) == nil {
+		if woc.wf.GetNodeByName(sgNodeName) == nil {
 			_ = woc.initializeNode(sgNodeName, wfv1.NodeTypeStepGroup, stepTemplateScope, tmpl, stepsCtx.boundaryID, wfv1.NodeRunning)
 		} else {
 			_ = woc.markNodePhase(sgNodeName, wfv1.NodeRunning)
@@ -72,7 +72,7 @@ func (woc *wfOperationCtx) executeSteps(nodeName string, tmplCtx *templateresolu
 			// Otherwise connect all the outbound nodes of the previous step group as parents to
 			// the current step group node.
 			prevStepGroupName := fmt.Sprintf("%s[%d]", nodeName, i-1)
-			prevStepGroupNode := woc.getNodeByName(prevStepGroupName)
+			prevStepGroupNode := woc.wf.GetNodeByName(prevStepGroupName)
 			if len(prevStepGroupNode.Children) == 0 {
 				// corner case which connects an empty StepGroup (e.g. due to empty withParams) to
 				// the previous StepGroup node
@@ -105,7 +105,7 @@ func (woc *wfOperationCtx) executeSteps(nodeName string, tmplCtx *templateresolu
 		// Add all outputs of each step in the group to the scope
 		for _, step := range stepGroup.Steps {
 			childNodeName := fmt.Sprintf("%s.%s", sgNodeName, step.Name)
-			childNode := woc.getNodeByName(childNodeName)
+			childNode := woc.wf.GetNodeByName(childNodeName)
 			prefix := fmt.Sprintf("steps.%s", step.Name)
 			if childNode == nil {
 				// This happens when there was `withItem/withParam` expansion.
@@ -146,7 +146,7 @@ func (woc *wfOperationCtx) executeSteps(nodeName string, tmplCtx *templateresolu
 		return node, err
 	}
 	if outputs != nil {
-		node := woc.getNodeByName(nodeName)
+		node := woc.wf.GetNodeByName(nodeName)
 		node.Outputs = outputs
 		woc.addOutputsToGlobalScope(node.Outputs)
 		woc.wf.Status.Nodes[node.ID] = *node
@@ -160,7 +160,7 @@ func (woc *wfOperationCtx) updateOutboundNodes(nodeName string, tmpl *wfv1.Templ
 	// Find the last, initialized stepgroup node
 	var lastSGNode *wfv1.NodeStatus
 	for i := len(tmpl.Steps) - 1; i >= 0; i-- {
-		sgNode := woc.getNodeByName(fmt.Sprintf("%s[%d]", nodeName, i))
+		sgNode := woc.wf.GetNodeByName(fmt.Sprintf("%s[%d]", nodeName, i))
 		if sgNode != nil {
 			lastSGNode = sgNode
 			break
@@ -175,7 +175,7 @@ func (woc *wfOperationCtx) updateOutboundNodes(nodeName string, tmpl *wfv1.Templ
 		woc.log.Infof("Outbound nodes of %s is %s", childID, outboundNodeIDs)
 		outbound = append(outbound, outboundNodeIDs...)
 	}
-	node := woc.getNodeByName(nodeName)
+	node := woc.wf.GetNodeByName(nodeName)
 	woc.log.Infof("Outbound nodes of %s is %s", node.ID, outbound)
 	node.OutboundNodes = outbound
 	woc.wf.Status.Nodes[node.ID] = *node
@@ -184,7 +184,7 @@ func (woc *wfOperationCtx) updateOutboundNodes(nodeName string, tmpl *wfv1.Templ
 // executeStepGroup examines a list of parallel steps and executes them in parallel.
 // Handles referencing of variables in scope, expands `withItem` clauses, and evaluates `when` expressions
 func (woc *wfOperationCtx) executeStepGroup(stepGroup []wfv1.WorkflowStep, sgNodeName string, stepsCtx *stepsContext) *wfv1.NodeStatus {
-	node := woc.getNodeByName(sgNodeName)
+	node := woc.wf.GetNodeByName(sgNodeName)
 	if node.Fulfilled() {
 		woc.log.Debugf("Step group node %v already marked completed", node)
 		return node
@@ -221,7 +221,7 @@ func (woc *wfOperationCtx) executeStepGroup(stepGroup []wfv1.WorkflowStep, sgNod
 			return woc.markNodeError(sgNodeName, err)
 		}
 		if !proceed {
-			if woc.getNodeByName(childNodeName) == nil {
+			if woc.wf.GetNodeByName(childNodeName) == nil {
 				skipReason := fmt.Sprintf("when '%s' evaluated false", step.When)
 				woc.log.Infof("Skipping %s: %s", childNodeName, skipReason)
 				woc.initializeNode(childNodeName, wfv1.NodeTypeSkipped, stepTemplateScope, &step, stepsCtx.boundaryID, wfv1.NodeSkipped, skipReason)
@@ -249,7 +249,7 @@ func (woc *wfOperationCtx) executeStepGroup(stepGroup []wfv1.WorkflowStep, sgNod
 		}
 	}
 
-	node = woc.getNodeByName(sgNodeName)
+	node = woc.wf.GetNodeByName(sgNodeName)
 	// Return if not all children completed
 	completed := true
 	for _, childNodeID := range node.Children {
@@ -412,7 +412,7 @@ func (woc *wfOperationCtx) expandStepGroup(sgNodeName string, stepGroup []wfv1.W
 		if len(expandedStep) == 0 {
 			// Empty list
 			childNodeName := fmt.Sprintf("%s.%s", sgNodeName, step.Name)
-			if woc.getNodeByName(childNodeName) == nil {
+			if woc.wf.GetNodeByName(childNodeName) == nil {
 				stepTemplateScope := stepsCtx.tmplCtx.GetTemplateScope()
 				skipReason := fmt.Sprint("Skipped, empty params")
 				woc.log.Infof("Skipping %s: %s", childNodeName, skipReason)


Test patch:
diff --git a/workflow/controller/dag_test.go b/workflow/controller/dag_test.go
index 9cb21e162..ab72311c7 100644
--- a/workflow/controller/dag_test.go
+++ b/workflow/controller/dag_test.go
@@ -110,6 +110,8 @@ func TestSingleDependency(t *testing.T) {
 		// Mark the status of the pod according to the test
 		if _, ok := statusMap[status]; ok {
 			makePodsPhase(t, statusMap[status], controller.kubeclientset, wf.ObjectMeta.Namespace)
+		} else {
+			makePodsPhase(t, v1.PodPending, controller.kubeclientset, wf.ObjectMeta.Namespace)
 		}
 
 		woc.operate()
@@ -121,9 +123,9 @@ func TestSingleDependency(t *testing.T) {
 			}
 		}
 		assert.True(t, found)
-	}
-	if closer != nil {
-		closer()
+		if closer != nil {
+			closer()
+		}
 	}
 }
 
@@ -1275,7 +1277,7 @@ func TestTerminateDAGWithMaxDurationLimitExpiredAndMoreAttempts(t *testing.T) {
 
 	woc.operate()
 
-	retryNode := woc.getNodeByName("dag-diamond-dj7q5.A")
+	retryNode := woc.wf.GetNodeByName("dag-diamond-dj7q5.A")
 	if assert.NotNil(t, retryNode) {
 		assert.Equal(t, wfv1.NodeFailed, retryNode.Phase)
 		assert.Contains(t, retryNode.Message, "Max duration limit exceeded")
@@ -1461,18 +1463,12 @@ func TestRetryStrategyNodes(t *testing.T) {
 	woc := newWorkflowOperationCtx(wf, controller)
 
 	woc.operate()
-	retryNode := woc.getNodeByName("wf-retry-pol")
-	if assert.NotNil(t, retryNode) {
-		assert.Equal(t, wfv1.NodeRunning, retryNode.Phase)
-	}
-
-	woc.operate()
-	retryNode = woc.getNodeByName("wf-retry-pol")
+	retryNode := woc.wf.GetNodeByName("wf-retry-pol")
 	if assert.NotNil(t, retryNode) {
 		assert.Equal(t, wfv1.NodeFailed, retryNode.Phase)
 	}
 
-	onExitNode := woc.getNodeByName("wf-retry-pol.onExit")
+	onExitNode := woc.wf.GetNodeByName("wf-retry-pol.onExit")
 	if assert.NotNil(t, onExitNode) {
 		assert.Equal(t, wfv1.NodePending, onExitNode.Phase)
 	}
@@ -1631,12 +1627,12 @@ func TestOnExitDAGPhase(t *testing.T) {
 	woc := newWorkflowOperationCtx(wf, controller)
 
 	woc.operate()
-	retryNode := woc.getNodeByName("dag-diamond-88trp")
+	retryNode := woc.wf.GetNodeByName("dag-diamond-88trp")
 	if assert.NotNil(t, retryNode) {
 		assert.Equal(t, wfv1.NodeRunning, retryNode.Phase)
 	}
 
-	retryNode = woc.getNodeByName("B.onExit")
+	retryNode = woc.wf.GetNodeByName("B.onExit")
 	if assert.NotNil(t, retryNode) {
 		assert.Equal(t, wfv1.NodePending, retryNode.Phase)
 	}
@@ -1760,11 +1756,17 @@ func TestOnExitNonLeaf(t *testing.T) {
 	woc := newWorkflowOperationCtx(wf, controller)
 
 	woc.operate()
-	retryNode := woc.getNodeByName("step-2.onExit")
+	retryNode := woc.wf.GetNodeByName("step-2.onExit")
 	if assert.NotNil(t, retryNode) {
 		assert.Equal(t, wfv1.NodePending, retryNode.Phase)
 	}
-	retryNode = woc.getNodeByName("exit-handler-bug-example.step-3")
+
+	assert.Nil(t, woc.wf.GetNodeByName("exit-handler-bug-example.step-3"))
+
+	retryNode.Phase = wfv1.NodeSucceeded
+	woc.wf.Status.Nodes[retryNode.ID] = *retryNode
+	woc.operate()
+	retryNode = woc.wf.GetNodeByName("exit-handler-bug-example.step-3")
 	if assert.NotNil(t, retryNode) {
 		assert.Equal(t, wfv1.NodePending, retryNode.Phase)
 	}
diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
index fc0b6a3e4..49b1c52ca 100644
--- a/workflow/controller/operator_test.go
+++ b/workflow/controller/operator_test.go
@@ -198,7 +198,7 @@ func TestProcessNodesWithRetries(t *testing.T) {
 		woc.addChildNode(nodeName, childNode)
 	}
 
-	n := woc.getNodeByName(nodeName)
+	n := woc.wf.GetNodeByName(nodeName)
 	lastChild = getChildNodeIndex(n, woc.wf.Status.Nodes, -1)
 	assert.NotNil(t, lastChild)
 
@@ -220,14 +220,14 @@ func TestProcessNodesWithRetries(t *testing.T) {
 	woc.markNodePhase(lastChild.Name, wfv1.NodeFailed)
 	_, _, err = woc.processNodeRetries(n, retries, &executeTemplateOpts{})
 	assert.NoError(t, err)
-	n = woc.getNodeByName(nodeName)
+	n = woc.wf.GetNodeByName(nodeName)
 	assert.Equal(t, n.Phase, wfv1.NodeRunning)
 
 	// Add a third node that has failed.
 	childNode := "child-node-3"
 	woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.Template{}, "", wfv1.NodeFailed)
 	woc.addChildNode(nodeName, childNode)
-	n = woc.getNodeByName(nodeName)
+	n = woc.wf.GetNodeByName(nodeName)
 	n, _, err = woc.processNodeRetries(n, retries, &executeTemplateOpts{})
 	assert.NoError(t, err)
 	assert.Equal(t, n.Phase, wfv1.NodeFailed)
@@ -270,7 +270,7 @@ func TestProcessNodesWithRetriesOnErrors(t *testing.T) {
 		woc.addChildNode(nodeName, childNode)
 	}
 
-	n := woc.getNodeByName(nodeName)
+	n := woc.wf.GetNodeByName(nodeName)
 	lastChild = getChildNodeIndex(n, woc.wf.Status.Nodes, -1)
 	assert.NotNil(t, lastChild)
 
@@ -292,14 +292,14 @@ func TestProcessNodesWithRetriesOnErrors(t *testing.T) {
 	woc.markNodePhase(lastChild.Name, wfv1.NodeError)
 	_, _, err = woc.processNodeRetries(n, retries, &executeTemplateOpts{})
 	assert.NoError(t, err)
-	n = woc.getNodeByName(nodeName)
+	n = woc.wf.GetNodeByName(nodeName)
 	assert.Equal(t, n.Phase, wfv1.NodeRunning)
 
 	// Add a third node that has errored.
 	childNode := "child-node-3"
 	woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.Template{}, "", wfv1.NodeError)
 	woc.addChildNode(nodeName, childNode)
-	n = woc.getNodeByName(nodeName)
+	n = woc.wf.GetNodeByName(nodeName)
 	n, _, err = woc.processNodeRetries(n, retries, &executeTemplateOpts{})
 	assert.Nil(t, err)
 	assert.Equal(t, n.Phase, wfv1.NodeError)
@@ -343,7 +343,7 @@ func TestProcessNodesWithRetriesWithBackoff(t *testing.T) {
 	woc.initializeNode("child-node-1", wfv1.NodeTypePod, "", &wfv1.Template{}, "", wfv1.NodeRunning)
 	woc.addChildNode(nodeName, "child-node-1")
 
-	n := woc.getNodeByName(nodeName)
+	n := woc.wf.GetNodeByName(nodeName)
 	lastChild = getChildNodeIndex(n, woc.wf.Status.Nodes, -1)
 	assert.NotNil(t, lastChild)
 
@@ -398,7 +398,7 @@ func TestProcessNodesNoRetryWithError(t *testing.T) {
 		woc.addChildNode(nodeName, childNode)
 	}
 
-	n := woc.getNodeByName(nodeName)
+	n := woc.wf.GetNodeByName(nodeName)
 	lastChild = getChildNodeIndex(n, woc.wf.Status.Nodes, -1)
 	assert.NotNil(t, lastChild)
 
@@ -421,7 +421,7 @@ func TestProcessNodesNoRetryWithError(t *testing.T) {
 	woc.markNodePhase(lastChild.Name, wfv1.NodeError)
 	_, _, err = woc.processNodeRetries(n, retries, &executeTemplateOpts{})
 	assert.NoError(t, err)
-	n = woc.getNodeByName(nodeName)
+	n = woc.wf.GetNodeByName(nodeName)
 	assert.Equal(t, wfv1.NodeError, n.Phase)
 }
 
@@ -557,7 +557,7 @@ func TestBackoffMessage(t *testing.T) {
 	assert.NotNil(t, woc)
 	_, _, err := woc.loadExecutionSpec()
 	assert.NoError(t, err)
-	retryNode := woc.getNodeByName("retry-backoff-s69z6")
+	retryNode := woc.wf.GetNodeByName("retry-backoff-s69z6")
 
 	// Simulate backoff of 4 secods
 	firstNode := getChildNodeIndex(retryNode, woc.wf.Status.Nodes, 0)
@@ -3033,7 +3033,7 @@ func TestRetryNodeOutputs(t *testing.T) {
 	assert.NoError(t, err)
 	woc := newWorkflowOperationCtx(wf, controller)
 
-	retryNode := woc.getNodeByName("daemon-step-dvbnn[0].influx")
+	retryNode := woc.wf.GetNodeByName("daemon-step-dvbnn[0].influx")
 	assert.NotNil(t, retryNode)
 	fmt.Println(retryNode)
 	scope := &wfScope{
@@ -3637,7 +3637,7 @@ func TestNoOnExitWhenSkipped(t *testing.T) {
 
 	woc := newWoc(*wf)
 	woc.operate()
-	assert.Nil(t, woc.getNodeByName("B.onExit"))
+	assert.Nil(t, woc.wf.GetNodeByName("B.onExit"))
 }
 
 func TestGenerateNodeName(t *testing.T) {
@@ -3898,7 +3898,7 @@ func TestPropagateMaxDurationProcess(t *testing.T) {
 	woc.addChildNode(nodeName, childNode)
 
 	var opts executeTemplateOpts
-	n := woc.getNodeByName(nodeName)
+	n := woc.wf.GetNodeByName(nodeName)
 	_, _, err = woc.processNodeRetries(n, retries, &opts)
 	if assert.NoError(t, err) {
 		assert.Equal(t, n.StartedAt.Add(20*time.Second).Round(time.Second).String(), opts.executionDeadline.Round(time.Second).String())