Merge pull request #35900 from deads2k/api-34-healthz

Automatic merge from submit-queue

promote /healthz and /metrics to genericapiserver

Promotes `/healthz` to genericapiserver with methods to add healthz checks before running.

Promotes `/metrics` to genericapiserver gated by config flag.

@lavalamp adds the healthz checks linked to `postStartHooks` as promised.

cmd/kube-apiserver/app/server.go

@@ -305,6 +305,7 @@ func Run(s *options.ServerRunOptions) error {
 	genericConfig.OpenAPIConfig.Info.Title = "Kubernetes"
 	genericConfig.OpenAPIConfig.Definitions = generatedopenapi.OpenAPIDefinitions
 	genericConfig.EnableOpenAPISupport = true
+	genericConfig.EnableMetrics = true
 	genericConfig.OpenAPIConfig.SecurityDefinitions = securityDefinitions
 
 	config := &master.Config{

pkg/genericapiserver/BUILD

@@ -17,6 +17,7 @@ go_library(
         "default_storage_factory_builder.go",
         "doc.go",
         "genericapiserver.go",
+        "healthz.go",
         "hooks.go",
         "resource_config.go",
         "resource_encoding_config.go",
@@ -48,6 +49,7 @@ go_library(
         "//pkg/genericapiserver/options:go_default_library",
         "//pkg/genericapiserver/routes:go_default_library",
         "//pkg/genericapiserver/validation:go_default_library",
+        "//pkg/healthz:go_default_library",
         "//pkg/registry/core/service/ipallocator:go_default_library",
         "//pkg/registry/generic:go_default_library",
         "//pkg/runtime:go_default_library",

pkg/genericapiserver/config.go

@@ -81,6 +81,7 @@ type Config struct {
 	// allow downstream consumers to disable the index route
 	EnableIndex             bool
 	EnableProfiling         bool
+	EnableMetrics           bool
 	EnableGarbageCollection bool
 
 	Version               *version.Info
@@ -452,6 +453,8 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 
 		enableOpenAPISupport: c.EnableOpenAPISupport,
 		openAPIConfig:        c.OpenAPIConfig,
+
+		postStartHooks: map[string]postStartHookEntry{},
 	}
 
 	s.HandlerContainer = mux.NewAPIContainer(http.NewServeMux(), c.Serializer)
@@ -525,6 +528,13 @@ func (s *GenericAPIServer) installAPI(c *Config) {
 	if c.EnableProfiling {
 		routes.Profiling{}.Install(s.HandlerContainer)
 	}
+	if c.EnableMetrics {
+		if c.EnableProfiling {
+			routes.MetricsWithReset{}.Install(s.HandlerContainer)
+		} else {
+			routes.DefaultMetrics{}.Install(s.HandlerContainer)
+		}
+	}
 	routes.Version{Version: c.Version}.Install(s.HandlerContainer)
 	s.HandlerContainer.Add(s.DynamicApisDiscovery())
 }

pkg/genericapiserver/genericapiserver.go

@@ -42,6 +42,7 @@ import (
 	genericmux "k8s.io/kubernetes/pkg/genericapiserver/mux"
 	"k8s.io/kubernetes/pkg/genericapiserver/openapi/common"
 	"k8s.io/kubernetes/pkg/genericapiserver/routes"
+	"k8s.io/kubernetes/pkg/healthz"
 	"k8s.io/kubernetes/pkg/runtime"
 	utilnet "k8s.io/kubernetes/pkg/util/net"
 	"k8s.io/kubernetes/pkg/util/sets"
@@ -143,10 +144,15 @@ type GenericAPIServer struct {
 	// PostStartHooks are each called after the server has started listening, in a separate go func for each
 	// with no guaranteee of ordering between them.  The map key is a name used for error reporting.
 	// It may kill the process with a panic if it wishes to by returning an error
-	postStartHooks       map[string]PostStartHookFunc
 	postStartHookLock    sync.Mutex
+	postStartHooks       map[string]postStartHookEntry
 	postStartHooksCalled bool
 
+	// healthz checks
+	healthzLock    sync.Mutex
+	healthzChecks  []healthz.HealthzChecker
+	healthzCreated bool
+
 	// See Config.$name for documentation of these flags:
 
 	MasterCount               int
@@ -188,6 +194,9 @@ func (s *GenericAPIServer) PrepareRun() preparedGenericAPIServer {
 			Config: s.openAPIConfig,
 		}.Install(s.HandlerContainer)
 	}
+
+	s.installHealthz()
+
 	return preparedGenericAPIServer{s}
 }
 

pkg/genericapiserver/healthz.go

@@ -0,0 +1,45 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package genericapiserver
+
+import (
+	"fmt"
+
+	"k8s.io/kubernetes/pkg/healthz"
+)
+
+// AddHealthzCheck allows you to add a HealthzCheck.
+func (s *GenericAPIServer) AddHealthzChecks(checks ...healthz.HealthzChecker) error {
+	s.healthzLock.Lock()
+	defer s.healthzLock.Unlock()
+
+	if s.healthzCreated {
+		return fmt.Errorf("unable to add because the healthz endpoint has already been created")
+	}
+
+	s.healthzChecks = append(s.healthzChecks, checks...)
+	return nil
+}
+
+// installHealthz creates the healthz endpoint for this server
+func (s *GenericAPIServer) installHealthz() {
+	s.healthzLock.Lock()
+	defer s.healthzLock.Unlock()
+	s.healthzCreated = true
+
+	healthz.InstallHandler(&s.HandlerContainer.NonSwaggerRoutes, s.healthzChecks...)
+}

pkg/genericapiserver/hooks.go

@@ -17,11 +17,14 @@ limitations under the License.
 package genericapiserver
 
 import (
+	"errors"
 	"fmt"
+	"net/http"
 
 	"github.com/golang/glog"
 
 	"k8s.io/kubernetes/pkg/client/restclient"
+	"k8s.io/kubernetes/pkg/healthz"
 	utilruntime "k8s.io/kubernetes/pkg/util/runtime"
 )
 
@@ -47,6 +50,13 @@ type PostStartHookProvider interface {
 	PostStartHook() (string, PostStartHookFunc, error)
 }
 
+type postStartHookEntry struct {
+	hook PostStartHookFunc
+
+	// done will be closed when the postHook is finished
+	done chan struct{}
+}
+
 // AddPostStartHook allows you to add a PostStartHook.
 func (s *GenericAPIServer) AddPostStartHook(name string, hook PostStartHookFunc) error {
 	if len(name) == 0 {
@@ -62,14 +72,15 @@ func (s *GenericAPIServer) AddPostStartHook(name string, hook PostStartHookFunc)
 	if s.postStartHooksCalled {
 		return fmt.Errorf("unable to add %q because PostStartHooks have already been called", name)
 	}
-	if s.postStartHooks == nil {
-		s.postStartHooks = map[string]PostStartHookFunc{}
-	}
 	if _, exists := s.postStartHooks[name]; exists {
 		return fmt.Errorf("unable to add %q because it is already registered", name)
 	}
 
-	s.postStartHooks[name] = hook
+	// done is closed when the poststarthook is finished.  This is used by the health check to be able to indicate
+	// that the poststarthook is finished
+	done := make(chan struct{})
+	s.AddHealthzChecks(postStartHookHealthz{name: "poststarthook/" + name, done: done})
+	s.postStartHooks[name] = postStartHookEntry{hook: hook, done: done}
 
 	return nil
 }
@@ -82,20 +93,48 @@ func (s *GenericAPIServer) RunPostStartHooks() {
 
 	context := PostStartHookContext{LoopbackClientConfig: s.LoopbackClientConfig}
 
-	for hookName, hook := range s.postStartHooks {
-		go runPostStartHook(hookName, hook, context)
+	for hookName, hookEntry := range s.postStartHooks {
+		go runPostStartHook(hookName, hookEntry, context)
 	}
 }
 
-func runPostStartHook(name string, hook PostStartHookFunc, context PostStartHookContext) {
+func runPostStartHook(name string, entry postStartHookEntry, context PostStartHookContext) {
 	var err error
 	func() {
 		// don't let the hook *accidentally* panic and kill the server
 		defer utilruntime.HandleCrash()
-		err = hook(context)
+		err = entry.hook(context)
 	}()
 	// if the hook intentionally wants to kill server, let it.
 	if err != nil {
 		glog.Fatalf("PostStartHook %q failed: %v", name, err)
 	}
+
+	close(entry.done)
+}
+
+// postStartHookHealthz implements a healthz check for poststarthooks.  It will return a "hookNotFinished"
+// error until the poststarthook is finished.
+type postStartHookHealthz struct {
+	name string
+
+	// done will be closed when the postStartHook is finished
+	done chan struct{}
+}
+
+var _ healthz.HealthzChecker = postStartHookHealthz{}
+
+func (h postStartHookHealthz) Name() string {
+	return h.name
+}
+
+var hookNotFinished = errors.New("not finished")
+
+func (h postStartHookHealthz) Check(req *http.Request) error {
+	select {
+	case <-h.done:
+		return nil
+	default:
+		return hookNotFinished
+	}
 }

pkg/genericapiserver/routes/BUILD

@@ -15,6 +15,7 @@ go_library(
     srcs = [
         "doc.go",
         "index.go",
+        "metrics.go",
         "openapi.go",
         "profiling.go",
         "swagger.go",
@@ -25,15 +26,18 @@ go_library(
     deps = [
         "//pkg/api/unversioned:go_default_library",
         "//pkg/apiserver:go_default_library",
+        "//pkg/apiserver/metrics:go_default_library",
         "//pkg/genericapiserver/mux:go_default_library",
         "//pkg/genericapiserver/openapi:go_default_library",
         "//pkg/genericapiserver/openapi/common:go_default_library",
         "//pkg/genericapiserver/routes/data/swagger:go_default_library",
+        "//pkg/storage/etcd/metrics:go_default_library",
         "//pkg/version:go_default_library",
         "//vendor:github.com/elazarl/go-bindata-assetfs",
         "//vendor:github.com/emicklei/go-restful",
         "//vendor:github.com/emicklei/go-restful/swagger",
         "//vendor:github.com/golang/glog",
+        "//vendor:github.com/prometheus/client_golang/prometheus",
     ],
 )
 

pkg/genericapiserver/routes/metrics.go

@@ -30,6 +30,7 @@ import (
 // DefaultMetrics installs the default prometheus metrics handler
 type DefaultMetrics struct{}
 
+// Install adds the DefaultMetrics handler
 func (m DefaultMetrics) Install(c *mux.APIContainer) {
 	c.NonSwaggerRoutes.Handle("/metrics", prometheus.Handler())
 }
@@ -38,6 +39,7 @@ func (m DefaultMetrics) Install(c *mux.APIContainer) {
 // which resets the metrics.
 type MetricsWithReset struct{}
 
+// Install adds the MetricsWithReset handler
 func (m MetricsWithReset) Install(c *mux.APIContainer) {
 	defaultMetricsHandler := prometheus.Handler().ServeHTTP
 	c.NonSwaggerRoutes.HandleFunc("/metrics", func(w http.ResponseWriter, req *http.Request) {

pkg/master/master.go

@@ -119,6 +119,9 @@ func (c *Config) Complete() completedConfig {
 		c.EndpointReconcilerConfig.Reconciler = NewMasterCountEndpointReconciler(c.GenericConfig.MasterCount, endpointClient)
 	}
 
+	// this has always been hardcoded true in the past
+	c.GenericConfig.EnableMetrics = true
+
 	return completedConfig{c}
 }
 
@@ -192,7 +195,9 @@ func (c completedConfig) New() (*Master, error) {
 	}
 	m.InstallAPIs(c.Config.GenericConfig.APIResourceConfigSource, restOptionsFactory.NewFor, restStorageProviders...)
 
-	m.InstallGeneralEndpoints(c.Config)
+	if c.Tunneler != nil {
+		m.installTunneler(c.Tunneler, coreclient.NewForConfigOrDie(c.GenericConfig.LoopbackClientConfig).Nodes())
+	}
 
 	return m, nil
 }
@@ -215,29 +220,13 @@ func (m *Master) InstallLegacyAPI(c *Config, restOptionsGetter genericapiserver.
 	}
 }
 
-// TODO this needs to be refactored so we have a way to add general health checks to genericapiserver
-// TODO profiling should be generic
-func (m *Master) InstallGeneralEndpoints(c *Config) {
-	// Run the tunneler.
-	healthzChecks := []healthz.HealthzChecker{}
-	if c.Tunneler != nil {
-		nodeClient := coreclient.NewForConfigOrDie(c.GenericConfig.LoopbackClientConfig).Nodes()
-		c.Tunneler.Run(nodeAddressProvider{nodeClient}.externalAddresses)
-
-		healthzChecks = append(healthzChecks, healthz.NamedCheck("SSH Tunnel Check", genericapiserver.TunnelSyncHealthChecker(c.Tunneler)))
-		prometheus.NewGaugeFunc(prometheus.GaugeOpts{
-			Name: "apiserver_proxy_tunnel_sync_latency_secs",
-			Help: "The time since the last successful synchronization of the SSH tunnels for proxy requests.",
-		}, func() float64 { return float64(c.Tunneler.SecondsSinceSync()) })
-	}
-	healthz.InstallHandler(&m.GenericAPIServer.HandlerContainer.NonSwaggerRoutes, healthzChecks...)
-
-	if c.GenericConfig.EnableProfiling {
-		routes.MetricsWithReset{}.Install(m.GenericAPIServer.HandlerContainer)
-	} else {
-		routes.DefaultMetrics{}.Install(m.GenericAPIServer.HandlerContainer)
-	}
-
+func (m *Master) installTunneler(tunneler genericapiserver.Tunneler, nodeClient coreclient.NodeInterface) {
+	tunneler.Run(nodeAddressProvider{nodeClient}.externalAddresses)
+	m.GenericAPIServer.AddHealthzChecks(healthz.NamedCheck("SSH Tunnel Check", genericapiserver.TunnelSyncHealthChecker(tunneler)))
+	prometheus.NewGaugeFunc(prometheus.GaugeOpts{
+		Name: "apiserver_proxy_tunnel_sync_latency_secs",
+		Help: "The time since the last successful synchronization of the SSH tunnels for proxy requests.",
+	}, func() float64 { return float64(tunneler.SecondsSinceSync()) })
 }
 
 // InstallAPIs will install the APIs for the restStorageProviders if they are enabled.

pkg/master/master_test.go

@@ -90,6 +90,7 @@ func setUp(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.
 	config.GenericConfig.APIResourceConfigSource = DefaultAPIResourceConfigSource()
 	config.GenericConfig.RequestContextMapper = api.NewRequestContextMapper()
 	config.GenericConfig.LoopbackClientConfig = &restclient.Config{APIPath: "/api", ContentConfig: restclient.ContentConfig{NegotiatedSerializer: api.Codecs}}
+	config.GenericConfig.EnableMetrics = true
 	config.EnableCoreControllers = false
 	config.KubeletClientConfig = kubeletclient.KubeletClientConfig{Port: 10250}
 	config.ProxyTransport = utilnet.SetTransportDefaults(&http.Transport{

pkg/routes/BUILD

@@ -15,15 +15,11 @@ go_library(
     srcs = [
         "doc.go",
         "logs.go",
-        "metrics.go",
         "ui.go",
     ],
     tags = ["automanaged"],
     deps = [
-        "//pkg/apiserver/metrics:go_default_library",
         "//pkg/genericapiserver/mux:go_default_library",
-        "//pkg/storage/etcd/metrics:go_default_library",
         "//vendor:github.com/emicklei/go-restful",
-        "//vendor:github.com/prometheus/client_golang/prometheus",
     ],
 )

test/integration/auth/rbac_test.go

@@ -475,4 +475,10 @@ func TestBootstrapping(t *testing.T) {
 	}
 
 	t.Errorf("missing cluster-admin: %v", clusterRoles)
+
+	healthBytes, err := clientset.Discovery().RESTClient().Get().AbsPath("/healthz/poststarthooks/rbac/bootstrap-roles").DoRaw()
+	if err != nil {
+		t.Error(err)
+	}
+	t.Errorf("expected %v, got %v", "asdf", string(healthBytes))
 }

test/integration/framework/master_utils.go

@@ -180,6 +180,7 @@ func startMasterOrDie(masterConfig *master.Config, incomingServer *httptest.Serv
 	if masterConfig == nil {
 		masterConfig = NewMasterConfig()
 		masterConfig.GenericConfig.EnableProfiling = true
+		masterConfig.GenericConfig.EnableMetrics = true
 		masterConfig.GenericConfig.EnableSwaggerSupport = true
 		masterConfig.GenericConfig.EnableOpenAPISupport = true
 		masterConfig.GenericConfig.OpenAPIConfig.Info = &spec.Info{
@@ -235,6 +236,12 @@ func startMasterOrDie(masterConfig *master.Config, incomingServer *httptest.Serv
 		masterReceiver.SetMaster(m)
 	}
 
+	// TODO have this start method actually use the normal start sequence for the API server
+	// this method never actually calls the `Run` method for the API server
+	// fire the post hooks ourselves
+	m.GenericAPIServer.PrepareRun()
+	m.GenericAPIServer.RunPostStartHooks()
+
 	cfg := *masterConfig.GenericConfig.LoopbackClientConfig
 	cfg.ContentConfig.GroupVersion = &unversioned.GroupVersion{}
 	privilegedClient, err := restclient.RESTClientFor(&cfg)
@@ -254,12 +261,6 @@ func startMasterOrDie(masterConfig *master.Config, incomingServer *httptest.Serv
 		glog.Fatal(err)
 	}
 
-	// TODO have this start method actually use the normal start sequence for the API server
-	// this method never actually calls the `Run` method for the API server
-	// fire the post hooks ourselves
-	m.GenericAPIServer.PrepareRun()
-	m.GenericAPIServer.RunPostStartHooks()
-
 	// wait for services to be ready
 	if masterConfig.EnableCoreControllers {
 		// TODO Once /healthz is updated for posthooks, we'll wait for good health
@@ -350,6 +351,7 @@ func NewMasterConfig() *master.Config {
 	genericConfig.APIResourceConfigSource = master.DefaultAPIResourceConfigSource()
 	genericConfig.Authorizer = authorizer.NewAlwaysAllowAuthorizer()
 	genericConfig.AdmissionControl = admit.NewAlwaysAdmit()
+	genericConfig.EnableMetrics = true
 
 	return &master.Config{
 		GenericConfig:         genericConfig,
@@ -450,6 +452,7 @@ func RunAMaster(masterConfig *master.Config) (*master.Master, *httptest.Server)
 	if masterConfig == nil {
 		masterConfig = NewMasterConfig()
 		masterConfig.GenericConfig.EnableProfiling = true
+		masterConfig.GenericConfig.EnableMetrics = true
 	}
 	return startMasterOrDie(masterConfig, nil, nil)
 }

test/integration/openshift/openshift_test.go

@@ -29,6 +29,7 @@ func TestMasterExportsSymbols(t *testing.T) {
 	_ = &master.Config{
 		GenericConfig: &genericapiserver.Config{
 			EnableSwaggerSupport: false,
+			EnableMetrics:        true,
 		},
 		EnableCoreControllers: false,
 		EnableUISupport:       false,