Merge pull request kubernetes#5677 from smarterclayton/remove_healthz…

…_global Remove global map from healthz
duda · Mar 20, 2015 · e94c5c3 · e94c5c3
2 parents ec77230 + 015bc3b
commit e94c5c3
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()