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

use for_each instead of count to create resource #15

Conversation

adrian-gierakowski
Copy link
Contributor

This prevents unnecessary recreation of resources when content/order of the
names list changes.

Addresses issue mentioned in: https://github.com/terraform-google-modules/cloud-foundation-fabric/tree/73b7613cc062491305ebfabf8a8b1db0c53fc55b/foundations#things-to-be-aware-of

WARNING: this is a breaking change as the terraform state created before this
change will be incompatible with state expected after the change. This can
be fixed with terraform state mv.

This prevents unnecessary recreation of resources when content/order of the
names list changes.

Addresses issue mentioned in: https://github.com/terraform-google-modules/cloud-foundation-fabric/tree/73b7613cc062491305ebfabf8a8b1db0c53fc55b/foundations#things-to-be-aware-of

WARNING: this is a breaking change as the terraform state created before this
change will be incompatible with state expected after the change. This can
be fixed with `terraform state mv`.
@adrian-gierakowski
Copy link
Contributor Author

this is similar to terraform-google-modules/terraform-google-folders#21

@ludoo
Copy link
Contributor

ludoo commented Dec 2, 2019

Adrian, can you make sure tests pass? I know it's kind of a bother, but it's pretty important especially when doing breaking changes and setting a new baseline.

@adrian-gierakowski
Copy link
Contributor Author

Yes, sure. Will try to fix it later today.

@adrian-gierakowski
Copy link
Contributor Author

@ludoo so the tests pass, but I had to patch the gcr/images/cloud-foundation-cicd/cft/kitchen-terraform image locally since the latest available version (2.4.0) comes with terraform 0.12.3 which has no support for for_each in resources. I guess someone is going to have to push a new version of the test image

@morgante
Copy link
Contributor

morgante commented Dec 3, 2019

@adrian-gierakowski We should switch to using the devtools image, which has support for Terraform 0.12.6. Ex. https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/Makefile#L22

@adrian-gierakowski
Copy link
Contributor Author

not sure why it's failing this time, maybe someone with access to cloudbuild logs can pick this up

@ludoo
Copy link
Contributor

ludoo commented Dec 3, 2019

not sure why it's failing this time, maybe someone with access to cloudbuild logs can pick this up

The Cloud Build trigger is looking for a missing cloudbuild.yaml file. @averbuks can you check it out?

@averbuks
Copy link
Member

averbuks commented Dec 5, 2019

@ludoo @adrian-gierakowski it's failing because that repo is still using "old way" of testing, I will take it #7

@averbuks
Copy link
Member

averbuks commented Dec 5, 2019

@adrian-gierakowski looks really good, could you pls add that change as unreleased to the CHANGELOG

@averbuks
Copy link
Member

averbuks commented Dec 5, 2019

@adrian-gierakowski looks really good, could you pls add that change as unreleased to the CHANGELOG

I hope you don't mind I did it myself

Copy link
Member

@averbuks averbuks left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for contribution.

@bkamin29
Copy link

UP, can you merge this PR ?
It's very better to use for_each instead of count with Terraform 0.12

@morgante morgante merged commit 19c8a02 into terraform-google-modules:master Jan 31, 2020
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.

5 participants