diff --git a/pkg/controller/volume/ephemeral/controller.go b/pkg/controller/volume/ephemeral/controller.go index 07c1c200771..0f474e9470f 100644 --- a/pkg/controller/volume/ephemeral/controller.go +++ b/pkg/controller/volume/ephemeral/controller.go @@ -25,7 +25,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -282,16 +281,10 @@ func (ec *ephemeralController) handleVolume(pod *v1.Pod, vol v1.Volume) error { }, Spec: ephemeral.VolumeClaimTemplate.Spec, } + ephemeralvolumemetrics.EphemeralVolumeCreateAttempts.Inc() _, err = ec.kubeClient.CoreV1().PersistentVolumeClaims(pod.Namespace).Create(context.TODO(), pvc, metav1.CreateOptions{}) - reason := "" - if err != nil { - reason = string(apierrors.ReasonForError(err)) - if reason == "" { - reason = "Unknown" - } - } - ephemeralvolumemetrics.EphemeralVolumeCreate.WithLabelValues(reason).Inc() if err != nil { + ephemeralvolumemetrics.EphemeralVolumeCreateFailures.Inc() return fmt.Errorf("create PVC %s: %v", pvcName, err) } return nil diff --git a/pkg/controller/volume/ephemeral/controller_test.go b/pkg/controller/volume/ephemeral/controller_test.go index 158997001c5..7b1c29ea5a7 100644 --- a/pkg/controller/volume/ephemeral/controller_test.go +++ b/pkg/controller/volume/ephemeral/controller_test.go @@ -22,7 +22,7 @@ import ( "sort" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" // storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" // "k8s.io/apimachinery/pkg/types" @@ -116,7 +116,7 @@ func TestSyncHandler(t *testing.T) { name: "create-conflict", pods: []*v1.Pod{testPodWithEphemeral}, podKey: podKey(testPodWithEphemeral), - expectedMetrics: expectedMetrics{0, 1}, + expectedMetrics: expectedMetrics{1, 1}, expectedError: true, }, } @@ -139,7 +139,7 @@ func TestSyncHandler(t *testing.T) { } fakeKubeClient := createTestClient(objects...) - if tc.expectedMetrics.numConflicts > 0 { + if tc.expectedMetrics.numFailures > 0 { fakeKubeClient.PrependReactor("create", "persistentvolumeclaims", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { return true, nil, apierrors.NewConflict(action.GetResource().GroupResource(), "fake name", errors.New("fake conflict")) }) @@ -245,22 +245,22 @@ func createTestClient(objects ...runtime.Object) *fake.Clientset { // Metrics helpers type expectedMetrics struct { - numCreated int - numConflicts int + numCreated int + numFailures int } func expectMetrics(t *testing.T, em expectedMetrics) { t.Helper() - actualCreated, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreate.WithLabelValues("")) + actualCreated, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreateAttempts) handleErr(t, err, "ephemeralVolumeCreate") if actualCreated != float64(em.numCreated) { t.Errorf("Expected PVCs to be created %d, got %v", em.numCreated, actualCreated) } - actualConflicts, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreate.WithLabelValues("Conflict")) + actualConflicts, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreateFailures) handleErr(t, err, "ephemeralVolumeCreate/Conflict") - if actualConflicts != float64(em.numConflicts) { - t.Errorf("Expected PVCs to have conflicts %d, got %v", em.numConflicts, actualConflicts) + if actualConflicts != float64(em.numFailures) { + t.Errorf("Expected PVCs to have conflicts %d, got %v", em.numFailures, actualConflicts) } } @@ -272,6 +272,6 @@ func handleErr(t *testing.T, err error, metricName string) { func setupMetrics() { ephemeralvolumemetrics.RegisterMetrics() - ephemeralvolumemetrics.EphemeralVolumeCreate.Delete(map[string]string{"reason": ""}) - ephemeralvolumemetrics.EphemeralVolumeCreate.Delete(map[string]string{"reason": "Conflict"}) + ephemeralvolumemetrics.EphemeralVolumeCreateAttempts.Reset() + ephemeralvolumemetrics.EphemeralVolumeCreateFailures.Reset() } diff --git a/pkg/controller/volume/ephemeral/metrics/metrics.go b/pkg/controller/volume/ephemeral/metrics/metrics.go index 813f56ba0a5..20eb9f1f6e4 100644 --- a/pkg/controller/volume/ephemeral/metrics/metrics.go +++ b/pkg/controller/volume/ephemeral/metrics/metrics.go @@ -27,20 +27,24 @@ import ( const EphemeralVolumeSubsystem = "ephemeral_volume_controller" var ( - // EphemeralVolumeCreate tracks the number of - // PersistentVolumeClaims().Create calls for each failure - // reason - // (https://pkg.go.dev/k8s.io/apimachinery@v0.20.2/pkg/apis/meta/v1#StatusReason), - // with empty for successful calls. - EphemeralVolumeCreate = metrics.NewCounterVec( + // EphemeralVolumeCreateAttempts tracks the number of + // PersistentVolumeClaims().Create calls (both successful and unsuccessful) + EphemeralVolumeCreateAttempts = metrics.NewCounter( &metrics.CounterOpts{ Subsystem: EphemeralVolumeSubsystem, - Name: "create", + Name: "create_total", Help: "Number of PersistenVolumeClaims creation requests", StabilityLevel: metrics.ALPHA, - }, - []string{"reason"}, - ) + }) + // EphemeralVolumeCreateFailures tracks the number of unsuccessful + // PersistentVolumeClaims().Create calls + EphemeralVolumeCreateFailures = metrics.NewCounter( + &metrics.CounterOpts{ + Subsystem: EphemeralVolumeSubsystem, + Name: "create_failures_total", + Help: "Number of PersistenVolumeClaims creation requests", + StabilityLevel: metrics.ALPHA, + }) ) var registerMetrics sync.Once @@ -48,6 +52,7 @@ var registerMetrics sync.Once // RegisterMetrics registers EphemeralVolume metrics. func RegisterMetrics() { registerMetrics.Do(func() { - legacyregistry.MustRegister(EphemeralVolumeCreate) + legacyregistry.MustRegister(EphemeralVolumeCreateAttempts) + legacyregistry.MustRegister(EphemeralVolumeCreateFailures) }) }