From 22f940ce922eda3cd51ef7ddbdcbbc9c56a59001 Mon Sep 17 00:00:00 2001 From: Han Kang Date: Mon, 5 Jun 2023 10:04:25 -0700 Subject: [PATCH 1/5] add livez/readyz for etcd Change-Id: Ia440a82b2bf3d275b7cd7d88b5a6e86fe9fe1c28 Signed-off-by: Han Kang Change-Id: Ief9475a92429be58eb7b1f96246bbdb00e996e75 --- server/embed/etcd.go | 4 + server/etcdserver/api/etcdhttp/health.go | 49 +++- server/etcdserver/api/etcdhttp/health_test.go | 219 +++++++++++++++++- tests/framework/integration/cluster.go | 2 + 4 files changed, 263 insertions(+), 11 deletions(-) diff --git a/server/embed/etcd.go b/server/embed/etcd.go index 7d30562bc58..8f783d30d28 100644 --- a/server/embed/etcd.go +++ b/server/embed/etcd.go @@ -744,6 +744,8 @@ func (e *Etcd) serveClients() (err error) { etcdhttp.HandleVersion(mux, e.Server) etcdhttp.HandleMetrics(mux) etcdhttp.HandleHealth(e.cfg.logger, mux, e.Server) + etcdhttp.HandleLivez(e.cfg.logger, mux, e.Server) + etcdhttp.HandleReadyz(e.cfg.logger, mux, e.Server) var gopts []grpc.ServerOption if e.cfg.GRPCKeepAliveMinTime > time.Duration(0) { @@ -831,6 +833,8 @@ func (e *Etcd) serveMetrics() (err error) { metricsMux := http.NewServeMux() etcdhttp.HandleMetrics(metricsMux) etcdhttp.HandleHealth(e.cfg.logger, metricsMux, e.Server) + etcdhttp.HandleLivez(e.cfg.logger, metricsMux, e.Server) + etcdhttp.HandleReadyz(e.cfg.logger, metricsMux, e.Server) for _, murl := range e.cfg.ListenMetricsUrls { tlsInfo := &e.cfg.ClientTLSInfo diff --git a/server/etcdserver/api/etcdhttp/health.go b/server/etcdserver/api/etcdhttp/health.go index 95950de8961..74224c1e4e0 100644 --- a/server/etcdserver/api/etcdhttp/health.go +++ b/server/etcdserver/api/etcdhttp/health.go @@ -23,16 +23,19 @@ import ( "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" + "go.etcd.io/raft/v3" + "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/server/v3/auth" "go.etcd.io/etcd/server/v3/config" - "go.etcd.io/raft/v3" ) const ( PathHealth = "/health" + PathLivez = "/livez" + PathReadyz = "/readyz" PathProxyHealth = "/proxy/health" ) @@ -46,33 +49,59 @@ type ServerHealth interface { // HandleHealth registers metrics and health handlers. it checks health by using v3 range request // and its corresponding timeout. func HandleHealth(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) { - mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool) Health { - if h := checkAlarms(lg, srv, excludedAlarms); h.Health != "true" { + mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool, endpoint string) Health { + if h := checkAlarms(lg, srv, excludedAlarms, endpoint); h.Health != "true" { return h } if h := checkLeader(lg, srv, serializable); h.Health != "true" { return h } return checkAPI(lg, srv, serializable) - })) + }, PathHealth)) +} + +// HandleLivez registers metrics and health handlers. it checks health by using v3 range request +// and its corresponding timeout. +func HandleLivez(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) { + mux.Handle(PathLivez, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool, endpoint string) Health { + if h := checkAlarms(lg, srv, excludedAlarms, endpoint); h.Health != "true" { + return h + } + // TODO(logicalhan) should we require quorum for livez? + return checkAPI(lg, srv, serializable) + }, PathLivez, []string{etcdserverpb.AlarmType_NOSPACE.String()}...)) +} + +// HandleReadyz registers metrics and health handlers. it checks health by using v3 range request +// and its corresponding timeout. +func HandleReadyz(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) { + mux.Handle(PathReadyz, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool, endpoint string) Health { + if h := checkAlarms(lg, srv, excludedAlarms, endpoint); h.Health != "true" { + return h + } + return checkAPI(lg, srv, serializable) + }, PathReadyz)) } // 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(excludedAlarms AlarmSet, Serializable bool, endpoint string) Health, endpoint string, alwaysExclude ...string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { w.Header().Set("Allow", http.MethodGet) http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed) - lg.Warn("/health error", zap.Int("status-code", http.StatusMethodNotAllowed)) + lg.Warn(fmt.Sprintf("%s error", endpoint), zap.Int("status-code", http.StatusMethodNotAllowed)) return } excludedAlarms := getExcludedAlarms(r) + for _, additionalExcludes := range alwaysExclude { + excludedAlarms[additionalExcludes] = struct{}{} + } // Passing the query parameter "serializable=true" ensures that the // health of the local etcd is checked vs the health of the cluster. // This is useful for probes attempting to validate the liveness of // the etcd process vs readiness of the cluster to serve requests. serializableFlag := getSerializableFlag(r) - h := hfunc(excludedAlarms, serializableFlag) + h := hfunc(excludedAlarms, serializableFlag, endpoint) defer func() { if h.Health == "true" { healthSuccess.Inc() @@ -83,12 +112,12 @@ func NewHealthHandler(lg *zap.Logger, hfunc func(excludedAlarms AlarmSet, Serial d, _ := json.Marshal(h) if h.Health != "true" { http.Error(w, string(d), http.StatusServiceUnavailable) - lg.Warn("/health error", zap.String("output", string(d)), zap.Int("status-code", http.StatusServiceUnavailable)) + lg.Warn(fmt.Sprintf("%s error", endpoint), zap.String("output", string(d)), zap.Int("status-code", http.StatusServiceUnavailable)) return } w.WriteHeader(http.StatusOK) w.Write(d) - lg.Debug("/health OK", zap.Int("status-code", http.StatusOK)) + lg.Debug(fmt.Sprintf("%s ok", endpoint), zap.Int("status-code", http.StatusOK)) } } @@ -141,7 +170,7 @@ func getSerializableFlag(r *http.Request) bool { // TODO: etcdserver.ErrNoLeader in health API -func checkAlarms(lg *zap.Logger, srv ServerHealth, excludedAlarms AlarmSet) Health { +func checkAlarms(lg *zap.Logger, srv ServerHealth, excludedAlarms AlarmSet, healthType string) Health { h := Health{Health: "true"} as := srv.Alarms() if len(as) > 0 { diff --git a/server/etcdserver/api/etcdhttp/health_test.go b/server/etcdserver/api/etcdhttp/health_test.go index 29172b5ee5c..53644154a0e 100644 --- a/server/etcdserver/api/etcdhttp/health_test.go +++ b/server/etcdserver/api/etcdhttp/health_test.go @@ -25,13 +25,14 @@ import ( "go.uber.org/zap/zaptest" + "go.etcd.io/raft/v3" + pb "go.etcd.io/etcd/api/v3/etcdserverpb" "go.etcd.io/etcd/client/pkg/v3/testutil" "go.etcd.io/etcd/client/pkg/v3/types" "go.etcd.io/etcd/server/v3/auth" "go.etcd.io/etcd/server/v3/config" "go.etcd.io/etcd/server/v3/etcdserver" - "go.etcd.io/raft/v3" ) type fakeStats struct{} @@ -185,9 +186,225 @@ func TestHealthHandler(t *testing.T) { } } +func TestReadyzHandler(t *testing.T) { + // define the input and expected output + // input: alarms, and healthCheckURL + tests := []struct { + name string + alarms []*pb.AlarmMember + healthCheckURL string + apiError error + + expectStatusCode int + expectHealth string + }{ + { + name: "Healthy if no alarm", + alarms: []*pb.AlarmMember{}, + healthCheckURL: "/readyz", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Unhealthy if NOSPACE alarm is on", + alarms: []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}}, + healthCheckURL: "/readyz", + expectStatusCode: http.StatusServiceUnavailable, + expectHealth: "false", + }, + { + name: "Healthy if NOSPACE alarm is on and excluded", + alarms: []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}}, + healthCheckURL: "/readyz?exclude=NOSPACE", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Healthy if NOSPACE alarm is excluded", + alarms: []*pb.AlarmMember{}, + healthCheckURL: "/readyz?exclude=NOSPACE", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Healthy if multiple NOSPACE alarms are on and excluded", + alarms: []*pb.AlarmMember{{MemberID: uint64(1), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(2), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(3), Alarm: pb.AlarmType_NOSPACE}}, + healthCheckURL: "/readyz?exclude=NOSPACE", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Unhealthy if NOSPACE alarms is excluded and CORRUPT is on", + alarms: []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(1), Alarm: pb.AlarmType_CORRUPT}}, + healthCheckURL: "/readyz?exclude=NOSPACE", + expectStatusCode: http.StatusServiceUnavailable, + expectHealth: "false", + }, + { + name: "Unhealthy if both NOSPACE and CORRUPT are on and excluded", + alarms: []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(1), Alarm: pb.AlarmType_CORRUPT}}, + healthCheckURL: "/readyz?exclude=NOSPACE&exclude=CORRUPT", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Healthy even if authentication failed", + healthCheckURL: "/readyz", + apiError: auth.ErrUserEmpty, + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Healthy even if authorization failed", + healthCheckURL: "/readyz", + apiError: auth.ErrPermissionDenied, + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Unhealthy if api is not available", + healthCheckURL: "/readyz", + apiError: fmt.Errorf("Unexpected error"), + expectStatusCode: http.StatusServiceUnavailable, + expectHealth: "false", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mux := http.NewServeMux() + HandleReadyz(zaptest.NewLogger(t), mux, &fakeHealthServer{ + fakeServer: fakeServer{alarms: tt.alarms}, + health: tt.expectHealth, + apiError: tt.apiError, + }) + ts := httptest.NewServer(mux) + defer ts.Close() + + res, err := ts.Client().Do(&http.Request{Method: http.MethodGet, URL: testutil.MustNewURL(t, ts.URL+tt.healthCheckURL)}) + if err != nil { + t.Errorf("fail serve http request %s %v", tt.healthCheckURL, err) + } + if res == nil { + t.Errorf("got nil http response with http request %s", tt.healthCheckURL) + return + } + if res.StatusCode != tt.expectStatusCode { + t.Errorf("want statusCode %d but got %d", tt.expectStatusCode, res.StatusCode) + } + health, err := parseHealthOutput(res.Body) + if err != nil { + t.Errorf("fail parse health check output %v", err) + } + if health.Health != tt.expectHealth { + t.Errorf("want health %s but got %s", tt.expectHealth, health.Health) + } + }) + } +} + +func TestLivezHandler(t *testing.T) { + // define the input and expected output + // input: alarms, and healthCheckURL + tests := []struct { + name string + alarms []*pb.AlarmMember + healthCheckURL string + apiError error + + expectStatusCode int + expectHealth string + }{ + { + name: "Healthy if no alarm", + alarms: []*pb.AlarmMember{}, + healthCheckURL: "/livez", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Unhealthy if NOSPACE alarm is on", + alarms: []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}}, + healthCheckURL: "/livez", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Unhealthy if NOSPACE alarms is excluded and CORRUPT is on", + alarms: []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(1), Alarm: pb.AlarmType_CORRUPT}}, + healthCheckURL: "/livez?exclude=NOSPACE", + expectStatusCode: http.StatusServiceUnavailable, + expectHealth: "false", + }, + { + name: "Unhealthy if both NOSPACE and CORRUPT are on and excluded", + alarms: []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(1), Alarm: pb.AlarmType_CORRUPT}}, + healthCheckURL: "/livez?exclude=NOSPACE&exclude=CORRUPT", + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Healthy even if authentication failed", + healthCheckURL: "/livez", + apiError: auth.ErrUserEmpty, + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Healthy even if authorization failed", + healthCheckURL: "/livez", + apiError: auth.ErrPermissionDenied, + expectStatusCode: http.StatusOK, + expectHealth: "true", + }, + { + name: "Unhealthy if api is not available", + healthCheckURL: "/livez", + apiError: fmt.Errorf("Unexpected error"), + expectStatusCode: http.StatusServiceUnavailable, + expectHealth: "false", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mux := http.NewServeMux() + HandleLivez(zaptest.NewLogger(t), mux, &fakeHealthServer{ + fakeServer: fakeServer{alarms: tt.alarms}, + health: tt.expectHealth, + apiError: tt.apiError, + }) + ts := httptest.NewServer(mux) + defer ts.Close() + + res, err := ts.Client().Do(&http.Request{Method: http.MethodGet, URL: testutil.MustNewURL(t, ts.URL+tt.healthCheckURL)}) + if err != nil { + t.Errorf("fail serve http request %s %v", tt.healthCheckURL, err) + } + if res == nil { + t.Errorf("got nil http response with http request %s", tt.healthCheckURL) + return + } + if res.StatusCode != tt.expectStatusCode { + t.Errorf("want statusCode %d but got %d", tt.expectStatusCode, res.StatusCode) + } + + health, err := parseHealthOutput(res.Body) + + if err != nil { + t.Errorf("fail parse health check output %v", err) + } + if health.Health != tt.expectHealth { + t.Errorf("want health %s but got %s", tt.expectHealth, health.Health) + } + }) + } +} + func parseHealthOutput(body io.Reader) (Health, error) { obj := Health{} d, derr := io.ReadAll(body) + println(string(d)) if derr != nil { return obj, derr } diff --git a/tests/framework/integration/cluster.go b/tests/framework/integration/cluster.go index c023528047b..d7a6024bccb 100644 --- a/tests/framework/integration/cluster.go +++ b/tests/framework/integration/cluster.go @@ -1043,6 +1043,8 @@ func (m *Member) Launch() error { etcdhttp.HandleVersion(handler, m.Server) etcdhttp.HandleMetrics(handler) etcdhttp.HandleHealth(m.Logger, handler, m.Server) + etcdhttp.HandleLivez(m.Logger, handler, m.Server) + etcdhttp.HandleReadyz(m.Logger, handler, m.Server) hs := &httptest.Server{ Listener: ln, Config: &http.Server{ From 73db456d9fc255ca6d269c6d821bbcdc27e904ad Mon Sep 17 00:00:00 2001 From: Han Kang Date: Mon, 5 Jun 2023 10:23:48 -0700 Subject: [PATCH 2/5] refactor tests to basically use the same logic Change-Id: Ie699ae11d0ecc315b91365f85f0ac0b2d339c28d Signed-off-by: Han Kang Change-Id: Iee6f469f63cb1fbcc22a4d633a621b7915a1a799 --- server/etcdserver/api/etcdhttp/health_test.go | 138 ++++++------------ 1 file changed, 45 insertions(+), 93 deletions(-) diff --git a/server/etcdserver/api/etcdhttp/health_test.go b/server/etcdserver/api/etcdhttp/health_test.go index 53644154a0e..f86595d38ed 100644 --- a/server/etcdserver/api/etcdhttp/health_test.go +++ b/server/etcdserver/api/etcdhttp/health_test.go @@ -23,6 +23,7 @@ import ( "net/http/httptest" "testing" + "go.uber.org/zap" "go.uber.org/zap/zaptest" "go.etcd.io/raft/v3" @@ -69,18 +70,20 @@ func (s *fakeHealthServer) Do(ctx context.Context, r pb.Request) (etcdserver.Res } func (s *fakeHealthServer) ClientCertAuthEnabled() bool { return false } +type testcase struct { + name string + alarms []*pb.AlarmMember + healthCheckURL string + apiError error + + expectStatusCode int + expectHealth string +} + func TestHealthHandler(t *testing.T) { // define the input and expected output // input: alarms, and healthCheckURL - tests := []struct { - name string - alarms []*pb.AlarmMember - healthCheckURL string - apiError error - - expectStatusCode int - expectHealth string - }{ + tests := []testcase{ { name: "Healthy if no alarm", alarms: []*pb.AlarmMember{}, @@ -155,37 +158,41 @@ func TestHealthHandler(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mux := http.NewServeMux() - HandleHealth(zaptest.NewLogger(t), mux, &fakeHealthServer{ - fakeServer: fakeServer{alarms: tt.alarms}, - health: tt.expectHealth, - apiError: tt.apiError, - }) - ts := httptest.NewServer(mux) - defer ts.Close() - - res, err := ts.Client().Do(&http.Request{Method: http.MethodGet, URL: testutil.MustNewURL(t, ts.URL+tt.healthCheckURL)}) - if err != nil { - t.Errorf("fail serve http request %s %v", tt.healthCheckURL, err) - } - if res == nil { - t.Errorf("got nil http response with http request %s", tt.healthCheckURL) - return - } - if res.StatusCode != tt.expectStatusCode { - t.Errorf("want statusCode %d but got %d", tt.expectStatusCode, res.StatusCode) - } - health, err := parseHealthOutput(res.Body) - if err != nil { - t.Errorf("fail parse health check output %v", err) - } - if health.Health != tt.expectHealth { - t.Errorf("want health %s but got %s", tt.expectHealth, health.Health) - } + testHealth(t, tt, HandleHealth) }) } } +func testHealth(t *testing.T, tt testcase, handleFunc func(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth)) { + mux := http.NewServeMux() + handleFunc(zaptest.NewLogger(t), mux, &fakeHealthServer{ + fakeServer: fakeServer{alarms: tt.alarms}, + health: tt.expectHealth, + apiError: tt.apiError, + }) + ts := httptest.NewServer(mux) + defer ts.Close() + + res, err := ts.Client().Do(&http.Request{Method: http.MethodGet, URL: testutil.MustNewURL(t, ts.URL+tt.healthCheckURL)}) + if err != nil { + t.Errorf("fail serve http request %s %v", tt.healthCheckURL, err) + } + if res == nil { + t.Errorf("got nil http response with http request %s", tt.healthCheckURL) + return + } + if res.StatusCode != tt.expectStatusCode { + t.Errorf("want statusCode %d but got %d", tt.expectStatusCode, res.StatusCode) + } + health, err := parseHealthOutput(res.Body) + if err != nil { + t.Errorf("fail parse health check output %v", err) + } + if health.Health != tt.expectHealth { + t.Errorf("want health %s but got %s", tt.expectHealth, health.Health) + } +} + func TestReadyzHandler(t *testing.T) { // define the input and expected output // input: alarms, and healthCheckURL @@ -272,33 +279,7 @@ func TestReadyzHandler(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mux := http.NewServeMux() - HandleReadyz(zaptest.NewLogger(t), mux, &fakeHealthServer{ - fakeServer: fakeServer{alarms: tt.alarms}, - health: tt.expectHealth, - apiError: tt.apiError, - }) - ts := httptest.NewServer(mux) - defer ts.Close() - - res, err := ts.Client().Do(&http.Request{Method: http.MethodGet, URL: testutil.MustNewURL(t, ts.URL+tt.healthCheckURL)}) - if err != nil { - t.Errorf("fail serve http request %s %v", tt.healthCheckURL, err) - } - if res == nil { - t.Errorf("got nil http response with http request %s", tt.healthCheckURL) - return - } - if res.StatusCode != tt.expectStatusCode { - t.Errorf("want statusCode %d but got %d", tt.expectStatusCode, res.StatusCode) - } - health, err := parseHealthOutput(res.Body) - if err != nil { - t.Errorf("fail parse health check output %v", err) - } - if health.Health != tt.expectHealth { - t.Errorf("want health %s but got %s", tt.expectHealth, health.Health) - } + testHealth(t, tt, HandleReadyz) }) } } @@ -368,35 +349,7 @@ func TestLivezHandler(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mux := http.NewServeMux() - HandleLivez(zaptest.NewLogger(t), mux, &fakeHealthServer{ - fakeServer: fakeServer{alarms: tt.alarms}, - health: tt.expectHealth, - apiError: tt.apiError, - }) - ts := httptest.NewServer(mux) - defer ts.Close() - - res, err := ts.Client().Do(&http.Request{Method: http.MethodGet, URL: testutil.MustNewURL(t, ts.URL+tt.healthCheckURL)}) - if err != nil { - t.Errorf("fail serve http request %s %v", tt.healthCheckURL, err) - } - if res == nil { - t.Errorf("got nil http response with http request %s", tt.healthCheckURL) - return - } - if res.StatusCode != tt.expectStatusCode { - t.Errorf("want statusCode %d but got %d", tt.expectStatusCode, res.StatusCode) - } - - health, err := parseHealthOutput(res.Body) - - if err != nil { - t.Errorf("fail parse health check output %v", err) - } - if health.Health != tt.expectHealth { - t.Errorf("want health %s but got %s", tt.expectHealth, health.Health) - } + testHealth(t, tt, HandleLivez) }) } } @@ -404,7 +357,6 @@ func TestLivezHandler(t *testing.T) { func parseHealthOutput(body io.Reader) (Health, error) { obj := Health{} d, derr := io.ReadAll(body) - println(string(d)) if derr != nil { return obj, derr } From 28b8b9f3a8419d1b1813b68d27a8e52b9e574754 Mon Sep 17 00:00:00 2001 From: Han Kang Date: Mon, 5 Jun 2023 16:33:45 -0700 Subject: [PATCH 3/5] fix broken grpc handler Signed-off-by: Han Kang Change-Id: I7e95be58cff6b7bc47fa3114249074a9f69a1620 --- server/proxy/grpcproxy/health.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/proxy/grpcproxy/health.go b/server/proxy/grpcproxy/health.go index ec9781bfe27..fc217e14d50 100644 --- a/server/proxy/grpcproxy/health.go +++ b/server/proxy/grpcproxy/health.go @@ -32,7 +32,8 @@ func HandleHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) { if lg == nil { 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(excludedAlarms etcdhttp.AlarmSet, serializable bool, endpoint string) etcdhttp.Health { return checkHealth(c) }, etcdhttp.PathHealth)) } // HandleProxyHealth registers health handler on '/proxy/health'. @@ -40,7 +41,7 @@ func HandleProxyHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) { if lg == nil { 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(excludedAlarms etcdhttp.AlarmSet, serializable bool, endpoint string) etcdhttp.Health { return checkProxyHealth(c) }, etcdhttp.PathProxyMetrics)) } func checkHealth(c *clientv3.Client) etcdhttp.Health { From 4c3e52c6e5bcc82500c060d5cbdd6e1b77584fec Mon Sep 17 00:00:00 2001 From: Han Kang Date: Mon, 5 Jun 2023 16:47:19 -0700 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Benjamin Wang Signed-off-by: Han Kang Change-Id: I45fda0a8ee7d80638af96fee4efb3bfdf2aebaf8 --- server/etcdserver/api/etcdhttp/health.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/etcdserver/api/etcdhttp/health.go b/server/etcdserver/api/etcdhttp/health.go index 74224c1e4e0..66222241651 100644 --- a/server/etcdserver/api/etcdhttp/health.go +++ b/server/etcdserver/api/etcdhttp/health.go @@ -67,8 +67,7 @@ func HandleLivez(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) { if h := checkAlarms(lg, srv, excludedAlarms, endpoint); h.Health != "true" { return h } - // TODO(logicalhan) should we require quorum for livez? - return checkAPI(lg, srv, serializable) + return checkAPI(lg, srv, true) }, PathLivez, []string{etcdserverpb.AlarmType_NOSPACE.String()}...)) } @@ -79,7 +78,7 @@ func HandleReadyz(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) { if h := checkAlarms(lg, srv, excludedAlarms, endpoint); h.Health != "true" { return h } - return checkAPI(lg, srv, serializable) + return checkAPI(lg, srv, false) }, PathReadyz)) } From 7f9aeb3cf6ec03a0ddee4e31a6c124cdfc3f7071 Mon Sep 17 00:00:00 2001 From: Han Kang Date: Mon, 5 Jun 2023 16:57:11 -0700 Subject: [PATCH 5/5] gofmt file Change-Id: Ie5f02bba1a63f7592c6f3500db9070e6f1022df0 Signed-off-by: Han Kang --- server/proxy/grpcproxy/health.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/proxy/grpcproxy/health.go b/server/proxy/grpcproxy/health.go index fc217e14d50..5b80a67c1b2 100644 --- a/server/proxy/grpcproxy/health.go +++ b/server/proxy/grpcproxy/health.go @@ -33,7 +33,9 @@ func HandleHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) { lg = zap.NewNop() } mux.Handle(etcdhttp.PathHealth, etcdhttp.NewHealthHandler(lg, - func(excludedAlarms etcdhttp.AlarmSet, serializable bool, endpoint string) etcdhttp.Health { return checkHealth(c) }, etcdhttp.PathHealth)) + func(excludedAlarms etcdhttp.AlarmSet, serializable bool, endpoint string) etcdhttp.Health { + return checkHealth(c) + }, etcdhttp.PathHealth)) } // HandleProxyHealth registers health handler on '/proxy/health'. @@ -41,7 +43,9 @@ func HandleProxyHealth(lg *zap.Logger, mux *http.ServeMux, c *clientv3.Client) { if lg == nil { lg = zap.NewNop() } - mux.Handle(etcdhttp.PathProxyHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet, serializable bool, endpoint string) etcdhttp.Health { return checkProxyHealth(c) }, etcdhttp.PathProxyMetrics)) + mux.Handle(etcdhttp.PathProxyHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet, serializable bool, endpoint string) etcdhttp.Health { + return checkProxyHealth(c) + }, etcdhttp.PathProxyMetrics)) } func checkHealth(c *clientv3.Client) etcdhttp.Health {