#1300 Supporting regional clusters for node pools

google/node_config.go

@@ -4,6 +4,8 @@ import (
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/helper/validation"
 	containerBeta "google.golang.org/api/container/v1beta1"
+	"strconv"
+	"strings"
 )
 
 // Matches gke-default scope from https://cloud.google.com/sdk/gcloud/reference/container/clusters/create
@@ -132,9 +134,10 @@ var schemaNodeConfig = &schema.Schema{
 			},
 
 			"taint": {
-				Type:     schema.TypeList,
-				Optional: true,
-				ForceNew: true,
+				Type:             schema.TypeList,
+				Optional:         true,
+				ForceNew:         true,
+				DiffSuppressFunc: taintDiffSuppress,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"key": {
@@ -353,3 +356,20 @@ func flattenWorkloadMetadataConfig(c *containerBeta.WorkloadMetadataConfig) []ma
 	}
 	return result
 }
+
+func taintDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
+	if strings.HasSuffix(k, "#") {
+		oldCount, oldErr := strconv.Atoi(old)
+		newCount, newErr := strconv.Atoi(new)
+		// If either of them isn't a number somehow, or if there's one that we didn't have before.
+		return oldErr != nil || newErr != nil || oldCount == newCount+1
+	} else {
+		lastDot := strings.LastIndex(k, ".")
+		taintKey := d.Get(k[:lastDot] + ".key").(string)
+		if taintKey == "nvidia.com/gpu" {
+			return true
+		} else {
+			return false
+		}
+	}
+}

google/regional_utils.go

@@ -0,0 +1,37 @@
+package google
+
+import (
+	"fmt"
+	"github.com/hashicorp/terraform/helper/schema"
+	"strings"
+)
+
+//These functions are used by both the `resource_container_node_pool` and `resource_container_cluster` for handling regional clusters
+
+func isZone(location string) bool {
+	return len(strings.Split(location, "-")) == 3
+}
+
+func getLocation(d *schema.ResourceData, config *Config) (string, error) {
+	if v, isRegionalCluster := d.GetOk("region"); isRegionalCluster {
+		return v.(string), nil
+	} else {
+		// If region is not explicitly set, use "zone" (or fall back to the provider-level zone).
+		// For now, to avoid confusion, we require region to be set in the config to create a regional
+		// cluster rather than falling back to the provider-level region.
+		return getZone(d, config)
+	}
+}
+
+// getZone reads the "zone" value from the given resource data and falls back
+// to provider's value if not given.  If neither is provided, returns an error.
+func getZone(d TerraformResourceData, config *Config) (string, error) {
+	res, ok := d.GetOk("zone")
+	if !ok {
+		if config.Zone != "" {
+			return config.Zone, nil
+		}
+		return "", fmt.Errorf("Cannot determine zone: set in this resource, or set provider-level zone.")
+	}
+	return GetResourceNameFromSelfLink(res.(string)), nil
+}

google/resource_container_cluster.go

@@ -1159,7 +1159,12 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 
 	if n, ok := d.GetOk("node_pool.#"); ok {
 		for i := 0; i < n.(int); i++ {
-			if err := nodePoolUpdate(d, meta, clusterName, fmt.Sprintf("node_pool.%d.", i), timeoutInMinutes); err != nil {
+			nodePoolInfo, err := extractNodePoolInformationFromCluster(d, config, clusterName)
+			if err != nil {
+				return err
+			}
+
+			if err := nodePoolUpdate(d, meta, nodePoolInfo, fmt.Sprintf("node_pool.%d.", i), timeoutInMinutes); err != nil {
 				return err
 			}
 		}
@@ -1254,7 +1259,6 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 }
 
 func resourceContainerClusterDelete(d *schema.ResourceData, meta interface{}) error {
-	containerAPIVersion := getContainerApiVersion(d, ContainerClusterBaseApiVersion, ContainerClusterVersionedFeatures)
 	config := meta.(*Config)
 
 	project, err := getProject(d, config)
@@ -1282,15 +1286,26 @@ func resourceContainerClusterDelete(d *schema.ResourceData, meta interface{}) er
 	defer mutexKV.Unlock(containerClusterMutexKey(project, location, clusterName))
 
 	var op interface{}
-	switch containerAPIVersion {
-	case v1:
-		op, err = config.clientContainer.Projects.Zones.Clusters.Delete(project, location, clusterName).Do()
-	case v1beta1:
+	var count = 0
+	err = resource.Retry(30*time.Second, func() *resource.RetryError {
+		count++
+
 		name := containerClusterFullName(project, location, clusterName)
 		op, err = config.clientContainerBeta.Projects.Locations.Clusters.Delete(name).Do()
-	}
+
+		if err != nil {
+			log.Printf("[WARNING] Cluster is still not ready to delete, retrying %s", clusterName)
+			return resource.RetryableError(err)
+		}
+
+		if count == 15 {
+			return resource.NonRetryableError(fmt.Errorf("Error retrying to delete cluster %s", clusterName))
+		}
+		return nil
+	})
+
 	if err != nil {
-		return err
+		return fmt.Errorf("Error deleting Cluster: %s", err)
 	}
 
 	// Wait until it's deleted
@@ -1597,17 +1612,20 @@ func containerClusterFullName(project, location, cluster string) string {
 	return fmt.Sprintf("projects/%s/locations/%s/clusters/%s", project, location, cluster)
 }
 
-func getLocation(d *schema.ResourceData, config *Config) (string, error) {
-	if v, isRegionalCluster := d.GetOk("region"); isRegionalCluster {
-		return v.(string), nil
-	} else {
-		// If region is not explicitly set, use "zone" (or fall back to the provider-level zone).
-		// For now, to avoid confusion, we require region to be set in the config to create a regional
-		// cluster rather than falling back to the provider-level region.
-		return getZone(d, config)
+func extractNodePoolInformationFromCluster(d *schema.ResourceData, config *Config, clusterName string) (*NodePoolInformation, error) {
+	project, err := getProject(d, config)
+	if err != nil {
+		return nil, err
+	}
+
+	location, err := getLocation(d, config)
+	if err != nil {
+		return nil, err
 	}
-}
 
-func isZone(location string) bool {
-	return len(strings.Split(location, "-")) == 3
+	return &NodePoolInformation{
+		project:  project,
+		location: location,
+		cluster:  d.Get("name").(string),
+	}, nil
 }

google/resource_container_cluster_test.go

@@ -272,6 +272,30 @@ func TestAccContainerCluster_regional(t *testing.T) {
 	})
 }
 
+func TestAccContainerCluster_regionalWithNodePool(t *testing.T) {
+	t.Parallel()
+
+	clusterName := fmt.Sprintf("cluster-test-regional-%s", acctest.RandString(10))
+	npName := fmt.Sprintf("tf-cluster-nodepool-test-%s", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckContainerClusterDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccContainerCluster_regionalWithNodePool(clusterName, npName),
+			},
+			{
+				ResourceName:        "google_container_cluster.regional",
+				ImportStateIdPrefix: "us-central1/",
+				ImportState:         true,
+				ImportStateVerify:   true,
+			},
+		},
+	})
+}
+
 func TestAccContainerCluster_withAdditionalZones(t *testing.T) {
 	t.Parallel()
 
@@ -1373,6 +1397,18 @@ resource "google_container_cluster" "regional" {
 }`, clusterName)
 }
 
+func testAccContainerCluster_regionalWithNodePool(cluster, nodePool string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "regional" {
+	name   = "%s"
+	region = "us-central1"
+
+	node_pool {
+		name = "%s"
+	}
+}`, cluster, nodePool)
+}
+
 func testAccContainerCluster_withAdditionalZones(clusterName string) string {
 	return fmt.Sprintf(`
 resource "google_container_cluster" "with_additional_zones" {