From b449b448564dab418a335f5be9dc358a07b30b87 Mon Sep 17 00:00:00 2001 From: Mengjiao Liu Date: Fri, 29 Jul 2022 13:07:14 +0800 Subject: [PATCH] Cleanup: remove prometheus dependencies for volume Co-authored-by: gavinfish --- hack/verify-prometheus-imports.sh | 1 - pkg/volume/util/metrics.go | 7 +- .../operation_generator_test.go | 73 ++----------------- 3 files changed, 11 insertions(+), 70 deletions(-) diff --git a/hack/verify-prometheus-imports.sh b/hack/verify-prometheus-imports.sh index 3572a3d905f..e46a2833e19 100755 --- a/hack/verify-prometheus-imports.sh +++ b/hack/verify-prometheus-imports.sh @@ -37,7 +37,6 @@ source "${KUBE_ROOT}/hack/lib/util.sh" # See: https://github.com/kubernetes/kubernetes/issues/89267 allowed_prometheus_importers=( ./cluster/images/etcd-version-monitor/etcd-version-monitor.go - ./pkg/volume/util/operationexecutor/operation_generator_test.go ./staging/src/k8s.io/component-base/metrics/prometheusextension/timing_histogram.go ./staging/src/k8s.io/component-base/metrics/prometheusextension/timing_histogram_test.go ./staging/src/k8s.io/component-base/metrics/prometheusextension/timing_histogram_vec.go diff --git a/pkg/volume/util/metrics.go b/pkg/volume/util/metrics.go index e80008f7464..544ecf8c70e 100644 --- a/pkg/volume/util/metrics.go +++ b/pkg/volume/util/metrics.go @@ -44,7 +44,8 @@ const ( * involves explicitly acknowledging support for the metric across multiple releases, in accordance with * the metric stability policy. */ -var storageOperationMetric = metrics.NewHistogramVec( + +var StorageOperationMetric = metrics.NewHistogramVec( &metrics.HistogramOpts{ Name: "storage_operation_duration_seconds", Help: "Storage operation duration", @@ -82,7 +83,7 @@ func init() { func registerMetrics() { // legacyregistry is the internal k8s wrapper around the prometheus // global registry, used specifically for metric stability enforcement - legacyregistry.MustRegister(storageOperationMetric) + legacyregistry.MustRegister(StorageOperationMetric) legacyregistry.MustRegister(storageOperationEndToEndLatencyMetric) legacyregistry.MustRegister(csiOperationsLatencyMetric) } @@ -103,7 +104,7 @@ func OperationCompleteHook(plugin, operationName string) func(types.CompleteFunc if c.Migrated != nil { migrated = *c.Migrated } - storageOperationMetric.WithLabelValues(plugin, operationName, status, strconv.FormatBool(migrated)).Observe(timeTaken) + StorageOperationMetric.WithLabelValues(plugin, operationName, status, strconv.FormatBool(migrated)).Observe(timeTaken) } return opComplete } diff --git a/pkg/volume/util/operationexecutor/operation_generator_test.go b/pkg/volume/util/operationexecutor/operation_generator_test.go index 1ae2994deb1..23387554f90 100644 --- a/pkg/volume/util/operationexecutor/operation_generator_test.go +++ b/pkg/volume/util/operationexecutor/operation_generator_test.go @@ -28,20 +28,20 @@ import ( "os" "testing" - io_prometheus_client "github.com/prometheus/client_model/go" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" fakeclient "k8s.io/client-go/kubernetes/fake" - "k8s.io/component-base/metrics/legacyregistry" + "k8s.io/component-base/metrics/testutil" "k8s.io/csi-translation-lib/plugins" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/awsebs" csitesting "k8s.io/kubernetes/pkg/volume/csi/testing" "k8s.io/kubernetes/pkg/volume/gcepd" volumetesting "k8s.io/kubernetes/pkg/volume/testing" + "k8s.io/kubernetes/pkg/volume/util" volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) @@ -91,30 +91,15 @@ func TestOperationGenerator_GenerateUnmapVolumeFunc_PluginName(t *testing.T) { t.Fatalf("Error occurred while generating unmapVolumeFunc: %v", e) } - metricFamilyName := "storage_operation_duration_seconds" - labelFilter := map[string]string{ - "migrated": "false", - "status": "success", - "operation_name": "unmap_volume", - "volume_plugin": expectedPluginName, - } - // compare the relative change of the metric because of the global state of the prometheus.DefaultGatherer.Gather() - storageOperationDurationSecondsMetricBefore := findMetricWithNameAndLabels(metricFamilyName, labelFilter) + m := util.StorageOperationMetric.WithLabelValues(expectedPluginName, "unmap_volume", "success", "false") + storageOperationDurationSecondsMetricBefore, _ := testutil.GetHistogramMetricCount(m) var ee error unmapVolumeFunc.CompleteFunc(volumetypes.CompleteFuncParam{Err: &ee}) - storageOperationDurationSecondsMetricAfter := findMetricWithNameAndLabels(metricFamilyName, labelFilter) - if storageOperationDurationSecondsMetricAfter == nil { - t.Fatalf("Couldn't find the metric with name(%s) and labels(%v)", metricFamilyName, labelFilter) - } - - if storageOperationDurationSecondsMetricBefore == nil { - assert.Equal(t, uint64(1), *storageOperationDurationSecondsMetricAfter.Histogram.SampleCount, tc.name) - } else { - metricValueDiff := *storageOperationDurationSecondsMetricAfter.Histogram.SampleCount - *storageOperationDurationSecondsMetricBefore.Histogram.SampleCount - assert.Equal(t, uint64(1), metricValueDiff, tc.name) - } + storageOperationDurationSecondsMetricAfter, _ := testutil.GetHistogramMetricCount(m) + metricValueDiff := storageOperationDurationSecondsMetricAfter - storageOperationDurationSecondsMetricBefore + assert.Equal(t, uint64(1), metricValueDiff, tc.name) } } @@ -406,40 +391,6 @@ func getTestPV(volumeName string, specSize string) *v1.PersistentVolume { } } -func findMetricWithNameAndLabels(metricFamilyName string, labelFilter map[string]string) *io_prometheus_client.Metric { - metricFamily := getMetricFamily(metricFamilyName) - if metricFamily == nil { - return nil - } - - for _, metric := range metricFamily.GetMetric() { - if isLabelsMatchWithMetric(labelFilter, metric) { - return metric - } - } - - return nil -} - -func isLabelsMatchWithMetric(labelFilter map[string]string, metric *io_prometheus_client.Metric) bool { - if len(labelFilter) != len(metric.Label) { - return false - } - for labelName, labelValue := range labelFilter { - labelFound := false - for _, labelPair := range metric.Label { - if labelName == *labelPair.Name && labelValue == *labelPair.Value { - labelFound = true - break - } - } - if !labelFound { - return false - } - } - return true -} - func getTestOperationGenerator(volumePluginMgr *volume.VolumePluginMgr, objects ...runtime.Object) OperationGenerator { fakeKubeClient := fakeclient.NewSimpleClientset(objects...) fakeRecorder := &record.FakeRecorder{} @@ -492,16 +443,6 @@ func getTestVolumeToUnmount(pod *v1.Pod, pvSpec v1.PersistentVolumeSpec, pluginN return volumeToUnmount } -func getMetricFamily(metricFamilyName string) *io_prometheus_client.MetricFamily { - metricFamilies, _ := legacyregistry.DefaultGatherer.Gather() - for _, mf := range metricFamilies { - if *mf.Name == metricFamilyName { - return mf - } - } - return nil -} - func initTestPlugins(t *testing.T, plugs []volume.VolumePlugin, pluginName string) (*volume.VolumePluginMgr, string) { client := fakeclient.NewSimpleClientset() pluginMgr, _, tmpDir := csitesting.NewTestPlugin(t, client)