From 305ef43aa6d644c7344a6a6621bf5e74857316f2 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 1 Apr 2017 15:42:13 -0400 Subject: [PATCH] provide contexts to clistate.Lock calls Add fields required to create an appropriate context for all calls to clistate.Lock. Add missing checks for Meta.stateLock, where we would attempt to lock, even if locking should be skipped. --- backend/backend.go | 4 ++ backend/local/backend_apply.go | 5 +- backend/local/backend_plan.go | 5 +- backend/local/backend_refresh.go | 5 +- command/env_delete.go | 22 +++++--- command/env_new.go | 22 +++++--- command/meta.go | 20 +++++--- command/meta_backend.go | 86 ++++++++++++++++++++------------ command/meta_backend_migrate.go | 40 ++++++++------- command/taint.go | 8 ++- command/untaint.go | 8 ++- 11 files changed, 145 insertions(+), 80 deletions(-) diff --git a/backend/backend.go b/backend/backend.go index f6c567c71..09a16fbaa 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -7,6 +7,7 @@ package backend import ( "context" "errors" + "time" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/state" @@ -132,6 +133,9 @@ type Operation struct { // state.Lockers for its duration, and Unlock when complete. LockState bool + // The duration to retry obtaining a State lock. + StateLockTimeout time.Duration + // Environment is the named state that should be loaded from the Backend. Environment string } diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 6fc5677f6..d7bf534a1 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -52,9 +52,12 @@ func (b *Local) opApply( } if op.LockState { + lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout) + defer cancel() + lockInfo := state.NewLockInfo() lockInfo.Operation = op.Type.String() - lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize()) + lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize()) if err != nil { runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err) return diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index a55c2afa0..42a56eb29 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -61,9 +61,12 @@ func (b *Local) opPlan( } if op.LockState { + lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout) + defer cancel() + lockInfo := state.NewLockInfo() lockInfo.Operation = op.Type.String() - lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize()) + lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize()) if err != nil { runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err) return diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index 9de954663..282e63045 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -51,9 +51,12 @@ func (b *Local) opRefresh( } if op.LockState { + lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout) + defer cancel() + lockInfo := state.NewLockInfo() lockInfo.Operation = op.Type.String() - lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize()) + lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize()) if err != nil { runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err) return diff --git a/command/env_delete.go b/command/env_delete.go index bad3d76bc..a0fb01fea 100644 --- a/command/env_delete.go +++ b/command/env_delete.go @@ -1,6 +1,7 @@ package command import ( + "context" "fmt" "strings" @@ -92,15 +93,20 @@ func (c *EnvDeleteCommand) Run(args []string) int { return 1 } - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "env delete" - lockID, err := clistate.Lock(sMgr, lockInfo, c.Ui, c.Colorize()) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) - return 1 + if c.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + + // Lock the state if we can + lockInfo := state.NewLockInfo() + lockInfo.Operation = "env 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 + } + defer clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize()) } - defer clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize()) err = b.DeleteState(delEnv) if err != nil { diff --git a/command/env_new.go b/command/env_new.go index b40b4e232..84b21bf8b 100644 --- a/command/env_new.go +++ b/command/env_new.go @@ -1,6 +1,7 @@ package command import ( + "context" "fmt" "os" "strings" @@ -87,15 +88,20 @@ func (c *EnvNewCommand) Run(args []string) int { return 1 } - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "env new" - lockID, err := clistate.Lock(sMgr, lockInfo, c.Ui, c.Colorize()) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) - return 1 + if c.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + + // Lock the state if we can + lockInfo := state.NewLockInfo() + lockInfo.Operation = "env 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 clistate.Unlock(sMgr, lockID, c.Ui, c.Colorize()) // read the existing state file stateFile, err := os.Open(statePath) diff --git a/command/meta.go b/command/meta.go index daf949a29..b1bfa4397 100644 --- a/command/meta.go +++ b/command/meta.go @@ -90,16 +90,20 @@ type Meta struct { // // stateLock is set to false to disable state locking // + // stateLockTimeout is the optional duration to retry a state locks locks + // when it is already locked by another process. + // // forceInitCopy suppresses confirmation for copying state data during // init. - statePath string - stateOutPath string - backupPath string - parallelism int - shadow bool - provider string - stateLock bool - forceInitCopy bool + statePath string + stateOutPath string + backupPath string + parallelism int + shadow bool + provider string + stateLock bool + stateLockTimeout time.Duration + forceInitCopy bool } // initStatePaths is used to initialize the default values for diff --git a/command/meta_backend.go b/command/meta_backend.go index 197ede993..c9a469f78 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -4,6 +4,7 @@ package command // exported and private. import ( + "context" "errors" "fmt" "io/ioutil" @@ -166,10 +167,11 @@ func (m *Meta) IsLocalBackend(b backend.Backend) bool { // be called. func (m *Meta) Operation() *backend.Operation { return &backend.Operation{ - PlanOutBackend: m.backendState, - Targets: m.targets, - UIIn: m.UIInput(), - Environment: m.Env(), + PlanOutBackend: m.backendState, + Targets: m.targets, + UIIn: m.UIInput(), + Environment: m.Env(), + StateLockTimeout: m.stateLockTimeout, } } @@ -609,15 +611,20 @@ func (m *Meta) backendFromPlan(opts *BackendOpts) (backend.Backend, error) { return nil, fmt.Errorf("Error reading state: %s", err) } - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from plan" + if m.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() - lockID, err := clistate.Lock(realMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { - return nil, fmt.Errorf("Error locking state: %s", err) + // 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 clistate.Unlock(realMgr, lockID, m.Ui, m.Colorize()) } - defer clistate.Unlock(realMgr, lockID, m.Ui, m.Colorize()) if err := realMgr.RefreshState(); err != nil { return nil, fmt.Errorf("Error reading state: %s", err) @@ -1024,15 +1031,20 @@ func (m *Meta) backend_C_r_s( } } - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from config" + if m.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() - lockID, err := clistate.Lock(sMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { - return nil, fmt.Errorf("Error locking state: %s", err) + // 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 clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) } - defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) // Store the metadata in our saved state location s := sMgr.State() @@ -1116,15 +1128,20 @@ func (m *Meta) backend_C_r_S_changed( } } - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from config" + if m.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() - lockID, err := clistate.Lock(sMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { - return nil, fmt.Errorf("Error locking state: %s", err) + // 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 clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) } - defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) // Update the backend state s = sMgr.State() @@ -1272,15 +1289,20 @@ func (m *Meta) backend_C_R_S_unchanged( } } - // Lock the state if we can - lockInfo := state.NewLockInfo() - lockInfo.Operation = "backend from config" + if m.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() - lockID, err := clistate.Lock(sMgr, lockInfo, m.Ui, m.Colorize()) - if err != nil { - return nil, fmt.Errorf("Error locking state: %s", err) + // 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 clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) } - defer clistate.Unlock(sMgr, lockID, m.Ui, m.Colorize()) // Unset the remote state s = sMgr.State() diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index 21b48b8d1..9a0b05285 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -1,6 +1,7 @@ package command import ( + "context" "fmt" "io/ioutil" "os" @@ -217,25 +218,30 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { errMigrateSingleLoadDefault), opts.TwoType, err) } - lockInfoOne := state.NewLockInfo() - lockInfoOne.Operation = "migration" - lockInfoOne.Info = "source state" + if m.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() - lockIDOne, err := clistate.Lock(stateOne, lockInfoOne, m.Ui, m.Colorize()) - if err != nil { - return fmt.Errorf("Error locking source state: %s", err) + 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 clistate.Unlock(stateOne, lockIDOne, 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 clistate.Unlock(stateTwo, lockIDTwo, m.Ui, m.Colorize()) } - defer clistate.Unlock(stateOne, lockIDOne, m.Ui, m.Colorize()) - - lockInfoTwo := state.NewLockInfo() - lockInfoTwo.Operation = "migration" - lockInfoTwo.Info = "destination state" - - lockIDTwo, err := clistate.Lock(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()) one := stateOne.State() two := stateTwo.State() diff --git a/command/taint.go b/command/taint.go index 5940f6ed4..483640419 100644 --- a/command/taint.go +++ b/command/taint.go @@ -1,6 +1,7 @@ package command import ( + "context" "fmt" "log" "strings" @@ -78,10 +79,13 @@ func (c *TaintCommand) Run(args []string) int { return 1 } - if c.Meta.stateLock { + if c.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + lockInfo := state.NewLockInfo() lockInfo.Operation = "taint" - lockID, err := clistate.Lock(st, lockInfo, c.Ui, c.Colorize()) + 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 diff --git a/command/untaint.go b/command/untaint.go index 95ccf4e26..cc4a99aa5 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -1,6 +1,7 @@ package command import ( + "context" "fmt" "log" "strings" @@ -66,10 +67,13 @@ func (c *UntaintCommand) Run(args []string) int { return 1 } - if c.Meta.stateLock { + if c.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + lockInfo := state.NewLockInfo() lockInfo.Operation = "untaint" - lockID, err := clistate.Lock(st, lockInfo, c.Ui, c.Colorize()) + 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