From eb55e2b980dafdb81608bdc0f851f8639e12a3af Mon Sep 17 00:00:00 2001 From: Richa Banker Date: Mon, 9 Jan 2023 17:33:44 -0800 Subject: [PATCH 1/2] Add e2e test for checking /metrics/slis endpoint for API server --- .../framework/metrics/api_server_metrics.go | 8 ++++++++ test/e2e/framework/metrics/metrics_grabber.go | 18 ++++++++++++++++++ .../monitoring/metrics_grabber.go | 10 ++++++++++ 3 files changed, 36 insertions(+) diff --git a/test/e2e/framework/metrics/api_server_metrics.go b/test/e2e/framework/metrics/api_server_metrics.go index c22c5c45a00..5d9e1bb5743 100644 --- a/test/e2e/framework/metrics/api_server_metrics.go +++ b/test/e2e/framework/metrics/api_server_metrics.go @@ -50,3 +50,11 @@ func (g *Grabber) getMetricsFromAPIServer(ctx context.Context) (string, error) { } return string(rawOutput), nil } + +func (g *Grabber) getMetricsSLIsFromAPIServer(ctx context.Context) (string, error) { + rawOutput, err := g.client.CoreV1().RESTClient().Get().RequestURI("/metrics/slis").Do(ctx).Raw() + if err != nil { + return "", err + } + return string(rawOutput), nil +} diff --git a/test/e2e/framework/metrics/metrics_grabber.go b/test/e2e/framework/metrics/metrics_grabber.go index f7c34d8ec77..87e1bbaec8d 100644 --- a/test/e2e/framework/metrics/metrics_grabber.go +++ b/test/e2e/framework/metrics/metrics_grabber.go @@ -54,6 +54,7 @@ var MetricsGrabbingDisabledError = errors.New("metrics grabbing disabled") // Collection is metrics collection of components type Collection struct { APIServerMetrics APIServerMetrics + APIServerMetricsSLIs APIServerMetrics ControllerManagerMetrics ControllerManagerMetrics SnapshotControllerMetrics SnapshotControllerMetrics KubeletMetrics map[string]KubeletMetrics @@ -323,6 +324,15 @@ func (g *Grabber) GrabFromAPIServer(ctx context.Context) (APIServerMetrics, erro return parseAPIServerMetrics(output) } +// GrabMetricsSLIsFromAPIServer returns metrics from API server +func (g *Grabber) GrabMetricsSLIsFromAPIServer(ctx context.Context) (APIServerMetrics, error) { + output, err := g.getMetricsSLIsFromAPIServer(ctx) + if err != nil { + return APIServerMetrics{}, err + } + return parseAPIServerMetrics(output) +} + // Grab returns metrics from corresponding component func (g *Grabber) Grab(ctx context.Context) (Collection, error) { result := Collection{} @@ -383,6 +393,14 @@ func (g *Grabber) Grab(ctx context.Context) (Collection, error) { } } } + if g.grabFromAPIServer { + metrics, err := g.GrabMetricsSLIsFromAPIServer(ctx) + if err != nil { + errs = append(errs, err) + } else { + result.APIServerMetricsSLIs = metrics + } + } if len(errs) > 0 { return result, fmt.Errorf("Errors while grabbing metrics: %v", errs) } diff --git a/test/e2e/instrumentation/monitoring/metrics_grabber.go b/test/e2e/instrumentation/monitoring/metrics_grabber.go index 4d2df772a05..e7e7c4f7a72 100644 --- a/test/e2e/instrumentation/monitoring/metrics_grabber.go +++ b/test/e2e/instrumentation/monitoring/metrics_grabber.go @@ -93,4 +93,14 @@ var _ = instrumentation.SIGDescribe("MetricsGrabber", func() { framework.ExpectNoError(err) gomega.Expect(response).NotTo(gomega.BeEmpty()) }) + + ginkgo.It("should grab all metrics slis from API server.", func(ctx context.Context) { + ginkgo.By("Connecting to /metrics/slis endpoint") + response, err := grabber.GrabMetricsSLIsFromAPIServer(ctx) + if errors.Is(err, e2emetrics.MetricsGrabbingDisabledError) { + e2eskipper.Skipf("%v", err) + } + framework.ExpectNoError(err) + gomega.Expect(response).NotTo(gomega.BeEmpty()) + }) }) From 452343367c21c68f325078357c5dfe794169422c Mon Sep 17 00:00:00 2001 From: Richa Banker Date: Mon, 9 Jan 2023 17:25:50 -0800 Subject: [PATCH 2/2] Enable ComponentSLIs as beta feature --- .../apiserver/pkg/server/config_test.go | 1 + .../metrics/features/kube_features.go | 2 +- test/e2e/framework/metrics/OWNERS | 14 +++++ .../framework/metrics/api_server_metrics.go | 18 ------ test/e2e/framework/metrics/kubelet_metrics.go | 25 --------- test/e2e/framework/metrics/metrics_grabber.go | 55 ++++++++++++++++--- 6 files changed, 63 insertions(+), 52 deletions(-) create mode 100644 test/e2e/framework/metrics/OWNERS diff --git a/staging/src/k8s.io/apiserver/pkg/server/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/config_test.go index fcbc15324d3..7ae05c2f960 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config_test.go @@ -170,6 +170,7 @@ func TestNewWithDelegate(t *testing.T) { "/livez/poststarthook/storage-object-count-tracker-hook", "/livez/poststarthook/wrapping-post-start-hook", "/metrics", + "/metrics/slis", "/readyz", "/readyz/delegate-health", "/readyz/informer-sync", diff --git a/staging/src/k8s.io/component-base/metrics/features/kube_features.go b/staging/src/k8s.io/component-base/metrics/features/kube_features.go index 3f17132149a..3cd6c22afae 100644 --- a/staging/src/k8s.io/component-base/metrics/features/kube_features.go +++ b/staging/src/k8s.io/component-base/metrics/features/kube_features.go @@ -29,7 +29,7 @@ const ( func featureGates() map[featuregate.Feature]featuregate.FeatureSpec { return map[featuregate.Feature]featuregate.FeatureSpec{ - ComponentSLIs: {Default: false, PreRelease: featuregate.Alpha}, + ComponentSLIs: {Default: true, PreRelease: featuregate.Beta}, } } diff --git a/test/e2e/framework/metrics/OWNERS b/test/e2e/framework/metrics/OWNERS new file mode 100644 index 00000000000..e31c11166ba --- /dev/null +++ b/test/e2e/framework/metrics/OWNERS @@ -0,0 +1,14 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +approvers: + - sig-instrumentation-approvers +emeritus_approvers: + - fabxc + - piosz + - fgrzadkowski + - kawych + - x13n +reviewers: + - sig-instrumentation-reviewers +labels: + - sig/instrumentation diff --git a/test/e2e/framework/metrics/api_server_metrics.go b/test/e2e/framework/metrics/api_server_metrics.go index 5d9e1bb5743..78b46c73372 100644 --- a/test/e2e/framework/metrics/api_server_metrics.go +++ b/test/e2e/framework/metrics/api_server_metrics.go @@ -17,8 +17,6 @@ limitations under the License. package metrics import ( - "context" - "k8s.io/component-base/metrics/testutil" ) @@ -42,19 +40,3 @@ func parseAPIServerMetrics(data string) (APIServerMetrics, error) { } return result, nil } - -func (g *Grabber) getMetricsFromAPIServer(ctx context.Context) (string, error) { - rawOutput, err := g.client.CoreV1().RESTClient().Get().RequestURI("/metrics").Do(ctx).Raw() - if err != nil { - return "", err - } - return string(rawOutput), nil -} - -func (g *Grabber) getMetricsSLIsFromAPIServer(ctx context.Context) (string, error) { - rawOutput, err := g.client.CoreV1().RESTClient().Get().RequestURI("/metrics/slis").Do(ctx).Raw() - if err != nil { - return "", err - } - return string(rawOutput), nil -} diff --git a/test/e2e/framework/metrics/kubelet_metrics.go b/test/e2e/framework/metrics/kubelet_metrics.go index 5118f6aa050..545f61388b6 100644 --- a/test/e2e/framework/metrics/kubelet_metrics.go +++ b/test/e2e/framework/metrics/kubelet_metrics.go @@ -93,31 +93,6 @@ func parseKubeletMetrics(data string) (KubeletMetrics, error) { return result, nil } -func (g *Grabber) getMetricsFromNode(ctx context.Context, nodeName string, kubeletPort int) (string, error) { - // There's a problem with timing out during proxy. Wrapping this in a goroutine to prevent deadlock. - finished := make(chan struct{}, 1) - var err error - var rawOutput []byte - go func() { - rawOutput, err = g.client.CoreV1().RESTClient().Get(). - Resource("nodes"). - SubResource("proxy"). - Name(fmt.Sprintf("%v:%v", nodeName, kubeletPort)). - Suffix("metrics"). - Do(ctx).Raw() - finished <- struct{}{} - }() - select { - case <-time.After(proxyTimeout): - return "", fmt.Errorf("Timed out when waiting for proxy to gather metrics from %v", nodeName) - case <-finished: - if err != nil { - return "", err - } - return string(rawOutput), nil - } -} - // KubeletLatencyMetric stores metrics scraped from the kubelet server's /metric endpoint. // TODO: Get some more structure around the metrics and this type type KubeletLatencyMetric struct { diff --git a/test/e2e/framework/metrics/metrics_grabber.go b/test/e2e/framework/metrics/metrics_grabber.go index 87e1bbaec8d..2fdcd842df4 100644 --- a/test/e2e/framework/metrics/metrics_grabber.go +++ b/test/e2e/framework/metrics/metrics_grabber.go @@ -195,6 +195,31 @@ func (g *Grabber) grabFromKubeletInternal(ctx context.Context, nodeName string, return parseKubeletMetrics(output) } +func (g *Grabber) getMetricsFromNode(ctx context.Context, nodeName string, kubeletPort int) (string, error) { + // There's a problem with timing out during proxy. Wrapping this in a goroutine to prevent deadlock. + finished := make(chan struct{}, 1) + var err error + var rawOutput []byte + go func() { + rawOutput, err = g.client.CoreV1().RESTClient().Get(). + Resource("nodes"). + SubResource("proxy"). + Name(fmt.Sprintf("%v:%v", nodeName, kubeletPort)). + Suffix("metrics"). + Do(ctx).Raw() + finished <- struct{}{} + }() + select { + case <-time.After(proxyTimeout): + return "", fmt.Errorf("Timed out when waiting for proxy to gather metrics from %v", nodeName) + case <-finished: + if err != nil { + return "", err + } + return string(rawOutput), nil + } +} + // GrabFromScheduler returns metrics from scheduler func (g *Grabber) GrabFromScheduler(ctx context.Context) (SchedulerMetrics, error) { if !g.grabFromScheduler { @@ -333,6 +358,22 @@ func (g *Grabber) GrabMetricsSLIsFromAPIServer(ctx context.Context) (APIServerMe return parseAPIServerMetrics(output) } +func (g *Grabber) getMetricsFromAPIServer(ctx context.Context) (string, error) { + rawOutput, err := g.client.CoreV1().RESTClient().Get().RequestURI("/metrics").Do(ctx).Raw() + if err != nil { + return "", err + } + return string(rawOutput), nil +} + +func (g *Grabber) getMetricsSLIsFromAPIServer(ctx context.Context) (string, error) { + rawOutput, err := g.client.CoreV1().RESTClient().Get().RequestURI("/metrics/slis").Do(ctx).Raw() + if err != nil { + return "", err + } + return string(rawOutput), nil +} + // Grab returns metrics from corresponding component func (g *Grabber) Grab(ctx context.Context) (Collection, error) { result := Collection{} @@ -344,6 +385,12 @@ func (g *Grabber) Grab(ctx context.Context) (Collection, error) { } else { result.APIServerMetrics = metrics } + metrics, err = g.GrabMetricsSLIsFromAPIServer(ctx) + if err != nil { + errs = append(errs, err) + } else { + result.APIServerMetricsSLIs = metrics + } } if g.grabFromScheduler { metrics, err := g.GrabFromScheduler(ctx) @@ -393,14 +440,6 @@ func (g *Grabber) Grab(ctx context.Context) (Collection, error) { } } } - if g.grabFromAPIServer { - metrics, err := g.GrabMetricsSLIsFromAPIServer(ctx) - if err != nil { - errs = append(errs, err) - } else { - result.APIServerMetricsSLIs = metrics - } - } if len(errs) > 0 { return result, fmt.Errorf("Errors while grabbing metrics: %v", errs) }