Skip to content

Commit

Permalink
Add location / node_locations fields to GKE
Browse files Browse the repository at this point in the history
Signed-off-by: Modular Magician <magic-modules@google.com>
  • Loading branch information
rileykarson authored and modular-magician committed Feb 25, 2019
1 parent 9119654 commit 9b31266
Show file tree
Hide file tree
Showing 11 changed files with 254 additions and 91 deletions.
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 @@ -15,14 +15,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 @@ -63,7 +71,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 All @@ -37,6 +38,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 @@ -115,12 +117,20 @@ 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"
}
`

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
127 changes: 112 additions & 15 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 @@ -652,14 +670,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 @@ -790,10 +823,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 @@ -961,6 +1001,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 @@ -982,7 +1025,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 @@ -998,16 +1041,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 @@ -1752,22 +1842,29 @@ func resourceContainerClusterStateImporter(d *schema.ResourceData, meta interfac

switch len(parts) {
case 2:
if loc := parts[0]; isZone(loc) {
loc := parts[0]
if isZone(loc) {
d.Set("zone", loc)
} else {
d.Set("region", loc)
}

d.Set("location", loc)
d.Set("name", parts[1])
case 3:
d.Set("project", parts[0])
if loc := parts[1]; isZone(loc) {

loc := parts[1]
if isZone(loc) {
d.Set("zone", loc)
} else {
d.Set("region", loc)
}

d.Set("project", parts[0])
d.Set("location", loc)
d.Set("name", 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}")
}

d.SetId(parts[len(parts)-1])
Expand Down
Loading

0 comments on commit 9b31266

Please sign in to comment.