Merge pull request #8429 from heyitsanthony/leasing-no-acquire-ttl

leasing: don't acquire lease on ttl'd keys
etcd-io · Aug 21, 2017 · 65c0540 · 65c0540
2 parents 0bf4046 + 5c03ade
commit 65c0540
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 2 deletions.
diff --git a/clientv3/integration/leasing_test.go b/clientv3/integration/leasing_test.go
@@ -190,6 +190,34 @@ func TestLeasingPutInvalidatExisting(t *testing.T) {
 	}
 }
 
+// TestLeasingGetLease checks that keys with TTLs are not leased.
+func TestLeasingGetNoLeaseTTL(t *testing.T) {
+	defer testutil.AfterTest(t)
+	clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1})
+	defer clus.Terminate(t)
+
+	lkv, closeLKV, err := leasing.NewKV(clus.Client(0), "pfx/")
+	testutil.AssertNil(t, err)
+	defer closeLKV()
+
+	lresp, err := clus.Client(0).Grant(context.TODO(), 60)
+	testutil.AssertNil(t, err)
+
+	_, err = clus.Client(0).Put(context.TODO(), "k", "v", clientv3.WithLease(lresp.ID))
+	testutil.AssertNil(t, err)
+
+	gresp, err := lkv.Get(context.TODO(), "k")
+	testutil.AssertNil(t, err)
+	testutil.AssertEqual(t, len(gresp.Kvs), 1)
+
+	clus.Members[0].Stop(t)
+
+	ctx, cancel := context.WithTimeout(context.TODO(), time.Second)
+	_, err = lkv.Get(ctx, "k")
+	cancel()
+	testutil.AssertEqual(t, err, ctx.Err())
+}
+
 // TestLeasingGetSerializable checks the leasing KV can make serialized requests
 // when the etcd cluster is partitioned.
 func TestLeasingGetSerializable(t *testing.T) {

diff --git a/clientv3/leasing/kv.go b/clientv3/leasing/kv.go
@@ -22,6 +22,7 @@ import (
 	v3 "github.com/coreos/etcd/clientv3"
 	"github.com/coreos/etcd/clientv3/concurrency"
 	"github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes"
+	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
 	"github.com/coreos/etcd/mvcc/mvccpb"
 
 	"golang.org/x/net/context"
@@ -261,8 +262,10 @@ func (lkv *leasingKV) acquire(ctx context.Context, key string, op v3.Op) (*v3.Tx
 		if err := lkv.waitSession(ctx); err != nil {
 			return nil, err
 		}
+		lcmp := v3.Cmp{Key: []byte(key), Target: pb.Compare_LEASE}
 		resp, err := lkv.kv.Txn(ctx).If(
-			v3.Compare(v3.CreateRevision(lkv.pfx+key), "=", 0)).
+			v3.Compare(v3.CreateRevision(lkv.pfx+key), "=", 0),
+			v3.Compare(lcmp, "=", 0)).
 			Then(
 				op,
 				v3.OpPut(lkv.pfx+key, "", v3.WithLease(lkv.leaseID()))).
@@ -274,7 +277,7 @@ func (lkv *leasingKV) acquire(ctx context.Context, key string, op v3.Op) (*v3.Tx
 			if !resp.Succeeded {
 				kvs := resp.Responses[1].GetResponseRange().Kvs
 				// if txn failed since already owner, lease is acquired
-				resp.Succeeded = v3.LeaseID(kvs[0].Lease) == lkv.leaseID()
+				resp.Succeeded = len(kvs) > 0 && v3.LeaseID(kvs[0].Lease) == lkv.leaseID()
 			}
 			return resp, nil
 		}