From 5f3fe5ccea6168d2111cc564bbef06557fddf690 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Fri, 3 May 2024 00:29:43 +0800 Subject: [PATCH 1/5] fix: ignore retry node when check descendant nodes. Signed-off-by: shuangkun --- test/e2e/testdata/retry-workflow-with-continueon.yaml | 3 +++ workflow/util/util.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/test/e2e/testdata/retry-workflow-with-continueon.yaml b/test/e2e/testdata/retry-workflow-with-continueon.yaml index 9d2ce1414860..a5176abe0ed9 100644 --- a/test/e2e/testdata/retry-workflow-with-continueon.yaml +++ b/test/e2e/testdata/retry-workflow-with-continueon.yaml @@ -4,6 +4,9 @@ metadata: name: retry-workflow-with-continueon spec: entrypoint: dag + retryStrategy: + limit: 2 + retryPolicy: OnError templates: - name: dag dag: diff --git a/workflow/util/util.go b/workflow/util/util.go index 8917c51c1d3c..5cadd22cbab9 100644 --- a/workflow/util/util.go +++ b/workflow/util/util.go @@ -971,7 +971,7 @@ func FormulateRetryWorkflow(ctx context.Context, wf *wfv1.Workflow, restartSucce log.Debugf("Reset %s node %s since it's a group node", node.Name, string(node.Phase)) continue } else { - if isDescendantNodeSucceeded(wf, node, nodeIDsToReset) { + if node.Type != wfv1.NodeTypeRetry && isDescendantNodeSucceeded(wf, node, nodeIDsToReset) { log.Debugf("Node %s remains as is since it has succeed child nodes.", node.Name) newWF.Status.Nodes.Set(node.ID, node) continue From dafa099beb1d5d90e0b473f3b1c6e7324af83bd2 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Fri, 3 May 2024 01:22:22 +0800 Subject: [PATCH 2/5] fix: test Signed-off-by: shuangkun --- test/e2e/cli_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/cli_test.go b/test/e2e/cli_test.go index 6c84c4e2c4d8..672a4b3d8aed 100644 --- a/test/e2e/cli_test.go +++ b/test/e2e/cli_test.go @@ -938,7 +938,7 @@ func (s *CLISuite) TestRetryWorkflowWithContinueOn() { Then(). ExpectWorkflow(func(t *testing.T, metadata *metav1.ObjectMeta, status *wfv1.WorkflowStatus) { workflowName = metadata.Name - assert.Equal(t, 6, len(status.Nodes)) + assert.Equal(t, 7, len(status.Nodes)) }). RunCli([]string{"retry", workflowName}, func(t *testing.T, output string, err error) { if assert.NoError(t, err, output) { @@ -954,7 +954,7 @@ func (s *CLISuite) TestRetryWorkflowWithContinueOn() { ExpectWorkflow(func(t *testing.T, metadata *metav1.ObjectMeta, status *wfv1.WorkflowStatus) { workflowName = metadata.Name assert.Equal(t, wfv1.WorkflowFailed, status.Phase) - assert.Equal(t, 6, len(status.Nodes)) + assert.Equal(t, 7, len(status.Nodes)) }). ExpectWorkflowNode(func(status wfv1.NodeStatus) bool { return strings.Contains(status.Name, "retry-workflow-with-continueon.success") From 638ed50bc071fc2f1bcf20a3408c022af9b904d4 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Fri, 3 May 2024 01:46:03 +0800 Subject: [PATCH 3/5] fix: test Signed-off-by: shuangkun --- test/e2e/cli_test.go | 2 +- test/e2e/testdata/retry-workflow-with-continueon.yaml | 6 +++--- workflow/util/util.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/cli_test.go b/test/e2e/cli_test.go index 672a4b3d8aed..95c12c39ff6b 100644 --- a/test/e2e/cli_test.go +++ b/test/e2e/cli_test.go @@ -957,7 +957,7 @@ func (s *CLISuite) TestRetryWorkflowWithContinueOn() { assert.Equal(t, 7, len(status.Nodes)) }). ExpectWorkflowNode(func(status wfv1.NodeStatus) bool { - return strings.Contains(status.Name, "retry-workflow-with-continueon.success") + return strings.Contains(status.Name, ".success") }, func(t *testing.T, status *wfv1.NodeStatus, pod *corev1.Pod) { assert.Equal(t, 2, len(status.Children)) }) diff --git a/test/e2e/testdata/retry-workflow-with-continueon.yaml b/test/e2e/testdata/retry-workflow-with-continueon.yaml index a5176abe0ed9..43a3b5670078 100644 --- a/test/e2e/testdata/retry-workflow-with-continueon.yaml +++ b/test/e2e/testdata/retry-workflow-with-continueon.yaml @@ -4,11 +4,11 @@ metadata: name: retry-workflow-with-continueon spec: entrypoint: dag - retryStrategy: - limit: 2 - retryPolicy: OnError templates: - name: dag + retryStrategy: + limit: 2 + retryPolicy: OnError dag: failFast: false tasks: diff --git a/workflow/util/util.go b/workflow/util/util.go index 5cadd22cbab9..01f2c97bf65d 100644 --- a/workflow/util/util.go +++ b/workflow/util/util.go @@ -971,7 +971,7 @@ func FormulateRetryWorkflow(ctx context.Context, wf *wfv1.Workflow, restartSucce log.Debugf("Reset %s node %s since it's a group node", node.Name, string(node.Phase)) continue } else { - if node.Type != wfv1.NodeTypeRetry && isDescendantNodeSucceeded(wf, node, nodeIDsToReset) { + if node.Type == wfv1.NodeTypePod && isDescendantNodeSucceeded(wf, node, nodeIDsToReset) { log.Debugf("Node %s remains as is since it has succeed child nodes.", node.Name) newWF.Status.Nodes.Set(node.ID, node) continue From 3d532da93831d22e0a04615c1aef49a3db51d88b Mon Sep 17 00:00:00 2001 From: shuangkun Date: Wed, 8 May 2024 10:56:45 +0800 Subject: [PATCH 4/5] fix: test Signed-off-by: shuangkun --- workflow/util/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/util/util.go b/workflow/util/util.go index 01f2c97bf65d..5cadd22cbab9 100644 --- a/workflow/util/util.go +++ b/workflow/util/util.go @@ -971,7 +971,7 @@ func FormulateRetryWorkflow(ctx context.Context, wf *wfv1.Workflow, restartSucce log.Debugf("Reset %s node %s since it's a group node", node.Name, string(node.Phase)) continue } else { - if node.Type == wfv1.NodeTypePod && isDescendantNodeSucceeded(wf, node, nodeIDsToReset) { + if node.Type != wfv1.NodeTypeRetry && isDescendantNodeSucceeded(wf, node, nodeIDsToReset) { log.Debugf("Node %s remains as is since it has succeed child nodes.", node.Name) newWF.Status.Nodes.Set(node.ID, node) continue From 6a31bf298353d18eed96692c0e4ddcbfdee72fb4 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Wed, 8 May 2024 18:11:30 +0800 Subject: [PATCH 5/5] fix: Empty-Commit Signed-off-by: shuangkun