Merge branch 'b-prototype-new-retryer' into b-aws_instance.RunInstances-retry

Author: ewbankkit

.teamcity/scripts/provider_tests/acceptance_tests.sh

@@ -55,7 +55,7 @@ TF_ACC=1 go test \
     ./internal/provider/... \
     ./internal/reflect/... \
     ./internal/retry/... \
-    ./internal/retry2/... \
+    ./internal/backoff/... \
     ./internal/sdkv2/... \
     ./internal/semver/... \
     ./internal/slices/... \

.teamcity/scripts/provider_tests/unit_tests.sh

@@ -7,6 +7,7 @@ set -euo pipefail
 go test \
     ./internal/acctest/... \
     ./internal/attrmap/... \
+    ./internal/backoff/... \
     ./internal/conns/... \
     ./internal/create/... \
     ./internal/dns/... \
@@ -27,7 +28,6 @@ go test \
     ./internal/provider/... \
     ./internal/reflect/... \
     ./internal/retry/... \
-    ./internal/retry2/... \
     ./internal/sdkv2/... \
     ./internal/semver/... \
     ./internal/slices/... \

internal/retry2/retry.go renamed to internal/backoff/backoff.go

@@ -1,7 +1,7 @@
 // Copyright (c) HashiCorp, Inc.
 // SPDX-License-Identifier: MPL-2.0
 
-package retry2
+package backoff
 
 import (
 	"context"
@@ -20,7 +20,7 @@ type Timer interface {
 // DelayFunc returns the duration to wait before the next retry attempt.
 type DelayFunc func(uint) time.Duration
 
-// FixedDelay returns a delay that is the same through all iterations except the first (when it is 0).
+// FixedDelay returns a delay. The first retry attempt has no delay (0), and subsequent attempts use the fixed delay.
 func FixedDelay(delay time.Duration) DelayFunc {
 	return func(n uint) time.Duration {
 		if n == 0 {
@@ -35,21 +35,24 @@ func FixedDelay(delay time.Duration) DelayFunc {
 // to pick the same deterministic random sequence.
 var rng = rand.New(rand.NewSource(time.Now().UnixNano()))
 
-// ExponentialBackoffWithJitterDelay returns a duration of backoffMinDuration * backoffMultiplier**n, with added jitter.
-func ExponentialBackoffWithJitterDelay(backoffMinDuration time.Duration, backoffMultiplier float64) DelayFunc {
+// ExponentialJitterBackoff returns a duration of backoffMinDuration * backoffMultiplier**n, with added jitter.
+func ExponentialJitterBackoff(backoffMinDuration time.Duration, backoffMultiplier float64) DelayFunc {
 	return func(n uint) time.Duration {
 		if n == 0 {
 			return 0
 		}
 
 		mult := math.Pow(backoffMultiplier, float64(n))
-		d := time.Duration(float64(backoffMinDuration) * mult)
-		const jitter = 0.4
-		mult = 1 - jitter*rng.Float64() // Subtract up to 40%.
-		return time.Duration(float64(d) * mult)
+		return applyJitter(time.Duration(float64(backoffMinDuration) * mult))
 	}
 }
 
+func applyJitter(base time.Duration) time.Duration {
+	const jitterFactor = 0.4
+	jitter := 1 - jitterFactor*rng.Float64() // Subtract up to 40%.
+	return time.Duration(float64(base) * jitter)
+}
+
 type sdkv2HelperRetryCompatibleDelay struct {
 	minTimeout   time.Duration
 	pollInterval time.Duration
@@ -105,20 +108,20 @@ func DefaultSDKv2HelperRetryCompatibleDelay() DelayFunc {
 	return SDKv2HelperRetryCompatibleDelay(0, 0, 500*time.Millisecond) //nolint:mnd // 500ms is the Plugin SDKv2 default
 }
 
-// Config configures a retry loop.
-type Config struct {
+// RetryConfig configures a retry loop.
+type RetryConfig struct {
 	delay       DelayFunc
 	gracePeriod time.Duration
 	timer       Timer
 }
 
 // Option represents an option for retry.
-type Option func(*Config)
+type Option func(*RetryConfig)
 
-func emptyOption(c *Config) {}
+func emptyOption(c *RetryConfig) {}
 
 func WithGracePeriod(d time.Duration) Option {
-	return func(c *Config) {
+	return func(c *RetryConfig) {
 		c.gracePeriod = d
 	}
 }
@@ -128,7 +131,7 @@ func WithDelay(d DelayFunc) Option {
 		return emptyOption
 	}
 
-	return func(c *Config) {
+	return func(c *RetryConfig) {
 		c.delay = d
 	}
 }
@@ -137,7 +140,7 @@ func WithDelay(d DelayFunc) Option {
 // This primarily is useful for mocking/testing, where you may not want to explicitly wait for a set duration
 // for retries.
 func WithTimer(t Timer) Option {
-	return func(c *Config) {
+	return func(c *RetryConfig) {
 		c.timer = t
 	}
 }
@@ -149,53 +152,49 @@ func (t *timerImpl) After(d time.Duration) <-chan time.Time {
 	return time.After(d)
 }
 
-// The default Config is backwards compatible with github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.
-func defaultConfig() Config {
-	return Config{
+// The default RetryConfig is backwards compatible with github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.
+func defaultRetryConfig() RetryConfig {
+	return RetryConfig{
 		delay:       DefaultSDKv2HelperRetryCompatibleDelay(),
 		gracePeriod: 30 * time.Second,
 		timer:       &timerImpl{},
 	}
 }
 
-// RetryWithTimeout holds state for managing retry loops with a timeout.
-type RetryWithTimeout struct {
+// RetryLoop holds state for managing retry loops with a timeout.
+type RetryLoop struct {
 	attempt  uint
-	config   Config
+	config   RetryConfig
 	deadline deadline
 	timedOut bool
 }
 
-// BeginWithOptions returns a new retry loop configured with the provided options.
-func BeginWithOptions(timeout time.Duration, opts ...Option) *RetryWithTimeout {
-	config := defaultConfig()
+// NewRetryLoopWithOptions returns a new retry loop configured with the provided options.
+func NewRetryLoopWithOptions(timeout time.Duration, opts ...Option) *RetryLoop {
+	config := defaultRetryConfig()
 	for _, opt := range opts {
 		opt(&config)
 	}
 
-	return &RetryWithTimeout{
+	return &RetryLoop{
 		config:   config,
-		deadline: NewDeadline(timeout),
+		deadline: NewDeadline(timeout + config.gracePeriod),
 	}
 }
 
-// Begin returns a new retry loop configured with the default options.
-func Begin(timeout time.Duration) *RetryWithTimeout {
-	return BeginWithOptions(timeout)
+// NewRetryLoop returns a new retry loop configured with the default options.
+func NewRetryLoop(timeout time.Duration) *RetryLoop {
+	return NewRetryLoopWithOptions(timeout)
 }
 
 // Continue sleeps between retry attempts.
 // It returns false if the timeout has been exceeded.
 // The deadline is not checked on the first call to Continue.
-func (r *RetryWithTimeout) Continue(ctx context.Context) bool {
+func (r *RetryLoop) Continue(ctx context.Context) bool {
 	if r.attempt != 0 && r.deadline.Remaining() == 0 {
-		if r.config.gracePeriod == 0 {
-			r.timedOut = true
-			return false
-		}
+		r.timedOut = true
 
-		r.deadline = NewDeadline(r.config.gracePeriod)
-		r.config.gracePeriod = 0
+		return false
 	}
 
 	r.sleep(ctx, r.config.delay(r.attempt))
@@ -204,18 +203,18 @@ func (r *RetryWithTimeout) Continue(ctx context.Context) bool {
 	return true
 }
 
-// Reset resets a RetryWithTimeout to its initial state.
-func (r *RetryWithTimeout) Reset() {
+// Reset resets a RetryLoop to its initial state.
+func (r *RetryLoop) Reset() {
 	r.attempt = 0
 }
 
 // TimedOut return whether the retry timed out.
-func (r *RetryWithTimeout) TimedOut() bool {
+func (r *RetryLoop) TimedOut() bool {
 	return r.timedOut
 }
 
-// Sleeps for the specified duration or until context is done, whichever occurs first.
-func (r *RetryWithTimeout) sleep(ctx context.Context, d time.Duration) {
+// sleep sleeps for the specified duration or until the context is canceled, whichever occurs first.
+func (r *RetryLoop) sleep(ctx context.Context, d time.Duration) {
 	if d == 0 {
 		return
 	}

internal/retry2/retry_test.go renamed to internal/backoff/backoff_test.go

@@ -1,7 +1,7 @@
 // Copyright (c) HashiCorp, Inc.
 // SPDX-License-Identifier: MPL-2.0
 
-package retry2
+package backoff
 
 import (
 	"context"
@@ -85,7 +85,7 @@ func TestRetryWithTimeoutDefaultGracePeriod(t *testing.T) {
 	ctx := context.Background()
 
 	var n int
-	for r := BeginWithOptions(1*time.Minute, WithDelay(FixedDelay(1*time.Second))); r.Continue(ctx); {
+	for r := NewRetryLoopWithOptions(1*time.Minute, WithDelay(FixedDelay(1*time.Second))); r.Continue(ctx); {
 		time.Sleep(35 * time.Second)
 		n++
 	}
@@ -102,7 +102,7 @@ func TestRetryWithTimeoutNoGracePeriod(t *testing.T) {
 	ctx := context.Background()
 
 	var n int
-	for r := BeginWithOptions(1*time.Minute, WithDelay(FixedDelay(1*time.Second)), WithGracePeriod(0)); r.Continue(ctx); {
+	for r := NewRetryLoopWithOptions(1*time.Minute, WithDelay(FixedDelay(1*time.Second)), WithGracePeriod(0)); r.Continue(ctx); {
 		time.Sleep(35 * time.Second)
 		n++
 	}

internal/retry2/deadline.go renamed to internal/backoff/deadline.go

@@ -1,7 +1,7 @@
 // Copyright (c) HashiCorp, Inc.
 // SPDX-License-Identifier: MPL-2.0
 
-package retry2
+package backoff
 
 import (
 	"time"

internal/service/ec2/ec2_instance.go

@@ -31,13 +31,13 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
+	"github.com/hashicorp/terraform-provider-aws/internal/backoff"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
 	"github.com/hashicorp/terraform-provider-aws/internal/create"
 	"github.com/hashicorp/terraform-provider-aws/internal/enum"
 	"github.com/hashicorp/terraform-provider-aws/internal/errs"
 	"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
 	"github.com/hashicorp/terraform-provider-aws/internal/flex"
-	"github.com/hashicorp/terraform-provider-aws/internal/retry2"
 	tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
 	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
 	itypes "github.com/hashicorp/terraform-provider-aws/internal/types"
@@ -1051,7 +1051,7 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta an
 	}
 
 	var output *ec2.RunInstancesOutput
-	for r := retry2.Begin(iamPropagationTimeout); r.Continue(ctx); {
+	for r := backoff.NewRetryLoop(iamPropagationTimeout); r.Continue(ctx); {
 		output, err = conn.RunInstances(ctx, &input)
 
 		// IAM instance profiles can take ~10 seconds to propagate in AWS: