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 #44

Closed
wants to merge 1 commit into from
Closed

Use for_each instead of count #44

wants to merge 1 commit into from

Conversation

gechr
Copy link

@gechr gechr commented Aug 7, 2019

This is a pretty large change which uses Terraform 0.12.6's new resource-level for_each keyword instead of count to create >1 instances of the same resource. See hashicorp/terraform#21922 for implementation details.

It solves the general Terraform problem that, when count is used to create multiple resources, each resource is an element in a list, meaning if one is deleted/moved from the middle of the list (thus changing its numeric index), the entire list is reshuffled and a bunch of resources are destroyed/(re)created as a result. See hashicorp/terraform#14275 for the main bug report.

Fixes #38

@gechr
Copy link
Author

gechr commented Aug 7, 2019

The Integration tests are currently failing because the version of Terraform used for the tests is currently 0.12.3 and the changes require 0.12.6+

Error: Reserved argument name in resource block

  on ../../../organizations_iam.tf line 39, in resource "google_organization_iam_member" "organization_iam_additive":
  39:   for_each = {

The name "for_each" is reserved for use in a future version of Terraform.

I've updated to 0.12.6 locally and run the integration tests again and they'll need to be updated before they'll pass. That said, I'd first like to understand whether this PR is likely to be accepted at all before I spend the time doing so. Thoughts?

@morgante
Copy link
Contributor

Thanks for the contribution! I definitely think this is a preferable approach and we would like to go in this direction.

However, I would suggest holding on further work until we complete a planned refactor of moving each resource into a separate submodule.

@aaron-lane aaron-lane added the enhancement New feature or request label Aug 16, 2019
@jason-huling
Copy link

Hey @morgante, is there an issue or PR tracking that refactor you mentioned?

@morgante
Copy link
Contributor

morgante commented Sep 4, 2019

Yes, see #43 to track the refactor.

@gechr
Copy link
Author

gechr commented Sep 16, 2019

Will revisit this once the refactor is complete.

Closing for now.

@gechr gechr closed this Sep 16, 2019
@gechr gechr deleted the gc-for-each branch December 2, 2019 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix adding new bindings in additive mode
4 participants