Remove global map from healthz

It currently is impossible to use two healthz handlers on different ports in the same process. This removes the global variables in favor of requiring the consumer to specify all health checks up front.
duda · Mar 19, 2015 · 015bc3b · 015bc3b
1 parent 1ce5dda
commit 015bc3b
Show file tree

Hide file tree

Showing 9 changed files with 94 additions and 67 deletions.
diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go
@@ -32,7 +32,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider"
 	nodeControllerPkg "github.com/GoogleCloudPlatform/kubernetes/pkg/cloudprovider/controller"
 	replicationControllerPkg "github.com/GoogleCloudPlatform/kubernetes/pkg/controller"
-	_ "github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/master/ports"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/resourcequota"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/service"

diff --git a/cmd/kube-controller-manager/controller-manager.go b/cmd/kube-controller-manager/controller-manager.go
@@ -26,12 +26,17 @@ import (
 	"runtime"
 
 	"github.com/GoogleCloudPlatform/kubernetes/cmd/kube-controller-manager/app"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version/verflag"
 
 	"github.com/spf13/pflag"
 )
 
+func init() {
+	healthz.DefaultHealthz()
+}
+
 func main() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
 	s := app.NewCMServer()

diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go
@@ -26,7 +26,6 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
-	_ "github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/proxy"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/proxy/config"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"

diff --git a/cmd/kube-proxy/proxy.go b/cmd/kube-proxy/proxy.go
@@ -22,12 +22,17 @@ import (
 	"runtime"
 
 	"github.com/GoogleCloudPlatform/kubernetes/cmd/kube-proxy/app"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version/verflag"
 
 	"github.com/spf13/pflag"
 )
 
+func init() {
+	healthz.DefaultHealthz()
+}
+
 func main() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
 	s := app.NewProxyServer()

diff --git a/pkg/healthz/healthz.go b/pkg/healthz/healthz.go
@@ -20,97 +20,109 @@ import (
 	"bytes"
 	"fmt"
 	"net/http"
-	"sync"
 )
 
-var (
-	// guards names and checks
-	lock = sync.RWMutex{}
-	// used to ensure checks are performed in the order added
-	names  = []string{}
-	checks = map[string]*healthzCheck{}
-)
+// HealthzChecker is a named healthz check.
+type HealthzChecker interface {
+	Name() string
+	Check(req *http.Request) error
+}
 
-func init() {
-	http.HandleFunc("/healthz", handleRootHealthz)
-	// add ping health check by default
-	AddHealthzFunc("ping", func(_ *http.Request) error {
-		return nil
-	})
+// DefaultHealthz installs the default healthz check to the http.DefaultServeMux.
+func DefaultHealthz(checks ...HealthzChecker) {
+	InstallHandler(http.DefaultServeMux, checks...)
 }
 
-// AddHealthzFunc adds a health check under the url /healhz/{name}
-func AddHealthzFunc(name string, check func(r *http.Request) error) {
-	lock.Lock()
-	defer lock.Unlock()
-	if _, found := checks[name]; !found {
-		names = append(names, name)
-	}
-	checks[name] = &healthzCheck{name, check}
+// PingHealthz returns true automatically when checked
+var PingHealthz HealthzChecker = ping{}
+
+// ping implements the simplest possible health checker.
+type ping struct{}
+
+func (ping) Name() string {
+	return "ping"
+}
+
+// PingHealthz is a health check that returns true.
+func (ping) Check(_ *http.Request) error {
+	return nil
+}
+
+// NamedCheck returns a health checker for the given name and function.
+func NamedCheck(name string, check func(r *http.Request) error) HealthzChecker {
+	return &healthzCheck{name, check}
 }
 
 // InstallHandler registers a handler for health checking on the path "/healthz" to mux.
-func InstallHandler(mux mux) {
-	lock.RLock()
-	defer lock.RUnlock()
-	mux.HandleFunc("/healthz", handleRootHealthz)
+func InstallHandler(mux mux, checks ...HealthzChecker) {
+	if len(checks) == 0 {
+		checks = []HealthzChecker{PingHealthz}
+	}
+	mux.Handle("/healthz", handleRootHealthz(checks...))
 	for _, check := range checks {
-		mux.HandleFunc(fmt.Sprintf("/healthz/%v", check.name), adaptCheckToHandler(check.check))
+		mux.Handle(fmt.Sprintf("/healthz/%v", check.Name()), adaptCheckToHandler(check.Check))
 	}
 }
 
 // mux is an interface describing the methods InstallHandler requires.
 type mux interface {
-	HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request))
+	Handle(pattern string, handler http.Handler)
 }
 
+// healthzCheck implements HealthzChecker on an arbitrary name and check function.
 type healthzCheck struct {
 	name  string
 	check func(r *http.Request) error
 }
 
-func handleRootHealthz(w http.ResponseWriter, r *http.Request) {
-	lock.RLock()
-	defer lock.RUnlock()
-	failed := false
-	var verboseOut bytes.Buffer
-	for _, name := range names {
-		check, found := checks[name]
-		if !found {
-			// this should not happen
-			http.Error(w, fmt.Sprintf("Internal server error: check \"%q\" not registered", name), http.StatusInternalServerError)
+var _ HealthzChecker = &healthzCheck{}
+
+func (c *healthzCheck) Name() string {
+	return c.name
+}
+
+func (c *healthzCheck) Check(r *http.Request) error {
+	return c.check(r)
+}
+
+// handleRootHealthz returns an http.HandlerFunc that serves the provided checks.
+func handleRootHealthz(checks ...HealthzChecker) http.HandlerFunc {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		failed := false
+		var verboseOut bytes.Buffer
+		for _, check := range checks {
+			err := check.Check(r)
+			if err != nil {
+				fmt.Fprintf(&verboseOut, "[-]%v failed: %v\n", check.Name(), err)
+				failed = true
+			} else {
+				fmt.Fprintf(&verboseOut, "[+]%v ok\n", check.Name())
+			}
+		}
+		// always be verbose on failure
+		if failed {
+			http.Error(w, fmt.Sprintf("%vhealthz check failed", verboseOut.String()), http.StatusInternalServerError)
 			return
 		}
-		err := check.check(r)
-		if err != nil {
-			fmt.Fprintf(&verboseOut, "[-]%v failed: %v\n", check.name, err)
-			failed = true
-		} else {
-			fmt.Fprintf(&verboseOut, "[+]%v ok\n", check.name)
+
+		if _, found := r.URL.Query()["verbose"]; !found {
+			fmt.Fprint(w, "ok")
+			return
 		}
-	}
-	// always be verbose on failure
-	if failed {
-		http.Error(w, fmt.Sprintf("%vhealthz check failed", verboseOut.String()), http.StatusInternalServerError)
-		return
-	}
 
-	if _, found := r.URL.Query()["verbose"]; !found {
-		fmt.Fprint(w, "ok")
-		return
-	} else {
 		verboseOut.WriteTo(w)
 		fmt.Fprint(w, "healthz check passed\n")
-	}
+	})
 }
 
-func adaptCheckToHandler(c func(r *http.Request) error) func(w http.ResponseWriter, r *http.Request) {
-	return func(w http.ResponseWriter, r *http.Request) {
+// adaptCheckToHandler returns an http.HandlerFunc that serves the provided checks.
+func adaptCheckToHandler(c func(r *http.Request) error) http.HandlerFunc {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		err := c(r)
 		if err != nil {
 			http.Error(w, fmt.Sprintf("Internal server error: %v", err), http.StatusInternalServerError)
 		} else {
 			fmt.Fprint(w, "ok")
 		}
-	}
+	})
 }
diff --git a/pkg/healthz/healthz_test.go b/pkg/healthz/healthz_test.go
@@ -59,12 +59,13 @@ func TestMulitipleChecks(t *testing.T) {
 
 	for i, test := range tests {
 		mux := http.NewServeMux()
+		checks := []HealthzChecker{PingHealthz}
 		if test.addBadCheck {
-			AddHealthzFunc("bad", func(_ *http.Request) error {
+			checks = append(checks, NamedCheck("bad", func(_ *http.Request) error {
 				return errors.New("this will fail")
-			})
+			}))
 		}
-		InstallHandler(mux)
+		InstallHandler(mux, checks...)
 		req, err := http.NewRequest("GET", fmt.Sprintf("http://example.com%v", test.path), nil)
 		if err != nil {
 			t.Fatalf("case[%d] Unexpected error: %v", i, err)

diff --git a/pkg/kubelet/server.go b/pkg/kubelet/server.go
@@ -110,9 +110,11 @@ func NewServer(host HostInterface, enableDebuggingHandlers bool) Server {
 
 // InstallDefaultHandlers registers the default set of supported HTTP request patterns with the mux.
 func (s *Server) InstallDefaultHandlers() {
-	healthz.AddHealthzFunc("docker", s.dockerHealthCheck)
-	healthz.AddHealthzFunc("hostname", s.hostnameHealthCheck)
-	healthz.InstallHandler(s.mux)
+	healthz.InstallHandler(s.mux,
+		healthz.PingHealthz,
+		healthz.NamedCheck("docker", s.dockerHealthCheck),
+		healthz.NamedCheck("hostname", s.hostnameHealthCheck),
+	)
 	s.mux.HandleFunc("/podInfo", s.handlePodInfoOld)
 	s.mux.HandleFunc("/api/v1beta1/podInfo", s.handlePodInfoVersioned)
 	s.mux.HandleFunc("/api/v1beta1/nodeInfo", s.handleNodeInfoVersioned)

diff --git a/plugin/cmd/kube-scheduler/app/server.go b/plugin/cmd/kube-scheduler/app/server.go
@@ -28,7 +28,6 @@ import (
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client/record"
-	_ "github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/master/ports"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/scheduler"

diff --git a/plugin/cmd/kube-scheduler/scheduler.go b/plugin/cmd/kube-scheduler/scheduler.go
@@ -19,13 +19,18 @@ package main
 import (
 	"runtime"
 
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/healthz"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version/verflag"
 	"github.com/GoogleCloudPlatform/kubernetes/plugin/cmd/kube-scheduler/app"
 
 	"github.com/spf13/pflag"
 )
 
+func init() {
+	healthz.DefaultHealthz()
+}
+
 func main() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
 	s := app.NewSchedulerServer()