From 9a66116d6fa843b2f008c76394fdb171f95667eb Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Fri, 28 Jun 2024 10:11:45 +0100 Subject: [PATCH] Self review changes Signed-off-by: Richard Wall --- cmd/app/options/options.go | 8 +++++--- deploy/charts/csi-driver/README.md | 2 +- deploy/charts/csi-driver/values.schema.json | 2 +- deploy/charts/csi-driver/values.yaml | 3 ++- go.mod | 2 +- make/test-e2e.mk | 2 -- test/e2e/suite/cases/metrics.go | 18 +++++++++++++++++- 7 files changed, 27 insertions(+), 10 deletions(-) diff --git a/cmd/app/options/options.go b/cmd/app/options/options.go index 43e1eae5..c96ca8ca 100644 --- a/cmd/app/options/options.go +++ b/cmd/app/options/options.go @@ -70,7 +70,9 @@ type Options struct { // CMClient is a rest client for interacting with cert-manager resources. CMClient cmclient.Interface - // The host and port that the metrics endpoint should listen on. + // MetricsBindAddress is the TCP address for exposing HTTP Prometheus metrics + // which will be served on the HTTP path '/metrics'. The value "0" will + // disable exposing metrics. MetricsBindAddress string } @@ -156,6 +158,6 @@ func (o *Options) addAppFlags(fs *pflag.FlagSet) { fs.BoolVar(&o.UseTokenRequest, "use-token-request", false, "Use the empty audience token request for creating CertificateRequests. Requires the token request to be defined on the CSIDriver manifest.") fs.StringVar(&o.MetricsBindAddress, "metrics-bind-address", "0", - "The host and port that the metrics endpoint should listen on (for example ::9402). "+ - "If 0, the metrics server will be disabled. (default).") + "TCP address for exposing HTTP Prometheus metrics which will be served on the HTTP path '/metrics'. "+ + `The value "0" will disable exposing metrics.`) } diff --git a/deploy/charts/csi-driver/README.md b/deploy/charts/csi-driver/README.md index 5b7d2927..eab401d8 100644 --- a/deploy/charts/csi-driver/README.md +++ b/deploy/charts/csi-driver/README.md @@ -27,7 +27,7 @@ The TCP port on which the metrics server will listen. > false > ``` -Create a PodMonitor to add csi-driver to Prometheus. +Create a PodMonitor to add csi-driver to Prometheus if you are using Prometheus Operator. See https://prometheus-operator.dev/docs/operator/api/#monitoring.coreos.com/v1.PodMonitor #### **metrics.podmonitor.namespace** ~ `string` The namespace that the pod monitor should live in, defaults to the cert-manager-csi-driver namespace. diff --git a/deploy/charts/csi-driver/values.schema.json b/deploy/charts/csi-driver/values.schema.json index c7d75b4d..13b90212 100644 --- a/deploy/charts/csi-driver/values.schema.json +++ b/deploy/charts/csi-driver/values.schema.json @@ -300,7 +300,7 @@ }, "helm-values.metrics.podmonitor.enabled": { "default": false, - "description": "Create a PodMonitor to add csi-driver to Prometheus.", + "description": "Create a PodMonitor to add csi-driver to Prometheus if you are using Prometheus Operator. See https://prometheus-operator.dev/docs/operator/api/#monitoring.coreos.com/v1.PodMonitor", "type": "boolean" }, "helm-values.metrics.podmonitor.endpointAdditionalProperties": { diff --git a/deploy/charts/csi-driver/values.yaml b/deploy/charts/csi-driver/values.yaml index 0a2796f5..9783f21a 100644 --- a/deploy/charts/csi-driver/values.yaml +++ b/deploy/charts/csi-driver/values.yaml @@ -5,7 +5,8 @@ metrics: # The TCP port on which the metrics server will listen. port: 9402 podmonitor: - # Create a PodMonitor to add csi-driver to Prometheus. + # Create a PodMonitor to add csi-driver to Prometheus if you are using Prometheus Operator. + # See https://prometheus-operator.dev/docs/operator/api/#monitoring.coreos.com/v1.PodMonitor enabled: false # The namespace that the pod monitor should live in, defaults diff --git a/go.mod b/go.mod index 3e21621c..60e4f964 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/go-logr/logr v1.4.2 github.com/onsi/ginkgo/v2 v2.19.0 github.com/onsi/gomega v1.33.1 - github.com/prometheus/client_golang v1.18.0 github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 @@ -75,6 +74,7 @@ require ( github.com/peterbourgon/diskv v2.0.1+incompatible // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect + github.com/prometheus/client_golang v1.18.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.46.0 // indirect github.com/prometheus/procfs v0.15.0 // indirect diff --git a/make/test-e2e.mk b/make/test-e2e.mk index c6d4dce2..516eee18 100644 --- a/make/test-e2e.mk +++ b/make/test-e2e.mk @@ -34,7 +34,6 @@ e2e-setup-cert-manager: | kind-cluster $(NEEDS_HELM) $(NEEDS_KUBECTL) --set startupapicheck.image.repository=$(quay.io/jetstack/cert-manager-startupapicheck.REPO) \ --set startupapicheck.image.tag=$(quay.io/jetstack/cert-manager-startupapicheck.TAG) \ --set startupapicheck.image.pullPolicy=Never \ - --set prometheus.podmonitor.enabled=true \ cert-manager cert-manager >/dev/null # The "install" target can be run on its own with any currently active cluster, @@ -47,7 +46,6 @@ endif test-e2e-deps: INSTALL_OPTIONS := test-e2e-deps: INSTALL_OPTIONS += --set image.repository=$(oci_manager_image_name_development) -test-e2e-deps: INSTALL_OPTIONS += --set metrics.podmonitor.enabled=true test-e2e-deps: e2e-setup-cert-manager test-e2e-deps: install diff --git a/test/e2e/suite/cases/metrics.go b/test/e2e/suite/cases/metrics.go index 4a2b290b..d7a27592 100644 --- a/test/e2e/suite/cases/metrics.go +++ b/test/e2e/suite/cases/metrics.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The cert-manager 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 cases import ( @@ -17,7 +33,7 @@ const ( var _ = framework.CasesDescribe("Metrics", func() { f := framework.NewDefaultFramework("metrics") - FIt("Should serve Go and process metrics on port 9402", func() { + It("Should serve Go and process metrics on port 9402", func() { By("Discovering the name of the csi-driver Pod") pods, err := f.KubeClientSet.CoreV1().Pods(certManagerNamespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: "app.kubernetes.io/instance=csi-driver",