Merge pull request #16697 from chaochn47/health_check_bug_fix

http health check bug fixes
This commit is contained in:
Benjamin Wang 2023-10-13 05:48:24 +01:00 committed by GitHub
commit bc0f724ae4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 29 deletions

View File

@ -40,24 +40,25 @@ type ServerHealth interface {
Leader() types.ID Leader() types.ID
Range(context.Context, *pb.RangeRequest) (*pb.RangeResponse, error) Range(context.Context, *pb.RangeRequest) (*pb.RangeResponse, error)
Config() config.ServerConfig Config() config.ServerConfig
AuthStore() auth.AuthStore
} }
// HandleHealth registers metrics and health handlers. it checks health by using v3 range request // HandleHealth registers metrics and health handlers. it checks health by using v3 range request
// and its corresponding timeout. // and its corresponding timeout.
func HandleHealth(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) { func HandleHealth(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) {
mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool) Health { mux.Handle(PathHealth, NewHealthHandler(lg, func(ctx context.Context, excludedAlarms AlarmSet, serializable bool) Health {
if h := checkAlarms(lg, srv, excludedAlarms); h.Health != "true" { if h := checkAlarms(lg, srv, excludedAlarms); h.Health != "true" {
return h return h
} }
if h := checkLeader(lg, srv, serializable); h.Health != "true" { if h := checkLeader(lg, srv, serializable); h.Health != "true" {
return h return h
} }
return checkAPI(lg, srv, serializable) return checkAPI(ctx, lg, srv, serializable)
})) }))
} }
// NewHealthHandler handles '/health' requests. // NewHealthHandler handles '/health' requests.
func NewHealthHandler(lg *zap.Logger, hfunc func(excludedAlarms AlarmSet, Serializable bool) Health) http.HandlerFunc { func NewHealthHandler(lg *zap.Logger, hfunc func(ctx context.Context, excludedAlarms AlarmSet, Serializable bool) Health) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet { if r.Method != http.MethodGet {
w.Header().Set("Allow", http.MethodGet) w.Header().Set("Allow", http.MethodGet)
@ -71,7 +72,7 @@ func NewHealthHandler(lg *zap.Logger, hfunc func(excludedAlarms AlarmSet, Serial
// This is useful for probes attempting to validate the liveness of // This is useful for probes attempting to validate the liveness of
// the etcd process vs readiness of the cluster to serve requests. // the etcd process vs readiness of the cluster to serve requests.
serializableFlag := getSerializableFlag(r) serializableFlag := getSerializableFlag(r)
h := hfunc(excludedAlarms, serializableFlag) h := hfunc(r.Context(), excludedAlarms, serializableFlag)
defer func() { defer func() {
if h.Health == "true" { if h.Health == "true" {
healthSuccess.Inc() healthSuccess.Inc()
@ -178,13 +179,14 @@ func checkLeader(lg *zap.Logger, srv ServerHealth, serializable bool) Health {
return h return h
} }
func checkAPI(lg *zap.Logger, srv ServerHealth, serializable bool) Health { func checkAPI(ctx context.Context, lg *zap.Logger, srv ServerHealth, serializable bool) Health {
h := Health{Health: "true"} h := Health{Health: "true"}
cfg := srv.Config() cfg := srv.Config()
ctx, cancel := context.WithTimeout(context.Background(), cfg.ReqTimeout()) ctx = srv.AuthStore().WithRoot(ctx)
_, err := srv.Range(ctx, &pb.RangeRequest{KeysOnly: true, Limit: 1, Serializable: serializable}) cctx, cancel := context.WithTimeout(ctx, cfg.ReqTimeout())
_, err := srv.Range(cctx, &pb.RangeRequest{KeysOnly: true, Limit: 1, Serializable: serializable})
cancel() cancel()
if err != nil && err != auth.ErrUserEmpty && err != auth.ErrPermissionDenied { if err != nil {
h.Health = "false" h.Health = "false"
h.Reason = fmt.Sprintf("RANGE ERROR:%s", err) h.Reason = fmt.Sprintf("RANGE ERROR:%s", err)
lg.Warn("serving /health false; Range fails", zap.Error(err)) lg.Warn("serving /health false; Range fails", zap.Error(err))

View File

@ -25,22 +25,26 @@ import (
"go.uber.org/zap/zaptest" "go.uber.org/zap/zaptest"
"go.etcd.io/raft/v3"
pb "go.etcd.io/etcd/api/v3/etcdserverpb" pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/client/pkg/v3/testutil" "go.etcd.io/etcd/client/pkg/v3/testutil"
"go.etcd.io/etcd/client/pkg/v3/types" "go.etcd.io/etcd/client/pkg/v3/types"
"go.etcd.io/etcd/server/v3/auth" "go.etcd.io/etcd/server/v3/auth"
"go.etcd.io/etcd/server/v3/config" "go.etcd.io/etcd/server/v3/config"
"go.etcd.io/etcd/server/v3/etcdserver" "go.etcd.io/etcd/server/v3/etcdserver"
"go.etcd.io/raft/v3" betesting "go.etcd.io/etcd/server/v3/storage/backend/testing"
"go.etcd.io/etcd/server/v3/storage/schema"
) )
type fakeHealthServer struct { type fakeHealthServer struct {
fakeServer fakeServer
health string health string
apiError error apiError error
authStore auth.AuthStore
} }
func (s *fakeHealthServer) Range(ctx context.Context, request *pb.RangeRequest) (*pb.RangeResponse, error) { func (s *fakeHealthServer) Range(_ context.Context, _ *pb.RangeRequest) (*pb.RangeResponse, error) {
return nil, s.apiError return nil, s.apiError
} }
@ -54,12 +58,13 @@ func (s *fakeHealthServer) Leader() types.ID {
} }
return types.ID(raft.None) return types.ID(raft.None)
} }
func (s *fakeHealthServer) Do(ctx context.Context, r pb.Request) (etcdserver.Response, error) { func (s *fakeHealthServer) Do(_ context.Context, _ pb.Request) (etcdserver.Response, error) {
if s.health == "true" { if s.health == "true" {
return etcdserver.Response{}, nil return etcdserver.Response{}, nil
} }
return etcdserver.Response{}, fmt.Errorf("fail health check") return etcdserver.Response{}, fmt.Errorf("fail health check")
} }
func (s *fakeHealthServer) AuthStore() auth.AuthStore { return s.authStore }
func (s *fakeHealthServer) ClientCertAuthEnabled() bool { return false } func (s *fakeHealthServer) ClientCertAuthEnabled() bool { return false }
func TestHealthHandler(t *testing.T) { func TestHealthHandler(t *testing.T) {
@ -123,20 +128,6 @@ func TestHealthHandler(t *testing.T) {
expectStatusCode: http.StatusOK, expectStatusCode: http.StatusOK,
expectHealth: "true", expectHealth: "true",
}, },
{
name: "Healthy even if authentication failed",
healthCheckURL: "/health",
apiError: auth.ErrUserEmpty,
expectStatusCode: http.StatusOK,
expectHealth: "true",
},
{
name: "Healthy even if authorization failed",
healthCheckURL: "/health",
apiError: auth.ErrPermissionDenied,
expectStatusCode: http.StatusOK,
expectHealth: "true",
},
{ {
name: "Unhealthy if api is not available", name: "Unhealthy if api is not available",
healthCheckURL: "/health", healthCheckURL: "/health",
@ -149,10 +140,14 @@ func TestHealthHandler(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
mux := http.NewServeMux() mux := http.NewServeMux()
lg := zaptest.NewLogger(t)
be, _ := betesting.NewDefaultTmpBackend(t)
defer betesting.Close(t, be)
HandleHealth(zaptest.NewLogger(t), mux, &fakeHealthServer{ HandleHealth(zaptest.NewLogger(t), mux, &fakeHealthServer{
fakeServer: fakeServer{alarms: tt.alarms}, fakeServer: fakeServer{alarms: tt.alarms},
health: tt.expectHealth, health: tt.expectHealth,
apiError: tt.apiError, apiError: tt.apiError,
authStore: auth.NewAuthStore(lg, schema.NewAuthBackend(lg, be), nil, 0),
}) })
ts := httptest.NewServer(mux) ts := httptest.NewServer(mux)
defer ts.Close() defer ts.Close()

View File

@ -32,7 +32,9 @@ func HandleHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) {
if lg == nil { if lg == nil {
lg = zap.NewNop() lg = zap.NewNop()
} }
mux.Handle(etcdhttp.PathHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health { return checkHealth(c) })) mux.Handle(etcdhttp.PathHealth, etcdhttp.NewHealthHandler(lg, func(ctx context.Context, excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health {
return checkHealth(c)
}))
} }
// HandleProxyHealth registers health handler on '/proxy/health'. // HandleProxyHealth registers health handler on '/proxy/health'.
@ -40,7 +42,9 @@ func HandleProxyHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) {
if lg == nil { if lg == nil {
lg = zap.NewNop() lg = zap.NewNop()
} }
mux.Handle(etcdhttp.PathProxyHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health { return checkProxyHealth(c) })) mux.Handle(etcdhttp.PathProxyHealth, etcdhttp.NewHealthHandler(lg, func(ctx context.Context, excludedAlarms etcdhttp.AlarmSet, serializable bool) etcdhttp.Health {
return checkProxyHealth(c)
}))
} }
func checkHealth(c *clientv3.Client) etcdhttp.Health { func checkHealth(c *clientv3.Client) etcdhttp.Health {