From 54416c6b7c2a0f1174b628af3925d2bf0beda820 Mon Sep 17 00:00:00 2001 From: Stefan Vassilev Date: Sat, 13 Apr 2019 20:58:35 +0200 Subject: [PATCH] Return info msg for `/health` endpoint (#1465) * Return info msg for `/health` endpoint Return 200 OK for status `ready` and `upTimeStats` Resolves #1450 Signed-off-by: stefan vassilev * Address PR comments Signed-off-by: stefan vassilev * Fix failing test Signed-off-by: stefan vassilev * Address PR comments Signed-off-by: stefan vassilev * Retrigger tests Signed-off-by: stefan vassilev * Retrigger tests Signed-off-by: stefan vassilev * Make thread-safe Signed-off-by: Yuri Shkuro --- crossdock/main.go | 6 ++- pkg/healthcheck/handler.go | 78 +++++++++++++++++++++++++------- pkg/healthcheck/internal_test.go | 35 +++++++++++++- 3 files changed, 101 insertions(+), 18 deletions(-) diff --git a/crossdock/main.go b/crossdock/main.go index 9ea5b839925..03201d8fa14 100644 --- a/crossdock/main.go +++ b/crossdock/main.go @@ -102,10 +102,14 @@ func (h *clientHandler) isInitialized() bool { return atomic.LoadUint64(&h.initialized) != 0 } +func is2xxStatusCode(statusCode int) bool { + return statusCode >= 200 && statusCode <= 299 +} + func httpHealthCheck(logger *zap.Logger, service, healthURL string) { for i := 0; i < 240; i++ { res, err := http.Get(healthURL) - if err == nil && res.StatusCode == 204 { + if err == nil && is2xxStatusCode(res.StatusCode) { logger.Info("Health check successful", zap.String("service", service)) return } diff --git a/pkg/healthcheck/handler.go b/pkg/healthcheck/handler.go index d48d2b9e523..87b68f3c2d1 100644 --- a/pkg/healthcheck/handler.go +++ b/pkg/healthcheck/handler.go @@ -15,8 +15,11 @@ package healthcheck import ( + "encoding/json" + "fmt" "net/http" "sync/atomic" + "time" "go.uber.org/zap" ) @@ -46,24 +49,43 @@ func (s Status) String() string { } } +type healthCheckResponse struct { + statusCode int + StatusMsg string `json:"status"` + UpSince time.Time `json:"upSince"` + Uptime string `json:"uptime"` +} + +type state struct { + status Status + upSince time.Time +} + // HealthCheck provides an HTTP endpoint that returns the health status of the service type HealthCheck struct { - state int32 // atomic, keep at the top to be word-aligned - logger *zap.Logger - mapping map[Status]int - server *http.Server + state atomic.Value // stores state struct + logger *zap.Logger + responses map[Status]healthCheckResponse + server *http.Server } // New creates a HealthCheck with the specified initial state. func New() *HealthCheck { hc := &HealthCheck{ - state: int32(Unavailable), - mapping: map[Status]int{ - Unavailable: http.StatusServiceUnavailable, - Ready: http.StatusNoContent, - }, logger: zap.NewNop(), + responses: map[Status]healthCheckResponse{ + Unavailable: { + statusCode: http.StatusServiceUnavailable, + StatusMsg: "Server not available", + }, + Ready: { + statusCode: http.StatusOK, + StatusMsg: "Server available", + }, + }, + server: nil, } + hc.state.Store(state{status: Unavailable}) return hc } @@ -75,21 +97,45 @@ func (hc *HealthCheck) SetLogger(logger *zap.Logger) { // Handler creates a new HTTP handler. func (hc *HealthCheck) Handler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(hc.mapping[hc.Get()]) - // this is written only for response with an entity, so, it won't be used for a 204 - No content - w.Write([]byte("Server not available")) + state := hc.getState() + template := hc.responses[state.status] + w.WriteHeader(template.statusCode) + + w.Header().Set("Content-Type", "application/json") + w.Write(hc.createRespBody(state, template)) }) } +func (hc *HealthCheck) createRespBody(state state, template healthCheckResponse) []byte { + resp := template // clone + if state.status == Ready { + resp.UpSince = state.upSince + resp.Uptime = fmt.Sprintf("%v", time.Since(state.upSince)) + } + healthCheckStatus, _ := json.Marshal(resp) + return healthCheckStatus +} + // Set a new health check status -func (hc *HealthCheck) Set(state Status) { - atomic.StoreInt32(&hc.state, int32(state)) - hc.logger.Info("Health Check state change", zap.Stringer("status", hc.Get())) +func (hc *HealthCheck) Set(status Status) { + oldState := hc.getState() + newState := state{status: status} + if status == Ready { + if oldState.status != Ready { + newState.upSince = time.Now() + } + } + hc.state.Store(newState) + hc.logger.Info("Health Check state change", zap.Stringer("status", status)) } // Get the current status of this health check func (hc *HealthCheck) Get() Status { - return Status(atomic.LoadInt32(&hc.state)) + return hc.getState().status +} + +func (hc *HealthCheck) getState() state { + return hc.state.Load().(state) } // Ready is a shortcut for Set(Ready) (kept for backwards compatibility) diff --git a/pkg/healthcheck/internal_test.go b/pkg/healthcheck/internal_test.go index f7c7184ab7e..f813e40c569 100644 --- a/pkg/healthcheck/internal_test.go +++ b/pkg/healthcheck/internal_test.go @@ -15,9 +15,12 @@ package healthcheck import ( + "encoding/json" + "io/ioutil" "net/http" "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -34,5 +37,35 @@ func TestHttpCall(t *testing.T) { resp, err := http.Get(server.URL + "/") require.NoError(t, err) - assert.Equal(t, http.StatusNoContent, resp.StatusCode) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + hr := parseHealthCheckResponse(t, resp) + assert.Equal(t, "Server available", hr.StatusMsg) + // de-serialized timestamp loses monotonic clock value, so assert.Equals() doesn't work. + // https://github.com/stretchr/testify/issues/502 + if want, have := hc.getState().upSince, hr.UpSince; !assert.True(t, want.Equal(have)) { + t.Logf("want='%v', have='%v'", want, have) + } + assert.NotZero(t, hr.Uptime) + t.Logf("uptime=%v", hr.Uptime) + + time.Sleep(time.Millisecond) + hc.Set(Unavailable) + + resp, err = http.Get(server.URL + "/") + require.NoError(t, err) + assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) + + hrNew := parseHealthCheckResponse(t, resp) + assert.Zero(t, hrNew.Uptime) + assert.Zero(t, hrNew.UpSince) +} + +func parseHealthCheckResponse(t *testing.T, resp *http.Response) healthCheckResponse { + body, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + var hr healthCheckResponse + err = json.Unmarshal(body, &hr) + require.NoError(t, err) + return hr }