chore: add e2e tests on operator metrics

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

pkg/k8sutil/metrics.go

@@ -37,7 +37,7 @@ type clientGoRateLimiterMetricAdapter struct {
 
 var _ = metrics.LatencyMetric(&clientGoRateLimiterMetricAdapter{})
 
-// MustRegisterClientGoMetrics registers k8s.io/client-go metrics.
+// MustRegisterClientGoMetrics registers the k8s.io/client-go metrics.
 // It panics if it encounters an error (e.g. metrics already registered).
 func MustRegisterClientGoMetrics(registerer prometheus.Registerer) {
 	httpMetrics := &clientGoHTTPMetricAdapter{
@@ -73,7 +73,8 @@ func MustRegisterClientGoMetrics(registerer prometheus.Registerer) {
 	// function can be called only once. To ensure that the k8s client metrics
 	// get updated, the global variables need to be set again here.
 	//
-	// Details:
+	// Details in:
+	// https://github.com/kubernetes-sigs/controller-runtime/issues/3054
 	// https://github.com/kubernetes-sigs/controller-runtime/blob/67b27f27e514bd9ac4cf9a2d84dec089ece95bf7/pkg/metrics/client_go_adapter.go#L42-L55
 	// https://github.com/kubernetes/client-go/blob/aa7909e7d7c0661792ba21b9e882f3cd6ad0ce53/tools/metrics/metrics.go#L129-L170
 	metrics.Register(

test/e2e/alertmanager_test.go

@@ -2177,7 +2177,7 @@ func testAMWeb(t *testing.T) {
 			}
 		}
 
-		reloadSuccessTimestamp, err := framework.GetMetricVal(context.Background(), "https", ns, podName, "8080", "reloader_last_reload_success_timestamp_seconds")
+		reloadSuccessTimestamp, err := framework.GetMetricValueFromPod(context.Background(), "https", ns, podName, "8080", "reloader_last_reload_success_timestamp_seconds")
 		if err != nil {
 			pollErr = err
 			return false, nil

test/e2e/main_test.go

@@ -76,12 +76,6 @@ func skipPromVersionUpgradeTests(t *testing.T) {
 	}
 }
 
-func skipAllNSTests(t *testing.T) {
-	if os.Getenv("EXCLUDE_ALL_NS_TESTS") != "" {
-		t.Skip("Skipping AllNS upgrade tests")
-	}
-}
-
 func skipFeatureGatedTests(t *testing.T) {
 	if os.Getenv("EXCLUDE_FEATURE_GATED_TESTS") != "" {
 		t.Skip("Skipping Feature Gated tests")
@@ -171,21 +165,27 @@ func TestMain(m *testing.M) {
 // TestAllNS tests the Prometheus Operator watching all namespaces in a
 // Kubernetes cluster.
 func TestAllNS(t *testing.T) {
-	skipAllNSTests(t)
+	ctx := context.Background()
 
 	testCtx := framework.NewTestCtx(t)
 	defer testCtx.Cleanup(t)
 
-	ns := framework.CreateNamespace(context.Background(), t, testCtx)
+	ns := framework.CreateNamespace(ctx, t, testCtx)
 
-	finalizers, err := framework.CreateOrUpdatePrometheusOperator(context.Background(), ns, nil, nil, nil, nil, true, true, true)
+	finalizers, err := framework.CreateOrUpdatePrometheusOperator(ctx, ns, nil, nil, nil, nil, true, true, true)
 	require.NoError(t, err)
 
 	for _, f := range finalizers {
 		testCtx.AddFinalizerFn(f)
 	}
 
-	t.Run("TestServerTLS", testServerTLS(context.Background(), ns))
+	t.Run("TestServerTLS", func(t *testing.T) {
+		testServerTLS(t, ns)
+	})
+	t.Run("TestPrometheusOperatorMetrics", func(t *testing.T) {
+		t.Helper()
+		testPrometheusOperatorMetrics(t, ns)
+	})
 
 	// t.Run blocks until the function passed as the second argument (f) returns or
 	// calls t.Parallel to become a parallel test. Run reports whether f succeeded
@@ -203,10 +203,10 @@ func TestAllNS(t *testing.T) {
 		"app.kubernetes.io/name": "prometheus-operator",
 	})).String()}
 
-	pl, err := framework.KubeClient.CoreV1().Pods(ns).List(context.Background(), opts)
+	pl, err := framework.KubeClient.CoreV1().Pods(ns).List(ctx, opts)
 	require.NoError(t, err)
 	require.Len(t, pl.Items, 1, "expected %v Prometheus Operator pods, but got %v", 1, len(pl.Items))
-	restarts, err := framework.GetPodRestartCount(context.Background(), ns, pl.Items[0].GetName())
+	restarts, err := framework.GetPodRestartCount(ctx, ns, pl.Items[0].GetName())
 	require.NoError(t, err)
 	require.Len(t, restarts, 1)
 	for _, restart := range restarts {
@@ -441,20 +441,9 @@ func TestPrometheusVersionUpgrade(t *testing.T) {
 	t.Run("PromVersionMigration", testPromVersionMigration)
 }
 
-func testServerTLS(ctx context.Context, namespace string) func(t *testing.T) {
-	return func(t *testing.T) {
-		skipPrometheusTests(t)
-		err := framework.WaitForServiceReady(context.Background(), namespace, prometheusOperatorServiceName)
-		require.NoError(t, err)
-
-		operatorService := framework.KubeClient.CoreV1().Services(namespace)
-		request := operatorService.ProxyGet("https", prometheusOperatorServiceName, "https", "/healthz", make(map[string]string))
-		_, err = request.DoRaw(ctx)
-		require.NoError(t, err)
-	}
-}
-
-// TestIsManagedByController test prometheus operator managing object with correct ControlerID.
+// testMultipleOperators checks that multiple Prometheus operator instances can
+// run concurrently without stepping on each other toes when properly
+// configured with ControllerID.
 func testMultipleOperators(testCtx *operatorFramework.TestCtx) func(t *testing.T) {
 	return func(t *testing.T) {
 		skipPrometheusTests(t)

test/e2e/prometheus_operator_test.go

@@ -0,0 +1,64 @@
+// Copyright 2020 The prometheus-operator 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 e2e
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// testServerTLS verifies that the Prometheus operator web server is working
+// with HTTPS.
+func testServerTLS(t *testing.T, namespace string) {
+	skipPrometheusTests(t)
+
+	ctx := context.Background()
+	err := framework.WaitForServiceReady(ctx, namespace, prometheusOperatorServiceName)
+	require.NoError(t, err)
+
+	operatorService := framework.KubeClient.CoreV1().Services(namespace)
+	request := operatorService.ProxyGet("https", prometheusOperatorServiceName, "https", "/healthz", nil)
+	_, err = request.DoRaw(ctx)
+	require.NoError(t, err)
+}
+
+// testPrometheusOperatorMetrics verifies that the Prometheus operator exposes
+// the expected metrics.
+func testPrometheusOperatorMetrics(t *testing.T, namespace string) {
+	skipPrometheusTests(t)
+
+	ctx := context.Background()
+	err := framework.WaitForServiceReady(ctx, namespace, prometheusOperatorServiceName)
+	require.NoError(t, err)
+
+	// Explicitly check the client-go metrics to validate the registration
+	// workaround we have in place due to
+	// https://github.com/kubernetes-sigs/controller-runtime/issues/3054
+	err = framework.EnsureMetricsFromService(
+		ctx,
+		"https",
+		namespace,
+		prometheusOperatorServiceName,
+		"https",
+		"prometheus_operator_kubernetes_client_http_requests_total",
+		"prometheus_operator_kubernetes_client_http_request_duration_seconds_count",
+		"prometheus_operator_kubernetes_client_http_request_duration_seconds_sum",
+		"prometheus_operator_kubernetes_client_rate_limiter_duration_seconds_count",
+		"prometheus_operator_kubernetes_client_rate_limiter_duration_seconds_sum",
+	)
+	require.NoError(t, err)
+}

test/e2e/prometheus_test.go

@@ -3893,7 +3893,7 @@ func testPromWebWithThanosSidecar(t *testing.T) {
 			}
 		}
 
-		reloadSuccessTimestamp, err := framework.GetMetricVal(context.Background(), "https", ns, podName, "8080", "reloader_last_reload_success_timestamp_seconds")
+		reloadSuccessTimestamp, err := framework.GetMetricValueFromPod(context.Background(), "https", ns, podName, "8080", "reloader_last_reload_success_timestamp_seconds")
 		if err != nil {
 			pollErr = err
 			return false, nil
@@ -3904,7 +3904,7 @@ func testPromWebWithThanosSidecar(t *testing.T) {
 			return false, nil
 		}
 
-		thanosSidecarPrometheusUp, err := framework.GetMetricVal(context.Background(), "http", ns, podName, "10902", "thanos_sidecar_prometheus_up")
+		thanosSidecarPrometheusUp, err := framework.GetMetricValueFromPod(context.Background(), "http", ns, podName, "10902", "thanos_sidecar_prometheus_up")
 		if err != nil {
 			pollErr = err
 			return false, nil

test/framework/alertmanager.go

@@ -404,22 +404,20 @@ func (f *Framework) WaitForAlertmanagerPodInitialized(ctx context.Context, ns, n
 	return nil
 }
 
-func (f *Framework) GetAlertmanagerPodStatus(ctx context.Context, ns, n string, https bool) (models.AlertmanagerStatus, error) {
+func (f *Framework) GetAlertmanagerPodStatus(ctx context.Context, ns, pod string, https bool) (models.AlertmanagerStatus, error) {
 	var amStatus models.AlertmanagerStatus
 
-	proxyName := n
+	var scheme string
 	if https {
-		proxyName = fmt.Sprintf("https:%v:", n)
+		scheme = "https"
 	}
 
-	request := f.ProxyGetPod(ns, proxyName, "/api/v2/status")
-	resp, err := request.DoRaw(ctx)
-
+	b, err := f.ProxyGetPod(ctx, scheme, ns, pod, "/api/v2/status")
 	if err != nil {
 		return amStatus, err
 	}
 
-	if err := json.Unmarshal(resp, &amStatus); err != nil {
+	if err := json.Unmarshal(b, &amStatus); err != nil {
 		return amStatus, err
 	}
 	return amStatus, nil
@@ -469,16 +467,15 @@ func (f *Framework) SendAlertToAlertmanager(ctx context.Context, ns, n string) e
 	return nil
 }
 
-func (f *Framework) GetSilences(ctx context.Context, ns, n string) (models.GettableSilences, error) {
+func (f *Framework) GetSilences(ctx context.Context, ns, pod string) (models.GettableSilences, error) {
 	var getSilencesResponse models.GettableSilences
 
-	request := f.ProxyGetPod(ns, n, "/api/v2/silences")
-	resp, err := request.DoRaw(ctx)
+	b, err := f.ProxyGetPod(ctx, "", ns, pod, "/api/v2/silences")
 	if err != nil {
 		return getSilencesResponse, err
 	}
 
-	if err := json.Unmarshal(resp, &getSilencesResponse); err != nil {
+	if err := json.Unmarshal(b, &getSilencesResponse); err != nil {
 		return getSilencesResponse, err
 	}
 
@@ -568,7 +565,7 @@ func (f *Framework) WaitForAlertmanagerConfigToContainString(ctx context.Context
 func (f *Framework) WaitForAlertmanagerConfigToBeReloaded(ctx context.Context, ns, amName string, previousReloadTimestamp time.Time) error {
 	const configReloadMetricName = "alertmanager_config_last_reload_success_timestamp_seconds"
 	err := wait.PollUntilContextTimeout(ctx, 10*time.Second, time.Minute*5, false, func(ctx context.Context) (bool, error) {
-		timestampSec, err := f.GetMetricVal(ctx, "https", ns, "alertmanager-"+amName+"-0", "", configReloadMetricName)
+		timestampSec, err := f.GetMetricValueFromPod(ctx, "https", ns, "alertmanager-"+amName+"-0", "", configReloadMetricName)
 		if err != nil {
 			return false, err
 		}

test/framework/helpers.go

@@ -161,22 +161,19 @@ func podRunsImage(p v1.Pod, image string) bool {
 	return false
 }
 
-// ProxyGetPod expects resourceName as "[protocol:]podName[:portNameOrNumber]".
-// protocol is optional and the valid values are "http" and "https".
-// Without specifying protocol, "http" will be used.
-// podName is mandatory.
-// portNameOrNumber is optional.
-// Without specifying portNameOrNumber, default port will be used.
-func (f *Framework) ProxyGetPod(namespace, resourceName, path string) *rest.Request {
-	return f.KubeClient.
+// ProxyGetPod executes an HTTP(S) request against the default port of the pod
+// using the Proxy API.
+func (f *Framework) ProxyGetPod(ctx context.Context, scheme, namespace, pod, path string) ([]byte, error) {
+	b, err := f.KubeClient.
 		CoreV1().
-		RESTClient().
-		Get().
-		Namespace(namespace).
-		Resource("pods").
-		SubResource("proxy").
-		Name(resourceName).
-		Suffix(path)
+		Pods(namespace).
+		ProxyGet(scheme, pod, "", path, nil).
+		DoRaw(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	return b, nil
 }
 
 // ProxyPostPod expects resourceName as "[protocol:]podName[:portNameOrNumber]".
@@ -199,32 +196,66 @@ func (f *Framework) ProxyPostPod(namespace, resourceName, path, body string) *re
 		SetHeader("Content-Type", "application/json")
 }
 
-// GetMetricVal get a particular metric value from a pod.
-// When portNumberOfName is "", default port will be used to access metrics endpoint.
-func (f *Framework) GetMetricVal(ctx context.Context, protocol, ns, podName, portNumberOrName, metricName string) (float64, error) {
-	resourceName := podName
-	if protocol == "" {
-		protocol = "http"
-	}
-	if portNumberOrName != "" {
-		resourceName = fmt.Sprintf("%s:%s:%s", protocol, podName, portNumberOrName)
-	}
-
-	request := f.ProxyGetPod(ns, resourceName, "/metrics")
-	resp, err := request.DoRaw(ctx)
+// GetMetricValueFromPod sends an HTTP(S) request to the /metrics endpoint of the pod
+// using the Proxy API, parses the response and returns the flot64 value of the
+// first series matching the metric name.
+// If protocol is empty, HTTP is used.
+// If portNumberOfName is empty, the default pod's port is used.
+func (f *Framework) GetMetricValueFromPod(ctx context.Context, protocol, ns, podName, portNumberOrName, metricName string) (float64, error) {
+	b, err := f.KubeClient.
+		CoreV1().
+		Pods(ns).
+		ProxyGet(protocol, podName, portNumberOrName, "/metrics", nil).
+		DoRaw(ctx)
 	if err != nil {
-		return 0, err
+		return 0, fmt.Errorf("error reading /metrics: %w", err)
 	}
 
-	parser := textparse.NewPromParser(resp, labels.NewSymbolTable())
+	return getMetricValue(b, metricName)
+}
+
+// GetMetricValueFromService sends an HTTP(S) request to the /metrics endpoint
+// of the service using the Proxy API, parses the response and returns the
+// flot64 value of the first series matching the metric name.
+// If protocol is empty, HTTP is used.
+// If portNumberOfName is empty, the default pod's port is used.
+func (f *Framework) EnsureMetricsFromService(ctx context.Context, protocol, ns, service, portNumberOrName string, metrics ...string) error {
+	if len(metrics) == 0 {
+		return fmt.Errorf("need to provide at least 1 metric to check")
+	}
+
+	b, err := f.KubeClient.
+		CoreV1().
+		Services(ns).
+		ProxyGet(protocol, service, portNumberOrName, "/metrics", nil).
+		DoRaw(ctx)
+	if err != nil {
+		return fmt.Errorf("error reading /metrics: %w", err)
+	}
+
+	for _, m := range metrics {
+		_, err = getMetricValue(b, m)
+		if err != nil {
+			return fmt.Errorf("metric %s: %w", m, err)
+		}
+	}
+
+	return nil
+}
+
+func getMetricValue(b []byte, metricName string) (float64, error) {
+	parser := textparse.NewPromParser(b, labels.NewSymbolTable())
+
 	for {
 		entry, err := parser.Next()
 		if err != nil {
 			return 0, err
 		}
+
 		if entry == textparse.EntryInvalid {
 			return 0, fmt.Errorf("invalid prometheus metric entry")
 		}
+
 		if entry != textparse.EntrySeries {
 			continue
 		}