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 | 128 ++++++++++-------- 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, 94 insertions(+), 67 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 777948e83a6a2..7a3429f6964ca 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 d4603ad3360c4..027474bee1a65 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 c49b9cc3c950c..78b4214bcc6e3 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 12d3b49303dd6..487cd140d16e2 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 8632847917a08..2e8fda9b5f778 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{} -) +// 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 index 0fffdfe05f857..432750861fb4d 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 371b208d9320c..711fcb418edbc 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 120c116ebc2cf..ead60f5d7ed07 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 ade8ff6135d17..db50a1b36437e 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()