Skip to content

Commit e818759

Browse files
wallrjwallrj
committed
Use controller-runtime metrics server
Signed-off-by: Richard Wall <[email protected]>
1 parent a824729 commit e818759

File tree

10 files changed

+101
-74
lines changed

10 files changed

+101
-74
lines changed

‎cmd/app/app.go

Lines changed: 53 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,8 @@ import (
2323
"crypto/x509"
2424
"encoding/pem"
2525
"fmt"
26-
"time"
26+
"net/http"
2727

28-
"github.com/cert-manager/cert-manager/pkg/server"
2928
"github.com/cert-manager/csi-lib/driver"
3029
"github.com/cert-manager/csi-lib/manager"
3130
"github.com/cert-manager/csi-lib/manager/util"
@@ -34,9 +33,10 @@ import (
3433
"github.com/spf13/cobra"
3534
"golang.org/x/sync/errgroup"
3635
"k8s.io/utils/clock"
36+
ctrl "sigs.k8s.io/controller-runtime"
37+
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
3738

3839
"github.com/cert-manager/csi-driver/cmd/app/options"
39-
"github.com/cert-manager/csi-driver/internal/metrics"
4040
"github.com/cert-manager/csi-driver/internal/version"
4141
csiapi "github.com/cert-manager/csi-driver/pkg/apis/v1alpha1"
4242
"github.com/cert-manager/csi-driver/pkg/filestore"
@@ -61,8 +61,11 @@ func NewCommand(ctx context.Context) *cobra.Command {
6161
},
6262
RunE: func(cmd *cobra.Command, args []string) error {
6363
log := opts.Logr.WithName("main")
64-
log.Info("Starting driver", "version", version.VersionInfo())
64+
// Set the controller-runtime logger so that we get the
65+
// controller-runtime metricsserver logs.
66+
ctrl.SetLogger(log)
6567

68+
log.Info("Starting driver", "version", version.VersionInfo())
6669
store, err := storage.NewFilesystem(opts.Logr.WithName("storage"), opts.DataRoot)
6770
if err != nil {
6871
return fmt.Errorf("failed to setup filesystem: %w", err)
@@ -100,14 +103,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
100103
return fmt.Errorf("failed to setup driver: " + err.Error())
101104
}
102105

103-
// Start metrics server
104-
metricsLn, err := server.Listen("tcp", opts.MetricsListenAddress)
105-
if err != nil {
106-
return fmt.Errorf("failed to listen on prometheus address %s: %v", opts.MetricsListenAddress, err)
107-
}
108-
metricsServer := metrics.NewServer(metricsLn)
109-
110-
g, _ := errgroup.WithContext(ctx)
106+
g, gCTX := errgroup.WithContext(ctx)
111107
g.Go(func() error {
112108
<-ctx.Done()
113109
log.Info("shutting down driver", "context", ctx.Err())
@@ -123,21 +119,51 @@ func NewCommand(ctx context.Context) *cobra.Command {
123119
return nil
124120
})
125121

126-
g.Go(func() error {
127-
<-ctx.Done()
128-
// allow a timeout for graceful shutdown
129-
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
130-
defer cancel()
131-
132-
// nolint: contextcheck
133-
return metricsServer.Shutdown(shutdownCtx)
134-
})
135-
136-
g.Go(func() error {
137-
log.V(3).Info("starting metrics server", "address", metricsLn.Addr())
138-
return metricsServer.Serve(metricsLn)
139-
})
140-
122+
// Start a metrics server if the --metrics-bind-address is not "0".
123+
//
124+
// By default this will serve all the metrics that are registered by
125+
// controller-runtime to its global metrics registry. Including:
126+
// * Go Runtime metrics
127+
// * Process metrics
128+
// * Various controller-runtime controller metrics
129+
// (not updated by csi-driver because it doesn't use controller-runtime)
130+
// * Leader election metrics
131+
// (not updated by csi-driver because it doesn't use leader-election)
132+
//
133+
// The full list is here:
134+
// https://github.com/kubernetes-sigs/controller-runtime/blob/700befecdffa803d19830a6a43adc5779ed01e26/pkg/internal/controller/metrics/metrics.go#L73-L86
135+
//
136+
// The advantages of using the controller-runtime metricsserver are:
137+
// * It already exists and is actively maintained.
138+
// * Provides optional features for securing the metrics endpoint by
139+
// TLS and by authentication with a K8S service account token,
140+
// should that be requested by users in the future.
141+
// * Consistency with cert-manager/approver-policy, which also uses
142+
// this library and therefore publishes the same set of
143+
// controller-runtime base metrics.
144+
// Disadvantages:
145+
// * It introduces a dependency on controller-runtime, which often
146+
// introduces breaking changes.
147+
// * It uses a global metrics registry, which has the usual risks
148+
// associated with globals and makes it difficult for us to control
149+
// which metrics are published for csi-driver.
150+
// https://github.com/kubernetes-sigs/controller-runtime/issues/210
151+
var unusedHttpClient *http.Client
152+
metricsServer, err := metricsserver.NewServer(
153+
metricsserver.Options{
154+
BindAddress: opts.MetricsBindAddress,
155+
},
156+
opts.RestConfig,
157+
unusedHttpClient,
158+
)
159+
if err != nil {
160+
return err
161+
}
162+
if metricsServer != nil {
163+
g.Go(func() error {
164+
return metricsServer.Start(gCTX)
165+
})
166+
}
141167
return g.Wait()
142168
},
143169
}

‎cmd/app/options/options.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ import (
3232
_ "k8s.io/client-go/plugin/pkg/client/auth"
3333
)
3434

35-
const defaultPrometheusMetricsServerAddress = "0.0.0.0:9402"
36-
3735
// Options are the main options for the driver. Populated via processing
3836
// command line flags.
3937
type Options struct {
@@ -73,7 +71,7 @@ type Options struct {
7371
CMClient cmclient.Interface
7472

7573
// The host and port that the metrics endpoint should listen on.
76-
MetricsListenAddress string
74+
MetricsBindAddress string
7775
}
7876

7977
func New() *Options {
@@ -157,6 +155,7 @@ func (o *Options) addAppFlags(fs *pflag.FlagSet) {
157155

158156
fs.BoolVar(&o.UseTokenRequest, "use-token-request", false,
159157
"Use the empty audience token request for creating CertificateRequests. Requires the token request to be defined on the CSIDriver manifest.")
160-
fs.StringVar(&o.MetricsListenAddress, "metrics-listen-address", defaultPrometheusMetricsServerAddress,
161-
"The host and port that the metrics endpoint should listen on.")
158+
fs.StringVar(&o.MetricsBindAddress, "metrics-bind-address", "0",
159+
"The host and port that the metrics endpoint should listen on (for example ::9402). "+
160+
"If 0, the metrics server will be disabled. (default).")
162161
}

‎deploy/charts/csi-driver/README.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,14 @@
1313
> ```
1414
1515
Enable the metrics server on csi-driver pods.
16-
If false, then the other metrics fields below will be ignored.
16+
If false, the metrics server will be disabled and the other metrics fields below will be ignored.
17+
#### **metrics.port** ~ `number`
18+
> Default value:
19+
> ```yaml
20+
> 9402
21+
> ```
22+
23+
The TCP port on which the metrics server will listen.
1724
#### **metrics.podmonitor.enabled** ~ `bool`
1825
> Default value:
1926
> ```yaml

‎deploy/charts/csi-driver/templates/daemonset.yaml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ spec:
8484
- --endpoint=$(CSI_ENDPOINT)
8585
- --data-root=csi-data-dir
8686
- --use-token-request={{ .Values.app.driver.useTokenRequest }}
87+
{{- if .Values.metrics.enabled }}
88+
- --metrics-bind-address=:{{ .Values.metrics.port }}
89+
{{- else }}
90+
- --metrics-bind-address=0
91+
{{- end }}
8792
env:
8893
- name: NODE_ID
8994
valueFrom:
@@ -103,8 +108,10 @@ spec:
103108
ports:
104109
- containerPort: {{.Values.app.livenessProbe.port}}
105110
name: healthz
106-
- containerPort: 9402
111+
{{- if .Values.metrics.enabled }}
112+
- containerPort: {{ .Values.metrics.port }}
107113
name: http-metrics
114+
{{- end }}
108115
livenessProbe:
109116
httpGet:
110117
path: /healthz

‎deploy/charts/csi-driver/values.schema.json

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,13 +248,16 @@
248248
},
249249
"podmonitor": {
250250
"$ref": "#/$defs/helm-values.metrics.podmonitor"
251+
},
252+
"port": {
253+
"$ref": "#/$defs/helm-values.metrics.port"
251254
}
252255
},
253256
"type": "object"
254257
},
255258
"helm-values.metrics.enabled": {
256259
"default": true,
257-
"description": "Enable the metrics server on csi-driver pods.\nIf false, then the other metrics fields below will be ignored.",
260+
"description": "Enable the metrics server on csi-driver pods.\nIf false, the metrics server will be disabled and the other metrics fields below will be ignored.",
258261
"type": "boolean"
259262
},
260263
"helm-values.metrics.podmonitor": {
@@ -334,6 +337,11 @@
334337
"description": "The timeout before a metrics scrape fails.",
335338
"type": "string"
336339
},
340+
"helm-values.metrics.port": {
341+
"default": 9402,
342+
"description": "The TCP port on which the metrics server will listen.",
343+
"type": "number"
344+
},
337345
"helm-values.nodeDriverRegistrarImage": {
338346
"additionalProperties": false,
339347
"properties": {

‎deploy/charts/csi-driver/values.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
metrics:
22
# Enable the metrics server on csi-driver pods.
3-
# If false, then the other metrics fields below will be ignored.
3+
# If false, the metrics server will be disabled and the other metrics fields below will be ignored.
44
enabled: true
5+
# The TCP port on which the metrics server will listen.
6+
port: 9402
57
podmonitor:
68
# Create a PodMonitor to add csi-driver to Prometheus.
79
enabled: false

‎go.mod

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ require (
3535
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
3636
github.com/emicklei/go-restful/v3 v3.12.0 // indirect
3737
github.com/evanphx/json-patch v5.9.0+incompatible // indirect
38+
github.com/evanphx/json-patch/v5 v5.9.0 // indirect
39+
github.com/fsnotify/fsnotify v1.7.0 // indirect
3840
github.com/go-asn1-ber/asn1-ber v1.5.6 // indirect
3941
github.com/go-errors/errors v1.4.2 // indirect
4042
github.com/go-ldap/ldap/v3 v3.4.8 // indirect
@@ -44,6 +46,7 @@ require (
4446
github.com/go-openapi/swag v0.23.0 // indirect
4547
github.com/go-task/slim-sprig/v3 v3.0.0 // indirect
4648
github.com/gogo/protobuf v1.3.2 // indirect
49+
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
4750
github.com/golang/protobuf v1.5.4 // indirect
4851
github.com/google/btree v1.1.2 // indirect
4952
github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
@@ -80,13 +83,15 @@ require (
8083
go.uber.org/multierr v1.11.0 // indirect
8184
go.uber.org/zap v1.27.0 // indirect
8285
golang.org/x/crypto v0.23.0 // indirect
86+
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
8387
golang.org/x/net v0.25.0 // indirect
8488
golang.org/x/oauth2 v0.20.0 // indirect
8589
golang.org/x/sys v0.20.0 // indirect
8690
golang.org/x/term v0.20.0 // indirect
8791
golang.org/x/text v0.15.0 // indirect
8892
golang.org/x/time v0.5.0 // indirect
8993
golang.org/x/tools v0.21.0 // indirect
94+
gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
9095
google.golang.org/genproto/googleapis/rpc v0.0.0-20240515191416-fc5f0ca64291 // indirect
9196
google.golang.org/grpc v1.64.0 // indirect
9297
google.golang.org/protobuf v1.34.1 // indirect

‎go.sum

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m
3838
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
3939
github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lShz4oaXpDTX2bLe7ls=
4040
github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
41+
github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
42+
github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
43+
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
44+
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
4145
github.com/go-asn1-ber/asn1-ber v1.5.5/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0=
4246
github.com/go-asn1-ber/asn1-ber v1.5.6 h1:CYsqysemXfEaQbyrLJmdsCRuufHoLa3P/gGWGl5TDrM=
4347
github.com/go-asn1-ber/asn1-ber v1.5.6/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0=
@@ -60,6 +64,8 @@ github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZ
6064
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
6165
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
6266
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
67+
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE=
68+
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
6369
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
6470
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
6571
github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
@@ -217,6 +223,8 @@ golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOM
217223
golang.org/x/crypto v0.23.0 h1:dIJU/v2J8Mdglj/8rJ6UUOM3Zc9zLZxVZwwxMooUSAI=
218224
golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8=
219225
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
226+
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 h1:vr/HnozRka3pE4EsMEg1lgkXJkTFJCVUX+S/ZT6wYzM=
227+
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842/go.mod h1:XtvwrStGgqGPLc4cjQfWqZHG1YFdYs6swckp8vpsjnc=
220228
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
221229
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
222230
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
@@ -305,6 +313,8 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T
305313
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
306314
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
307315
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
316+
gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw=
317+
gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY=
308318
google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
309319
google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
310320
google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=

‎internal/metrics/metrics.go

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎test/e2e/suite/cases/metrics.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const (
1717
var _ = framework.CasesDescribe("Metrics", func() {
1818
f := framework.NewDefaultFramework("metrics")
1919

20-
It("Should serve Go and process metrics on port 9402", func() {
20+
FIt("Should serve Go and process metrics on port 9402", func() {
2121
By("Discovering the name of the csi-driver Pod")
2222
pods, err := f.KubeClientSet.CoreV1().Pods(certManagerNamespace).List(context.TODO(), metav1.ListOptions{
2323
LabelSelector: "app.kubernetes.io/instance=csi-driver",

0 commit comments

Comments
 (0)