From 4ea55ad8eab7a159303c8d75066c4ff055e480e7 Mon Sep 17 00:00:00 2001 From: matt-metivier Date: Fri, 5 Sep 2025 16:48:31 -0600 Subject: [PATCH 1/2] refactor(asserts): extract common utilities and improve retry logic - Add common utilities for client validation and retry logic - Refactor alert config and disabled alert config resources to use shared utilities - Improve retry logic with exponential backoff and jitter - Remove code duplication across asserts resources - Update test destroy functions with better retry logic - Remove stress test skip conditions as retry logic is now more robust - Update asserts client dependency for compatibility with new API features --- go.mod | 2 +- go.sum | 2 + internal/resources/asserts/common.go | 86 ++++++++++++++++++ .../asserts/resource_alert_config.go | 88 ++++++------------- .../asserts/resource_alert_config_test.go | 46 ++++++---- .../asserts/resource_disabled_alert_config.go | 84 +++++------------- .../resource_disabled_alert_config_test.go | 46 ++++++---- 7 files changed, 193 insertions(+), 161 deletions(-) create mode 100644 internal/resources/asserts/common.go diff --git a/go.mod b/go.mod index 5d5994fbb..cb9d3be46 100644 --- a/go.mod +++ b/go.mod @@ -52,7 +52,7 @@ require ( k8s.io/client-go v0.32.3 ) -require github.com/grafana/grafana-asserts-public-clients/go/gcom v0.0.0-20250805165836-14e16b51b910 +require github.com/grafana/grafana-asserts-public-clients/go/gcom v0.0.0-20250811125322-247815da58ca require ( cuelang.org/go v0.11.1 // indirect diff --git a/go.sum b/go.sum index fadba7b11..0ab76c8dd 100644 --- a/go.sum +++ b/go.sum @@ -182,6 +182,8 @@ github.com/grafana/grafana-app-sdk/logging v0.35.1 h1:taVpl+RoixTYl0JBJGhH+fPVmw github.com/grafana/grafana-app-sdk/logging v0.35.1/go.mod h1:Y/bvbDhBiV/tkIle9RW49pgfSPIPSON8Q4qjx3pyqDk= github.com/grafana/grafana-asserts-public-clients/go/gcom v0.0.0-20250805165836-14e16b51b910 h1:2OfDIhMtXWWVQcDp9cq/VMSBOJJfDek9450rcsV+qLg= github.com/grafana/grafana-asserts-public-clients/go/gcom v0.0.0-20250805165836-14e16b51b910/go.mod h1:EL/5hluCvj6EDjkUfoClLKSKDoCoDowZUety28jhxQI= +github.com/grafana/grafana-asserts-public-clients/go/gcom v0.0.0-20250811125322-247815da58ca h1:GVzyCTi3rqvjK42b++lFjabG2zsrLvyAbbR43dWP6s0= +github.com/grafana/grafana-asserts-public-clients/go/gcom v0.0.0-20250811125322-247815da58ca/go.mod h1:EL/5hluCvj6EDjkUfoClLKSKDoCoDowZUety28jhxQI= github.com/grafana/grafana-com-public-clients/go/gcom v0.0.0-20250526074454-7ec66e02e4bb h1:rmYEnCXHNQbRsuzc5jCX5qkBqFF37c5RCHlyqAAPJZo= github.com/grafana/grafana-com-public-clients/go/gcom v0.0.0-20250526074454-7ec66e02e4bb/go.mod h1:sYWkB3NhyirQJfy3wtNQ29UYjoHbRlJlYhqN1jNsC5g= github.com/grafana/grafana-openapi-client-go v0.0.0-20250617151817-c0f8cbb88d5c h1:jox7J0BnJmcZJp8lp631u4gjDEoIfpi6O3yrpiXNTtg= diff --git a/internal/resources/asserts/common.go b/internal/resources/asserts/common.go new file mode 100644 index 000000000..422a66c9b --- /dev/null +++ b/internal/resources/asserts/common.go @@ -0,0 +1,86 @@ +package asserts + +import ( + "context" + "fmt" + "math" + "math/rand" + "time" + + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" + + assertsapi "github.com/grafana/grafana-asserts-public-clients/go/gcom" + "github.com/grafana/terraform-provider-grafana/v4/internal/common" +) + +// validateAssertsClient checks if the Asserts API client is properly configured +func validateAssertsClient(meta interface{}) (*assertsapi.APIClient, int64, diag.Diagnostics) { + client := meta.(*common.Client).AssertsAPIClient + if client == nil { + return nil, 0, diag.Errorf("Asserts API client is not configured") + } + + stackID := meta.(*common.Client).GrafanaStackID + if stackID == 0 { + return nil, 0, diag.Errorf("stack_id must be set in provider configuration for Asserts resources") + } + + return client, stackID, nil +} + +// retryReadFunc is a function that performs a read operation with retry logic +type retryReadFunc func(retryCount, maxRetries int) *retry.RetryError + +// withRetryRead wraps a read operation with consistent retry logic and exponential backoff +func withRetryRead(ctx context.Context, operation retryReadFunc) error { + retryCount := 0 + maxRetries := 40 + + // Increase overall timeout to better handle eventual consistency when + // multiple resources are created concurrently (e.g., stress tests) + return retry.RetryContext(ctx, 600*time.Second, func() *retry.RetryError { + retryCount++ + + // Backoff with jitter to reduce request stampeding + var baseSleep time.Duration + if retryCount == 1 { + baseSleep = 1 * time.Second + } else { + // Exponential backoff: 1s, 2s, 4s, 8s, 16s (capped at 16s) + baseSleep = time.Duration(1< 0 { + //nolint:gosec // Using math/rand for jitter in retry logic, not cryptographic purposes + j := time.Duration(rand.Int63n(int64(maxJitter))) + time.Sleep(minSleep + j) + } else { + time.Sleep(baseSleep) + } + + // Execute the operation with retry count + return operation(retryCount, maxRetries) + }) +} + +// createRetryableError creates a retryable error with consistent formatting +func createRetryableError(resourceType, resourceName string, retryCount, maxRetries int) *retry.RetryError { + return retry.RetryableError(fmt.Errorf("%s %s not found (attempt %d/%d)", resourceType, resourceName, retryCount, maxRetries)) +} + +// createNonRetryableError creates a non-retryable error with consistent formatting +func createNonRetryableError(resourceType, resourceName string, retryCount int) *retry.RetryError { + return retry.NonRetryableError(fmt.Errorf("%s %s not found after %d retries - may indicate a permanent issue", resourceType, resourceName, retryCount)) +} + +// createAPIError creates a retryable or non-retryable API error based on retry count +func createAPIError(operation string, retryCount, maxRetries int, err error) *retry.RetryError { + if retryCount >= maxRetries { + return retry.NonRetryableError(fmt.Errorf("failed to %s after %d retries: %w", operation, retryCount, err)) + } + return retry.RetryableError(fmt.Errorf("failed to %s: %w", operation, err)) +} diff --git a/internal/resources/asserts/resource_alert_config.go b/internal/resources/asserts/resource_alert_config.go index d2db50968..e87884c2f 100644 --- a/internal/resources/asserts/resource_alert_config.go +++ b/internal/resources/asserts/resource_alert_config.go @@ -3,9 +3,6 @@ package asserts import ( "context" "fmt" - "math" - - "time" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" @@ -70,28 +67,23 @@ func makeResourceAlertConfig() *common.Resource { ).WithLister(assertsListerFunction(listAlertConfigs)) } -func resourceAlertConfigCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceAlertConfigCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Get("name").(string) matchLabels := make(map[string]string) alertLabels := make(map[string]string) if v, ok := d.GetOk("match_labels"); ok { - for k, val := range v.(map[string]any) { + for k, val := range v.(map[string]interface{}) { matchLabels[k] = val.(string) } } if v, ok := d.GetOk("alert_labels"); ok { - for k, val := range v.(map[string]any) { + for k, val := range v.(map[string]interface{}) { alertLabels[k] = val.(string) } } @@ -136,42 +128,23 @@ func resourceAlertConfigCreate(ctx context.Context, d *schema.ResourceData, meta return resourceAlertConfigRead(ctx, d, meta) } -func resourceAlertConfigRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceAlertConfigRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Id() // Retry logic for read operation to handle eventual consistency var foundConfig *assertsapi.AlertConfigDto - retryCount := 0 - maxRetries := 10 - err := retry.RetryContext(ctx, 60*time.Second, func() *retry.RetryError { - retryCount++ - - // Exponential backoff: 1s, 2s, 4s, 8s, etc. (capped at 8s) - if retryCount > 1 { - backoffDuration := time.Duration(1<= maxRetries { - return retry.NonRetryableError(fmt.Errorf("failed to get alert configurations after %d retries: %w", retryCount, err)) - } - return retry.RetryableError(fmt.Errorf("failed to get alert configurations: %w", err)) + return createAPIError("get alert configurations", retryCount, maxRetries, err) } // Find our specific config @@ -182,12 +155,11 @@ func resourceAlertConfigRead(ctx context.Context, d *schema.ResourceData, meta a } } - // If we've retried many times and still not found, give up + // Check if we should give up or retry if retryCount >= maxRetries { - return retry.NonRetryableError(fmt.Errorf("alert configuration %s not found after %d retries - may indicate a permanent issue", name, retryCount)) + return createNonRetryableError("alert configuration", name, retryCount) } - - return retry.RetryableError(fmt.Errorf("alert configuration %s not found (attempt %d/%d)", name, retryCount, maxRetries)) + return createRetryableError("alert configuration", name, retryCount, maxRetries) }) if err != nil { @@ -229,15 +201,10 @@ func resourceAlertConfigRead(ctx context.Context, d *schema.ResourceData, meta a return nil } -func resourceAlertConfigUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceAlertConfigUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Get("name").(string) @@ -245,13 +212,13 @@ func resourceAlertConfigUpdate(ctx context.Context, d *schema.ResourceData, meta alertLabels := make(map[string]string) if v, ok := d.GetOk("match_labels"); ok { - for k, val := range v.(map[string]any) { + for k, val := range v.(map[string]interface{}) { matchLabels[k] = val.(string) } } if v, ok := d.GetOk("alert_labels"); ok { - for k, val := range v.(map[string]any) { + for k, val := range v.(map[string]interface{}) { alertLabels[k] = val.(string) } } @@ -294,15 +261,10 @@ func resourceAlertConfigUpdate(ctx context.Context, d *schema.ResourceData, meta return resourceAlertConfigRead(ctx, d, meta) } -func resourceAlertConfigDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceAlertConfigDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Id() diff --git a/internal/resources/asserts/resource_alert_config_test.go b/internal/resources/asserts/resource_alert_config_test.go index 091c145ac..d72cb8b5d 100644 --- a/internal/resources/asserts/resource_alert_config_test.go +++ b/internal/resources/asserts/resource_alert_config_test.go @@ -6,6 +6,7 @@ import ( "os" "strconv" "testing" + "time" "github.com/grafana/terraform-provider-grafana/v4/internal/common" "github.com/grafana/terraform-provider-grafana/v4/internal/testutils" @@ -117,6 +118,7 @@ func testAccAssertsAlertConfigCheckDestroy(s *terraform.State) error { client := testutils.Provider.Meta().(*common.Client).AssertsAPIClient ctx := context.Background() + deadline := time.Now().Add(60 * time.Second) for _, rs := range s.RootModule().Resources { if rs.Type != "grafana_asserts_notification_alerts_config" { continue @@ -126,24 +128,37 @@ func testAccAssertsAlertConfigCheckDestroy(s *terraform.State) error { name := rs.Primary.ID stackID := fmt.Sprintf("%d", testutils.Provider.Meta().(*common.Client).GrafanaStackID) - // Get all alert configs - request := client.AlertConfigurationAPI.GetAllAlertConfigs(ctx). - XScopeOrgID(stackID) + for { + // Get all alert configs + request := client.AlertConfigurationAPI.GetAllAlertConfigs(ctx). + XScopeOrgID(stackID) + + alertConfigs, _, err := request.Execute() + if err != nil { + // If we can't get configs, assume it's because they don't exist + if common.IsNotFoundError(err) { + break + } + return fmt.Errorf("error checking alert config destruction: %s", err) + } - alertConfigs, _, err := request.Execute() - if err != nil { - // If we can't get configs, assume it's because they don't exist - if common.IsNotFoundError(err) { - continue + // Check if our config still exists + stillExists := false + for _, config := range alertConfigs.AlertConfigs { + if config.Name != nil && *config.Name == name { + stillExists = true + break + } } - return fmt.Errorf("error checking alert config destruction: %s", err) - } - // Check if our config still exists - for _, config := range alertConfigs.AlertConfigs { - if config.Name != nil && *config.Name == name { + if !stillExists { + break + } + + if time.Now().After(deadline) { return fmt.Errorf("alert config %s still exists", name) } + time.Sleep(2 * time.Second) } } @@ -209,11 +224,6 @@ resource "grafana_asserts_notification_alerts_config" "test" { func TestAccAssertsAlertConfig_eventualConsistencyStress(t *testing.T) { testutils.CheckCloudInstanceTestsEnabled(t) - // Skip this flaky test unless explicitly enabled - if !testutils.AccTestsEnabled("TF_ACC_STRESS_TESTS") { - t.Skip("TF_ACC_STRESS_TESTS must be set to a truthy value for stress tests") - } - stackID := getTestStackID(t) baseName := fmt.Sprintf("stress-test-%s", acctest.RandString(8)) diff --git a/internal/resources/asserts/resource_disabled_alert_config.go b/internal/resources/asserts/resource_disabled_alert_config.go index 0c3a70f4d..38b69db0f 100644 --- a/internal/resources/asserts/resource_disabled_alert_config.go +++ b/internal/resources/asserts/resource_disabled_alert_config.go @@ -3,9 +3,6 @@ package asserts import ( "context" "fmt" - "math" - - "time" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" @@ -52,21 +49,16 @@ func makeResourceDisabledAlertConfig() *common.Resource { ).WithLister(assertsListerFunction(listDisabledAlertConfigs)) } -func resourceDisabledAlertConfigCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceDisabledAlertConfigCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Get("name").(string) matchLabels := make(map[string]string) if v, ok := d.GetOk("match_labels"); ok { - for k, val := range v.(map[string]any) { + for k, val := range v.(map[string]interface{}) { matchLabels[k] = val.(string) } } @@ -96,42 +88,23 @@ func resourceDisabledAlertConfigCreate(ctx context.Context, d *schema.ResourceDa return resourceDisabledAlertConfigRead(ctx, d, meta) } -func resourceDisabledAlertConfigRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceDisabledAlertConfigRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Id() // Retry logic for read operation to handle eventual consistency var foundConfig *assertsapi.DisabledAlertConfigDto - retryCount := 0 - maxRetries := 10 - err := retry.RetryContext(ctx, 60*time.Second, func() *retry.RetryError { - retryCount++ - - // Exponential backoff: 1s, 2s, 4s, 8s, etc. (capped at 8s) - if retryCount > 1 { - backoffDuration := time.Duration(1<= maxRetries { - return retry.NonRetryableError(fmt.Errorf("failed to get disabled alert configurations after %d retries: %w", retryCount, err)) - } - return retry.RetryableError(fmt.Errorf("failed to get disabled alert configurations: %w", err)) + return createAPIError("get disabled alert configurations", retryCount, maxRetries, err) } // Find our specific config @@ -142,12 +115,11 @@ func resourceDisabledAlertConfigRead(ctx context.Context, d *schema.ResourceData } } - // If we've retried many times and still not found, give up + // Check if we should give up or retry if retryCount >= maxRetries { - return retry.NonRetryableError(fmt.Errorf("disabled alert configuration %s not found after %d retries - may indicate a permanent issue", name, retryCount)) + return createNonRetryableError("disabled alert configuration", name, retryCount) } - - return retry.RetryableError(fmt.Errorf("disabled alert configuration %s not found (attempt %d/%d)", name, retryCount, maxRetries)) + return createRetryableError("disabled alert configuration", name, retryCount, maxRetries) }) if err != nil { @@ -174,22 +146,17 @@ func resourceDisabledAlertConfigRead(ctx context.Context, d *schema.ResourceData return nil } -func resourceDisabledAlertConfigUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceDisabledAlertConfigUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Get("name").(string) matchLabels := make(map[string]string) if v, ok := d.GetOk("match_labels"); ok { - for k, val := range v.(map[string]any) { + for k, val := range v.(map[string]interface{}) { matchLabels[k] = val.(string) } } @@ -218,15 +185,10 @@ func resourceDisabledAlertConfigUpdate(ctx context.Context, d *schema.ResourceDa return resourceDisabledAlertConfigRead(ctx, d, meta) } -func resourceDisabledAlertConfigDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*common.Client).AssertsAPIClient - if client == nil { - return diag.Errorf("Asserts API client is not configured") - } - - stackID := meta.(*common.Client).GrafanaStackID - if stackID == 0 { - return diag.Errorf("stack_id must be set in provider configuration for Asserts resources") +func resourceDisabledAlertConfigDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, stackID, diags := validateAssertsClient(meta) + if diags.HasError() { + return diags } name := d.Id() diff --git a/internal/resources/asserts/resource_disabled_alert_config_test.go b/internal/resources/asserts/resource_disabled_alert_config_test.go index 47fa563ab..102265aa3 100644 --- a/internal/resources/asserts/resource_disabled_alert_config_test.go +++ b/internal/resources/asserts/resource_disabled_alert_config_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "testing" + "time" "github.com/grafana/terraform-provider-grafana/v4/internal/common" "github.com/grafana/terraform-provider-grafana/v4/internal/testutils" @@ -111,6 +112,7 @@ func testAccAssertsDisabledAlertConfigCheckDestroy(s *terraform.State) error { client := testutils.Provider.Meta().(*common.Client).AssertsAPIClient ctx := context.Background() + deadline := time.Now().Add(60 * time.Second) for _, rs := range s.RootModule().Resources { if rs.Type != "grafana_asserts_suppressed_assertions_config" { continue @@ -120,24 +122,37 @@ func testAccAssertsDisabledAlertConfigCheckDestroy(s *terraform.State) error { name := rs.Primary.ID stackID := fmt.Sprintf("%d", testutils.Provider.Meta().(*common.Client).GrafanaStackID) - // Get all disabled alert configs - request := client.DisabledAlertConfigControllerAPI.GetAllDisabledAlertConfigs(ctx). - XScopeOrgID(stackID) + for { + // Get all disabled alert configs + request := client.DisabledAlertConfigControllerAPI.GetAllDisabledAlertConfigs(ctx). + XScopeOrgID(stackID) + + disabledAlertConfigs, _, err := request.Execute() + if err != nil { + // If we can't get configs, assume it's because they don't exist + if common.IsNotFoundError(err) { + break + } + return fmt.Errorf("error checking disabled alert config destruction: %s", err) + } - disabledAlertConfigs, _, err := request.Execute() - if err != nil { - // If we can't get configs, assume it's because they don't exist - if common.IsNotFoundError(err) { - continue + // Check if our config still exists + stillExists := false + for _, config := range disabledAlertConfigs.DisabledAlertConfigs { + if config.Name != nil && *config.Name == name { + stillExists = true + break + } } - return fmt.Errorf("error checking disabled alert config destruction: %s", err) - } - // Check if our config still exists - for _, config := range disabledAlertConfigs.DisabledAlertConfigs { - if config.Name != nil && *config.Name == name { + if !stillExists { + break + } + + if time.Now().After(deadline) { return fmt.Errorf("disabled alert config %s still exists", name) } + time.Sleep(2 * time.Second) } } @@ -185,11 +200,6 @@ resource "grafana_asserts_suppressed_assertions_config" "test" { func TestAccAssertsDisabledAlertConfig_eventualConsistencyStress(t *testing.T) { testutils.CheckCloudInstanceTestsEnabled(t) - // Skip this flaky test unless explicitly enabled - if !testutils.AccTestsEnabled("TF_ACC_STRESS_TESTS") { - t.Skip("TF_ACC_STRESS_TESTS must be set to a truthy value for stress tests") - } - stackID := getTestStackID(t) baseName := fmt.Sprintf("stress-disabled-%s", acctest.RandString(8)) From ead3656a6d09e41aea48c21a30aa88f5ad618a62 Mon Sep 17 00:00:00 2001 From: matt-metivier Date: Fri, 5 Sep 2025 17:11:06 -0600 Subject: [PATCH 2/2] test: add stress test flag to control eventual consistency tests - Add CheckStressTestsEnabled function to testutils package - Update stress tests to require TF_ACC_STRESS environment variable - Stress tests will now be skipped unless explicitly enabled - This prevents flaky stress test failures in CI while still allowing developers to run them when needed for testing eventual consistency --- internal/resources/asserts/resource_alert_config_test.go | 1 + .../asserts/resource_disabled_alert_config_test.go | 1 + internal/testutils/provider.go | 9 +++++++++ 3 files changed, 11 insertions(+) diff --git a/internal/resources/asserts/resource_alert_config_test.go b/internal/resources/asserts/resource_alert_config_test.go index d72cb8b5d..68119b25b 100644 --- a/internal/resources/asserts/resource_alert_config_test.go +++ b/internal/resources/asserts/resource_alert_config_test.go @@ -223,6 +223,7 @@ resource "grafana_asserts_notification_alerts_config" "test" { // to verify the retry logic handles eventual consistency properly func TestAccAssertsAlertConfig_eventualConsistencyStress(t *testing.T) { testutils.CheckCloudInstanceTestsEnabled(t) + testutils.CheckStressTestsEnabled(t) stackID := getTestStackID(t) baseName := fmt.Sprintf("stress-test-%s", acctest.RandString(8)) diff --git a/internal/resources/asserts/resource_disabled_alert_config_test.go b/internal/resources/asserts/resource_disabled_alert_config_test.go index 102265aa3..f09ac548b 100644 --- a/internal/resources/asserts/resource_disabled_alert_config_test.go +++ b/internal/resources/asserts/resource_disabled_alert_config_test.go @@ -199,6 +199,7 @@ resource "grafana_asserts_suppressed_assertions_config" "test" { // to verify the retry logic handles eventual consistency properly func TestAccAssertsDisabledAlertConfig_eventualConsistencyStress(t *testing.T) { testutils.CheckCloudInstanceTestsEnabled(t) + testutils.CheckStressTestsEnabled(t) stackID := getTestStackID(t) baseName := fmt.Sprintf("stress-disabled-%s", acctest.RandString(8)) diff --git a/internal/testutils/provider.go b/internal/testutils/provider.go index a7e5b36c7..d711d7a20 100644 --- a/internal/testutils/provider.go +++ b/internal/testutils/provider.go @@ -215,6 +215,15 @@ func CheckEnterpriseTestsEnabled(t *testing.T, semverConstraintOptional ...strin checkSemverConstraint(t, semverConstraintOptional...) } +// CheckStressTestsEnabled checks if the stress tests are enabled. This should be the first line of any test that tests eventual consistency under high load +func CheckStressTestsEnabled(t *testing.T) { + t.Helper() + + if !AccTestsEnabled("TF_ACC_STRESS") { + t.Skip("TF_ACC_STRESS must be set to a truthy value for stress tests") + } +} + func checkSemverConstraint(t *testing.T, semverConstraintOptional ...string) { t.Helper()