Skip to content

Commit 1c29293

Browse files
modular-magicianmodular-magician
authored
Retry on Cloud Run 409s (foreground deletion) (#4460) (#8440)
Signed-off-by: Modular Magician <magic-modules@google.com>
1 parent 2425e7f commit 1c29293

9 files changed

+186
-13
lines changed

‎.changelog/4460.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:enhancement
2+
cloudrun: Terraform will attempt to retry 409 errors from the Cloud Run API, which may be returned intermittently on create.
3+
```

‎google/error_retry_predicates.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,3 +327,17 @@ func healthcareDatasetNotInitialized(err error) (bool, string) {
327327
}
328328
return false, ""
329329
}
330+
331+
// Cloud Run APIs may return a 409 on create to indicate that a resource has been deleted in the foreground
332+
// (eg GET and LIST) but not the backing apiserver. When we encounter a 409, we can retry it.
333+
// Note that due to limitations in MMv1's error_retry_predicates this is currently applied to all requests.
334+
// We only expect to receive it on create, though.
335+
func isCloudRunCreationConflict(err error) (bool, string) {
336+
if gerr, ok := err.(*googleapi.Error); ok {
337+
if gerr.Code == 409 {
338+
return true, "saw a 409 - waiting until background deletion completes"
339+
}
340+
}
341+
342+
return false, ""
343+
}

‎google/iam_cloud_run_service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ func (u *CloudRunServiceIamUpdater) GetResourceIamPolicy() (*cloudresourcemanage
155155
return nil, err
156156
}
157157

158-
policy, err := sendRequest(u.Config, "GET", project, url, userAgent, obj)
158+
policy, err := sendRequest(u.Config, "GET", project, url, userAgent, obj, isCloudRunCreationConflict)
159159
if err != nil {
160160
return nil, errwrap.Wrapf(fmt.Sprintf("Error retrieving IAM policy for %s: {{err}}", u.DescribeResource()), err)
161161
}
@@ -192,7 +192,7 @@ func (u *CloudRunServiceIamUpdater) SetResourceIamPolicy(policy *cloudresourcema
192192
return err
193193
}
194194

195-
_, err = sendRequestWithTimeout(u.Config, "POST", project, url, userAgent, obj, u.d.Timeout(schema.TimeoutCreate))
195+
_, err = sendRequestWithTimeout(u.Config, "POST", project, url, userAgent, obj, u.d.Timeout(schema.TimeoutCreate), isCloudRunCreationConflict)
196196
if err != nil {
197197
return errwrap.Wrapf(fmt.Sprintf("Error setting IAM policy for %s: {{err}}", u.DescribeResource()), err)
198198
}

‎google/resource_cloud_run_domain_mapping.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ func resourceCloudRunDomainMappingCreate(d *schema.ResourceData, meta interface{
318318
billingProject = bp
319319
}
320320

321-
res, err := sendRequestWithTimeout(config, "POST", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutCreate))
321+
res, err := sendRequestWithTimeout(config, "POST", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutCreate), isCloudRunCreationConflict)
322322
if err != nil {
323323
return fmt.Errorf("Error creating DomainMapping: %s", err)
324324
}
@@ -367,7 +367,7 @@ func resourceCloudRunDomainMappingPollRead(d *schema.ResourceData, meta interfac
367367
return nil, err
368368
}
369369

370-
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil)
370+
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil, isCloudRunCreationConflict)
371371
if err != nil {
372372
return res, err
373373
}
@@ -412,7 +412,7 @@ func resourceCloudRunDomainMappingRead(d *schema.ResourceData, meta interface{})
412412
billingProject = bp
413413
}
414414

415-
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil)
415+
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil, isCloudRunCreationConflict)
416416
if err != nil {
417417
return handleNotFoundError(err, d, fmt.Sprintf("CloudRunDomainMapping %q", d.Id()))
418418
}
@@ -474,7 +474,7 @@ func resourceCloudRunDomainMappingDelete(d *schema.ResourceData, meta interface{
474474
billingProject = bp
475475
}
476476

477-
res, err := sendRequestWithTimeout(config, "DELETE", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutDelete))
477+
res, err := sendRequestWithTimeout(config, "DELETE", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutDelete), isCloudRunCreationConflict)
478478
if err != nil {
479479
return handleNotFoundError(err, d, "DomainMapping")
480480
}

‎google/resource_cloud_run_domain_mapping_generated_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func testAccCheckCloudRunDomainMappingDestroyProducer(t *testing.T) func(s *terr
110110
billingProject = config.BillingProject
111111
}
112112

113-
_, err = sendRequest(config, "GET", billingProject, url, config.userAgent, nil)
113+
_, err = sendRequest(config, "GET", billingProject, url, config.userAgent, nil, isCloudRunCreationConflict)
114114
if err == nil {
115115
return fmt.Errorf("CloudRunDomainMapping still exists at %s", url)
116116
}

‎google/resource_cloud_run_domain_mapping_test.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package google
2+
3+
import (
4+
"regexp"
5+
"testing"
6+
7+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
8+
)
9+
10+
// Destroy and recreate the mapping, testing that Terraform doesn't return a 409
11+
func TestAccCloudRunDomainMapping_foregroundDeletion(t *testing.T) {
12+
t.Parallel()
13+
14+
context := map[string]interface{}{
15+
"namespace": getTestProjectFromEnv(),
16+
"random_suffix": randString(t, 10),
17+
}
18+
19+
vcrTest(t, resource.TestCase{
20+
PreCheck: func() { testAccPreCheck(t) },
21+
Providers: testAccProviders,
22+
ExternalProviders: map[string]resource.ExternalProvider{
23+
"random": {},
24+
},
25+
CheckDestroy: testAccCheckCloudRunDomainMappingDestroyProducer(t),
26+
Steps: []resource.TestStep{
27+
{
28+
Config: testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated1(context),
29+
},
30+
{
31+
ResourceName: "google_cloud_run_domain_mapping.default",
32+
ImportState: true,
33+
ImportStateVerify: true,
34+
ImportStateVerifyIgnore: []string{"name", "location", "status", "metadata.0.resource_version"},
35+
},
36+
{
37+
Config: testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated2(context),
38+
ExpectError: regexp.MustCompile("Domain mapping already exists"), // when this error is no longer returned, remove ExpectError and add the ISV below.
39+
},
40+
/*{
41+
ResourceName: "google_cloud_run_domain_mapping.default",
42+
ImportState: true,
43+
ImportStateVerify: true,
44+
ImportStateVerifyIgnore: []string{"name", "location", "status", "metadata.0.resource_version"},
45+
},*/
46+
},
47+
})
48+
}
49+
50+
func testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated1(context map[string]interface{}) string {
51+
return Nprintf(`
52+
resource "google_cloud_run_service" "default" {
53+
name = "tf-test-cloudrun-srv%{random_suffix}"
54+
location = "us-central1"
55+
56+
metadata {
57+
namespace = "%{namespace}"
58+
}
59+
60+
template {
61+
spec {
62+
containers {
63+
image = "us-docker.pkg.dev/cloudrun/container/hello"
64+
}
65+
}
66+
}
67+
}
68+
69+
resource "google_cloud_run_domain_mapping" "default" {
70+
location = "us-central1"
71+
name = "tf-test-domain%{random_suffix}.gcp.tfacc.hashicorptest.com"
72+
73+
metadata {
74+
namespace = "%{namespace}"
75+
}
76+
77+
spec {
78+
route_name = google_cloud_run_service.default.name
79+
}
80+
}
81+
`, context)
82+
}
83+
84+
func testAccCloudRunDomainMapping_cloudRunDomainMappingUpdated2(context map[string]interface{}) string {
85+
return Nprintf(`
86+
87+
resource "google_cloud_run_service" "default" {
88+
name = "tf-test-cloudrun-srv%{random_suffix}"
89+
location = "us-central1"
90+
91+
metadata {
92+
namespace = "%{namespace}"
93+
}
94+
95+
template {
96+
spec {
97+
containers {
98+
image = "us-docker.pkg.dev/cloudrun/container/hello"
99+
}
100+
}
101+
}
102+
}
103+
104+
resource "google_cloud_run_domain_mapping" "default" {
105+
location = "us-central1"
106+
name = "tf-test-domain%{random_suffix}.gcp.tfacc.hashicorptest.com"
107+
108+
metadata {
109+
namespace = "%{namespace}"
110+
labels = {
111+
"my-label" = "my-value"
112+
}
113+
}
114+
115+
spec {
116+
route_name = google_cloud_run_service.default.name
117+
}
118+
}
119+
`, context)
120+
}

‎google/resource_cloud_run_service.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,7 @@ func resourceCloudRunServiceCreate(d *schema.ResourceData, meta interface{}) err
700700
billingProject = bp
701701
}
702702

703-
res, err := sendRequestWithTimeout(config, "POST", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutCreate))
703+
res, err := sendRequestWithTimeout(config, "POST", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutCreate), isCloudRunCreationConflict)
704704
if err != nil {
705705
return fmt.Errorf("Error creating Service: %s", err)
706706
}
@@ -749,7 +749,7 @@ func resourceCloudRunServicePollRead(d *schema.ResourceData, meta interface{}) P
749749
return nil, err
750750
}
751751

752-
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil)
752+
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil, isCloudRunCreationConflict)
753753
if err != nil {
754754
return res, err
755755
}
@@ -794,7 +794,7 @@ func resourceCloudRunServiceRead(d *schema.ResourceData, meta interface{}) error
794794
billingProject = bp
795795
}
796796

797-
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil)
797+
res, err := sendRequest(config, "GET", billingProject, url, userAgent, nil, isCloudRunCreationConflict)
798798
if err != nil {
799799
return handleNotFoundError(err, d, fmt.Sprintf("CloudRunService %q", d.Id()))
800800
}
@@ -892,7 +892,7 @@ func resourceCloudRunServiceUpdate(d *schema.ResourceData, meta interface{}) err
892892
billingProject = bp
893893
}
894894

895-
res, err := sendRequestWithTimeout(config, "PUT", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutUpdate))
895+
res, err := sendRequestWithTimeout(config, "PUT", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutUpdate), isCloudRunCreationConflict)
896896

897897
if err != nil {
898898
return fmt.Errorf("Error updating Service %q: %s", d.Id(), err)
@@ -936,7 +936,7 @@ func resourceCloudRunServiceDelete(d *schema.ResourceData, meta interface{}) err
936936
billingProject = bp
937937
}
938938

939-
res, err := sendRequestWithTimeout(config, "DELETE", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutDelete))
939+
res, err := sendRequestWithTimeout(config, "DELETE", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutDelete), isCloudRunCreationConflict)
940940
if err != nil {
941941
return handleNotFoundError(err, d, "Service")
942942
}

‎google/resource_cloud_run_service_generated_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ func testAccCheckCloudRunServiceDestroyProducer(t *testing.T) func(s *terraform.
297297
billingProject = config.BillingProject
298298
}
299299

300-
_, err = sendRequest(config, "GET", billingProject, url, config.userAgent, nil)
300+
_, err = sendRequest(config, "GET", billingProject, url, config.userAgent, nil, isCloudRunCreationConflict)
301301
if err == nil {
302302
return fmt.Errorf("CloudRunService still exists at %s", url)
303303
}

‎google/resource_cloud_run_service_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,42 @@ func TestAccCloudRunService_cloudRunServiceUpdate(t *testing.T) {
3939
})
4040
}
4141

42+
// this test checks that Terraform does not fail with a 409 recreating the same service
43+
func TestAccCloudRunService_foregroundDeletion(t *testing.T) {
44+
t.Parallel()
45+
46+
project := getTestProjectFromEnv()
47+
name := "tftest-cloudrun-" + randString(t, 6)
48+
49+
vcrTest(t, resource.TestCase{
50+
PreCheck: func() { testAccPreCheck(t) },
51+
Providers: testAccProviders,
52+
Steps: []resource.TestStep{
53+
{
54+
Config: testAccCloudRunService_cloudRunServiceUpdate(name, project, "10", "600"),
55+
},
56+
{
57+
ResourceName: "google_cloud_run_service.default",
58+
ImportState: true,
59+
ImportStateVerify: true,
60+
ImportStateVerifyIgnore: []string{"metadata.0.resource_version", "status.0.conditions"},
61+
},
62+
{
63+
Config: " ", // very explicitly add a space, as the test runner fails if this is just ""
64+
},
65+
{
66+
Config: testAccCloudRunService_cloudRunServiceUpdate(name, project, "10", "600"),
67+
},
68+
{
69+
ResourceName: "google_cloud_run_service.default",
70+
ImportState: true,
71+
ImportStateVerify: true,
72+
ImportStateVerifyIgnore: []string{"metadata.0.resource_version", "status.0.conditions"},
73+
},
74+
},
75+
})
76+
}
77+
4278
func testAccCloudRunService_cloudRunServiceUpdate(name, project, concurrency, timeoutSeconds string) string {
4379
return fmt.Sprintf(`
4480
resource "google_cloud_run_service" "default" {

0 commit comments

Comments
 (0)