Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

retryStrategy duration cannot be a string or else it defaults to 0 #1200

Closed
gavinmead opened this issue May 5, 2021 · 1 comment
Closed
Labels
bug Something isn't working

Comments

@gavinmead
Copy link

When a string representation of a duration is used on the retryStrategy.Duration field it defaults to 0.

The following yaml will not work since https://github.com/argoproj/argo-events/blob/master/common/retry.go#L56 looks at the Int64Value() only for the duration field

      retryStrategy:
        steps: 10
        duration: "2s"

This workaround can be used in the interim

      retryStrategy:
        steps: 10
        duration: 2000000000

To Reproduce
Steps to reproduce the behavior:
Created two tests in retry_test.go

func TestConnectDurationBug(t *testing.T) {

	start := time.Now()
	count := 4
	err := Connect(nil, func() error {
		if count == 0 {
			return nil
		} else {
			count--
			return fmt.Errorf("new error")
		}
	})
	end := time.Now()
	elapsed := end.Sub(start)
	assert.NoError(t, err)
	assert.Equal(t, 0, count)
	assert.True(t, elapsed >= 4 * time.Second) //Will fail as the duration defaults to 0
}

func TestConnectRetry(t *testing.T) {

	factor   := apicommon.NewAmount("1.0")
	jitter   := apicommon.NewAmount("1")
	duration := apicommon.FromInt64(1000000000) //This must be set as int64
	backoff := apicommon.Backoff{
		Duration: &duration,
		Factor:   &factor,
		Jitter:   &jitter,
		Steps:    5,
	}
	count := 4
	start := time.Now()
	err := Connect(&backoff, func() error {
		if count == 0 {
			return nil
		} else {
			count--
			return fmt.Errorf("new error")
		}
	})
	end := time.Now()
	elapsed := end.Sub(start)
	assert.NoError(t, err)
	assert.Equal(t, 0, count)
	assert.True(t, elapsed >= 4 * time.Second) //Will pass
}

Expected behavior
retries will backoff using a duration that is not 0

Environment (please complete the following information):

  • Kubernetes: v1.19.7
  • Argo Events: [e.g. v1.3.0]
@gavinmead gavinmead added the bug Something isn't working label May 5, 2021
whynowy added a commit to whynowy/argo-events that referenced this issue May 5, 2021
@whynowy
Copy link
Member

whynowy commented May 5, 2021

Good catch! Thanks!

@whynowy whynowy closed this as completed in 08b4fed May 6, 2021
whynowy added a commit that referenced this issue May 6, 2021
juliev0 pushed a commit to juliev0/argo-events that referenced this issue Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants