cleanup consul lock entries

This matches the consul cli behavior, where locks are cleaned up after
use.

Return an error from re-locking the state. This isn't required by the
Locker interface, but it's an added sanity check for state operations.
What was incorrect here was returning an empty ID and error, which would
indicate that Lock/Unlock isn't supported.

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 {

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)
+	}
+}