From 801de96635b7a1e27f984aa414a42205b236e07b Mon Sep 17 00:00:00 2001 From: Sunyanan Choochotkaew Date: Thu, 6 Nov 2025 03:15:43 +0900 Subject: [PATCH] DRA: ConsumableCapacity update allocating and registry test cases Signed-off-by: Sunyanan Choochotkaew --- pkg/api/resourceclaimspec/util_test.go | 149 ++++++++++++++++++ .../resourceclaimtemplate/strategy_test.go | 35 ++++ .../allocatortesting/allocator_testing.go | 91 +++++++++++ .../experimental/allocator_experimental.go | 27 ++-- .../internal/experimental/constraint.go | 5 +- 5 files changed, 288 insertions(+), 19 deletions(-) create mode 100644 pkg/api/resourceclaimspec/util_test.go diff --git a/pkg/api/resourceclaimspec/util_test.go b/pkg/api/resourceclaimspec/util_test.go new file mode 100644 index 00000000000..d1f69d425da --- /dev/null +++ b/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) + }) + } +} diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go b/pkg/registry/resource/resourceclaimtemplate/strategy_test.go index cfe470c6478..73f2fae8296 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go +++ b/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() diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go index 9ae2b92f2a8..de1bfb9f5e7 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go +++ b/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, diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go index b618d7aabe0..3fcbd960bbc 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go +++ b/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 } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go index 87fa60c5840..f528832faa7 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go +++ b/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 }