-
-
Notifications
You must be signed in to change notification settings - Fork 979
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
Add a --terragrunt-source-update flag #117
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left minor comments only. Feel free to merge when ready.
cli/args.go
Outdated
@@ -69,6 +71,7 @@ func parseTerragruntOptionsFromArgs(args []string) (*options.TerragruntOptions, | |||
Logger: util.CreateLogger(""), | |||
RunTerragrunt: runTerragrunt, | |||
Source: terraformSource, | |||
UpdateSource: updateSource, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the preferred phrasing "updateSource" or "sourceUpdate"? It seems to be used in both ways in different places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed everywhere to sourceUpdate
for consistency: 1bf8ed3
cli/cli_app.go
Outdated
@@ -56,6 +57,7 @@ GLOBAL OPTIONS: | |||
terragrunt-non-interactive Assume "yes" for all prompts. | |||
terragrunt-working-dir The path to the Terraform templates. Default is current directory. | |||
terragrunt-source Download Terraform configurations from the specified source into a temporary folder, and run Terraform in that temporary folder | |||
terragrunt-source-update Delete the contents of the temporary folder before downloading source code into it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this perfectly describes what's happening, the user still has to make the leap "...and that's valuable because the temporary folder acts as a cache so this is a way of busting the cache." Consider writing what this is doing more so than how it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated description to: "Delete the contents of the temporary folder to clear out any old, cached source code before downloading new source code into it"
cli/download_source_test.go
Outdated
terragruntOptions := options.NewTerragruntOptionsForTest("./should-not-be-used") | ||
terragruntOptions.UpdateSource = updateSource | ||
|
||
err := downloadTerraformSourceIfNecessary(terraformSource, terragruntOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a little hard for me to following how this test was working. Looks like you're just testing that terragruntOptions.UpdateSource == true
allows it to run successfully. Thinking it through I don't think there's a great way to test that this works except possibly reading file timestamps in the temp folder before and after, but that's a little cumbersome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a helper function that is used by a bunch of actual test cases up above. The TestDownloadTerraformSourceIfNecessaryRemoteUrlToAlreadyDownloadedDirSameVersion
and TestDownloadTerraformSourceIfNecessaryRemoteUrlOverrideSource
test cases both set the source
URL to the same value and both run against folders that contain downloaded code of exactly the same version hash as the source
URL. The difference is that the first test case doesn't set UpdateSource
, so it verifies that nothing is downloaded, while the second test case does set UpdateSource
, so it verifies that the new code is downloaded from the source
URL.
Merging now. |
In #114, I added the ability for Terraform to cache the temporary folder so we don’t have to redownload code unnecessarily. This PR adds a new flag that I (naively) hoped wouldn’t be necessary:
—terragrunt-source-update
. This flag is a bit like the-update
flag in terraform. It deletes the temporary folder before downloading into it.