Skip to content

Commit e20c92b

Browse files
matt-FFFFFFmatt-FFFFFF
committed
fix(retry): add WithMaxElapsedTime to retry options and refactor to use functional options
1 parent 327514e commit e20c92b

12 files changed

+244
-132
lines changed

‎internal/clients/data_plane_client_test.go‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/Azure/terraform-provider-azapi/internal/clients"
1010
"github.com/Azure/terraform-provider-azapi/internal/services/parse"
11+
"github.com/cenkalti/backoff/v4"
1112
"github.com/stretchr/testify/assert"
1213
)
1314

@@ -27,9 +28,16 @@ import (
2728
//
2829
// We check these timings are expected using the assert.InDeltaSlice function.
2930
func TestRetryDataPlaneClient(t *testing.T) {
31+
t.Parallel()
3032
mock := NewMockDataPlaneClient(t, nil, nil, 3, errors.New("retry error"))
31-
bkof, errRegExps := clients.NewRetryableErrors(1, 30, 2, 0.0, []string{"retry error"})
32-
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
33+
bkof := backoff.NewExponentialBackOff(
34+
backoff.WithInitialInterval(1*time.Second),
35+
backoff.WithMaxInterval(30*time.Second),
36+
backoff.WithMultiplier(2),
37+
backoff.WithRandomizationFactor(0.0),
38+
)
39+
rexs := clients.StringSliceToRegexpSliceMust([]string{"retry error"})
40+
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, rexs, nil, nil)
3341
_, err := retryClient.Get(context.Background(), parse.DataPlaneResourceId{}, clients.DefaultRequestOptions())
3442
assert.NoError(t, err)
3543
assert.Equal(t, 3, mock.requestCount)
@@ -47,36 +55,64 @@ func TestRetryDataPlaneClient(t *testing.T) {
4755
}
4856

4957
func TestRetryDataPlaneClientRegexp(t *testing.T) {
58+
t.Parallel()
5059
mock := NewMockDataPlaneClient(t, nil, nil, 3, errors.New("retry error"))
51-
bkof, errRegExps := clients.NewRetryableErrors(1, 5, 1.5, 0.0, []string{"^retry"})
52-
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
60+
bkof := backoff.NewExponentialBackOff(
61+
backoff.WithInitialInterval(1*time.Second),
62+
backoff.WithMaxInterval(5*time.Second),
63+
backoff.WithMultiplier(1.5),
64+
backoff.WithRandomizationFactor(0.0),
65+
)
66+
rexs := clients.StringSliceToRegexpSliceMust([]string{"^retry"})
67+
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, rexs, nil, nil)
5368
_, err := retryClient.Get(context.Background(), parse.DataPlaneResourceId{}, clients.DefaultRequestOptions())
5469
assert.NoError(t, err)
5570
assert.Equal(t, 3, mock.RequestCount())
5671
}
5772

5873
func TestRetryDataPlaneClientMultiRegexp(t *testing.T) {
74+
t.Parallel()
5975
mock := NewMockDataPlaneClient(t, nil, nil, 3, errors.New("retry error"))
60-
bkof, errRegExps := clients.NewRetryableErrors(1, 5, 1.5, 0.0, []string{"nomatch", "^retry"})
61-
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
76+
bkof := backoff.NewExponentialBackOff(
77+
backoff.WithInitialInterval(1*time.Second),
78+
backoff.WithMaxInterval(5*time.Second),
79+
backoff.WithMultiplier(1.5),
80+
backoff.WithRandomizationFactor(0.0),
81+
)
82+
rexs := clients.StringSliceToRegexpSliceMust([]string{"nomatch", "^retry"})
83+
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, rexs, nil, nil)
6284
_, err := retryClient.Get(context.Background(), parse.DataPlaneResourceId{}, clients.DefaultRequestOptions())
6385
assert.NoError(t, err)
6486
assert.Equal(t, 3, mock.RequestCount())
6587
}
6688

6789
func TestRetryDataPlaneClientMultiRegexpNoMatchWithPermError(t *testing.T) {
90+
t.Parallel()
6891
mock := NewMockDataPlaneClient(t, nil, errors.New("perm error"), 3, errors.New("retry error"))
69-
bkof, errRegExps := clients.NewRetryableErrors(1, 5, 1.5, 0.0, []string{"retry"})
70-
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
92+
bkof := backoff.NewExponentialBackOff(
93+
backoff.WithInitialInterval(1*time.Second),
94+
backoff.WithMaxInterval(5*time.Second),
95+
backoff.WithMultiplier(1.5),
96+
backoff.WithRandomizationFactor(0.0),
97+
)
98+
rexs := clients.StringSliceToRegexpSliceMust([]string{"retry"})
99+
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, rexs, nil, nil)
71100
_, err := retryClient.Get(context.Background(), parse.DataPlaneResourceId{}, clients.DefaultRequestOptions())
72101
assert.ErrorContains(t, err, "perm error")
73102
assert.Equal(t, 3, mock.RequestCount())
74103
}
75104

76105
func TestRetryDataPlaneClientContextDeadline(t *testing.T) {
106+
t.Parallel()
77107
mock := NewMockDataPlaneClient(t, nil, nil, 3, errors.New("retry error"))
78-
bkof, errRegExps := clients.NewRetryableErrors(60, 60, 1.5, 0.0, []string{"^retry"})
79-
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
108+
bkof := backoff.NewExponentialBackOff(
109+
backoff.WithInitialInterval(60*time.Second),
110+
backoff.WithMaxInterval(60*time.Second),
111+
backoff.WithMultiplier(1.5),
112+
backoff.WithRandomizationFactor(0.0),
113+
)
114+
rexs := clients.StringSliceToRegexpSliceMust([]string{"^retry"})
115+
retryClient := clients.NewDataPlaneClientRetryableErrors(mock, bkof, rexs, nil, nil)
80116
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
81117
defer cancel()
82118
start := time.Now()

‎internal/clients/resource_client.go‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,20 +84,30 @@ func NewResourceClient(credential azcore.TokenCredential, opt *arm.ClientOptions
8484
}, nil
8585
}
8686

87-
// NewRetryableErrors creates the backoff and error regexs for retryable errors.
88-
func NewRetryableErrors(intervalSeconds, maxIntervalSeconds int, multiplier, randomizationFactor float64, errorRegexs []string) (*backoff.ExponentialBackOff, []regexp.Regexp) {
89-
bkof := backoff.NewExponentialBackOff(
90-
backoff.WithInitialInterval(time.Duration(intervalSeconds)*time.Second),
91-
backoff.WithRandomizationFactor(randomizationFactor),
92-
backoff.WithMaxInterval(time.Duration(maxIntervalSeconds)*time.Second),
93-
backoff.WithRandomizationFactor(randomizationFactor),
94-
backoff.WithMultiplier(multiplier),
95-
)
96-
res := make([]regexp.Regexp, len(errorRegexs))
97-
for i, e := range errorRegexs {
98-
res[i] = *regexp.MustCompile(e) // MustCompile as schema has custom validation so we know it's valid
99-
}
100-
return bkof, res
87+
// // NewRetryableErrors creates the backoff and error regexs for retryable errors.
88+
// func NewRetryableErrors(intervalSeconds, maxIntervalSeconds int, multiplier, randomizationFactor float64, errorRegexs []string) (*backoff.ExponentialBackOff, []regexp.Regexp) {
89+
// bkof := backoff.NewExponentialBackOff(
90+
// backoff.WithInitialInterval(time.Duration(intervalSeconds)*time.Second),
91+
// backoff.WithRandomizationFactor(randomizationFactor),
92+
// backoff.WithMaxInterval(time.Duration(maxIntervalSeconds)*time.Second),
93+
// backoff.WithRandomizationFactor(randomizationFactor),
94+
// backoff.WithMultiplier(multiplier),
95+
// )
96+
// res := make([]regexp.Regexp, len(errorRegexs))
97+
// for i, e := range errorRegexs {
98+
// res[i] = *regexp.MustCompile(e) // MustCompile as schema has custom validation so we know it's valid
99+
// }
100+
// return bkof, res
101+
// }
102+
103+
// StringSliceToRegexpSliceMust converts a slice of strings to a slice of regexps.
104+
// It panics if any of the strings are invalid regexps.
105+
func StringSliceToRegexpSliceMust(ss []string) []regexp.Regexp {
106+
res := make([]regexp.Regexp, len(ss))
107+
for i, e := range ss {
108+
res[i] = *regexp.MustCompile(e)
109+
}
110+
return res
101111
}
102112

103113
// WithRetry configures the retryable errors for the client.

‎internal/clients/resource_client_test.go‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"time"
88

99
"github.com/Azure/terraform-provider-azapi/internal/clients"
10+
"github.com/cenkalti/backoff/v4"
1011
"github.com/stretchr/testify/assert"
1112
)
1213

@@ -26,9 +27,16 @@ import (
2627
//
2728
// We check these timings are expected using the assert.InDeltaSlice function.
2829
func TestRetryClient(t *testing.T) {
30+
t.Parallel()
2931
mock := NewMockResourceClient(t, nil, nil, 3, errors.New("retry error"))
30-
bkof, errRegExps := clients.NewRetryableErrors(1, 30, 2, 0.0, []string{"retry error"})
31-
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
32+
rexs := clients.StringSliceToRegexpSliceMust([]string{"retry error"})
33+
bkof := backoff.NewExponentialBackOff(
34+
backoff.WithInitialInterval(1*time.Second),
35+
backoff.WithMaxInterval(30*time.Second),
36+
backoff.WithMultiplier(2),
37+
backoff.WithRandomizationFactor(0.0),
38+
)
39+
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, rexs, nil, nil)
3240
_, err := retryClient.Get(context.Background(), "", "", clients.DefaultRequestOptions())
3341
assert.NoError(t, err)
3442
assert.Equal(t, 3, mock.requestCount)
@@ -46,36 +54,64 @@ func TestRetryClient(t *testing.T) {
4654
}
4755

4856
func TestRetryClientRegexp(t *testing.T) {
57+
t.Parallel()
4958
mock := NewMockResourceClient(t, nil, nil, 3, errors.New("retry error"))
50-
bkof, errRegExps := clients.NewRetryableErrors(1, 5, 1.5, 0.0, []string{"^retry"})
51-
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
59+
rexs := clients.StringSliceToRegexpSliceMust([]string{"^retry"})
60+
bkof := backoff.NewExponentialBackOff(
61+
backoff.WithInitialInterval(1*time.Second),
62+
backoff.WithMaxInterval(5*time.Second),
63+
backoff.WithMultiplier(1.5),
64+
backoff.WithRandomizationFactor(0.0),
65+
)
66+
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, rexs, nil, nil)
5267
_, err := retryClient.Get(context.Background(), "", "", clients.DefaultRequestOptions())
5368
assert.NoError(t, err)
5469
assert.Equal(t, 3, mock.RequestCount())
5570
}
5671

5772
func TestRetryClientMultiRegexp(t *testing.T) {
73+
t.Parallel()
5874
mock := NewMockResourceClient(t, nil, nil, 3, errors.New("retry error"))
59-
bkof, errRegExps := clients.NewRetryableErrors(1, 5, 1.5, 0.0, []string{"nomatch", "^retry"})
60-
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
75+
rexs := clients.StringSliceToRegexpSliceMust([]string{"nomatch", "^retry"})
76+
bkof := backoff.NewExponentialBackOff(
77+
backoff.WithInitialInterval(1*time.Second),
78+
backoff.WithMaxInterval(5*time.Second),
79+
backoff.WithMultiplier(1.5),
80+
backoff.WithRandomizationFactor(0.0),
81+
)
82+
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, rexs, nil, nil)
6183
_, err := retryClient.Get(context.Background(), "", "", clients.DefaultRequestOptions())
6284
assert.NoError(t, err)
6385
assert.Equal(t, 3, mock.RequestCount())
6486
}
6587

6688
func TestRetryClientMultiRegexpNoMatchWithPermError(t *testing.T) {
89+
t.Parallel()
6790
mock := NewMockResourceClient(t, nil, errors.New("perm error"), 3, errors.New("retry error"))
68-
bkof, errRegExps := clients.NewRetryableErrors(1, 5, 1.5, 0.0, []string{"retry"})
69-
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
91+
rexs := clients.StringSliceToRegexpSliceMust([]string{"retry"})
92+
bkof := backoff.NewExponentialBackOff(
93+
backoff.WithInitialInterval(1*time.Second),
94+
backoff.WithMaxInterval(5*time.Second),
95+
backoff.WithMultiplier(1.5),
96+
backoff.WithRandomizationFactor(0.0),
97+
)
98+
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, rexs, nil, nil)
7099
_, err := retryClient.Get(context.Background(), "", "", clients.DefaultRequestOptions())
71100
assert.ErrorContains(t, err, "perm error")
72101
assert.Equal(t, 3, mock.RequestCount())
73102
}
74103

75104
func TestRetryClientContextDeadline(t *testing.T) {
105+
t.Parallel()
76106
mock := NewMockResourceClient(t, nil, nil, 3, errors.New("retry error"))
77-
bkof, errRegExps := clients.NewRetryableErrors(60, 60, 1.5, 0.0, []string{"^retry"})
78-
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, errRegExps, nil, nil)
107+
bkof := backoff.NewExponentialBackOff(
108+
backoff.WithInitialInterval(60*time.Second),
109+
backoff.WithMaxInterval(60*time.Second),
110+
backoff.WithMultiplier(1.5),
111+
backoff.WithRandomizationFactor(0.0),
112+
)
113+
rexs := clients.StringSliceToRegexpSliceMust([]string{"^retry"})
114+
retryClient := clients.NewResourceClientRetryableErrors(mock, bkof, rexs, nil, nil)
79115
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
80116
defer cancel()
81117
start := time.Now()

‎internal/retry/retryable_errors.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"math/big"
77
"regexp"
88
"strings"
9+
"time"
910

1011
"github.com/hashicorp/terraform-plugin-framework/attr"
1112
"github.com/hashicorp/terraform-plugin-framework/diag"
@@ -606,10 +607,18 @@ func (v RetryValue) GetIntervalSeconds() int {
606607
return v.getInt64AttrValue(intervalSecondsAttributeName)
607608
}
608609

610+
func (v RetryValue) GetIntervalSecondsAsDuration() time.Duration {
611+
return time.Duration(v.IntervalSeconds.ValueInt64()) * time.Second
612+
}
613+
609614
func (v RetryValue) GetMaxIntervalSeconds() int {
610615
return v.getInt64AttrValue(maxIntervalSecondsAttributeName)
611616
}
612617

618+
func (v RetryValue) GetMaxIntervalSecondsAsDuration() time.Duration {
619+
return time.Duration(v.MaxIntervalSeconds.ValueInt64()) * time.Second
620+
}
621+
613622
func (v RetryValue) GetMultiplier() float64 {
614623
return v.getNumberAttrValue(multiplierAttributeName)
615624
}

‎internal/services/azapi_data_plane_resource.go‎

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -374,19 +374,6 @@ func (r *DataPlaneResource) CreateUpdate(ctx context.Context, plan tfsdk.Plan, s
374374

375375
ctx = tflog.SetField(ctx, "resource_id", id.ID())
376376

377-
var client clients.DataPlaneRequester
378-
client = r.ProviderData.DataPlaneClient
379-
if !model.Retry.IsNull() {
380-
bkof, regexps := clients.NewRetryableErrors(
381-
model.Retry.GetIntervalSeconds(),
382-
model.Retry.GetMaxIntervalSeconds(),
383-
model.Retry.GetMultiplier(),
384-
model.Retry.GetRandomizationFactor(),
385-
model.Retry.GetErrorMessages(),
386-
)
387-
tflog.Debug(ctx, "azapi_data_plane_resource.CreateUpdate is using retry")
388-
client = r.ProviderData.DataPlaneClient.WithRetry(bkof, regexps, nil, nil)
389-
}
390377
isNewResource := state == nil || state.Raw.IsNull()
391378

392379
var timeout time.Duration
@@ -402,6 +389,22 @@ func (r *DataPlaneResource) CreateUpdate(ctx context.Context, plan tfsdk.Plan, s
402389
return
403390
}
404391
}
392+
393+
var client clients.DataPlaneRequester
394+
client = r.ProviderData.DataPlaneClient
395+
if !model.Retry.IsNull() {
396+
regexps := clients.StringSliceToRegexpSliceMust(model.Retry.GetErrorMessages())
397+
bkof := backoff.NewExponentialBackOff(
398+
backoff.WithInitialInterval(model.Retry.GetIntervalSecondsAsDuration()),
399+
backoff.WithMaxInterval(model.Retry.GetMaxIntervalSecondsAsDuration()),
400+
backoff.WithMultiplier(model.Retry.GetMultiplier()),
401+
backoff.WithRandomizationFactor(model.Retry.GetRandomizationFactor()),
402+
backoff.WithMaxElapsedTime(timeout),
403+
)
404+
tflog.Debug(ctx, "azapi_data_plane_resource.CreateUpdate is using retry")
405+
client = r.ProviderData.DataPlaneClient.WithRetry(bkof, regexps, nil, nil)
406+
}
407+
405408
ctx, cancel := context.WithTimeout(ctx, timeout)
406409
defer cancel()
407410

@@ -500,12 +503,13 @@ func (r *DataPlaneResource) Read(ctx context.Context, request resource.ReadReque
500503
var client clients.DataPlaneRequester
501504
client = r.ProviderData.DataPlaneClient
502505
if !model.Retry.IsNull() && !model.Retry.IsUnknown() {
503-
bkof, regexps := clients.NewRetryableErrors(
504-
model.Retry.GetIntervalSeconds(),
505-
model.Retry.GetMaxIntervalSeconds(),
506-
model.Retry.GetMultiplier(),
507-
model.Retry.GetRandomizationFactor(),
508-
model.Retry.GetErrorMessages(),
506+
regexps := clients.StringSliceToRegexpSliceMust(model.Retry.GetErrorMessages())
507+
bkof := backoff.NewExponentialBackOff(
508+
backoff.WithInitialInterval(model.Retry.GetIntervalSecondsAsDuration()),
509+
backoff.WithMaxInterval(model.Retry.GetMaxIntervalSecondsAsDuration()),
510+
backoff.WithMultiplier(model.Retry.GetMultiplier()),
511+
backoff.WithRandomizationFactor(model.Retry.GetRandomizationFactor()),
512+
backoff.WithMaxElapsedTime(readTimeout),
509513
)
510514
tflog.Debug(ctx, "azapi_data_plane_resource.Read is using retry")
511515
client = r.ProviderData.DataPlaneClient.WithRetry(bkof, regexps, nil, nil)
@@ -592,12 +596,13 @@ func (r *DataPlaneResource) Delete(ctx context.Context, request resource.DeleteR
592596
var client clients.DataPlaneRequester
593597
client = r.ProviderData.DataPlaneClient
594598
if !model.Retry.IsNull() && !model.Retry.IsUnknown() {
595-
bkof, regexps := clients.NewRetryableErrors(
596-
model.Retry.GetIntervalSeconds(),
597-
model.Retry.GetMaxIntervalSeconds(),
598-
model.Retry.GetMultiplier(),
599-
model.Retry.GetRandomizationFactor(),
600-
model.Retry.GetErrorMessages(),
599+
regexps := clients.StringSliceToRegexpSliceMust(model.Retry.GetErrorMessages())
600+
bkof := backoff.NewExponentialBackOff(
601+
backoff.WithInitialInterval(model.Retry.GetIntervalSecondsAsDuration()),
602+
backoff.WithMaxInterval(model.Retry.GetMaxIntervalSecondsAsDuration()),
603+
backoff.WithMultiplier(model.Retry.GetMultiplier()),
604+
backoff.WithRandomizationFactor(model.Retry.GetRandomizationFactor()),
605+
backoff.WithMaxElapsedTime(deleteTimeout),
601606
)
602607
tflog.Debug(ctx, "azapi_data_plane_resource.Delete is using retry")
603608
client = r.ProviderData.DataPlaneClient.WithRetry(bkof, regexps, nil, nil)

0 commit comments

Comments
 (0)