Merge pull request #7305 from fanminshi/return_header_for_timetolive

lease: LeaseTimeToLive returns TTL=-1 resp on lease not found
etcd-io · Feb 13, 2017 · 2925f02 · 2925f02
2 parents 9f8e82e + 0d08ffa
commit 2925f02
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 7 deletions.
diff --git a/clientv3/integration/lease_test.go b/clientv3/integration/lease_test.go
@@ -504,6 +504,42 @@ func TestLeaseTimeToLive(t *testing.T) {
 	}
 }
 
+func TestLeaseTimeToLiveLeaseNotFound(t *testing.T) {
+	defer testutil.AfterTest(t)
+
+	clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1})
+	defer clus.Terminate(t)
+
+	cli := clus.RandClient()
+	resp, err := cli.Grant(context.Background(), 10)
+	if err != nil {
+		t.Errorf("failed to create lease %v", err)
+	}
+	_, err = cli.Revoke(context.Background(), resp.ID)
+	if err != nil {
+		t.Errorf("failed to Revoke lease %v", err)
+	}
+
+	lresp, err := cli.TimeToLive(context.Background(), resp.ID)
+	// TimeToLive() doesn't return LeaseNotFound error
+	// but return a response with TTL to be -1
+	if err != nil {
+		t.Fatalf("expected err to be nil")
+	}
+	if lresp == nil {
+		t.Fatalf("expected lresp not to be nil")
+	}
+	if lresp.ResponseHeader == nil {
+		t.Fatalf("expected ResponseHeader not to be nil")
+	}
+	if lresp.ID != resp.ID {
+		t.Fatalf("expected Lease ID %v, but got %v", resp.ID, lresp.ID)
+	}
+	if lresp.TTL != -1 {
+		t.Fatalf("expected TTL %v, but got %v", lresp.TTL, lresp.TTL)
+	}
+}
+
 // TestLeaseRenewLostQuorum ensures keepalives work after losing quorum
 // for a while.
 func TestLeaseRenewLostQuorum(t *testing.T) {

diff --git a/etcdserver/api/v3rpc/lease.go b/etcdserver/api/v3rpc/lease.go
@@ -53,9 +53,16 @@ func (ls *LeaseServer) LeaseRevoke(ctx context.Context, rr *pb.LeaseRevokeReques
 
 func (ls *LeaseServer) LeaseTimeToLive(ctx context.Context, rr *pb.LeaseTimeToLiveRequest) (*pb.LeaseTimeToLiveResponse, error) {
 	resp, err := ls.le.LeaseTimeToLive(ctx, rr)
-	if err != nil {
+	if err != nil && err != lease.ErrLeaseNotFound {
 		return nil, togRPCError(err)
 	}
+	if err == lease.ErrLeaseNotFound {
+		resp = &pb.LeaseTimeToLiveResponse{
+			Header: &pb.ResponseHeader{},
+			ID:     rr.ID,
+			TTL:    -1,
+		}
+	}
 	ls.hdr.fill(resp.Header)
 	return resp, nil
 }

diff --git a/integration/v3_lease_test.go b/integration/v3_lease_test.go
@@ -334,8 +334,7 @@ func TestV3PutOnNonExistLease(t *testing.T) {
 	}
 }
 
-// TestV3GetNonExistLease tests the case where the non exist lease is report as lease not found error using LeaseTimeToLive()
-// A bug was found when a non leader etcd server returns nil instead of lease not found error which caues the server to crash.
+// TestV3GetNonExistLease ensures client retriving nonexistent lease on a follower doesn't result node panic
 // related issue https://github.com/coreos/etcd/issues/6537
 func TestV3GetNonExistLease(t *testing.T) {
 	defer testutil.AfterTest(t)
@@ -344,16 +343,28 @@ func TestV3GetNonExistLease(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
+	lc := toGRPC(clus.RandClient()).Lease
+	lresp, err := lc.LeaseGrant(ctx, &pb.LeaseGrantRequest{TTL: 10})
+	if err != nil {
+		t.Errorf("failed to create lease %v", err)
+	}
+	_, err = lc.LeaseRevoke(context.TODO(), &pb.LeaseRevokeRequest{ID: lresp.ID})
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	leaseTTLr := &pb.LeaseTimeToLiveRequest{
-		ID:   123,
+		ID:   lresp.ID,
 		Keys: true,
 	}
 
 	for _, client := range clus.clients {
-		_, err := toGRPC(client).Lease.LeaseTimeToLive(ctx, leaseTTLr)
-		if !eqErrGRPC(err, rpctypes.ErrGRPCLeaseNotFound) {
-			t.Errorf("err = %v, want %v", err, rpctypes.ErrGRPCLeaseNotFound)
+		resp, err := toGRPC(client).Lease.LeaseTimeToLive(ctx, leaseTTLr)
+		if err != nil {
+			t.Fatalf("expected non nil error, but go %v", err)
+		}
+		if resp.TTL != -1 {
+			t.Fatalf("expected TTL to be -1, but got %v \n", resp.TTL)
 		}
 	}
 }