From 5d0641658fcb5c805e4329aad3a2213da1edc2f3 Mon Sep 17 00:00:00 2001 From: Ben Rosenbloom Date: Mon, 6 Oct 2025 11:43:26 -0400 Subject: [PATCH 1/6] Added Claude Code suggestion for retries on 409 --- sysdig/common_test.go | 41 +++++++++++++++++++++ sysdig/resource_sysdig_secure_list_test.go | 2 +- sysdig/resource_sysdig_secure_macro_test.go | 2 +- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/sysdig/common_test.go b/sysdig/common_test.go index 56f9e3d4a..327fa4795 100644 --- a/sysdig/common_test.go +++ b/sysdig/common_test.go @@ -4,8 +4,10 @@ import ( "os" "strings" "testing" + "time" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) const ( @@ -45,3 +47,42 @@ func sysdigOrIBMMonitorPreCheck(t *testing.T) func() { func randomText(len int) string { return acctest.RandStringFromCharSet(len, acctest.CharSetAlphaNum) } + +// retryOn409 wraps a TestStep to retry on 409 Conflict errors +func retryOn409(step resource.TestStep) resource.TestStep { + if step.PlanOnly { + return step + } + + originalConfig := step.Config + step.Config = "" + step.PreConfig = func() { + for i := 0; i < 5; i++ { + if i > 0 { + time.Sleep(time.Duration(i*2) * time.Second) + } + step.Config = originalConfig + break + } + } + return step +} + +// testCaseWithRetry creates a TestCase with retry logic for all steps +func testCaseWithRetry(testCase resource.TestCase) resource.TestCase { + for i := range testCase.Steps { + testCase.Steps[i] = retryOn409(testCase.Steps[i]) + } + return testCase +} + +// retryTestStep creates a single TestStep with retry logic +func retryTestStep(config string, checks ...resource.TestCheckFunc) resource.TestStep { + return resource.TestStep{ + Config: config, + Check: resource.ComposeTestCheckFunc(checks...), + Retry: func() *resource.RetryError { + return resource.RetryableError(nil) // Retry on any error + }, + } +} diff --git a/sysdig/resource_sysdig_secure_list_test.go b/sysdig/resource_sysdig_secure_list_test.go index 527384e61..db1e53c3d 100644 --- a/sysdig/resource_sysdig_secure_list_test.go +++ b/sysdig/resource_sysdig_secure_list_test.go @@ -17,7 +17,7 @@ func TestAccList(t *testing.T) { rText := func() string { return acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) } fixedRandomText := rText() - resource.Test(t, resource.TestCase{ + resource.Test(t, testCaseWithRetry(resource.TestCase{...}){ PreCheck: preCheckAnyEnv(t, SysdigSecureApiTokenEnv), ProviderFactories: map[string]func() (*schema.Provider, error){ "sysdig": func() (*schema.Provider, error) { diff --git a/sysdig/resource_sysdig_secure_macro_test.go b/sysdig/resource_sysdig_secure_macro_test.go index 8e1213889..ecad6b2e3 100644 --- a/sysdig/resource_sysdig_secure_macro_test.go +++ b/sysdig/resource_sysdig_secure_macro_test.go @@ -17,7 +17,7 @@ func TestAccMacro(t *testing.T) { rText := func() string { return acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) } fixedRandomText := rText() - resource.Test(t, resource.TestCase{ + resource.Test(t, testCaseWithRetry(resource.TestCase{...}){ PreCheck: preCheckAnyEnv(t, SysdigSecureApiTokenEnv), ProviderFactories: map[string]func() (*schema.Provider, error){ "sysdig": func() (*schema.Provider, error) { From 05b3b59a869709578d877fb3e117c829a914ae96 Mon Sep 17 00:00:00 2001 From: Ben Rosenbloom Date: Mon, 6 Oct 2025 11:55:44 -0400 Subject: [PATCH 2/6] Fixed syntax errors --- sysdig/resource_sysdig_secure_list_test.go | 4 ++-- sysdig/resource_sysdig_secure_macro_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sysdig/resource_sysdig_secure_list_test.go b/sysdig/resource_sysdig_secure_list_test.go index db1e53c3d..00b611621 100644 --- a/sysdig/resource_sysdig_secure_list_test.go +++ b/sysdig/resource_sysdig_secure_list_test.go @@ -17,7 +17,7 @@ func TestAccList(t *testing.T) { rText := func() string { return acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) } fixedRandomText := rText() - resource.Test(t, testCaseWithRetry(resource.TestCase{...}){ + resource.Test(t, testCaseWithRetry(resource.TestCase{ PreCheck: preCheckAnyEnv(t, SysdigSecureApiTokenEnv), ProviderFactories: map[string]func() (*schema.Provider, error){ "sysdig": func() (*schema.Provider, error) { @@ -46,7 +46,7 @@ func TestAccList(t *testing.T) { Config: listWithList(rText(), rText()), }, }, - }) + })) } func listWithName(name string) string { diff --git a/sysdig/resource_sysdig_secure_macro_test.go b/sysdig/resource_sysdig_secure_macro_test.go index ecad6b2e3..93330b428 100644 --- a/sysdig/resource_sysdig_secure_macro_test.go +++ b/sysdig/resource_sysdig_secure_macro_test.go @@ -17,7 +17,7 @@ func TestAccMacro(t *testing.T) { rText := func() string { return acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) } fixedRandomText := rText() - resource.Test(t, testCaseWithRetry(resource.TestCase{...}){ + resource.Test(t, testCaseWithRetry(resource.TestCase{ PreCheck: preCheckAnyEnv(t, SysdigSecureApiTokenEnv), ProviderFactories: map[string]func() (*schema.Provider, error){ "sysdig": func() (*schema.Provider, error) { @@ -52,7 +52,7 @@ func TestAccMacro(t *testing.T) { Config: macroWithMinimumEngineVersion(rText()), }, }, - }) + })) } func macroWithName(name string) string { From 43db882d3fca95d47d4bc619a7788428dbb9892c Mon Sep 17 00:00:00 2001 From: Ben Rosenbloom Date: Mon, 6 Oct 2025 12:00:03 -0400 Subject: [PATCH 3/6] Remove unneeded code --- sysdig/common_test.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/sysdig/common_test.go b/sysdig/common_test.go index 327fa4795..7dc522cce 100644 --- a/sysdig/common_test.go +++ b/sysdig/common_test.go @@ -53,7 +53,7 @@ func retryOn409(step resource.TestStep) resource.TestStep { if step.PlanOnly { return step } - + originalConfig := step.Config step.Config = "" step.PreConfig = func() { @@ -75,14 +75,3 @@ func testCaseWithRetry(testCase resource.TestCase) resource.TestCase { } return testCase } - -// retryTestStep creates a single TestStep with retry logic -func retryTestStep(config string, checks ...resource.TestCheckFunc) resource.TestStep { - return resource.TestStep{ - Config: config, - Check: resource.ComposeTestCheckFunc(checks...), - Retry: func() *resource.RetryError { - return resource.RetryableError(nil) // Retry on any error - }, - } -} From 2181922255d81a4b0f14adf5c180c85672638835 Mon Sep 17 00:00:00 2001 From: Ben Rosenbloom Date: Mon, 6 Oct 2025 12:23:15 -0400 Subject: [PATCH 4/6] Implement retry logic in client code --- sysdig/common_test.go | 29 ++++------------------------- sysdig/internal/client/v2/client.go | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/sysdig/common_test.go b/sysdig/common_test.go index 7dc522cce..2350e5ddd 100644 --- a/sysdig/common_test.go +++ b/sysdig/common_test.go @@ -4,7 +4,6 @@ import ( "os" "strings" "testing" - "time" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -48,30 +47,10 @@ func randomText(len int) string { return acctest.RandStringFromCharSet(len, acctest.CharSetAlphaNum) } -// retryOn409 wraps a TestStep to retry on 409 Conflict errors -func retryOn409(step resource.TestStep) resource.TestStep { - if step.PlanOnly { - return step - } - - originalConfig := step.Config - step.Config = "" - step.PreConfig = func() { - for i := 0; i < 5; i++ { - if i > 0 { - time.Sleep(time.Duration(i*2) * time.Second) - } - step.Config = originalConfig - break - } - } - return step -} - -// testCaseWithRetry creates a TestCase with retry logic for all steps +// testCaseWithRetry wraps a TestCase to handle it with retry logic for 409 Conflict errors +// Note: This returns the original TestCase since Terraform's SDK doesn't support +// automatic retries at the TestCase level. The retry logic should be implemented +// at the HTTP client level using retryablehttp.CheckRetry func testCaseWithRetry(testCase resource.TestCase) resource.TestCase { - for i := range testCase.Steps { - testCase.Steps[i] = retryOn409(testCase.Steps[i]) - } return testCase } diff --git a/sysdig/internal/client/v2/client.go b/sysdig/internal/client/v2/client.go index 7730bc943..0e04fc3d5 100644 --- a/sysdig/internal/client/v2/client.go +++ b/sysdig/internal/client/v2/client.go @@ -185,5 +185,23 @@ func newHTTPClient(cfg *config) *http.Client { transport := http.DefaultTransport.(*http.Transport).Clone() transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: cfg.insecure} httpClient.HTTPClient = &http.Client{Transport: transport} + + // Configure retry logic for 409 Conflict errors + httpClient.RetryMax = 5 + httpClient.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) { + // Use default retry logic for connection errors and 5xx + shouldRetry, checkErr := retryablehttp.DefaultRetryPolicy(ctx, resp, err) + if shouldRetry || checkErr != nil { + return shouldRetry, checkErr + } + + // Additionally retry on 409 Conflict + if resp != nil && resp.StatusCode == http.StatusConflict { + return true, nil + } + + return false, nil + } + return httpClient.StandardClient() } From 975979c9ae59c0048c8d47494fce82aef601458f Mon Sep 17 00:00:00 2001 From: Ben Rosenbloom Date: Mon, 6 Oct 2025 13:40:57 -0400 Subject: [PATCH 5/6] Cleanup code --- sysdig/common_test.go | 9 --------- sysdig/resource_sysdig_secure_list_test.go | 4 ++-- sysdig/resource_sysdig_secure_macro_test.go | 4 ++-- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/sysdig/common_test.go b/sysdig/common_test.go index 2350e5ddd..56f9e3d4a 100644 --- a/sysdig/common_test.go +++ b/sysdig/common_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) const ( @@ -46,11 +45,3 @@ func sysdigOrIBMMonitorPreCheck(t *testing.T) func() { func randomText(len int) string { return acctest.RandStringFromCharSet(len, acctest.CharSetAlphaNum) } - -// testCaseWithRetry wraps a TestCase to handle it with retry logic for 409 Conflict errors -// Note: This returns the original TestCase since Terraform's SDK doesn't support -// automatic retries at the TestCase level. The retry logic should be implemented -// at the HTTP client level using retryablehttp.CheckRetry -func testCaseWithRetry(testCase resource.TestCase) resource.TestCase { - return testCase -} diff --git a/sysdig/resource_sysdig_secure_list_test.go b/sysdig/resource_sysdig_secure_list_test.go index 00b611621..527384e61 100644 --- a/sysdig/resource_sysdig_secure_list_test.go +++ b/sysdig/resource_sysdig_secure_list_test.go @@ -17,7 +17,7 @@ func TestAccList(t *testing.T) { rText := func() string { return acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) } fixedRandomText := rText() - resource.Test(t, testCaseWithRetry(resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: preCheckAnyEnv(t, SysdigSecureApiTokenEnv), ProviderFactories: map[string]func() (*schema.Provider, error){ "sysdig": func() (*schema.Provider, error) { @@ -46,7 +46,7 @@ func TestAccList(t *testing.T) { Config: listWithList(rText(), rText()), }, }, - })) + }) } func listWithName(name string) string { diff --git a/sysdig/resource_sysdig_secure_macro_test.go b/sysdig/resource_sysdig_secure_macro_test.go index 93330b428..8e1213889 100644 --- a/sysdig/resource_sysdig_secure_macro_test.go +++ b/sysdig/resource_sysdig_secure_macro_test.go @@ -17,7 +17,7 @@ func TestAccMacro(t *testing.T) { rText := func() string { return acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) } fixedRandomText := rText() - resource.Test(t, testCaseWithRetry(resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: preCheckAnyEnv(t, SysdigSecureApiTokenEnv), ProviderFactories: map[string]func() (*schema.Provider, error){ "sysdig": func() (*schema.Provider, error) { @@ -52,7 +52,7 @@ func TestAccMacro(t *testing.T) { Config: macroWithMinimumEngineVersion(rText()), }, }, - })) + }) } func macroWithName(name string) string { From afef79d8a3564c87c745dfb32535563e82f64fc8 Mon Sep 17 00:00:00 2001 From: Ben Rosenbloom Date: Mon, 6 Oct 2025 14:19:41 -0400 Subject: [PATCH 6/6] Add exponential backoff --- sysdig/internal/client/v2/client.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sysdig/internal/client/v2/client.go b/sysdig/internal/client/v2/client.go index 0e04fc3d5..5b975a2f9 100644 --- a/sysdig/internal/client/v2/client.go +++ b/sysdig/internal/client/v2/client.go @@ -12,6 +12,7 @@ import ( "net/http" "net/http/httputil" "strings" + "time" "github.com/draios/terraform-provider-sysdig/buildinfo" "github.com/hashicorp/go-retryablehttp" @@ -186,8 +187,12 @@ func newHTTPClient(cfg *config) *http.Client { transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: cfg.insecure} httpClient.HTTPClient = &http.Client{Transport: transport} - // Configure retry logic for 409 Conflict errors + // Configure retry logic for 409 Conflict errors with exponential backoff httpClient.RetryMax = 5 + httpClient.RetryWaitMin = 1 * time.Second + httpClient.RetryWaitMax = 30 * time.Second + httpClient.Backoff = retryablehttp.DefaultBackoff // Exponential backoff strategy + httpClient.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) { // Use default retry logic for connection errors and 5xx shouldRetry, checkErr := retryablehttp.DefaultRetryPolicy(ctx, resp, err)