From 12bda53558ed47c43671a8719a5d0ce796fa07ac Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 23 Feb 2018 11:31:13 -0500 Subject: [PATCH] Revert "create clistate.Locker interface" This reverts commit e88bd74bb70801564ba1ffa6e52ecbab78ce8f15. --- backend/local/backend_apply.go | 9 +- backend/local/backend_plan.go | 11 ++- backend/local/backend_refresh.go | 11 ++- backend/remote-state/consul/client.go | 5 -- command/clistate/state.go | 114 +++++--------------------- command/meta_backend.go | 32 ++++++-- command/meta_backend_migrate.go | 16 +++- command/taint.go | 7 +- command/untaint.go | 8 +- command/workspace_delete.go | 8 +- command/workspace_new.go | 8 +- 11 files changed, 104 insertions(+), 125 deletions(-) diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 8a97fc8299..da8c684bc3 100644 --- a/backend/local/backend_apply.go +++ b/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) + } }() } diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index d2956e341d..ebf0531922 100644 --- a/backend/local/backend_plan.go +++ b/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) + } }() } diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index a19aa9e5e5..959f969a32 100644 --- a/backend/local/backend_refresh.go +++ b/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) + } }() } diff --git a/backend/remote-state/consul/client.go b/backend/remote-state/consul/client.go index 8ccb1814e0..bd37712f3f 100644 --- a/backend/remote-state/consul/client.go +++ b/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) diff --git a/command/clistate/state.go b/command/clistate/state.go index 9bd9fe940e..f02f053be9 100644 --- a/command/clistate/state.go +++ b/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 -// and LockInfo for the caller, while storing any related data required for -// Unlock. -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. -// 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 +// 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 err := slowmessage.Do(LockThreshold, func() error { - id, err := state.LockWithContext(ctx, s, lockInfo) - l.lockID = id + id, err := state.LockWithContext(ctx, s, info) + lockID = id return err }, func() { - if l.ui != nil { - l.ui.Output(l.color.Color(LockMessage)) + if ui != nil { + 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 { - l.ui.Output(l.color.Color(UnlockMessage)) + if ui != nil { + 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 { - parentErr = multierror.Append(parentErr, err) - } + err = fmt.Errorf( + "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 } diff --git a/command/meta_backend.go b/command/meta_backend.go index 726f3b70cc..2fa4ff542e 100644 --- a/command/meta_backend.go +++ b/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 diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index be20a44768..7f60a6605d 100644 --- a/command/meta_backend_migrate.go +++ b/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. diff --git a/command/taint.go b/command/taint.go index c3d6c4cc99..626f88a46f 100644 --- a/command/taint.go +++ b/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 diff --git a/command/untaint.go b/command/untaint.go index 9a2e43b246..1eca202779 100644 --- a/command/untaint.go +++ b/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 diff --git a/command/workspace_delete.go b/command/workspace_delete.go index 8a9f792e1f..99dac86d3b 100644 --- a/command/workspace_delete.go +++ b/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) diff --git a/command/workspace_new.go b/command/workspace_new.go index 430ecc58b5..71c9fdc1fc 100644 --- a/command/workspace_new.go +++ b/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