allow updating node image types (#1843)

Fixes #1702.

@chrisst I'm putting you as a reviewer, but no rush. Feel free to ask as many questions as you have! Also feel free to offer suggestions 😃 (or just say it's perfect as-is, that works too)

google/node_config.go

@@ -69,7 +69,6 @@ var schemaNodeConfig = &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
 				Computed: true,
-				ForceNew: true,
 			},
 
 			"labels": {

google/resource_container_cluster.go

@@ -1120,6 +1120,36 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 		d.SetPartial("logging_service")
 	}
 
+	if d.HasChange("node_config") {
+		if d.HasChange("node_config.0.image_type") {
+			it := d.Get("node_config.0.image_type").(string)
+			req := &containerBeta.UpdateClusterRequest{
+				Update: &containerBeta.ClusterUpdate{
+					DesiredImageType: it,
+				},
+			}
+
+			updateF := func() error {
+				name := containerClusterFullName(project, location, clusterName)
+				op, err := config.clientContainerBeta.Projects.Locations.Clusters.Update(name, req).Do()
+				if err != nil {
+					return err
+				}
+
+				// Wait until it's updated
+				return containerSharedOperationWait(config, op, project, location, "updating GKE image type", timeoutInMinutes, 2)
+			}
+
+			// Call update serially.
+			if err := lockedCall(lockKey, updateF); err != nil {
+				return err
+			}
+
+			log.Printf("[INFO] GKE cluster %s: image type has been updated to %s", d.Id(), it)
+		}
+		d.SetPartial("node_config")
+	}
+
 	if d.HasChange("pod_security_policy_config") {
 		c := d.Get("pod_security_policy_config")
 		req := &containerBeta.UpdateClusterRequest{

google/resource_container_cluster_test.go

@@ -564,13 +564,24 @@ func TestAccContainerCluster_updateVersion(t *testing.T) {
 func TestAccContainerCluster_withNodeConfig(t *testing.T) {
 	t.Parallel()
 
+	clusterName := fmt.Sprintf("cluster-test-%s", acctest.RandString(10))
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckContainerClusterDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccContainerCluster_withNodeConfig(),
+				Config: testAccContainerCluster_withNodeConfig(clusterName),
+			},
+			{
+				ResourceName:        "google_container_cluster.with_node_config",
+				ImportStateIdPrefix: "us-central1-f/",
+				ImportState:         true,
+				ImportStateVerify:   true,
+			},
+			{
+				Config: testAccContainerCluster_withNodeConfigUpdate(clusterName),
 			},
 			{
 				ResourceName:        "google_container_cluster.with_node_config",
@@ -1670,10 +1681,10 @@ resource "google_container_cluster" "with_version" {
 }`, clusterName)
 }
 
-func testAccContainerCluster_withNodeConfig() string {
+func testAccContainerCluster_withNodeConfig(clusterName string) string {
 	return fmt.Sprintf(`
 resource "google_container_cluster" "with_node_config" {
-	name = "cluster-test-%s"
+	name = "%s"
 	zone = "us-central1-f"
 	initial_node_count = 1
 
@@ -1692,15 +1703,52 @@ resource "google_container_cluster" "with_node_config" {
 		metadata {
 			foo = "bar"
 		}
-		image_type = "COS"
 		labels {
 			foo = "bar"
 		}
 		tags = ["foo", "bar"]
 		preemptible = true
 		min_cpu_platform = "Intel Broadwell"
+
+		// Updatable fields
+		image_type = "COS"
 	}
-}`, acctest.RandString(10))
+}`, clusterName)
+}
+
+func testAccContainerCluster_withNodeConfigUpdate(clusterName string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "with_node_config" {
+	name = "%s"
+	zone = "us-central1-f"
+	initial_node_count = 1
+
+	node_config {
+		machine_type = "n1-standard-1"
+		disk_size_gb = 15
+		disk_type = "pd-ssd"
+		local_ssd_count = 1
+		oauth_scopes = [
+			"https://www.googleapis.com/auth/monitoring",
+			"https://www.googleapis.com/auth/compute",
+			"https://www.googleapis.com/auth/devstorage.read_only",
+			"https://www.googleapis.com/auth/logging.write"
+		]
+		service_account = "default"
+		metadata {
+			foo = "bar"
+		}
+		labels {
+			foo = "bar"
+		}
+		tags = ["foo", "bar"]
+		preemptible = true
+		min_cpu_platform = "Intel Broadwell"
+
+		// Updatable fields
+		image_type = "UBUNTU"
+	}
+}`, clusterName)
 }
 
 func testAccContainerCluster_withNodeConfigScopeAlias() string {

google/resource_container_node_pool.go

@@ -584,6 +584,41 @@ func nodePoolUpdate(d *schema.ResourceData, meta interface{}, nodePoolInfo *Node
 		}
 	}
 
+	if d.HasChange(prefix + "node_config") {
+		if d.HasChange(prefix + "node_config.0.image_type") {
+			req := &containerBeta.UpdateClusterRequest{
+				Update: &containerBeta.ClusterUpdate{
+					DesiredNodePoolId: name,
+					DesiredImageType:  d.Get(prefix + "node_config.0.image_type").(string),
+				},
+			}
+
+			updateF := func() error {
+				op, err := config.clientContainerBeta.Projects.Locations.Clusters.Update(nodePoolInfo.parent(), req).Do()
+				if err != nil {
+					return err
+				}
+
+				// Wait until it's updated
+				return containerBetaOperationWait(config, op,
+					nodePoolInfo.project,
+					nodePoolInfo.location, "updating GKE node pool",
+					timeoutInMinutes, 2)
+			}
+
+			// Call update serially.
+			if err := lockedCall(lockKey, updateF); err != nil {
+				return err
+			}
+
+			log.Printf("[INFO] Updated image type in Node Pool %s", d.Id())
+		}
+
+		if prefix == "" {
+			d.SetPartial("node_config")
+		}
+	}
+
 	if d.HasChange(prefix + "node_count") {
 		newSize := int64(d.Get(prefix + "node_count").(int))
 		req := &containerBeta.SetNodePoolSizeRequest{

google/resource_container_node_pool_test.go

@@ -80,13 +80,27 @@ func TestAccContainerNodePool_noName(t *testing.T) {
 func TestAccContainerNodePool_withNodeConfig(t *testing.T) {
 	t.Parallel()
 
+	cluster := fmt.Sprintf("tf-nodepool-test-%s", acctest.RandString(10))
+	nodePool := fmt.Sprintf("tf-nodepool-test-%s", acctest.RandString(10))
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckContainerNodePoolDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config: testAccContainerNodePool_withNodeConfig(),
+				Config: testAccContainerNodePool_withNodeConfig(cluster, nodePool),
+			},
+			resource.TestStep{
+				ResourceName:      "google_container_node_pool.np_with_node_config",
+				ImportState:       true,
+				ImportStateVerify: true,
+				// autoscaling.# = 0 is equivalent to no autoscaling at all,
+				// but will still cause an import diff
+				ImportStateVerifyIgnore: []string{"autoscaling.#"},
+			},
+			resource.TestStep{
+				Config: testAccContainerNodePool_withNodeConfigUpdate(cluster, nodePool),
 			},
 			resource.TestStep{
 				ResourceName:      "google_container_node_pool.np_with_node_config",
@@ -672,15 +686,15 @@ resource "google_container_node_pool" "np_with_management" {
 }`, cluster, nodePool, management)
 }
 
-func testAccContainerNodePool_withNodeConfig() string {
+func testAccContainerNodePool_withNodeConfig(cluster, nodePool string) string {
 	return fmt.Sprintf(`
 resource "google_container_cluster" "cluster" {
-	name = "tf-cluster-nodepool-test-%s"
+	name = "%s"
 	zone = "us-central1-a"
 	initial_node_count = 1
 }
 resource "google_container_node_pool" "np_with_node_config" {
-	name = "tf-nodepool-test-%s"
+	name = "%s"
 	zone = "us-central1-a"
 	cluster = "${google_container_cluster.cluster.name}"
 	initial_node_count = 1
@@ -695,8 +709,41 @@ resource "google_container_node_pool" "np_with_node_config" {
 		]
 		preemptible = true
 		min_cpu_platform = "Intel Broadwell"
+
+		// Updatable fields
+		image_type = "COS"
 	}
-}`, acctest.RandString(10), acctest.RandString(10))
+}`, cluster, nodePool)
+}
+
+func testAccContainerNodePool_withNodeConfigUpdate(cluster, nodePool string) string {
+	return fmt.Sprintf(`
+resource "google_container_cluster" "cluster" {
+	name = "%s"
+	zone = "us-central1-a"
+	initial_node_count = 1
+}
+resource "google_container_node_pool" "np_with_node_config" {
+	name = "%s"
+	zone = "us-central1-a"
+	cluster = "${google_container_cluster.cluster.name}"
+	initial_node_count = 1
+	node_config {
+		machine_type = "g1-small"
+		disk_size_gb = 10
+		oauth_scopes = [
+			"https://www.googleapis.com/auth/compute",
+			"https://www.googleapis.com/auth/devstorage.read_only",
+			"https://www.googleapis.com/auth/logging.write",
+			"https://www.googleapis.com/auth/monitoring"
+		]
+		preemptible = true
+		min_cpu_platform = "Intel Broadwell"
+
+		// Updatable fields
+		image_type = "UBUNTU"
+	}
+}`, cluster, nodePool)
 }
 
 func testAccContainerNodePool_withNodeConfigTaints() string {

website/docs/r/container_cluster.html.markdown

@@ -285,7 +285,8 @@ The `node_config` block supports:
 * `guest_accelerator` - (Optional) List of the type and count of accelerator cards attached to the instance.
     Structure documented below.
 
-* `image_type` - (Optional) The image type to use for this node.
+* `image_type` - (Optional) The image type to use for this node. Note that changing the image type
+    will delete and recreate all nodes in the node pool.
 
 * `labels` - (Optional) The Kubernetes labels (key/value pairs) to be applied to each node.