terraform-google-modules/terraform-google-vm

Wrong number of interfaces using compute_instance module

thiagonache opened this issue · 4 comments

TL;DR

I have an instance template with multiple NICs and a compute instance that uses that instance template. The instance created matches everything on the instance template but the number of NICs.

Expected behavior

All configurations in the instance template to be applied in the instance created including all subnets in the instance template.

Observed behavior

I can see that the instance template is properly created because if I use the console to create the instance using the same instance template, the instance is properly created with all the subnets (NICs).

Terraform Configuration

- Create instance template with multiple interfaces using the proper module
- Create an instance with compute_instance module using the instance template previously created

Terraform Version

v1.0.7
on darwin_amd64

Additional information

No response

A friend of mine has found the bug.

resource "google_compute_instance_from_template" "compute_instance" {
  provider = google
  count    = local.num_instances
  name     = var.add_hostname_suffix ? format("%s%s%s", local.hostname, var.hostname_suffix_separator, format("%03d", count.index + 1)) : local.hostname
  project  = local.project_id
  zone     = var.zone == null ? data.google_compute_zones.available.names[count.index % length(data.google_compute_zones.available.names)] : var.zone

  network_interface {
    network            = var.network
    subnetwork         = var.subnetwork
    subnetwork_project = var.subnetwork_project
    network_ip         = length(var.static_ips) == 0 ? "" : element(local.static_ips, count.index)
    dynamic "access_config" {
      for_each = var.access_config
      content {
        nat_ip       = access_config.value.nat_ip
        network_tier = access_config.value.network_tier
      }
    }
  }

  source_instance_template = var.instance_template
}

When you set the one network again you overwrite the configuration from the instance template. The fix is to rely 100% in the instance template

resource "google_compute_instance_from_template" "compute_instance" {
  provider = google
  count    = local.num_instances
  name     = var.add_hostname_suffix ? format("%s%s%s", local.hostname, var.hostname_suffix_separator, format("%03d", count.index + 1)) : local.hostname
  project  = local.project_id
  zone     = var.zone == null ? data.google_compute_zones.available.names[count.index % length(data.google_compute_zones.available.names)] : var.zone

  source_instance_template = var.instance_template
}

Would you guys accept a PR for such change?

It's complicated because we also need to allow overrides as a feature request (where people want to deploy the same template on multiple networks).

The better thing to do would be to have the network interface block be conditional on whether var.network is provided. We would be happy to accept a PR which does that.

It's complicated because we also need to allow overrides as a feature request (where people want to deploy the same template on multiple networks).

The better thing to do would be to have the network interface block be conditional on whether var.network is provided. We would be happy to accept a PR which does that.

Sounds good. I'm going to work on it when I find some time to spare.

I also ran into this issue while trying to add multiple interfaces to an instance. Another wrinkle to the issue is that when trying to add multiple interfaces to an instance when constraints/compute.vmExternalIpAccess is enabled it fails when trying to add an external ip. From the GCP api's perspective access_config is optional, but from the module's perspective nat_ip and network_ip are required. If there's no value for nat_ip a random external ip gets assigned, but if vmExternalIpAccess is enabled if all just fails. In order for the external address not to bbe randomly assigned access_config need to be ommited.

Instead of getting rid of network_interface I wrapped the appropriate bits with try(), passing along a null value when no value is provided, and it seems to work properly now.

network_interface {
  network                    = var.network
  subnetwork              = var.subnetwork
  subnetwork_project = var.subnetwork_project
  network_ip               = length(var.network_ip) > 0 ? var.network_ip : null
  dynamic "access_config" {
    for_each = var.access_config
    content {
      nat_ip           = try(access_config.value["nat_ip"], null )
      network_tier = try(access_config.value["network_tier"], null)
    }
  }
  dynamic "alias_ip_range" {
    for_each = local.alias_ip_range_enabled ? [var.alias_ip_range] : []
    content {
      ip_cidr_range                    = alias_ip_range.value.ip_cidr_range
      subnetwork_range_name = alias_ip_range.value.subnetwork_range_name
    }
  }
}

With nat_ip wrapped in the try() you can pass null from the module and no external ip will get generated. I can submit a PR if you like. I've never done that before so if there's guidelines on submitting a PR that would be helpful.