resource retry win

Author: avivm-lang

akeyless/common/provider_retry_skip.go

@@ -0,0 +1,46 @@
+package common
+
+import (
+	"bytes"
+	"runtime"
+	"strconv"
+	"sync"
+)
+
+// skipProviderHTTPRetry is a per-worker "resource retry is active" flag.
+// Keyed by goroutine id so parallel Terraform resource workers stay isolated:
+// if resource A has retry {}, only A's HTTP calls skip provider retry; resource B
+// (no resource retry) still uses provider HTTP retry. A single global bool would
+// incorrectly disable provider retry for every concurrent worker.
+var skipProviderHTTPRetry sync.Map // goroutineID -> struct{}
+
+// SetSkipProviderHTTPRetry marks (or clears) "skip provider HTTP retry" for the
+// current worker only. Used while a resource-level retry {} attempt is in flight.
+func SetSkipProviderHTTPRetry(skip bool) {
+	id := goroutineID()
+	if skip {
+		skipProviderHTTPRetry.Store(id, struct{}{})
+		return
+	}
+	skipProviderHTTPRetry.Delete(id)
+}
+
+// SkipProviderHTTPRetry is true when the current worker has resource retry active
+// (see SetSkipProviderHTTPRetry). Checked from the HTTP transport.
+func SkipProviderHTTPRetry() bool {
+	_, ok := skipProviderHTTPRetry.Load(goroutineID())
+	return ok
+}
+
+func goroutineID() uint64 {
+	b := make([]byte, 64)
+	b = b[:runtime.Stack(b, false)]
+	// "goroutine 123 [running]:..."
+	b = bytes.TrimPrefix(b, []byte("goroutine "))
+	i := bytes.IndexByte(b, ' ')
+	if i <= 0 {
+		return 0
+	}
+	id, _ := strconv.ParseUint(string(b[:i]), 10, 64)
+	return id
+}

akeyless/common/retry.go

@@ -9,28 +9,44 @@ import (
 	"time"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
+// sdkRetryTimeout is large on purpose: we stop with max_retries inside RetryFunc,
+// not with the SDK helper's timeout clock.
+const sdkRetryTimeout = 24 * time.Hour
+
+// default config values
 const (
 	resourceRetryDefaultIntervalSeconds    = 10.0
 	resourceRetryDefaultMaxIntervalSeconds = 180.0
 	resourceRetryDefaultMultiplier         = 1.5
 )
 
-// ResourceRetrySchema is the optional resource-level retry block (after provider HTTP retry).
+// defaultResourceRetryErrorPatterns mirrors provider-level transient/rate-limit text defaults.
+var defaultResourceRetryErrorPatterns = []string{
+	"Too Many Requests",
+	"will be released in",
+	"EOF",
+	"connection reset by peer",
+	"connection refused",
+}
+
+// ResourceRetrySchema is the optional resource-level retry block.
+// When set, it overrides provider HTTP retry for that resource's CRUD calls.
 func ResourceRetrySchema() *schema.Schema {
 	return &schema.Schema{
 		Type:        schema.TypeList,
 		Optional:    true,
 		MaxItems:    1,
-		Description: "Optional resource-level retry. Runs after provider HTTP retries; matches error messages.",
+		Description: "Optional resource-level retry. When set, overrides provider HTTP retry for this resource.",
 		Elem: &schema.Resource{
 			Schema: map[string]*schema.Schema{
-				"error_message_regex": {
+				"retry_on_messages": {
 					Type:        schema.TypeList,
 					Optional:    true,
-					Description: "Regexes matched against error messages. If any match, the operation is retried.",
+					Description: "Error message texts that trigger a retry. Defaults to provider transient/rate-limit messages when omitted.",
 					Elem:        &schema.Schema{Type: schema.TypeString},
 				},
 				"interval_seconds": {
@@ -157,26 +173,27 @@ func resourceRetryConfigFromData(d *schema.ResourceData) (*resourceRetryConfig,
 		cfg.Multiplier = v
 	}
 
-	patterns, _ := m["error_message_regex"].([]interface{})
+	patterns, _ := m["retry_on_messages"].([]interface{})
 	for _, p := range patterns {
 		s, ok := p.(string)
 		if !ok || s == "" {
 			continue
 		}
 		re, err := regexp.Compile(s)
 		if err != nil {
-			return nil, fmt.Errorf("invalid retry.error_message_regex %q: %w", s, err)
+			return nil, fmt.Errorf("invalid retry.retry_on_messages %q: %w", s, err)
 		}
 		cfg.ErrorMessageRegex = append(cfg.ErrorMessageRegex, re)
 	}
 	if len(cfg.ErrorMessageRegex) == 0 {
-		// Without message matchers, resource-level retry would retry every error — refuse that.
-		return nil, fmt.Errorf("retry.error_message_regex must contain at least one pattern")
+		for _, s := range defaultResourceRetryErrorPatterns {
+			cfg.ErrorMessageRegex = append(cfg.ErrorMessageRegex, regexp.MustCompile(s))
+		}
 	}
 	return cfg, nil
 }
 
-// RetryResourceOp runs fn, retrying when a resource retry block matches the error message.
+// RetryResourceOp wraps older CRUD hooks that return error (Create/Update/Delete).
 func RetryResourceOp(d *schema.ResourceData, fn func() error) error {
 	cfg, err := resourceRetryConfigFromData(d)
 	if err != nil {
@@ -185,24 +202,11 @@ func RetryResourceOp(d *schema.ResourceData, fn func() error) error {
 	if cfg == nil {
 		return fn()
 	}
-
-	var lastErr error
-	for attempt := 0; attempt <= cfg.MaxRetries; attempt++ {
-		lastErr = fn()
-		if lastErr == nil {
-			return nil
-		}
-		if attempt == cfg.MaxRetries || !cfg.matches(lastErr.Error()) {
-			return lastErr
-		}
-		wait := cfg.backoff(attempt)
-		log.Printf("[DEBUG] akeyless: resource retry attempt %d/%d after %s: %v", attempt+1, cfg.MaxRetries, wait, lastErr)
-		time.Sleep(wait)
-	}
-	return lastErr
+	return cfg.run(fn)
 }
 
-// RetryResourceOpDiag is the Context-CRUD equivalent of RetryResourceOp.
+// RetryResourceOpDiag wraps Context CRUD hooks that return diag.Diagnostics
+// (CreateContext/UpdateContext/DeleteContext). Same retry policy as RetryResourceOp.
 func RetryResourceOpDiag(d *schema.ResourceData, fn func() diag.Diagnostics) diag.Diagnostics {
 	cfg, err := resourceRetryConfigFromData(d)
 	if err != nil {
@@ -213,23 +217,45 @@ func RetryResourceOpDiag(d *schema.ResourceData, fn func() diag.Diagnostics) dia
 	}
 
 	var last diag.Diagnostics
-	for attempt := 0; attempt <= cfg.MaxRetries; attempt++ {
+	_ = cfg.run(func() error {
 		last = fn()
 		if !last.HasError() {
-			return last
+			return nil
 		}
-		msg := diagnosticsMessage(last)
-		if attempt == cfg.MaxRetries || !cfg.matches(msg) {
-			return last
+		return fmt.Errorf("%s", diagnosticsMessage(last))
+	})
+	return last
+}
+
+// run is the shared resource-retry loop. Set skip inside RetryFunc: SDK runs it on a
+// worker goroutine (same one as HTTP calls).
+func (cfg *resourceRetryConfig) run(fn func() error) error {
+	attempt := 0
+	var lastErr error
+	err := retry.RetryContext(context.Background(), sdkRetryTimeout, func() *retry.RetryError {
+		SetSkipProviderHTTPRetry(true)
+		defer SetSkipProviderHTTPRetry(false)
+
+		lastErr = fn()
+		if lastErr == nil {
+			return nil
+		}
+		if attempt >= cfg.MaxRetries || !cfg.matches(lastErr.Error()) {
+			return retry.NonRetryableError(lastErr)
 		}
 		wait := cfg.backoff(attempt)
-		log.Printf("[DEBUG] akeyless: resource retry attempt %d/%d after %s: %s", attempt+1, cfg.MaxRetries, wait, msg)
+		log.Printf("[DEBUG] akeyless: resource retry attempt %d/%d after %s: %v", attempt+1, cfg.MaxRetries, wait, lastErr)
 		time.Sleep(wait)
+		attempt++
+		return retry.RetryableError(lastErr)
+	})
+	if lastErr != nil {
+		return lastErr
 	}
-	return last
+	return err
 }
 
-// diagnosticsMessage returns the first error text so resource retry can match error_message_regex.
+// diagnosticsMessage returns the first error text so resource retry can match retry_on_messages.
 func diagnosticsMessage(diags diag.Diagnostics) string {
 	for _, d := range diags {
 		if d.Severity != diag.Error {

akeyless/common/retry_test.go

@@ -32,7 +32,7 @@ func TestRetryResourceOp_MatchesAndRetries(t *testing.T) {
 	d := schema.TestResourceDataRaw(t, r.Schema, map[string]interface{}{
 		"retry": []interface{}{
 			map[string]interface{}{
-				"error_message_regex":  []interface{}{"Too Many Requests"},
+				"retry_on_messages":  []interface{}{"Too Many Requests"},
 				"interval_seconds":     0.01,
 				"max_interval_seconds": 0.05,
 				"max_retries":          2,
@@ -63,7 +63,7 @@ func TestRetryResourceOp_NoMatch(t *testing.T) {
 	d := schema.TestResourceDataRaw(t, r.Schema, map[string]interface{}{
 		"retry": []interface{}{
 			map[string]interface{}{
-				"error_message_regex": []interface{}{"will be released in"},
+				"retry_on_messages": []interface{}{"will be released in"},
 				"interval_seconds":    0.01,
 				"max_retries":         3,
 			},
@@ -83,19 +83,60 @@ func TestRetryResourceOp_NoMatch(t *testing.T) {
 	}
 }
 
-func TestRetryResourceOp_RequiresRegex(t *testing.T) {
+func TestRetryResourceOp_DefaultRegexWhenOmitted(t *testing.T) {
 	r := &schema.Resource{Schema: map[string]*schema.Schema{}}
 	AddResourceRetrySchema(r.Schema)
 	d := schema.TestResourceDataRaw(t, r.Schema, map[string]interface{}{
 		"retry": []interface{}{
 			map[string]interface{}{
-				"max_retries": 1,
+				"interval_seconds": 0.01,
+				"max_retries":      2,
+				"multiplier":       1.0,
 			},
 		},
 	})
 
-	err := RetryResourceOp(d, func() error { return nil })
-	if err == nil {
-		t.Fatal("expected error for missing error_message_regex")
+	var calls int32
+	err := RetryResourceOp(d, func() error {
+		n := atomic.AddInt32(&calls, 1)
+		if n < 3 {
+			return errors.New("429 Too Many Requests")
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if calls != 3 {
+		t.Fatalf("calls=%d want 3", calls)
+	}
+}
+
+func TestRetryResourceOp_SkipsProviderHTTPRetry(t *testing.T) {
+	r := &schema.Resource{Schema: map[string]*schema.Schema{}}
+	AddResourceRetrySchema(r.Schema)
+	d := schema.TestResourceDataRaw(t, r.Schema, map[string]interface{}{
+		"retry": []interface{}{
+			map[string]interface{}{
+				"retry_on_messages": []interface{}{"x"},
+				"max_retries":         0,
+			},
+		},
+	})
+
+	if SkipProviderHTTPRetry() {
+		t.Fatal("skip should be off before RetryResourceOp")
+	}
+	err := RetryResourceOp(d, func() error {
+		if !SkipProviderHTTPRetry() {
+			t.Fatal("skip should be on during resource retry")
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if SkipProviderHTTPRetry() {
+		t.Fatal("skip should be off after RetryResourceOp")
 	}
 }

akeyless/retry_config.go

@@ -86,10 +86,10 @@ func providerRetrySchema() *schema.Schema {
 					Default:     defaultMultiplier,
 					Description: "Exponential backoff multiplier. Env: AKEYLESS_RETRY_MULTIPLIER.",
 				},
-				"error_message_regex": {
+				"retry_on_messages": {
 					Type:        schema.TypeList,
 					Optional:    true,
-					Description: "Additional regexes matched against response bodies to trigger a retry.",
+					Description: "Additional response-body message texts that trigger a retry.",
 					Elem:        &schema.Schema{Type: schema.TypeString},
 				},
 			},
@@ -123,7 +123,7 @@ func retryConfigFromProviderData(d *schema.ResourceData) (retryConfig, error) {
 			}
 			cfg.RetryOnStatusCodes = statusCodeSet(parsed)
 		}
-		if patterns, ok := m["error_message_regex"].([]interface{}); ok {
+		if patterns, ok := m["retry_on_messages"].([]interface{}); ok {
 			regexes := make([]*regexp.Regexp, 0, len(patterns))
 			for _, p := range patterns {
 				s, ok := p.(string)
@@ -132,7 +132,7 @@ func retryConfigFromProviderData(d *schema.ResourceData) (retryConfig, error) {
 				}
 				re, err := regexp.Compile(s)
 				if err != nil {
-					return cfg, fmt.Errorf("invalid error_message_regex %q: %w", s, err)
+					return cfg, fmt.Errorf("invalid retry_on_messages %q: %w", s, err)
 				}
 				regexes = append(regexes, re)
 			}