internal/health: new health check package

Currently only serving Redis, but could be used for other GCP gRPC
connections.

Mildly surprising behavior: taking down Redis does not make Pool.Get
return an error: it isn't until sending another request on a connection
before the pool starts failing.  This only matters when testing the
health checks with no load.

Change-Id: Ie720b80a398fd9f7d4aa6ab0c6a88adaa85ccdc9
Reviewed-on: https://go-review.googlesource.com/76750
Reviewed-by: Tuo Shan <[email protected]>

database/database.go

@@ -144,12 +144,6 @@ func New(serverURI string, idleTimeout time.Duration, logConn bool, gaeEndpoint
 		IdleTimeout: idleTimeout,
 	}
 
-	c := pool.Get()
-	if c.Err() != nil {
-		return nil, c.Err()
-	}
-	c.Close()
-
 	var rc *remote_api.Client
 	if gaeEndpoint != "" {
 		var err error
@@ -161,6 +155,16 @@ func New(serverURI string, idleTimeout time.Duration, logConn bool, gaeEndpoint
 	return &Database{Pool: pool, RemoteClient: rc}, nil
 }
 
+func (db *Database) CheckHealth() error {
+	// TODO(light): get() can trigger a dial.  Ideally, the pool could
+	// inform whether or not a lack of connections is due to idleness or
+	// errors.
+	c := db.Pool.Get()
+	err := c.Err()
+	c.Close()
+	return err
+}
+
 // Exists returns true if package with import path exists in the database.
 func (db *Database) Exists(path string) (bool, error) {
 	c := db.Pool.Get()

gddo-server/app.yaml

@@ -1,10 +1,9 @@
 # This YAML file is used for local deployment with GAE development environment.
+env: flex
 runtime: go
-vm: true
-api_version: 1
-threadsafe: true
 
-handlers:
-- url: /.*
-  script: IGNORED
-  secure: always
+liveness_check:
+  path: /_ah/health
+
+readiness_check:
+  path: /_ah/ready

gddo-server/main.go

@@ -36,6 +36,7 @@ import (
 	"github.com/golang/gddo/doc"
 	"github.com/golang/gddo/gosrc"
 	"github.com/golang/gddo/httputil"
+	"github.com/golang/gddo/internal/health"
 )
 
 const (
@@ -586,10 +587,6 @@ func (s *server) serveBot(resp http.ResponseWriter, req *http.Request) error {
 	return s.templates.execute(resp, "bot.html", http.StatusOK, nil, nil)
 }
 
-func serveHealthCheck(resp http.ResponseWriter, req *http.Request) {
-	resp.Write([]byte("Health check: ok\n"))
-}
-
 func logError(req *http.Request, err error, rv interface{}) {
 	if err != nil {
 		var buf bytes.Buffer
@@ -936,7 +933,9 @@ func newServer(ctx context.Context, v *viper.Viper) (*server, error) {
 	mux.Handle("/", handler(s.serveHome))
 
 	ahMux := http.NewServeMux()
-	ahMux.HandleFunc("/_ah/health", serveHealthCheck)
+	ready := new(health.Handler)
+	ahMux.HandleFunc("/_ah/health", health.HandleLive)
+	ahMux.Handle("/_ah/ready", ready)
 
 	mainMux := http.NewServeMux()
 	mainMux.Handle("/_ah/", ahMux)
@@ -962,6 +961,7 @@ func newServer(ctx context.Context, v *viper.Viper) (*server, error) {
 	if err != nil {
 		return nil, fmt.Errorf("open database: %v", err)
 	}
+	ready.Add(s.db)
 	if gceLogName := v.GetString(ConfigGCELogName); gceLogName != "" {
 		logc, err := logging.NewClient(ctx, v.GetString(ConfigProject))
 		if err != nil {

internal/health/health.go

@@ -0,0 +1,66 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file or at
+// https://developers.google.com/open-source/licenses/bsd.
+
+// Package health provides health check handlers.
+package health
+
+import (
+	"io"
+	"net/http"
+)
+
+// Handler is an HTTP handler that reports on the success of an
+// aggregate of Checkers.  The zero value is always healthy.
+type Handler struct {
+	checkers []Checker
+}
+
+// Add adds a new check to the handler.
+func (h *Handler) Add(c Checker) {
+	h.checkers = append(h.checkers, c)
+}
+
+// ServeHTTP returns 200 if it is healthy, 500 otherwise.
+func (h *Handler) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
+	for _, c := range h.checkers {
+		if err := c.CheckHealth(); err != nil {
+			writeUnhealthy(w)
+			return
+		}
+	}
+	writeHealthy(w)
+}
+
+func writeUnhealthy(w http.ResponseWriter) {
+	w.Header().Set("Content-Length", "9")
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.WriteHeader(http.StatusInternalServerError)
+	io.WriteString(w, "unhealthy")
+}
+
+// HandleLive is an http.HandleFunc that handles liveness checks by
+// immediately responding with an HTTP 200 status.
+func HandleLive(w http.ResponseWriter, _ *http.Request) {
+	writeHealthy(w)
+}
+
+func writeHealthy(w http.ResponseWriter) {
+	w.Header().Set("Content-Length", "2")
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+	w.WriteHeader(http.StatusOK)
+	io.WriteString(w, "ok")
+}
+
+// Checker wraps the CheckHealth method.
+//
+// CheckHealth returns nil if the resource is healthy, or a non-nil
+// error if the resource is not healthy.  CheckHealth must be safe to
+// call from multiple goroutines.
+type Checker interface {
+	CheckHealth() error
+}

internal/health/health_test.go

@@ -0,0 +1,93 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file or at
+// https://developers.google.com/open-source/licenses/bsd.
+
+package health
+
+import (
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+)
+
+func TestNewHandler(t *testing.T) {
+	s := httptest.NewServer(new(Handler))
+	defer s.Close()
+	code, err := check(s)
+	if err != nil {
+		t.Fatalf("GET %s: %v", s.URL, err)
+	}
+	if code != http.StatusOK {
+		t.Errorf("got HTTP status %d; want %d", code, http.StatusOK)
+	}
+}
+
+func TestChecker(t *testing.T) {
+	c1 := &checker{err: errors.New("checker 1 down")}
+	c2 := &checker{err: errors.New("checker 2 down")}
+	h := new(Handler)
+	h.Add(c1)
+	h.Add(c2)
+	s := httptest.NewServer(h)
+	defer s.Close()
+
+	t.Run("AllUnhealthy", func(t *testing.T) {
+		code, err := check(s)
+		if err != nil {
+			t.Fatalf("GET %s: %v", s.URL, err)
+		}
+		if code != http.StatusInternalServerError {
+			t.Errorf("got HTTP status %d; want %d", code, http.StatusInternalServerError)
+		}
+	})
+	c1.set(nil)
+	t.Run("PartialHealthy", func(t *testing.T) {
+		code, err := check(s)
+		if err != nil {
+			t.Fatalf("GET %s: %v", s.URL, err)
+		}
+		if code != http.StatusInternalServerError {
+			t.Errorf("got HTTP status %d; want %d", code, http.StatusInternalServerError)
+		}
+	})
+	c2.set(nil)
+	t.Run("AllHealthy", func(t *testing.T) {
+		code, err := check(s)
+		if err != nil {
+			t.Fatalf("GET %s: %v", s.URL, err)
+		}
+		if code != http.StatusOK {
+			t.Errorf("got HTTP status %d; want %d", code, http.StatusOK)
+		}
+	})
+}
+
+func check(s *httptest.Server) (code int, err error) {
+	resp, err := s.Client().Get(s.URL)
+	if err != nil {
+		return 0, err
+	}
+	resp.Body.Close()
+	return resp.StatusCode, nil
+}
+
+type checker struct {
+	mu  sync.Mutex
+	err error
+}
+
+func (c *checker) CheckHealth() error {
+	defer c.mu.Unlock()
+	c.mu.Lock()
+	return c.err
+}
+
+func (c *checker) set(e error) {
+	defer c.mu.Unlock()
+	c.mu.Lock()
+	c.err = e
+}