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

backend/remote: Fix broken state lock retry #27845

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

alisdair
Copy link
Contributor

When using the -lock-timeout option with the remote backend configured in local operations mode, Terraform would fail to retry acquiring the lock. This was caused by the lock error message having a missing Info field, which the state manager requires to be present in order to attempt retries:

le, ok := err.(*LockError)
if !ok {
// not a lock error, so we can't retry
return "", err
}
if le == nil || le.Info == nil || le.Info.ID == "" {
// If we don't have a complete LockError then there's something
// wrong with the lock.
return "", err
}

Other remote state storage backends attempt to fetch lock information to populate this field. Because the Terraform Cloud API doesn't really have any lock information exposed, we can't do that here, so we just round-trip the lock info we were passed in to make sure there's something non-nil in the LockErr.

While I can't find anywhere to add test coverage for this, I've manually verified that this fixes the problem reported in #27844. Repro steps:

  1. Create a Terraform Cloud workspace in local operations mode
  2. Start an apply which takes several minutes (e.g. using time_sleep)
  3. In another session:
    1. Run terraform plan: verify that it fails immediately
    2. Run terraform plan -lock-timeout=10s: verify that it waits for 10s and then errors
    3. Run terraform plan -lock-timeout=300s: verify that it waits long enough for the apply to finish and then runs

Fixes #27844.

When using the -lock-timeout option with the remote backend configured
in local operations mode, Terraform would fail to retry acquiring the
lock. This was caused by the lock error message having a missing Info
field, which the state manager requires to be present in order to
attempt retries.
@alisdair alisdair requested review from chrisarcand, radditude and a team February 19, 2021 21:02
@alisdair alisdair self-assigned this Feb 19, 2021
@alisdair alisdair added the 0.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Feb 19, 2021
@codecov
Copy link

codecov bot commented Feb 19, 2021

Codecov Report

Merging #27845 (02eb283) into master (28d3505) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
command/format/diagnostic.go 69.42% <ø> (ø)
internal/getproviders/didyoumean.go 54.32% <ø> (ø)
internal/getproviders/filesystem_search.go 63.20% <ø> (ø)
internal/legacy/terraform/resource_address.go 62.31% <ø> (ø)
provider_source.go 24.73% <ø> (ø)
states/statefile/version3_upgrade.go 61.65% <ø> (ø)
backend/remote/backend_state.go 55.84% <100.00%> (+0.58%) ⬆️
backend/remote/backend_common.go 50.35% <0.00%> (-0.72%) ⬇️

Copy link
Member

@chrisarcand chrisarcand left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! 🙇‍♂️

Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

LGTM!

@alisdair alisdair merged commit 4ccd818 into master Feb 22, 2021
@alisdair alisdair deleted the alisdair/fix-remote-backend-state-lock-timeout branch February 22, 2021 16:48
@ghost
Copy link

ghost commented Mar 25, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged backend/remote
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TFC local execution doesn't seem to respect the -lock-timeout option
3 participants