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

A helper function to run arbitrary commands #670

Merged
merged 3 commits into from
Mar 8, 2019
Merged

A helper function to run arbitrary commands #670

merged 3 commits into from
Mar 8, 2019

Conversation

bernardoVale
Copy link
Contributor

@bernardoVale bernardoVale commented Mar 3, 2019

In my use case, we'll have multiple live-infrastructure repos. One particular drawback of such implementation is that you can end up with a lack of pattern.

To cope with that, I wanted to implement generic terraform.tfvars that can be easily copy-pasted, everywhere and upgraded to a new version when needed. Since there's information that changes from account to account and repo per repo, like the bucket name and the dynamoDB table I plan to write a script that determines such information based on the path and the repo name.

This PR provides this functionality with a simple helper function that runs any command with a list of arguments and returns the stdout as the result.

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.

This is great, thank you! Just a couple fixes and this should be good to go.

README.md Outdated
`run_cmd(command, arg1, arg2...)` runs a shell command and returns the stdout as the result of the interpolation.

This is useful when you want to have a generic `terraform.tfvars` for all accounts and use a script to determine
information that changes from account to account, like the bucket name and the DynamoDB table:
Copy link
Member

Choose a reason for hiding this comment

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

I think this is more powerful than just that! It allows you to write scripts to dynamically fill in arbitrary information in your Terragrunt configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

take a look now

return "", errors.WithStackTrace(EmptyStringNotAllowed("parameter to the run_cmd function"))
}

currentPath := filepath.Dir(terragruntOptions.TerragruntConfigPath)
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a terragruntOptions.WorkingDir

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea as that we run the command from the module folder because the command might need the path as the context. A good example (my actual use-case) is if you want to write a script that returns the bucket name based on the account (folder) where you are running the command.

Using the WorkingDir wouldn't work when you run *-all between accounts using --terragrunt-include-dir

Copy link
Member

Choose a reason for hiding this comment

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

OK, fair enough. Please make sure to document that in the README!

config/config_helpers.go Outdated Show resolved Hide resolved
Args: args,
Dir: currentPath,
}
stdout, err := command.Output()
Copy link
Member

Choose a reason for hiding this comment

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

Hm, we should probably both capture the output and stream it to console so users can see what happened when running their scripts... There's a function that does that already in Terragrunt: https://github.com/gruntwork-io/terragrunt/blob/master/shell/run_shell_cmd.go#L39

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't use this function since it currently forces us to use terragruntOptions.WorkingDir. There are two options:

A - We change the function to accept a custom workingDir.
B - We don't use it and just posts the stdout to the logs

I'm leaning towards B. I think that most use cases for run_cmd will be running small shell scripts that take less than a second to execute, so I think that streaming the logs is overkill, we can simply add the stdout to the logs once the function finishes its execution.

thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I would vote for A. Should be a minor change and we improve the user experience.

I can see this feature being used by teams to run terragrunt output xxx in some other folder to get outputs from other modules. Having streaming logs for that sort of thing is quite handy.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, A it is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

take a look now

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.

Wonderful, thank you! Merging now and will let tests run. If they pass, I'll issue a new release.

@brikis98 brikis98 merged commit 31d47cf into gruntwork-io:master Mar 8, 2019
@brikis98
Copy link
Member

brikis98 commented Mar 8, 2019

Hm, got a test failure:

--- FAIL: TestCommandOutputOrder (0.00s)
	Error Trace:	run_shell_cmd_output_test.go:31
	Error:      	Expected value not to be nil.
	Messages:   	Should get output

Any idea what that is?

@bernardoVale
Copy link
Contributor Author

weird this test is passing here.

@bernardoVale
Copy link
Contributor Author

@brikis98 found the error. I forgot to add the emtpyworkingDir param in one of the tests. The binary wasn't breaking since the last argument accepts a list or arguments :P

@bernardoVale
Copy link
Contributor Author

check #673

brikis98 added a commit that referenced this pull request Mar 8, 2019
@ansgarm
Copy link

ansgarm commented Mar 14, 2019

Is anything blocking a release? I could really make use of this new feature 👍

@brikis98
Copy link
Member

This is now released in https://github.com/gruntwork-io/terragrunt/releases/tag/v0.18.2! Thx @bernardoVale!

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.

3 participants