fix: updated subnets to use for_each rather then count

[tfhartmann]

I also updated the outputs as well. This will be large state change however and probably needs to be a major release

This should address issue #70 - however I didn't update the routes resource which will likely have a similar problem

[tfhartmann]

Hmm outputs have changed a bit, I'm going to see if I can create some tests for them

[tfhartmann]

@skenmy @morgante this PR should be ready for a review! It will result in a pretty large state change for the end use, so some amount of state surgery will be required , something like `

terraform state mv module.example.google_compute_subnetwork.subnetwork[0] module.example.google_compute_subnetwork.subnetwork[0]  module.example.google_compute_subnetwork.subnetwork[0] module.example.google_compute_subnetwork.subnetwork["us-east1/subnetwork1"]

[tfhartmann]

Please let me know if there are other requested changes, or concerns I can address in this PR 😃

[ludoo]

LGTM with a couple comments on syntax.

[morgante]

Thanks for the contribution. Just want to note that we need to write a migration guide before merging/releasing.

[tfhartmann]

Thanks for the contribution. Just want to note that we need to write a migration guide before merging/releasing.

Is adding that to the README ok, or would you prefer it somewhere else?

[morgante]

Is adding that to the README ok, or would you prefer it somewhere else?

Preferable a separate doc + script, linked from changelog, following the GKE example.

[skenmy]

LGTM

[tfhartmann]

Preferable a separate doc + script, linked from changelog, following the GKE example.

I rebased this branch against master, as well as attempted to add a migration doc, and a small helper script.

I'm now fixing a bunch of small linting errors, after that I think this will be good to go

[tfhartmann]

Rebased against master this morning so that this PR wouldn't have merge conflicts

[tfhartmann]

@morgante I rebased this again this morning so as not to conflict with master. I don't mean to nag, could I get some feedback on this PR? Is there anything else you folks would like to see? I think I have addressed the outstanding feedback, if there is more I would be happy to address that as well. Is this blocked for some reason? This bug has help me up from using this module and I'd really like to use it.

[morgante]

@tfhartmann Apologies for sitting on this so long. Code is looking great, but unfortunately the migration script isn't working for me:

$ MODULE_NAME=vpc ../../../modules/network/scripts/migrate.sh

Error: Invalid character

  on  line 2:
  (source code not available)

Expected an attribute access or an index operator.

Dry-run looks incorrect too:

MODULE_NAME="vpc" ../../../modules/network/scripts/migrate.sh  --dry-run
terraform state mv module.vpc-nonprod.module.vpc.data.google_compute_subnetwork.created_subnets[0] module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"us-central1/clf-shared-net-gke\"]
terraform state mv module.vpc-nonprod.module.vpc.google_compute_network.network module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"clf-shared-net-nonprod\"]
terraform state mv module.vpc-nonprod.module.vpc.google_compute_shared_vpc_host_project.shared_vpc_host[0] module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"clf-shared-net-nonprod\"]
terraform state mv module.vpc-nonprod.module.vpc.google_compute_subnetwork.subnetwork[0] module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"us-central1/clf-shared-net-gke\"]
terraform state mv module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[0] module.vpc-nonprod.module.vpc

Would you mind taking another look at the migration script? I'd suggest simply reusing one of the existing migration scripts for consistency/reliability.

Also: could we place the migration script in helpers/? (Not scripts/.)

[tfhartmann]

@tfhartmann Apologies for sitting on this so long. Code is looking great, but unfortunately the migration script isn't working for me:
Ah, I think I see the problem, you actually have two modules named vpc nested under module.vpc-prod and module.vpc-nonprod respectively. Wrote the shell script with the input of MODULE_NAME so that people who were using multiple modules could explicitly determine which module they wanted to upgrade, and tested it against the examples, which use module.generic.module.explicit as a naming pattern, and against flat modules, - module.name both of which worked ok. I didn't test it against module.explicit.module.generic as a pattern. Honestly I think that may be a bit of an outlier in that I think most folks will use this against a module with a flat naming structure, however I've updated the script to just migrate all of the VPC modules it finds, which should now work against all the above combinations.

$ MODULE_NAME=vpc ../../../modules/network/scripts/migrate.sh

Error: Invalid character

  on  line 2:
  (source code not available)

Expected an attribute access or an index operator.

Dry-run looks incorrect too:

MODULE_NAME="vpc" ../../../modules/network/scripts/migrate.sh  --dry-run
terraform state mv module.vpc-nonprod.module.vpc.data.google_compute_subnetwork.created_subnets[0] module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"us-central1/clf-shared-net-gke\"]
terraform state mv module.vpc-nonprod.module.vpc.google_compute_network.network module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"clf-shared-net-nonprod\"]
terraform state mv module.vpc-nonprod.module.vpc.google_compute_shared_vpc_host_project.shared_vpc_host[0] module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"clf-shared-net-nonprod\"]
terraform state mv module.vpc-nonprod.module.vpc.google_compute_subnetwork.subnetwork[0] module.vpc-nonprod.module.vpc
module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[\"us-central1/clf-shared-net-gke\"]
terraform state mv module.vpc-prod.module.vpc.google_compute_subnetwork.subnetwork[0] module.vpc-nonprod.module.vpc

Would you mind taking another look at the migration script? I'd suggest simply reusing one of the existing migration scripts for consistency/reliability.

To be honest, when I looked at the python script, it looked like a bit more overhead then is actually needed. I would have to remove the bits that update the manifest files, and the parts that manipulate state looked like it was making calls to the shell. I think that rather than use python to invoke shell commands, it is simper to use a bash script in this instance.

Also: could we place the migration script in helpers/? (Not scripts/.)

I moved the script to helpers/

[morgante]

@tfhartmann Unfortunately I'm still having issues using the migration script, but I'm going to take a stab at fixing it myself.