Omitted NodePhase -> NodeType

cmd/argo/commands/get.go

@@ -403,7 +403,7 @@ func filterNode(node wfv1.NodeStatus, getArgs getFlags) (bool, bool) {
 		return true, false
 	} else if node.Type == wfv1.NodeTypeStepGroup {
 		return true, true
-	} else if node.Type == wfv1.NodeTypeSkipped && node.Phase == wfv1.NodeOmitted {
+	} else if node.Type == wfv1.NodeTypeOmitted {
 		return true, false
 	} else if !getArgs.shouldPrint(node) {
 		return true, false

pkg/apis/workflow/v1alpha1/workflow_types.go

@@ -39,7 +39,6 @@ const (
 	NodeSkipped   NodePhase = "Skipped"
 	NodeFailed    NodePhase = "Failed"
 	NodeError     NodePhase = "Error"
-	NodeOmitted   NodePhase = "Omitted"
 )
 
 // NodeType is the type of a node
@@ -55,6 +54,7 @@ const (
 	NodeTypeRetry     NodeType = "Retry"
 	NodeTypeSkipped   NodeType = "Skipped"
 	NodeTypeSuspend   NodeType = "Suspend"
+	NodeTypeOmitted   NodeType = "Omitted"
 )
 
 // PodGCStrategy is the strategy when to delete completed pods for GC.
@@ -1217,7 +1217,7 @@ func (n Nodes) GetResourcesDuration() ResourcesDuration {
 
 // Fulfilled returns whether a phase is fulfilled, i.e. it completed execution or was skipped or omitted
 func (phase NodePhase) Fulfilled() bool {
-	return phase.Completed() || phase == NodeSkipped || phase == NodeOmitted
+	return phase.Completed() || phase == NodeSkipped
 }
 
 // Completed returns whether or not a phase completed. Notably, a skipped phase is not considered as having completed

test/e2e/functional_test.go

@@ -477,7 +477,7 @@ func (s *FunctionalSuite) TestDAGDepends() {
 			assert.Equal(t, wfv1.NodeSucceeded, nodeStatus.Phase)
 			nodeStatus = status.Nodes.FindByDisplayName("should-not-execute")
 			assert.NotNil(t, nodeStatus)
-			assert.Equal(t, wfv1.NodeOmitted, nodeStatus.Phase)
+			assert.Equal(t, wfv1.NodeFailed, nodeStatus.Phase)
 			nodeStatus = status.Nodes.FindByDisplayName("should-execute-3")
 			assert.NotNil(t, nodeStatus)
 			assert.Equal(t, wfv1.NodeSucceeded, nodeStatus.Phase)

ui/src/app/workflows/components/workflow-dag/workflow-dag.scss

@@ -71,10 +71,6 @@
       fill: $argo-color-gray-4;
     }
 
-    &--omitted {
-      fill: $argo-color-gray-4;
-    }
-
     &--suspended {
       fill: $argo-color-gray-4;
     }

ui/src/app/workflows/components/workflow-dag/workflow-dag.tsx

@@ -1,7 +1,7 @@
 import * as classNames from 'classnames';
 import * as React from 'react';
 
-import {NODE_PHASE, NodePhase, NodeStatus} from '../../../../models';
+import {NodePhase, NodeStatus} from '../../../../models';
 import {Loading} from '../../../shared/components/loading';
 import {Utils} from '../../../shared/utils';
 import {CoffmanGrahamSorter} from './graph/coffman-graham-sorter';
@@ -79,14 +79,6 @@ export class WorkflowDag extends React.Component<WorkflowDagProps, WorkflowDagRe
             d='M500.5 231.4l-192-160C287.9 54.3 256 68.6 256 96v320c0 27.4 31.9 41.8 52.5 24.6l192-160c15.3-12.8 15.3-36.4 0-49.2zm-256 0l-192-160C31.9 54.3 0 68.6 0 96v320c0 27.4 31.9 41.8 52.5 24.6l192-160c15.3-12.8 15.3-36.4 0-49.2z'
                     />
                 );
-            case 'Omitted':
-                return (
-                    <path
-                        fill='currentColor'
-                        // tslint:disable-next-line
-                        d='M500.5 231.4l-192-160C287.9 54.3 256 68.6 256 96v320c0 27.4 31.9 41.8 52.5 24.6l192-160c15.3-12.8 15.3-36.4 0-49.2zm-256 0l-192-160C31.9 54.3 0 68.6 0 96v320c0 27.4 31.9 41.8 52.5 24.6l192-160c15.3-12.8 15.3-36.4 0-49.2z'
-                    />
-                );
             case 'Succeeded':
                 return (
                     <path
@@ -160,7 +152,6 @@ export class WorkflowDag extends React.Component<WorkflowDagProps, WorkflowDagRe
                 'phase:Running',
                 'phase:Succeeded',
                 'phase:Skipped',
-                'phase:Omitted',
                 'phase:Failed',
                 'phase:Error',
                 'type:Pod',
@@ -242,15 +233,15 @@ export class WorkflowDag extends React.Component<WorkflowDagProps, WorkflowDagRe
     private prepareGraph() {
         const nodes = Object.values(this.props.nodes)
             .filter(node => !!node)
-            .filter(node => node.phase !== NODE_PHASE.OMITTED)
+            .filter(node => node.type !== 'Omitted')
             .map(node => node.id);
         const edges = Object.values(this.props.nodes)
             .filter(node => !!node)
             .map(node =>
                 (node.children || [])
                     // we can get outbound nodes, but no node
                     .filter(childId => this.props.nodes[childId])
-                    .filter(childId => this.props.nodes[childId].phase !== NODE_PHASE.OMITTED)
+                    .filter(childId => this.props.nodes[childId].type !== 'Omitted')
                     .map(childId => ({v: node.id, w: childId}))
             )
             .reduce((a, b) => a.concat(b));

ui/src/app/workflows/components/workflow-timeline/workflow-timeline.scss

@@ -48,10 +48,6 @@
         &--skipped {
             background-color: $argo-color-gray-4;
         }
-
-        &--omitted {
-            background-color: $argo-color-gray-4;
-        }
     }
 
     &__start-line {

ui/src/app/workflows/components/workflow-timeline/workflow-timeline.tsx

@@ -128,7 +128,7 @@ export class WorkflowTimeline extends React.Component<WorkflowTimelineProps, Wor
     }
 
     private ensureRunningWorkflowRefreshing(workflow: models.Workflow) {
-        const completedPhases = [models.NODE_PHASE.ERROR, models.NODE_PHASE.SUCCEEDED, models.NODE_PHASE.SKIPPED, models.NODE_PHASE.OMITTED, models.NODE_PHASE.FAILED];
+        const completedPhases = [models.NODE_PHASE.ERROR, models.NODE_PHASE.SUCCEEDED, models.NODE_PHASE.SKIPPED, models.NODE_PHASE.FAILED];
         const isCompleted = workflow && workflow.status && completedPhases.indexOf(workflow.status.phase) > -1;
         if (!this.refreshSubscription && !isCompleted) {
             this.refreshSubscription = Observable.interval(1000).subscribe(() => {

ui/src/models/workflows.ts

@@ -589,7 +589,7 @@ export interface Workflow {
     status?: WorkflowStatus;
 }
 
-export type NodeType = 'Pod' | 'Steps' | 'StepGroup' | 'DAG' | 'Retry' | 'Skipped' | 'TaskGroup' | 'Suspend';
+export type NodeType = 'Pod' | 'Steps' | 'StepGroup' | 'DAG' | 'Retry' | 'Skipped' | 'TaskGroup' | 'Suspend' | 'Omitted';
 
 export interface NodeStatus {
     /**
@@ -939,16 +939,15 @@ export interface WorkflowStep {
     templateRef?: TemplateRef;
 }
 
-export type NodePhase = 'Pending' | 'Running' | 'Succeeded' | 'Skipped' | 'Failed' | 'Error' | 'Omitted';
+export type NodePhase = 'Pending' | 'Running' | 'Succeeded' | 'Skipped' | 'Failed' | 'Error';
 
 export const NODE_PHASE = {
     PENDING: 'Pending',
     RUNNING: 'Running',
     SUCCEEDED: 'Succeeded',
     SKIPPED: 'Skipped',
     FAILED: 'Failed',
-    ERROR: 'Error',
-    OMITTED: 'Omitted'
+    ERROR: 'Error'
 };
 
 export type ResourceScope = 'local' | 'namespaced' | 'cluster';

workflow/controller/dag.go

@@ -171,11 +171,8 @@ func (d *dagContext) assessDAGPhase(targetTasks []string, nodes wfv1.Nodes) wfv1
 				break
 			}
 		} else if depNode.Failed() {
-			if depNode.Phase == wfv1.NodeError {
-				result = wfv1.NodeError
-			}
-			result = wfv1.NodeFailed
-			// If failFast is enabled, don't check and see if other target tasks are completed and fail now
+			result = depNode.Phase
+			// If failFast is enabled, don't check to see if other target tasks are complete and fail now instead
 			if failFast {
 				break
 			}
@@ -353,7 +350,15 @@ func (woc *wfOperationCtx) executeDAGTask(dagCtx *dagContext, taskName string) {
 		}
 		if !execute {
 			// Given the results of this node's dependencies, this node should not be executed. Mark it omitted
-			woc.initializeNode(nodeName, wfv1.NodeTypeSkipped, dagTemplateScope, task, dagCtx.boundaryID, wfv1.NodeOmitted, "omitted: depends condition not met")
+			// We still want to preserve and pass down the phase of the parent nodes for use in determining the overall
+			// DAG's phase.
+			omittedNodePhase := wfv1.NodeSucceeded
+			for _, parentTaskName := range dagCtx.GetTaskDependencies(taskName) {
+				if parentNode := dagCtx.getTaskNode(parentTaskName); parentNode.Failed() {
+					omittedNodePhase = parentNode.Phase
+				}
+			}
+			woc.initializeNode(nodeName, wfv1.NodeTypeOmitted, dagTemplateScope, task, dagCtx.boundaryID, omittedNodePhase, "omitted: depends condition not met")
 			connectDependencies(nodeName)
 			return
 		}