Fix cluster version upgrades (hashicorp#577)

* wait for running status on a cluster on read

* add min_master_version field

* respond to comments

* add docs

* no node_version on create

google/resource_container_cluster.go

@@ -8,6 +8,8 @@ import (
 	"strings"
 	"time"
 
+	version "github.com/hashicorp/go-version"
+	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/hashicorp/terraform/helper/validation"
 	"google.golang.org/api/container/v1"
@@ -244,10 +246,14 @@ func resourceContainerCluster() *schema.Resource {
 
 			"master_version": {
 				Type:     schema.TypeString,
-				Optional: true,
 				Computed: true,
 			},
 
+			"min_master_version": {
+				Type:     schema.TypeString,
+				Optional: true,
+			},
+
 			"node_config": schemaNodeConfig,
 
 			"node_version": {
@@ -302,12 +308,19 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er
 		}
 	}
 
-	if v, ok := d.GetOk("master_version"); ok {
+	if v, ok := d.GetOk("min_master_version"); ok {
 		cluster.InitialClusterVersion = v.(string)
 	}
 
-	if _, ok := d.GetOk("node_version"); ok {
-		return fmt.Errorf("cannot set node_version on create, use master_version instead")
+	// Only allow setting node_version on create if it's set to the equivalent master version,
+	// since `InitialClusterVersion` only accepts valid master-style versions.
+	if v, ok := d.GetOk("node_version"); ok {
+		// ignore -gke.X suffix for now. if it becomes a problem later, we can fix it.
+		mv := strings.Split(cluster.InitialClusterVersion, "-")[0]
+		nv := strings.Split(v.(string), "-")[0]
+		if mv != nv {
+			return fmt.Errorf("node_version and min_master_version must be set to equivalent values on create")
+		}
 	}
 
 	if v, ok := d.GetOk("additional_zones"); ok {
@@ -435,8 +448,18 @@ func resourceContainerClusterRead(d *schema.ResourceData, meta interface{}) erro
 
 	zoneName := d.Get("zone").(string)
 
-	cluster, err := config.clientContainer.Projects.Zones.Clusters.Get(
-		project, zoneName, d.Get("name").(string)).Do()
+	var cluster *container.Cluster
+	err = resource.Retry(2*time.Minute, func() *resource.RetryError {
+		cluster, err = config.clientContainer.Projects.Zones.Clusters.Get(
+			project, zoneName, d.Get("name").(string)).Do()
+		if err != nil {
+			return resource.NonRetryableError(err)
+		}
+		if cluster.Status != "RUNNING" {
+			return resource.RetryableError(fmt.Errorf("Cluster %q has status %q with message %q", d.Get("name"), cluster.Status, cluster.StatusMessage))
+		}
+		return nil
+	})
 	if err != nil {
 		return handleNotFoundError(err, d, fmt.Sprintf("Container Cluster %q", d.Get("name").(string)))
 	}
@@ -508,29 +531,42 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
 	d.Partial(true)
 
 	// The master must be updated before the nodes
-	if d.HasChange("master_version") {
-		desiredMasterVersion := d.Get("master_version").(string)
-		req := &container.UpdateClusterRequest{
-			Update: &container.ClusterUpdate{
-				DesiredMasterVersion: desiredMasterVersion,
-			},
+	if d.HasChange("min_master_version") {
+		desiredMasterVersion := d.Get("min_master_version").(string)
+		currentMasterVersion := d.Get("master_version").(string)
+		des, err := version.NewVersion(desiredMasterVersion)
+		if err != nil {
+			return err
 		}
-		op, err := config.clientContainer.Projects.Zones.Clusters.Update(
-			project, zoneName, clusterName, req).Do()
+		cur, err := version.NewVersion(currentMasterVersion)
 		if err != nil {
 			return err
 		}
 
-		// Wait until it's updated
-		waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE master version", timeoutInMinutes, 2)
-		if waitErr != nil {
-			return waitErr
-		}
+		// Only upgrade the master if the current version is lower than the desired version
+		if cur.LessThan(des) {
+			req := &container.UpdateClusterRequest{
+				Update: &container.ClusterUpdate{
+					DesiredMasterVersion: desiredMasterVersion,
+				},
+			}
+			op, err := config.clientContainer.Projects.Zones.Clusters.Update(
+				project, zoneName, clusterName, req).Do()
+			if err != nil {
+				return err
+			}
 
-		log.Printf("[INFO] GKE cluster %s: master has been updated to %s", d.Id(),
-			desiredMasterVersion)
+			// Wait until it's updated
+			waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE master version", timeoutInMinutes, 2)
+			if waitErr != nil {
+				return waitErr
+			}
+
+			log.Printf("[INFO] GKE cluster %s: master has been updated to %s", d.Id(),
+				desiredMasterVersion)
+		}
 
-		d.SetPartial("master_version")
+		d.SetPartial("min_master_version")
 	}
 
 	if d.HasChange("node_version") {

google/resource_container_cluster_test.go

@@ -824,7 +824,7 @@ data "google_container_engine_versions" "central1a" {
 resource "google_container_cluster" "with_version" {
 	name = "cluster-test-%s"
 	zone = "us-central1-a"
-	master_version = "${data.google_container_engine_versions.central1a.latest_master_version}"
+	min_master_version = "${data.google_container_engine_versions.central1a.latest_master_version}"
 	initial_node_count = 1
 
 	master_auth {
@@ -843,7 +843,7 @@ data "google_container_engine_versions" "central1a" {
 resource "google_container_cluster" "with_version" {
 	name = "cluster-test-%s"
 	zone = "us-central1-a"
-	master_version = "${data.google_container_engine_versions.central1a.valid_master_versions.1}"
+	min_master_version = "${data.google_container_engine_versions.central1a.valid_master_versions.2}"
 	initial_node_count = 1
 
 	master_auth {
@@ -862,8 +862,8 @@ data "google_container_engine_versions" "central1a" {
 resource "google_container_cluster" "with_version" {
 	name = "cluster-test-%s"
 	zone = "us-central1-a"
-	master_version = "${data.google_container_engine_versions.central1a.latest_master_version}"
-	node_version = "${data.google_container_engine_versions.central1a.latest_node_version}"
+	min_master_version = "${data.google_container_engine_versions.central1a.valid_master_versions.1}"
+	node_version = "${data.google_container_engine_versions.central1a.valid_node_versions.1}"
 	initial_node_count = 1
 
 	master_auth {

website/docs/r/container_cluster.html.markdown

@@ -13,10 +13,6 @@ Creates a GKE cluster. For more information see
 and
 [API](https://cloud.google.com/container-engine/reference/rest/v1/projects.zones.clusters).
 
-!> **Warning:** Due to limitations of the API, all arguments except
-`node_version` are non-updateable. Changing any will cause recreation of the
-whole cluster!
-
 ~> **Note:** All arguments including the username and password will be stored in the raw state as plain-text.
 [Read more about sensitive data in state](/docs/state/sensitive-data.html).
 
@@ -85,6 +81,12 @@ resource "google_container_cluster" "primary" {
     write logs to. Available options include `logging.googleapis.com` and
     `none`. Defaults to `logging.googleapis.com`
 
+* `min_master_version` - (Optional) The minimum version of the master. GKE
+    will auto-update the master to new versions, so this does not guarantee the
+    current master version--use the read-only `master_version` field to obtain that.
+    If unset, the cluster's version will be set by GKE to the version of the most recent
+    official release (which is not necessarily the latest version).
+
 * `monitoring_service` - (Optional) The monitoring service that the cluster
     should write metrics to. Available options include
     `monitoring.googleapis.com` and `none`. Defaults to
@@ -209,6 +211,10 @@ exported:
 * `master_auth.cluster_ca_certificate` - Base64 encoded public certificate
     that is the root of trust for the cluster
 
+* `master_version` - The current version of the master in the cluster. This may
+    be different than the `min_master_version` set in the config if the master
+    has been updated by GKE.
+
 <a id="timeouts"></a>
 ## Timeouts