Improve partial parsing HCL strings by adding cache

[maunzCache]

Description

Fixes #2203.

This PR will add a caching behavior to PartialParseConfigString in config.go. The original function has been wrapped by a new function called TerragruntConfigFromPartialConfigString which handles cache access if the flag --terragrunt-use-partial-parse-config-cache was provided.

This fix was co-authored by @nilreml.

We are very open to suggestions or objections of this PR. If you find anything that we overlooked, please speak out.

Technical subtleties

The implementation uses the same approach as the setIAMRole cache in config.go (#2010) but drops the additional hashing of the key used in the cache map. We decided to drop this as our understanding is that this interferes with the native behavior of go maps. Go implements the map type as hashmap which already handles uniqueness and collisions by providing a cache bucket implementation. See the docs and implementation for reference.

Testing

As we have no concrete unit test for this implementation besides verifying that our cache works in general, we decided to add two fixtures.

The fixtures are very similar: Both implement dependencies and require heavy parsing due to massive stub data. The first variant implements includes by creating locals which call read_terragrunt_config. The second implements the multi-include approach which was implemented in 0.32. Both fixtures are used in benchmark tests to verify the impact of the implementation.

One benchmark has been implemented for the execution of runGraphDependencies which is the fastest functionality that accesses PartialParseConfigString and could additionally be used to verify if dependency order is correct even after enabling the cache. The second benchmark was implemented for ReadTerragruntConfig as this will call all subsequent functionality which is necessary for parsing. Each benchmark consists of one of the fixtures plus the caching enabled or disabled.

The benchmarks verify that CPU and memory usage don't change drastically when using the cache. However, for huge projects which surely exceed the fixture, use cases should be considered to implement a cache invalidation to prevent out of memory errors.

We realize the limited benefits since this project doesn't make use of benchmarks yet and may have to decide on how to handle them. We suggest implementing least a benchmark call in the CI/CD pipeline which is executing the unit and integration tests. For the future all benchmarks should be collected and somehow be compared to easily find regressions if features are added or changed. This blog post might help with an implementation.

It has to be noted that benchmark tests are not representative but validate our use case.

Implementation notes

During our implementation we couldn't find an answer to the following questions:

1. Was a side effect created for setIamRole?
2. Have the .terragrunt-cache/ directories which are created during the benchmark test to be removed or not?

TODOs

Read the Gruntwork contribution guidelines.

• [x] Update the docs.
• [x] Run the relevant tests successfully, including pre-commit checks.
• [x] Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Add caching to PartialParseConfigString which can be activated via --terragrunt-use-partial-parse-config-cache.

Migration Guide

Nothing to do.

[maunzCache]

Improvements in our scenario. Please note that those times are not 100% reproducible. Sometimes they will differ by up to 5 seconds for the larger runs.

All measurements were successfully crosschecked with a terragrunt plan that has no changes on all live deployments. Edit: Also checked with destroy & apply to ensure no false-positives.

Parsing times - terragrunt init on Lenovo T14s (using WSL Ubuntu 20.04)

scope	0.29.11	caching (0.38.4)
dev2/hub	00:23	00:10
dev2/spoke/account-dev2-b	00:11	00:05
dev2/spoke	00:19	00:07
dev2	00:27	00:10
prod/hub	03:49	00:14
prod/spoke	12:13	00:07
prod	07:21	00:07
shared	00:20	00:09
test/hub	00:26	00:11
test/spoke	00:42	00:07
test	00:45	00:12
cicd	00:22	00:12

Parsing times - terragrunt init on EC2 c5.4xlarge

scope	0.29.11	caching (0.38.4)
dev2/hub	00:27	00:11
dev2/spoke/account-dev2-b	00:13	00:07
dev2/spoke	00:23	00:08
dev2	00:30	00:12
prod/hub	04:06	00:15
prod/spoke	12:34	00:08
prod	08:05	00:16
shared	00:23	00:10
test/hub	00:29	00:13
test/spoke	00:48	00:08
test	00:57	00:13
cicd	00:26	00:11

[maunzCache]

Found that there was a Clone() call which i did not implement. It's fixed now and will be comitted. Also merged upstream to not get any other funny errors.

[nilreml]

Additional note on hashing: Using a hash function on the cache key in the manner done by setIAMRole in config.go is indeed prone to unmitigated collisions of the hash function, potentially leading to incorrect results. I'd suggest fixing it in setIAMRole as well - let us know and we'll add the fix to this PR.

[maunzCache]

Only resolved the merge conflict via github editor. Does seem to create side effects for the unit tests. Will pull locally and check the broken unit tests. Guess it is formatting related.

[denis256]

Terraform fmt............................................................Passed
goimports................................................................Failed
- hook id: goimports
- files were modified by this hook

options/options.g

[maunzCache]

Was just an additional newline. Fixed and pushed. pre-commit had no further issues on my system

[maunzCache]

As i'll leave the customer project @nilreml may take over if there is any changes required for the PR.,

[maunzCache]

Any chance to get endorsement for this feature?

[maunzCache]

@yorinasub17 @zackproser @rhoboat @denis256 Do you have an ETA for me when this gets a chance for a review?

[denis256]

LGTM

[martin566]

@denis256: Thanks for approving this pull-request.

[nilreml]

Thanks a bunch for merging and releasing this :+1: