Merge pull request #132800 from ritazh/dra-adminaccess-metrics

DRAAdminAccess: update resourceclaim_controller metrics

pkg/controller/resourceclaim/controller.go

@@ -28,6 +28,7 @@ import (
 	resourceapi "k8s.io/api/resource/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -42,10 +43,11 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
+	"k8s.io/component-base/metrics"
 	"k8s.io/dynamic-resource-allocation/resourceclaim"
 	"k8s.io/klog/v2"
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
-	"k8s.io/kubernetes/pkg/controller/resourceclaim/metrics"
+	resourceclaimmetrics "k8s.io/kubernetes/pkg/controller/resourceclaim/metrics"
 	"k8s.io/utils/ptr"
 )
 
@@ -150,7 +152,7 @@ func NewController(
 		deletedObjects: newUIDCache(maxUIDCacheEntries),
 	}
 
-	metrics.RegisterMetrics()
+	resourceclaimmetrics.RegisterMetrics(newCustomCollector(ec.claimLister, getAdminAccessMetricLabel, logger))
 
 	if _, err := podInformer.Informer().AddEventHandlerWithOptions(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
@@ -351,28 +353,9 @@ func (ec *Controller) enqueueResourceClaim(logger klog.Logger, oldObj, newObj in
 		return
 	}
 
-	// Maintain metrics based on what was observed.
-	switch {
-	case oldClaim == nil:
-		// Added.
-		metrics.NumResourceClaims.Inc()
-		if newClaim.Status.Allocation != nil {
-			metrics.NumAllocatedResourceClaims.Inc()
-		}
-	case newClaim == nil:
-		// Deleted.
-		metrics.NumResourceClaims.Dec()
-		if oldClaim.Status.Allocation != nil {
-			metrics.NumAllocatedResourceClaims.Dec()
-		}
-	default:
-		// Updated.
-		switch {
-		case oldClaim.Status.Allocation == nil && newClaim.Status.Allocation != nil:
-			metrics.NumAllocatedResourceClaims.Inc()
-		case oldClaim.Status.Allocation != nil && newClaim.Status.Allocation == nil:
-			metrics.NumAllocatedResourceClaims.Dec()
-		}
+	// Check if both the old and new claim are nil in case DeletedFinalStateUnknown.Obj can be nil.
+	if oldClaim == nil && newClaim == nil {
+		return
 	}
 
 	claim := newClaim
@@ -669,13 +652,14 @@ func (ec *Controller) handleClaim(ctx context.Context, pod *v1.Pod, podClaim v1.
 			},
 			Spec: template.Spec.Spec,
 		}
-		metrics.ResourceClaimCreateAttempts.Inc()
+		metricLabel := getAdminAccessMetricLabel(claim)
 		claimName := claim.Name
 		claim, err = ec.kubeClient.ResourceV1beta1().ResourceClaims(pod.Namespace).Create(ctx, claim, metav1.CreateOptions{})
 		if err != nil {
-			metrics.ResourceClaimCreateFailures.Inc()
+			resourceclaimmetrics.ResourceClaimCreate.WithLabelValues("failure", metricLabel).Inc()
 			return fmt.Errorf("create ResourceClaim %s: %v", claimName, err)
 		}
+		resourceclaimmetrics.ResourceClaimCreate.WithLabelValues("success", metricLabel).Inc()
 		logger.V(4).Info("Created ResourceClaim", "claim", klog.KObj(claim), "pod", klog.KObj(pod))
 		ec.claimCache.Mutation(claim)
 	}
@@ -991,3 +975,61 @@ func claimPodOwnerIndexFunc(obj interface{}) ([]string, error) {
 	}
 	return keys, nil
 }
+func getAdminAccessMetricLabel(claim *resourceapi.ResourceClaim) string {
+	if claim == nil {
+		return "false"
+	}
+	for _, request := range claim.Spec.Devices.Requests {
+		if ptr.Deref(request.AdminAccess, false) {
+			return "true"
+		}
+	}
+	return "false"
+}
+
+func newCustomCollector(rcLister resourcelisters.ResourceClaimLister, adminAccessFunc func(*resourceapi.ResourceClaim) string, logger klog.Logger) metrics.StableCollector {
+	return &customCollector{
+		rcLister:        rcLister,
+		adminAccessFunc: adminAccessFunc,
+		logger:          logger,
+	}
+}
+
+type customCollector struct {
+	metrics.BaseStableCollector
+	rcLister        resourcelisters.ResourceClaimLister
+	adminAccessFunc func(*resourceapi.ResourceClaim) string
+	logger          klog.Logger
+}
+
+var _ metrics.StableCollector = &customCollector{}
+
+func (collector *customCollector) DescribeWithStability(ch chan<- *metrics.Desc) {
+	ch <- resourceclaimmetrics.NumResourceClaimsDesc
+}
+
+func (collector *customCollector) CollectWithStability(ch chan<- metrics.Metric) {
+	allocateMetrics := make(map[string]map[string]int)
+	rcList, err := collector.rcLister.List(labels.Everything())
+	if err != nil {
+		collector.logger.Error(err, "failed to list resource claims for metrics collection")
+		return
+	}
+	for _, rc := range rcList {
+		// Determine if the ResourceClaim is allocated
+		allocated := "false"
+		if rc.Status.Allocation != nil {
+			allocated = "true"
+		}
+		adminAccess := collector.adminAccessFunc(rc)
+		if allocateMetrics[allocated] == nil {
+			allocateMetrics[allocated] = make(map[string]int)
+		}
+		allocateMetrics[allocated][adminAccess]++
+	}
+	for allocated, adminAccessMap := range allocateMetrics {
+		for adminAccess, count := range adminAccessMap {
+			ch <- metrics.NewLazyConstMetric(resourceclaimmetrics.NumResourceClaimsDesc, metrics.GaugeValue, float64(count), allocated, adminAccess)
+		}
+	}
+}

pkg/controller/resourceclaim/controller_test.go

@@ -36,10 +36,13 @@ import (
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
+	resourcelisters "k8s.io/client-go/listers/resource/v1beta1"
 	k8stesting "k8s.io/client-go/testing"
+	"k8s.io/component-base/metrics"
 	"k8s.io/component-base/metrics/testutil"
+	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/controller"
-	"k8s.io/kubernetes/pkg/controller/resourceclaim/metrics"
+	resourceclaimmetrics "k8s.io/kubernetes/pkg/controller/resourceclaim/metrics"
 	"k8s.io/kubernetes/test/utils/ktesting"
 	"k8s.io/utils/ptr"
 )
@@ -56,7 +59,8 @@ var (
 
 	testPod             = makePod(testPodName, testNamespace, testPodUID)
 	testPodWithResource = makePod(testPodName, testNamespace, testPodUID, *makePodResourceClaim(podResourceClaimName, templateName))
-	otherTestPod        = makePod(testPodName+"-II", testNamespace, testPodUID+"-II")
+
+	otherTestPod = makePod(testPodName+"-II", testNamespace, testPodUID+"-II")
 
 	testClaim              = makeClaim(testPodName+"-"+podResourceClaimName, testNamespace, className, makeOwnerReference(testPodWithResource, true))
 	testClaimAllocated     = allocateClaim(testClaim)
@@ -65,10 +69,12 @@ var (
 	testClaimKey           = claimKeyPrefix + testClaim.Namespace + "/" + testClaim.Name
 
 	generatedTestClaim          = makeGeneratedClaim(podResourceClaimName, testPodName+"-"+podResourceClaimName+"-", testNamespace, className, 1, makeOwnerReference(testPodWithResource, true), nil)
-	generatedTestClaimWithAdmin = makeGeneratedClaim(podResourceClaimName, testPodName+"-"+podResourceClaimName+"-", testNamespace, className, 1, makeOwnerReference(testPodWithResource, true), ptr.To(true))
 	generatedTestClaimAllocated = allocateClaim(generatedTestClaim)
 	generatedTestClaimReserved  = reserveClaim(generatedTestClaimAllocated, testPodWithResource)
 
+	generatedTestClaimWithAdmin          = makeGeneratedClaim(podResourceClaimName, testPodName+"-"+podResourceClaimName+"-", testNamespace, className, 1, makeOwnerReference(testPodWithResource, true), ptr.To(true))
+	generatedTestClaimWithAdminAllocated = allocateClaim(generatedTestClaimWithAdmin)
+
 	conflictingClaim        = makeClaim(testPodName+"-"+podResourceClaimName, testNamespace, className, nil)
 	otherNamespaceClaim     = makeClaim(testPodName+"-"+podResourceClaimName, otherNamespace, className, nil)
 	template                = makeTemplate(templateName, testNamespace, className, nil)
@@ -113,7 +119,7 @@ func TestSyncHandler(t *testing.T) {
 					{Name: testPodWithResource.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaim.Name},
 				},
 			},
-			expectedMetrics: expectedMetrics{1, 0},
+			expectedMetrics: expectedMetrics{1, 0, 0, 0},
 		},
 		{
 			name:          "create with admin and feature gate off",
@@ -134,7 +140,7 @@ func TestSyncHandler(t *testing.T) {
 				},
 			},
 			adminAccessEnabled: true,
-			expectedMetrics:    expectedMetrics{1, 0},
+			expectedMetrics:    expectedMetrics{0, 1, 0, 0},
 		},
 		{
 			name: "nop",
@@ -154,7 +160,7 @@ func TestSyncHandler(t *testing.T) {
 					{Name: testPodWithResource.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaim.Name},
 				},
 			},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name: "recreate",
@@ -173,7 +179,7 @@ func TestSyncHandler(t *testing.T) {
 					{Name: testPodWithResource.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaim.Name},
 				},
 			},
-			expectedMetrics: expectedMetrics{1, 0},
+			expectedMetrics: expectedMetrics{1, 0, 0, 0},
 		},
 		{
 			name:          "missing-template",
@@ -193,7 +199,7 @@ func TestSyncHandler(t *testing.T) {
 					{Name: testPodWithResource.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaim.Name},
 				},
 			},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name:          "find-created-claim-in-cache",
@@ -205,7 +211,7 @@ func TestSyncHandler(t *testing.T) {
 					{Name: testPodWithResource.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaim.Name},
 				},
 			},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name: "no-such-pod",
@@ -238,7 +244,7 @@ func TestSyncHandler(t *testing.T) {
 					{Name: testPodWithResource.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaim.Name},
 				},
 			},
-			expectedMetrics: expectedMetrics{1, 0},
+			expectedMetrics: expectedMetrics{1, 0, 0, 0},
 		},
 		{
 			name:           "wrong-claim-owner",
@@ -253,7 +259,7 @@ func TestSyncHandler(t *testing.T) {
 			pods:            []*v1.Pod{testPodWithResource},
 			templates:       []*resourceapi.ResourceClaimTemplate{template},
 			key:             podKey(testPodWithResource),
-			expectedMetrics: expectedMetrics{1, 1},
+			expectedMetrics: expectedMetrics{1, 0, 1, 0},
 			expectedError:   "create ResourceClaim : Operation cannot be fulfilled on resourceclaims.resource.k8s.io \"fake name\": fake conflict",
 		},
 		{
@@ -262,7 +268,7 @@ func TestSyncHandler(t *testing.T) {
 			key:             claimKey(testClaimReserved),
 			claims:          []*resourceapi.ResourceClaim{testClaimReserved},
 			expectedClaims:  []resourceapi.ResourceClaim{*testClaimReserved},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name:            "stay-reserved-not-seen",
@@ -270,7 +276,7 @@ func TestSyncHandler(t *testing.T) {
 			key:             claimKey(testClaimReserved),
 			claims:          []*resourceapi.ResourceClaim{testClaimReserved},
 			expectedClaims:  []resourceapi.ResourceClaim{*testClaimReserved},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name:   "clear-reserved-structured",
@@ -283,7 +289,7 @@ func TestSyncHandler(t *testing.T) {
 				claim.Status.Allocation = nil
 				return []resourceapi.ResourceClaim{*claim}
 			}(),
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name: "dont-clear-reserved-structured",
@@ -295,7 +301,7 @@ func TestSyncHandler(t *testing.T) {
 				return []*resourceapi.ResourceClaim{claim}
 			}(),
 			expectedClaims:  []resourceapi.ResourceClaim{*structuredParameters(testClaimReserved)},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name: "clear-reserved-structured-deleted",
@@ -313,7 +319,7 @@ func TestSyncHandler(t *testing.T) {
 				claim.Status.Allocation = nil
 				return []resourceapi.ResourceClaim{*claim}
 			}(),
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name: "structured-deleted",
@@ -331,7 +337,7 @@ func TestSyncHandler(t *testing.T) {
 				claim.Status.Allocation = nil
 				return []resourceapi.ResourceClaim{*claim}
 			}(),
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name: "clear-reserved-when-done",
@@ -351,7 +357,7 @@ func TestSyncHandler(t *testing.T) {
 				claims[0].OwnerReferences = nil
 				return claims
 			}(),
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name:            "remove-reserved",
@@ -359,7 +365,7 @@ func TestSyncHandler(t *testing.T) {
 			key:             claimKey(testClaimReservedTwice),
 			claims:          []*resourceapi.ResourceClaim{testClaimReservedTwice},
 			expectedClaims:  []resourceapi.ResourceClaim{*testClaimReserved},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name: "delete-claim-when-done",
@@ -371,7 +377,7 @@ func TestSyncHandler(t *testing.T) {
 			key:             claimKey(testClaimReserved),
 			claims:          []*resourceapi.ResourceClaim{testClaimReserved},
 			expectedClaims:  nil,
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 		{
 			name:           "add-reserved",
@@ -385,7 +391,7 @@ func TestSyncHandler(t *testing.T) {
 					{Name: testPodWithNodeName.Spec.ResourceClaims[0].Name, ResourceClaimName: &generatedTestClaim.Name},
 				},
 			},
-			expectedMetrics: expectedMetrics{0, 0},
+			expectedMetrics: expectedMetrics{0, 0, 0, 0},
 		},
 	}
 
@@ -412,11 +418,11 @@ func TestSyncHandler(t *testing.T) {
 					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()
 			claimInformer := informerFactory.Resource().V1beta1().ResourceClaims()
 			templateInformer := informerFactory.Resource().V1beta1().ResourceClaimTemplates()
+			setupMetrics()
 
 			features := Features{
 				AdminAccess:     tc.adminAccessEnabled,
@@ -491,11 +497,11 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	tCtx = ktesting.WithCancel(tCtx)
 
 	fakeKubeClient := createTestClient()
-	setupMetrics()
 	informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, controller.NoResyncPeriodFunc())
 	podInformer := informerFactory.Core().V1().Pods()
 	claimInformer := informerFactory.Resource().V1beta1().ResourceClaims()
 	templateInformer := informerFactory.Resource().V1beta1().ResourceClaimTemplates()
+	setupMetrics()
 	claimClient := fakeKubeClient.ResourceV1beta1().ResourceClaims(testNamespace)
 
 	ec, err := NewController(tCtx.Logger(), Features{}, fakeKubeClient, podInformer, claimInformer, templateInformer)
@@ -508,7 +514,7 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	}
 	defer stopInformers()
 
-	var em numMetrics
+	em := newNumMetrics(claimInformer.Lister(), 0, 0, 0, 0)
 
 	expectQueue := func(tCtx ktesting.TContext, expectedKeys []string) {
 		g := gomega.NewWithT(tCtx)
@@ -546,7 +552,7 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	expectQueue(tCtx, []string{})
 
 	_, err = claimClient.Create(tCtx, testClaim, metav1.CreateOptions{})
-	em.claims++
+	em = em.withUpdates(1, 0, 0, 0)
 	ktesting.Step(tCtx, "create claim", func(tCtx ktesting.TContext) {
 		tCtx.ExpectNoError(err)
 		em.Eventually(tCtx)
@@ -563,7 +569,7 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	})
 
 	_, err = claimClient.Update(tCtx, testClaimAllocated, metav1.UpdateOptions{})
-	em.allocated++
+	em = em.withUpdates(-1, 0, 1, 0)
 	ktesting.Step(tCtx, "allocate claim", func(tCtx ktesting.TContext) {
 		tCtx.ExpectNoError(err)
 		em.Eventually(tCtx)
@@ -582,8 +588,7 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	otherClaimAllocated := testClaimAllocated.DeepCopy()
 	otherClaimAllocated.Name += "2"
 	_, err = claimClient.Create(tCtx, otherClaimAllocated, metav1.CreateOptions{})
-	em.claims++
-	em.allocated++
+	em = em.withUpdates(0, 0, 1, 0)
 	ktesting.Step(tCtx, "create allocated claim", func(tCtx ktesting.TContext) {
 		tCtx.ExpectNoError(err)
 		em.Eventually(tCtx)
@@ -591,7 +596,7 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	})
 
 	_, err = claimClient.Update(tCtx, testClaim, metav1.UpdateOptions{})
-	em.allocated--
+	em = em.withUpdates(1, 0, -1, 0)
 	ktesting.Step(tCtx, "deallocate claim", func(tCtx ktesting.TContext) {
 		tCtx.ExpectNoError(err)
 		em.Eventually(tCtx)
@@ -599,7 +604,7 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	})
 
 	err = claimClient.Delete(tCtx, testClaim.Name, metav1.DeleteOptions{})
-	em.claims--
+	em = em.withUpdates(-1, 0, 0, 0)
 	ktesting.Step(tCtx, "delete deallocated claim", func(tCtx ktesting.TContext) {
 		tCtx.ExpectNoError(err)
 		em.Eventually(tCtx)
@@ -607,17 +612,176 @@ func TestResourceClaimEventHandler(t *testing.T) {
 	})
 
 	err = claimClient.Delete(tCtx, otherClaimAllocated.Name, metav1.DeleteOptions{})
-	em.claims--
-	em.allocated--
+	em = em.withUpdates(0, 0, -1, 0)
 	ktesting.Step(tCtx, "delete allocated claim", func(tCtx ktesting.TContext) {
 		tCtx.ExpectNoError(err)
 		em.Eventually(tCtx)
 		expectQueue(tCtx, []string{})
 	})
 
+	_, err = claimClient.Create(tCtx, generatedTestClaimWithAdmin, metav1.CreateOptions{})
+	em = em.withUpdates(0, 1, 0, 0)
+	ktesting.Step(tCtx, "create claim with admin access", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Eventually(tCtx)
+	})
+
+	modifiedClaim = generatedTestClaimWithAdmin.DeepCopy()
+	modifiedClaim.Labels = map[string]string{"foo": "bar"}
+	_, err = claimClient.Update(tCtx, modifiedClaim, metav1.UpdateOptions{})
+	ktesting.Step(tCtx, "modify claim", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Consistently(tCtx)
+	})
+
+	_, err = claimClient.Update(tCtx, generatedTestClaimWithAdminAllocated, metav1.UpdateOptions{})
+	em = em.withUpdates(0, -1, 0, 1)
+	ktesting.Step(tCtx, "allocate claim with admin access", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Eventually(tCtx)
+	})
+
+	modifiedClaim = generatedTestClaimWithAdminAllocated.DeepCopy()
+	modifiedClaim.Labels = map[string]string{"foo": "bar2"}
+	_, err = claimClient.Update(tCtx, modifiedClaim, metav1.UpdateOptions{})
+	ktesting.Step(tCtx, "modify claim", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Consistently(tCtx)
+	})
+
+	otherClaimAllocated = generatedTestClaimWithAdminAllocated.DeepCopy()
+	otherClaimAllocated.Name += "2"
+	_, err = claimClient.Create(tCtx, otherClaimAllocated, metav1.CreateOptions{})
+	em = em.withUpdates(0, 0, 0, 1)
+	ktesting.Step(tCtx, "create allocated claim with admin access", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Eventually(tCtx)
+	})
+
+	_, err = claimClient.Update(tCtx, generatedTestClaimWithAdmin, metav1.UpdateOptions{})
+	em = em.withUpdates(0, 1, 0, -1)
+	ktesting.Step(tCtx, "deallocate claim with admin access", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Eventually(tCtx)
+	})
+
+	err = claimClient.Delete(tCtx, generatedTestClaimWithAdmin.Name, metav1.DeleteOptions{})
+	em = em.withUpdates(0, -1, 0, 0)
+	ktesting.Step(tCtx, "delete deallocated claim with admin access", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Eventually(tCtx)
+	})
+
+	err = claimClient.Delete(tCtx, otherClaimAllocated.Name, metav1.DeleteOptions{})
+	em = em.withUpdates(0, 0, 0, -1)
+	ktesting.Step(tCtx, "delete allocated claim with admin access", func(tCtx ktesting.TContext) {
+		tCtx.ExpectNoError(err)
+		em.Eventually(tCtx)
+	})
+
 	em.Consistently(tCtx)
 }
 
+func TestGetAdminAccessMetricLabel(t *testing.T) {
+	tests := []struct {
+		name  string
+		claim *resourceapi.ResourceClaim
+		want  string
+	}{
+		{
+			name:  "nil claim",
+			claim: nil,
+			want:  "false",
+		},
+		{
+			name: "no requests",
+			claim: &resourceapi.ResourceClaim{
+				Spec: resourceapi.ResourceClaimSpec{
+					Devices: resourceapi.DeviceClaim{
+						Requests: nil,
+					},
+				},
+			},
+			want: "false",
+		},
+		{
+			name: "admin access false",
+			claim: &resourceapi.ResourceClaim{
+				Spec: resourceapi.ResourceClaimSpec{
+					Devices: resourceapi.DeviceClaim{
+						Requests: []resourceapi.DeviceRequest{
+							{
+								AdminAccess: ptr.To(false),
+							},
+						},
+					},
+				},
+			},
+			want: "false",
+		},
+		{
+			name: "admin access true",
+			claim: &resourceapi.ResourceClaim{
+				Spec: resourceapi.ResourceClaimSpec{
+					Devices: resourceapi.DeviceClaim{
+						Requests: []resourceapi.DeviceRequest{
+							{
+								AdminAccess: ptr.To(true),
+							},
+						},
+					},
+				},
+			},
+			want: "true",
+		},
+		{
+			name: "multiple requests, one with admin access true",
+			claim: &resourceapi.ResourceClaim{
+				Spec: resourceapi.ResourceClaimSpec{
+					Devices: resourceapi.DeviceClaim{
+						Requests: []resourceapi.DeviceRequest{
+							{
+								AdminAccess: ptr.To(false),
+							},
+							{
+								AdminAccess: ptr.To(true),
+							},
+						},
+					},
+				},
+			},
+			want: "true",
+		},
+		{
+			name: "multiple requests, all admin access false or nil",
+			claim: &resourceapi.ResourceClaim{
+				Spec: resourceapi.ResourceClaimSpec{
+					Devices: resourceapi.DeviceClaim{
+						Requests: []resourceapi.DeviceRequest{
+							{
+								AdminAccess: nil,
+							},
+							{
+								AdminAccess: ptr.To(false),
+							},
+						},
+					},
+				},
+			},
+			want: "false",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := getAdminAccessMetricLabel(tt.claim)
+			if got != tt.want {
+				t.Errorf("GetAdminAccessMetricLabel() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func makeClaim(name, namespace, classname string, owner *metav1.OwnerReference) *resourceapi.ResourceClaim {
 	claim := &resourceapi.ResourceClaim{
 		ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
@@ -785,66 +949,150 @@ func createResourceClaimReactor() func(action k8stesting.Action) (handled bool,
 	}
 }
 
-// Metrics helpers
-
 type numMetrics struct {
-	claims    float64
-	allocated float64
+	notAllocated                float64
+	notAllocatedWithAdminAccess float64
+	allocated                   float64
+	allocatedWithAdminAccess    float64
+	lister                      resourcelisters.ResourceClaimLister
 }
 
-func getNumMetric() (em numMetrics, err error) {
-	em.claims, err = testutil.GetGaugeMetricValue(metrics.NumResourceClaims)
-	if err != nil {
-		return
+func getNumMetric(lister resourcelisters.ResourceClaimLister, logger klog.Logger) (em numMetrics, err error) {
+	if lister == nil {
+		return numMetrics{}, nil
 	}
-	em.allocated, err = testutil.GetGaugeMetricValue(metrics.NumAllocatedResourceClaims)
-	return
+
+	// Create a fresh collector instance for each call to avoid registration conflicts
+	freshCollector := newCustomCollector(lister, getAdminAccessMetricLabel, logger)
+	testRegistry := metrics.NewKubeRegistry()
+	testRegistry.CustomMustRegister(freshCollector)
+
+	gatheredMetrics, err := testRegistry.Gather()
+	if err != nil {
+		return numMetrics{}, fmt.Errorf("failed to gather metrics: %w", err)
+	}
+
+	metricName := "resourceclaim_controller_resource_claims"
+
+	for _, mf := range gatheredMetrics {
+		if mf.GetName() != metricName {
+			continue
+		}
+		for _, metric := range mf.GetMetric() {
+			labels := make(map[string]string)
+			for _, labelPair := range metric.GetLabel() {
+				labels[labelPair.GetName()] = labelPair.GetValue()
+			}
+
+			allocated := labels["allocated"]
+			adminAccess := labels["admin_access"]
+			value := metric.GetGauge().GetValue()
+
+			switch {
+			case allocated == "false" && adminAccess == "false":
+				em.notAllocated = value
+			case allocated == "false" && adminAccess == "true":
+				em.notAllocatedWithAdminAccess = value
+			case allocated == "true" && adminAccess == "false":
+				em.allocated = value
+			case allocated == "true" && adminAccess == "true":
+				em.allocatedWithAdminAccess = value
+			}
+		}
+	}
+
+	return em, nil
 }
 
 func (em numMetrics) Eventually(tCtx ktesting.TContext) {
 	g := gomega.NewWithT(tCtx)
 	tCtx.Helper()
 
-	g.Eventually(getNumMetric).WithTimeout(5 * time.Second).Should(gomega.Equal(em))
+	g.Eventually(func() (numMetrics, error) {
+		result, err := getNumMetric(em.lister, tCtx.Logger())
+		result.lister = em.lister
+		return result, err
+	}).WithTimeout(5 * time.Second).Should(gomega.Equal(em))
 }
 
 func (em numMetrics) Consistently(tCtx ktesting.TContext) {
 	g := gomega.NewWithT(tCtx)
 	tCtx.Helper()
 
-	g.Consistently(getNumMetric).WithTimeout(time.Second).Should(gomega.Equal(em))
+	g.Consistently(func() (numMetrics, error) {
+		result, err := getNumMetric(em.lister, tCtx.Logger())
+		result.lister = em.lister
+		return result, err
+	}).WithTimeout(time.Second).Should(gomega.Equal(em))
 }
 
 type expectedMetrics struct {
-	numCreated  int
-	numFailures int
+	numCreated          int
+	numCreatedWithAdmin int
+	numFailures         int
+	numFailureWithAdmin int
 }
 
 func expectMetrics(t *testing.T, em expectedMetrics) {
 	t.Helper()
 
-	actualCreated, err := testutil.GetCounterMetricValue(metrics.ResourceClaimCreateAttempts)
-	handleErr(t, err, "ResourceClaimCreate")
+	// Check created claims
+	actualCreated, err := testutil.GetCounterMetricValue(resourceclaimmetrics.ResourceClaimCreate.WithLabelValues("success", "false"))
+	handleErr(t, err, "ResourceClaimCreateSuccesses")
 	if actualCreated != float64(em.numCreated) {
 		t.Errorf("Expected claims to be created %d, got %v", em.numCreated, actualCreated)
 	}
-	actualConflicts, err := testutil.GetCounterMetricValue(metrics.ResourceClaimCreateFailures)
-	handleErr(t, err, "ResourceClaimCreate/Conflict")
-	if actualConflicts != float64(em.numFailures) {
-		t.Errorf("Expected claims to have conflicts %d, got %v", em.numFailures, actualConflicts)
+
+	// Check created claims with admin access
+	actualCreatedWithAdmin, err := testutil.GetCounterMetricValue(resourceclaimmetrics.ResourceClaimCreate.WithLabelValues("success", "true"))
+	handleErr(t, err, "ResourceClaimCreateSuccessesWithAdminAccess")
+	if actualCreatedWithAdmin != float64(em.numCreatedWithAdmin) {
+		t.Errorf("Expected claims with admin access to be created %d, got %v", em.numCreatedWithAdmin, actualCreatedWithAdmin)
+	}
+
+	// Check failed claims
+	actualFailed, err := testutil.GetCounterMetricValue(resourceclaimmetrics.ResourceClaimCreate.WithLabelValues("failure", "false"))
+	handleErr(t, err, "ResourceClaimCreateFailures")
+	if actualFailed != float64(em.numFailures) {
+		t.Errorf("Expected claims to have failed %d, got %v", em.numFailures, actualFailed)
+	}
+
+	// Check failed claims with admin access
+	actualFailedWithAdmin, err := testutil.GetCounterMetricValue(resourceclaimmetrics.ResourceClaimCreate.WithLabelValues("failure", "true"))
+	handleErr(t, err, "ResourceClaimCreateFailuresWithAdminAccess")
+	if actualFailedWithAdmin != float64(em.numFailureWithAdmin) {
+		t.Errorf("Expected claims with admin access to have failed %d, got %v", em.numFailureWithAdmin, actualFailedWithAdmin)
 	}
 }
-
 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() {
-	metrics.RegisterMetrics()
-	metrics.ResourceClaimCreateAttempts.Reset()
-	metrics.ResourceClaimCreateFailures.Reset()
-	metrics.NumResourceClaims.Set(0)
-	metrics.NumAllocatedResourceClaims.Set(0)
+	// Enable test mode to prevent global custom collector registration
+	resourceclaimmetrics.SetTestMode(true)
+
+	// Reset counter metrics for each test (they are registered by the controller itself)
+	resourceclaimmetrics.ResourceClaimCreate.Reset()
+}
+
+func newNumMetrics(lister resourcelisters.ResourceClaimLister, notAllocated, notAllocatedWithAdmin, allocated, allocatedWithAdmin float64) numMetrics {
+	return numMetrics{
+		notAllocated:                notAllocated,
+		notAllocatedWithAdminAccess: notAllocatedWithAdmin,
+		allocated:                   allocated,
+		allocatedWithAdminAccess:    allocatedWithAdmin,
+		lister:                      lister,
+	}
+}
+
+func (em numMetrics) withUpdates(notAllocatedDelta, notAllocatedWithAdminDelta, allocatedDelta, allocatedWithAdminDelta float64) numMetrics {
+	return numMetrics{
+		notAllocated:                em.notAllocated + notAllocatedDelta,
+		notAllocatedWithAdminAccess: em.notAllocatedWithAdminAccess + notAllocatedWithAdminDelta,
+		allocated:                   em.allocated + allocatedDelta,
+		allocatedWithAdminAccess:    em.allocatedWithAdminAccess + allocatedWithAdminDelta,
+		lister:                      em.lister,
+	}
 }

pkg/controller/resourceclaim/metrics/metrics.go

@@ -27,50 +27,45 @@ import (
 const ResourceClaimSubsystem = "resourceclaim_controller"
 
 var (
-	// ResourceClaimCreateAttempts tracks the number of
-	// ResourceClaims().Create calls (both successful and unsuccessful)
-	ResourceClaimCreateAttempts = metrics.NewCounter(
+	// ResourceClaimCreate tracks the total number of
+	// ResourceClaims creation requests
+	// categorized by their creation status and admin access.
+	ResourceClaimCreate = metrics.NewCounterVec(
 		&metrics.CounterOpts{
 			Subsystem:      ResourceClaimSubsystem,
-			Name:           "create_attempts_total",
-			Help:           "Number of ResourceClaims creation requests",
+			Name:           "creates_total",
+			Help:           "Number of ResourceClaims creation requests, categorized by creation status and admin access",
 			StabilityLevel: metrics.ALPHA,
-		})
-	// ResourceClaimCreateFailures tracks the number of unsuccessful
-	// ResourceClaims().Create calls
-	ResourceClaimCreateFailures = metrics.NewCounter(
-		&metrics.CounterOpts{
-			Subsystem:      ResourceClaimSubsystem,
-			Name:           "create_failures_total",
-			Help:           "Number of ResourceClaims creation request failures",
-			StabilityLevel: metrics.ALPHA,
-		})
-	// NumResourceClaims tracks the current number of ResourceClaims.
-	NumResourceClaims = metrics.NewGauge(
-		&metrics.GaugeOpts{
-			Subsystem:      ResourceClaimSubsystem,
-			Name:           "resource_claims",
-			Help:           "Number of ResourceClaims",
-			StabilityLevel: metrics.ALPHA,
-		})
-	// NumAllocatedResourceClaims tracks the current number of allocated ResourceClaims.
-	NumAllocatedResourceClaims = metrics.NewGauge(
-		&metrics.GaugeOpts{
-			Subsystem:      ResourceClaimSubsystem,
-			Name:           "allocated_resource_claims",
-			Help:           "Number of allocated ResourceClaims",
-			StabilityLevel: metrics.ALPHA,
-		})
+		},
+		[]string{"status", "admin_access"},
+	)
+
+	// NumResourceClaimsDesc tracks the number of ResourceClaims,
+	// categorized by their allocation status and admin access.
+	NumResourceClaimsDesc = metrics.NewDesc(ResourceClaimSubsystem+"_resource_claims",
+		"Number of ResourceClaims, categorized by allocation status and admin access",
+		[]string{"allocated", "admin_access"}, nil,
+		metrics.ALPHA, "")
 )
 
 var registerMetrics sync.Once
 
+// testMode indicates whether we're running in test mode
+// In test mode, we don't register the custom collector in the global registry
+var testMode bool
+
+// SetTestMode enables or disables test mode
+func SetTestMode(enabled bool) {
+	testMode = enabled
+}
+
 // RegisterMetrics registers ResourceClaim metrics.
-func RegisterMetrics() {
+func RegisterMetrics(collector metrics.StableCollector) {
 	registerMetrics.Do(func() {
-		legacyregistry.MustRegister(ResourceClaimCreateAttempts)
-		legacyregistry.MustRegister(ResourceClaimCreateFailures)
-		legacyregistry.MustRegister(NumResourceClaims)
-		legacyregistry.MustRegister(NumAllocatedResourceClaims)
+		legacyregistry.MustRegister(ResourceClaimCreate)
+		if !testMode && collector != nil {
+			// Only register custom collector in non-test mode
+			legacyregistry.CustomMustRegister(collector)
+		}
 	})
 }