fix: Differentiate between Fulfilled and Completed

cmd/argo/commands/get.go

@@ -8,15 +8,16 @@ import (
 	"strings"
 	"text/tabwriter"
 
-	workflowpkg "github.com/argoproj/argo/pkg/apiclient/workflow"
-	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 	"github.com/argoproj/pkg/errors"
 	"github.com/argoproj/pkg/humanize"
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"sigs.k8s.io/yaml"
 
+	workflowpkg "github.com/argoproj/argo/pkg/apiclient/workflow"
+	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
+
 	"github.com/argoproj/argo/cmd/argo/commands/client"
 	"github.com/argoproj/argo/util/printer"
 	"github.com/argoproj/argo/workflow/util"

pkg/apis/workflow/v1alpha1/workflow_types.go

@@ -1189,11 +1189,17 @@ func (n Nodes) GetResourcesDuration() ResourcesDuration {
 	return i
 }
 
+// Completed returns whether a phase is completed, i.e. it finished execution or was skipped
 func (phase NodePhase) Completed() bool {
+	return phase.Executed() || phase == NodeSkipped
+}
+
+// Executed returns whether or not a phase finished executing.
+// Mainly, a skipped phase is not considered as having executed.
+func (phase NodePhase) Executed() bool {
 	return phase == NodeSucceeded ||
 		phase == NodeFailed ||
-		phase == NodeError ||
-		phase == NodeSkipped
+		phase == NodeError
 }
 
 // Completed returns whether or not the workflow has completed execution
@@ -1219,11 +1225,17 @@ func (ws WorkflowStatus) FinishTime() *metav1.Time {
 	return &ws.FinishedAt
 }
 
-// Completed returns whether or not the node has completed execution
+// Completed returns whether or not a node is completed, i.e. it finished execution or was skipped
 func (n NodeStatus) Completed() bool {
 	return n.Phase.Completed() || n.IsDaemoned() && n.Phase != NodePending
 }
 
+// Completed returns whether or not the node finished executing
+// Mainly, a Skipped node is not considered as having executed
+func (n NodeStatus) Executed() bool {
+	return n.Phase.Executed()
+}
+
 func (in *WorkflowStatus) AnyActiveSuspendNode() bool {
 	return in.Nodes.Any(func(node NodeStatus) bool { return node.IsActiveSuspendNode() })
 }

workflow/controller/dag.go

@@ -377,7 +377,7 @@ func (woc *wfOperationCtx) executeDAGTask(dagCtx *dagContext, taskName string) {
 
 	node := dagCtx.getTaskNode(taskName)
 	task := dagCtx.GetTask(taskName)
-	if node != nil && node.Completed() {
+	if node != nil && node.Executed() {
 		// Run the node's onExit node, if any.
 		hasOnExitNode, onExitNode, err := woc.runOnExitNode(task.Name, task.OnExit, dagCtx.boundaryID, dagCtx.tmplCtx)
 		if hasOnExitNode && (onExitNode == nil || !onExitNode.Completed() || err != nil) {

workflow/controller/steps.go

@@ -257,7 +257,7 @@ func (woc *wfOperationCtx) executeStepGroup(stepGroup []wfv1.WorkflowStep, sgNod
 		step := nodeSteps[childNode.Name]
 		if !childNode.Completed() {
 			completed = false
-		} else {
+		} else if childNode.Executed() {
 			hasOnExitNode, onExitNode, err := woc.runOnExitNode(step.Name, step.OnExit, stepsCtx.boundaryID, stepsCtx.tmplCtx)
 			if hasOnExitNode && (onExitNode == nil || !onExitNode.Completed() || err != nil) {
 				// The onExit node is either not complete or has errored out, return.