diff --git a/backend/remote-state/consul/client.go b/backend/remote-state/consul/client.go index cd5971163..b11f31ba1 100644 --- a/backend/remote-state/consul/client.go +++ b/backend/remote-state/consul/client.go @@ -121,16 +121,15 @@ func (c *RemoteClient) Lock(info *state.LockInfo) (string, error) { default: if c.lockCh != nil { // we have an active lock already - return "", nil + return "", fmt.Errorf("state %q already locked", c.Path) } } if c.consulLock == nil { opts := &consulapi.LockOptions{ Key: c.Path + lockSuffix, - // We currently don't procide any options to block terraform and - // retry lock acquisition, but we can wait briefly in case the - // lock is about to be freed. + // only wait briefly, so terraform has the choice to fail fast or + // retry as needed. LockWaitTime: time.Second, LockTryOnce: true, } @@ -191,6 +190,10 @@ func (c *RemoteClient) Unlock(id string) error { err := c.consulLock.Unlock() c.lockCh = nil + // This is only cleanup, and will fail if the lock was immediately taken by + // another client, so we don't report an error to the user here. + c.consulLock.Destroy() + kv := c.Client.KV() _, delErr := kv.Delete(c.Path+lockInfoSuffix, nil) if delErr != nil { diff --git a/backend/remote-state/consul/client_test.go b/backend/remote-state/consul/client_test.go index 57b7c452e..dc7988135 100644 --- a/backend/remote-state/consul/client_test.go +++ b/backend/remote-state/consul/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" ) @@ -98,3 +99,43 @@ func TestConsul_stateLock(t *testing.T) { remote.TestRemoteLocks(t, sA.(*remote.State).Client, sB.(*remote.State).Client) } + +func TestConsul_destroyLock(t *testing.T) { + srv := newConsulTestServer(t) + defer srv.Stop() + + // Get the backend + b := backend.TestBackendConfig(t, New(), map[string]interface{}{ + "address": srv.HTTPAddr, + "path": fmt.Sprintf("tf-unit/%s", time.Now().String()), + }) + + // Grab the client + s, err := b.State(backend.DefaultStateName) + if err != nil { + t.Fatalf("err: %s", err) + } + + c := s.(*remote.State).Client.(*RemoteClient) + + info := state.NewLockInfo() + id, err := c.Lock(info) + if err != nil { + t.Fatal(err) + } + + lockPath := c.Path + lockSuffix + + if err := c.Unlock(id); err != nil { + t.Fatal(err) + } + + // get the lock val + pair, _, err := c.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) + } +}