From 400f6ca4c540aee8cc4ffc17e2d6c2ce5d4959d5 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 23 Feb 2018 11:28:47 -0500 Subject: [PATCH] use the new clistate.Locker in commands Use the new StateLocker field to provide a wrapper for locking the state during terraform.Context creation in the commands that directly manipulate the state. --- command/meta_backend.go | 36 +++++++++------------------ command/meta_backend_migrate.go | 15 ++++++------ command/taint.go | 10 +++----- command/untaint.go | 9 +++---- command/workspace_delete.go | 43 ++++++++++++++++----------------- command/workspace_new.go | 9 +++---- 6 files changed, 49 insertions(+), 73 deletions(-) diff --git a/command/meta_backend.go b/command/meta_backend.go index 726f3b70c..1bee95339 100644 --- a/command/meta_backend.go +++ b/command/meta_backend.go @@ -583,14 +583,11 @@ func (m *Meta) backendFromPlan(opts *BackendOpts) (backend.Backend, error) { } if m.stateLock { - 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 { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(realMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer unlock(nil) + defer stateLocker.Unlock(nil) } if err := realMgr.RefreshState(); err != nil { @@ -960,14 +957,11 @@ func (m *Meta) backend_C_r_s( } if m.stateLock { - 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 { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer unlock(nil) + defer stateLocker.Unlock(nil) } // Store the metadata in our saved state location @@ -1039,14 +1033,11 @@ func (m *Meta) backend_C_r_S_changed( } if m.stateLock { - 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 { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer unlock(nil) + defer stateLocker.Unlock(nil) } // Update the backend state @@ -1178,14 +1169,11 @@ func (m *Meta) backend_C_R_S_unchanged( } if m.stateLock { - 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 { + stateLocker := clistate.NewLocker(context.Background(), m.stateLockTimeout, m.Ui, m.Colorize()) + if err := stateLocker.Lock(sMgr, "backend from plan"); err != nil { return nil, fmt.Errorf("Error locking state: %s", err) } - defer unlock(nil) + defer stateLocker.Unlock(nil) } // Unset the remote state diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index be20a4476..0c3610a8a 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -233,20 +233,19 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { } if m.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) - defer cancel() + lockCtx := context.Background() - unlockOne, err := clistate.Lock(lockCtx, stateOne, "migration", "source state", m.Ui, m.Colorize()) - if err != nil { + lockerOne := clistate.NewLocker(lockCtx, m.stateLockTimeout, m.Ui, m.Colorize()) + if err := lockerOne.Lock(stateOne, "migration source state"); err != nil { return fmt.Errorf("Error locking source state: %s", err) } - defer unlockOne(nil) + defer lockerOne.Unlock(nil) - unlockTwo, err := clistate.Lock(lockCtx, stateTwo, "migration", "destination state", m.Ui, m.Colorize()) - if err != nil { + lockerTwo := clistate.NewLocker(lockCtx, m.stateLockTimeout, m.Ui, m.Colorize()) + if err := lockerTwo.Lock(stateTwo, "migration destination state"); err != nil { return fmt.Errorf("Error locking destination state: %s", err) } - defer unlockTwo(nil) + defer lockerTwo.Unlock(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 c3d6c4cc9..4dc16e60c 100644 --- a/command/taint.go +++ b/command/taint.go @@ -83,16 +83,12 @@ func (c *TaintCommand) Run(args []string) int { } if c.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) - defer cancel() - - unlock, err := clistate.Lock(lockCtx, st, "taint", "", c.Ui, c.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) + if err := stateLocker.Lock(st, "taint"); err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - - defer unlock(nil) + defer stateLocker.Unlock(nil) } // Get the actual state structure diff --git a/command/untaint.go b/command/untaint.go index 9a2e43b24..39d047fd6 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -71,15 +71,12 @@ func (c *UntaintCommand) Run(args []string) int { } if c.stateLock { - lockCtx, cancel := context.WithTimeout(context.Background(), c.stateLockTimeout) - defer cancel() - - unlock, err := clistate.Lock(lockCtx, st, "untaint", "", c.Ui, c.Colorize()) - if err != nil { + stateLocker := clistate.NewLocker(context.Background(), c.stateLockTimeout, c.Ui, c.Colorize()) + if err := stateLocker.Lock(st, "untaint"); err != nil { c.Ui.Error(fmt.Sprintf("Error locking state: %s", err)) return 1 } - defer unlock(nil) + defer stateLocker.Unlock(nil) } // Get the actual state structure diff --git a/command/workspace_delete.go b/command/workspace_delete.go index 8a9f792e1..1e1eb1182 100644 --- a/command/workspace_delete.go +++ b/command/workspace_delete.go @@ -96,6 +96,17 @@ 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 @@ -108,28 +119,16 @@ func (c *WorkspaceDeleteCommand) Run(args []string) int { return 1 } - // 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) - } + // 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) err = b.DeleteState(delEnv) if err != nil { diff --git a/command/workspace_new.go b/command/workspace_new.go index 430ecc58b..810a776fc 100644 --- a/command/workspace_new.go +++ b/command/workspace_new.go @@ -114,15 +114,12 @@ func (c *WorkspaceNewCommand) Run(args []string) int { } if c.stateLock { - 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 { + 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 } - defer unlock(nil) + defer stateLocker.Unlock(nil) } // read the existing state file