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 6b80aac34..d7bf534a1 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" @@ -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 f63735873..42a56eb29 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" @@ -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 c8b23bd32..282e63045 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" ) @@ -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/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/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/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 diff --git a/command/state/state.go b/command/clistate/state.go similarity index 89% rename from command/state/state.go rename to command/clistate/state.go index 3915e8ca2..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,17 +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) { - sl, ok := s.(state.Locker) - if !ok { - return "", nil - } - +// 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 := sl.Lock(info) + id, err := state.LockWithContext(ctx, s, info) lockID = id return err }, func() { @@ -77,13 +74,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/env_delete.go b/command/env_delete.go index be21b76ed..a0fb01fea 100644 --- a/command/env_delete.go +++ b/command/env_delete.go @@ -1,13 +1,13 @@ package command import ( + "context" "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 { @@ -93,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 8b0e8fcdb..84b21bf8b 100644 --- a/command/env_new.go +++ b/command/env_new.go @@ -1,15 +1,15 @@ package command import ( + "context" "fmt" "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 { @@ -88,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/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.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 afcfe533a..415efa02c 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" @@ -16,13 +17,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" ) @@ -166,10 +167,12 @@ 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(), + LockState: m.stateLock, + StateLockTimeout: m.stateLockTimeout, } } @@ -609,15 +612,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) @@ -1040,15 +1048,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() @@ -1132,15 +1145,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() @@ -1288,15 +1306,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 b9133a052..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" @@ -9,7 +10,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" ) @@ -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/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 487099187..e4e443620 100644 --- a/command/taint.go +++ b/command/taint.go @@ -1,11 +1,12 @@ package command import ( + "context" "fmt" "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" ) @@ -28,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 @@ -78,10 +80,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 @@ -188,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/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/command/untaint.go b/command/untaint.go index ab697b823..adce511f7 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -1,11 +1,12 @@ package command import ( + "context" "fmt" "log" "strings" - clistate "github.com/hashicorp/terraform/command/state" + "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/state" ) @@ -27,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 @@ -66,10 +68,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 @@ -176,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). 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 }, 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 { diff --git a/state/inmem.go b/state/inmem.go index ff8daab8f..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" ) @@ -26,3 +30,59 @@ 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 +} + +// 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.go b/state/state.go index 9491958a3..f6c4f16d4 100644 --- a/state/state.go +++ b/state/state.go @@ -2,6 +2,7 @@ package state import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -28,6 +29,7 @@ type State interface { StateWriter StateRefresher StatePersister + Locker } // StateReader is the interface for things that can return a state. Retrieving @@ -72,6 +74,48 @@ 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) { + delay := time.Second + maxDelay := 16 * time.Second + 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) + } + + if postLockHook != nil { + postLockHook() + } + + // 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(delay): + if delay < maxDelay { + delay *= 2 + } + } + } +} + // Generate a LockInfo structure, populating the required fields. func NewLockInfo() *LockInfo { // this doesn't need to be cryptographically secure, just unique. diff --git a/state/state_test.go b/state/state_test.go index e93f5680a..a8fdec6ab 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,59 @@ 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") + } + + // 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) + <-attempted + 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) + } +} 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 ```