Retry on Cloud Run 409s (foreground deletion)

mmv1/products/cloudrun/terraform.yaml

@@ -16,6 +16,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
   DomainMapping: !ruby/object:Overrides::Terraform::ResourceOverride
     id_format: "locations/{{location}}/namespaces/{{project}}/domainmappings/{{name}}"
     import_format: ["locations/{{location}}/namespaces/{{project}}/domainmappings/{{name}}"]
+    error_retry_predicates: ["isCloudRunCreationConflict"]
     async: !ruby/object:Provider::Terraform::PollAsync
       check_response_func_existence: PollCheckKnativeStatus
       actions: ['create', 'update']
@@ -62,6 +63,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
   Service: !ruby/object:Overrides::Terraform::ResourceOverride
     id_format: "locations/{{location}}/namespaces/{{project}}/services/{{name}}"
     import_format: ["locations/{{location}}/namespaces/{{project}}/services/{{name}}"]
+    error_retry_predicates: ["isCloudRunCreationConflict"]
     async: !ruby/object:Provider::Terraform::PollAsync
       check_response_func_existence: PollCheckKnativeStatus
       actions: ['create', 'update']

mmv1/third_party/terraform/tests/resource_cloud_run_domain_mapping_test.go

@@ -0,0 +1,120 @@
+package google
+
+import (
+	"regexp"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+)
+
+// Destroy and recreate the mapping, testing that Terraform doesn't return a 409
+func TestAccCloudRunDomainMapping_foregroundDeletion(t *testing.T) {
+	t.Parallel()
+
+	context := map[string]interface{}{
+		"namespace":     getTestProjectFromEnv(),
+		"random_suffix": randString(t, 10),
+	}
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		ExternalProviders: map[string]resource.ExternalProvider{
+			"random": {},
+		},
+		CheckDestroy: testAccCheckCloudRunDomainMappingDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated1(context),
+			},
+			{
+				ResourceName:            "google_cloud_run_domain_mapping.default",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"name", "location", "status", "metadata.0.resource_version"},
+			},
+			{
+				Config:      testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated2(context),
+				ExpectError: regexp.MustCompile("Domain mapping already exists"), // when this error is no longer returned, remove ExpectError and add the ISV below.
+			},
+			/*{
+				ResourceName:            "google_cloud_run_domain_mapping.default",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"name", "location", "status", "metadata.0.resource_version"},
+			},*/
+		},
+	})
+}
+
+func testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated1(context map[string]interface{}) string {
+	return Nprintf(`
+resource "google_cloud_run_service" "default" {
+    name     = "tf-test-cloudrun-srv%{random_suffix}"
+    location = "us-central1"
+
+    metadata {
+      namespace = "%{namespace}"
+    }
+
+    template {
+      spec {
+        containers {
+          image = "us-docker.pkg.dev/cloudrun/container/hello"
+        }
+      }
+    }
+  }
+
+resource "google_cloud_run_domain_mapping" "default" {
+  location = "us-central1"
+  name     = "tf-test-domain%{random_suffix}.gcp.tfacc.hashicorptest.com"
+
+  metadata {
+    namespace = "%{namespace}"
+  }
+
+  spec {
+    route_name = google_cloud_run_service.default.name
+  }
+}
+`, context)
+}
+
+func testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated2(context map[string]interface{}) string {
+	return Nprintf(`
+
+resource "google_cloud_run_service" "default" {
+  name     = "tf-test-cloudrun-srv%{random_suffix}"
+  location = "us-central1"
+
+  metadata {
+    namespace = "%{namespace}"
+  }
+
+  template {
+    spec {
+      containers {
+        image = "us-docker.pkg.dev/cloudrun/container/hello"
+      }
+    }
+  }
+}
+
+resource "google_cloud_run_domain_mapping" "default" {
+  location = "us-central1"
+  name     = "tf-test-domain%{random_suffix}.gcp.tfacc.hashicorptest.com"
+
+  metadata {
+    namespace = "%{namespace}"
+    labels = {
+      "my-label" = "my-value"
+    }
+  }
+
+  spec {
+    route_name = google_cloud_run_service.default.name
+  }
+}
+`, context)
+}

mmv1/third_party/terraform/tests/resource_cloud_run_service_test.go

@@ -39,6 +39,42 @@ func TestAccCloudRunService_cloudRunServiceUpdate(t *testing.T) {
 	})
 }
 
+// this test checks that Terraform does not fail with a 409 recreating the same service
+func TestAccCloudRunService_foregroundDeletion(t *testing.T) {
+	t.Parallel()
+
+	project := getTestProjectFromEnv()
+	name := "tftest-cloudrun-" + randString(t, 6)
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCloudRunService_cloudRunServiceUpdate(name, project, "10", "600"),
+			},
+			{
+				ResourceName:            "google_cloud_run_service.default",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"metadata.0.resource_version", "status.0.conditions"},
+			},
+			{
+				Config: " ", // very explicitly add a space, as the test runner fails if this is just ""
+			},
+			{
+				Config: testAccCloudRunService_cloudRunServiceUpdate(name, project, "10", "600"),
+			},
+			{
+				ResourceName:            "google_cloud_run_service.default",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"metadata.0.resource_version", "status.0.conditions"},
+			},
+		},
+	})
+}
+
 func testAccCloudRunService_cloudRunServiceUpdate(name, project, concurrency, timeoutSeconds string) string {
 	return fmt.Sprintf(`
 resource "google_cloud_run_service" "default" {

mmv1/third_party/terraform/utils/error_retry_predicates.go

@@ -327,3 +327,17 @@ func healthcareDatasetNotInitialized(err error) (bool, string) {
 	}
 	return false, ""
 }
+
+// Cloud Run APIs may return a 409 on create to indicate that a resource has been deleted in the foreground
+// (eg GET and LIST) but not the backing apiserver. When we encounter a 409, we can retry it.
+// Note that due to limitations in MMv1's error_retry_predicates this is currently applied to all requests.
+// We only expect to receive it on create, though.
+func isCloudRunCreationConflict(err error) (bool, string) {
+	if gerr, ok := err.(*googleapi.Error); ok {
+		if gerr.Code == 409 {
+			return true, "saw a 409 - waiting until background deletion completes"
+		}
+	}
+
+	return false, ""
+}