Skip to content

Commit

Permalink
fix: skip clear message when node transition from pending to fail
Browse files Browse the repository at this point in the history 
Signed-off-by: Tianchu Zhao <[email protected]>
  • Loading branch information
@tczhao
tczhao committed Jun 17, 2024
1 parent 669d4a4 commit fad81f4
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 37 deletions.
4 changes: 2 additions & 2 deletions workflow/controller/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1471,8 +1471,8 @@ func (woc *wfOperationCtx) assessNodeStatus(pod *apiv1.Pod, old *wfv1.NodeStatus
}
}

// 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), clear out unchanged message
if old.Phase == wfv1.NodePending && new.Phase != wfv1.NodePending && new.Phase != wfv1.NodeFailed && old.Message == new.Message {
new.Message = ""
}

Expand Down
95 changes: 60 additions & 35 deletions workflow/controller/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1555,59 +1555,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{
Expand All @@ -1616,8 +1622,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{
Expand All @@ -1642,8 +1660,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{
Expand All @@ -1668,8 +1687,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{
Expand Down Expand Up @@ -1698,8 +1718,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{
Expand All @@ -1724,22 +1745,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)
Expand All @@ -1756,7 +1780,8 @@ func TestAssessNodeStatus(t *testing.T) {
defer cancel()
woc := newWorkflowOperationCtx(wf, controller)
got := woc.assessNodeStatus(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)
})
}
}
Expand Down

0 comments on commit fad81f4

Please sign in to comment.