From e1f1b84d67da081fa8f8dbad1dceacdff7588711 Mon Sep 17 00:00:00 2001 From: Sean Teeling Date: Mon, 12 Oct 2020 21:45:25 -0700 Subject: [PATCH 1/3] Refresh state outside of grabbing the lock; only grab the lock on provisioning if the state file doesn't exist; this is similar to the GCS backend --- backend/remote-state/azure/backend_state.go | 25 +++++++++------------ 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/backend/remote-state/azure/backend_state.go b/backend/remote-state/azure/backend_state.go index e7d316287..b794bc32e 100644 --- a/backend/remote-state/azure/backend_state.go +++ b/backend/remote-state/azure/backend_state.go @@ -95,9 +95,13 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { stateMgr := &remote.State{Client: client} + // Grab the value + if err := stateMgr.RefreshState(); err != nil { + return nil, err + } //if this isn't the default state name, we need to create the object so //it's listed by States. - if name != backend.DefaultStateName { + if v := stateMgr.State(); v == nil { // take a lock on this state while we write it lockInfo := statemgr.NewLockInfo() lockInfo.Operation = "init" @@ -114,29 +118,20 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { return parent } - // Grab the value - if err := stateMgr.RefreshState(); err != nil { + // If we have no state, we have to create an empty state + if err := stateMgr.WriteState(states.NewState()); err != nil { err = lockUnlock(err) return nil, err } - - // If we have no state, we have to create an empty state - if v := stateMgr.State(); v == nil { - if err := stateMgr.WriteState(states.NewState()); err != nil { - err = lockUnlock(err) - return nil, err - } - if err := stateMgr.PersistState(); err != nil { - err = lockUnlock(err) - return nil, err - } + if err := stateMgr.PersistState(); err != nil { + err = lockUnlock(err) + return nil, err } // Unlock, the state should now be initialized if err := lockUnlock(nil); err != nil { return nil, err } - } return stateMgr, nil From 7d6ec431d2b4ade7ef9bbbf1212e5448e0a2a505 Mon Sep 17 00:00:00 2001 From: Sean Teeling Date: Tue, 13 Oct 2020 08:18:54 -0700 Subject: [PATCH 2/3] test locks in non-default workspace --- backend/remote-state/azure/backend_test.go | 6 ++++++ backend/testing.go | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/backend/remote-state/azure/backend_test.go b/backend/remote-state/azure/backend_test.go index 3f0a28534..037a2d83a 100644 --- a/backend/remote-state/azure/backend_test.go +++ b/backend/remote-state/azure/backend_test.go @@ -258,6 +258,9 @@ func TestBackendAccessKeyLocked(t *testing.T) { backend.TestBackendStateLocks(t, b1, b2) backend.TestBackendStateForceUnlock(t, b1, b2) + + backend.TestBackendStateLocksInWS(t, b1, b2, "foo") + backend.TestBackendStateForceUnlockInWS(t, b1, b2, "foo") } func TestBackendServicePrincipalLocked(t *testing.T) { @@ -301,4 +304,7 @@ func TestBackendServicePrincipalLocked(t *testing.T) { backend.TestBackendStateLocks(t, b1, b2) backend.TestBackendStateForceUnlock(t, b1, b2) + + backend.TestBackendStateLocksInWS(t, b1, b2, "foo") + backend.TestBackendStateForceUnlockInWS(t, b1, b2, "foo") } diff --git a/backend/testing.go b/backend/testing.go index b1d78ea61..1e0fd3b11 100644 --- a/backend/testing.go +++ b/backend/testing.go @@ -279,7 +279,27 @@ func TestBackendStateForceUnlock(t *testing.T, b1, b2 Backend) { testLocks(t, b1, b2, true) } +// TestBackendStateLocksInWS will test the locking functionality of the remote +// state backend. +func TestBackendStateLocksInWS(t *testing.T, b1, b2 Backend, ws string) { + t.Helper() + testLocksInWorkspace(t, b1, b2, false, ws) +} + +// TestBackendStateForceUnlockInWS verifies that the lock error is the expected +// type, and the lock can be unlocked using the ID reported in the error. +// Remote state backends that support -force-unlock should call this in at +// least one of the acceptance tests. +func TestBackendStateForceUnlockInWS(t *testing.T, b1, b2 Backend, ws string) { + t.Helper() + testLocksInWorkspace(t, b1, b2, true, ws) +} + func testLocks(t *testing.T, b1, b2 Backend, testForceUnlock bool) { + testLocksInWorkspace(t, b1, b2, testForceUnlock, DefaultStateName) +} + +func testLocksInWorkspace(t *testing.T, b1, b2 Backend, testForceUnlock bool, workspace string) { t.Helper() // Get the default state for each From 8980b6dc9e7ea5fa293fa5cf5b4150ec1ff592f4 Mon Sep 17 00:00:00 2001 From: Sean Teeling Date: Wed, 14 Oct 2020 20:25:41 -0700 Subject: [PATCH 3/3] double check the state wasn't created in the short time prior to grabbing the lock --- backend/remote-state/azure/backend_state.go | 27 ++++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/backend/remote-state/azure/backend_state.go b/backend/remote-state/azure/backend_state.go index b794bc32e..9017690e3 100644 --- a/backend/remote-state/azure/backend_state.go +++ b/backend/remote-state/azure/backend_state.go @@ -118,19 +118,28 @@ func (b *Backend) StateMgr(name string) (statemgr.Full, error) { return parent } - // If we have no state, we have to create an empty state - if err := stateMgr.WriteState(states.NewState()); err != nil { - err = lockUnlock(err) - return nil, err - } - if err := stateMgr.PersistState(); err != nil { + // Grab the value + if err := stateMgr.RefreshState(); err != nil { err = lockUnlock(err) return nil, err } + //if this isn't the default state name, we need to create the object so + //it's listed by States. + if v := stateMgr.State(); v == nil { + // If we have no state, we have to create an empty state + if err := stateMgr.WriteState(states.NewState()); err != nil { + err = lockUnlock(err) + return nil, err + } + if err := stateMgr.PersistState(); err != nil { + err = lockUnlock(err) + return nil, err + } - // Unlock, the state should now be initialized - if err := lockUnlock(nil); err != nil { - return nil, err + // Unlock, the state should now be initialized + if err := lockUnlock(nil); err != nil { + return nil, err + } } }