make grpc status err FailedPrecondition retryable (#4840) (#9305)

Signed-off-by: Modular Magician <magic-modules@google.com>

.changelog/4840.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+bigtable: fixed bug that would error if creating multiple bigtable gc policies at the same time
+```

google/error_retry_predicates.go

@@ -10,6 +10,7 @@ import (
 
 	"google.golang.org/api/googleapi"
 	sqladmin "google.golang.org/api/sqladmin/v1beta4"
+	"google.golang.org/grpc/status"
 )
 
 type RetryErrorPredicateFunc func(error) (bool, string)
@@ -370,3 +371,15 @@ func iamServiceAccountNotFound(err error) (bool, string) {
 
 	return false, ""
 }
+
+// Big Table uses gRPC and thus does not return errors of type *googleapi.Error.
+// Instead the errors returned are *status.Error. See the types of codes returned
+// here (https://pkg.go.dev/google.golang.org/grpc/codes#Code).
+func isBigTableRetryableError(err error) (bool, string) {
+	statusCode := status.Code(err)
+	if statusCode.String() == "FailedPrecondition" {
+		return true, "Waiting for table to be in a valid state"
+	}
+
+	return false, ""
+}

google/error_retry_predicates_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 
 	"google.golang.org/api/googleapi"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestIsAppEngineRetryableError_operationInProgress(t *testing.T) {
@@ -89,3 +91,21 @@ func TestIsOperationReadQuotaError_quotaExceeded(t *testing.T) {
 		t.Errorf("Error not detected as retryable")
 	}
 }
+
+func TestGRPCRetryable(t *testing.T) {
+	code := codes.FailedPrecondition
+	err := status.Error(code, "is retryable")
+	isRetryable, _ := isBigTableRetryableError(err)
+	if !isRetryable {
+		t.Errorf("Error not detected as retryable")
+	}
+}
+
+func TestGRPCNotRetryable(t *testing.T) {
+	code := codes.InvalidArgument
+	err := status.Error(code, "is noto retryable")
+	isRetryable, _ := isBigTableRetryableError(err)
+	if isRetryable {
+		t.Errorf("Error incorrectly detected as retryable")
+	}
+}

google/resource_bigtable_gc_policy.go

@@ -184,7 +184,11 @@ func resourceBigtableGCPolicyCreate(d *schema.ResourceData, meta interface{}) er
 	tableName := d.Get("table").(string)
 	columnFamily := d.Get("column_family").(string)
 
-	if err := c.SetGCPolicy(ctx, tableName, columnFamily, gcPolicy); err != nil {
+	err = retryTimeDuration(func() error {
+		reqErr := c.SetGCPolicy(ctx, tableName, columnFamily, gcPolicy)
+		return reqErr
+	}, d.Timeout(schema.TimeoutCreate), isBigTableRetryableError)
+	if err != nil {
 		return err
 	}
 
@@ -266,7 +270,11 @@ func resourceBigtableGCPolicyDestroy(d *schema.ResourceData, meta interface{}) e
 
 	defer c.Close()
 
-	if err := c.SetGCPolicy(ctx, d.Get("table").(string), d.Get("column_family").(string), bigtable.NoGcPolicy()); err != nil {
+	err = retryTimeDuration(func() error {
+		reqErr := c.SetGCPolicy(ctx, d.Get("table").(string), d.Get("column_family").(string), bigtable.NoGcPolicy())
+		return reqErr
+	}, d.Timeout(schema.TimeoutDelete), isBigTableRetryableError)
+	if err != nil {
 		return err
 	}
 

google/resource_bigtable_gc_policy_test.go

@@ -91,6 +91,35 @@ func TestAccBigtableGCPolicy_union(t *testing.T) {
 	})
 }
 
+func TestAccBigtableGCPolicy_multiplePolicies(t *testing.T) {
+	// bigtable instance does not use the shared HTTP client, this test creates an instance
+	skipIfVcr(t)
+	t.Parallel()
+
+	instanceName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+	tableName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+	familyName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckBigtableGCPolicyDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccBigtableGCPolicy_multiplePolicies(instanceName, tableName, familyName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccBigtableGCPolicyExists(
+						t, "google_bigtable_gc_policy.policyA"),
+					testAccBigtableGCPolicyExists(
+						t, "google_bigtable_gc_policy.policyB"),
+					testAccBigtableGCPolicyExists(
+						t, "google_bigtable_gc_policy.policyC"),
+				),
+			},
+		},
+	})
+}
+
 func TestUnitBigtableGCPolicy_customizeDiff(t *testing.T) {
 	for _, tc := range testUnitBigtableGCPolicyCustomizeDiffTestcases {
 		tc.check(t)
@@ -347,3 +376,64 @@ resource "google_bigtable_gc_policy" "policy" {
 }
 `, instanceName, instanceName, tableName, family, family)
 }
+
+func testAccBigtableGCPolicy_multiplePolicies(instanceName, tableName, family string) string {
+	return fmt.Sprintf(`
+resource "google_bigtable_instance" "instance" {
+  name = "%s"
+
+  cluster {
+    cluster_id = "%s"
+    zone       = "us-central1-b"
+  }
+
+  instance_type = "DEVELOPMENT"
+  deletion_protection = false
+}
+
+resource "google_bigtable_table" "table" {
+  name          = "%s"
+  instance_name = google_bigtable_instance.instance.id
+
+  column_family {
+    family = "%s"
+  }
+}
+
+resource "google_bigtable_gc_policy" "policyA" {
+  instance_name = google_bigtable_instance.instance.id
+  table         = google_bigtable_table.table.name
+  column_family = "%s"
+
+  max_age {
+    days = 30
+  }
+}
+
+resource "google_bigtable_gc_policy" "policyB" {
+  instance_name = google_bigtable_instance.instance.id
+  table         = google_bigtable_table.table.name
+  column_family = "%s"
+
+  max_version {
+    number = 8
+  }
+}
+
+resource "google_bigtable_gc_policy" "policyC" {
+	instance_name = google_bigtable_instance.instance.id
+  table         = google_bigtable_table.table.name
+  column_family = "%s"
+
+  max_age {
+    days = 7
+  }
+
+  max_version {
+    number = 10
+  }
+
+  mode        = "UNION"
+}
+`, instanceName, instanceName, tableName, family, family, family, family)
+}