Merge pull request #134615 from sunya-ch/consumable-capacity-testing-update

DRA: ConsumableCapacity update allocating and registry test cases

pkg/api/resourceclaimspec/util_test.go

@@ -0,0 +1,149 @@
+/*
+Copyright 2025 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 resourceclaimspec
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	apiresource "k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/kubernetes/pkg/apis/resource"
+	"k8s.io/utils/ptr"
+)
+
+var testCapacity = map[resource.QualifiedName]apiresource.Quantity{
+	resource.QualifiedName("test-capacity"): apiresource.MustParse("1"),
+}
+
+var obj = &resource.ResourceClaim{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "valid-claim",
+		Namespace: "kube-system",
+	},
+	Spec: resource.ResourceClaimSpec{
+		Devices: resource.DeviceClaim{
+			Requests: []resource.DeviceRequest{
+				{
+					Name: "req-0",
+					Exactly: &resource.ExactDeviceRequest{
+						DeviceClassName: "class",
+						AllocationMode:  resource.DeviceAllocationModeAll,
+					},
+				},
+			},
+		},
+	},
+}
+
+var objWithPrioritizedList = &resource.ResourceClaim{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "valid-claim",
+		Namespace: "kube-system",
+	},
+	Spec: resource.ResourceClaimSpec{
+		Devices: resource.DeviceClaim{
+			Requests: []resource.DeviceRequest{
+				{
+					Name: "req-0",
+					FirstAvailable: []resource.DeviceSubRequest{
+						{
+							Name:            "subreq-0",
+							DeviceClassName: "class",
+							AllocationMode:  resource.DeviceAllocationModeExactCount,
+							Count:           1,
+						},
+					},
+				},
+			},
+		},
+	},
+}
+
+func modifySpecDeviceRequestWithCapacityRequests(resourceClaim *resource.ResourceClaim,
+	capacity map[resource.QualifiedName]apiresource.Quantity, prioritizedListFeature bool) {
+	if capacity != nil {
+		if prioritizedListFeature {
+			resourceClaim.Spec.Devices.Requests[0].FirstAvailable[0].Capacity = &resource.CapacityRequirements{
+				Requests: capacity,
+			}
+		} else {
+			resourceClaim.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{
+				Requests: capacity,
+			}
+		}
+	}
+}
+
+func addDistinctAttribute(resourceClaim *resource.ResourceClaim) {
+	distinctConstraint := resource.DeviceConstraint{
+		Requests:          []string{"req-0"},
+		DistinctAttribute: ptr.To(resource.FullyQualifiedName("driver-a/attr")),
+	}
+	resourceClaim.Spec.Devices.Constraints = append(resourceClaim.Spec.Devices.Constraints, distinctConstraint)
+}
+
+func TestDRAConsumableCapacityFeatureInUse(t *testing.T) {
+	testcases := map[string]struct {
+		obj    *resource.ResourceClaim
+		expect bool
+	}{
+		"consumable-capacity-empty": {
+			obj:    nil,
+			expect: false,
+		},
+		"consumable-capacity-no-inuse": {
+			obj:    obj,
+			expect: false,
+		},
+		"consumable-capacity-with-inuse-fields": {
+			obj: func() *resource.ResourceClaim {
+				obj := obj.DeepCopy()
+				modifySpecDeviceRequestWithCapacityRequests(obj, testCapacity, false)
+				return obj
+			}(),
+			expect: true,
+		},
+		"consumable-capacity--with-inuse-fields-with-distinct-attribute": {
+			obj: func() *resource.ResourceClaim {
+				obj := obj.DeepCopy()
+				modifySpecDeviceRequestWithCapacityRequests(obj, testCapacity, false)
+				addDistinctAttribute(obj)
+				return obj
+			}(),
+			expect: true,
+		},
+		"consumable-capacity--with-inuse-fields-in-subrequests": {
+			obj: func() *resource.ResourceClaim {
+				obj := objWithPrioritizedList.DeepCopy()
+				modifySpecDeviceRequestWithCapacityRequests(obj, testCapacity, true)
+				return obj
+			}(),
+			expect: true,
+		},
+	}
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			var spec *resource.ResourceClaimSpec
+			if tc.obj != nil {
+				spec = &tc.obj.Spec
+			}
+			assert.Equal(t, DRAConsumableCapacityFeatureInUse(spec), tc.expect)
+		})
+	}
+}

pkg/registry/resource/resourceclaimtemplate/strategy_test.go

@@ -585,6 +585,7 @@ func TestStrategyUpdate(t *testing.T) {
 		adminAccess            bool
 		deviceTaints           bool
 		prioritizedList        bool
+		consumableCapacity     bool
 		expectValidationErrors []string
 		expectObj              *resource.ResourceClaimTemplate
 		verify                 func(*testing.T, []testclient.Action)
@@ -808,6 +809,39 @@ func TestStrategyUpdate(t *testing.T) {
 				}
 			},
 		},
+		"keep-existing-fields-consumable-capacity": {
+			oldObj:             objWithCapacityRequests,
+			newObj:             objWithCapacityRequests,
+			consumableCapacity: true,
+			expectObj:          objWithCapacityRequests,
+			verify: func(t *testing.T, as []testclient.Action) {
+				if len(as) != 0 {
+					t.Errorf("expected no action to be taken")
+				}
+			},
+		},
+		"keep-existing-fields-consumable-capacity-disabled-feature": {
+			oldObj:             objWithCapacityRequests,
+			newObj:             objWithCapacityRequests,
+			consumableCapacity: false,
+			expectObj:          objWithCapacityRequests,
+			verify: func(t *testing.T, as []testclient.Action) {
+				if len(as) != 0 {
+					t.Errorf("expected no action to be taken")
+				}
+			},
+		},
+		"drop-fields-consumable-capacity": {
+			oldObj:                 obj,
+			newObj:                 objWithCapacityRequests,
+			consumableCapacity:     false,
+			expectValidationErrors: []string{fieldImmutableError}, // Spec is immutable.
+			verify: func(t *testing.T, as []testclient.Action) {
+				if len(as) != 0 {
+					t.Errorf("expected no action to be taken")
+				}
+			},
+		},
 	}
 
 	for name, tc := range testcases {
@@ -818,6 +852,7 @@ func TestStrategyUpdate(t *testing.T) {
 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAAdminAccess, tc.adminAccess)
 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRADeviceTaints, tc.deviceTaints)
 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAPrioritizedList, tc.prioritizedList)
+			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAConsumableCapacity, tc.consumableCapacity)
 			strategy := NewStrategy(mockNSClient)
 
 			oldObj := tc.oldObj.DeepCopy()

staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go

@@ -4304,6 +4304,97 @@ func TestAllocator(t *testing.T,
 				},
 			},
 		},
+		"consumable-capacity-disabled-feature": {
+			features: Features{
+				DeviceBindingAndStatus: true, // add to forcefully use experimenting allocator
+			},
+			claimsToAllocate: objects(
+				claim(claim0).withRequests(deviceRequest(req0, classA, 1).withCapacityRequest(ptr.To(one))),
+			),
+			classes:       objects(class(classA, driverA)),
+			slices:        unwrap(sliceWithOneDevice(slice1, node1, pool1, driverA)),
+			node:          node(node1, region1),
+			expectResults: nil,
+			expectError:   gomega.MatchError(gomega.ContainSubstring("claim claim-0, request req-0: has capacity requests, but the DRAConsumableCapacity feature is disabled")),
+		},
+		"consumable-capacity-disabled-feature-with-prioritized-list": {
+			features: Features{
+				PrioritizedList:        true,
+				DeviceBindingAndStatus: true, // add to forcefully use experimenting allocator
+			},
+			claimsToAllocate: objects(
+				claim(claim0).withRequests(
+					requestWithPrioritizedList(
+						req0,
+						subRequest(subReq0, classA, 1).withCapacityRequest(ptr.To(one)),
+						subRequest(subReq1, classA, 1).withCapacityRequest(ptr.To(one)),
+					),
+				),
+			),
+			classes:       objects(class(classA, driverA)),
+			slices:        unwrap(sliceWithOneDevice(slice1, node1, pool1, driverA)),
+			node:          node(node1, region1),
+			expectResults: nil,
+			expectError:   gomega.MatchError(gomega.ContainSubstring("claim claim-0, subrequest subReq-0: has capacity requests, but the DRAConsumableCapacity feature is disabled")),
+		},
+		"consumable-capacity-multi-allocatable-device-with-missing-capacity-request": {
+			features: Features{
+				ConsumableCapacity: true,
+			},
+			claimsToAllocate: objects(
+				claim(claim0).withRequests(deviceRequest(req0, classA, 1).withCapacityRequest(ptr.To(one))),
+			),
+			classes: objects(class(classA, driverA)),
+			slices: unwrap(
+				slice(slice1, node1, pool1, driverA,
+					device(device1, map[resourceapi.QualifiedName]resource.Quantity{capacity1: one}, nil).withAllowMultipleAllocations(),
+				),
+			),
+			node:          node(node1, region1),
+			expectResults: []any{},
+		},
+		"consumable-capacity-multi-allocatable-device-allocation-mode-all-with-missing-capacity-request": {
+			features: Features{
+				ConsumableCapacity: true,
+			},
+			claimsToAllocate: objects(
+				claim(claim0).withRequests(allDeviceRequest(req0, classA).withCapacityRequest(ptr.To(one))),
+			),
+			classes: objects(classWithAllowMultipleAllocations(classA, driverA, true)),
+			slices: unwrap(
+				slice(slice1, node1, pool1, driverA,
+					device(device1, map[resourceapi.QualifiedName]resource.Quantity{capacity1: one}, nil).withAllowMultipleAllocations(),
+				),
+			),
+			node:          node(node1, region1),
+			expectResults: []any{},
+		},
+		"consumable-capacity-multi-allocatable-device-without-capacity-without-capacity-request": {
+			features: Features{
+				ConsumableCapacity: true,
+			},
+			claimsToAllocate: objects(
+				claim(claim0).withRequests(deviceRequest(req0, classA, 1)),
+				claim(claim1).withRequests(deviceRequest(req0, classA, 1)),
+			),
+			classes: objects(classWithAllowMultipleAllocations(classA, driverA, true)),
+			slices: unwrap(
+				slice(slice1, node1, pool1, driverA,
+					device(device1, nil, nil).withAllowMultipleAllocations(),
+				),
+			),
+			node: node(node1, region1),
+			expectResults: []any{ // both share the same device; no capacity is consumed since none is defined.
+				allocationResult(
+					localNodeSelector(node1),
+					deviceRequestAllocationResult(req0, driverA, pool1, device1).withConsumedCapacity(&fixedShareID, nil),
+				),
+				allocationResult(
+					localNodeSelector(node1),
+					deviceRequestAllocationResult(req0, driverA, pool1, device1).withConsumedCapacity(&fixedShareID, nil),
+				),
+			},
+		},
 		"consumable-capacity-multi-allocatable-device-without-policy-without-capacity-request": {
 			features: Features{
 				ConsumableCapacity: true,

staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go

@@ -205,20 +205,17 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node, claims []*resou
 			// Error out if the consumableCapacity feature is not enabled
 			// and the request contains capacity requests.
 			if !a.features.ConsumableCapacity {
-				containsCapacityRequest := false
 				if request.Exactly != nil && request.Exactly.Capacity != nil {
-					containsCapacityRequest = true
-				}
-				for _, request := range request.FirstAvailable {
-					if request.Capacity != nil {
-						containsCapacityRequest = true
-						break
-					}
-				}
-				if containsCapacityRequest {
 					return nil, fmt.Errorf("claim %s, request %s: has capacity requests, but the DRAConsumableCapacity feature is disabled",
 						klog.KObj(claim), request.Name)
 				}
+				for _, subReq := range request.FirstAvailable {
+					// Error out if any subrequest contains capacity requests.
+					if subReq.Capacity != nil {
+						return nil, fmt.Errorf("claim %s, subrequest %s: has capacity requests, but the DRAConsumableCapacity feature is disabled",
+							klog.KObj(claim), subReq.Name)
+					}
+				}
 			}
 
 			if hasSubRequests {
@@ -226,17 +223,10 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node, claims []*resou
 				// for the request, so setting this to a high number so we can do the
 				// easy comparison in the loop.
 				minDevicesPerRequest := math.MaxInt
-
 				// A request with subrequests gets one entry per subrequest in alloc.requestData.
 				// We can only predict a lower number of devices because it depends on which
 				// subrequest gets chosen.
 				for i, subReq := range request.FirstAvailable {
-					// Error out if the consumableCapacity feature is not enabled
-					// and the subrequest contains capacity requests.
-					if !a.features.ConsumableCapacity && subReq.Capacity != nil {
-						return nil, fmt.Errorf("claim %s, subrequest %s: has capacity requests, but the DRAConsumableCapacity feature is disabled",
-							klog.KObj(claim), subReq.Name)
-					}
 					reqData, err := alloc.validateDeviceRequest(&deviceSubRequestAccessor{subRequest: &subReq},
 						&exactDeviceRequestAccessor{request: request}, requestKey, pools)
 					if err != nil {
@@ -1329,8 +1319,9 @@ func (alloc *allocator) allocateDevice(r deviceIndices, device deviceWithID, mus
 	if alloc.features.ConsumableCapacity {
 		// Validate whether resource request over capacity
 		success, err := alloc.CmpRequestOverCapacity(requestData.request, device.slice, *device.Device)
+		// The error should not occur at this point as it should be detected in the previous step.
 		if err != nil {
-			alloc.logger.V(7).Info("Failed to compare device capacity request",
+			alloc.logger.V(7).Info("Failed to compare device capacity request on allocateDevice",
 				"device", device, "request", requestData.request.name(), "err", err)
 			return false, nil, nil
 		}

staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go

@@ -63,7 +63,7 @@ func (m *distinctAttributeConstraint) add(requestName, subRequestName string, de
 	}
 
 	if !m.matchesAttribute(*attribute) {
-		m.logger.V(7).Info("Constraint not satisfied, duplicated attribute")
+		m.logger.V(7).Info("Constraint not satisfied, has some duplicated attributes")
 		return false
 	}
 	m.attributes[requestName] = *attribute
@@ -120,9 +120,12 @@ func (m *distinctAttributeConstraint) matchesAttribute(attribute resourceapi.Dev
 			}
 		default:
 			// Unknown value type, cannot match.
+			// This condition should not be reached
+			// as the unknown value type should be failed on CEL compile (getAttributeValue).
 			m.logger.V(7).Info("Distinct attribute type unknown")
 			return false
 		}
 	}
+	// All distinct
 	return true
 }