-
Notifications
You must be signed in to change notification settings - Fork 100
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add retry logic to UCP
GetAWSResourceWithPost handler (#8170)
# Description We've seen flaky functional test failures with AWS S3: #5963 This PR adds retries to the handler that I think is causing this 404 error. * Add `pkg/retry` directory for standard retries * Use `pkg/retry` in UCP `GetAWSResourceWithPost` handler ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: #7352 ## Contributor checklist Please verify that the PR meets the following requirements, where applicable: - [ ] An overview of proposed schema changes is included in a linked GitHub issue. - [ ] A design document PR is created in the [design-notes repository](https://github.com/radius-project/design-notes/), if new APIs are being introduced. - [ ] If applicable, design document has been reviewed and approved by Radius maintainers/approvers. - [ ] A PR for the [samples repository](https://github.com/radius-project/samples) is created, if existing samples are affected by the changes in this PR. - [ ] A PR for the [documentation repository](https://github.com/radius-project/docs) is created, if the changes in this PR affect the documentation or any user facing updates are made. - [ ] A PR for the [recipes repository](https://github.com/radius-project/recipes) is created, if existing recipes are affected by the changes in this PR. --------- Signed-off-by: willdavsmith <willdavsmith@gmail.com>
- Loading branch information
1 parent
a1782fc
commit e4db991
Showing
11 changed files
with
324 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package retry | ||
|
|
||
| import ( | ||
| "context" | ||
| "time" | ||
|
|
||
| "github.com/sethvargo/go-retry" | ||
| ) | ||
|
|
||
| const ( | ||
| defaultInterval = 1 * time.Second | ||
| defaultMaxRetries = 10 | ||
| defaultMaxDuration = 60 * time.Second | ||
| ) | ||
|
|
||
| // RetryConfig is the configuration for a retry operation. | ||
| type RetryConfig struct { | ||
| // BackoffStrategy is the backoff strategy to use. | ||
| BackoffStrategy retry.Backoff | ||
| } | ||
|
|
||
| // Retryer is a utility for retrying functions. | ||
| type Retryer struct { | ||
| config *RetryConfig | ||
| } | ||
|
|
||
| // NewNoOpRetryer creates a new Retryer that does not retry. | ||
| // This is useful for testing. | ||
| func NewNoOpRetryer() *Retryer { | ||
| b := retry.NewConstant(1 * time.Second) | ||
| b = retry.WithMaxRetries(0, b) | ||
|
|
||
| return NewRetryer(&RetryConfig{ | ||
| BackoffStrategy: b, | ||
| }) | ||
| } | ||
|
|
||
| // DefaultBackoffStrategy returns the default backoff strategy. | ||
| // The default backoff strategy is an exponential backoff with a maximum duration and maximum retries. | ||
| func DefaultBackoffStrategy() retry.Backoff { | ||
| b := retry.NewExponential(1 * time.Second) | ||
| b = retry.WithMaxDuration(defaultMaxDuration, b) | ||
| b = retry.WithMaxRetries(defaultMaxRetries, b) | ||
|
|
||
| return b | ||
| } | ||
|
|
||
| // NewDefaultRetryer creates a new Retryer with the default configuration. | ||
| // The default configuration is an exponential backoff with a maximum duration and maximum retries. | ||
| func NewDefaultRetryer() *Retryer { | ||
| return NewRetryer(&RetryConfig{ | ||
| BackoffStrategy: DefaultBackoffStrategy(), | ||
| }) | ||
| } | ||
|
|
||
| // NewRetryer creates a new Retryer with the given configuration. | ||
| // If either the config or config.BackoffStrategy are nil, | ||
| // the default configuration is used. | ||
| // The default configuration is an exponential backoff with a maximum duration and maximum retries. | ||
| func NewRetryer(config *RetryConfig) *Retryer { | ||
| retryConfig := &RetryConfig{} | ||
|
|
||
| if config != nil && config.BackoffStrategy != nil { | ||
| retryConfig.BackoffStrategy = config.BackoffStrategy | ||
| } else { | ||
| retryConfig.BackoffStrategy = DefaultBackoffStrategy() | ||
| } | ||
|
|
||
| return &Retryer{ | ||
| config: retryConfig, | ||
| } | ||
| } | ||
|
|
||
| // RetryFunc retries the given function with the backoff strategy. | ||
| func (r *Retryer) RetryFunc(ctx context.Context, f func(ctx context.Context) error) error { | ||
| return retry.Do(ctx, r.config.BackoffStrategy, f) | ||
| } | ||
|
|
||
| // RetryableError marks an error as retryable. | ||
| func RetryableError(err error) error { | ||
| return retry.RetryableError(err) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| package retry | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
| "time" | ||
|
|
||
| goretry "github.com/sethvargo/go-retry" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestNewNoOpRetryer(t *testing.T) { | ||
| retryer := NewNoOpRetryer() | ||
| require.NotNil(t, retryer) | ||
| require.NotNil(t, retryer.config) | ||
| require.NotNil(t, retryer.config.BackoffStrategy) | ||
|
|
||
| expectedBackoffStrategy := goretry.NewConstant(time.Second * 1) | ||
| expectedBackoffStrategy = goretry.WithMaxRetries(0, expectedBackoffStrategy) | ||
|
|
||
| require.IsType(t, expectedBackoffStrategy, retryer.config.BackoffStrategy) | ||
| } | ||
|
|
||
| func TestDefaultBackoffStrategy(t *testing.T) { | ||
| backoff := DefaultBackoffStrategy() | ||
| require.NotNil(t, backoff) | ||
| } | ||
|
|
||
| func TestNewDefaultRetryer(t *testing.T) { | ||
| retryer := NewDefaultRetryer() | ||
| require.NotNil(t, retryer) | ||
| require.NotNil(t, retryer.config) | ||
| require.NotNil(t, retryer.config.BackoffStrategy) | ||
|
|
||
| expectedBackoffStrategy := goretry.NewConstant(time.Second * 1) | ||
| expectedBackoffStrategy = goretry.WithMaxRetries(0, expectedBackoffStrategy) | ||
|
|
||
| require.IsType(t, expectedBackoffStrategy, retryer.config.BackoffStrategy) | ||
| } | ||
|
|
||
| func TestNewRetryer(t *testing.T) { | ||
| config := &RetryConfig{ | ||
| BackoffStrategy: goretry.NewConstant(1 * time.Second), | ||
| } | ||
| retryer := NewRetryer(config) | ||
| require.NotNil(t, retryer) | ||
| require.NotNil(t, retryer.config) | ||
|
|
||
| retryer = NewRetryer(nil) | ||
| require.NotNil(t, retryer) | ||
| require.NotNil(t, retryer.config) | ||
| require.NotNil(t, retryer.config.BackoffStrategy) | ||
|
|
||
| config = &RetryConfig{} | ||
| retryer = NewRetryer(config) | ||
| require.NotNil(t, retryer) | ||
| require.NotNil(t, retryer.config) | ||
| require.NotNil(t, retryer.config.BackoffStrategy) | ||
| } | ||
|
|
||
| func TestRetryer_RetryFunc(t *testing.T) { | ||
| retryer := NewDefaultRetryer() | ||
| ctx := context.Background() | ||
|
|
||
| // Test successful function | ||
| err := retryer.RetryFunc(ctx, func(ctx context.Context) error { | ||
| return nil | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| // Test retryable error | ||
| retryCount := 0 | ||
| err = retryer.RetryFunc(ctx, func(ctx context.Context) error { | ||
| retryCount++ | ||
| if retryCount < 3 { | ||
| return RetryableError(errors.New("retryable error")) | ||
| } | ||
| return nil | ||
| }) | ||
| require.NoError(t, err) | ||
| require.Equal(t, 3, retryCount) | ||
|
|
||
| // Test non-retryable error | ||
| err = retryer.RetryFunc(ctx, func(ctx context.Context) error { | ||
| return errors.New("non-retryable error") | ||
| }) | ||
| require.Error(t, err) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.