Manage multinic instance groups in all zones in the region

Without this patch the `52_regional_multinic` module makes the blind assumption zones a, b, and c exist in the region. This is a problem for us-east1 and europe-west1 which do not have zone a. This patch fixes the problem by changing 50_compute's zone input parameter to a list of zone name strings, `zones`. Similarly, `52_regional_multinic` is updated to automatically determine all available zones in the region and create multinic groups in each zone. Resolves: #20
openinfrastructure · Sep 30, 2020 · e888b26 · e888b26
1 parent fba5400
commit e888b26
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 124 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,13 @@
+v2.0.0 - 2020-09-29
+===
+
+ * Fix [issue/20][issue20] `modules/52_regional_multinic` now deploys instance
+   groups to all available zones in the specified region.  Fixes error
+   deploying to us-east1 and europe-west1 where there is no `a` zone.
+ * Note, resources will be destroyed and re-created.  The inputs to
+   `52_regional_multinic` have *not* changed relative to v1.4.0.  The `zone`
+   input to `50_compute` is replaced by `zones`.
+
 v1.4.0 - 2020-09-28
 ===
 
@@ -65,3 +75,4 @@ v0.4.3
 
 [issue10]: https://github.com/openinfrastructure/terraform-google-multinic/issues/10
 [guest76]: https://github.com/GoogleCloudPlatform/guest-agent/issues/76
+[issue20]: https://github.com/openinfrastructure/terraform-google-multinic/issues/20
diff --git a/examples/compute/main.tf b/examples/compute/main.tf
@@ -47,17 +47,17 @@ locals {
 }
 
 # Manage the regional MIG formation
-module "multinic-a" {
+module "multinic" {
   source = "../../modules/50_compute"
 
   num_instances = var.num_instances
   preemptible   = var.preemptible
   autoscale     = var.num_instances == 0 ? false : true
 
   project_id  = local.project_id
-  name_prefix = "multinic-a"
+  name_prefix = "multinic"
   region      = local.region
-  zone        = "${local.region}-a"
+  zones       = [ "${local.region}-b", "${local.region}-c" ]
 
   nic0_network = local.nic0_network
   nic0_project = local.project_id
@@ -73,34 +73,6 @@ module "multinic-a" {
   service_account_email = "multinic@${local.project_id}.iam.gserviceaccount.com"
 }
 
-module "multinic-b" {
-  source = "../../modules/50_compute"
-
-  num_instances = var.num_instances_b
-  preemptible   = var.preemptible
-  autoscale     = var.num_instances_b == 0 ? false : true
-
-
-  project_id  = local.project_id
-  name_prefix = "multinic-b"
-  region      = local.region
-  zone        = "${local.region}-b"
-
-  nic0_network = local.nic0_network
-  nic0_project = local.project_id
-  nic0_subnet  = local.nic0_subnet
-  nic0_cidrs   = [local.nic0_netblock]
-
-  nic1_network = local.nic1_network
-  nic1_project = local.project_id
-  nic1_subnet  = local.nic1_subnet
-  nic1_cidrs   = [local.nic1_netblock]
-
-  # Note this is the auto-healing check, not the traffic check
-  hc_self_link = google_compute_health_check.multinic-health.self_link
-  service_account_email = "multinic@${local.project_id}.iam.gserviceaccount.com"
-}
-
 # The "health" health check is used for auto-healing with the MIG.  The
 # timeouts are longer to reduce the risk of removing an otherwise healthy
 # instance.
@@ -148,12 +120,11 @@ resource "google_compute_region_backend_service" "multinic-main" {
   region                = local.region
   load_balancing_scheme = "INTERNAL"
 
-  backend {
-    group = module.multinic-a.instance_group
-  }
-
-  backend {
-    group = module.multinic-b.instance_group
+  dynamic "backend" {
+    for_each = module.multinic.instance_groups
+    content {
+      group = backend.value
+    }
   }
 
   # Note this is the traffic health check, not the auto-healing check
@@ -169,12 +140,11 @@ resource "google_compute_region_backend_service" "multinic-transit" {
   region                = local.region
   load_balancing_scheme = "INTERNAL"
 
-  backend {
-    group = module.multinic-a.instance_group
-  }
-
-  backend {
-    group = module.multinic-b.instance_group
+  dynamic "backend" {
+    for_each = module.multinic.instance_groups
+    content {
+      group = backend.value
+    }
   }
 
   # Note this is the traffic health check, not the auto-healing check

diff --git a/modules/50_compute/main.tf b/modules/50_compute/main.tf
@@ -82,12 +82,13 @@ resource google_compute_instance_template "multinic" {
 }
 
 resource "google_compute_instance_group_manager" "multinic" {
+  for_each = toset(var.zones)
   project  = var.project_id
-  name     = "${var.name_prefix}-${var.zone}"
+  name     = "${var.name_prefix}-${each.value}"
 
   base_instance_name = var.name_prefix
 
-  zone = var.zone
+  zone = each.value
 
   update_policy {
     type                  = "PROACTIVE"
@@ -121,11 +122,11 @@ resource "google_compute_instance_group_manager" "multinic" {
 }
 
 resource "google_compute_autoscaler" "multinic" {
-  count   = var.autoscale ? 1 : 0
-  project = var.project_id
-  name    = "${var.name_prefix}-${var.zone}"
-  zone    = var.zone
-  target  = google_compute_instance_group_manager.multinic.id
+  for_each = toset(var.autoscale ? var.zones : [])
+  project  = var.project_id
+  name     = "${var.name_prefix}-${each.value}"
+  zone     = each.value
+  target   = google_compute_instance_group_manager.multinic[each.value].id
 
   autoscaling_policy {
     max_replicas    = var.max_replicas

diff --git a/modules/50_compute/outputs.tf b/modules/50_compute/outputs.tf
@@ -11,7 +11,7 @@
 # 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.
-output "instance_group" {
-  description = "The instance group intended for use with a google_compute_region_backend_service resource"
-  value       = google_compute_instance_group_manager.multinic.instance_group
+output "instance_groups" {
+  description = "The instance groups intended for use with a google_compute_region_backend_service resource"
+  value       = { for k,v in google_compute_instance_group_manager.multinic : k => v.instance_group }
 }
diff --git a/modules/50_compute/variables.tf b/modules/50_compute/variables.tf
@@ -27,9 +27,9 @@ variable "region" {
   type        = string
 }
 
-variable "zone" {
-  description = "The zone containing the managed resources"
-  type        = string
+variable "zones" {
+  description = "The zones containing the managed resources, for example ['us-west1-a', 'us-west1-b', 'us-west1-c']"
+  type        = list(string)
 }
 
 variable "service_account_email" {

diff --git a/modules/52_regional_multinic/main.tf b/modules/52_regional_multinic/main.tf
@@ -12,66 +12,27 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Manage the regional MIG formation
-module "multinic-a" {
-  source = "../50_compute"
-
-  num_instances = var.num_instances
-  preemptible   = var.preemptible
-  autoscale     = var.num_instances == 0 ? false : true
-
-  project_id  = var.project_id
-  name_prefix = "multinic-${var.region}-a"
-  region      = var.region
-  zone        = "${var.region}-a"
-
-  nic0_project = var.project_id
-  nic0_network = var.nic0_network
-  nic0_subnet  = var.nic0_subnet
-
-  nic1_project = var.project_id
-  nic1_network = var.nic1_network
-  nic1_subnet  = var.nic1_subnet
-
-  hc_self_link          = google_compute_health_check.multinic-health.self_link
-  service_account_email = var.service_account_email
+data "google_compute_zones" "available" {
+  project = var.project_id
+  region  = var.region
 }
 
-module "multinic-b" {
-  source = "../50_compute"
-
-  num_instances = var.num_instances
-  preemptible   = var.preemptible
-  autoscale     = var.num_instances == 0 ? false : true
-
-  project_id  = var.project_id
-  name_prefix = "multinic-${var.region}-b"
-  region      = var.region
-  zone        = "${var.region}-b"
-
-  nic0_project = var.project_id
-  nic0_network = var.nic0_network
-  nic0_subnet  = var.nic0_subnet
-
-  nic1_project = var.project_id
-  nic1_network = var.nic1_network
-  nic1_subnet  = var.nic1_subnet
-
-  hc_self_link          = google_compute_health_check.multinic-health.self_link
-  service_account_email = var.service_account_email
+locals {
+  zones = data.google_compute_zones.available.names
 }
 
-module "multinic-c" {
+# Manage the regional MIG formation
+module "multinic" {
   source = "../50_compute"
 
   num_instances = var.num_instances
   preemptible   = var.preemptible
   autoscale     = var.num_instances == 0 ? false : true
 
   project_id  = var.project_id
-  name_prefix = "multinic-${var.region}-c"
+  name_prefix = "multinic-${var.region}"
   region      = var.region
-  zone        = "${var.region}-c"
+  zones       = local.zones
 
   nic0_project = var.project_id
   nic0_network = var.nic0_network
@@ -132,16 +93,11 @@ resource "google_compute_region_backend_service" "multinic-main" {
   region                = var.region
   load_balancing_scheme = "INTERNAL"
 
-  backend {
-    group = module.multinic-a.instance_group
-  }
-
-  backend {
-    group = module.multinic-b.instance_group
-  }
-
-  backend {
-    group = module.multinic-c.instance_group
+  dynamic "backend" {
+    for_each = module.multinic.instance_groups
+    content {
+      group = backend.value
+    }
   }
 
   # Note this is the traffic health check, not the auto-healing check
@@ -157,16 +113,11 @@ resource "google_compute_region_backend_service" "multinic-transit" {
   region                = var.region
   load_balancing_scheme = "INTERNAL"
 
-  backend {
-    group = module.multinic-a.instance_group
-  }
-
-  backend {
-    group = module.multinic-b.instance_group
-  }
-
-  backend {
-    group = module.multinic-c.instance_group
+  dynamic "backend" {
+    for_each = module.multinic.instance_groups
+    content {
+      group = backend.value
+    }
   }
 
   # Note this is the traffic health check, not the auto-healing check