Implement terragrunt_output #828

yorinasub17 · 2019-08-09T20:05:01Z

The implementation has now been completely changed. get_output is no longer a function and instead is a new block called terragrunt_output. The benefits of this approach are:

Predictable execution paths: it is now very clear when outputs are pulled into the config. See https://github.com/gruntwork-io/terragrunt/blob/38aca82ff0890af3a4584870b4bd5eed8f788311/README.md#configuration-parsing-order for more info.
Automatic dependency graph injection: since the parsing order is now predictable, we can seed the dependency graph with the configs specified by terragrunt_output blocks. See https://github.com/gruntwork-io/terragrunt/blob/38aca82ff0890af3a4584870b4bd5eed8f788311/README.md#passing-outputs-between-modules for more info.
Reuse results of a single terragrunt output call: with get_output, terragrunt output had to be called each time the function was executed to keep the implementation sane. Since get_output couldn't be parsed before locals, this made it difficult to reuse the output results of a single call if you needed multiple variables from the output. Now only one call needs to be made per block and it can be referenced multiple times.

This provides an implementation of get_output, an interpolation function that can be used to extract the output of another terraform module wrapped with terragrunt config. See the README changes for more details.`

Note that as part of the implementation, it was required to refactor the configuration parsing during a xyz-all command to only do a partial decoding. Otherwise, apply-all will fail even if you specify dependencies because it will interpolate the get_output call in the initial pass to read the dependencies from the config. This is handled in a new, PartialParseConfigFile function which will only parse the sections specified. Note that because locals, include, dependencies, terraform blocks are evaluated in this section, apply-all in the initial setup will still fail if get_output. This is covered in the README.

brikis98

Wow, fantastic. So excited to see this 👍

README.md

brikis98 · 2019-08-09T21:29:24Z

README.md

+will fail when you run an `apply-all` since the output will be interpolated before the target config has applied.
+Additionally, during an `apply-all`, the terragrunt configuration has to be partially interpolated in order to build up
+the dependency tree. This means that you will have issues using `apply-all` if `get_output` is used in the following
+blocks, as it will be interpolated prior to any modules being applied:


Not sure I follow this last one? Can you give an example?

Imagine you had:

live ├── sql │ └── terragrunt.hcl └── vpc └── terragrunt.hcl

And you had the following in live/sql/terragrunt.hcl:

locals = { vpc_path = "../vpc" vpc_id = get_output(local.vpc_path, "vpc_id") } dependencies { paths = [local.vpc_path] } inputs = { vpc_id = local.vpc_id }

Now suppose we have a completely clean project and nothing is applied yet and we run apply-all. In order to build the dependency tree, terragrunt needs to parse out the dependencies block and decode them before any modules have been applied. In the above use case, the dependencies block also references locals, so we need to decode locals as well. As part of that, we need to interpolate get_output.

Well, get_output will try to read the output of the vpc module and fail, because that hasn't been applied yet, because we are still in the phase of building up the dependency tree.

Oh, I gotcha. So then the get_output function would have to:

Is this a single-module command (e.g., apply) or multi-module (e.g., apply-all).

If single-module, and the dependency has not yet been applied (i.e., output is missing), return an error immediately.

If multi-module, add the VPC to the dependency graph, pause config parsing until the command (e.g., apply) has been executed in that dependency, and then continue config parsing after.

Would that work?

This still exists, but is less of an issue because of the way you access terragrunt_output.

brikis98 · 2019-08-09T21:31:59Z

README.md

+the dependency tree. This means that you will have issues using `apply-all` if `get_output` is used in the following
+blocks, as it will be interpolated prior to any modules being applied:
+
+- `locals`


Hm, if you need to fetch 10 different outputs from a single dependency, I suspect 10 sequential terragrunt output calls would be slow. Do we cache the outputs in memory in between? If not, it would be nice to allow users to cache all the outputs in a locals variable once and then look up values within it...

Yea that is a good point. Under most circumstances, you can use get_output in locals. There are some caveats you have to be aware of: #828 (comment)

My first impression was that we should maintain an internal, in-memory cache of (module absolute path, all outputs for that module) pairs. Any time you call get_output, it first checks the cache for that value: if it's present, it returns it immediately; if it's absent, it calls terraform output and caches the result. This ensures maximal performance without any extra effort for the user...

Depending on the implementation, I worry about what happens when we are going through multiple passes of the config, as it does in the xxx-all command. Here is where a global memoized cache could be problematic:

In the first pass through, it would run get_output on the previous state. This result gets cached.

The dependency gets applied as part of apply-all, in the dependency order.

We parse the config again to apply the dependent module. As part of that, we call get_output. We look up in the cache, and return the previous value, which is NOT the newly applied state.

Note that this only happens when you depend on the awkward chicken-and-egg scenario of using get_output in locals, and in a naive implementation using a global cache that is stored for the duration of a single terragrunt call.

We could have a different implementation where the cache is scoped to a single config parsing, which trades off more reuse for better predictability. I am leaning towards implementing it this way, as cache bugs are always super hard to debug. This of course requires more refactoring, since now I need to store the cache somewhere in a struct that is available throughout the parsing stage.

In the new implementation, you will call terragrunt output at most once per dependency per config. There is still room for memoization to cache across apply-all calls, but I think this is world's better than get_output which was once per function call.

And we don't need to do caching (which is always a pain to maintain due to annoying cache busting bugs) :)

brikis98 · 2019-08-09T21:35:31Z

config/config_helpers.go

+	// target config check: make sure the target config exists
+	targetConfig := params[0]
+	if util.IsDir(targetConfig) {
+		targetConfig = util.JoinPath(targetConfig, DefaultTerragruntConfigPath)


TODO for the future: we could support reading outputs from normal Terraform modules (i.e., those not meant to be used with Terragrunt) by running terraform output instead of terragrunt output. Not particularly high priority for now though!

README.md

config/config_helpers.go

config/config_partial.go

- Always read the entire output map so we have type data - Refactor json to cty.Value conversion into its own function with unit tests

yorinasub17 · 2019-08-09T23:28:16Z

Ok the main sticking point is finding a good way to cache the get_output calls. Unfortunately, I am not sure what is the best way. I am not a fan of memoizing in the global space, and using locals has some issues as mentioned above.

I expect that the real solution is in #828 (comment), where we won't need to parse the dependencies block anymore. Although... we still need the locals block parsing since I expect you will want to use it for the terraform block parsing...

README.md

brikis98 · 2019-08-12T23:07:12Z

README.md

+will fail when you run an `apply-all` since the output will be interpolated before the target config has applied.
+Additionally, during an `apply-all`, the terragrunt configuration has to be partially interpolated in order to build up
+the dependency tree. This means that you will have issues using `apply-all` if `get_output` is used in the following
+blocks, as it will be interpolated prior to any modules being applied:


Oh, I gotcha. So then the get_output function would have to:

Is this a single-module command (e.g., apply) or multi-module (e.g., apply-all).

If single-module, and the dependency has not yet been applied (i.e., output is missing), return an error immediately.

If multi-module, add the VPC to the dependency graph, pause config parsing until the command (e.g., apply) has been executed in that dependency, and then continue config parsing after.

Would that work?

brikis98 · 2019-08-12T23:08:46Z

README.md

+the dependency tree. This means that you will have issues using `apply-all` if `get_output` is used in the following
+blocks, as it will be interpolated prior to any modules being applied:
+
+- `locals`


My first impression was that we should maintain an internal, in-memory cache of (module absolute path, all outputs for that module) pairs. Any time you call get_output, it first checks the cache for that value: if it's present, it returns it immediately; if it's absent, it calls terraform output and caches the result. This ensures maximal performance without any extra effort for the user...

config/config_helpers.go

config/config_partial.go

yorinasub17 · 2019-08-13T01:39:56Z

Ok new idea. This got me thinking of a potential way to solve this: what if we introduce a new block? After navigating HCL2, I think I am beginning to understand that it is significantly easier to implement partial parsing decoding of individual blocks as opposed to when it appears in the middle of an AST.

Given that, it seems like the best way to achieve all of the goals is to introduce a new block construct like terraform_remote_state, but at the terragrunt.hcl level. Here is an example, since I think it will be clear once I show it:

terragrunt_output "vpc" {
  config = "../vpc"
}

inputs = {
  vpc_id = terragrunt_output.vpc.vpc_id
}

In this model, the caching problem goes away because we can resolve all the terragrunt_outputs in the initial parsing and stored somewhere as a reference which can then be reused.

Parsing order problems go away as well, since it can be parsed independently of the rest of the config (except maybe locals and includes?).

Implementing partial parsing just for dependency building is easy as well, since all we need to do is parse the blocks into a list of structs to get the config: no need to walk an AST!

Another benefit is that we have full control over when the output pulling happens, since now it is a first class task in the decoding pipeline, happening before we pass the HCL struct to the decoder. So the parsing logic will now be:

parse locals
parse include
parse terragrunt_output (replaces dependencies)
Get outputs from each terragrunt_output
parse the rest of config

brikis98 · 2019-08-13T09:43:24Z

That seems like an elegant solution!

…that uses it yet) - Replace free strings in partial decode decodeList with enums

… now auto read these in as dependencies

yorinasub17 · 2019-08-14T23:20:11Z

Ok @brikis98 this is ready for a rereview. I have implemented terragrunt_output blocks as described in #828 (comment). I believe this addresses almost all of your concerns from the initial PR:

Predictable execution paths: it is now very clear when outputs are pulled into the config. See https://github.com/gruntwork-io/terragrunt/blob/38aca82ff0890af3a4584870b4bd5eed8f788311/README.md#configuration-parsing-order for more info.
Automatic dependency graph injection: since the parsing order is now predictable, we can seed the dependency graph with the configs specified by terragrunt_output blocks. See https://github.com/gruntwork-io/terragrunt/blob/38aca82ff0890af3a4584870b4bd5eed8f788311/README.md#passing-outputs-between-modules for more info.
Reuse results of a single terragrunt output call: with get_output, terragrunt output had to be called each time the function was executed to keep the implementation sane. Since get_output couldn't be parsed before locals, this made it difficult to reuse the output results of a single call if you needed multiple variables from the output. Now only one call needs to be made per block and it can be referenced multiple times.

brikis98

Wow, superb work. This ended up even more complicated to accomplish than I originally assume, but I think this is a great solution 👍

brikis98 · 2019-08-15T00:32:49Z

README.md

+
+terragrunt_output "redis" {
+  config_path = "../redis"
+}


This is very, very cool 👍

Now that I've seen this, I can't wonder if this should be:

dependency "mysql" { config_path = "../mysql" } dependency "redis" { config_path = "../redis" } inputs = { mysql_url = dependency.mysql.outputs.domain redis_url = dependency.redis.outputs.domain }

For this PR, the only difference would be a slight naming change: terragrunt_output becomes dependency and you read outputs from it via the .outputs attribute. However, in a future PR, we could allow you to read any part of that module's config! E.g., Perhaps dependency.mysql.inputs.foo would return the value of the "foo" output from the mysql dependency and dependency.redis.remote_state.config.bucket would return the bucket configured for remote state storage in the redis dependency.

I like this. I went through and made this change.

brikis98 · 2019-08-15T00:34:39Z

README.md

@@ -2126,6 +2206,46 @@ The `skip` flag must be set explicitly in terragrunt modules that should be skip
 `terragrunt.hcl` file that is included by another `terragrunt.hcl` file, only the `terragrunt.hcl` file that explicitly
 set `skip = true` will be skipped.

+
+### Configuration parsing order


Thanks for documenting this 👍

config/config.go

brikis98 · 2019-08-15T00:36:03Z

config/config.go

+//    Allowed References:
+//      - locals
+//      - terragrunt_output
+// 5. Merge the included config with the parsed config. Note that all the config data is mergable except for `locals`


Great docs 👍

brikis98 · 2019-08-15T00:43:03Z

util/logger.go

@@ -28,7 +29,7 @@ func CreateLoggerWithWriter(writer io.Writer, prefix string) *log.Logger {
 // logging solution.
 // Debugf will only print out terragrunt logs if the TG_LOG environment variable is set to DEBUG.
 func Debugf(logger *log.Logger, fmtString string, fmtArgs ...interface{}) {
-	if os.Getenv("TG_LOG") == "DEBUG" {
+	if strings.ToLower(os.Getenv("TG_LOG")) == "debug" {


Do we document this anywhere?

Added to README

…tputs"

Co-Authored-By: Yevgeniy Brikman <brikis98@users.noreply.github.com>

yorinasub17 · 2019-08-15T15:24:03Z

Ok merging and releasing this! Thanks for the review!

yorinasub17 added 8 commits August 9, 2019 08:54

Always run terratest log parser

e1b1bf6

Refactor -all commands to only partially parse the config

c8ae1f8

Fix tests from IsPartial refactor

30ab5b2

Regression test for partial execution

7b5b5c0

Add more unit testing for partial parsing of terragrunt config

ef872ef

Implement get_output helper function

e9ec6c9

Add docs on the function

74273a9

Add caveat on apply-all with get_output

4d7386e

yorinasub17 requested review from autero1, brikis98 and eak12913 as code owners August 9, 2019 20:05

yorinasub17 mentioned this pull request Aug 9, 2019

Would it be possible to add a get_output interpolation syntax function #418

Closed

yorinasub17 added 2 commits August 9, 2019 13:23

Return context specific error types with errors in get_output

39c3a29

Wrap error at end of get_output only if there was an error

2fbcd9b

brikis98 reviewed Aug 9, 2019

View reviewed changes

yorinasub17 added 3 commits August 9, 2019 15:59

- Add better documentation for partial parsing function

aa29cab

- Always read the entire output map so we have type data - Refactor json to cty.Value conversion into its own function with unit tests

Extract run terragrunt output json into its own function

321ca3b

Support relative file paths

74821d4

yorinasub17 mentioned this pull request Aug 12, 2019

get a output from one module into another #831

Closed

brikis98 reviewed Aug 12, 2019

View reviewed changes

yorinasub17 mentioned this pull request Aug 13, 2019

Feature Request: Shared and overridable variables (globals?) #814

Open

yorinasub17 added 2 commits August 14, 2019 08:10

- Add structs for parsing terragrunt_output block (no implementation …

ae82723

…that uses it yet) - Replace free strings in partial decode decodeList with enums

Reimplement get_output as terragrunt_output. As a side effect, we can…

548ace0

… now auto read these in as dependencies

yorinasub17 changed the title ~~Implement get_output~~ Implement terragrunt_output Aug 14, 2019

yorinasub17 added 2 commits August 14, 2019 16:08

Add clarification on configuration parsing order in -all commands

38aca82

Fix tests

bc70249

Fix typo in docs

d9f07a1

brikis98 approved these changes Aug 15, 2019

View reviewed changes

yorinasub17 and others added 4 commits August 14, 2019 19:40

Rename terragrunt_output to dependency and nest the outputs under "ou…

fca4ac2

…tputs"

Update config/config.go

8675507

Co-Authored-By: Yevgeniy Brikman <brikis98@users.noreply.github.com>

Add mention of TG_LOG

85c1acd

Code cleanup

8570095

yorinasub17 merged commit 8fa2f71 into master Aug 15, 2019

yorinasub17 deleted the yori-get-output branch August 15, 2019 15:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement terragrunt_output #828

Implement terragrunt_output #828

Implement terragrunt_output #828

Implement terragrunt_output #828

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment