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

cli: Restore -lock and -lock-timeout init flags #29773

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

alisdair
Copy link
Contributor

The -lock and -lock-timeout flags were removed in #27464 prior to the release of 1.0 as they were thought to have no effect. This is not true in the case of state migrations when changing backends. This commit restores these flags, and adds test coverage for locking during backend state migration.

Documentation does not need to be updated, as it currently shows that the flags are present. I intend to backport this change to 1.0 because I believe this removal was a bug.

Fixes #29765.

@alisdair alisdair added cli 1.0-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged labels Oct 18, 2021
@alisdair alisdair requested a review from a team October 18, 2021 18:55
@alisdair alisdair self-assigned this Oct 18, 2021
Comment on lines 987 to 992
-lock=true Lock the state file when locking is supported.

Copy link
Contributor

@apparentlymart apparentlymart Oct 18, 2021

Choose a reason for hiding this comment

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

While updating the usages for these boolean options we've gradually been trying to update them to show the argument as the user would type it rather than the default behavior, because historically folks got confused by us only showing a usage that changes nothing.

In this case I think -lock is already the default (please correct me if I'm wrong!), so I'd prefer to write this as something like:

  -lock=false         Don't hold a state lock during the operation. This is
                      dangerous if others might concurrently run commands
                      against the same workspace.

(I took the above from the terraform plan -help, which we updated as part of the CLI consistency work we did in the run-up to v1.0.)

It seems like I neglected to update terraform init -help in general during that work, for whatever reason, since I see various other ones here that seem to just re-state the default, rather than showing how to disable the thing that's on by default:

  • -backend=true
  • -get=true
  • -input=true

I'm not sure why I didn't update these along with the others in that earlier work, but if you'd be willing to do some "while in the area" updates here it'd be nice to get these all lined up with the other usages. Other than -input I don't think the rest of these are present in any other commands so there isn't a convenient existing wording to copy from somewhere else, but of course the general idea here is to just reword it to be a "don't" or "disable" sentence instead of a "do" or "enable" sentence, in each case.

I also notice -upgrade=false here, which I think ought to just be -upgrade and talk about overriding the default behavior of selecting exactly the version recorded in the lock file, if any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks! I've updated the help output for these arguments.

The -lock and -lock-timeout flags were removed prior to the release of
1.0 as they were thought to have no effect. This is not true in the case
of state migrations when changing backends. This commit restores these
flags, and adds test coverage for locking during backend state
migration.

Also update the help output describing other boolean flags, showing the
argument as the user would type it rather than the default behavior.
@jspiro
Copy link

jspiro commented Nov 2, 2021

Thank you!!!!!

@github-actions
Copy link

github-actions bot commented Dec 2, 2021

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.0-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged cli
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terraform init locks the state and does not have the -lock option anymore
3 participants