From 2607a03554fed82af62b24856f6d456fb2ebbe7f Mon Sep 17 00:00:00 2001
From: The Magician <magic-modules@google.com>
Date: Wed, 27 May 2020 11:59:35 -0700
Subject: [PATCH] Add precondition retry to node pool deletion - upstreams
 tpg#6334 (#3515) (#2115)

* add precondition retry to node pool deletion - upstreams tpg#6335

* switch to Delete

Signed-off-by: Modular Magician <magic-modules@google.com>
---
 .changelog/3515.txt                         |  3 +++
 google-beta/resource_container_node_pool.go | 27 ++++++++++++---------
 2 files changed, 19 insertions(+), 11 deletions(-)
 create mode 100644 .changelog/3515.txt

diff --git a/.changelog/3515.txt b/.changelog/3515.txt
new file mode 100644
index 0000000000..c1f80b7648
--- /dev/null
+++ b/.changelog/3515.txt
@@ -0,0 +1,3 @@
+```release-note:enhancement
+container: Changed retry logic for `google_container_node_pool` deletion to use timeouts and retry errors more specifically when cluster is updating.
+```
diff --git a/google-beta/resource_container_node_pool.go b/google-beta/resource_container_node_pool.go
index 74be787988..7df13916d6 100644
--- a/google-beta/resource_container_node_pool.go
+++ b/google-beta/resource_container_node_pool.go
@@ -395,20 +395,23 @@ func resourceContainerNodePoolDelete(d *schema.ResourceData, meta interface{}) e
 	mutexKV.Lock(nodePoolInfo.lockKey())
 	defer mutexKV.Unlock(nodePoolInfo.lockKey())
 
-	var op = &containerBeta.Operation{}
-	var count = 0
-	err = resource.Retry(30*time.Second, func() *resource.RetryError {
-		count++
-		op, err = config.clientContainerBeta.Projects.Locations.
-			Clusters.NodePools.Delete(nodePoolInfo.fullyQualifiedName(name)).Do()
+	timeout := d.Timeout(schema.TimeoutDelete)
+	startTime := time.Now()
+
+	var operation *containerBeta.Operation
+	err = resource.Retry(timeout, func() *resource.RetryError {
+		operation, err = config.clientContainerBeta.
+			Projects.Locations.Clusters.NodePools.Delete(nodePoolInfo.fullyQualifiedName(name)).Do()
 
 		if err != nil {
-			return resource.RetryableError(err)
+			if isFailedPreconditionError(err) {
+				// We get failed precondition errors if the cluster is updating
+				// while we try to delete the node pool.
+				return resource.RetryableError(err)
+			}
+			return resource.NonRetryableError(err)
 		}
 
-		if count == 15 {
-			return resource.NonRetryableError(fmt.Errorf("Error retrying to delete node pool %s", name))
-		}
 		return nil
 	})
 
@@ -416,8 +419,10 @@ func resourceContainerNodePoolDelete(d *schema.ResourceData, meta interface{}) e
 		return fmt.Errorf("Error deleting NodePool: %s", err)
 	}
 
+	timeout -= time.Since(startTime)
+
 	// Wait until it's deleted
-	waitErr := containerOperationWait(config, op, nodePoolInfo.project, nodePoolInfo.location, "deleting GKE NodePool", d.Timeout(schema.TimeoutDelete))
+	waitErr := containerOperationWait(config, operation, nodePoolInfo.project, nodePoolInfo.location, "deleting GKE NodePool", timeout)
 	if waitErr != nil {
 		return waitErr
 	}