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

[RFC] Iteration of modules #853

Merged
merged 3 commits into from
Sep 13, 2019
Merged

[RFC] Iteration of modules #853

merged 3 commits into from
Sep 13, 2019

Conversation

yorinasub17
Copy link
Contributor

This proposes an RFC for potentially supporting iteration of modules in Terragrunt. The document contains the background for why we should consider this.

@yorinasub17
Copy link
Contributor Author

cc @bwhaley

`for_each` list to only reference the items you want to destroy, then run `terragrunt destroy` to destroy just those
items, and then restore the list to the version with the items removed).

### Option 3: scaffolding tool that code gens live config using a template
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bwhaley : After contemplating about this feature, I am leaning towards this approach for our purposes. The implementation for supporting iteration in Terragrunt has many nuances that suggest it is not a great idea.

Better to wait for the official solution, working around it using code generation. Code generation appears to be the lesser evil of a workaround.

@yorinasub17 yorinasub17 changed the title RFC for iteration of modules [RFC] Iteration of modules Sep 5, 2019
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Great RFC. Really highlights the complexity of for_each natively being built into Terragrunt, and the limited shelf life of such a feature.

I'm reluctantly a +1 on code generation. I'd be tempted to do the generate in the Terraform code (.tf files), simulating for_each on the module by just generating N copies of the module from some template, and committing that. Later, when for_each is officially added to Terraform, remove the generated code, and replace it with one module with the for_each on it.

- `include`
- `dependency`
- Everything else
- Support `merge_parent` in `remote_state`, which ultimately adjusts how `mergeConfigWithIncludedConfig` merges the
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way for us to check the user isn't accidentally overriding the config multiple times? Does remote_state config in Terragrunt become required to use this feature? Or do we read the .terraform/terraform.tfstate file? Seems a bit complicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think local state will work because I was imagining each iteration will have it's own directory in the cache, but yes this isn't immediately obvious.


**Cons**:

- The implementation of the runners can become complex, especially in the dependency detection for the `xxx-all`
Copy link
Member

Choose a reason for hiding this comment

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

Yea, there are a lot of complexities here, and many, many ways to get it wrong.


**Cons**:

- Code generation is complex. When should you run the code generation step? How often? How do you know if there is
Copy link
Member

Choose a reason for hiding this comment

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

Do you check in the generated code? If so, you have to maintain it (e.g., 19 sets of files/folders for 19 AWS regions). If it's always generated at runtime, then interacting with just one of the generated things becomes harder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Valid points! Adding this in as one of the concerns.

@antonbabenko
Copy link
Contributor

@yorinasub17 Thanks for this RFC! Here are my 5 cents...

After reading the whole RFC and solving it using cookiecutter before, I like code generation as the proposed solution.

  1. I think keeping all generated code in a repo (even if there are 19*19 similar folders) is better because this will allow working on any of them differently (eg, try changes in playground AWS account/region before propagating to the rest).

  2. Keeping more files and folders is better for humans to understand the whole footprint (which regions do we use? how they are configured?)

  3. Code generation tool can be not related to terragrunt as such because the same is needed for native terraform projects where multiple similar modules have to be managed (while we are waiting for for_each with modules).

  4. Code generation (especially if implemented as another tool, see above) will allow to keep terragrunt user-friendly and not overload it with features.

@yorinasub17
Copy link
Contributor Author

I'd be tempted to do the generate in the Terraform code (.tf files), simulating for_each on the module by just generating N copies of the module from some template, and committing that. Later, when for_each is officially added to Terraform, remove the generated code, and replace it with one module with the for_each on it.

Ah that is a good point.


I'm reluctantly a +1 on code generation.

The one thing I am not sure about is if this should be a first class thing in terragrunt. I think this meshes really well with the scaffolding idea (which should be a separate RFC). As in, if we have the terragrunt scaffold (or terragrunt codegen, which I like slightly better) feature, this need of iteration can be resolved by the codegen system supporting template processing. What do you think of that approach?

@yorinasub17
Copy link
Contributor Author

yorinasub17 commented Sep 6, 2019

Here are my 5 cents...

Thanks for your input @antonbabenko !

I think keeping all generated code in a repo (even if there are 19*19 similar folders) is better because this will allow working on any of them differently (eg, try changes in playground AWS account/region before propagating to the rest).

Good point, although I would be concerned about accidentally checking in the changes in the code itself instead of the generator. This could happen if one is not aware of the codegen process.

Code generation (especially if implemented as another tool, see above) will allow to keep terragrunt user-friendly and not overload it with features.

I understand this sentiment and we definitely want to keep terragrunt simple as possible. That said, having codegen baked into terragrunt has some advantages in improving the UX (think rails new). I plan on writing a different RFC with our latest thoughts around this within Gruntwork which highlights this.

@bwhaley
Copy link
Contributor

bwhaley commented Sep 9, 2019

Thanks @yorinasub17 for writing this RFC and thinking through the problem, and thanks to everyone else for your input. Seems like the consensus is that adding iteration to terragrunt will be hard, and given that for_each is coming to terraform modules natively very soon, it probably isn't worthwhile to add it here.

Does it even make sense to include code generation (including examples or docs) here in terragrunt? Seems tangential and perhaps should live elsewhere.

Since we need the "run this module in all regions" functionality specifically for the AWS Config module as part of a CIS compliance project, perhaps we should just do the code generation in that repo instead, and close this one out. We could have a new PR in that repo discussing exactly how the code generation would work, including addressing some of the cons raised here. For example:

When should you run the code generation step?

After modifying the template.

How often?

Every time the generated code for all regions need to change.

How do you know if there is drift?

All the generated files should have headers saying "don't touch this, it's generated code." The generated files would be overwritten any time changes are made to the master.

Do you check in the generated code? If so, you have to maintain it (e.g., 19 sets of files/folders for 19 AWS regions). If it's always generated at runtime, then interacting with just one of the generated things becomes harder.

Yes, always check in the generated code, and never edit anything but the master.

Obviously there are some deeper complexities that will come up during development, but the point is, code generation probably doesn't belong in terragrunt.

@yorinasub17
Copy link
Contributor Author

Given all the discussions, I believe we have reached the conclusion that we shouldn't implement anything specific in Terragrunt regarding this specific use case. There may be other use cases for having code generation in Terragrunt, but those should be optimized for the use cases where it makes sense to bake into Terragrunt.

I have updated the RFC with the recommendation and conclusions. Going to merge this in now so we have it in the repo for reference.

Thanks to everyone who took the time to review this!

@yorinasub17 yorinasub17 merged commit a3534f1 into master Sep 13, 2019
@yorinasub17 yorinasub17 deleted the yori-rfc-iteration branch September 13, 2019 10:15
@s1mark s1mark mentioned this pull request Oct 12, 2020
@smitthakkar96
Copy link
Contributor

Hey since terraform implemented support for for_each, can we revisit this?

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