From 015bc3b7bd7b69d254a103d6b108b595b90f68f4 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 19 Mar 2015 18:56:29 -0400 Subject: [PATCH] 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. --- .../app/controllermanager.go | 1 - .../controller-manager.go | 5 + cmd/kube-proxy/app/server.go | 1 - cmd/kube-proxy/proxy.go | 5 + pkg/healthz/healthz.go | 140 ++++++++++-------- pkg/healthz/healthz_test.go | 7 +- pkg/kubelet/server.go | 8 +- plugin/cmd/kube-scheduler/app/server.go | 1 - plugin/cmd/kube-scheduler/scheduler.go | 5 + 9 files changed, 100 insertions(+), 73 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 777948e83a6..7a3429f6964 100644 --- 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 index d4603ad3360..027474bee1a 100644 --- 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 index c49b9cc3c95..78b4214bcc6 100644 --- 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 index 12d3b49303d..487cd140d16 100644 --- 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 index 8632847917a..2e8fda9b5f7 100644 --- 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{} -) - -func init() { - http.HandleFunc("/healthz", handleRootHealthz) - // add ping health check by default - AddHealthzFunc("ping", func(_ *http.Request) error { - return nil - }) +// HealthzChecker is a named healthz check. +type HealthzChecker interface { + Name() string + Check(req *http.Request) error } -// 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} +// DefaultHealthz installs the default healthz check to the http.DefaultServeMux. +func DefaultHealthz(checks ...HealthzChecker) { + InstallHandler(http.DefaultServeMux, checks...) +} + +// 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) - 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) - } - } - // always be verbose on failure - if failed { - http.Error(w, fmt.Sprintf("%vhealthz check failed", verboseOut.String()), http.StatusInternalServerError) - return - } +var _ HealthzChecker = &healthzCheck{} - if _, found := r.URL.Query()["verbose"]; !found { - fmt.Fprint(w, "ok") - return - } else { - verboseOut.WriteTo(w) - fmt.Fprint(w, "healthz check passed\n") - } +func (c *healthzCheck) Name() string { + return c.name } -func adaptCheckToHandler(c func(r *http.Request) error) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { +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 + } + + if _, found := r.URL.Query()["verbose"]; !found { + fmt.Fprint(w, "ok") + return + } + + verboseOut.WriteTo(w) + fmt.Fprint(w, "healthz check passed\n") + }) +} + +// 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 index 0fffdfe05f8..432750861fb 100644 --- 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 index 371b208d932..711fcb418ed 100644 --- 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 index 120c116ebc2..ead60f5d7ed 100644 --- 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 index ade8ff6135d..db50a1b3643 100644 --- a/plugin/cmd/kube-scheduler/scheduler.go +++ b/plugin/cmd/kube-scheduler/scheduler.go @@ -19,6 +19,7 @@ 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" @@ -26,6 +27,10 @@ import ( "github.com/spf13/pflag" ) +func init() { + healthz.DefaultHealthz() +} + func main() { runtime.GOMAXPROCS(runtime.NumCPU()) s := app.NewSchedulerServer()