From e9e9f835ecf7bf5c95a7fdda27afcbc6f02d1e21 Mon Sep 17 00:00:00 2001
From: Dana Hoffman <danahoffman@google.com>
Date: Tue, 8 Aug 2017 11:31:12 -0700
Subject: [PATCH] Allow upgrading GKE versions and provide better error message
 handling (#291)

* Better error handling for GKE operations

* Handle GKE version upgrades

* clarify log message
---
 google/container_operation.go             |  4 ++
 google/resource_container_cluster.go      | 28 ++++++++++--
 google/resource_container_cluster_test.go | 55 +++++++++++++++++++++--
 3 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/google/container_operation.go b/google/container_operation.go
index fb1b9cab..a4ac4e2c 100644
--- a/google/container_operation.go
+++ b/google/container_operation.go
@@ -33,6 +33,10 @@ func (w *ContainerOperationWaiter) RefreshFunc() resource.StateRefreshFunc {
 			return nil, "", err
 		}
 
+		if resp.StatusMessage != "" {
+			return resp, resp.Status, fmt.Errorf(resp.StatusMessage)
+		}
+
 		log.Printf("[DEBUG] Progress of operation %q: %q", w.Op.Name, resp.Status)
 
 		return resp, resp.Status, err
diff --git a/google/resource_container_cluster.go b/google/resource_container_cluster.go
index 3529b23b..b0173119 100644
--- a/google/resource_container_cluster.go
+++ b/google/resource_container_cluster.go
@@ -552,9 +552,10 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 	if d.HasChange("node_version") {
 		desiredNodeVersion := d.Get("node_version").(string)
 
+		// The master must be updated before the nodes
 		req := &container.UpdateClusterRequest{
 			Update: &container.ClusterUpdate{
-				DesiredNodeVersion: desiredNodeVersion,
+				DesiredMasterVersion: desiredNodeVersion,
 			},
 		}
 		op, err := config.clientContainer.Projects.Zones.Clusters.Update(
@@ -564,12 +565,33 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 		}
 
 		// Wait until it's updated
-		waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE cluster version", timeoutInMinutes, 2)
+		waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE master version", timeoutInMinutes, 2)
 		if waitErr != nil {
 			return waitErr
 		}
 
-		log.Printf("[INFO] GKE cluster %s has been updated to %s", d.Id(),
+		log.Printf("[INFO] GKE cluster %s: master has been updated to %s", d.Id(),
+			desiredNodeVersion)
+
+		// Update the nodes
+		req = &container.UpdateClusterRequest{
+			Update: &container.ClusterUpdate{
+				DesiredNodeVersion: desiredNodeVersion,
+			},
+		}
+		op, err = config.clientContainer.Projects.Zones.Clusters.Update(
+			project, zoneName, clusterName, req).Do()
+		if err != nil {
+			return err
+		}
+
+		// Wait until it's updated
+		waitErr = containerOperationWait(config, op, project, zoneName, "updating GKE node version", timeoutInMinutes, 2)
+		if waitErr != nil {
+			return waitErr
+		}
+
+		log.Printf("[INFO] GKE cluster %s: nodes have been updated to %s", d.Id(),
 			desiredNodeVersion)
 
 		d.SetPartial("node_version")
diff --git a/google/resource_container_cluster_test.go b/google/resource_container_cluster_test.go
index 4e3939e6..4cf66330 100644
--- a/google/resource_container_cluster_test.go
+++ b/google/resource_container_cluster_test.go
@@ -121,13 +121,41 @@ func TestAccContainerCluster_withLegacyAbac(t *testing.T) {
 }
 
 func TestAccContainerCluster_withVersion(t *testing.T) {
+	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_withVersion,
+				Config: testAccContainerCluster_withVersion(clusterName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckContainerCluster(
+						"google_container_cluster.with_version"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccContainerCluster_updateVersion(t *testing.T) {
+	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_withLowerVersion(clusterName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckContainerCluster(
+						"google_container_cluster.with_version"),
+				),
+			},
+			{
+				Config: testAccContainerCluster_withVersion(clusterName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckContainerCluster(
 						"google_container_cluster.with_version"),
@@ -586,7 +614,8 @@ resource "google_container_cluster" "with_legacy_abac" {
 }`, clusterName)
 }
 
-var testAccContainerCluster_withVersion = fmt.Sprintf(`
+func testAccContainerCluster_withVersion(clusterName string) string {
+	return fmt.Sprintf(`
 data "google_container_engine_versions" "central1a" {
 	zone = "us-central1-a"
 }
@@ -601,7 +630,27 @@ resource "google_container_cluster" "with_version" {
 		username = "mr.yoda"
 		password = "adoy.rm"
 	}
-}`, acctest.RandString(10))
+}`, clusterName)
+}
+
+func testAccContainerCluster_withLowerVersion(clusterName string) string {
+	return fmt.Sprintf(`
+data "google_container_engine_versions" "central1a" {
+	zone = "us-central1-a"
+}
+
+resource "google_container_cluster" "with_version" {
+	name = "cluster-test-%s"
+	zone = "us-central1-a"
+	node_version = "${data.google_container_engine_versions.central1a.valid_master_versions.1}"
+	initial_node_count = 1
+
+	master_auth {
+		username = "mr.yoda"
+		password = "adoy.rm"
+	}
+}`, clusterName)
+}
 
 var testAccContainerCluster_withNodeConfig = fmt.Sprintf(`
 resource "google_container_cluster" "with_node_config" {