Support multiple zones in compute-vm module (#114)

* support multiple zones in compute-vm module

* update compute-vm variables in end-to-end examples

* update README examples

cloud-operations/asset-inventory-feed-remediation/main.tf

@@ -90,10 +90,9 @@ module "cf" {
 }
 
 module "simple-vm-example" {
-  source     = "github.com/terraform-google-modules/cloud-foundation-fabric//modules/compute-vm?ref=v2.3.0"
+  source     = "../../modules/compute-vm"
   project_id = module.project.project_id
   region     = var.region
-  zone       = "${var.region}-b"
   name       = var.name
   network_interfaces = [{
     network    = module.vpc.self_link,

infrastructure/hub-and-spoke-peering/main.tf

@@ -147,7 +147,6 @@ module "vm-spoke-1" {
   source     = "../../modules/compute-vm"
   project_id = var.project_id
   region     = var.region
-  zone       = "${var.region}-b"
   name       = "spoke-1-test"
   network_interfaces = [{
     network    = module.vpc-spoke-1.self_link,
@@ -165,7 +164,6 @@ module "vm-spoke-2" {
   source     = "../../modules/compute-vm"
   project_id = var.project_id
   region     = var.region
-  zone       = "${var.region}-b"
   name       = "spoke-2-test"
   network_interfaces = [{
     network    = module.vpc-spoke-2.self_link,

infrastructure/hub-and-spoke-vpn/main.tf

@@ -247,7 +247,6 @@ module "vm-spoke-1" {
   source     = "../../modules/compute-vm"
   project_id = var.project_id
   region     = var.regions.b
-  zone       = "${var.regions.b}-b"
   name       = "spoke-1-test"
   network_interfaces = [{
     network    = module.vpc-spoke-1.self_link,
@@ -263,7 +262,6 @@ module "vm-spoke-2" {
   source     = "../../modules/compute-vm"
   project_id = var.project_id
   region     = var.regions.b
-  zone       = "${var.regions.b}-b"
   name       = "spoke-2-test"
   network_interfaces = [{
     network    = module.vpc-spoke-2.self_link,

infrastructure/onprem-google-access-dns/main.tf

@@ -185,7 +185,6 @@ module "vm-test" {
   source     = "../../modules/compute-vm"
   project_id = var.project_id
   region     = var.region
-  zone       = "${var.region}-b"
   name       = "test"
   network_interfaces = [{
     network    = module.vpc.self_link,
@@ -238,7 +237,6 @@ module "vm-onprem" {
   source        = "../../modules/compute-vm"
   project_id    = var.project_id
   region        = var.region
-  zone          = "${var.region}-b"
   instance_type = "f1-micro"
   name          = "onprem"
   boot_disk = {

infrastructure/shared-vpc-gke/main.tf

@@ -174,7 +174,6 @@ module "vm-bastion" {
   source     = "../../modules/compute-vm"
   project_id = module.project-svc-gce.project_id
   region     = var.region
-  zone       = "${var.region}-b"
   name       = "bastion"
   network_interfaces = [{
     network    = module.vpc-shared.self_link,

modules/compute-vm/README.md

@@ -18,7 +18,6 @@ module "simple-vm-example" {
   source     = "../modules/compute-vm"
   project_id = "my-project"
   region     = "europe-west1"
-  zone       = "europe-west1-b"
   name       = "test"
   network_interfaces = [{
     network    = local.network_self_link,
@@ -40,7 +39,6 @@ module "kms-vm-example" {
   source     = "../modules/compute-vm"
   project_id = local.project_id
   region     = local.region
-  zone       = local.zone
   name       = "kms-test"
   network_interfaces = [{
     network    = local.network_self_link,
@@ -85,7 +83,6 @@ module "cos-test" {
   source     = "../modules/compute-vm"
   project_id = "my-project"
   region     = "europe-west1"
-  zone       = "europe-west1-b"
   name       = "test"
   network_interfaces = [{
     network    = local.network_self_link,
@@ -116,7 +113,6 @@ module "instance-group" {
   source     = "../../cloud-foundation-fabric/modules/compute-vm"
   project_id = "my-project"
   region     = "europe-west1"
-  zone       = "europe-west1-b"
   name       = "ilb-test"
   network_interfaces = [{
     network    = local.network_self_link,
@@ -148,7 +144,6 @@ module "instance-group" {
 | network_interfaces | Network interfaces configuration. Use self links for Shared VPC, set addresses to null if not needed. | <code title="list&#40;object&#40;&#123;&#10;nat        &#61; bool&#10;network    &#61; string&#10;subnetwork &#61; string&#10;addresses &#61; object&#40;&#123;&#10;internal &#61; list&#40;string&#41;&#10;external &#61; list&#40;string&#41;&#10;&#125;&#41;&#10;&#125;&#41;&#41;">list(object({...}))</code> | ✓ |  |
 | project_id | Project id. | <code title="">string</code> | ✓ |  |
 | region | Compute region. | <code title="">string</code> | ✓ |  |
-| zone | Compute zone. | <code title="">string</code> | ✓ |  |
 | *attached_disk_defaults* | Defaults for attached disks options. | <code title="object&#40;&#123;&#10;auto_delete &#61; bool&#10;mode        &#61; string&#10;type &#61; string&#10;source      &#61; string&#10;&#125;&#41;">object({...})</code> |  | <code title="&#123;&#10;auto_delete &#61; true&#10;source      &#61; null&#10;mode        &#61; &#34;READ_WRITE&#34;&#10;type &#61; &#34;pd-ssd&#34;&#10;&#125;">...</code> |
 | *attached_disks* | Additional disks, if options is null defaults will be used in its place. | <code title="list&#40;object&#40;&#123;&#10;name  &#61; string&#10;image &#61; string&#10;size  &#61; string&#10;options &#61; object&#40;&#123;&#10;auto_delete &#61; bool&#10;mode        &#61; string&#10;source      &#61; string&#10;type &#61; string&#10;&#125;&#41;&#10;&#125;&#41;&#41;">list(object({...}))</code> |  | <code title="">[]</code> |
 | *boot_disk* | Boot disk properties. | <code title="object&#40;&#123;&#10;image &#61; string&#10;size  &#61; number&#10;type &#61; string&#10;&#125;&#41;">object({...})</code> |  | <code title="&#123;&#10;image &#61; &#34;projects&#47;debian-cloud&#47;global&#47;images&#47;family&#47;debian-10&#34;&#10;type &#61; &#34;pd-ssd&#34;&#10;size  &#61; 10&#10;&#125;">...</code> |
@@ -170,6 +165,7 @@ module "instance-group" {
 | *shielded_config* | Shielded VM configuration of the instances. | <code title="object&#40;&#123;&#10;enable_secure_boot          &#61; bool&#10;enable_vtpm                 &#61; bool&#10;enable_integrity_monitoring &#61; bool&#10;&#125;&#41;">object({...})</code> |  | <code title="">null</code> |
 | *tags* | Instance tags. | <code title="list&#40;string&#41;">list(string)</code> |  | <code title="">[]</code> |
 | *use_instance_template* | Create instance template instead of instances. | <code title="">bool</code> |  | <code title="">false</code> |
+| *zones* | Compute zone, instance will cycle through the list, defaults to the 'b' zone in the region. | <code title="list&#40;string&#41;">list(string)</code> |  | <code title="">[]</code> |
 
 ## Outputs
 

modules/compute-vm/main.tf

@@ -23,16 +23,16 @@ locals {
   }
   attached_disks_pairs = {
     for pair in setproduct(keys(local.names), keys(local.attached_disks)) :
-    "${pair[0]}-${pair[1]}" => { name = pair[0], disk_name = pair[1] }
+    "${pair[0]}-${pair[1]}" => { disk_name = pair[1], name = pair[0] }
   }
   iam_roles = var.use_instance_template ? {} : {
     for pair in setproduct(var.iam_roles, keys(local.names)) :
     "${pair.0}/${pair.1}" => { role = pair.0, name = pair.1 }
   }
   names = (
-    var.use_instance_template
+    var.use_instance_template ? { "${var.name}" = 0 } : {
-    ? { "${var.name}" = 0 }
+      for i in range(0, var.instance_count) : "${var.name}-${i + 1}" => i
-    : { for i in range(0, var.instance_count) : "${var.name}-${i + 1}" => i }
+    }
   )
   service_account_email = (
     var.service_account_create
@@ -56,12 +56,16 @@ locals {
       ]
     )
   )
+  zones_list = length(var.zones) == 0 ? ["${var.region}-b"] : var.zones
+  zones = {
+    for name, i in local.names : name => element(local.zones_list, i)
+  }
 }
 
 resource "google_compute_disk" "disks" {
   for_each = var.use_instance_template ? {} : local.attached_disks_pairs
   project  = var.project_id
-  zone     = var.zone
+  zone     = local.zones[each.value.name]
   name     = each.key
   type     = local.attached_disks[each.value.disk_name].options.type
   size     = local.attached_disks[each.value.disk_name].size
@@ -83,7 +87,7 @@ resource "google_compute_disk" "disks" {
 resource "google_compute_instance" "default" {
   for_each                  = var.use_instance_template ? {} : local.names
   project                   = var.project_id
-  zone                      = var.zone
+  zone                      = local.zones[each.key]
   name                      = each.key
   hostname                  = var.hostname
   description               = "Managed by the compute-vm Terraform module."
@@ -179,7 +183,7 @@ resource "google_compute_instance" "default" {
 resource "google_compute_instance_iam_binding" "default" {
   for_each      = local.iam_roles
   project       = var.project_id
-  zone          = var.zone
+  zone          = local.zones[each.value.name]
   instance_name = each.value.name
   role          = each.value.role
   members       = lookup(var.iam_members, each.value.role, [])
@@ -260,7 +264,7 @@ resource "google_compute_instance_group" "unmanaged" {
     ? var.network_interfaces.0.network
     : ""
   )
-  zone        = var.zone
+  zone        = local.zones_list[0]
   name        = var.name
   description = "Terraform-managed."
   instances = [

modules/compute-vm/variables.tf

@@ -214,9 +214,10 @@ variable "use_instance_template" {
   default     = false
 }
 
-variable "zone" {
+variable "zones" {
-  description = "Compute zone."
+  description = "Compute zone, instance will cycle through the list, defaults to the 'b' zone in the region."
-  type        = string
+  type        = list(string)
+  default     = []
 }
 
 variable "shielded_config" {

tests/modules/compute_vm/fixture/main.tf

@@ -18,7 +18,7 @@ module "test" {
   source                 = "../../../../modules/compute-vm"
   project_id             = "my-project"
   region                 = "europe-west1"
-  zone                   = "europe-west1-b"
+  zones                  = var.zones
   name                   = "test"
   network_interfaces     = var.network_interfaces
   service_account_create = var.service_account_create

tests/modules/compute_vm/fixture/variables.tf

@@ -61,3 +61,8 @@ variable "service_account_create" {
   type    = bool
   default = false
 }
+
+variable "zones" {
+  type    = list(string)
+  default = []
+}

tests/modules/compute_vm/test_plan_zones.py

@@ -0,0 +1,68 @@
+# Copyright 2020 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+import os
+import pytest
+
+
+FIXTURES_DIR = os.path.join(os.path.dirname(__file__), 'fixture')
+
+
+def test_default(plan_runner):
+  _, resources = plan_runner(FIXTURES_DIR)
+  assert resources[0]['values']['zone'] == 'europe-west1-b'
+
+
+def test_multiple_default(plan_runner):
+  _, resources = plan_runner(FIXTURES_DIR, instance_count=2)
+  assert set(r['values']['zone'] for r in resources) == set(['europe-west1-b'])
+
+
+def test_custom(plan_runner):
+  _, resources = plan_runner(FIXTURES_DIR, zones='["a", "b"]')
+  assert resources[0]['values']['zone'] == 'a'
+
+
+def test_custom_default(plan_runner):
+  _, resources = plan_runner(
+      FIXTURES_DIR, instance_count=3, zones='["a", "b"]')
+  assert [r['values']['zone'] for r in resources] == ['a', 'b', 'a']
+
+
+def test_group(plan_runner):
+  _, resources = plan_runner(FIXTURES_DIR, instance_count=2,
+                             group='{named_ports={}}', zones='["a", "b"]')
+  assert resources[2]['type'] == 'google_compute_instance_group'
+  assert resources[2]['values']['zone'] == 'a'
+
+
+def test_iam(plan_runner):
+  iam_roles = '["roles/a", "roles/b"]'
+  iam_members = '{"roles/a" = ["user:a@a.com"], "roles/b" = ["user:a@a.com"]}'
+  _, resources = plan_runner(FIXTURES_DIR, instance_count=3,
+                             iam_roles=iam_roles, iam_members=iam_members,
+                             zones='["a", "b"]')
+  iam_bindings = dict(
+      (r['index'], r['values']['zone']) for r in resources if r['type']
+      == 'google_compute_instance_iam_binding'
+  )
+  assert iam_bindings == {
+      'roles/a/test-1': 'a',
+      'roles/a/test-2': 'b',
+      'roles/a/test-3': 'a',
+      'roles/b/test-1': 'a',
+      'roles/b/test-2': 'b',
+      'roles/b/test-3': 'a',
+  }