diff --git a/command/meta_backend.go b/command/meta_backend.go index 1bee95339c..726f3b70cc 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -583,11 +583,14 @@ func (m *Meta) backendFromPlan(opts *BackendOpts) (backend.Backend, error) { } if m.stateLock { - stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) - if err := stateLocker.Lock(realMgr, "backend from plan"); err != nil { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, realMgr, "backend from plan", "", m.Ui, m.Colorize()) + if err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer stateLocker.Unlock(nil) + defer unlock(nil) } if err := realMgr.RefreshState(); err != nil { @@ -957,11 +960,14 @@ func (m *Meta) backend_C_r_s( } if m.stateLock { - stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) - if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize()) + if err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer stateLocker.Unlock(nil) + defer unlock(nil) } // Store the metadata in our saved state location @@ -1033,11 +1039,14 @@ func (m *Meta) backend_C_r_S_changed( } if m.stateLock { - stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) - if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize()) + if err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer stateLocker.Unlock(nil) + defer unlock(nil) } // Update the backend state @@ -1169,11 +1178,14 @@ func (m *Meta) backend_C_R_S_unchanged( } if m.stateLock { - stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) - if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, sMgr, "backend from config", "", m.Ui, m.Colorize()) + if err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer stateLocker.Unlock(nil) + defer unlock(nil) } // Unset the remote state diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index 0c3610a8a1..be20a44768 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -233,19 +233,20 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { } if m.stateLock { - lockCtx := context.Background() + lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) + defer cancel() - lockerOne := clistate.NewLocker(lockCtx, m.stateLockTimeout, m.Ui, m.Colorize()) - if err := lockerOne.Lock(stateOne, "migration source state"); err != nil { + unlockOne, err := clistate.Lock(lockCtx, stateOne, "migration", "source state", m.Ui, m.Colorize()) + if err != nil { return fmt.Errorf("Error locking source state: %s", err) } - defer lockerOne.Unlock(nil) + defer unlockOne(nil) - lockerTwo := clistate.NewLocker(lockCtx, m.stateLockTimeout, m.Ui, m.Colorize()) - if err := lockerTwo.Lock(stateTwo, "migration destination state"); err != nil { + unlockTwo, err := clistate.Lock(lockCtx, stateTwo, "migration", "destination state", m.Ui, m.Colorize()) + if err != nil { return fmt.Errorf("Error locking destination state: %s", err) } - defer lockerTwo.Unlock(nil) + defer unlockTwo(nil) // We now own a lock, so double check that we have the version // corresponding to the lock. diff --git a/command/taint.go b/command/taint.go index 4dc16e60cd..c3d6c4cc99 100644 --- a/command/taint.go +++ b/command/taint.go @@ -83,12 +83,16 @@ func (c *TaintCommand) Run(args []string) int { } if c.stateLock { - stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) - if err := stateLocker.Lock(st, "taint"); err != nil { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, st, "taint", "", c.Ui, c.Colorize()) + if err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - defer stateLocker.Unlock(nil) + + defer unlock(nil) } // Get the actual state structure diff --git a/command/untaint.go b/command/untaint.go index 39d047fd67..9a2e43b246 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -71,12 +71,15 @@ func (c *UntaintCommand) Run(args []string) int { } if c.stateLock { - stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) - if err := stateLocker.Lock(st, "untaint"); err != nil { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, st, "untaint", "", c.Ui, c.Colorize()) + if err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - defer stateLocker.Unlock(nil) + defer unlock(nil) } // Get the actual state structure diff --git a/command/workspace_delete.go b/command/workspace_delete.go index 1e1eb1182f..8a9f792e1f 100644 --- a/command/workspace_delete.go +++ b/command/workspace_delete.go @@ -96,17 +96,6 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { return 1 } - var stateLocker clistate.Locker - if c.stateLock { - stateLocker = clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) - if err := stateLocker.Lock(sMgr, "workspace_delete"); err != nil { - c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) - return 1 - } - } else { - stateLocker = clistate.NewNoopLocker() - } - if err := sMgr.RefreshState(); err != nil { c.Ui.Error(err.Error()) return 1 @@ -119,16 +108,28 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { return 1 } - // We need to release the lock just before deleting the state, in case - // the backend can't remove the resource while holding the lock. This - // is currently true for Windows local files. - // - // TODO: While there is little safety in locking while deleting the - // state, it might be nice to be able to coordinate processes around - // state deletion, i.e. in a CI environment. Adding Delete() as a - // required method of States would allow the removal of the resource to - // be delegated from the Backend to the State itself. - stateLocker.Unlock(nil) + // Honor the lock request, for consistency and one final safety check. + if c.stateLock { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, sMgr, "workspace delete", "", c.Ui, c.Colorize()) + if err != nil { + c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) + return 1 + } + + // We need to release the lock just before deleting the state, in case + // the backend can't remove the resource while holding the lock. This + // is currently true for Windows local files. + // + // TODO: While there is little safety in locking while deleting the + // state, it might be nice to be able to coordinate processes around + // state deletion, i.e. in a CI environment. Adding Delete() as a + // required method of States would allow the removal of the resource to + // be delegated from the Backend to the State itself. + unlock(nil) + } err = b.DeleteState(delEnv) if err != nil { diff --git a/command/workspace_new.go b/command/workspace_new.go index 810a776fc4..430ecc58b5 100644 --- a/command/workspace_new.go +++ b/command/workspace_new.go @@ -114,12 +114,15 @@ func (c *WorkspaceNewCommand) Run(args []string) int { } if c.stateLock { - stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) - if err := stateLocker.Lock(sMgr, "workspace_delete"); err != nil { + lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) + defer cancel() + + unlock, err := clistate.Lock(lockCtx, sMgr, "workspace_new", "", c.Ui, c.Colorize()) + if err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - defer stateLocker.Unlock(nil) + defer unlock(nil) } // read the existing state file