-
Notifications
You must be signed in to change notification settings - Fork 3
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 ability to force unlock TF state and fix digest value of lock file in DynamoDB #235
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nb1701
force-pushed
the
nb1701/tf_lock
branch
3 times, most recently
from
August 23, 2023 15:22
a116810
to
be43ad9
Compare
dkatzz
approved these changes
Aug 23, 2023
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.
praise: Thanks for adding this!!
dkatzz
reviewed
Aug 23, 2023
bion
suggested changes
Aug 23, 2023
Sometimes, if a failure occurs or you've Ctrl+C'ed out of the middle of an apply, the Terraform state may be left in a locked state. When this happens, you must run 'terraform force-unlock' with the lock ID to unlock it. This change detects when an apply results in a lock error, and prompts the user to rerun the command with the --force-unlock flag. It does this by capturing the stderr stream of the 'terraform apply' command to inspect the message if an error occurs. An added bonus is that since we are using Popen to execute the command, we can handle things more gracefully if a user Ctrl+Cs out of the apply and allow Terraform to release the lock, reducing the amount of ways we can get into this failure state. In order to support the --force-unlock flag, there will be a change to the civiform-deploy repo as well.
If something goes wrong during deployment, especially when a user has force-unlocked due to a previous issue and then multiple apply actions are happening at once, the digest value for the Terraform lock file in S3 can be incorrect. This lets us set the digest value to the correct value, as given by the error message of a previous Terraform command, without having to go into the AWS console to set it manually.
This currently relies on the binaries being installed locally. We may want to move to using the civiform formatter container instead, or something like it, in the future.
If we hit a TF lock state issue, or the S3 digest issue, this allows the user to fix the problem during this particular invocation of the command, so they do not need to rerun the command. If we are not running in a TTY, we'll fall back to prompting the user to rerun the command with the appropriate flag.
… fix a problem This allows us not to have to change a CE's fork of civiform-deploy.
Sorry for re-reviewers who want to see changes since the last review, things got screwy and I had to force push a full set of new commits :( |
dkatzz
reviewed
Aug 30, 2023
bion
approved these changes
Sep 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometimes, if a failure occurs or you've Ctrl+C'ed out of the middle of an apply, the Terraform state may be left in a locked state. When this happens, you must run 'terraform force-unlock' with the lock ID to unlock it.
This change detects when an apply results in a lock error, and prompts the user to rerun the command with the --force-unlock flag. It does this by capturing the stderr stream of the 'terraform apply' command to inspect the message if an error occurs. An added bonus is that since we are using Popen to execute the command, we can handle things more gracefully if a user Ctrl+Cs out of the apply and allow Terraform to release the lock, reducing the amount of ways we can get into this failure state.
Secondly, if something goes wrong during deployment, especially when a user has force-unlocked due to a previous issue and then multiple apply actions are happening at once, the digest value for the Terraform lock file in S3 can be incorrect. This lets us set the digest value to the correct value, as given by the error message of a previous Terraform command, without having to go into the AWS console to set it manually.
Requires civiform/civiform-deploy#50 and addresses civiform/civiform#5381
This also adds a
bin/fmt
script to format files like the checks are expecting. It is not containerized and requires the binaries to be installed locally, so we probably want to containerize it later. But it works as a first pass.