fix: skip clear message when node transition from pending to fail. Fixes

 #13200 (#13201)

Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>

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 = ""
  }
 

workflow/controller/operator_test.go

@@ -1554,59 +1554,65 @@ 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{
  Status: apiv1.PodStatus{
  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{
  Status: apiv1.PodStatus{
  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{
  Status: apiv1.PodStatus{
  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{
  Status: apiv1.PodStatus{
  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{
  Status: apiv1.PodStatus{
  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,22 +1744,25 @@ 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{
  Status: apiv1.PodStatus{
  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)
  })
  }
 }