Add retries/backoff when (only) reading IAM policies (#1665)

Merged PR #1665.
GoogleCloudPlatform · Apr 19, 2019 · e74a54c · e74a54c
1 parent c4d7a82
commit e74a54c
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 7 deletions.
diff --git a/build/terraform b/build/terraform
diff --git a/build/terraform-beta b/build/terraform-beta
diff --git a/third_party/terraform/resources/resource_iam_audit_config.go b/third_party/terraform/resources/resource_iam_audit_config.go
@@ -86,7 +86,7 @@ func resourceIamAuditConfigRead(newUpdaterFunc newResourceIamUpdaterFunc) schema
 		}
 
 		eAuditConfig := getResourceIamAuditConfig(d)
-		p, err := updater.GetResourceIamPolicy()
+		p, err := iamPolicyReadWithRetry(updater)
 		if err != nil {
 			if isGoogleApiErrorWithCode(err, 404) {
 				log.Printf("[DEBUG]: AuditConfig for service %q not found for non-existent resource %s, removing from state file.", eAuditConfig.Service, updater.DescribeResource())

diff --git a/third_party/terraform/resources/resource_iam_binding.go b/third_party/terraform/resources/resource_iam_binding.go
@@ -77,7 +77,7 @@ func resourceIamBindingRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Rea
 		}
 
 		eBinding := getResourceIamBinding(d)
-		p, err := updater.GetResourceIamPolicy()
+		p, err := iamPolicyReadWithRetry(updater)
 		if err != nil {
 			if isGoogleApiErrorWithCode(err, 404) {
 				log.Printf("[DEBUG]: Binding for role %q not found for non-existent resource %s, removing from state file.", updater.DescribeResource(), eBinding.Role)

diff --git a/third_party/terraform/resources/resource_iam_member.go b/third_party/terraform/resources/resource_iam_member.go
@@ -112,7 +112,7 @@ func resourceIamMemberRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read
 		}
 
 		eMember := getResourceIamMember(d)
-		p, err := updater.GetResourceIamPolicy()
+		p, err := iamPolicyReadWithRetry(updater)
 		if err != nil {
 			if isGoogleApiErrorWithCode(err, 404) {
 				log.Printf("[DEBUG]: Binding of member %q with role %q does not exist for non-existent resource %s, removing from state.", eMember.Members[0], eMember.Role, updater.DescribeResource())

diff --git a/third_party/terraform/resources/resource_iam_policy.go b/third_party/terraform/resources/resource_iam_policy.go
@@ -81,7 +81,7 @@ func ResourceIamPolicyRead(newUpdaterFunc newResourceIamUpdaterFunc) schema.Read
 			return err
 		}
 
-		policy, err := updater.GetResourceIamPolicy()
+		policy, err := iamPolicyReadWithRetry(updater)
 		if err != nil {
 			if isGoogleApiErrorWithCode(err, 404) {
 				log.Printf("[DEBUG]: Policy does not exist for non-existent resource %q", updater.GetResourceId())

diff --git a/third_party/terraform/utils/iam.go b/third_party/terraform/utils/iam.go
@@ -10,6 +10,8 @@ import (
 	"google.golang.org/api/cloudresourcemanager/v1"
 )
 
+const maxBackoffSeconds = 30
+
 // The ResourceIamUpdater interface is implemented for each GCP resource supporting IAM policy.
 //
 // Implementations should keep track of the resource identifier.
@@ -44,6 +46,26 @@ type iamPolicyModifyFunc func(p *cloudresourcemanager.Policy) error
 
 type resourceIdParserFunc func(d *schema.ResourceData, config *Config) error
 
+// Wrapper around updater.GetResourceIamPolicy() to handle retry/backoff
+// for just reading policies from IAM
+func iamPolicyReadWithRetry(updater ResourceIamUpdater) (*cloudresourcemanager.Policy, error) {
+	mutexKey := updater.GetMutexKey()
+	mutexKV.Lock(mutexKey)
+	defer mutexKV.Unlock(mutexKey)
+
+	log.Printf("[DEBUG] Retrieving policy for %s\n", updater.DescribeResource())
+	var policy *cloudresourcemanager.Policy
+	err := retryTime(func() (perr error) {
+		policy, perr = updater.GetResourceIamPolicy()
+		return perr
+	}, 10)
+	if err != nil {
+		return nil, err
+	}
+	log.Printf("[DEBUG] Retrieved policy for %s: %+v\n", updater.DescribeResource(), policy)
+	return policy, nil
+}
+
 func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModifyFunc) error {
 	mutexKey := updater.GetMutexKey()
 	mutexKV.Lock(mutexKey)
@@ -54,6 +76,7 @@ func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModify
 		log.Printf("[DEBUG]: Retrieving policy for %s\n", updater.DescribeResource())
 		p, err := updater.GetResourceIamPolicy()
 		if isGoogleApiErrorWithCode(err, 429) {
+			log.Printf("[DEBUG] 429 while attempting to read policy for %s, waiting %v before attempting again", updater.DescribeResource(), backoff)
 			time.Sleep(backoff)
 			continue
 		} else if err != nil {
@@ -71,7 +94,7 @@ func iamPolicyReadModifyWrite(updater ResourceIamUpdater, modify iamPolicyModify
 		if err == nil {
 			fetchBackoff := 1 * time.Second
 			for successfulFetches := 0; successfulFetches < 3; {
-				if fetchBackoff > 30*time.Second {
+				if fetchBackoff > maxBackoffSeconds*time.Second {
 					return fmt.Errorf("Error applying IAM policy to %s: Waited too long for propagation.\n", updater.DescribeResource())
 				}
 				time.Sleep(fetchBackoff)