From bf6384a163390062cee54ca59887c02ca074a82d Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 31 Mar 2017 16:30:51 -0400 Subject: [PATCH 01/12] All states are lockers Since moving to the new backends, all states (except InmemState) are Lockers. Add the methods to the State interface to remove a heap of assertion checks. --- state/inmem.go | 8 ++++++++ state/state.go | 1 + 2 files changed, 9 insertions(+) diff --git a/state/inmem.go b/state/inmem.go index ff8daab8f..a930f78c7 100644 --- a/state/inmem.go +++ b/state/inmem.go @@ -26,3 +26,11 @@ func (s *InmemState) WriteState(state *terraform.State) error { func (s *InmemState) PersistState() error { return nil } + +func (s *InmemState) Lock(*LockInfo) (string, error) { + return "", nil +} + +func (s *InmemState) Unlock(string) error { + return nil +} diff --git a/state/state.go b/state/state.go index 9491958a3..39f81c9d1 100644 --- a/state/state.go +++ b/state/state.go @@ -28,6 +28,7 @@ type State interface { StateWriter StateRefresher StatePersister + Locker } // StateReader is the interface for things that can return a state. Retrieving From 75458a182d4f17e00d7e16dfb759dd75dda296d0 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 31 Mar 2017 17:04:56 -0400 Subject: [PATCH 02/12] remove extra state.Locker assertions All states are lockers, so get rid of extra asertions. --- backend/remote-state/consul/backend_state.go | 7 ++----- command/state/state.go | 14 ++------------ command/unlock.go | 9 +-------- state/backup.go | 11 ++--------- 4 files changed, 7 insertions(+), 34 deletions(-) diff --git a/backend/remote-state/consul/backend_state.go b/backend/remote-state/consul/backend_state.go index 74f30c842..9a8fd080f 100644 --- a/backend/remote-state/consul/backend_state.go +++ b/backend/remote-state/consul/backend_state.go @@ -102,22 +102,19 @@ func (b *Backend) State(name string) (state.State, error) { stateMgr = &state.LockDisabled{Inner: stateMgr} } - // Get the locker, which we know always exists - stateMgrLocker := stateMgr.(state.Locker) - // Grab a lock, we use this to write an empty state if one doesn't // exist already. We have to write an empty state as a sentinel value // so States() knows it exists. lockInfo := state.NewLockInfo() lockInfo.Operation = "init" - lockId, err := stateMgrLocker.Lock(lockInfo) + lockId, err := stateMgr.Lock(lockInfo) if err != nil { return nil, fmt.Errorf("failed to lock state in Consul: %s", err) } // Local helper function so we can call it multiple places lockUnlock := func(parent error) error { - if err := stateMgrLocker.Unlock(lockId); err != nil { + if err := stateMgr.Unlock(lockId); err != nil { return fmt.Errorf(strings.TrimSpace(errStateUnlock), lockId, err) } diff --git a/command/state/state.go b/command/state/state.go index 3915e8ca2..52adf62a6 100644 --- a/command/state/state.go +++ b/command/state/state.go @@ -50,15 +50,10 @@ 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. func Lock(s state.State, info *state.LockInfo, ui cli.Ui, color *colorstring.Colorize) (string, error) { - sl, ok := s.(state.Locker) - if !ok { - return "", nil - } - var lockID string err := slowmessage.Do(LockThreshold, func() error { - id, err := sl.Lock(info) + id, err := s.Lock(info) lockID = id return err }, func() { @@ -77,13 +72,8 @@ func Lock(s state.State, info *state.LockInfo, ui cli.Ui, color *colorstring.Col // 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 { - sl, ok := s.(state.Locker) - if !ok { - return nil - } - err := slowmessage.Do(LockThreshold, func() error { - return sl.Unlock(id) + return s.Unlock(id) }, func() { if ui != nil { ui.Output(color.Color(UnlockMessage)) diff --git a/command/unlock.go b/command/unlock.go index 010fd9332..666e4f346 100644 --- a/command/unlock.go +++ b/command/unlock.go @@ -59,13 +59,6 @@ func (c *UnlockCommand) Run(args []string) int { return 1 } - s, ok := st.(state.Locker) - if !ok { - c.Ui.Error("The remote state backend in use does not support locking, and therefor\n" + - "cannot be unlocked.") - return 1 - } - isLocal := false switch s := st.(type) { case *state.BackupState: @@ -103,7 +96,7 @@ func (c *UnlockCommand) Run(args []string) int { } } - if err := s.Unlock(lockID); err != nil { + if err := st.Unlock(lockID); err != nil { c.Ui.Error(fmt.Sprintf("Failed to unlock state: %s", err)) return 1 } diff --git a/state/backup.go b/state/backup.go index 15d8f6f3e..c357bba49 100644 --- a/state/backup.go +++ b/state/backup.go @@ -41,19 +41,12 @@ func (s *BackupState) PersistState() error { return s.Real.PersistState() } -// all states get wrapped by BackupState, so it has to be a Locker func (s *BackupState) Lock(info *LockInfo) (string, error) { - if s, ok := s.Real.(Locker); ok { - return s.Lock(info) - } - return "", nil + return s.Real.Lock(info) } func (s *BackupState) Unlock(id string) error { - if s, ok := s.Real.(Locker); ok { - return s.Unlock(id) - } - return nil + return s.Real.Unlock(id) } func (s *BackupState) backup() error { From 826771a830aecfd4b76ad003a786e664b58a3f1f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 31 Mar 2017 17:27:39 -0400 Subject: [PATCH 03/12] add state.LockWithContext LockWithContext will retry a lock until the context expires or is cancelled. This will let us implement a `-lock-timeout` flag, and make use of existing contexts when applicable. --- state/state.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/state/state.go b/state/state.go index 39f81c9d1..246c9c44d 100644 --- a/state/state.go +++ b/state/state.go @@ -2,6 +2,7 @@ package state import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -73,6 +74,36 @@ type Locker interface { Unlock(id string) error } +// Lock the state, using the provided context for timeout and cancellation +// TODO: this should probably backoff somewhat. +func LockWithContext(s State, info *LockInfo, ctx context.Context) (string, error) { + for { + id, err := s.Lock(info) + if err == nil { + return id, nil + } + + le, ok := err.(*LockError) + if !ok { + // not a lock error, so we can't retry + return "", err + } + + if le.Info.ID == "" { + // the lock has no ID, something is wrong so don't keep trying + return "", fmt.Errorf("lock error missing ID: %s", err) + } + + // there's an existing lock, wait and try again + select { + case <-ctx.Done(): + // return the last lock error with the info + return "", err + case <-time.After(time.Second): + } + } +} + // Generate a LockInfo structure, populating the required fields. func NewLockInfo() *LockInfo { // this doesn't need to be cryptographically secure, just unique. From 3f0dcd130838ea8e8880d76f178b2f46ec0665bf Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 1 Apr 2017 14:58:19 -0400 Subject: [PATCH 04/12] Have the clistate Lock use LockWithContext - Have the ui Lock helper use state.LockWithContext. - Rename the message package to clistate, since that's how it's imported everywhere. - Use a more idiomatic placement of the Context in the LockWithContext args. --- backend/local/backend_apply.go | 2 +- backend/local/backend_plan.go | 2 +- backend/local/backend_refresh.go | 2 +- command/{state => clistate}/state.go | 10 ++++++---- command/env_delete.go | 3 +-- command/env_new.go | 3 +-- command/meta_backend.go | 4 ++-- command/meta_backend_migrate.go | 2 +- command/taint.go | 2 +- command/untaint.go | 2 +- state/state.go | 4 ++-- 11 files changed, 18 insertions(+), 18 deletions(-) rename command/{state => clistate}/state.go (90%) diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 6b80aac34..6fc5677f6 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" - clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index f63735873..a55c2afa0 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -10,8 +10,8 @@ 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" - clistate "github.com/hashicorp/terraform/command/state" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index c8b23bd32..9de954663 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -9,7 +9,7 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform/backend" - clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/state" ) diff --git a/command/state/state.go b/command/clistate/state.go similarity index 90% rename from command/state/state.go rename to command/clistate/state.go index 52adf62a6..f02f053be 100644 --- a/command/state/state.go +++ b/command/clistate/state.go @@ -2,9 +2,10 @@ // // This is a separate package so that backends can use this for consistent // messaging without creating a circular reference to the command package. -package message +package clistate import ( + "context" "fmt" "strings" "time" @@ -48,12 +49,13 @@ 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. -func Lock(s state.State, info *state.LockInfo, ui cli.Ui, color *colorstring.Colorize) (string, error) { +// 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 := s.Lock(info) + id, err := state.LockWithContext(ctx, s, info) lockID = id return err }, func() { diff --git a/command/env_delete.go b/command/env_delete.go index be21b76ed..bad3d76bc 100644 --- a/command/env_delete.go +++ b/command/env_delete.go @@ -4,10 +4,9 @@ import ( "fmt" "strings" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/state" "github.com/mitchellh/cli" - - clistate "github.com/hashicorp/terraform/command/state" ) type EnvDeleteCommand struct { diff --git a/command/env_new.go b/command/env_new.go index 8b0e8fcdb..b40b4e232 100644 --- a/command/env_new.go +++ b/command/env_new.go @@ -5,11 +5,10 @@ import ( "os" "strings" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/cli" - - clistate "github.com/hashicorp/terraform/command/state" ) type EnvNewCommand struct { diff --git a/command/meta_backend.go b/command/meta_backend.go index c50214116..197ede993 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -16,13 +16,13 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/hcl" "github.com/hashicorp/terraform/backend" - backendinit "github.com/hashicorp/terraform/backend/init" - clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/mapstructure" + backendinit "github.com/hashicorp/terraform/backend/init" backendlocal "github.com/hashicorp/terraform/backend/local" ) diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index b9133a052..21b48b8d1 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -9,7 +9,7 @@ import ( "strings" "github.com/hashicorp/terraform/backend" - clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) diff --git a/command/taint.go b/command/taint.go index 487099187..5940f6ed4 100644 --- a/command/taint.go +++ b/command/taint.go @@ -5,7 +5,7 @@ import ( "log" "strings" - clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/terraform" ) diff --git a/command/untaint.go b/command/untaint.go index ab697b823..95ccf4e26 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -5,7 +5,7 @@ import ( "log" "strings" - clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/state" ) diff --git a/state/state.go b/state/state.go index 246c9c44d..6e851c2b3 100644 --- a/state/state.go +++ b/state/state.go @@ -74,9 +74,9 @@ type Locker interface { Unlock(id string) error } -// Lock the state, using the provided context for timeout and cancellation +// Lock the state, using the provided context for timeout and cancellation. // TODO: this should probably backoff somewhat. -func LockWithContext(s State, info *LockInfo, ctx context.Context) (string, error) { +func LockWithContext(ctx context.Context, s State, info *LockInfo) (string, error) { for { id, err := s.Lock(info) if err == nil { From 305ef43aa6d644c7344a6a6621bf5e74857316f2 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 1 Apr 2017 15:42:13 -0400 Subject: [PATCH 05/12] 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 From 9e9d0b1bdfafbb27e26da5f3f3f17c101200d388 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 1 Apr 2017 16:06:28 -0400 Subject: [PATCH 06/12] move force-unlock to plumbing shouldn't be listed as a common command --- commands.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/commands.go b/commands.go index b9343301c..409f4d85d 100644 --- a/commands.go +++ b/commands.go @@ -42,8 +42,9 @@ func init() { // that to match. PlumbingCommands = map[string]struct{}{ - "state": struct{}{}, // includes all subcommands - "debug": struct{}{}, // includes all subcommands + "state": struct{}{}, // includes all subcommands + "debug": struct{}{}, // includes all subcommands + "force-unlock": struct{}{}, } Commands = map[string]cli.CommandFactory{ @@ -105,12 +106,6 @@ func init() { }, nil }, - "force-unlock": func() (cli.Command, error) { - return &command.UnlockCommand{ - Meta: meta, - }, nil - }, - "get": func() (cli.Command, error) { return &command.GetCommand{ Meta: meta, @@ -215,6 +210,12 @@ func init() { }, nil }, + "force-unlock": func() (cli.Command, error) { + return &command.UnlockCommand{ + Meta: meta, + }, nil + }, + "state": func() (cli.Command, error) { return &command.StateCommand{}, nil }, From 5eca913b14a38a6c48168ef4d821d1fd2f2773c3 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 1 Apr 2017 16:19:59 -0400 Subject: [PATCH 07/12] add cli flags for -lock-timeout Add the -lock-timeout flag to the appropriate commands. Add the -lock flag to `init` and `import` which were missing it. Set both stateLock and stateLockTimeout in Meta.flagsSet, and remove the extra references for clarity. --- command/apply.go | 6 +++++- command/import.go | 6 ++++++ command/init.go | 6 ++++++ command/meta_backend.go | 1 + command/plan.go | 4 +++- command/refresh.go | 4 +++- command/taint.go | 3 +++ command/untaint.go | 3 +++ 8 files changed, 30 insertions(+), 3 deletions(-) diff --git a/command/apply.go b/command/apply.go index 696fc00c7..ca9813eb5 100644 --- a/command/apply.go +++ b/command/apply.go @@ -48,6 +48,7 @@ func (c *ApplyCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path") cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path") cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -183,7 +184,6 @@ func (c *ApplyCommand) Run(args []string) int { opReq.Plan = plan opReq.PlanRefresh = refresh opReq.Type = backend.OperationTypeApply - opReq.LockState = c.Meta.stateLock // Perform the operation ctx, ctxCancel := context.WithCancel(context.Background()) @@ -276,6 +276,8 @@ Options: -lock=true Lock the state file when locking is supported. + -lock-timeout=0s Duration to retry a state lock. + -input=true Ask for input for variables if not directly set. -no-color If specified, output won't contain any color. @@ -325,6 +327,8 @@ Options: -lock=true Lock the state file when locking is supported. + -lock-timeout=0s Duration to retry a state lock. + -no-color If specified, output won't contain any color. -parallelism=n Limit the number of concurrent operations. diff --git a/command/import.go b/command/import.go index df8e420bd..1636ab9d9 100644 --- a/command/import.go +++ b/command/import.go @@ -35,6 +35,8 @@ func (c *ImportCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path") cmdFlags.StringVar(&configPath, "config", pwd, "path") cmdFlags.StringVar(&c.Meta.provider, "provider", "", "provider") + cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -162,6 +164,10 @@ Options: -input=true Ask for input for variables if not directly set. + -lock=true Lock the state file when locking is supported. + + -lock-timeout=0s Duration to retry a state lock. + -no-color If specified, output won't contain any color. -provider=provider Specific provider to use for import. This is used for diff --git a/command/init.go b/command/init.go index d2a9e835c..6721b87a1 100644 --- a/command/init.go +++ b/command/init.go @@ -28,6 +28,8 @@ func (c *InitCommand) Run(args []string) int { cmdFlags.Var((*variables.FlagAny)(&flagConfigExtra), "backend-config", "") cmdFlags.BoolVar(&flagGet, "get", true, "") cmdFlags.BoolVar(&c.forceInitCopy, "force-copy", false, "suppress prompts about copying state data") + cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { @@ -226,6 +228,10 @@ Options: -input=true Ask for input if necessary. If false, will error if input was required. + -lock=true Lock the state file when locking is supported. + + -lock-timeout=0s Duration to retry a state lock. + -no-color If specified, output won't contain any color. -force-copy Suppress prompts about copying state data. This is diff --git a/command/meta_backend.go b/command/meta_backend.go index c9a469f78..64f6a7861 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -171,6 +171,7 @@ func (m *Meta) Operation() *backend.Operation { Targets: m.targets, UIIn: m.UIInput(), Environment: m.Env(), + LockState: m.stateLock, StateLockTimeout: m.stateLockTimeout, } } diff --git a/command/plan.go b/command/plan.go index 5b89634ed..0b66fdffd 100644 --- a/command/plan.go +++ b/command/plan.go @@ -32,6 +32,7 @@ func (c *PlanCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.statePath, "state", "", "path") cmdFlags.BoolVar(&detailed, "detailed-exitcode", false, "detailed-exitcode") cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -85,7 +86,6 @@ func (c *PlanCommand) Run(args []string) int { opReq.PlanRefresh = refresh opReq.PlanOutPath = outPath opReq.Type = backend.OperationTypePlan - opReq.LockState = c.Meta.stateLock // Perform the operation op, err := b.Operation(context.Background(), opReq) @@ -145,6 +145,8 @@ Options: -lock=true Lock the state file when locking is supported. + -lock-timeout=0s Duration to retry a state lock. + -module-depth=n Specifies the depth of modules to show in the output. This does not affect the plan itself, only the output shown. By default, this is -1, which will expand all. diff --git a/command/refresh.go b/command/refresh.go index c9c5527b9..3f1b8bf28 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -24,6 +24,7 @@ func (c *RefreshCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path") cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path") cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -53,7 +54,6 @@ func (c *RefreshCommand) Run(args []string) int { opReq := c.Operation() opReq.Type = backend.OperationTypeRefresh opReq.Module = mod - opReq.LockState = c.Meta.stateLock // Perform the operation op, err := b.Operation(context.Background(), opReq) @@ -98,6 +98,8 @@ Options: -lock=true Lock the state file when locking is supported. + -lock-timeout=0s Duration to retry a state lock. + -no-color If specified, output won't contain any color. -state=path Path to read and save state (unless state-out diff --git a/command/taint.go b/command/taint.go index 483640419..e4e443620 100644 --- a/command/taint.go +++ b/command/taint.go @@ -29,6 +29,7 @@ func (c *TaintCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path") cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path") cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -192,6 +193,8 @@ Options: -lock=true Lock the state file when locking is supported. + -lock-timeout=0s Duration to retry a state lock. + -module=path The module path where the resource lives. By default this will be root. Child modules can be specified by names. Ex. "consul" or "consul.vpc" (nested modules). diff --git a/command/untaint.go b/command/untaint.go index cc4a99aa5..adce511f7 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -28,6 +28,7 @@ func (c *UntaintCommand) Run(args []string) int { cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path") cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path") cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state") + cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -180,6 +181,8 @@ Options: -lock=true Lock the state file when locking is supported. + -lock-timeout=0s Duration to retry a state lock. + -module=path The module path where the resource lives. By default this will be root. Child modules can be specified by names. Ex. "consul" or "consul.vpc" (nested modules). From 93b1dd6323ea0896b683409e559730fbe3f0b1cb Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 1 Apr 2017 17:56:03 -0400 Subject: [PATCH 08/12] give LockWithContext a little backoff Backoff the Lock calls exponentially, to a reasonable limit. --- state/state.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/state/state.go b/state/state.go index 6e851c2b3..67cf91e61 100644 --- a/state/state.go +++ b/state/state.go @@ -75,8 +75,10 @@ type Locker interface { } // Lock the state, using the provided context for timeout and cancellation. -// TODO: this should probably backoff somewhat. +// This backs off slightly to an upper limit. func LockWithContext(ctx context.Context, s State, info *LockInfo) (string, error) { + delay := time.Second + maxDelay := 16 * time.Second for { id, err := s.Lock(info) if err == nil { @@ -99,7 +101,10 @@ func LockWithContext(ctx context.Context, s State, info *LockInfo) (string, erro case <-ctx.Done(): // return the last lock error with the info return "", err - case <-time.After(time.Second): + case <-time.After(delay): + if delay < maxDelay { + delay *= 2 + } } } } From d1460d8c824fedfd9ce8e933a4b601717559f637 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 3 Apr 2017 11:00:45 -0400 Subject: [PATCH 09/12] test LockWithContext --- state/inmem.go | 52 +++++++++++++++++++++++++++++++++++++++++++++ state/inmem_test.go | 36 +++++++++++++++++++++++++++++++ state/state_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+) diff --git a/state/inmem.go b/state/inmem.go index a930f78c7..2bbfb3d44 100644 --- a/state/inmem.go +++ b/state/inmem.go @@ -1,6 +1,10 @@ package state import ( + "errors" + "sync" + "time" + "github.com/hashicorp/terraform/terraform" ) @@ -34,3 +38,51 @@ func (s *InmemState) Lock(*LockInfo) (string, error) { func (s *InmemState) Unlock(string) error { return nil } + +// inmemLocker is an in-memory State implementation for testing locks. +type inmemLocker struct { + *InmemState + + mu sync.Mutex + lockInfo *LockInfo + // count the calls to Lock + lockCounter int +} + +func (s *inmemLocker) Lock(info *LockInfo) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.lockCounter++ + + lockErr := &LockError{ + Info: &LockInfo{}, + } + + if s.lockInfo != nil { + lockErr.Err = errors.New("state locked") + *lockErr.Info = *s.lockInfo + return "", lockErr + } + + info.Created = time.Now().UTC() + s.lockInfo = info + return s.lockInfo.ID, nil +} + +func (s *inmemLocker) Unlock(id string) error { + s.mu.Lock() + defer s.mu.Unlock() + + lockErr := &LockError{ + Info: &LockInfo{}, + } + + if id != s.lockInfo.ID { + lockErr.Err = errors.New("invalid lock id") + *lockErr.Info = *s.lockInfo + return lockErr + } + + s.lockInfo = nil + return nil +} diff --git a/state/inmem_test.go b/state/inmem_test.go index 885127122..6ca8a69a5 100644 --- a/state/inmem_test.go +++ b/state/inmem_test.go @@ -14,3 +14,39 @@ func TestInmemState_impl(t *testing.T) { var _ StatePersister = new(InmemState) var _ StateRefresher = new(InmemState) } + +func TestInmemLocker(t *testing.T) { + inmem := &InmemState{state: TestStateInitial()} + // test that it correctly wraps the inmem state + s := &inmemLocker{InmemState: inmem} + TestState(t, s) + + info := NewLockInfo() + + id, err := s.Lock(info) + if err != nil { + t.Fatal(err) + } + + if id == "" { + t.Fatal("no lock id from state lock") + } + + // locking again should fail + _, err = s.Lock(NewLockInfo()) + if err == nil { + t.Fatal("state locked while locked") + } + + if err.(*LockError).Info.ID != id { + t.Fatal("wrong lock id from lock failure") + } + + if err := s.Unlock(id); err != nil { + t.Fatal(err) + } + + if _, err := s.Lock(NewLockInfo()); err != nil { + t.Fatal(err) + } +} diff --git a/state/state_test.go b/state/state_test.go index e93f5680a..df7a6fd05 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -1,12 +1,14 @@ package state import ( + "context" "encoding/json" "flag" "io/ioutil" "log" "os" "testing" + "time" "github.com/hashicorp/terraform/helper/logging" ) @@ -50,3 +52,52 @@ func TestNewLockInfo(t *testing.T) { t.Fatal(err) } } + +func TestLockWithContext(t *testing.T) { + inmem := &InmemState{state: TestStateInitial()} + // test that it correctly wraps the inmem state + s := &inmemLocker{InmemState: inmem} + + id, err := s.Lock(NewLockInfo()) + if err != nil { + t.Fatal(err) + } + + // use a cancelled context for an immediate timeout + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + info := NewLockInfo() + info.Info = "lock with context" + _, err = LockWithContext(ctx, s, info) + if err == nil { + t.Fatal("lock should have failed immediately") + } + + // unlock the state during LockWithContext + unlocked := make(chan struct{}) + go func() { + defer close(unlocked) + time.Sleep(500 * time.Millisecond) + if err := s.Unlock(id); err != nil { + t.Fatal(err) + } + }() + + ctx, cancel = context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + id, err = LockWithContext(ctx, s, info) + if err != nil { + t.Fatal("lock should have completed within 2s:", err) + } + + // ensure the goruotine completes + <-unlocked + + // Lock should have been called a total of 4 times. + // 1 initial lock, 1 failure, 1 failure + 1 retry + if s.lockCounter != 4 { + t.Fatalf("lock only called %d times", s.lockCounter) + } +} From 3d604851c27caf01c3f70728aaa2cf6061cae1e7 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 3 Apr 2017 11:33:38 -0400 Subject: [PATCH 10/12] test -lock-timeout from cli --- command/apply_test.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/command/apply_test.go b/command/apply_test.go index 661d88c76..abc33dae5 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "io/ioutil" + "log" "net" "net/http" "net/url" @@ -92,6 +93,42 @@ func TestApply_lockedState(t *testing.T) { } } +// test apply with locked state, waiting for unlock +func TestApply_lockedStateWait(t *testing.T) { + statePath := testTempFile(t) + + unlock, err := testLockState("./testdata", statePath) + if err != nil { + t.Fatal(err) + } + + // unlock during apply + go func() { + time.Sleep(500 * time.Millisecond) + unlock() + }() + + p := testProvider() + ui := new(cli.MockUi) + c := &ApplyCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + // wait 4s just in case the lock process doesn't release in under a second, + // and we want our context to be alive for a second retry at the 3s mark. + args := []string{ + "-state", statePath, + "-lock-timeout", "4s", + testFixturePath("apply"), + } + if code := c.Run(args); code != 0 { + log.Fatalf("lock should have succeed in less than 3s: %s", ui.ErrorWriter) + } +} + // high water mark counter type hwm struct { sync.Mutex From af2e289212a2c9b838de38a4bd652634b8c27ada Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 3 Apr 2017 14:26:05 -0400 Subject: [PATCH 11/12] remove Sleep from TestLockWithContext --- state/state.go | 7 +++++++ state/state_test.go | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/state/state.go b/state/state.go index 67cf91e61..f6c4f16d4 100644 --- a/state/state.go +++ b/state/state.go @@ -74,6 +74,9 @@ type Locker interface { Unlock(id string) error } +// test hook to verify that LockWithContext has attempted a lock +var postLockHook func() + // Lock the state, using the provided context for timeout and cancellation. // This backs off slightly to an upper limit. func LockWithContext(ctx context.Context, s State, info *LockInfo) (string, error) { @@ -96,6 +99,10 @@ func LockWithContext(ctx context.Context, s State, info *LockInfo) (string, erro return "", fmt.Errorf("lock error missing ID: %s", err) } + if postLockHook != nil { + postLockHook() + } + // there's an existing lock, wait and try again select { case <-ctx.Done(): diff --git a/state/state_test.go b/state/state_test.go index df7a6fd05..a8fdec6ab 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -74,11 +74,18 @@ func TestLockWithContext(t *testing.T) { t.Fatal("lock should have failed immediately") } + // block until LockwithContext has made a first attempt + attempted := make(chan struct{}) + postLockHook = func() { + close(attempted) + postLockHook = nil + } + // unlock the state during LockWithContext unlocked := make(chan struct{}) go func() { defer close(unlocked) - time.Sleep(500 * time.Millisecond) + <-attempted if err := s.Unlock(id); err != nil { t.Fatal(err) } From 7cfb515a0387d47322a1f8decc3ba4765a48de40 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 4 Apr 2017 13:48:59 -0400 Subject: [PATCH 12/12] update command docs --- website/source/docs/commands/apply.html.markdown | 4 ++++ website/source/docs/commands/import.html.md | 16 +++++++++++----- website/source/docs/commands/index.html.markdown | 3 ++- website/source/docs/commands/init.html.markdown | 9 +++++++++ website/source/docs/commands/plan.html.markdown | 4 ++++ .../source/docs/commands/refresh.html.markdown | 8 ++++++++ website/source/docs/commands/taint.html.markdown | 4 ++++ .../source/docs/commands/untaint.html.markdown | 4 ++++ .../intro/getting-started/install.html.markdown | 3 ++- 9 files changed, 48 insertions(+), 7 deletions(-) diff --git a/website/source/docs/commands/apply.html.markdown b/website/source/docs/commands/apply.html.markdown index 8224c8a14..3c00e7c05 100644 --- a/website/source/docs/commands/apply.html.markdown +++ b/website/source/docs/commands/apply.html.markdown @@ -31,6 +31,10 @@ The command-line flags are all optional. The list of available flags are: * `-backup=path` - Path to the backup file. Defaults to `-state-out` with the ".backup" extension. Disabled by setting to "-". +* `-lock=true` - Lock the state file when locking is supported. + +* `-lock-timeout=0s` - Duration to retry a state lock. + * `-input=true` - Ask for input for variables if not directly set. * `-no-color` - Disables output with coloring. diff --git a/website/source/docs/commands/import.html.md b/website/source/docs/commands/import.html.md index 650241972..b894b58aa 100644 --- a/website/source/docs/commands/import.html.md +++ b/website/source/docs/commands/import.html.md @@ -42,6 +42,17 @@ The command-line flags are all optional. The list of available flags are: * `-input=true` - Whether to ask for input for provider configuration. +* `-lock=true` - Lock the state file when locking is supported. + +* `-lock-timeout=0s` - Duration to retry a state lock. + +* `-no-color` - If specified, output won't contain any color. + +* `-provider=provider` - Specified provider to use for import. This is used for + specifying provider aliases, such as "aws.eu". This defaults to the normal + provider based on the prefix of the resource being imported. You usually + don't need to specify this. + * `-state=path` - The path to read and save state files (unless state-out is specified). Ignored when [remote state](/docs/state/remote.html) is used. @@ -49,11 +60,6 @@ The command-line flags are all optional. The list of available flags are: the state path. Ignored when [remote state](/docs/state/remote.html) is used. -* `-provider=provider` - Specified provider to use for import. This is used for - specifying provider aliases, such as "aws.eu". This defaults to the normal - provider based on the prefix of the resource being imported. You usually - don't need to specify this. - * `-var 'foo=bar'` - Set a variable in the Terraform configuration. This flag can be set multiple times. Variable values are interpreted as [HCL](/docs/configuration/syntax.html#HCL), so list and map values can be diff --git a/website/source/docs/commands/index.html.markdown b/website/source/docs/commands/index.html.markdown index 120bd8f09..0a01fed7b 100644 --- a/website/source/docs/commands/index.html.markdown +++ b/website/source/docs/commands/index.html.markdown @@ -33,8 +33,8 @@ Common commands: apply Builds or changes infrastructure console Interactive console for Terraform interpolations destroy Destroy Terraform-managed infrastructure + env Environment management fmt Rewrites config files to canonical format - force-unlock Manually unlock the terraform state get Download and install modules for the configuration graph Create a visual graph of Terraform resources import Import existing infrastructure into Terraform @@ -51,6 +51,7 @@ Common commands: All other commands: debug Debug output management (experimental) + force-unlock Manually unlock the terraform state state Advanced state management ``` diff --git a/website/source/docs/commands/init.html.markdown b/website/source/docs/commands/init.html.markdown index a9c0863fe..d28316395 100644 --- a/website/source/docs/commands/init.html.markdown +++ b/website/source/docs/commands/init.html.markdown @@ -54,6 +54,15 @@ The command-line flags are all optional. The list of available flags are: * `-input=true` - Ask for input interactively if necessary. If this is false and input is required, `init` will error. +* `-lock=true` - Lock the state file when locking is supported. + +* `-lock-timeout=0s` - Duration to retry a state lock. + +* `-no-color` - If specified, output won't contain any color. + +* `-force-copy` - Suppress prompts about copying state data. This is equivalent + to providing a "yes" to all confirmation prompts. + ## Backend Config The `-backend-config` can take a path or `key=value` pair to specify additional diff --git a/website/source/docs/commands/plan.html.markdown b/website/source/docs/commands/plan.html.markdown index fb0b506b8..5d4c91039 100644 --- a/website/source/docs/commands/plan.html.markdown +++ b/website/source/docs/commands/plan.html.markdown @@ -39,6 +39,10 @@ The command-line flags are all optional. The list of available flags are: * `-input=true` - Ask for input for variables if not directly set. +* `-lock=true` - Lock the state file when locking is supported. + +* `-lock-timeout=0s` - Duration to retry a state lock. + * `-module-depth=n` - Specifies the depth of modules to show in the output. This does not affect the plan itself, only the output shown. By default, this is -1, which will expand all. diff --git a/website/source/docs/commands/refresh.html.markdown b/website/source/docs/commands/refresh.html.markdown index faaff0846..101057762 100644 --- a/website/source/docs/commands/refresh.html.markdown +++ b/website/source/docs/commands/refresh.html.markdown @@ -31,6 +31,14 @@ The command-line flags are all optional. The list of available flags are: * `-no-color` - Disables output with coloring +* `-input=true` - Ask for input for variables if not directly set. + +* `-lock=true` - Lock the state file when locking is supported. + +* `-lock-timeout=0s` - Duration to retry a state lock. + +* `-no-color` - If specified, output won't contain any color. + * `-state=path` - Path to read and write the state file to. Defaults to "terraform.tfstate". Ignored when [remote state](/docs/state/remote.html) is used. diff --git a/website/source/docs/commands/taint.html.markdown b/website/source/docs/commands/taint.html.markdown index 6ec901136..8b6a78655 100644 --- a/website/source/docs/commands/taint.html.markdown +++ b/website/source/docs/commands/taint.html.markdown @@ -47,6 +47,10 @@ The command-line flags are all optional. The list of available flags are: * `-backup=path` - Path to the backup file. Defaults to `-state-out` with the ".backup" extension. Disabled by setting to "-". +* `-lock=true` - Lock the state file when locking is supported. + +* `-lock-timeout=0s` - Duration to retry a state lock. + * `-module=path` - The module path where the resource to taint exists. By default this is the root path. Other modules can be specified by a period-separated list. Example: "foo" would reference the module diff --git a/website/source/docs/commands/untaint.html.markdown b/website/source/docs/commands/untaint.html.markdown index b077098c5..4f74a2722 100644 --- a/website/source/docs/commands/untaint.html.markdown +++ b/website/source/docs/commands/untaint.html.markdown @@ -47,6 +47,10 @@ certain cases, see above note). The list of available flags are: time, there is a maxiumum of one tainted instance per resource, so this flag can be safely omitted. +* `-lock=true` - Lock the state file when locking is supported. + +* `-lock-timeout=0s` - Duration to retry a state lock. + * `-module=path` - The module path where the resource to untaint exists. By default this is the root path. Other modules can be specified by a period-separated list. Example: "foo" would reference the module diff --git a/website/source/intro/getting-started/install.html.markdown b/website/source/intro/getting-started/install.html.markdown index 9ae769aa4..e27fdd2cd 100644 --- a/website/source/intro/getting-started/install.html.markdown +++ b/website/source/intro/getting-started/install.html.markdown @@ -54,8 +54,8 @@ Common commands: apply Builds or changes infrastructure console Interactive console for Terraform interpolations destroy Destroy Terraform-managed infrastructure + env Environment management fmt Rewrites config files to canonical format - force-unlock Manually unlock the terraform state get Download and install modules for the configuration graph Create a visual graph of Terraform resources import Import existing infrastructure into Terraform @@ -72,6 +72,7 @@ Common commands: All other commands: debug Debug output management (experimental) + force-unlock Manually unlock the terraform state state Advanced state management ```