Revert "create clistate.Locker interface"

This reverts commit e88bd74bb7.

backend/local/backend_apply.go

@@ -59,13 +59,18 @@ func (b *Local) opApply(
 		lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, opState, op.Type.String(), "", b.CLI, b.Colorize())
+		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() {
-			runningOp.Err = unlock(runningOp.Err)
+			if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil {
+				runningOp.Err = multierror.Append(runningOp.Err, err)
+			}
 		}()
 	}
 

backend/local/backend_plan.go

@@ -9,10 +9,12 @@ 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"
 )
 
@@ -64,13 +66,18 @@ func (b *Local) opPlan(
 		lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, opState, op.Type.String(), "", b.CLI, b.Colorize())
+		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() {
-			runningOp.Err = unlock(runningOp.Err)
+			if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil {
+				runningOp.Err = multierror.Append(runningOp.Err, err)
+			}
 		}()
 	}
 

backend/local/backend_refresh.go

@@ -8,9 +8,11 @@ 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"
 )
 
@@ -55,13 +57,18 @@ func (b *Local) opRefresh(
 		lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, opState, op.Type.String(), "", b.CLI, b.Colorize())
+		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() {
-			runningOp.Err = unlock(runningOp.Err)
+			if err := clistate.Unlock(opState, lockID, b.CLI, b.Colorize()); err != nil {
+				runningOp.Err = multierror.Append(runningOp.Err, err)
+			}
 		}()
 	}
 

backend/remote-state/consul/client.go

@@ -9,7 +9,6 @@ import (
 	"errors"
 	"fmt"
 	"log"
-	"runtime/debug"
 	"sync"
 	"time"
 
@@ -82,9 +81,6 @@ func (c *RemoteClient) Get() (*remote.Payload, error) {
 
 	c.modifyIndex = pair.ModifyIndex
 
-	log.Println("[XXXX] GOT STATE WITH INDEX:", c.modifyIndex)
-	log.Println(string(debug.Stack()))
-
 	payload := pair.Value
 	// If the payload starts with 0x1f, it's gzip, not json
 	if len(pair.Value) >= 1 && pair.Value[0] == '\x1f' {
@@ -284,7 +280,6 @@ func (c *RemoteClient) lock() (string, error) {
 		return "", lockErr
 	}
 
-	defer log.Println("[XXXX] OBTAINED CONSUL LOCK")
 	c.lockCh = lockCh
 
 	err = c.putLockInfo(c.info)

command/clistate/state.go

@@ -8,11 +8,9 @@ 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"
@@ -50,119 +48,47 @@ that no one else is holding a lock.
 `
 )
 
-// Locker allows for more convenient locking, by creating the timeout context
+// Lock locks the given state and outputs to the user if locking
-// and LockInfo for the caller, while storing any related data required for
+// is taking longer than the threshold.  The lock is retried until the context
-// Unlock.
+// is cancelled.
-type Locker interface {
+func Lock(ctx context.Context, s state.State, info *state.LockInfo, ui cli.Ui, color *colorstring.Colorize) (string, error) {
-	// Lock the provided state, storing the reason string in the LockInfo.
+	var lockID string
-	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, lockInfo)
+		id, err := state.LockWithContext(ctx, s, info)
-		l.lockID = id
+		lockID = id
 		return err
 	}, func() {
-		if l.ui != nil {
+		if ui != nil {
-			l.ui.Output(l.color.Color(LockMessage))
+			ui.Output(color.Color(LockMessage))
 		}
 	})
 
 	if err != nil {
-		return errwrap.Wrapf(strings.TrimSpace(LockErrorMessage), err)
+		err = errwrap.Wrapf(strings.TrimSpace(LockErrorMessage), err)
 	}
 
-	return nil
+	return lockID, err
-}
-
-func (l *locker) Unlock(parentErr error) error {
-	l.mu.Lock()
-	defer l.mu.Unlock()
-
-	if l.lockID == "" {
-		return parentErr
 }
 
+// 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 {
 	err := slowmessage.Do(LockThreshold, func() error {
-		return l.state.Unlock(l.lockID)
+		return s.Unlock(id)
 	}, func() {
-		if l.ui != nil {
+		if ui != nil {
-			l.ui.Output(l.color.Color(UnlockMessage))
+			ui.Output(color.Color(UnlockMessage))
 		}
 	})
 
 	if err != nil {
-		l.ui.Output(l.color.Color(fmt.Sprintf(
+		ui.Output(color.Color(fmt.Sprintf(
 			"\n"+strings.TrimSpace(UnlockErrorMessage)+"\n", err)))
 
-		if parentErr != nil {
+		err = fmt.Errorf(
-			parentErr = multierror.Append(parentErr, err)
+			"Error releasing the state lock. Please see the longer error message above.")
-		}
 	}
 
-	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,11 +586,15 @@ func (m *Meta) backendFromPlan(opts *BackendOpts) (backend.Backend, error) {
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, realMgr, "backend from plan", "", m.Ui, m.Colorize())
+		// 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 {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer unlock(nil)
+		defer clistate.Unlock(realMgr, lockID, m.Ui, m.Colorize())
 	}
 
 	if err := realMgr.RefreshState(); err != nil {
@@ -963,11 +967,15 @@ func (m *Meta) backend_C_r_s(
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize())
+		// 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 {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer unlock(nil)
+		defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize())
 	}
 
 	// Store the metadata in our saved state location
@@ -1042,11 +1050,15 @@ func (m *Meta) backend_C_r_S_changed(
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize())
+		// 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 {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer unlock(nil)
+		defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize())
 	}
 
 	// Update the backend state
@@ -1181,11 +1193,15 @@ func (m *Meta) backend_C_R_S_unchanged(
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize())
+		// 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 {
 			return nil, fmt.Errorf("Error locking state: %s", err)
 		}
-		defer unlock(nil)
+		defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize())
 	}
 
 	// Unset the remote state

command/meta_backend_migrate.go

@@ -236,17 +236,25 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error {
 		lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout)
 		defer cancel()
 
-		unlockOne, err := clistate.Lock(lockCtx, stateOne, "migration", "source state", m.Ui, m.Colorize())
+		lockInfoOne := state.NewLockInfo()
+		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 unlockOne(nil)
+		defer clistate.Unlock(stateOne, lockIDOne, m.Ui, m.Colorize())
 
-		unlockTwo, err := clistate.Lock(lockCtx, stateTwo, "migration", "destination state", m.Ui, m.Colorize())
+		lockInfoTwo := state.NewLockInfo()
+		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 unlockTwo(nil)
+		defer clistate.Unlock(stateTwo, lockIDTwo, m.Ui, m.Colorize())
 
 		// We now own a lock, so double check that we have the version
 		// corresponding to the lock.

command/taint.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/command/clistate"
+	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -86,13 +87,15 @@ func (c *TaintCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, st, "taint", "", c.Ui, c.Colorize())
+		lockInfo := state.NewLockInfo()
+		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 unlock(nil)
+		defer clistate.Unlock(st, lockID, c.Ui, c.Colorize())
 	}
 
 	// Get the actual state structure

command/untaint.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/command/clistate"
+	"github.com/hashicorp/terraform/state"
 )
 
 // UntaintCommand is a cli.Command implementation that manually untaints
@@ -74,12 +75,15 @@ func (c *UntaintCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, st, "untaint", "", c.Ui, c.Colorize())
+		lockInfo := state.NewLockInfo()
+		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,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/command/clistate"
+	"github.com/hashicorp/terraform/state"
 	"github.com/mitchellh/cli"
 	"github.com/posener/complete"
 )
@@ -113,7 +114,10 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, sMgr, "workspace delete", "", c.Ui, c.Colorize())
+		// 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
@@ -128,7 +132,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.
-		unlock(nil)
+		clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize())
 	}
 
 	err = b.DeleteState(delEnv)

command/workspace_new.go

@@ -7,6 +7,7 @@ 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"
@@ -117,12 +118,15 @@ func (c *WorkspaceNewCommand) Run(args []string) int {
 		lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout)
 		defer cancel()
 
-		unlock, err := clistate.Lock(lockCtx, sMgr, "workspace_new", "", c.Ui, c.Colorize())
+		// 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 {
 			c.Ui.Error(fmt.Sprintf("Error locking state: %s", err))
 			return 1
 		}
-		defer unlock(nil)
+		defer clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize())
 	}
 
 	// read the existing state file