-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Update to Terraform 0.15 #1651
Update to Terraform 0.15 #1651
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.
LGTM pending tests!
- Remove `TestTerragruntHonorsS3RemoteStateSkipFlagsRegression` because it relies on switching from local Terraform state to remote state, but with Terraform 0.15, that requires an interactive prompt (asking if you want to move your state) that does not work well in an automated test. - Fix `TestTerragruntSetsAccessLoggingForTfSTateS3BuckeToADifferentBucketWithGiven` and `TestTerragruntSetsAccessLoggingForTfSTateS3BuckeToADifferentBucketWithDefau` to not rely on the state moving aspects. Those don't seem relevant for those test cases and were probably just copy/pasted in.
{[]string{"terragrunt", "plan", "--help"}, "Usage: terraform .* plan"}, | ||
{[]string{"terragrunt", "apply", "-help"}, "Usage: terraform .* apply"}, | ||
{[]string{"terragrunt", "apply", "-h"}, "Usage: terraform .* apply"}, |
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.
Terraform's help text is a bit different now, so had to update this test accordingly.
// This bug only happens if terragrunt calls remotestate.NeedsInitialization on the s3 initializer, so we need to setup | ||
// a situation where terragrunt calls this. To do this, we first setup the module with local backend (so that | ||
// moduleNeedsInit is false), before switching to the remote backend. | ||
func TestTerragruntHonorsS3RemoteStateSkipFlagsRegression(t *testing.T) { |
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 test runs Terragrunt first with a local backend and then with an S3 backend. Unfortunately, with TF 0.15, when you switch to the S3 backend, it shows an interactive prompt asking if you want to migrate state over from the local backend. This interactive prompt now causes the test to fail. I tried switching the second Terragrunt run from validate
to apply -input=false
, but the interactive prompt still came up.
Not sure how this worked before 0.15, but as per @yorinasub17's comment here, this test doesn't quite test what we are looking for anyway, and give the complexity of working around an interactive prompt, I opted to remove it.
|
||
// Pass 1 with local backend. This should only download and setup the terragrunt-cache. Note that the first pass has | ||
// to be an apply so that the state file is created. Otherwise, terragrunt short circuits the problematic routine. | ||
runTerragrunt(t, fmt.Sprintf("terragrunt apply -auto-approve --terragrunt-non-interactive --terragrunt-config %s --terragrunt-working-dir %s", localBackendTerragruntConfigPath, examplePath)) |
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 test case and the one below look like the copy/pasted the logic from TestTerragruntHonorsS3RemoteStateSkipFlagsRegression
around running first with a local backend and then with an S3 backend, so they hit the same interactive prompt. However, I don't think these two test cases needed this two-step process, so I updated them to only run with the S3 backend, which seems to work.
Woot, tests are passing! |
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.
LGTM
Thanks for the review! Merging now. |
@brikis98 seems like the binaries should have dropped by now... did the CI fail when building them again? |
Gah, yea, there's a timing issue in one test that fails now and then. Just re-ran the build, binaries should show up shortly. Thanks for the heads up! |
This PR updates the Terragrunt build to run with Terraform 0.15.