From 16c2ad5b842f23eda3674d3cd908de359554d5dc Mon Sep 17 00:00:00 2001 From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com> Date: Tue, 23 Jul 2024 15:21:29 -0400 Subject: [PATCH] Add labels to PVCollector bound/unbound PVC metrics for VolumeAttributesClass Feature (#126166) * Add labels to PVCollector bound/unbound PVC metrics * fixup! Add labels to PVCollector bound/unbound PVC metrics * wip: Fix 'Unknown Decorator' * fixup! Add labels to PVCollector bound/unbound PVC metrics --- .../persistentvolume/metrics/metrics.go | 44 ++++++++---- test/e2e/storage/volume_metrics.go | 71 ++++++++++++++----- 2 files changed, 85 insertions(+), 30 deletions(-) diff --git a/pkg/controller/volume/persistentvolume/metrics/metrics.go b/pkg/controller/volume/persistentvolume/metrics/metrics.go index 22f25792def..8d86414b5cb 100644 --- a/pkg/controller/volume/persistentvolume/metrics/metrics.go +++ b/pkg/controller/volume/persistentvolume/metrics/metrics.go @@ -23,8 +23,10 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/component-base/metrics" "k8s.io/component-base/metrics/legacyregistry" + storagehelpers "k8s.io/component-helpers/storage/volume" "k8s.io/kubernetes/pkg/volume" metricutil "k8s.io/kubernetes/pkg/volume/util" + "k8s.io/utils/ptr" ) const ( @@ -39,10 +41,11 @@ const ( unboundPVCKey = "unbound_pvc_count" // Label names. - namespaceLabel = "namespace" - storageClassLabel = "storage_class" - pluginNameLabel = "plugin_name" - volumeModeLabel = "volume_mode" + namespaceLabel = "namespace" + storageClassLabel = "storage_class" + volumeAttributesClassLabel = "volume_attributes_class" + pluginNameLabel = "plugin_name" + volumeModeLabel = "volume_mode" // String to use when plugin name cannot be determined pluginNameNotAvailable = "N/A" @@ -86,6 +89,19 @@ type pvAndPVCCountCollector struct { pluginMgr *volume.VolumePluginMgr } +// Holds all dimensions for bound/unbound PVC metrics +type pvcBindingMetricDimensions struct { + namespace, storageClassName, volumeAttributesClassName string +} + +func getPVCMetricDimensions(pvc *v1.PersistentVolumeClaim) pvcBindingMetricDimensions { + return pvcBindingMetricDimensions{ + namespace: pvc.Namespace, + storageClassName: storagehelpers.GetPersistentVolumeClaimClass(pvc), + volumeAttributesClassName: ptr.Deref(pvc.Spec.VolumeAttributesClassName, ""), + } +} + // Check if our collector implements necessary collector interface var _ metrics.StableCollector = &pvAndPVCCountCollector{} @@ -109,12 +125,12 @@ var ( boundPVCCountDesc = metrics.NewDesc( metrics.BuildFQName("", pvControllerSubsystem, boundPVCKey), "Gauge measuring number of persistent volume claim currently bound", - []string{namespaceLabel}, nil, + []string{namespaceLabel, storageClassLabel, volumeAttributesClassLabel}, nil, metrics.ALPHA, "") unboundPVCCountDesc = metrics.NewDesc( metrics.BuildFQName("", pvControllerSubsystem, unboundPVCKey), "Gauge measuring number of persistent volume claim currently unbound", - []string{namespaceLabel}, nil, + []string{namespaceLabel, storageClassLabel, volumeAttributesClassLabel}, nil, metrics.ALPHA, "") volumeOperationErrorsMetric = metrics.NewCounterVec( @@ -218,32 +234,32 @@ func (collector *pvAndPVCCountCollector) pvCollect(ch chan<- metrics.Metric) { } func (collector *pvAndPVCCountCollector) pvcCollect(ch chan<- metrics.Metric) { - boundNumberByNamespace := make(map[string]int) - unboundNumberByNamespace := make(map[string]int) + boundNumber := make(map[pvcBindingMetricDimensions]int) + unboundNumber := make(map[pvcBindingMetricDimensions]int) for _, pvcObj := range collector.pvcLister.List() { pvc, ok := pvcObj.(*v1.PersistentVolumeClaim) if !ok { continue } if pvc.Status.Phase == v1.ClaimBound { - boundNumberByNamespace[pvc.Namespace]++ + boundNumber[getPVCMetricDimensions(pvc)]++ } else { - unboundNumberByNamespace[pvc.Namespace]++ + unboundNumber[getPVCMetricDimensions(pvc)]++ } } - for namespace, number := range boundNumberByNamespace { + for pvcLabels, number := range boundNumber { ch <- metrics.NewLazyConstMetric( boundPVCCountDesc, metrics.GaugeValue, float64(number), - namespace) + pvcLabels.namespace, pvcLabels.storageClassName, pvcLabels.volumeAttributesClassName) } - for namespace, number := range unboundNumberByNamespace { + for pvcLabels, number := range unboundNumber { ch <- metrics.NewLazyConstMetric( unboundPVCCountDesc, metrics.GaugeValue, float64(number), - namespace) + pvcLabels.namespace, pvcLabels.storageClassName, pvcLabels.volumeAttributesClassName) } } diff --git a/test/e2e/storage/volume_metrics.go b/test/e2e/storage/volume_metrics.go index c65b13edc94..935c82e1c66 100644 --- a/test/e2e/storage/volume_metrics.go +++ b/test/e2e/storage/volume_metrics.go @@ -32,7 +32,9 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/component-base/metrics/testutil" "k8s.io/component-helpers/storage/ephemeral" + "k8s.io/kubernetes/pkg/features" kubeletmetrics "k8s.io/kubernetes/pkg/kubelet/metrics" + "k8s.io/kubernetes/test/e2e/feature" "k8s.io/kubernetes/test/e2e/framework" e2emetrics "k8s.io/kubernetes/test/e2e/framework/metrics" e2epod "k8s.io/kubernetes/test/e2e/framework/pod" @@ -499,10 +501,11 @@ var _ = utils.SIGDescribe(framework.WithSerial(), "Volume metrics", func() { // Test for pv controller metrics, concretely: bound/unbound pv/pvc count. ginkgo.Describe("PVController", func() { const ( - classKey = "storage_class" - namespaceKey = "namespace" - pluginNameKey = "plugin_name" - volumeModeKey = "volume_mode" + namespaceKey = "namespace" + pluginNameKey = "plugin_name" + volumeModeKey = "volume_mode" + storageClassKey = "storage_class" + volumeAttributeClassKey = "volume_attributes_class" totalPVKey = "pv_collector_total_pv_count" boundPVKey = "pv_collector_bound_pv_count" @@ -515,22 +518,24 @@ var _ = utils.SIGDescribe(framework.WithSerial(), "Volume metrics", func() { pv *v1.PersistentVolume pvc *v1.PersistentVolumeClaim - className = "bound-unbound-count-test-sc" - pvConfig = e2epv.PersistentVolumeConfig{ + storageClassName = "bound-unbound-count-test-sc" + pvConfig = e2epv.PersistentVolumeConfig{ PVSource: v1.PersistentVolumeSource{ HostPath: &v1.HostPathVolumeSource{Path: "/data"}, }, NamePrefix: "pv-test-", - StorageClassName: className, + StorageClassName: storageClassName, } - pvcConfig = e2epv.PersistentVolumeClaimConfig{StorageClassName: &className} + // TODO: Insert volumeAttributesClassName into pvcConfig when "VolumeAttributesClass" is GA + volumeAttributesClassName = "bound-unbound-count-test-vac" + pvcConfig = e2epv.PersistentVolumeClaimConfig{StorageClassName: &storageClassName} e2emetrics = []struct { name string dimension string }{ - {boundPVKey, classKey}, - {unboundPVKey, classKey}, + {boundPVKey, storageClassKey}, + {unboundPVKey, storageClassKey}, {boundPVCKey, namespaceKey}, {unboundPVCKey, namespaceKey}, } @@ -556,7 +561,7 @@ var _ = utils.SIGDescribe(framework.WithSerial(), "Volume metrics", func() { if expectValues == nil { expectValues = make(map[string]int64) } - // We using relative increment value instead of absolute value to reduce unexpected flakes. + // We use relative increment value instead of absolute value to reduce unexpected flakes. // Concretely, we expect the difference of the updated values and original values for each // test suit are equal to expectValues. actualValues := calculateRelativeValues(originMetricValues[i], @@ -604,8 +609,8 @@ var _ = utils.SIGDescribe(framework.WithSerial(), "Volume metrics", func() { var err error pv, err = e2epv.CreatePV(ctx, c, f.Timeouts, pv) framework.ExpectNoError(err, "Error creating pv: %v", err) - waitForPVControllerSync(ctx, metricsGrabber, unboundPVKey, classKey) - validator(ctx, []map[string]int64{nil, {className: 1}, nil, nil}) + waitForPVControllerSync(ctx, metricsGrabber, unboundPVKey, storageClassKey) + validator(ctx, []map[string]int64{nil, {storageClassName: 1}, nil, nil}) }) ginkgo.It("should create unbound pvc count metrics for pvc controller after creating pvc only", @@ -622,11 +627,45 @@ var _ = utils.SIGDescribe(framework.WithSerial(), "Volume metrics", func() { var err error pv, pvc, err = e2epv.CreatePVPVC(ctx, c, f.Timeouts, pvConfig, pvcConfig, ns, true) framework.ExpectNoError(err, "Error creating pv pvc: %v", err) - waitForPVControllerSync(ctx, metricsGrabber, boundPVKey, classKey) + waitForPVControllerSync(ctx, metricsGrabber, boundPVKey, storageClassKey) waitForPVControllerSync(ctx, metricsGrabber, boundPVCKey, namespaceKey) - validator(ctx, []map[string]int64{{className: 1}, nil, {ns: 1}, nil}) - + validator(ctx, []map[string]int64{{storageClassName: 1}, nil, {ns: 1}, nil}) }) + + // TODO: Merge with bound/unbound tests when "VolumeAttributesClass" feature is enabled by default + f.It("should create unbound pvc count metrics for pvc controller with volume attributes class dimension after creating pvc only", + feature.VolumeAttributesClass, framework.WithFeatureGate(features.VolumeAttributesClass), func(ctx context.Context) { + var err error + dimensions := []string{namespaceKey, storageClassKey, volumeAttributeClassKey} + pvcConfigWithVAC := pvcConfig + pvcConfigWithVAC.VolumeAttributesClassName = &volumeAttributesClassName + pvcWithVAC := e2epv.MakePersistentVolumeClaim(pvcConfigWithVAC, ns) + pvc, err = e2epv.CreatePVC(ctx, c, ns, pvcWithVAC) + framework.ExpectNoError(err, "Error creating pvc: %v", err) + waitForPVControllerSync(ctx, metricsGrabber, unboundPVCKey, volumeAttributeClassKey) + controllerMetrics, err := metricsGrabber.GrabFromControllerManager(ctx) + framework.ExpectNoError(err, "Error getting c-m metricValues: %v", err) + err = testutil.ValidateMetrics(testutil.Metrics(controllerMetrics), unboundPVCKey, dimensions...) + framework.ExpectNoError(err, "Invalid metric in Controller Manager metrics: %q", unboundPVCKey) + }) + + // TODO: Merge with bound/unbound tests when "VolumeAttributesClass" feature is enabled by default + f.It("should create bound pv/pvc count metrics for pvc controller with volume attributes class dimension after creating both pv and pvc", + feature.VolumeAttributesClass, framework.WithFeatureGate(features.VolumeAttributesClass), func(ctx context.Context) { + var err error + dimensions := []string{namespaceKey, storageClassKey, volumeAttributeClassKey} + pvcConfigWithVAC := pvcConfig + pvcConfigWithVAC.VolumeAttributesClassName = &volumeAttributesClassName + pv, pvc, err = e2epv.CreatePVPVC(ctx, c, f.Timeouts, pvConfig, pvcConfigWithVAC, ns, true) + framework.ExpectNoError(err, "Error creating pv pvc: %v", err) + waitForPVControllerSync(ctx, metricsGrabber, boundPVKey, storageClassKey) + waitForPVControllerSync(ctx, metricsGrabber, boundPVCKey, volumeAttributeClassKey) + controllerMetrics, err := metricsGrabber.GrabFromControllerManager(ctx) + framework.ExpectNoError(err, "Error getting c-m metricValues: %v", err) + err = testutil.ValidateMetrics(testutil.Metrics(controllerMetrics), boundPVCKey, dimensions...) + framework.ExpectNoError(err, "Invalid metric in Controller Manager metrics: %q", boundPVCKey) + }) + ginkgo.It("should create total pv count metrics for with plugin and volume mode labels after creating pv", func(ctx context.Context) { var err error