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

fix(UMIG): access_config should be 2D array #111

Merged

Conversation

UrosSimovic
Copy link
Contributor

@UrosSimovic UrosSimovic commented Sep 9, 2020

Problem:

Currently it is not possible to have multiple instances with predefined access configs (external ips). If one would want to create 3 instances with 3 external ips, each instance would get same 3 predefined external ips. Which is not possible.

Solution:

Make access_config 2D array, so each instance would get an array of predefined ips.

@@ -56,7 +56,7 @@ resource "google_compute_instance_from_template" "compute_instance" {
network_ip = length(var.static_ips) == 0 ? "" : element(local.static_ips, count.index)

dynamic "access_config" {
for_each = var.access_config
for_each = var.access_config[count.index]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like our integration tests are failing here. In particular, this example when we default access_config to []. Perhaps we can have a check to make sure we only access var.access_config[count.index] if length(var.access_config) > 0? This however would mean if num_instances or static_ips > 1 and we want access_config for only one instance, we will need to provide empty lists like

[[{
  // when static_ips=4
    nat_ip       = google_compute_address.ip_address.address
    network_tier = "PREMIUM"
  }],[],[],[]]

Copy link
Contributor Author

@UrosSimovic UrosSimovic Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will solve the issue. And also one wouldn't need to provide a list as yours, but just a list with first element.

dynamic "access_config" {
  # convert to map to use lookup function with default value
  for_each = lookup({for k, v in var.access_config: k => v}, count.index, [])
  content {
    nat_ip       = access_config.value.nat_ip
    network_tier = access_config.value.network_tier
  }
}

Lookup with a default value works just with maps and didn't find anything similar for lists.

@morgante
Copy link
Contributor

@UrosSimovic Please regenerate docs.

@UrosSimovic
Copy link
Contributor Author

@UrosSimovic Please regenerate docs.

Nothing changes:

$ make generate_docs
docker run --rm -it \
	-v "/home/uros/tmp/terraform-google-vm":/workspace \
	gcr.io/cloud-foundation-cicd/cft/developer-tools:0 \
	/bin/bash -c 'source /usr/local/bin/task_helper_functions.sh && generate_docs'
Generating markdown docs with terraform-docs
Working in ./examples/compute_instance/simple ...
2020/09/15 11:54:37 At 24:21: Unknown token: 24:21 IDENT var.region
Warning! Exit code: 1
Working in ./examples/instance_template/additional_disks ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Working in ./examples/instance_template/simple ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Working in ./examples/mig/autoscaler ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Skipping ./examples/mig/full because README.md does not exist.
Working in ./examples/mig/simple ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.region
Warning! Exit code: 1
Working in ./examples/mig_with_percent/simple ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Working in ./examples/preemptible_and_regular_instance_templates/simple ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Skipping ./examples/umig/full because README.md does not exist.
Working in ./examples/umig/named_ports ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Working in ./examples/umig/simple ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Working in ./examples/umig/static_ips ...
2020/09/15 11:54:37 At 19:13: Unknown token: 19:13 IDENT var.project_id
Warning! Exit code: 1
Working in ./modules/compute_instance ...
2020/09/15 11:54:37 At 18:19: Unknown token: 18:19 IDENT var.hostname
Warning! Exit code: 1
Working in ./modules/instance_template ...
2020/09/15 11:54:37 At 21:13: Unknown token: 21:13 IDENT var.source_image
Warning! Exit code: 1
Working in ./modules/mig ...
2020/09/15 11:54:37 At 20:18: Unknown token: 20:18 IDENT concat
Warning! Exit code: 1
Working in ./modules/mig_with_percent ...
2020/09/15 11:54:37 At 20:18: Unknown token: 20:18 IDENT concat
Warning! Exit code: 1
Working in ./modules/preemptible_and_regular_instance_templates ...
2020/09/15 11:54:38 At 24:26: Unknown token: 24:26 IDENT var.project_id
Warning! Exit code: 1
Working in ./modules/umig ...
2020/09/15 11:54:38 At 18:19: Unknown token: 18:19 IDENT var.hostname
Warning! Exit code: 1
Skipping ./test/fixtures/compute_instance/simple because README.md does not exist.
Skipping ./test/fixtures/instance_template/additional_disks because README.md does not exist.
Skipping ./test/fixtures/instance_template/simple because README.md does not exist.
Skipping ./test/fixtures/mig/autoscaler because README.md does not exist.
Skipping ./test/fixtures/mig/simple because README.md does not exist.
Skipping ./test/fixtures/mig_with_percent/simple because README.md does not exist.
Skipping ./test/fixtures/preemptible_and_regular_instance_templates/simple because README.md does not exist.
Skipping ./test/fixtures/umig/named_ports because README.md does not exist.
Skipping ./test/fixtures/umig/simple because README.md does not exist.
Skipping ./test/fixtures/umig/static_ips because README.md does not exist.
Skipping ./test/setup because README.md does not exist.

@morgante morgante merged commit 69f7520 into terraform-google-modules:master Sep 15, 2020
raolivei added a commit to scoremedia/terraform-google-vm that referenced this pull request Feb 25, 2021
…s" for TF v0.13 support (#2)

* fix: Correct names for instances in preemptible and regular instance module (terraform-google-modules#81)

BREAKING CHANGE: The preemptible_and_regular_instance_templates modules have had name_prefixes renamed, forcing instances to be recreated.

* chore: fixing typo in instance_template README (terraform-google-modules#86)

* feat: Add support for assigning public IPs directly to instances. (terraform-google-modules#83)

* chore: release 3.0.0 (terraform-google-modules#82)

* updated CHANGELOG.md [ci skip]

* updated README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

* fix terraform-google-modules#88 issues with autogen tmpl for mig (terraform-google-modules#93)

* chore: fix format

* feat: Add stateful disk support (terraform-google-modules#90)

* Added: stateful configuration

* Added: var udpate

* Added: var udpate

* Updated: stateful_disk variable, autogen docs

* feat: Add wait_for_instances and configurable timeout support for mig (terraform-google-modules#96)

BREAKING CHANGE: instance_redistribution_type must now be specified for update policies.

* chore: release 4.0.0 (terraform-google-modules#98)

* fix: relax version constraints to enable terraform 0.13.x compatibility (terraform-google-modules#108)

* chore: Updating Links in README (terraform-google-modules#97) (terraform-google-modules#112)

* Update README.md

* Update README.md

* fix: Terraform version upgrade for compute_instance module from 0.12.6 to 0.12.7 (terraform-google-modules#103)

* fix(UMIG): access_config should be 2D array (terraform-google-modules#111)

BREAKING CHANGE: var.access_config has been changed to a 2D array, with a separate element for each VM.

* chore: release 5.0.0

* feat: Added instance_group_manager output (terraform-google-modules#105)

* add instance_group_manager output

needed to create a stateful MIG in conjunction with https://www.terraform.io/docs/providers/google/r/compute_region_per_instance_config.html

* fix: autogen/outputs.tf.tmpl use {{ module_name }} in instance_group_manager output

Co-authored-by: Morgante Pell <[email protected]>

* chore: release 5.1.0 (terraform-google-modules#115)

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

* feat: adds an output for the health check self_links to be consumed by load balancer resources outside this module (terraform-google-modules#119)

* feat: adds an output for the health check self_links to be consumed by load balancer resources outside this module

* chore: docs generation

* moved change up to autogen

* aligning CI and makefile image for docs generation

* feat!: Update default source image and family to latest CentOS 7 (terraform-google-modules#126)

* feat: add TF 0.13 constraint and module attribution (terraform-google-modules#128)

BREAKING CHANGE: Minimum Terraform version increased to 0.13.

* chore: Remove < 0.14 constraints in examples/fixtures [bot-pr] (terraform-google-modules#129)

* chore: release 6.0.0 (terraform-google-modules#127)

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

Co-authored-by: GeneralAardvark <[email protected]>
Co-authored-by: fclerg <[email protected]>
Co-authored-by: Richard Chen Zheng <[email protected]>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Pasquale D'Agostino <[email protected]>
Co-authored-by: Morgante Pell <[email protected]>
Co-authored-by: Konstantin Kirpichnikov <[email protected]>
Co-authored-by: Sean Johnson <[email protected]>
Co-authored-by: Brandon J. Bjelland <[email protected]>
Co-authored-by: Darpan Shah <[email protected]>
Co-authored-by: Aaron Yang <[email protected]>
Co-authored-by: Uroš <[email protected]>
Co-authored-by: Ryan Canty <[email protected]>
Co-authored-by: Adrian <[email protected]>
Co-authored-by: Brandon J. Bjelland <[email protected]>
Co-authored-by: Yurii Serhiichuk <[email protected]>
Co-authored-by: Bharath KKB <[email protected]>
Co-authored-by: cloud-foundation-bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants