Use github.com/heptio/healthcheck

Rather than a handrolled solution, this commit uses heptio's healthcheck
library. Future consumers of the health check handler need only to
register liveness or readiness probes.

Update install.yaml to use /live for liveness check.

Also, inline health handler function for consistent style.

Gopkg.toml

@@ -47,3 +47,7 @@
 [[constraint]]
   branch = "master"
   name = "github.com/mattbaird/jsonpatch"
+
+[[constraint]]
+  branch = "master"
+  name = "github.com/heptiolabs/healthcheck"

build/install.yaml

@@ -155,7 +155,7 @@ spec:
           value: "8000"
         livenessProbe:
           httpGet:
-            path: /healthz
+            path: /live
             port: 8080
           initialDelaySeconds: 3
           periodSeconds: 3
@@ -288,4 +288,4 @@ subjects:
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: ClusterRole
-  name: agones-controller
+  name: agones-controller

cmd/controller/main.go

@@ -16,6 +16,7 @@
 package main
 
 import (
+	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -28,6 +29,8 @@ import (
 	"agones.dev/agones/pkg/util/runtime"
 	"agones.dev/agones/pkg/util/signals"
 	"agones.dev/agones/pkg/util/webhooks"
+	"github.com/heptiolabs/healthcheck"
+	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 	extclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
@@ -120,11 +123,12 @@ func main() {
 		logger.WithError(err).Fatal("Could not create the agones api clientset")
 	}
 
+	health := healthcheck.NewHandler()
+	wh := webhooks.NewWebHook(certFile, keyFile)
 	agonesInformerFactory := externalversions.NewSharedInformerFactory(agonesClient, 30*time.Second)
 	kubeInformationFactory := informers.NewSharedInformerFactory(kubeClient, 30*time.Second)
 
-	wh := webhooks.NewWebHook(certFile, keyFile)
-	c := gameservers.NewController(wh, minPort, maxPort, sidecarImage, alwaysPullSidecar, kubeClient, kubeInformationFactory, extClient, agonesClient, agonesInformerFactory)
+	c := gameservers.NewController(wh, health, minPort, maxPort, sidecarImage, alwaysPullSidecar, kubeClient, kubeInformationFactory, extClient, agonesClient, agonesInformerFactory)
 
 	stop := signals.NewStopChannel()
 
@@ -136,6 +140,23 @@ func main() {
 			logger.WithError(err).Fatal("could not run webhook server")
 		}
 	}()
+	go func() {
+		logger.Info("Starting health check...")
+		srv := &http.Server{
+			Addr:    ":8080",
+			Handler: health,
+		}
+		defer srv.Close() // nolint: errcheck
+
+		if err := srv.ListenAndServe(); err != nil {
+			if err == http.ErrServerClosed {
+				logger.WithError(err).Info("health check: http server closed")
+			} else {
+				err := errors.Wrap(err, "Could not listen on :8080")
+				runtime.HandleError(logger.WithError(err), err)
+			}
+		}
+	}()
 
 	err = c.Run(2, stop)
 	if err != nil {

pkg/gameservers/controller.go

@@ -17,7 +17,6 @@ package gameservers
 import (
 	"encoding/json"
 	"fmt"
-	"net/http"
 
 	"agones.dev/agones/pkg/apis/stable"
 	stablev1alpha1 "agones.dev/agones/pkg/apis/stable/v1alpha1"
@@ -29,6 +28,7 @@ import (
 	"agones.dev/agones/pkg/util/runtime"
 	"agones.dev/agones/pkg/util/webhooks"
 	"agones.dev/agones/pkg/util/workerqueue"
+	"github.com/heptiolabs/healthcheck"
 	"github.com/mattbaird/jsonpatch"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -68,13 +68,13 @@ type Controller struct {
 	portAllocator          *PortAllocator
 	healthController       *HealthController
 	workerqueue            *workerqueue.WorkerQueue
-	server                 *http.Server
 	recorder               record.EventRecorder
 }
 
 // NewController returns a new gameserver crd controller
 func NewController(
 	wh *webhooks.WebHook,
+	health healthcheck.Handler,
 	minPort, maxPort int32,
 	sidecarImage string,
 	alwaysPullSidecarImage bool,
@@ -137,20 +137,6 @@ func NewController(
 		},
 	})
 
-	mux := http.NewServeMux()
-	mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
-		_, err := w.Write([]byte("ok"))
-		if err != nil {
-			c.logger.WithError(err).Error("could not send ok response on healthz")
-			w.WriteHeader(http.StatusInternalServerError)
-		}
-	})
-
-	c.server = &http.Server{
-		Addr:    ":8080",
-		Handler: mux,
-	}
-
 	return c
 }
 
@@ -217,19 +203,6 @@ func (c *Controller) creationHandler(review admv1beta1.AdmissionReview) (admv1be
 // Run the GameServer controller. Will block until stop is closed.
 // Runs threadiness number workers to process the rate limited queue
 func (c *Controller) Run(threadiness int, stop <-chan struct{}) error {
-	c.logger.Info("Starting health check...")
-	go func() {
-		if err := c.server.ListenAndServe(); err != nil {
-			if err == http.ErrServerClosed {
-				c.logger.WithError(err).Info("health check: http server closed")
-			} else {
-				err := errors.Wrap(err, "Could not listen on :8080")
-				runtime.HandleError(c.logger.WithError(err), err)
-			}
-		}
-	}()
-	defer c.server.Close() // nolint: errcheck
-
 	err := crd.WaitForEstablishedCRD(c.crdGetter, "gameservers.stable.agones.dev", c.logger)
 	if err != nil {
 		return err

pkg/gameservers/controller_test.go

@@ -24,6 +24,7 @@ import (
 	"agones.dev/agones/pkg/apis/stable"
 	"agones.dev/agones/pkg/apis/stable/v1alpha1"
 	"agones.dev/agones/pkg/util/webhooks"
+	"github.com/heptiolabs/healthcheck"
 	"github.com/mattbaird/jsonpatch"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
@@ -185,24 +186,29 @@ func TestControllerWatchGameServers(t *testing.T) {
 }
 
 func TestControllerHealthCheck(t *testing.T) {
-	c, mocks := newFakeController()
-	mocks.extClient.AddReactor("get", "customresourcedefinitions", func(action k8stesting.Action) (bool, runtime.Object, error) {
+	m := newMocks()
+	m.extClient.AddReactor("get", "customresourcedefinitions", func(action k8stesting.Action) (bool, runtime.Object, error) {
 		return true, newEstablishedCRD(), nil
 	})
+	health := healthcheck.NewHandler()
+	c := NewController(webhooks.NewWebHook("", ""), health, 10, 20, "sidecar:dev", false,
+		m.kubeClient, m.kubeInformationFactory, m.extClient, m.agonesClient, m.agonesInformerFactory)
 
 	c.workerqueue.SyncHandler = func(name string) error {
 		return nil
 	}
 
-	stop, cancel := startInformers(mocks, c.gameServerSynced)
+	stop, cancel := startInformers(m, c.gameServerSynced)
 	defer cancel()
 
+	go http.ListenAndServe("localhost:9090", health)
+
 	go func() {
 		err := c.Run(1, stop)
 		assert.Nil(t, err, "Run should not error")
 	}()
 
-	testHTTPHealth(t, "http://localhost:8080/healthz", "ok", http.StatusOK)
+	testHTTPHealth(t, "http://localhost:9090/live", "{}\n", http.StatusOK)
 }
 
 func TestControllerCreationHandler(t *testing.T) {
@@ -820,7 +826,7 @@ func testWithNonZeroDeletionTimestamp(t *testing.T, state v1alpha1.State, f func
 func newFakeController() (*Controller, mocks) {
 	m := newMocks()
 	wh := webhooks.NewWebHook("", "")
-	c := NewController(wh, 10, 20, "sidecar:dev", false,
+	c := NewController(wh, healthcheck.NewHandler(), 10, 20, "sidecar:dev", false,
 		m.kubeClient, m.kubeInformationFactory, m.extClient, m.agonesClient, m.agonesInformerFactory)
 	c.recorder = m.fakeRecorder
 	return c, m

pkg/gameservers/helper_test.go

@@ -153,8 +153,8 @@ func testHTTPHealth(t *testing.T, url string, expectedResponse string, expectedS
 			defer resp.Body.Close()
 			body, err := ioutil.ReadAll(resp.Body)
 			assert.Nil(t, err, "(%s) read response error should be nil: %v", url, err)
-			assert.Equal(t, []byte(expectedResponse), body, "(%s) response body should be '%s'", url, expectedResponse)
 			assert.Equal(t, expectedStatus, resp.StatusCode, "url: %s", url)
+			assert.Equal(t, []byte(expectedResponse), body, "(%s) response body should be '%s'", url, expectedResponse)
 		}
 
 		return true, nil