create clistate.Locker interface

Simplify the use of clistate.Lock by creating a clistate.Locker
instance, which stores the context of locking a state, to allow unlock
to be called without knowledge of how the state was locked.

This alows the backend code to bring the needed UI methods to the point
where the state is locked, and still unlock the state from an outer
scope.

Provide a NoopLocker as well, so that callers can always call Unlock
without verifying the status of the lock.

Add the StateLocker field to the backend.Operation, so that the state
lock can be carried between the different function scopes of the backend
code. This will allow the backend context to lock the state before it's
read, while allowing the different operations to unlock the state when
they complete.

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
 

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)
 	}()

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()
 

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)
 	}

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()
 

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() {

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,119 @@ that no one else is holding a lock.
 `
 )
 
-// Lock locks the given state and outputs to the user if locking
+// Locker allows for more convenient locking, by creating the timeout context
-// is taking longer than the threshold.  The lock is retried until the context
+// and LockInfo for the caller, while storing any related data required for
-// is cancelled.
+// Unlock.
-func Lock(ctx context.Context, s state.State, info *state.LockInfo, ui cli.Ui, color *colorstring.Colorize) (string, error) {
+type Locker interface {
-	var lockID string
+	// 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.
+// The provided context will be used for lock cancellation, and combined with
+// the timeout duration. 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)
+		id, err := state.LockWithContext(ctx, s, lockInfo)
-		lockID = id
+		l.lockID = id
 		return err
 	}, func() {
-		if ui != nil {
+		if l.ui != nil {
-			ui.Output(color.Color(LockMessage))
+			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
+func (l *locker) Unlock(parentErr error) error {
-// unlock fails what can be done.
+	l.mu.Lock()
-func Unlock(s state.State, id string, ui cli.Ui, color *colorstring.Colorize) error {
+	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 {
+		if l.ui != nil {
-			ui.Output(color.Color(UnlockMessage))
+			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(
+		if parentErr != nil {
-			"Error releasing the state lock. Please see the longer error message above.")
+			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
 }

command/meta_backend.go

@@ -586,15 +586,11 @@ func (m *Meta) backendFromPlan(opts *BackendOpts) (backend.Backend, error) {
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		// Lock the state if we can
+		unlock, err := clistate.Lock(lockCtx, realMgr, "backend from plan", "", m.Ui, m.Colorize())
-		lockInfo := state.NewLockInfo()
-		lockInfo.Operation = "backend from plan"
-
-		lockID, err := clistate.Lock(lockCtx, realMgr, lockInfo, m.Ui, m.Colorize())
 		if err != nil {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer clistate.Unlock(realMgr, lockID, m.Ui, m.Colorize())
+		defer unlock(nil)
 	}
 
 	if err := realMgr.RefreshState(); err != nil {
@@ -967,15 +963,11 @@ func (m *Meta) backend_C_r_s(
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		// Lock the state if we can
+		unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize())
-		lockInfo := state.NewLockInfo()
-		lockInfo.Operation = "backend from config"
-
-		lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, m.Ui, m.Colorize())
 		if err != nil {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize())
+		defer unlock(nil)
 	}
 
 	// Store the metadata in our saved state location
@@ -1050,15 +1042,11 @@ func (m *Meta) backend_C_r_S_changed(
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		// Lock the state if we can
+		unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize())
-		lockInfo := state.NewLockInfo()
-		lockInfo.Operation = "backend from config"
-
-		lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, m.Ui, m.Colorize())
 		if err != nil {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize())
+		defer unlock(nil)
 	}
 
 	// Update the backend state
@@ -1193,15 +1181,11 @@ func (m *Meta) backend_C_R_S_unchanged(
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		// Lock the state if we can
+		unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize())
-		lockInfo := state.NewLockInfo()
-		lockInfo.Operation = "backend from config"
-
-		lockID, err := clistate.Lock(lockCtx, sMgr, lockInfo, m.Ui, m.Colorize())
 		if err != nil {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize())
+		defer unlock(nil)
 	}
 
 	// Unset the remote state

command/meta_backend_migrate.go

@@ -236,25 +236,17 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error {
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		lockInfoOne := state.NewLockInfo()
+		unlockOne, err := clistate.Lock(lockCtx, stateOne, "migration", "source state", m.Ui, m.Colorize())
-		lockInfoOne.Operation = "migration"
-		lockInfoOne.Info = "source state"
-
-		lockIDOne, err := clistate.Lock(lockCtx, stateOne, lockInfoOne, m.Ui, m.Colorize())
 		if err != nil {
 			return fmt.Errorf("Error locking source state: %s", err)
 		}
-		defer clistate.Unlock(stateOne, lockIDOne, m.Ui, m.Colorize())
+		defer unlockOne(nil)
 
-		lockInfoTwo := state.NewLockInfo()
+		unlockTwo, err := clistate.Lock(lockCtx, stateTwo, "migration", "destination state", m.Ui, m.Colorize())
-		lockInfoTwo.Operation = "migration"
-		lockInfoTwo.Info = "destination state"
-
-		lockIDTwo, err := clistate.Lock(lockCtx, stateTwo, lockInfoTwo, m.Ui, m.Colorize())
 		if err != nil {
 			return fmt.Errorf("Error locking destination state: %s", err)
 		}
-		defer clistate.Unlock(stateTwo, lockIDTwo, m.Ui, m.Colorize())
+		defer unlockTwo(nil)
 
 		// We now own a lock, so double check that we have the version
 		// corresponding to the lock.

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"
 )
 
@@ -87,15 +86,13 @@ func (c *TaintCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		lockInfo := state.NewLockInfo()
+		unlock, err := clistate.Lock(lockCtx, st, "taint", "", c.Ui, c.Colorize())
-		lockInfo.Operation = "taint"
-		lockID, err := clistate.Lock(lockCtx, st, lockInfo, c.Ui, c.Colorize())
 		if err != nil {
 			c.Ui.Error(fmt.Sprintf("Error locking state: %s", err))
 			return 1
 		}
 
-		defer clistate.Unlock(st, lockID, c.Ui, c.Colorize())
+		defer unlock(nil)
 	}
 
 	// Get the actual state structure

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
@@ -75,15 +74,12 @@ func (c *UntaintCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		lockInfo := state.NewLockInfo()
+		unlock, err := clistate.Lock(lockCtx, st, "untaint", "", c.Ui, c.Colorize())
-		lockInfo.Operation = "untaint"
-		lockID, err := clistate.Lock(lockCtx, st, lockInfo, c.Ui, c.Colorize())
 		if err != nil {
 			c.Ui.Error(fmt.Sprintf("Error locking state: %s", err))
 			return 1
 		}
-
+		defer unlock(nil)
-		defer clistate.Unlock(st, lockID, c.Ui, c.Colorize())
 	}
 
 	// Get the actual state structure

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"
 )
@@ -114,10 +113,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		// Lock the state if we can
+		unlock, err := clistate.Lock(lockCtx, sMgr, "workspace delete", "", c.Ui, c.Colorize())
-		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
@@ -132,7 +128,7 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int {
 		// 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())
+		unlock(nil)
 	}
 
 	err = b.DeleteState(delEnv)

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"
@@ -118,15 +117,12 @@ func (c *WorkspaceNewCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		// Lock the state if we can
+		unlock, err := clistate.Lock(lockCtx, sMgr, "workspace_new", "", c.Ui, c.Colorize())
-		lockInfo := state.NewLockInfo()
-		lockInfo.Operation = "workspace new"
-		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
 		}
-		defer clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize())
+		defer unlock(nil)
 	}
 
 	// read the existing state file