diff --git a/backend/backend.go b/backend/backend.go index f67d60ab60..dfeb80ef6f 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -9,6 +9,7 @@ import ( "errors" "time" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" @@ -135,6 +136,10 @@ type Operation struct { // state.Lockers for its duration, and Unlock when complete. LockState bool + // StateLocker is used to lock the state while providing UI feedback to the + // user. This will be supplied by the Backend itself. + StateLocker clistate.Locker + // The duration to retry obtaining a State lock. StateLockTimeout time.Duration diff --git a/backend/local/backend.go b/backend/local/backend.go index b42aedb17c..eb4eff4b00 100644 --- a/backend/local/backend.go +++ b/backend/local/backend.go @@ -13,6 +13,7 @@ import ( "sync" "github.com/hashicorp/terraform/backend" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" @@ -260,12 +261,24 @@ func (b *Local) Operation(ctx context.Context, op *backend.Operation) (*backend. cancelCtx, cancel := context.WithCancel(context.Background()) runningOp.Cancel = cancel + if op.LockState { + op.StateLocker = clistate.NewLocker(stopCtx, op.StateLockTimeout, b.CLI, b.Colorize()) + } else { + op.StateLocker = clistate.NewNoopLocker() + } + // Do it go func() { defer done() defer stop() defer cancel() + // the state was locked during context creation, unlock the state when + // the operation completes + defer func() { + runningOp.Err = op.StateLocker.Unlock(runningOp.Err) + }() + defer b.opLock.Unlock() f(stopCtx, cancelCtx, op, runningOp) }() diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index da8c684bc3..102d40c39e 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" - "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/state" @@ -55,25 +54,6 @@ func (b *Local) opApply( return } - if op.LockState { - lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout) - defer cancel() - - lockInfo := state.NewLockInfo() - lockInfo.Operation = op.Type.String() - lockID, err := clistate.Lock(lockCtx, 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, lockID, b.CLI, b.Colorize()); err != nil { - runningOp.Err = multierror.Append(runningOp.Err, err) - } - }() - } - // Setup the state runningOp.State = tfCtx.State() diff --git a/backend/local/backend_local.go b/backend/local/backend_local.go index aa056a1a16..d26fefa57d 100644 --- a/backend/local/backend_local.go +++ b/backend/local/backend_local.go @@ -1,9 +1,11 @@ package local import ( + "context" "errors" "log" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/tfdiags" @@ -20,6 +22,12 @@ func (b *Local) Context(op *backend.Operation) (*terraform.Context, state.State, // to ask for input/validate. op.Type = backend.OperationTypeInvalid + if op.LockState { + op.StateLocker = clistate.NewLocker(context.Background(), op.StateLockTimeout, b.CLI, b.Colorize()) + } else { + op.StateLocker = clistate.NewNoopLocker() + } + return b.context(op) } @@ -30,6 +38,10 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, state.State, return nil, nil, errwrap.Wrapf("Error loading state: {{err}}", err) } + if err := op.StateLocker.Lock(s, op.Type.String()); 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 ebf0531922..00c0a859d5 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -9,12 +9,9 @@ import ( "strings" "github.com/hashicorp/errwrap" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" - "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/command/format" "github.com/hashicorp/terraform/config/module" - "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) @@ -62,25 +59,6 @@ func (b *Local) opPlan( return } - if op.LockState { - lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout) - defer cancel() - - lockInfo := state.NewLockInfo() - lockInfo.Operation = op.Type.String() - lockID, err := clistate.Lock(lockCtx, 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, lockID, b.CLI, b.Colorize()); err != nil { - runningOp.Err = multierror.Append(runningOp.Err, err) - } - }() - } - // Setup the state runningOp.State = tfCtx.State() diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index 959f969a32..b5ec9aa6d4 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -8,11 +8,8 @@ import ( "strings" "github.com/hashicorp/errwrap" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" - "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/config/module" - "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) @@ -53,25 +50,6 @@ func (b *Local) opRefresh( return } - if op.LockState { - lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout) - defer cancel() - - lockInfo := state.NewLockInfo() - lockInfo.Operation = op.Type.String() - lockID, err := clistate.Lock(lockCtx, 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, lockID, b.CLI, b.Colorize()); err != nil { - runningOp.Err = multierror.Append(runningOp.Err, err) - } - }() - } - // Set our state runningOp.State = opState.State() if runningOp.State.Empty() || !runningOp.State.HasResources() { diff --git a/command/clistate/state.go b/command/clistate/state.go index f02f053be9..a17f96c342 100644 --- a/command/clistate/state.go +++ b/command/clistate/state.go @@ -8,9 +8,11 @@ import ( "context" "fmt" "strings" + "sync" "time" "github.com/hashicorp/errwrap" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/helper/slowmessage" "github.com/hashicorp/terraform/state" "github.com/mitchellh/cli" @@ -48,47 +50,125 @@ that no one else is holding a lock. ` ) -// Lock locks the given state and outputs to the user if locking -// is taking longer than the threshold. The lock is retried until the context -// is cancelled. -func Lock(ctx context.Context, s state.State, info *state.LockInfo, ui cli.Ui, color *colorstring.Colorize) (string, error) { - var lockID string +// Locker allows for more convenient usage of the lower-level state.Locker +// implementations. +// The state.Locker API requires passing in a state.LockInfo struct. Locker +// implementations are expected to create the required LockInfo struct when +// Lock is called, populate the Operation field with the "reason" string +// provided, and pass that on to the underlying state.Locker. +// Locker implementations are also expected to store any state required to call +// Unlock, which is at a minimum the LockID string returned by the +// state.Locker. +type Locker interface { + // Lock the provided state, storing the reason string in the LockInfo. + Lock(s state.State, reason string) error + // Unlock the previously locked state. + // An optional error can be passed in, and will be combined with any error + // from the Unlock operation. + Unlock(error) error +} + +type locker struct { + mu sync.Mutex + ctx context.Context + timeout time.Duration + state state.State + ui cli.Ui + color *colorstring.Colorize + lockID string +} + +// Create a new Locker. +// This Locker uses state.LockWithContext to retry the lock until the provided +// timeout is reached, or the context is canceled. Lock progress will be be +// reported to the user through the provided UI. +func NewLocker( + ctx context.Context, + timeout time.Duration, + ui cli.Ui, + color *colorstring.Colorize) Locker { + + l := &locker{ + ctx: ctx, + timeout: timeout, + ui: ui, + color: color, + } + return l +} + +// Locker locks the given state and outputs to the user if locking is taking +// longer than the threshold. The lock is retried until the context is +// cancelled. +func (l *locker) Lock(s state.State, reason string) error { + l.mu.Lock() + defer l.mu.Unlock() + + l.state = s + + ctx, cancel := context.WithTimeout(l.ctx, l.timeout) + defer cancel() + + lockInfo := state.NewLockInfo() + lockInfo.Operation = reason err := slowmessage.Do(LockThreshold, func() error { - id, err := state.LockWithContext(ctx, s, info) - lockID = id + id, err := state.LockWithContext(ctx, s, lockInfo) + l.lockID = id return err }, func() { - if ui != nil { - ui.Output(color.Color(LockMessage)) + if l.ui != nil { + l.ui.Output(l.color.Color(LockMessage)) } }) if err != nil { - err = errwrap.Wrapf(strings.TrimSpace(LockErrorMessage), err) + return errwrap.Wrapf(strings.TrimSpace(LockErrorMessage), err) } - return lockID, err + return nil } -// Unlock unlocks the given state and outputs to the user if the -// unlock fails what can be done. -func Unlock(s state.State, id string, ui cli.Ui, color *colorstring.Colorize) error { +func (l *locker) Unlock(parentErr error) error { + l.mu.Lock() + defer l.mu.Unlock() + + if l.lockID == "" { + return parentErr + } + err := slowmessage.Do(LockThreshold, func() error { - return s.Unlock(id) + return l.state.Unlock(l.lockID) }, func() { - if ui != nil { - ui.Output(color.Color(UnlockMessage)) + if l.ui != nil { + l.ui.Output(l.color.Color(UnlockMessage)) } }) if err != nil { - ui.Output(color.Color(fmt.Sprintf( + l.ui.Output(l.color.Color(fmt.Sprintf( "\n"+strings.TrimSpace(UnlockErrorMessage)+"\n", err))) - err = fmt.Errorf( - "Error releasing the state lock. Please see the longer error message above.") + if parentErr != nil { + parentErr = multierror.Append(parentErr, err) + } } + return parentErr + +} + +type noopLocker struct{} + +// NewNoopLocker returns a valid Locker that does nothing. +func NewNoopLocker() Locker { + return noopLocker{} +} + +func (l noopLocker) Lock(state.State, string) error { + return nil +} + +func (l noopLocker) Unlock(err error) error { return err } diff --git a/command/meta_backend.go b/command/meta_backend.go index 2fa4ff542e..1bee95339c 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -583,18 +583,11 @@ func (m *Meta) backendFromPlan(opts *BackendOpts) (backend.Backend, error) { } if m.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) - defer cancel() - - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from plan" - - lockID, err := clistate.Lock(lockCtx, realMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(realMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer clistate.Unlock(realMgr, lockID, m.Ui, m.Colorize()) + defer stateLocker.Unlock(nil) } if err := realMgr.RefreshState(); err != nil { @@ -964,18 +957,11 @@ func (m *Meta) backend_C_r_s( } if m.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) - defer cancel() - - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from config" - - lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) + defer stateLocker.Unlock(nil) } // Store the metadata in our saved state location @@ -1047,18 +1033,11 @@ func (m *Meta) backend_C_r_S_changed( } if m.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) - defer cancel() - - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from config" - - lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) + defer stateLocker.Unlock(nil) } // Update the backend state @@ -1190,18 +1169,11 @@ func (m *Meta) backend_C_R_S_unchanged( } if m.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) - defer cancel() - - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from config" - - lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) + defer stateLocker.Unlock(nil) } // Unset the remote state diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index 7f60a6605d..0c3610a8a1 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -233,28 +233,19 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { } if m.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) - defer cancel() + lockCtx := context.Background() - lockInfoOne := state.NewLockInfo() - lockInfoOne.Operation = "migration" - lockInfoOne.Info = "source state" - - lockIDOne, err := clistate.Lock(lockCtx, stateOne, lockInfoOne, m.Ui, m.Colorize()) - if err != nil { + lockerOne := clistate.NewLocker(lockCtx, m.stateLockTimeout, m.Ui, m.Colorize()) + if err := lockerOne.Lock(stateOne, "migration source state"); err != nil { return fmt.Errorf("Error locking source state: %s", err) } - defer clistate.Unlock(stateOne, lockIDOne, m.Ui, m.Colorize()) + defer lockerOne.Unlock(nil) - lockInfoTwo := state.NewLockInfo() - lockInfoTwo.Operation = "migration" - lockInfoTwo.Info = "destination state" - - lockIDTwo, err := clistate.Lock(lockCtx, stateTwo, lockInfoTwo, m.Ui, m.Colorize()) - if err != nil { + lockerTwo := clistate.NewLocker(lockCtx, m.stateLockTimeout, m.Ui, m.Colorize()) + if err := lockerTwo.Lock(stateTwo, "migration destination state"); err != nil { return fmt.Errorf("Error locking destination state: %s", err) } - defer clistate.Unlock(stateTwo, lockIDTwo, m.Ui, m.Colorize()) + defer lockerTwo.Unlock(nil) // We now own a lock, so double check that we have the version // corresponding to the lock. diff --git a/command/taint.go b/command/taint.go index 626f88a46f..4dc16e60cd 100644 --- a/command/taint.go +++ b/command/taint.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/hashicorp/terraform/command/clistate" - "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) @@ -84,18 +83,12 @@ func (c *TaintCommand) Run(args []string) int { } if c.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) - defer cancel() - - lockInfo := state.NewLockInfo() - lockInfo.Operation = "taint" - lockID, err := clistate.Lock(lockCtx, st, lockInfo, c.Ui, c.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) + if err := stateLocker.Lock(st, "taint"); err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - - defer clistate.Unlock(st, lockID, c.Ui, c.Colorize()) + defer stateLocker.Unlock(nil) } // Get the actual state structure diff --git a/command/untaint.go b/command/untaint.go index 1eca202779..39d047fd67 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/hashicorp/terraform/command/clistate" - "github.com/hashicorp/terraform/state" ) // UntaintCommand is a cli.Command implementation that manually untaints @@ -72,18 +71,12 @@ func (c *UntaintCommand) Run(args []string) int { } if c.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) - defer cancel() - - lockInfo := state.NewLockInfo() - lockInfo.Operation = "untaint" - lockID, err := clistate.Lock(lockCtx, st, lockInfo, c.Ui, c.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) + if err := stateLocker.Lock(st, "untaint"); err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - - defer clistate.Unlock(st, lockID, c.Ui, c.Colorize()) + defer stateLocker.Unlock(nil) } // Get the actual state structure diff --git a/command/workspace_delete.go b/command/workspace_delete.go index 99dac86d3b..1e1eb1182f 100644 --- a/command/workspace_delete.go +++ b/command/workspace_delete.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/hashicorp/terraform/command/clistate" - "github.com/hashicorp/terraform/state" "github.com/mitchellh/cli" "github.com/posener/complete" ) @@ -97,6 +96,17 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { return 1 } + var stateLocker clistate.Locker + if c.stateLock { + stateLocker = clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) + if err := stateLocker.Lock(sMgr, "workspace_delete"); err != nil { + c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) + return 1 + } + } else { + stateLocker = clistate.NewNoopLocker() + } + if err := sMgr.RefreshState(); err != nil { c.Ui.Error(err.Error()) return 1 @@ -109,31 +119,16 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { return 1 } - // Honor the lock request, for consistency and one final safety check. - if c.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) - defer cancel() - - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "workspace delete" - lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, c.Ui, c.Colorize()) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) - return 1 - } - - // We need to release the lock just before deleting the state, in case - // the backend can't remove the resource while holding the lock. This - // is currently true for Windows local files. - // - // TODO: While there is little safety in locking while deleting the - // state, it might be nice to be able to coordinate processes around - // state deletion, i.e. in a CI environment. Adding Delete() as a - // required method of States would allow the removal of the resource to - // be delegated from the Backend to the State itself. - clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize()) - } + // We need to release the lock just before deleting the state, in case + // the backend can't remove the resource while holding the lock. This + // is currently true for Windows local files. + // + // TODO: While there is little safety in locking while deleting the + // state, it might be nice to be able to coordinate processes around + // state deletion, i.e. in a CI environment. Adding Delete() as a + // required method of States would allow the removal of the resource to + // be delegated from the Backend to the State itself. + stateLocker.Unlock(nil) err = b.DeleteState(delEnv) if err != nil { diff --git a/command/workspace_new.go b/command/workspace_new.go index 71c9fdc1fc..810a776fc4 100644 --- a/command/workspace_new.go +++ b/command/workspace_new.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/hashicorp/terraform/command/clistate" - "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/cli" "github.com/posener/complete" @@ -115,18 +114,12 @@ func (c *WorkspaceNewCommand) Run(args []string) int { } if c.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) - defer cancel() - - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "workspace new" - lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, c.Ui, c.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) + if err := stateLocker.Lock(sMgr, "workspace_delete"); err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - defer clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize()) + defer stateLocker.Unlock(nil) } // read the existing state file