diff --git a/backend/remote-state/consul/client.go b/backend/remote-state/consul/client.go index 9bbfb49fa..6173a49e1 100644 --- a/backend/remote-state/consul/client.go +++ b/backend/remote-state/consul/client.go @@ -366,6 +366,11 @@ func (c *RemoteClient) lock() (string, error) { // store the session ID for correlation with consul logs c.info.Info = "consul session: " + lockSession + // A random lock ID has been generated but we override it with the session + // ID as this will make it easier to manually invalidate the session + // if needed. + c.info.ID = lockSession + opts := &consulapi.LockOptions{ Key: c.lockPath() + lockSuffix, Session: lockSession, @@ -524,8 +529,25 @@ func (c *RemoteClient) Unlock(id string) error { // the unlock implementation. // Only to be called while holding Client.mu func (c *RemoteClient) unlock(id string) error { - // this doesn't use the lock id, because the lock is tied to the consul client. + // This method can be called in two circumstances: + // - when the plan apply or destroy operation finishes and the lock needs to be released, + // the watchdog stopped and the session closed + // - when the user calls `terraform force-unlock ` in which case + // we only need to release the lock. + if c.consulLock == nil || c.lockCh == nil { + // The user called `terraform force-unlock `, we just destroy + // the session which will release the lock, clean the KV store and quit. + + _, err := c.Client.Session().Destroy(id, nil) + if err != nil { + return err + } + // We ignore the errors that may happen during cleanup + kv := c.Client.KV() + kv.Delete(c.lockPath()+lockSuffix, nil) + kv.Delete(c.lockPath()+lockInfoSuffix, nil) + return nil } diff --git a/backend/remote-state/consul/client_test.go b/backend/remote-state/consul/client_test.go index a951147dc..42ebbf50a 100644 --- a/backend/remote-state/consul/client_test.go +++ b/backend/remote-state/consul/client_test.go @@ -255,6 +255,17 @@ func TestConsul_destroyLock(t *testing.T) { fmt.Sprintf("tf-unit/%s/", time.Now().String()), } + testLock := func(client *RemoteClient, lockPath string) { + // get the lock val + pair, _, err := client.Client.KV().Get(lockPath, nil) + if err != nil { + t.Fatal(err) + } + if pair != nil { + t.Fatalf("lock key not cleaned up at: %s", pair.Key) + } + } + for _, path := range testCases { t.Run(path, func(*testing.T) { // Get the backend @@ -269,27 +280,50 @@ func TestConsul_destroyLock(t *testing.T) { t.Fatalf("err: %s", err) } - c := s.(*remote.State).Client.(*RemoteClient) + clientA := s.(*remote.State).Client.(*RemoteClient) info := statemgr.NewLockInfo() - id, err := c.Lock(info) + id, err := clientA.Lock(info) if err != nil { t.Fatal(err) } - lockPath := c.Path + lockSuffix + lockPath := clientA.Path + lockSuffix - if err := c.Unlock(id); err != nil { + if err := clientA.Unlock(id); err != nil { t.Fatal(err) } - // get the lock val - pair, _, err := c.Client.KV().Get(lockPath, nil) + testLock(clientA, lockPath) + + // The release the lock from a second client to test the + // `terraform force-unlock ` functionnality + s, err = b.StateMgr(backend.DefaultStateName) + if err != nil { + t.Fatalf("err: %s", err) + } + + clientB := s.(*remote.State).Client.(*RemoteClient) + + info = statemgr.NewLockInfo() + id, err = clientA.Lock(info) if err != nil { t.Fatal(err) } - if pair != nil { - t.Fatalf("lock key not cleaned up at: %s", pair.Key) + + if err := clientB.Unlock(id); err != nil { + t.Fatal(err) + } + + testLock(clientA, lockPath) + + err = clientA.Unlock(id) + + if err == nil { + t.Fatal("consul lock should have been lost") + } + if err.Error() != "consul lock was lost" { + t.Fatal("got wrong error", err) } }) }