From 61be80ec1a211a0222b43f6d755a7082848f166e Mon Sep 17 00:00:00 2001 From: Dillen Padhiar <38965141+dpadhiar@users.noreply.github.com> Date: Mon, 1 Aug 2022 09:59:02 -0700 Subject: [PATCH] fix: retryStrategy.Limit is now read properly for backoff strategy. Fixes #9170. (#9213) * fix: retryStrategy.Limit is now read properly Signed-off-by: Dillen Padhiar * test: add test case for backoff Signed-off-by: Dillen Padhiar * chore: empty commit Signed-off-by: Dillen Padhiar * test: update test case to check for certain amount of retries Signed-off-by: Dillen Padhiar * test: added second case where limit is a string Signed-off-by: Dillen Padhiar Signed-off-by: Reddy --- test/e2e/retry_test.go | 58 ++++++++++++++++++++++++++++++++- workflow/controller/operator.go | 2 +- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/test/e2e/retry_test.go b/test/e2e/retry_test.go index 7a49eb80ed2a..5806965bf3d5 100644 --- a/test/e2e/retry_test.go +++ b/test/e2e/retry_test.go @@ -5,6 +5,7 @@ package e2e import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" @@ -22,7 +23,7 @@ func (s *RetryTestSuite) TestRetryLimit() { s.Given(). Workflow(` metadata: - generateName: test-backoff- + generateName: test-retry-limit- spec: entrypoint: main templates: @@ -48,6 +49,61 @@ spec: }) } +func (s *RetryTestSuite) TestRetryBackoff() { + s.Given(). + Workflow(` +metadata: + generateName: test-backoff-strategy- +spec: + entrypoint: main + templates: + - name: main + retryStrategy: + limit: '10' + backoff: + duration: 10s + maxDuration: 1m + container: + name: main + image: 'argoproj/argosay:v2' + args: [ exit, "1" ] +`). + When(). + SubmitWorkflow(). + WaitForWorkflow(time.Minute). + Then(). + ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) { + assert.Equal(t, wfv1.WorkflowPhase("Failed"), status.Phase) + assert.LessOrEqual(t, len(status.Nodes), 10) + }) + s.Given(). + Workflow(` +metadata: + generateName: test-backoff-strategy- +spec: + entrypoint: main + templates: + - name: main + retryStrategy: + limit: 10 + backoff: + duration: 10s + maxDuration: 1m + container: + name: main + image: 'argoproj/argosay:v2' + args: [ exit, "1" ] +`). + When(). + SubmitWorkflow(). + WaitForWorkflow(time.Minute). + Then(). + ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) { + assert.Equal(t, wfv1.WorkflowPhase("Failed"), status.Phase) + assert.LessOrEqual(t, len(status.Nodes), 10) + }) +} + func TestRetrySuite(t *testing.T) { suite.Run(t, new(RetryTestSuite)) } diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 822dfb0bb6fb..b64f5fd68602 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -898,7 +898,7 @@ func (woc *wfOperationCtx) processNodeRetries(node *wfv1.NodeStatus, retryStrate } // See if we have waited past the deadline - if time.Now().Before(waitingDeadline) && retryStrategy.Limit != nil && int32(len(node.Children)) <= retryStrategy.Limit.IntVal { + if time.Now().Before(waitingDeadline) && retryStrategy.Limit != nil && int32(len(node.Children)) <= int32(retryStrategy.Limit.IntValue()) { woc.requeueAfter(timeToWait) retryMessage := fmt.Sprintf("Backoff for %s", humanize.Duration(timeToWait)) return woc.markNodePhase(node.Name, node.Phase, retryMessage), false, nil