Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add location / node_locations fields to GKE #3114

Merged
merged 1 commit into from
Mar 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion google/data_source_google_container_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func dataSourceGoogleContainerCluster() *schema.Resource {
addRequiredFieldsToSchema(dsSchema, "name")

// Set 'Optional' schema elements
addOptionalFieldsToSchema(dsSchema, "project", "zone", "region")
addOptionalFieldsToSchema(dsSchema, "project", "zone", "region", "location")

return &schema.Resource{
Read: datasourceContainerClusterRead,
Expand Down
18 changes: 9 additions & 9 deletions google/data_source_google_container_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,18 @@ func testAccContainerClusterDatasource_zonal() string {
return fmt.Sprintf(`
resource "google_container_cluster" "kubes" {
name = "cluster-test-%s"
zone = "us-central1-a"
location = "us-central1-a"
initial_node_count = 1
master_auth {
username = "mr.yoda"
password = "adoy.rm.123456789"
}
}
data "google_container_cluster" "kubes" {
name = "${google_container_cluster.kubes.name}"
zone = "${google_container_cluster.kubes.zone}"
name = "${google_container_cluster.kubes.name}"
location = "${google_container_cluster.kubes.zone}"
}
`, acctest.RandString(10))
}
Expand All @@ -130,13 +130,13 @@ func testAccContainerClusterDatasource_regional() string {
return fmt.Sprintf(`
resource "google_container_cluster" "kubes" {
name = "cluster-test-%s"
region = "us-central1"
location = "us-central1"
initial_node_count = 1
}
data "google_container_cluster" "kubes" {
name = "${google_container_cluster.kubes.name}"
region = "${google_container_cluster.kubes.region}"
name = "${google_container_cluster.kubes.name}"
location = "${google_container_cluster.kubes.region}"
}
`, acctest.RandString(10))
}
16 changes: 12 additions & 4 deletions google/data_source_google_container_engine_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,22 @@ func dataSourceGoogleContainerEngineVersions() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"location": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"zone", "region"},
},
"zone": {
Type: schema.TypeString,
Optional: true,
Type: schema.TypeString,
Optional: true,
Deprecated: "Use location instead",
ConflictsWith: []string{"region", "location"},
},
"region": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"zone"},
Deprecated: "Use location instead",
ConflictsWith: []string{"zone", "location"},
},
"default_cluster_version": {
Type: schema.TypeString,
Expand Down Expand Up @@ -68,7 +76,7 @@ func dataSourceGoogleContainerEngineVersionsRead(d *schema.ResourceData, meta in
return err
}
if len(location) == 0 {
return fmt.Errorf("Cannot determine location: set zone or region in this data source or at provider-level")
return fmt.Errorf("Cannot determine location: set location, zone, or region in this data source or at provider-level")
}

location = fmt.Sprintf("projects/%s/locations/%s", project, location)
Expand Down
10 changes: 10 additions & 0 deletions google/data_source_google_container_engine_versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func TestAccContainerEngineVersions_basic(t *testing.T) {
{
Config: testAccCheckGoogleContainerEngineVersionsConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckGoogleContainerEngineVersionsMeta("data.google_container_engine_versions.location"),
testAccCheckGoogleContainerEngineVersionsMeta("data.google_container_engine_versions.versions"),
),
},
Expand Down Expand Up @@ -55,6 +56,7 @@ func TestAccContainerEngineVersions_regional(t *testing.T) {
{
Config: testAccCheckGoogleContainerEngineVersionsRegionalConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckGoogleContainerEngineVersionsMeta("data.google_container_engine_versions.location"),
testAccCheckGoogleContainerEngineVersionsMeta("data.google_container_engine_versions.versions"),
),
},
Expand Down Expand Up @@ -133,6 +135,10 @@ func testAccCheckGoogleContainerEngineVersionsMeta(n string) resource.TestCheckF
}

var testAccCheckGoogleContainerEngineVersionsConfig = `
data "google_container_engine_versions" "location" {
location = "us-central1-b"
}
data "google_container_engine_versions" "versions" {
zone = "us-central1-b"
}
Expand All @@ -146,6 +152,10 @@ data "google_container_engine_versions" "versions" {
`

var testAccCheckGoogleContainerEngineVersionsRegionalConfig = `
data "google_container_engine_versions" "location" {
location = "us-central1"
}
data "google_container_engine_versions" "versions" {
region = "us-central1"
}
Expand Down
4 changes: 3 additions & 1 deletion google/regional_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ func isZone(location string) bool {
}

func getLocation(d *schema.ResourceData, config *Config) (string, error) {
if v, isRegionalCluster := d.GetOk("region"); isRegionalCluster {
if v, ok := d.GetOk("location"); ok {
return v.(string), nil
} else 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).
Expand Down
115 changes: 103 additions & 12 deletions google/resource_container_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,29 +96,47 @@ func resourceContainerCluster() *schema.Resource {
},
},

"location": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"zone", "region"},
},

"region": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"zone"},
Deprecated: "Use location instead",
ConflictsWith: []string{"zone", "location"},
},

"zone": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"region"},
Deprecated: "Use location instead",
ConflictsWith: []string{"region", "location"},
},

"additional_zones": {
"node_locations": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},

"additional_zones": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
Deprecated: "Use node_locations instead",
Elem: &schema.Schema{Type: schema.TypeString},
},

"addons_config": {
Type: schema.TypeList,
Optional: true,
Expand Down Expand Up @@ -666,14 +684,29 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er
}
}

if v, ok := d.GetOk("additional_zones"); ok {
if v, ok := d.GetOk("node_locations"); ok {
locationsSet := v.(*schema.Set)
if locationsSet.Contains(location) {
return fmt.Errorf("additional_zones should not contain the original 'zone'")
return fmt.Errorf("when using a multi-zonal cluster, additional_zones should not contain the original 'zone'")
}

// GKE requires a full list of node locations
// but when using a multi-zonal cluster our schema only asks for the
// additional zones, so append the cluster location if it's a zone
if isZone(location) {
locationsSet.Add(location)
}
cluster.Locations = convertStringSet(locationsSet)
} else if v, ok := d.GetOk("additional_zones"); ok {
locationsSet := v.(*schema.Set)
if locationsSet.Contains(location) {
return fmt.Errorf("when using a multi-zonal cluster, additional_zones should not contain the original 'zone'")
}

// GKE requires a full list of node locations
// but when using a multi-zonal cluster our schema only asks for the
// additional zones, so append the cluster location if it's a zone
if isZone(location) {
// GKE requires a full list of locations (including the original zone),
// but our schema only asks for additional zones, so append the original.
locationsSet.Add(location)
}
cluster.Locations = convertStringSet(locationsSet)
Expand Down Expand Up @@ -804,10 +837,17 @@ func resourceContainerClusterRead(d *schema.ResourceData, meta interface{}) erro
if err := d.Set("network_policy", flattenNetworkPolicy(cluster.NetworkPolicy)); err != nil {
return err
}
d.Set("zone", cluster.Zone)

d.Set("location", cluster.Location)
if isZone(cluster.Location) {
d.Set("zone", cluster.Location)
} else {
d.Set("region", cluster.Location)
}

locations := schema.NewSet(schema.HashString, convertStringArrToInterface(cluster.Locations))
locations.Remove(cluster.Zone) // Remove the original zone since we only store additional zones
d.Set("node_locations", locations)
d.Set("additional_zones", locations)

d.Set("endpoint", cluster.Endpoint)
Expand Down Expand Up @@ -975,6 +1015,9 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
d.SetPartial("maintenance_policy")
}

// we can only ever see a change to one of additional_zones and node_locations; because
// thy conflict with each other and are each computed, Terraform will suppress the diff
// on one of them even when migrating from one to the other.
if d.HasChange("additional_zones") {
azSetOldI, azSetNewI := d.GetChange("additional_zones")
azSetNew := azSetNewI.(*schema.Set)
Expand All @@ -996,7 +1039,7 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
},
}

updateF := updateFunc(req, "updating GKE cluster locations")
updateF := updateFunc(req, "updating GKE cluster node locations")
// Call update serially.
if err := lockedCall(lockKey, updateF); err != nil {
return err
Expand All @@ -1012,16 +1055,63 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
},
}

updateF := updateFunc(req, "updating GKE cluster locations")
updateF := updateFunc(req, "updating GKE cluster node locations")
// Call update serially.
if err := lockedCall(lockKey, updateF); err != nil {
return err
}
}

log.Printf("[INFO] GKE cluster %s locations have been updated to %v", d.Id(), azSet.List())
log.Printf("[INFO] GKE cluster %s node locations have been updated to %v", d.Id(), azSet.List())

d.SetPartial("additional_zones")
} else if d.HasChange("node_locations") {
azSetOldI, azSetNewI := d.GetChange("node_locations")
azSetNew := azSetNewI.(*schema.Set)
azSetOld := azSetOldI.(*schema.Set)
if azSetNew.Contains(location) {
return fmt.Errorf("for multi-zonal clusters, node_locations should not contain the primary 'zone'")
}
// Since we can't add & remove zones in the same request, first add all the
// zones, then remove the ones we aren't using anymore.
azSet := azSetOld.Union(azSetNew)

if isZone(location) {
azSet.Add(location)
}

req := &containerBeta.UpdateClusterRequest{
Update: &containerBeta.ClusterUpdate{
DesiredLocations: convertStringSet(azSet),
},
}

updateF := updateFunc(req, "updating GKE cluster node locations")
// Call update serially.
if err := lockedCall(lockKey, updateF); err != nil {
return err
}

if isZone(location) {
azSetNew.Add(location)
}
if !azSet.Equal(azSetNew) {
req = &containerBeta.UpdateClusterRequest{
Update: &containerBeta.ClusterUpdate{
DesiredLocations: convertStringSet(azSetNew),
},
}

updateF := updateFunc(req, "updating GKE cluster node locations")
// Call update serially.
if err := lockedCall(lockKey, updateF); err != nil {
return err
}
}

log.Printf("[INFO] GKE cluster %s node locations have been updated to %v", d.Id(), azSet.List())

d.SetPartial("node_locations")
}

if d.HasChange("enable_legacy_abac") {
Expand Down Expand Up @@ -1796,7 +1886,7 @@ func resourceContainerClusterStateImporter(d *schema.ResourceData, meta interfac
location = parts[1]
clusterName = parts[2]
default:
return nil, fmt.Errorf("Invalid container cluster specifier. Expecting {zone}/{name} or {project}/{zone}/{name}")
return nil, fmt.Errorf("Invalid container cluster specifier. Expecting {location}/{name} or {project}/{location}/{name}")
}

if len(project) == 0 {
Expand All @@ -1808,6 +1898,7 @@ func resourceContainerClusterStateImporter(d *schema.ResourceData, meta interfac
}
d.Set("project", project)

d.Set("location", location)
if isZone(location) {
d.Set("zone", location)
} else {
Expand Down
Loading