Revert "backend/local: release lock if there is an error in Context() (#25427)"

This reverts commit 1ba0d615e7.
This commit is contained in:
Kristin Laemmert 2020-06-30 14:05:29 -04:00
parent 6b8d389d57
commit 604e65bb62
4 changed files with 6 additions and 94 deletions

View File

@ -49,18 +49,6 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, *configload.
diags = diags.Append(errwrap.Wrapf("Error locking state: {{err}}", err)) diags = diags.Append(errwrap.Wrapf("Error locking state: {{err}}", err))
return nil, nil, nil, diags return nil, nil, nil, diags
} }
defer func() {
// If we're returning with errors, and thus not producing a valid
// context, we'll want to avoid leaving the workspace locked.
if diags.HasErrors() {
err := op.StateLocker.Unlock(nil)
if err != nil {
diags = diags.Append(errwrap.Wrapf("Error unlocking state: {{err}}", err))
}
}
}()
log.Printf("[TRACE] backend/local: reading remote state for workspace %q", op.Workspace) log.Printf("[TRACE] backend/local: reading remote state for workspace %q", op.Workspace)
if err := s.RefreshState(); err != nil { if err := s.RefreshState(); err != nil {
diags = diags.Append(errwrap.Wrapf("Error loading state: {{err}}", err)) diags = diags.Append(errwrap.Wrapf("Error loading state: {{err}}", err))

View File

@ -1,64 +0,0 @@
package local
import (
"testing"
"github.com/hashicorp/terraform/backend"
"github.com/hashicorp/terraform/internal/initwd"
"github.com/hashicorp/terraform/states/statemgr"
)
func TestLocalContext(t *testing.T) {
configDir := "./testdata/empty"
b, cleanup := TestLocal(t)
defer cleanup()
_, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir)
defer configCleanup()
op := &backend.Operation{
ConfigDir: configDir,
ConfigLoader: configLoader,
Workspace: backend.DefaultStateName,
LockState: true,
}
_, _, diags := b.Context(op)
if diags.HasErrors() {
t.Fatalf("unexpected error: %s", diags.Err().Error())
}
// Conext() retains a lock on success, so this should fail.
stateMgr, _ := b.StateMgr(backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err == nil {
t.Fatalf("unexpected success locking state")
}
}
func TestLocalContext_error(t *testing.T) {
configDir := "./testdata/apply-error"
b, cleanup := TestLocal(t)
defer cleanup()
_, configLoader, configCleanup := initwd.MustLoadConfigForTests(t, configDir)
defer configCleanup()
op := &backend.Operation{
ConfigDir: configDir,
ConfigLoader: configLoader,
Workspace: backend.DefaultStateName,
LockState: true,
}
_, _, diags := b.Context(op)
if !diags.HasErrors() {
t.Fatal("unexpected success")
}
// When Context() returns an error, it also unlocks the state.
// This should therefore succeed.
stateMgr, _ := b.StateMgr(backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state: %s", err.Error())
}
}

View File

@ -8,7 +8,6 @@ import (
"github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/backend"
"github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs"
"github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/internal/initwd"
"github.com/hashicorp/terraform/states/statemgr"
"github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty"
) )
@ -187,7 +186,6 @@ func TestRemoteContextWithVars(t *testing.T) {
ConfigDir: configDir, ConfigDir: configDir,
ConfigLoader: configLoader, ConfigLoader: configLoader,
Workspace: backend.DefaultStateName, Workspace: backend.DefaultStateName,
LockState: true,
} }
v := test.Opts v := test.Opts
@ -207,21 +205,10 @@ func TestRemoteContextWithVars(t *testing.T) {
if errStr != test.WantError { if errStr != test.WantError {
t.Fatalf("wrong error\ngot: %s\nwant: %s", errStr, test.WantError) t.Fatalf("wrong error\ngot: %s\nwant: %s", errStr, test.WantError)
} }
// When Context() returns an error, it should unlock the state,
// so re-locking it is expected to succeed.
stateMgr, _ := b.StateMgr(backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err != nil {
t.Fatalf("unexpected error locking state: %s", err.Error())
}
} else { } else {
if diags.HasErrors() { if diags.HasErrors() {
t.Fatalf("unexpected error\ngot: %s\nwant: <no error>", diags.Err().Error()) t.Fatalf("unexpected error\ngot: %s\nwant: <no error>", diags.Err().Error())
} }
// If Context() succeeded, this should fail w/ "workspace already locked"
stateMgr, _ := b.StateMgr(backend.DefaultStateName)
if _, err := stateMgr.Lock(statemgr.NewLockInfo()); err == nil {
t.Fatal("unexpected success locking state after Context")
}
} }
}) })
} }

View File

@ -92,11 +92,6 @@ func (c *ConsoleCommand) Run(args []string) int {
// Get the context // Get the context
ctx, _, ctxDiags := local.Context(opReq) ctx, _, ctxDiags := local.Context(opReq)
diags = diags.Append(ctxDiags)
if ctxDiags.HasErrors() {
c.showDiagnostics(diags)
return 1
}
// Creating the context can result in a lock, so ensure we release it // Creating the context can result in a lock, so ensure we release it
defer func() { defer func() {
@ -106,6 +101,12 @@ func (c *ConsoleCommand) Run(args []string) int {
} }
}() }()
diags = diags.Append(ctxDiags)
if ctxDiags.HasErrors() {
c.showDiagnostics(diags)
return 1
}
// Setup the UI so we can output directly to stdout // Setup the UI so we can output directly to stdout
ui := &cli.BasicUi{ ui := &cli.BasicUi{
Writer: wrappedstreams.Stdout(), Writer: wrappedstreams.Stdout(),