generic ephemeral volume: add metrics

As discussed during the production readiness review, a metric for the
PVC create operations is useful. The "ephemeral_volume" workqueue
metrics were already added in the initial implementation.

The new code follows the example set by the endpoints controller.

pkg/controller/volume/ephemeral/controller.go

@@ -25,6 +25,7 @@ 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"
@@ -38,6 +39,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/kubernetes/pkg/controller/volume/common"
+	ephemeralvolumemetrics "k8s.io/kubernetes/pkg/controller/volume/ephemeral/metrics"
 	"k8s.io/kubernetes/pkg/controller/volume/events"
 	"k8s.io/kubernetes/pkg/volume/util"
 )
@@ -90,6 +92,8 @@ func NewController(
 		queue:      workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ephemeral_volume"),
 	}
 
+	ephemeralvolumemetrics.RegisterMetrics()
+
 	eventBroadcaster := record.NewBroadcaster()
 	eventBroadcaster.StartLogging(klog.Infof)
 	eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")})
@@ -279,6 +283,14 @@ func (ec *ephemeralController) handleVolume(pod *v1.Pod, vol v1.Volume) error {
 		Spec: ephemeral.VolumeClaimTemplate.Spec,
 	}
 	_, 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 {
 		return fmt.Errorf("create PVC %s: %v", pvcName, err)
 	}

pkg/controller/volume/ephemeral/controller_test.go

@@ -18,6 +18,7 @@ package ephemeral
 
 import (
 	"context"
+	"errors"
 	"sort"
 	"testing"
 
@@ -25,14 +26,18 @@ import (
 	// storagev1 "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	// "k8s.io/apimachinery/pkg/types"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
+	k8stesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
 	kcache "k8s.io/client-go/tools/cache"
+	"k8s.io/component-base/metrics/testutil"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/controller"
+	ephemeralvolumemetrics "k8s.io/kubernetes/pkg/controller/volume/ephemeral/metrics"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -57,18 +62,20 @@ func init() {
 
 func TestSyncHandler(t *testing.T) {
 	tests := []struct {
-		name          string
+		name            string
-		podKey        string
+		podKey          string
-		pvcs          []*v1.PersistentVolumeClaim
+		pvcs            []*v1.PersistentVolumeClaim
-		pods          []*v1.Pod
+		pods            []*v1.Pod
-		expectedPVCs  []v1.PersistentVolumeClaim
+		expectedPVCs    []v1.PersistentVolumeClaim
-		expectedError bool
+		expectedError   bool
+		expectedMetrics expectedMetrics
 	}{
 		{
-			name:         "create",
+			name:            "create",
-			pods:         []*v1.Pod{testPodWithEphemeral},
+			pods:            []*v1.Pod{testPodWithEphemeral},
-			podKey:       podKey(testPodWithEphemeral),
+			podKey:          podKey(testPodWithEphemeral),
-			expectedPVCs: []v1.PersistentVolumeClaim{*testPodEphemeralClaim},
+			expectedPVCs:    []v1.PersistentVolumeClaim{*testPodEphemeralClaim},
+			expectedMetrics: expectedMetrics{1, 0},
 		},
 		{
 			name:   "no-such-pod",
@@ -90,11 +97,12 @@ func TestSyncHandler(t *testing.T) {
 			podKey: podKey(testPod),
 		},
 		{
-			name:         "create-with-other-PVC",
+			name:            "create-with-other-PVC",
-			pods:         []*v1.Pod{testPodWithEphemeral},
+			pods:            []*v1.Pod{testPodWithEphemeral},
-			podKey:       podKey(testPodWithEphemeral),
+			podKey:          podKey(testPodWithEphemeral),
-			pvcs:         []*v1.PersistentVolumeClaim{otherNamespaceClaim},
+			pvcs:            []*v1.PersistentVolumeClaim{otherNamespaceClaim},
-			expectedPVCs: []v1.PersistentVolumeClaim{*otherNamespaceClaim, *testPodEphemeralClaim},
+			expectedPVCs:    []v1.PersistentVolumeClaim{*otherNamespaceClaim, *testPodEphemeralClaim},
+			expectedMetrics: expectedMetrics{1, 0},
 		},
 		{
 			name:          "wrong-PVC-owner",
@@ -104,10 +112,17 @@ func TestSyncHandler(t *testing.T) {
 			expectedPVCs:  []v1.PersistentVolumeClaim{*conflictingClaim},
 			expectedError: true,
 		},
+		{
+			name:            "create-conflict",
+			pods:            []*v1.Pod{testPodWithEphemeral},
+			podKey:          podKey(testPodWithEphemeral),
+			expectedMetrics: expectedMetrics{0, 1},
+			expectedError:   true,
+		},
 	}
 
 	for _, tc := range tests {
-		// Run sequentially because of global logging.
+		// Run sequentially because of global logging and global metrics.
 		t.Run(tc.name, func(t *testing.T) {
 			// There is no good way to shut down the informers. They spawn
 			// various goroutines and some of them (in particular shared informer)
@@ -124,6 +139,12 @@ func TestSyncHandler(t *testing.T) {
 			}
 
 			fakeKubeClient := createTestClient(objects...)
+			if tc.expectedMetrics.numConflicts > 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"))
+				})
+			}
+			setupMetrics()
 			informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, controller.NoResyncPeriodFunc())
 			podInformer := informerFactory.Core().V1().Pods()
 			pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims()
@@ -152,6 +173,7 @@ func TestSyncHandler(t *testing.T) {
 				t.Fatalf("unexpected error while listing PVCs: %v", err)
 			}
 			assert.Equal(t, sortPVCs(tc.expectedPVCs), sortPVCs(pvcs.Items))
+			expectMetrics(t, tc.expectedMetrics)
 		})
 	}
 }
@@ -219,3 +241,37 @@ func createTestClient(objects ...runtime.Object) *fake.Clientset {
 	fakeClient := fake.NewSimpleClientset(objects...)
 	return fakeClient
 }
+
+// Metrics helpers
+
+type expectedMetrics struct {
+	numCreated   int
+	numConflicts int
+}
+
+func expectMetrics(t *testing.T, em expectedMetrics) {
+	t.Helper()
+
+	actualCreated, err := testutil.GetCounterMetricValue(ephemeralvolumemetrics.EphemeralVolumeCreate.WithLabelValues(""))
+	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"))
+	handleErr(t, err, "ephemeralVolumeCreate/Conflict")
+	if actualConflicts != float64(em.numConflicts) {
+		t.Errorf("Expected PVCs to have conflicts %d, got %v", em.numConflicts, actualConflicts)
+	}
+}
+
+func handleErr(t *testing.T, err error, metricName string) {
+	if err != nil {
+		t.Errorf("Failed to get %s value, err: %v", metricName, err)
+	}
+}
+
+func setupMetrics() {
+	ephemeralvolumemetrics.RegisterMetrics()
+	ephemeralvolumemetrics.EphemeralVolumeCreate.Delete(map[string]string{"reason": ""})
+	ephemeralvolumemetrics.EphemeralVolumeCreate.Delete(map[string]string{"reason": "Conflict"})
+}

pkg/controller/volume/ephemeral/metrics/metrics.go

@@ -0,0 +1,53 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package metrics
+
+import (
+	"sync"
+
+	"k8s.io/component-base/metrics"
+	"k8s.io/component-base/metrics/legacyregistry"
+)
+
+// EphemeralVolumeSubsystem - subsystem name used for Endpoint Slices.
+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(
+		&metrics.CounterOpts{
+			Subsystem:      EphemeralVolumeSubsystem,
+			Name:           "create",
+			Help:           "Number of PersistenVolumeClaims creation requests",
+			StabilityLevel: metrics.ALPHA,
+		},
+		[]string{"reason"},
+	)
+)
+
+var registerMetrics sync.Once
+
+// RegisterMetrics registers EphemeralVolume metrics.
+func RegisterMetrics() {
+	registerMetrics.Do(func() {
+		legacyregistry.MustRegister(EphemeralVolumeCreate)
+	})
+}