From f2e496a14cdfd7eeb071f8231774cfea09f74f61 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 15 Feb 2017 11:53:19 -0500 Subject: [PATCH] Have backend operations properly unlock state Make sure unlock is called with the correct LockID during operations --- backend/local/backend_apply.go | 12 ++++++++++-- backend/local/backend_local.go | 10 ---------- backend/local/backend_plan.go | 12 ++++++++++-- backend/local/backend_refresh.go | 12 ++++++++++-- command/apply_destroy_test.go | 1 + command/command_test.go | 3 ++- command/testdata/statelocker.go | 2 +- command/unlock.go | 19 ++++++++++++++++--- command/unlock_test.go | 9 +++++++-- 9 files changed, 57 insertions(+), 23 deletions(-) diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index caffaa983..466bce468 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) @@ -35,10 +36,17 @@ func (b *Local) opApply( return } - // If we're locking state, unlock when we're done if op.LockState { + lockInfo := state.NewLockInfo() + lockInfo.Operation = op.Type.String() + lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize()) + if err != nil { + runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err) + return + } + defer func() { - if err := clistate.Unlock(opState, "", b.CLI, b.Colorize()); err != nil { + if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil { runningOp.Err = multierror.Append(runningOp.Err, err) } }() diff --git a/backend/local/backend_local.go b/backend/local/backend_local.go index e7a98b6e4..0ecba44ce 100644 --- a/backend/local/backend_local.go +++ b/backend/local/backend_local.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" - clistate "github.com/hashicorp/terraform/command/state" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) @@ -29,15 +28,6 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, state.State, return nil, nil, errwrap.Wrapf("Error loading state: {{err}}", err) } - if op.LockState { - lockInfo := state.NewLockInfo() - lockInfo.Operation = op.Type.String() - _, err := clistate.Lock(s, lockInfo, b.CLI, b.Colorize()) - if err != nil { - return nil, nil, errwrap.Wrapf("Error locking state: {{err}}", err) - } - } - if err := s.RefreshState(); err != nil { return nil, nil, errwrap.Wrapf("Error loading state: {{err}}", err) } diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index 70587cb36..afb483dad 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform/command/format" clistate "github.com/hashicorp/terraform/command/state" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) @@ -59,10 +60,17 @@ func (b *Local) opPlan( return } - // If we're locking state, unlock when we're done if op.LockState { + lockInfo := state.NewLockInfo() + lockInfo.Operation = op.Type.String() + lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize()) + if err != nil { + runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err) + return + } + defer func() { - if err := clistate.Unlock(opState, "", b.CLI, b.Colorize()); err != nil { + if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil { runningOp.Err = multierror.Append(runningOp.Err, err) } }() diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index 6a939d491..5c09597e8 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/state" ) func (b *Local) opRefresh( @@ -48,10 +49,17 @@ func (b *Local) opRefresh( return } - // If we're locking state, unlock when we're done if op.LockState { + lockInfo := state.NewLockInfo() + lockInfo.Operation = op.Type.String() + lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize()) + if err != nil { + runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err) + return + } + defer func() { - if err := clistate.Unlock(opState, "", b.CLI, b.Colorize()); err != nil { + if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil { runningOp.Err = multierror.Append(runningOp.Err, err) } }() diff --git a/command/apply_destroy_test.go b/command/apply_destroy_test.go index f8e37f8cb..e71072616 100644 --- a/command/apply_destroy_test.go +++ b/command/apply_destroy_test.go @@ -45,6 +45,7 @@ func TestApply_destroy(t *testing.T) { testFixturePath("apply"), } if code := c.Run(args); code != 0 { + t.Log(ui.OutputWriter.String()) t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } diff --git a/command/command_test.go b/command/command_test.go index abf134331..cb89f5e75 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -583,7 +583,8 @@ func testLockState(sourceDir, path string) (func(), error) { return deferFunc, fmt.Errorf("read from statelocker returned: %s", err) } - if string(buf[:n]) != "LOCKED" { + output := string(buf[:n]) + if !strings.HasPrefix(output, "LOCKID") { return deferFunc, fmt.Errorf("statelocker wrote: %s", string(buf[:n])) } return deferFunc, nil diff --git a/command/testdata/statelocker.go b/command/testdata/statelocker.go index 7bdce2112..e3a7f678f 100644 --- a/command/testdata/statelocker.go +++ b/command/testdata/statelocker.go @@ -34,7 +34,7 @@ func main() { } // signal to the caller that we're locked - io.WriteString(os.Stdout, "LOCKED") + io.WriteString(os.Stdout, "LOCKID "+lockID) defer func() { if err := s.Unlock(lockID); err != nil { diff --git a/command/unlock.go b/command/unlock.go index 952d9bacf..2e197e775 100644 --- a/command/unlock.go +++ b/command/unlock.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" + "github.com/mitchellh/cli" ) // UnlockCommand is a cli.Command implementation that manually unlocks @@ -25,9 +26,21 @@ func (c *UnlockCommand) Run(args []string) int { return 1 } + args = cmdFlags.Args() + if len(args) == 0 { + c.Ui.Error("unlock requires a lock id argument") + return cli.RunResultHelp + } + + lockID := args[0] + + if len(args) > 1 { + args = args[1:] + } + // assume everything is initialized. The user can manually init if this is // required. - configPath, err := ModulePath(cmdFlags.Args()) + configPath, err := ModulePath(args) if err != nil { c.Ui.Error(err.Error()) return 1 @@ -93,7 +106,7 @@ func (c *UnlockCommand) Run(args []string) int { } // FIXME: unlock should require the lock ID - if err := s.Unlock(""); err != nil { + if err := s.Unlock(lockID); err != nil { c.Ui.Error(fmt.Sprintf("Failed to unlock state: %s", err)) return 1 } @@ -104,7 +117,7 @@ func (c *UnlockCommand) Run(args []string) int { func (c *UnlockCommand) Help() string { helpText := ` -Usage: terraform force-unlock [DIR] +Usage: terraform force-unlock LOCK_ID [DIR] Manually unlock the state for the defined configuration. diff --git a/command/unlock_test.go b/command/unlock_test.go index 2496f3fe6..761701b2f 100644 --- a/command/unlock_test.go +++ b/command/unlock_test.go @@ -40,7 +40,12 @@ func TestUnlock(t *testing.T) { }, } - if code := c.Run([]string{"-force"}); code != 0 { - t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + args := []string{ + "-force", + "LOCK_ID", + } + + if code := c.Run(args); code != 1 { + t.Fatalf("bad: %d\n%s\n%s", code, ui.OutputWriter.String(), ui.ErrorWriter.String()) } }