From dae1f062c0994d724478ca8e6024e2ad366cb2a8 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Thu, 4 Jul 2024 01:08:42 +0530 Subject: [PATCH 1/3] fix: add `readyz` endpoint Discourage the usage of `/metrics` for any probe, and use `/readyz` in place of the earlier telemetry metrics endpoint to secure the exposition data. Signed-off-by: Pranshu Srivastava --- README.md | 8 +++-- README.md.tpl | 8 +++-- examples/autosharding/statefulset.yaml | 6 ++-- examples/daemonsetsharding/daemonset.yaml | 6 ++-- .../deployment-no-node-pods.yaml | 6 ++-- examples/daemonsetsharding/deployment.yaml | 6 ++-- examples/standard/deployment.yaml | 6 ++-- .../kube-state-metrics.libsonnet | 6 ++-- pkg/app/server.go | 33 ++++++++++--------- 9 files changed, 46 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index f8654e69de..8b29929216 100644 --- a/README.md +++ b/README.md @@ -348,9 +348,11 @@ After running the above, if you see `Clusterrolebinding "cluster-admin-binding" The following healthcheck endpoints are available, some of which are used to determine the result of the aforementioned probes: -* `/livez`: Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to use this as a liveness probe. -* `/metrics`: Returns a 200 status code if the application is able to serve metrics. While this is available for both ports, we recommend to use the telemetry metrics endpoint as a readiness probe. -* `/healthz`: Returns a 200 status code if the application is running. We recommend to use this as a startup probe. +* `/healthz`: Returns a 200 status code if the application is running. We recommend to use this for the startup probe. +* `/livez`: Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. +* `/readyz`: Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. + +Note that it is discouraged to use the telemetry metrics endpoint for any probe when proxying the exposition data. #### Limited privileges environment diff --git a/README.md.tpl b/README.md.tpl index d5beb3b089..67facfae63 100644 --- a/README.md.tpl +++ b/README.md.tpl @@ -349,9 +349,11 @@ After running the above, if you see `Clusterrolebinding "cluster-admin-binding" The following healthcheck endpoints are available, some of which are used to determine the result of the aforementioned probes: -* `/livez`: Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to use this as a liveness probe. -* `/metrics`: Returns a 200 status code if the application is able to serve metrics. While this is available for both ports, we recommend to use the telemetry metrics endpoint as a readiness probe. -* `/healthz`: Returns a 200 status code if the application is running. We recommend to use this as a startup probe. +* `/healthz`: Returns a 200 status code if the application is running. We recommend to use this for the startup probe. +* `/livez`: Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. +* `/readyz`: Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. + +Note that it is discouraged to use the telemetry metrics endpoint for any probe when proxying the exposition data. #### Limited privileges environment diff --git a/examples/autosharding/statefulset.yaml b/examples/autosharding/statefulset.yaml index 2b3a8e39b5..059832e8f8 100644 --- a/examples/autosharding/statefulset.yaml +++ b/examples/autosharding/statefulset.yaml @@ -38,7 +38,7 @@ spec: livenessProbe: httpGet: path: /livez - port: 8080 + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 name: kube-state-metrics @@ -49,8 +49,8 @@ spec: name: telemetry readinessProbe: httpGet: - path: /metrics - port: 8081 + path: /readyz + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/daemonsetsharding/daemonset.yaml b/examples/daemonsetsharding/daemonset.yaml index e856c673f7..f5f8c0908c 100644 --- a/examples/daemonsetsharding/daemonset.yaml +++ b/examples/daemonsetsharding/daemonset.yaml @@ -33,7 +33,7 @@ spec: livenessProbe: httpGet: path: /livez - port: 8080 + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 name: kube-state-metrics-shard @@ -44,8 +44,8 @@ spec: name: telemetry readinessProbe: httpGet: - path: /metrics - port: 8081 + path: /readyz + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/daemonsetsharding/deployment-no-node-pods.yaml b/examples/daemonsetsharding/deployment-no-node-pods.yaml index 8f6e7b4482..dc484a61a1 100644 --- a/examples/daemonsetsharding/deployment-no-node-pods.yaml +++ b/examples/daemonsetsharding/deployment-no-node-pods.yaml @@ -28,7 +28,7 @@ spec: livenessProbe: httpGet: path: /livez - port: 8080 + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 name: kube-state-metrics @@ -39,8 +39,8 @@ spec: name: telemetry readinessProbe: httpGet: - path: /metrics - port: 8081 + path: /readyz + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/daemonsetsharding/deployment.yaml b/examples/daemonsetsharding/deployment.yaml index 3b51b034d6..f9ad51f31a 100644 --- a/examples/daemonsetsharding/deployment.yaml +++ b/examples/daemonsetsharding/deployment.yaml @@ -27,7 +27,7 @@ spec: livenessProbe: httpGet: path: /livez - port: 8080 + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 name: kube-state-metrics @@ -38,8 +38,8 @@ spec: name: telemetry readinessProbe: httpGet: - path: /metrics - port: 8081 + path: /readyz + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/standard/deployment.yaml b/examples/standard/deployment.yaml index ce74e4fbf9..b34a433774 100644 --- a/examples/standard/deployment.yaml +++ b/examples/standard/deployment.yaml @@ -25,7 +25,7 @@ spec: livenessProbe: httpGet: path: /livez - port: 8080 + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 name: kube-state-metrics @@ -36,8 +36,8 @@ spec: name: telemetry readinessProbe: httpGet: - path: /metrics - port: 8081 + path: /readyz + port: http-metrics initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet b/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet index 82695482b4..6a96112632 100644 --- a/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet +++ b/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet @@ -192,12 +192,12 @@ seccompProfile: { type: 'RuntimeDefault' }, }, livenessProbe: { timeoutSeconds: 5, initialDelaySeconds: 5, httpGet: { - port: 8080, + port: "http-metrics", path: '/livez', } }, readinessProbe: { timeoutSeconds: 5, initialDelaySeconds: 5, httpGet: { - port: 8081, - path: '/metrics', + port: "http-metrics", + path: '/readyz', } }, }; diff --git a/pkg/app/server.go b/pkg/app/server.go index 6a7db543e5..eee48009f8 100644 --- a/pkg/app/server.go +++ b/pkg/app/server.go @@ -62,6 +62,7 @@ const ( metricsPath = "/metrics" healthzPath = "/healthz" livezPath = "/livez" + readyzPath = "/readyz" ) // promLogger implements promhttp.Logger @@ -396,6 +397,19 @@ func buildTelemetryServer(registry prometheus.Gatherer) *http.ServeMux { return mux } +func handleClusterDelegationForProber(client kubernetes.Interface, probeType string) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + got := client.CoreV1().RESTClient().Get().AbsPath(probeType).Do(context.Background()) + if got.Error() != nil { + w.WriteHeader(http.StatusServiceUnavailable) + w.Write([]byte(http.StatusText(http.StatusServiceUnavailable))) + return + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(http.StatusText(http.StatusOK))) + } +} + func buildMetricsServer(m *metricshandler.MetricsHandler, durationObserver prometheus.ObserverVec, client kubernetes.Interface) *http.ServeMux { mux := http.NewServeMux() @@ -410,24 +424,13 @@ func buildMetricsServer(m *metricshandler.MetricsHandler, durationObserver prome mux.Handle(metricsPath, promhttp.InstrumentHandlerDuration(durationObserver, m)) // Add livezPath - mux.Handle(livezPath, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + mux.Handle(livezPath, handleClusterDelegationForProber(client, livezPath)) - // Query the Kube API to make sure we are not affected by a network outage. - got := client.CoreV1().RESTClient().Get().AbsPath("/livez").Do(context.Background()) - if got.Error() != nil { - w.WriteHeader(http.StatusServiceUnavailable) - w.Write([]byte(http.StatusText(http.StatusServiceUnavailable))) - return - } - w.WriteHeader(http.StatusOK) - w.Write([]byte(http.StatusText(http.StatusOK))) - })) + // Add readyzPath + mux.Handle(readyzPath, handleClusterDelegationForProber(client, readyzPath)) // Add healthzPath - mux.HandleFunc(healthzPath, func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write([]byte(http.StatusText(http.StatusOK))) - }) + mux.Handle(healthzPath, handleClusterDelegationForProber(client, healthzPath)) // Add index landingConfig := web.LandingConfig{ From b980579e0885b5f6fe9a6b47ff71b4da9fef647e Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Fri, 5 Jul 2024 18:27:48 +0530 Subject: [PATCH 2/3] fixup! fix: add `readyz` endpoint --- README.md | 8 ++++---- README.md.tpl | 8 ++++---- examples/autosharding/statefulset.yaml | 2 +- examples/daemonsetsharding/daemonset.yaml | 2 +- .../deployment-no-node-pods.yaml | 2 +- examples/daemonsetsharding/deployment.yaml | 2 +- examples/standard/deployment.yaml | 2 +- .../kube-state-metrics.libsonnet | 2 +- pkg/app/server.go | 16 +++++++++++++--- 9 files changed, 27 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 8b29929216..58dbf31b5a 100644 --- a/README.md +++ b/README.md @@ -346,11 +346,11 @@ After running the above, if you see `Clusterrolebinding "cluster-admin-binding" #### Healthcheck Endpoints -The following healthcheck endpoints are available, some of which are used to determine the result of the aforementioned probes: +The following healthcheck endpoints are available (`self` refers to the telemetry port, while `main` refers to the exposition port): -* `/healthz`: Returns a 200 status code if the application is running. We recommend to use this for the startup probe. -* `/livez`: Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. -* `/readyz`: Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. +* `/healthz` (exposed on `main`): Returns a 200 status code if the application is running. We recommend to use this for the startup probe. +* `/livez` (exposed on `main`): Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. +* `/readyz` (exposed on `self`): Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. Note that it is discouraged to use the telemetry metrics endpoint for any probe when proxying the exposition data. diff --git a/README.md.tpl b/README.md.tpl index 67facfae63..2c57c9990e 100644 --- a/README.md.tpl +++ b/README.md.tpl @@ -347,11 +347,11 @@ After running the above, if you see `Clusterrolebinding "cluster-admin-binding" #### Healthcheck Endpoints -The following healthcheck endpoints are available, some of which are used to determine the result of the aforementioned probes: +The following healthcheck endpoints are available (`self` refers to the telemetry port, while `main` refers to the exposition port): -* `/healthz`: Returns a 200 status code if the application is running. We recommend to use this for the startup probe. -* `/livez`: Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. -* `/readyz`: Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. +* `/healthz` (exposed on `main`): Returns a 200 status code if the application is running. We recommend to use this for the startup probe. +* `/livez` (exposed on `main`): Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. +* `/readyz` (exposed on `self`): Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. Note that it is discouraged to use the telemetry metrics endpoint for any probe when proxying the exposition data. diff --git a/examples/autosharding/statefulset.yaml b/examples/autosharding/statefulset.yaml index 059832e8f8..a000c2dd93 100644 --- a/examples/autosharding/statefulset.yaml +++ b/examples/autosharding/statefulset.yaml @@ -50,7 +50,7 @@ spec: readinessProbe: httpGet: path: /readyz - port: http-metrics + port: telemetry initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/daemonsetsharding/daemonset.yaml b/examples/daemonsetsharding/daemonset.yaml index f5f8c0908c..6f5d59a6b1 100644 --- a/examples/daemonsetsharding/daemonset.yaml +++ b/examples/daemonsetsharding/daemonset.yaml @@ -45,7 +45,7 @@ spec: readinessProbe: httpGet: path: /readyz - port: http-metrics + port: telemetry initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/daemonsetsharding/deployment-no-node-pods.yaml b/examples/daemonsetsharding/deployment-no-node-pods.yaml index dc484a61a1..268f8fb450 100644 --- a/examples/daemonsetsharding/deployment-no-node-pods.yaml +++ b/examples/daemonsetsharding/deployment-no-node-pods.yaml @@ -40,7 +40,7 @@ spec: readinessProbe: httpGet: path: /readyz - port: http-metrics + port: telemetry initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/daemonsetsharding/deployment.yaml b/examples/daemonsetsharding/deployment.yaml index f9ad51f31a..d54a215b1c 100644 --- a/examples/daemonsetsharding/deployment.yaml +++ b/examples/daemonsetsharding/deployment.yaml @@ -39,7 +39,7 @@ spec: readinessProbe: httpGet: path: /readyz - port: http-metrics + port: telemetry initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/examples/standard/deployment.yaml b/examples/standard/deployment.yaml index b34a433774..9130ae7497 100644 --- a/examples/standard/deployment.yaml +++ b/examples/standard/deployment.yaml @@ -37,7 +37,7 @@ spec: readinessProbe: httpGet: path: /readyz - port: http-metrics + port: telemetry initialDelaySeconds: 5 timeoutSeconds: 5 securityContext: diff --git a/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet b/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet index 6a96112632..1b2a157dca 100644 --- a/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet +++ b/jsonnet/kube-state-metrics/kube-state-metrics.libsonnet @@ -196,7 +196,7 @@ path: '/livez', } }, readinessProbe: { timeoutSeconds: 5, initialDelaySeconds: 5, httpGet: { - port: "http-metrics", + port: "telemetry", path: '/readyz', } }, }; diff --git a/pkg/app/server.go b/pkg/app/server.go index eee48009f8..a9f1e7c2e6 100644 --- a/pkg/app/server.go +++ b/pkg/app/server.go @@ -42,6 +42,7 @@ import ( versionCollector "github.com/prometheus/client_golang/prometheus/collectors/version" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/client_golang/prometheus/promhttp" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/prometheus/common/version" "github.com/prometheus/exporter-toolkit/web" @@ -377,6 +378,18 @@ func buildTelemetryServer(registry prometheus.Gatherer) *http.ServeMux { // Add metricsPath mux.Handle(metricsPath, promhttp.HandlerFor(registry, promhttp.HandlerOpts{ErrorLog: promLogger{}})) + // Add readyzPath + mux.Handle(readyzPath, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + count, err := testutil.GatherAndCount(registry) + if err != nil || count == 0 { + w.WriteHeader(http.StatusServiceUnavailable) + w.Write([]byte(http.StatusText(http.StatusServiceUnavailable))) + return + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(http.StatusText(http.StatusOK))) + })) + // Add index landingConfig := web.LandingConfig{ Name: "kube-state-metrics", @@ -426,9 +439,6 @@ func buildMetricsServer(m *metricshandler.MetricsHandler, durationObserver prome // Add livezPath mux.Handle(livezPath, handleClusterDelegationForProber(client, livezPath)) - // Add readyzPath - mux.Handle(readyzPath, handleClusterDelegationForProber(client, readyzPath)) - // Add healthzPath mux.Handle(healthzPath, handleClusterDelegationForProber(client, healthzPath)) From dbb0276a420b0ec9f172c66bbe5905e86e2764c6 Mon Sep 17 00:00:00 2001 From: Pranshu Srivastava Date: Mon, 8 Jul 2024 13:57:38 +0530 Subject: [PATCH 3/3] fixup! fixup! fix: add `readyz` endpoint --- README.md | 2 +- README.md.tpl | 2 +- pkg/app/server.go | 8 +++++--- pkg/util/utils.go | 20 +++++++++++++++++++- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 58dbf31b5a..5b4b2e214f 100644 --- a/README.md +++ b/README.md @@ -350,7 +350,7 @@ The following healthcheck endpoints are available (`self` refers to the telemetr * `/healthz` (exposed on `main`): Returns a 200 status code if the application is running. We recommend to use this for the startup probe. * `/livez` (exposed on `main`): Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. -* `/readyz` (exposed on `self`): Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. +* `/readyz` (exposed on `self`): Returns a 200 status code if the application is ready to accept requests and expose metrics. We recommend using this for the readiness probe. Note that it is discouraged to use the telemetry metrics endpoint for any probe when proxying the exposition data. diff --git a/README.md.tpl b/README.md.tpl index 2c57c9990e..5548b022f5 100644 --- a/README.md.tpl +++ b/README.md.tpl @@ -351,7 +351,7 @@ The following healthcheck endpoints are available (`self` refers to the telemetr * `/healthz` (exposed on `main`): Returns a 200 status code if the application is running. We recommend to use this for the startup probe. * `/livez` (exposed on `main`): Returns a 200 status code if the application is not affected by an outage of the Kubernetes API Server. We recommend to using this for the liveness probe. -* `/readyz` (exposed on `self`): Returns a 200 status code if the application is ready to accept traffic. We recommend using this for the readiness probe. +* `/readyz` (exposed on `self`): Returns a 200 status code if the application is ready to accept requests and expose metrics. We recommend using this for the readiness probe. Note that it is discouraged to use the telemetry metrics endpoint for any probe when proxying the exposition data. diff --git a/pkg/app/server.go b/pkg/app/server.go index a9f1e7c2e6..253d712fc7 100644 --- a/pkg/app/server.go +++ b/pkg/app/server.go @@ -42,7 +42,6 @@ import ( versionCollector "github.com/prometheus/client_golang/prometheus/collectors/version" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/client_golang/prometheus/promhttp" - "github.com/prometheus/client_golang/prometheus/testutil" "github.com/prometheus/common/version" "github.com/prometheus/exporter-toolkit/web" @@ -380,7 +379,7 @@ func buildTelemetryServer(registry prometheus.Gatherer) *http.ServeMux { // Add readyzPath mux.Handle(readyzPath, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - count, err := testutil.GatherAndCount(registry) + count, err := util.GatherAndCount(registry) if err != nil || count == 0 { w.WriteHeader(http.StatusServiceUnavailable) w.Write([]byte(http.StatusText(http.StatusServiceUnavailable))) @@ -440,7 +439,10 @@ func buildMetricsServer(m *metricshandler.MetricsHandler, durationObserver prome mux.Handle(livezPath, handleClusterDelegationForProber(client, livezPath)) // Add healthzPath - mux.Handle(healthzPath, handleClusterDelegationForProber(client, healthzPath)) + mux.HandleFunc(healthzPath, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(http.StatusText(http.StatusOK))) + }) // Add index landingConfig := web.LandingConfig{ diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 63e5aba650..14b108f37e 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -21,7 +21,6 @@ import ( "runtime" "strings" - "github.com/prometheus/common/version" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" @@ -32,6 +31,9 @@ import ( "k8s.io/klog/v2" testUnstructuredMock "k8s.io/sample-controller/pkg/apis/samplecontroller/v1alpha1" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/version" + "k8s.io/kube-state-metrics/v2/pkg/customresource" ) @@ -154,3 +156,19 @@ func GVRFromType(resourceName string, expectedType interface{}) *schema.GroupVer Resource: r, } } + +// GatherAndCount gathers all metrics from the provided Gatherer and counts +// them. It returns the number of metric children in all gathered metric +// families together. +func GatherAndCount(g prometheus.Gatherer) (int, error) { + got, err := g.Gather() + if err != nil { + return 0, fmt.Errorf("gathering metrics failed: %w", err) + } + + result := 0 + for _, mf := range got { + result += len(mf.GetMetric()) + } + return result, nil +}