From 8ef66870183d1fa72594bd97c426e9ddf5883504 Mon Sep 17 00:00:00 2001 From: fanmin shi Date: Tue, 27 Sep 2016 15:32:06 -0700 Subject: [PATCH] etcdserver: fix a node panic bug caused LeaseTimeToLive call on a nonexistent lease When the non Leader etcd server receives a LeaseTimeToLive on a nonexistent lease, it responds with a nil resp and a nil error The invoking function parses the nil resp and results a segmentation fault. I fix the bug by making sure the lease not found error is returned so that the invoking function parses the the error message instead. fix #6537 --- etcdserver/v3_server.go | 6 ++---- integration/v3_lease_test.go | 26 +++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/etcdserver/v3_server.go b/etcdserver/v3_server.go index fb6e3aebb..7fd5b6bfd 100644 --- a/etcdserver/v3_server.go +++ b/etcdserver/v3_server.go @@ -308,17 +308,15 @@ func (s *EtcdServer) LeaseTimeToLive(ctx context.Context, r *pb.LeaseTimeToLiveR return nil, err } - var lresp *pb.LeaseTimeToLiveResponse for _, url := range leader.PeerURLs { lurl := url + leasehttp.LeaseInternalPrefix var iresp *leasepb.LeaseInternalResponse iresp, err = leasehttp.TimeToLiveHTTP(ctx, lease.LeaseID(r.ID), r.Keys, lurl, s.peerRt) if err == nil { - lresp = iresp.LeaseTimeToLiveResponse - break + return iresp.LeaseTimeToLiveResponse, nil } } - return lresp, nil + return nil, err } func (s *EtcdServer) waitLeader() (*membership.Member, error) { diff --git a/integration/v3_lease_test.go b/integration/v3_lease_test.go index ffde1980f..f3ce69ba3 100644 --- a/integration/v3_lease_test.go +++ b/integration/v3_lease_test.go @@ -243,7 +243,31 @@ func TestV3PutOnNonExistLease(t *testing.T) { putr := &pb.PutRequest{Key: []byte("foo"), Value: []byte("bar"), Lease: badLeaseID} _, err := toGRPC(clus.RandClient()).KV.Put(ctx, putr) if !eqErrGRPC(err, rpctypes.ErrGRPCLeaseNotFound) { - t.Errorf("err = %v, want %v", err, rpctypes.ErrGRPCCompacted) + t.Errorf("err = %v, want %v", err, rpctypes.ErrGRPCLeaseNotFound) + } +} + +// 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. +// related issue https://github.com/coreos/etcd/issues/6537 +func TestV3GetNonExistLease(t *testing.T) { + defer testutil.AfterTest(t) + clus := NewClusterV3(t, &ClusterConfig{Size: 3}) + defer clus.Terminate(t) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + leaseTTLr := &pb.LeaseTimeToLiveRequest{ + ID: 123, + 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) + } } }