server: Cover V3 health with tests

Signed-off-by: Siyuan Zhang <sizhang@google.com>

server/embed/etcd.go

@@ -747,7 +747,7 @@ func (e *Etcd) serveClients() (err error) {
 	} else {
 		mux := http.NewServeMux()
 		etcdhttp.HandleBasic(e.cfg.logger, mux, e.Server)
-		etcdhttp.HandleMetricsHealthForV3(e.cfg.logger, mux, e.Server)
+		etcdhttp.HandleMetricsHealth(e.cfg.logger, mux, e.Server)
 		h = mux
 	}
 
@@ -836,7 +836,7 @@ func (e *Etcd) serveMetrics() (err error) {
 
 	if len(e.cfg.ListenMetricsUrls) > 0 {
 		metricsMux := http.NewServeMux()
-		etcdhttp.HandleMetricsHealthForV3(e.cfg.logger, metricsMux, e.Server)
+		etcdhttp.HandleMetricsHealth(e.cfg.logger, metricsMux, e.Server)
 
 		for _, murl := range e.cfg.ListenMetricsUrls {
 			tlsInfo := &e.cfg.ClientTLSInfo

server/etcdserver/api/etcdhttp/metrics.go

@@ -24,8 +24,11 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"go.etcd.io/etcd/api/v3/etcdserverpb"
+	pb "go.etcd.io/etcd/api/v3/etcdserverpb"
+	"go.etcd.io/etcd/client/pkg/v3/types"
 	"go.etcd.io/etcd/raft/v3"
 	"go.etcd.io/etcd/server/v3/auth"
+	"go.etcd.io/etcd/server/v3/config"
 	"go.etcd.io/etcd/server/v3/etcdserver"
 	"go.uber.org/zap"
 )
@@ -37,8 +40,19 @@ const (
 	PathProxyHealth  = "/proxy/health"
 )
 
-// HandleMetricsHealth registers metrics and health handlers.
+type ServerHealth interface {
-func HandleMetricsHealth(lg *zap.Logger, mux *http.ServeMux, srv etcdserver.ServerV2) {
+	serverHealthV2V3
+	Range(context.Context, *pb.RangeRequest) (*pb.RangeResponse, error)
+	Config() config.ServerConfig
+}
+
+type serverHealthV2V3 interface {
+	Alarms() []*pb.AlarmMember
+	Leader() types.ID
+}
+
+// HandleMetricsHealthForV2 registers metrics and health handlers for v2.
+func HandleMetricsHealthForV2(lg *zap.Logger, mux *http.ServeMux, srv etcdserver.ServerV2) {
 	mux.Handle(PathMetrics, promhttp.Handler())
 	mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool) Health {
 		if h := checkAlarms(lg, srv, excludedAlarms); h.Health != "true" {
@@ -51,9 +65,9 @@ func HandleMetricsHealth(lg *zap.Logger, mux *http.ServeMux, srv etcdserver.Serv
 	}))
 }
 
-// HandleMetricsHealthForV3 registers metrics and health handlers. it checks health by using v3 range request
+// HandleMetricsHealth registers metrics and health handlers. it checks health by using v3 range request
 // and its corresponding timeout.
-func HandleMetricsHealthForV3(lg *zap.Logger, mux *http.ServeMux, srv *etcdserver.EtcdServer) {
+func HandleMetricsHealth(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) {
 	mux.Handle(PathMetrics, promhttp.Handler())
 	mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool) Health {
 		if h := checkAlarms(lg, srv, excludedAlarms); h.Health != "true" {
@@ -62,7 +76,7 @@ func HandleMetricsHealthForV3(lg *zap.Logger, mux *http.ServeMux, srv *etcdserve
 		if h := checkLeader(lg, srv, serializable); h.Health != "true" {
 			return h
 		}
-		return checkV3API(lg, srv, serializable)
+		return checkAPI(lg, srv, serializable)
 	}))
 }
 
@@ -155,7 +169,7 @@ func getSerializableFlag(r *http.Request) bool {
 
 // TODO: etcdserver.ErrNoLeader in health API
 
-func checkAlarms(lg *zap.Logger, srv etcdserver.ServerV2, excludedAlarms AlarmSet) Health {
+func checkAlarms(lg *zap.Logger, srv serverHealthV2V3, excludedAlarms AlarmSet) Health {
 	h := Health{Health: "true"}
 	as := srv.Alarms()
 	if len(as) > 0 {
@@ -168,9 +182,9 @@ func checkAlarms(lg *zap.Logger, srv etcdserver.ServerV2, excludedAlarms AlarmSe
 
 			h.Health = "false"
 			switch v.Alarm {
-			case etcdserverpb.AlarmType_NOSPACE:
+			case pb.AlarmType_NOSPACE:
 				h.Reason = "ALARM NOSPACE"
-			case etcdserverpb.AlarmType_CORRUPT:
+			case pb.AlarmType_CORRUPT:
 				h.Reason = "ALARM CORRUPT"
 			default:
 				h.Reason = "ALARM UNKNOWN"
@@ -183,7 +197,7 @@ func checkAlarms(lg *zap.Logger, srv etcdserver.ServerV2, excludedAlarms AlarmSe
 	return h
 }
 
-func checkLeader(lg *zap.Logger, srv etcdserver.ServerV2, serializable bool) Health {
+func checkLeader(lg *zap.Logger, srv serverHealthV2V3, serializable bool) Health {
 	h := Health{Health: "true"}
 	if !serializable && (uint64(srv.Leader()) == raft.None) {
 		h.Health = "false"
@@ -208,10 +222,11 @@ func checkV2API(lg *zap.Logger, srv etcdserver.ServerV2) Health {
 	return h
 }
 
-func checkV3API(lg *zap.Logger, srv *etcdserver.EtcdServer, serializable bool) Health {
+func checkAPI(lg *zap.Logger, srv ServerHealth, serializable bool) Health {
 	h := Health{Health: "true"}
-	ctx, cancel := context.WithTimeout(context.Background(), srv.Cfg.ReqTimeout())
+	cfg := srv.Config()
-	_, err := srv.Range(ctx, &etcdserverpb.RangeRequest{KeysOnly: true, Limit: 1, Serializable: serializable})
+	ctx, cancel := context.WithTimeout(context.Background(), cfg.ReqTimeout())
+	_, err := srv.Range(ctx, &pb.RangeRequest{KeysOnly: true, Limit: 1, Serializable: serializable})
 	cancel()
 	if err != nil && err != auth.ErrUserEmpty && err != auth.ErrPermissionDenied {
 		h.Health = "false"

server/etcdserver/api/etcdhttp/metrics_test.go

@@ -14,9 +14,11 @@ import (
 	"go.etcd.io/etcd/client/pkg/v3/testutil"
 	"go.etcd.io/etcd/client/pkg/v3/types"
 	"go.etcd.io/etcd/raft/v3"
+	"go.etcd.io/etcd/server/v3/auth"
+	"go.etcd.io/etcd/server/v3/config"
 	"go.etcd.io/etcd/server/v3/etcdserver"
 	stats "go.etcd.io/etcd/server/v3/etcdserver/api/v2stats"
-	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest"
 )
 
 type fakeStats struct{}
@@ -25,25 +27,34 @@ func (s *fakeStats) SelfStats() []byte   { return nil }
 func (s *fakeStats) LeaderStats() []byte { return nil }
 func (s *fakeStats) StoreStats() []byte  { return nil }
 
-type fakeServerV2 struct {
+type fakeHealthServer struct {
 	fakeServer
 	stats.Stats
-	health string
+	health   string
+	apiError error
 }
 
-func (s *fakeServerV2) Leader() types.ID {
+func (s *fakeHealthServer) Range(ctx context.Context, request *pb.RangeRequest) (*pb.RangeResponse, error) {
+	return nil, s.apiError
+}
+
+func (s *fakeHealthServer) Config() config.ServerConfig {
+	return config.ServerConfig{}
+}
+
+func (s *fakeHealthServer) Leader() types.ID {
 	if s.health == "true" {
 		return 1
 	}
 	return types.ID(raft.None)
 }
-func (s *fakeServerV2) Do(ctx context.Context, r pb.Request) (etcdserver.Response, error) {
+func (s *fakeHealthServer) Do(ctx context.Context, r pb.Request) (etcdserver.Response, error) {
 	if s.health == "true" {
 		return etcdserver.Response{}, nil
 	}
 	return etcdserver.Response{}, fmt.Errorf("fail health check")
 }
-func (s *fakeServerV2) ClientCertAuthEnabled() bool { return false }
+func (s *fakeHealthServer) ClientCertAuthEnabled() bool { return false }
 
 func TestHealthHandler(t *testing.T) {
 	// define the input and expected output
@@ -52,6 +63,7 @@ func TestHealthHandler(t *testing.T) {
 		name           string
 		alarms         []*pb.AlarmMember
 		healthCheckURL string
+		apiError       error
 
 		expectStatusCode int
 		expectHealth     string
@@ -105,15 +117,34 @@ func TestHealthHandler(t *testing.T) {
 			expectStatusCode: http.StatusOK,
 			expectHealth:     "true",
 		},
+		{
+			healthCheckURL:   "/health",
+			apiError:         auth.ErrUserEmpty,
+			expectStatusCode: http.StatusOK,
+			expectHealth:     "true",
+		},
+		{
+			healthCheckURL:   "/health",
+			apiError:         auth.ErrPermissionDenied,
+			expectStatusCode: http.StatusOK,
+			expectHealth:     "true",
+		},
+		{
+			healthCheckURL:   "/health",
+			apiError:         fmt.Errorf("Unexpected error"),
+			expectStatusCode: http.StatusServiceUnavailable,
+			expectHealth:     "false",
+		},
 	}
 
 	for i, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			mux := http.NewServeMux()
-			HandleMetricsHealth(zap.NewExample(), mux, &fakeServerV2{
+			HandleMetricsHealth(zaptest.NewLogger(t), mux, &fakeHealthServer{
 				fakeServer: fakeServer{alarms: tt.alarms},
 				Stats:      &fakeStats{},
 				health:     tt.expectHealth,
+				apiError:   tt.apiError,
 			})
 			ts := httptest.NewServer(mux)
 			defer ts.Close()

server/etcdserver/api/v2http/client.go

@@ -58,7 +58,7 @@ func NewClientHandler(lg *zap.Logger, server etcdserver.ServerPeer, timeout time
 	}
 	mux := http.NewServeMux()
 	etcdhttp.HandleBasic(lg, mux, server)
-	etcdhttp.HandleMetricsHealth(lg, mux, server)
+	etcdhttp.HandleMetricsHealthForV2(lg, mux, server)
 	handleV2(lg, mux, server, timeout)
 	return requestLogger(lg, mux)
 }

server/etcdserver/server.go

@@ -724,6 +724,10 @@ func (s *EtcdServer) Logger() *zap.Logger {
 	return l
 }
 
+func (s *EtcdServer) Config() config.ServerConfig {
+	return s.Cfg
+}
+
 func tickToDur(ticks int, tickMs uint) string {
 	return fmt.Sprintf("%v", time.Duration(ticks)*time.Duration(tickMs)*time.Millisecond)
 }