From 140ea4fa299513e191cd2e006da05deaece26eea Mon Sep 17 00:00:00 2001 From: Chao Chen Date: Tue, 20 Apr 2021 13:17:09 -0700 Subject: [PATCH 1/2] etcdhttp/metrics.go: exclude alarms from health check conditionally with ?exclude=NOSPACE --- server/etcdserver/api/etcdhttp/metrics.go | 52 +++++-- .../etcdserver/api/etcdhttp/metrics_test.go | 138 ++++++++++++++++++ server/etcdserver/api/etcdhttp/peer_test.go | 3 +- server/proxy/grpcproxy/health.go | 4 +- 4 files changed, 182 insertions(+), 15 deletions(-) create mode 100644 server/etcdserver/api/etcdhttp/metrics_test.go diff --git a/server/etcdserver/api/etcdhttp/metrics.go b/server/etcdserver/api/etcdhttp/metrics.go index 59595686341..b14a13c9c5e 100644 --- a/server/etcdserver/api/etcdhttp/metrics.go +++ b/server/etcdserver/api/etcdhttp/metrics.go @@ -39,14 +39,14 @@ const ( // HandleMetricsHealth registers metrics and health handlers. func HandleMetricsHealth(lg *zap.Logger, mux *http.ServeMux, srv etcdserver.ServerV2) { mux.Handle(PathMetrics, promhttp.Handler()) - mux.Handle(PathHealth, NewHealthHandler(lg, func() Health { return checkV2Health(lg, srv) })) + mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet) Health { return checkV2Health(lg, srv, excludedAlarms) })) } // HandleMetricsHealthForV3 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) { mux.Handle(PathMetrics, promhttp.Handler()) - mux.Handle(PathHealth, NewHealthHandler(lg, func() Health { return checkV3Health(lg, srv) })) + mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet) Health { return checkV3Health(lg, srv, excludedAlarms) })) } // HandlePrometheus registers prometheus handler on '/metrics'. @@ -55,7 +55,7 @@ func HandlePrometheus(mux *http.ServeMux) { } // NewHealthHandler handles '/health' requests. -func NewHealthHandler(lg *zap.Logger, hfunc func() Health) http.HandlerFunc { +func NewHealthHandler(lg *zap.Logger, hfunc func(excludedAlarms AlarmSet) Health) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { w.Header().Set("Allow", http.MethodGet) @@ -63,7 +63,8 @@ func NewHealthHandler(lg *zap.Logger, hfunc func() Health) http.HandlerFunc { lg.Warn("/health error", zap.Int("status-code", http.StatusMethodNotAllowed)) return } - h := hfunc() + excludedAlarms := getExcludedAlarms(r) + h := hfunc(excludedAlarms) defer func() { if h.Health == "true" { healthSuccess.Inc() @@ -110,15 +111,38 @@ type Health struct { Reason string `json:"reason"` } -// TODO: server NOSPACE, etcdserver.ErrNoLeader in health API +type AlarmSet map[string]struct{} -func checkHealth(lg *zap.Logger, srv etcdserver.ServerV2) Health { +func getExcludedAlarms(r *http.Request) (alarms AlarmSet) { + alarms = make(map[string]struct{}, 2) + alms, found := r.URL.Query()["exclude"] + if found { + for _, alm := range alms { + if len(alms) == 0 { + continue + } + alarms[alm] = struct{}{} + } + } + return alarms +} + +// TODO: etcdserver.ErrNoLeader in health API + +func checkHealth(lg *zap.Logger, srv etcdserver.ServerV2, excludedAlarms AlarmSet) Health { h := Health{} h.Health = "true" as := srv.Alarms() if len(as) > 0 { - h.Health = "false" for _, v := range as { + alarmName := v.Alarm.String() + if _, found := excludedAlarms[alarmName]; found { + lg.Debug("/health excluded alarm", zap.String("alarm", alarmName)) + delete(excludedAlarms, alarmName) + continue + } + + h.Health = "false" switch v.Alarm { case etcdserverpb.AlarmType_NOSPACE: h.Reason = "ALARM NOSPACE" @@ -128,8 +152,12 @@ func checkHealth(lg *zap.Logger, srv etcdserver.ServerV2) Health { h.Reason = "ALARM UNKNOWN" } lg.Warn("serving /health false due to an alarm", zap.String("alarm", v.String())) + return h } - return h + } + + if len(excludedAlarms) > 0 { + lg.Warn("fail exclude alarms from health check", zap.String("exclude alarms", fmt.Sprintf("%+v", excludedAlarms))) } if uint64(srv.Leader()) == raft.None { @@ -141,8 +169,8 @@ func checkHealth(lg *zap.Logger, srv etcdserver.ServerV2) Health { return h } -func checkV2Health(lg *zap.Logger, srv etcdserver.ServerV2) (h Health) { - if h = checkHealth(lg, srv); h.Health != "true" { +func checkV2Health(lg *zap.Logger, srv etcdserver.ServerV2, excludedAlarms AlarmSet) (h Health) { + if h = checkHealth(lg, srv, excludedAlarms); h.Health != "true" { return } ctx, cancel := context.WithTimeout(context.Background(), time.Second) @@ -158,8 +186,8 @@ func checkV2Health(lg *zap.Logger, srv etcdserver.ServerV2) (h Health) { return } -func checkV3Health(lg *zap.Logger, srv *etcdserver.EtcdServer) (h Health) { - if h = checkHealth(lg, srv); h.Health != "true" { +func checkV3Health(lg *zap.Logger, srv *etcdserver.EtcdServer, excludedAlarms AlarmSet) (h Health) { + if h = checkHealth(lg, srv, excludedAlarms); h.Health != "true" { return } ctx, cancel := context.WithTimeout(context.Background(), srv.Cfg.ReqTimeout()) diff --git a/server/etcdserver/api/etcdhttp/metrics_test.go b/server/etcdserver/api/etcdhttp/metrics_test.go new file mode 100644 index 00000000000..c8839d7ad35 --- /dev/null +++ b/server/etcdserver/api/etcdhttp/metrics_test.go @@ -0,0 +1,138 @@ +package etcdhttp + +import ( + "context" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + 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/raft/v3" + "go.etcd.io/etcd/server/v3/etcdserver" + stats "go.etcd.io/etcd/server/v3/etcdserver/api/v2stats" + "go.uber.org/zap" +) + +type fakeStats struct{} + +func (s *fakeStats) SelfStats() []byte { return nil } +func (s *fakeStats) LeaderStats() []byte { return nil } +func (s *fakeStats) StoreStats() []byte { return nil } + +type fakeServerV2 struct { + fakeServer + stats.Stats + health string +} + +func (s *fakeServerV2) 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) { + if s.health == "true" { + return etcdserver.Response{}, nil + } + return etcdserver.Response{}, fmt.Errorf("fail health check") +} +func (s *fakeServerV2) ClientCertAuthEnabled() bool { return false } + +func TestHealthHandler(t *testing.T) { + // define the input and expected output + // input: alarms, and healthCheckURL + tests := []struct { + alarms []*pb.AlarmMember + healthCheckURL string + statusCode int + health string + }{ + { + []*pb.AlarmMember{}, + "/health", + http.StatusOK, + "true", + }, + { + []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}}, + "/health", + http.StatusServiceUnavailable, + "false", + }, + { + []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}}, + "/health?exclude=NOSPACE", + http.StatusOK, + "true", + }, + { + []*pb.AlarmMember{}, + "/health?exclude=NOSPACE", + http.StatusOK, + "true", + }, + { + []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(1), Alarm: pb.AlarmType_CORRUPT}}, + "/health?exclude=NOSPACE", + http.StatusServiceUnavailable, + "false", + }, + { + []*pb.AlarmMember{{MemberID: uint64(0), Alarm: pb.AlarmType_NOSPACE}, {MemberID: uint64(1), Alarm: pb.AlarmType_CORRUPT}}, + "/health?exclude=NOSPACE&exclude=CORRUPT", + http.StatusOK, + "true", + }, + } + + for i, tt := range tests { + func() { + mux := http.NewServeMux() + HandleMetricsHealth(zap.NewExample(), mux, &fakeServerV2{ + fakeServer: fakeServer{alarms: tt.alarms}, + Stats: &fakeStats{}, + health: tt.health, + }) + 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 in test case #%d", tt.healthCheckURL, err, i+1) + } + if res == nil { + t.Errorf("got nil http response with http request %s in test case #%d", tt.healthCheckURL, i+1) + return + } + if res.StatusCode != tt.statusCode { + t.Errorf("want statusCode %d but got %d in test case #%d", tt.statusCode, res.StatusCode, i+1) + } + health, err := parseHealthOutput(res.Body) + if err != nil { + t.Errorf("fail parse health check output %v", err) + } + if health.Health != tt.health { + t.Errorf("want health %s but got %s", tt.health, health.Health) + } + }() + } +} + +func parseHealthOutput(body io.Reader) (Health, error) { + obj := Health{} + d, derr := ioutil.ReadAll(body) + if derr != nil { + return obj, derr + } + if err := json.Unmarshal(d, &obj); err != nil { + return obj, err + } + return obj, nil +} diff --git a/server/etcdserver/api/etcdhttp/peer_test.go b/server/etcdserver/api/etcdhttp/peer_test.go index de311c7e758..5a1ea753797 100644 --- a/server/etcdserver/api/etcdhttp/peer_test.go +++ b/server/etcdserver/api/etcdhttp/peer_test.go @@ -58,6 +58,7 @@ func (c *fakeCluster) Version() *semver.Version { return nil } type fakeServer struct { cluster api.Cluster + alarms []*pb.AlarmMember } func (s *fakeServer) AddMember(ctx context.Context, memb membership.Member) ([]*membership.Member, error) { @@ -74,7 +75,7 @@ func (s *fakeServer) PromoteMember(ctx context.Context, id uint64) ([]*membershi } func (s *fakeServer) ClusterVersion() *semver.Version { return nil } func (s *fakeServer) Cluster() api.Cluster { return s.cluster } -func (s *fakeServer) Alarms() []*pb.AlarmMember { return nil } +func (s *fakeServer) Alarms() []*pb.AlarmMember { return s.alarms } func (s *fakeServer) LeaderChangedNotify() <-chan struct{} { return nil } var fakeRaftHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/server/proxy/grpcproxy/health.go b/server/proxy/grpcproxy/health.go index 5a77b819e56..1d6f7a2d8b9 100644 --- a/server/proxy/grpcproxy/health.go +++ b/server/proxy/grpcproxy/health.go @@ -31,7 +31,7 @@ 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() etcdhttp.Health { return checkHealth(c) })) + mux.Handle(etcdhttp.PathHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet) etcdhttp.Health { return checkHealth(c) })) } // HandleProxyHealth registers health handler on '/proxy/health'. @@ -39,7 +39,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() etcdhttp.Health { return checkProxyHealth(c) })) + mux.Handle(etcdhttp.PathProxyHealth, etcdhttp.NewHealthHandler(lg, func(excludedAlarms etcdhttp.AlarmSet) etcdhttp.Health { return checkProxyHealth(c) })) } func checkHealth(c *clientv3.Client) etcdhttp.Health { From b321c48b2d52085b1c44885499c9a23554dce78b Mon Sep 17 00:00:00 2001 From: Chao Chen Date: Tue, 20 Apr 2021 13:17:26 -0700 Subject: [PATCH 2/2] Update CHANGELOG-3.5.md --- CHANGELOG-3.5.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG-3.5.md b/CHANGELOG-3.5.md index 1a875e8f687..7c0b7ce8da3 100644 --- a/CHANGELOG-3.5.md +++ b/CHANGELOG-3.5.md @@ -137,6 +137,7 @@ Note that any `etcd_debugging_*` metrics are experimental and subject to change. - Improve logging around snapshot send and receive. - [Push down RangeOptions.limit argv into index tree to reduce memory overhead](https://github.com/etcd-io/etcd/pull/11990). - Add [reason field for /health response](https://github.com/etcd-io/etcd/pull/11983). +- Add [exclude alarms from health check conditionally](https://github.com/etcd-io/etcd/pull/12880) - Add [`--unsafe-no-fsync`](https://github.com/etcd-io/etcd/pull/11946) flag. - Setting the flag disables all uses of fsync, which is unsafe and will cause data loss. This flag makes it possible to run an etcd node for testing and development without placing lots of load on the file system. - Add [etcd --auth-token-ttl](https://github.com/etcd-io/etcd/pull/11980) flag to customize `simpleTokenTTL` settings.