From 2ca1823a96105409febb89479d53c6c5e0f83dc2 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Fri, 10 Feb 2017 16:22:30 -0800 Subject: [PATCH 1/3] v3rpc: LeaseTimeToLive returns TTL=-1 resp on lease not found --- etcdserver/api/v3rpc/lease.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/etcdserver/api/v3rpc/lease.go b/etcdserver/api/v3rpc/lease.go index be6e20b97fb..2922b3be946 100644 --- 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 } From bcfbb096e240c8242533b0c539a57e78a1305407 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Fri, 10 Feb 2017 16:25:46 -0800 Subject: [PATCH 2/3] clientv3/integration: test lease not found on TimeToLive() --- clientv3/integration/lease_test.go | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/clientv3/integration/lease_test.go b/clientv3/integration/lease_test.go index 851b1fedd8b..dde8ddd4b38 100644 --- 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) { From 0d08ffa282529ccd0d5e3526df25b9bb9fb18bef Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Fri, 10 Feb 2017 17:35:43 -0800 Subject: [PATCH 3/3] integration: don't expect lease not found error for TestV3GetNonExistLease --- integration/v3_lease_test.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/integration/v3_lease_test.go b/integration/v3_lease_test.go index 6347c0c285a..6c54869c799 100644 --- 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) } } }