From a2d452acec09559fce8a5e3e4b93250e59b2906f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 19 May 2017 14:39:42 -0400 Subject: [PATCH 1/2] failing test to force-unlock a named state in s3 The State call attempts to get a lock before determining if a named state exists. This prevents force-unlock, since we need the state to call Unlock. --- backend/remote-state/s3/client_test.go | 88 ++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 6 deletions(-) diff --git a/backend/remote-state/s3/client_test.go b/backend/remote-state/s3/client_test.go index 0cef7c9edc..9696cd48b2 100644 --- a/backend/remote-state/s3/client_test.go +++ b/backend/remote-state/s3/client_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/hashicorp/terraform/backend" + "github.com/hashicorp/terraform/state" "github.com/hashicorp/terraform/state/remote" ) @@ -16,7 +17,6 @@ func TestRemoteClient_impl(t *testing.T) { func TestRemoteClient(t *testing.T) { testACC(t) - bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix()) keyName := "testState" @@ -26,20 +26,19 @@ func TestRemoteClient(t *testing.T) { "encrypt": true, }).(*Backend) + createS3Bucket(t, b.s3Client, bucketName) + defer deleteS3Bucket(t, b.s3Client, bucketName) + state, err := b.State(backend.DefaultStateName) if err != nil { t.Fatal(err) } - createS3Bucket(t, b.s3Client, bucketName) - defer deleteS3Bucket(t, b.s3Client, bucketName) - remote.TestClient(t, state.(*remote.State).Client) } func TestRemoteClientLocks(t *testing.T) { testACC(t) - bucketName := fmt.Sprintf("terraform-remote-s3-test-%x", time.Now().Unix()) keyName := "testState" @@ -57,6 +56,11 @@ func TestRemoteClientLocks(t *testing.T) { "lock_table": bucketName, }).(*Backend) + createS3Bucket(t, b1.s3Client, bucketName) + defer deleteS3Bucket(t, b1.s3Client, bucketName) + createDynamoDBTable(t, b1.dynClient, bucketName) + defer deleteDynamoDBTable(t, b1.dynClient, bucketName) + s1, err := b1.State(backend.DefaultStateName) if err != nil { t.Fatal(err) @@ -67,10 +71,82 @@ func TestRemoteClientLocks(t *testing.T) { t.Fatal(err) } + remote.TestRemoteLocks(t, s1.(*remote.State).Client, s2.(*remote.State).Client) +} + +// verify that we can unlock a state with an existing lock +func TestForceUnlock(t *testing.T) { + testACC(t) + bucketName := fmt.Sprintf("terraform-remote-s3-test-force-%x", time.Now().Unix()) + keyName := "testState" + + b1 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "bucket": bucketName, + "key": keyName, + "encrypt": true, + "lock_table": bucketName, + }).(*Backend) + + b2 := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "bucket": bucketName, + "key": keyName, + "encrypt": true, + "lock_table": bucketName, + }).(*Backend) + createS3Bucket(t, b1.s3Client, bucketName) defer deleteS3Bucket(t, b1.s3Client, bucketName) createDynamoDBTable(t, b1.dynClient, bucketName) defer deleteDynamoDBTable(t, b1.dynClient, bucketName) - remote.TestRemoteLocks(t, s1.(*remote.State).Client, s2.(*remote.State).Client) + // first test with default + s1, err := b1.State(backend.DefaultStateName) + if err != nil { + t.Fatal(err) + } + + info := state.NewLockInfo() + info.Operation = "test" + info.Who = "clientA" + + lockID, err := s1.Lock(info) + if err != nil { + t.Fatal("unable to get initial lock:", err) + } + + // s1 is now locked, get the same state through s2 and unlock it + s2, err := b2.State(backend.DefaultStateName) + if err != nil { + t.Fatal("failed to get default state to force unlock:", err) + } + + if err := s2.Unlock(lockID); err != nil { + t.Fatal("failed to force-unlock default state") + } + + // now try the same thing with a named state + // first test with default + s1, err = b1.State("test") + if err != nil { + t.Fatal(err) + } + + info = state.NewLockInfo() + info.Operation = "test" + info.Who = "clientA" + + lockID, err = s1.Lock(info) + if err != nil { + t.Fatal("unable to get initial lock:", err) + } + + // s1 is now locked, get the same state through s2 and unlock it + s2, err = b2.State("test") + if err != nil { + t.Fatal("failed to get named state to force unlock:", err) + } + + if err = s2.Unlock(lockID); err != nil { + t.Fatal("failed to force-unlock named state") + } } From b279b1abb57885bb74d8318cf4d10edfc8014b3a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 19 May 2017 14:40:59 -0400 Subject: [PATCH 2/2] check for named s3 states before acquiring a lock In order to force-unlock a named state, we have to fetch that state first. Don't attempt to acquire a lock if we know the state already exists in s3. --- backend/remote-state/s3/backend_state.go | 28 +++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/backend/remote-state/s3/backend_state.go b/backend/remote-state/s3/backend_state.go index f7a4d337de..e35a11884a 100644 --- a/backend/remote-state/s3/backend_state.go +++ b/backend/remote-state/s3/backend_state.go @@ -101,9 +101,29 @@ func (b *Backend) State(name string) (state.State, error) { stateMgr := &remote.State{Client: client} - //if this isn't the default state name, we need to create the object so - //it's listed by States. - if name != backend.DefaultStateName { + // Check to see if this state already exists. + // If we're trying to force-unlock a state, we can't take the lock before + // fetching the state. If the state doesn't exist, we have to assume this + // is a normal create operation, and take the lock at that point. + // + // If we need to force-unlock, but for some reason the state no longer + // exists, the user will have to use aws tools to manually fix the + // situation. + existing, err := b.States() + if err != nil { + return nil, err + } + + exists := false + for _, s := range existing { + if s == name { + exists = true + break + } + } + + // We need to create the object so it's listed by States. + if !exists { // take a lock on this state while we write it lockInfo := state.NewLockInfo() lockInfo.Operation = "init" @@ -121,6 +141,8 @@ func (b *Backend) State(name string) (state.State, error) { } // Grab the value + // This is to ensure that no one beat us to writing a state between + // the `exists` check and taking the lock. if err := stateMgr.RefreshState(); err != nil { err = lockUnlock(err) return nil, err