Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lease: LeaseTimeToLive returns TTL=-1 resp on lease not found #7305

Merged
merged 3 commits into from
Feb 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions clientv3/integration/lease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 8 additions & 1 deletion etcdserver/api/v3rpc/lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
23 changes: 17 additions & 6 deletions integration/v3_lease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
}
Expand Down