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

Make sure Terragrunt commands can be configured with -lock=true #788

Merged
merged 1 commit into from
Mar 22, 2021

Conversation

dz-sourced
Copy link
Contributor

This fixes #785

In the current implementation, when using Terragrunt binary instead of Terraform, Terratest explicitly passes -lock=false to the CLI.
This triggers a bug in Terraform (see issue for details on that), but also doesn't allow us to run integration tests with remote state
backend configured in the same way we would run actual deployment since Terraform uses -lock=true by default.

This change allows Options.Lock to be passed down to the Terragrunt apply-all and destroy-all commands.

Default behavior in Terratest does not change, since Options.Lock is still initialized to false, but at least we can control it 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.

Sanity check: it doesn't look like you changed anything in RunTerraformCommandE... So is the fix that -lock=false is no longer hard-coded and is now read from options instead? Or is the fix that apply-all is part of TerraformCommandsWithLockSupport now?

@dz-sourced
Copy link
Contributor Author

@brikis98 Yes, the fix is that -lock=false is no longer hardcoded and can be supplied via options, but for this to work with Terragrunt apply-all and destroy-all commands need to be added to the TerraformCommandsWithLockSupport because of this check here

lockSupported := collections.ListContains(TerraformCommandsWithLockSupport, commandType)

brikis98
brikis98 previously approved these changes Mar 1, 2021
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.

Ah, got it, thanks for explaining! Changes LGTM. I'll kick off tests now.

@dz-sourced
Copy link
Contributor Author

@brikis98 I see that a couple of tests failed, but they don't seem to be related to my PR. Is there anything I need to do on my end to sort this out?

@brikis98
Copy link
Member

We just merged some test stability fixes. Could you pull in the latest from master and I'll re-run tests?

@dz-sourced
Copy link
Contributor Author

@brikis98 I have merged the latest from the upstream master. Should be good to go for another tests run.

@brikis98
Copy link
Member

Thanks! I just kicked off the tests again.

@brikis98
Copy link
Member

There was one test failure, but it looks unrelated/transient, so I'm going to merge.

@brikis98 brikis98 merged commit 58567bc into gruntwork-io:master Mar 22, 2021
@brikis98
Copy link
Member

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.

When using terragrunt binary state locking is explicitly disabled
2 participants