diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 9ede00046354..5ddf1cdfce00 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -1478,8 +1478,8 @@ func (woc *wfOperationCtx) assessNodeStatus(ctx context.Context, pod *apiv1.Pod, woc.wf.Status.MarkTaskResultComplete(nodeID) } - // if we are transitioning from Pending to a different state, clear out unchanged message - if old.Phase == wfv1.NodePending && new.Phase != wfv1.NodePending && old.Message == new.Message { + // if we are transitioning from Pending to a different state (except Fail or Error), clear out unchanged message + if old.Phase == wfv1.NodePending && new.Phase != wfv1.NodePending && new.Phase != wfv1.NodeFailed && new.Phase != wfv1.NodeError && old.Message == new.Message { new.Message = "" } diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 19463ab7d09e..298725327cf2 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -1554,11 +1554,12 @@ func TestStepsRetriesVariable(t *testing.T) { func TestAssessNodeStatus(t *testing.T) { const templateName = "whalesay" tests := []struct { - name string - pod *apiv1.Pod - daemon bool - node *wfv1.NodeStatus - want wfv1.NodePhase + name string + pod *apiv1.Pod + daemon bool + node *wfv1.NodeStatus + wantPhase wfv1.NodePhase + wantMessage string }{{ name: "pod pending", pod: &apiv1.Pod{ @@ -1566,8 +1567,9 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodPending, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodePending, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodePending, + wantMessage: "", }, { name: "pod succeeded", pod: &apiv1.Pod{ @@ -1575,8 +1577,9 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodSucceeded, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeSucceeded, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeSucceeded, + wantMessage: "", }, { name: "pod failed - daemoned", pod: &apiv1.Pod{ @@ -1584,9 +1587,10 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodFailed, }, }, - daemon: true, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeSucceeded, + daemon: true, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeSucceeded, + wantMessage: "", }, { name: "daemon, pod running, node failed", pod: &apiv1.Pod{ @@ -1594,9 +1598,10 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodRunning, }, }, - daemon: true, - node: &wfv1.NodeStatus{TemplateName: templateName, Phase: wfv1.NodeFailed}, - want: wfv1.NodeFailed, + daemon: true, + node: &wfv1.NodeStatus{TemplateName: templateName, Phase: wfv1.NodeFailed}, + wantPhase: wfv1.NodeFailed, + wantMessage: "", }, { name: "daemon, pod running, node succeeded", pod: &apiv1.Pod{ @@ -1604,9 +1609,10 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodRunning, }, }, - daemon: true, - node: &wfv1.NodeStatus{TemplateName: templateName, Phase: wfv1.NodeSucceeded}, - want: wfv1.NodeSucceeded, + daemon: true, + node: &wfv1.NodeStatus{TemplateName: templateName, Phase: wfv1.NodeSucceeded}, + wantPhase: wfv1.NodeSucceeded, + wantMessage: "", }, { name: "pod failed - not daemoned", pod: &apiv1.Pod{ @@ -1615,8 +1621,20 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodFailed, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeFailed, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeFailed, + wantMessage: "failed for some reason", + }, { + name: "pod failed - transition from node pending", + pod: &apiv1.Pod{ + Status: apiv1.PodStatus{ + Message: "failed for some reason", + Phase: apiv1.PodFailed, + }, + }, + node: &wfv1.NodeStatus{TemplateName: templateName, Phase: wfv1.NodePending, Message: "failed for some reason"}, + wantPhase: wfv1.NodeFailed, + wantMessage: "failed for some reason", }, { name: "pod failed - init container failed", pod: &apiv1.Pod{ @@ -1641,8 +1659,9 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodFailed, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeFailed, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeFailed, + wantMessage: "failed since init container failed", }, { name: "pod failed - init container failed but neither wait nor main containers are finished", pod: &apiv1.Pod{ @@ -1667,8 +1686,9 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodFailed, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeFailed, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeFailed, + wantMessage: "failed since init container failed", }, { name: "pod failed - init container with non-standard init container name failed but neither wait nor main containers are finished", pod: &apiv1.Pod{ @@ -1697,8 +1717,9 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodFailed, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeFailed, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeFailed, + wantMessage: "failed since init container failed", }, { name: "pod failed - wait container waiting but pod was set failed", pod: &apiv1.Pod{ @@ -1723,8 +1744,9 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodFailed, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeFailed, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeFailed, + wantMessage: "failed since wait contain waiting", }, { name: "pod running", pod: &apiv1.Pod{ @@ -1732,13 +1754,15 @@ func TestAssessNodeStatus(t *testing.T) { Phase: apiv1.PodRunning, }, }, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeRunning, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeRunning, + wantMessage: "", }, { - name: "default", - pod: &apiv1.Pod{}, - node: &wfv1.NodeStatus{TemplateName: templateName}, - want: wfv1.NodeError, + name: "default", + pod: &apiv1.Pod{}, + node: &wfv1.NodeStatus{TemplateName: templateName}, + wantPhase: wfv1.NodeError, + wantMessage: "Unexpected pod phase for : ", }} nonDaemonWf := wfv1.MustUnmarshalWorkflow(helloWorldWf) @@ -1755,7 +1779,8 @@ func TestAssessNodeStatus(t *testing.T) { defer cancel() woc := newWorkflowOperationCtx(wf, controller) got := woc.assessNodeStatus(context.TODO(), tt.pod, tt.node) - assert.Equal(t, tt.want, got.Phase) + assert.Equal(t, tt.wantPhase, got.Phase) + assert.Equal(t, tt.wantMessage, got.Message) }) } }