diff --git a/mmv1/products/cloudrun/terraform.yaml b/mmv1/products/cloudrun/terraform.yaml index b8141e34949d..c08369684ca1 100644 --- a/mmv1/products/cloudrun/terraform.yaml +++ b/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'] diff --git a/mmv1/third_party/terraform/tests/resource_cloud_run_domain_mapping_test.go b/mmv1/third_party/terraform/tests/resource_cloud_run_domain_mapping_test.go new file mode 100644 index 000000000000..07ca331c3a2c --- /dev/null +++ b/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) +} diff --git a/mmv1/third_party/terraform/tests/resource_cloud_run_service_test.go b/mmv1/third_party/terraform/tests/resource_cloud_run_service_test.go index 438e7194aedd..fb8e04166882 100644 --- a/mmv1/third_party/terraform/tests/resource_cloud_run_service_test.go +++ b/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" { diff --git a/mmv1/third_party/terraform/utils/error_retry_predicates.go b/mmv1/third_party/terraform/utils/error_retry_predicates.go index 9e9f133db305..c6f126db3974 100644 --- a/mmv1/third_party/terraform/utils/error_retry_predicates.go +++ b/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, "" +}