Add support for Terraform 0.12

[ivankorn]

Please, note:
0.12 support for managed_instance_group example depends on this.
Module to be updated once this task is done.
Linked:

• module container-vm
• terraform-google-managed-instance-group

[ivankorn]

Fixed by #49

[ivankorn]

I'm moving discussion from closed PR #49 here

@itscaro, thank you for doing a good job on migrating examples!
Unfortunately your PR for it uses deprecated MIG template, so terraform plan discovers a lot of errors like this

$ terraform plan

Error: Invalid value for module argument

  on main.tf line 65, in module "gce-lb-http":
  65:   target_tags       = [module.mig1.target_tags, module.mig2.target_tags]

The given value is not suitable for child module variable "target_tags"
defined at ../../variables.tf:48,1-23: element 0: string required.


Error: Unsupported block type

  on mig.tf line 20, in data "template_file" "group-startup-script":
  20:   vars {

Blocks of type "vars" are not expected here. Did you mean to define argument
"vars"? If so, use the equals sign to assign it a value.

As @morgante advised here the older MIG module is deprecated and we should use https://github.com/terraform-google-modules/terraform-google-vm/tree/master/modules/mig instead.

@itscaro, unless you're willing to continue your work I'm going to pick up from where you left and proceed with following:

• Switch MIG and other depdency-modules to TF 0.12 compatible
• Add strict types for variables
• Remove instances of unnecessary string interpolation from the code base (if any left)
• Remove unnecessary "element" calls (if any left)
• Update types to number/bool where necessary

Also I'd like to alight this module a bit with terraform-google-modules

By adding terraform, bash and white-space linters + doc_generator as a first step in this direction.

[itscaro]

@ivankorn please go ahead, I'm going to be off till Sep. Thank you.

[ivankorn]

@ivankorn please go ahead, I'm going to be off till Sep. Thank you.
Okay, then :) have a good time @itscaro

[jtfogarty]

@ivankorn I've been following your work on this. I'm looking for this internal lb example
are you working on this? if not, are the items you mentioned above needed to convert this to TF 0.12? Thanks

[morgante]

@jtfogarty We're hoping to update all the LB modules to v0.12 in the next week or two.

[ivankorn]

@ivankorn I've been following your work on this. I'm looking for this internal lb example
are you working on this?

@jtfogarty, not at the moment. But given the update Morgante provided - there are chances for me to pick up some more of lb modules soon.

if not, are the items you mentioned above needed to convert this to TF 0.12? Thanks

More than that, I think in addition to those, we should take care of following:

• Add linters and updated helper scripts
• Standardize tests
• Switch to new CI

Just to satisfy immediate functionality needs last two points can probably be addressed out of migration scope.
All the TF modules will likely follow the terraform-google-modules new approach

[Dev25]

As part of this 0.12 move is it worth moving straight to using for_each (0.12.6+) over the count/index method for managing resources.

https://www.terraform.io/docs/configuration/resources.html#for_each-multiple-resource-instances-defined-by-a-map-or-set-of-strings

I know other modules are looking to move to using for_each now that is it released e.g. GKE with node-pools, in this module it can be used for backend_services/health_checks

[morgante]

@Dev25 Yes, I think that's a good idea. @ivankorn Please switch to using for_each.

[ivankorn]

@Dev25 Yes, I think that's a good idea. @ivankorn Please switch to using for_each.

for_each switch is commited here

For now interface is kept everywhere but here. That can be easily reverted, but I don't see any sense in keeping at list that. I'm also going to come up with a proposal on more changes as discussed with @aaron-lane on the call today
Since the PR is still in draft I left some comments here and there on interface changes I'd like to make. Followed by migration plan in readme/changelog of course

[ivankorn]

@morgante @Dev25 fyi: for_each adds a bit to a good amount of entropy on the migration task.
We're migrating

• from 0.11 to 0.12
• from GoogleCloudPlatform/managed-instance-group to terraform-google-modules/vm/google//modules/instance_template and terraform-google-modules/vm/google//modules/mig (and adding a number of resources as deps for them)
• from GoogleCloudPlatform/nat-gateway/google to terraform-google-modules/cloud-nat

As I reported currently for_each has a bug which brakes all multi-backend examples down

cc: @paulpalamarchuk @nick4fake @kopachevsky

[ivankorn]

Update: As @morgante advised I pushed for_each changes in a separate branch aside and reverted them back

TF issue ref: hashicorp/terraform#22735
cc: @kopachevsky @nick4fake @paulpalamarchuk @ingwarr @aaron-lane

[sebastiansirch]

Hi @ivankorn ,
just want to let you know that I already tested and used your current working state in a project where we needed a Terraform 12 compatible version :-) Works fine.
Best,
Sebastian

[ivankorn]

Hi, @sebastiansirch
Thank you for your feedback! I'm very happy to hear that migration efforts were successful! :)
I'll notify you once PR get merged, it's under code review which shouldn't take much time.

Please note: there are #53 and #54 opened for interface changes, so you might need to adjust your side code as well after those get merged.
Thanks

[mercuriete]

@ivankorn

Thank you for this.
I already have a custom version of this module removing everything wasnt working.
I will switch to this official when possible.

Thank you very much.