From c6efaf16c1ed07ce37485b7a272628f653cbf06f Mon Sep 17 00:00:00 2001 From: Damien Grisonnet Date: Thu, 10 Aug 2023 17:01:17 +0200 Subject: [PATCH] apiserver/etcd3: fix segv during metric collection Fix a segfault when collecting the storage size metrics when the getters used to collect the data on etcd haven't been initialized properly. This happens when the EtcdOptions are not applied which is the case for aggregated apiservers that don't care about storage. Signed-off-by: Damien Grisonnet --- .../pkg/storage/etcd3/metrics/metrics.go | 2 +- .../pkg/storage/etcd3/metrics/metrics_test.go | 58 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics.go index 091eef85ab5..ac023d55d8c 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics.go @@ -85,7 +85,7 @@ var ( []string{"endpoint"}, ) storageSizeDescription = compbasemetrics.NewDesc("apiserver_storage_size_bytes", "Size of the storage database file physically allocated in bytes.", []string{"cluster"}, nil, compbasemetrics.ALPHA, "") - storageMonitor = &monitorCollector{} + storageMonitor = &monitorCollector{monitorGetter: func() ([]Monitor, error) { return nil, nil }} etcdEventsReceivedCounts = compbasemetrics.NewCounterVec( &compbasemetrics.CounterOpts{ Subsystem: "apiserver", diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics_test.go index 93d15e4af15..6190131f075 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/metrics_test.go @@ -17,6 +17,7 @@ limitations under the License. package metrics import ( + "context" "errors" "strings" "testing" @@ -179,3 +180,60 @@ etcd_request_errors_total{operation="foo",type="bar"} 1 }) } } + +func TestStorageSizeCollector(t *testing.T) { + registry := metrics.NewKubeRegistry() + registry.CustomMustRegister(storageMonitor) + + testCases := []struct { + desc string + getterOverride func() ([]Monitor, error) + err error + want string + }{ + { + desc: "fake etcd getter", + getterOverride: func() ([]Monitor, error) { + return []Monitor{fakeEtcdMonitor{storageSize: 1e9}}, nil + }, + err: nil, + want: `# HELP apiserver_storage_size_bytes [ALPHA] Size of the storage database file physically allocated in bytes. + # TYPE apiserver_storage_size_bytes gauge + apiserver_storage_size_bytes{cluster="etcd-0"} 1e+09 + `, + }, + { + desc: "getters not configured", + getterOverride: nil, + err: nil, + want: ``, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + defer registry.Reset() + if test.getterOverride != nil { + oldGetter := storageMonitor.monitorGetter + defer SetStorageMonitorGetter(oldGetter) + SetStorageMonitorGetter(test.getterOverride) + } + if err := testutil.GatherAndCompare(registry, strings.NewReader(test.want), "apiserver_storage_size_bytes"); err != nil { + t.Fatal(err) + } + }) + } + +} + +type fakeEtcdMonitor struct { + storageSize int64 +} + +func (m fakeEtcdMonitor) Monitor(_ context.Context) (StorageMetrics, error) { + return StorageMetrics{Size: m.storageSize}, nil +} + +func (m fakeEtcdMonitor) Close() error { + return nil +}