From ac5046786e058d3415567ca8928ed29e16060817 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 14 Jul 2025 14:55:50 +0200 Subject: [PATCH 1/2] DRA API: implement ResourceClaim and ResourceClaimTemplate strategy for DRADeviceTaints This wasn't possible at the time of implementing the Device Taints API, at least not completely, because it depended on prioritized list being merged first, to cover the "FirstAvailable" field introduced together with that feature. That the device taints PR got merged despite this gap was an oversight. The confusing TODO probably didn't help: the entire implementation was missing (or got lost due to a bad merge conflict resolution, not sure anymore) and it referenced the wrong other feature (partitionable devices doesn't affect ResourceClaim). For some reason, ResourceClaimTemplate update testing was less complete than the update testing of ResourceClaim. Fixed by copying the entire TestStrategyUpdate over and switching it to testing ResourceClaimTemplates. --- .../resource/resourceclaim/strategy.go | 38 +- .../resource/resourceclaim/strategy_test.go | 190 ++++++++- .../resourceclaimtemplate/strategy.go | 36 ++ .../resourceclaimtemplate/strategy_test.go | 385 +++++++++++++++++- 4 files changed, 645 insertions(+), 4 deletions(-) diff --git a/pkg/registry/resource/resourceclaim/strategy.go b/pkg/registry/resource/resourceclaim/strategy.go index ee9bfb14933..7a3f6dbd2ee 100644 --- a/pkg/registry/resource/resourceclaim/strategy.go +++ b/pkg/registry/resource/resourceclaim/strategy.go @@ -239,6 +239,7 @@ func toSelectableFields(claim *resource.ResourceClaim) fields.Set { // dropDisabledFields removes fields which are covered by a feature gate. func dropDisabledFields(newClaim, oldClaim *resource.ResourceClaim) { dropDisabledDRAPrioritizedListFields(newClaim, oldClaim) + dropDisabledDRADeviceTaintsFields(newClaim, oldClaim) // Intentionally after dropDisabledDRAPrioritizedListFields to avoid iterating over FirstAvailable slice which needs to be dropped. dropDisabledDRAAdminAccessFields(newClaim, oldClaim) dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim) dropDisabledDRADeviceBindingConditionsFields(newClaim, oldClaim) @@ -323,6 +324,41 @@ func draAdminAccessFeatureInUse(claim *resource.ResourceClaim) bool { return false } +func dropDisabledDRADeviceTaintsFields(newClaim, oldClaim *resource.ResourceClaim) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceTaints) || + draDeviceTaintsInUse(oldClaim) { + return + } + + for i, req := range newClaim.Spec.Devices.Requests { + if exactly := req.Exactly; exactly != nil { + exactly.Tolerations = nil + } + for e := range req.FirstAvailable { + newClaim.Spec.Devices.Requests[i].FirstAvailable[e].Tolerations = nil + } + } +} + +func draDeviceTaintsInUse(claim *resource.ResourceClaim) bool { + if claim == nil { + return false + } + + for _, req := range claim.Spec.Devices.Requests { + if exactly := req.Exactly; exactly != nil && len(exactly.Tolerations) > 0 { + return true + } + for _, sub := range req.FirstAvailable { + if len(sub.Tolerations) > 0 { + return true + } + } + } + + return false +} + func isDRAResourceClaimDeviceStatusInUse(claim *resource.ResourceClaim) bool { return claim != nil && len(claim.Status.Devices) > 0 } @@ -382,8 +418,6 @@ func dropDeallocatedStatusDevices(newClaim, oldClaim *resource.ResourceClaim) { } } -// TODO: add tests after partitionable devices is merged (code conflict!) - // dropDisabledDRADeviceBindingConditionsFields removes fields which are covered by a feature gate. func dropDisabledDRADeviceBindingConditionsFields(newClaim, oldClaim *resource.ResourceClaim) { if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceBindingConditions) && utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) || diff --git a/pkg/registry/resource/resourceclaim/strategy_test.go b/pkg/registry/resource/resourceclaim/strategy_test.go index 245695c35b3..f280b7105e5 100644 --- a/pkg/registry/resource/resourceclaim/strategy_test.go +++ b/pkg/registry/resource/resourceclaim/strategy_test.go @@ -221,6 +221,27 @@ var objWithAdminAccessStatusInNonAdminNamespace = &resource.ResourceClaim{ }, } +var objWithDeviceTaints = &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, + Tolerations: []resource.DeviceToleration{{Key: "some-key", Operator: resource.DeviceTolerationOpExists}}, + }, + }, + }, + }, + }, +} + var objWithPrioritizedList = &resource.ResourceClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "valid-claim", @@ -245,6 +266,31 @@ var objWithPrioritizedList = &resource.ResourceClaim{ }, } +var objWithDeviceTaintsInPrioritizedList = &resource.ResourceClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-claim", + Namespace: "default", + }, + 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, + Tolerations: []resource.DeviceToleration{{Key: "some-key", Operator: resource.DeviceTolerationOpExists}}, + }, + }, + }, + }, + }, + }, +} + var objWithAdminAccessStatus = &resource.ResourceClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "valid-claim", @@ -375,6 +421,7 @@ func TestStrategyCreate(t *testing.T) { testcases := map[string]struct { obj *resource.ResourceClaim adminAccess bool + deviceTaints bool prioritizedList bool bindingConditions bool deviceStatus bool @@ -430,6 +477,48 @@ func TestStrategyCreate(t *testing.T) { } }, }, + "drop-fields-device-taints": { + obj: objWithDeviceTaints, + deviceTaints: false, + expectObj: obj, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints": { + obj: objWithDeviceTaints, + deviceTaints: true, + expectObj: objWithDeviceTaints, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "drop-fields-device-taints-in-prioritized-list": { + obj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints-in-prioritized-list": { + obj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: true, + prioritizedList: true, + expectObj: objWithDeviceTaintsInPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, "drop-fields-prioritized-list": { obj: objWithPrioritizedList, prioritizedList: false, @@ -532,6 +621,7 @@ func TestStrategyCreate(t *testing.T) { mockNSClient := fakeClient.CoreV1().Namespaces() featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ features.DRAAdminAccess: tc.adminAccess, + features.DRADeviceTaints: tc.deviceTaints, features.DRAPrioritizedList: tc.prioritizedList, features.DRAConsumableCapacity: tc.consumableCapacity, }) @@ -562,9 +652,10 @@ func TestStrategyUpdate(t *testing.T) { oldObj *resource.ResourceClaim newObj *resource.ResourceClaim adminAccess bool - expectValidationError string + deviceTaints bool prioritizedList bool consumableCapacity bool + expectValidationError string expectObj *resource.ResourceClaim verify func(*testing.T, []testclient.Action) }{ @@ -748,6 +839,102 @@ func TestStrategyUpdate(t *testing.T) { } }, }, + "drop-fields-device-taints": { + oldObj: obj, + newObj: objWithDeviceTaints, + deviceTaints: false, + prioritizedList: true, + expectObj: obj, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints": { + oldObj: obj, + newObj: objWithDeviceTaints, + deviceTaints: true, + prioritizedList: true, + expectValidationError: fieldImmutableError, // Spec is immutable, cannot add tolerations. + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints": { + oldObj: objWithDeviceTaints, + newObj: objWithDeviceTaints, + deviceTaints: true, + prioritizedList: true, + expectObj: objWithDeviceTaints, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints-disabled-feature": { + oldObj: objWithDeviceTaints, + newObj: objWithDeviceTaints, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithDeviceTaints, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "drop-fields-device-taints-in-prioritized-list": { + oldObj: objWithPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints-in-prioritized-list": { + oldObj: objWithPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: true, + prioritizedList: true, + expectValidationError: fieldImmutableError, // Spec is immutable, cannot add tolerations. + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints-in-prioritized-list": { + oldObj: objWithDeviceTaintsInPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: true, + prioritizedList: true, + expectObj: objWithDeviceTaintsInPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints-in-prioritized-list-disabled-feature": { + oldObj: objWithDeviceTaintsInPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithDeviceTaintsInPrioritizedList, + 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 { @@ -757,6 +944,7 @@ func TestStrategyUpdate(t *testing.T) { featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ features.DRAAdminAccess: tc.adminAccess, + features.DRADeviceTaints: tc.deviceTaints, features.DRAPrioritizedList: tc.prioritizedList, features.DRAConsumableCapacity: tc.consumableCapacity, }) diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy.go b/pkg/registry/resource/resourceclaimtemplate/strategy.go index 967bf85286b..1c551398e9a 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy.go +++ b/pkg/registry/resource/resourceclaimtemplate/strategy.go @@ -113,10 +113,46 @@ func toSelectableFields(template *resource.ResourceClaimTemplate) fields.Set { func dropDisabledFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { dropDisabledDRAPrioritizedListFields(newClaimTemplate, oldClaimTemplate) + dropDisabledDRADeviceTaintsFields(newClaimTemplate, oldClaimTemplate) // Intentionally after dropDisabledDRAPrioritizedListFields to avoid iterating over FirstAvailable slice which needs to be dropped. dropDisabledDRAAdminAccessFields(newClaimTemplate, oldClaimTemplate) dropDisabledDRAResourceClaimConsumableCapacityFields(newClaimTemplate, oldClaimTemplate) } +func dropDisabledDRADeviceTaintsFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceTaints) || + draDeviceTaintsInUse(oldClaimTemplate) { + return + } + + for i, req := range newClaimTemplate.Spec.Spec.Devices.Requests { + if exactly := req.Exactly; exactly != nil { + exactly.Tolerations = nil + } + for e := range req.FirstAvailable { + newClaimTemplate.Spec.Spec.Devices.Requests[i].FirstAvailable[e].Tolerations = nil + } + } +} + +func draDeviceTaintsInUse(claimTemplate *resource.ResourceClaimTemplate) bool { + if claimTemplate == nil { + return false + } + + for _, req := range claimTemplate.Spec.Spec.Devices.Requests { + if exactly := req.Exactly; exactly != nil && len(exactly.Tolerations) > 0 { + return true + } + for _, sub := range req.FirstAvailable { + if len(sub.Tolerations) > 0 { + return true + } + } + } + + return false +} + func dropDisabledDRAPrioritizedListFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { if utilfeature.DefaultFeatureGate.Enabled(features.DRAPrioritizedList) { return diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go b/pkg/registry/resource/resourceclaimtemplate/strategy_test.go index 33b834c54ce..670addae080 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go +++ b/pkg/registry/resource/resourceclaimtemplate/strategy_test.go @@ -79,6 +79,28 @@ var objWithAdminAccess = &resource.ResourceClaimTemplate{ }, } +var objInNonAdminNamespace = &resource.ResourceClaimTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-claim-template", + Namespace: "default", + }, + Spec: resource.ResourceClaimTemplateSpec{ + Spec: resource.ResourceClaimSpec{ + Devices: resource.DeviceClaim{ + Requests: []resource.DeviceRequest{ + { + Name: "req-0", + Exactly: &resource.ExactDeviceRequest{ + DeviceClassName: "class", + AllocationMode: resource.DeviceAllocationModeAll, + }, + }, + }, + }, + }, + }, +} + var objWithAdminAccessInNonAdminNamespace = &resource.ResourceClaimTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "valid-claim-template", @@ -102,6 +124,29 @@ var objWithAdminAccessInNonAdminNamespace = &resource.ResourceClaimTemplate{ }, } +var objWithDeviceTaints = &resource.ResourceClaimTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-claim-template", + Namespace: "kube-system", + }, + Spec: resource.ResourceClaimTemplateSpec{ + Spec: resource.ResourceClaimSpec{ + Devices: resource.DeviceClaim{ + Requests: []resource.DeviceRequest{ + { + Name: "req-0", + Exactly: &resource.ExactDeviceRequest{ + DeviceClassName: "class", + AllocationMode: resource.DeviceAllocationModeAll, + Tolerations: []resource.DeviceToleration{{Key: "some-key", Operator: resource.DeviceTolerationOpExists}}, + }, + }, + }, + }, + }, + }, +} + var objWithPrioritizedList = &resource.ResourceClaimTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "valid-claim-template", @@ -170,6 +215,33 @@ func addSpecDeviceRequestWithCapacityRequests(resourceClaimTemplate *resource.Re resourceClaimTemplate.Spec.Spec.Devices.Requests = append(resourceClaimTemplate.Spec.Spec.Devices.Requests, r) } +var objWithDeviceTaintsInPrioritizedList = &resource.ResourceClaimTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-claim-template", + Namespace: "default", + }, + Spec: resource.ResourceClaimTemplateSpec{ + 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, + Tolerations: []resource.DeviceToleration{{Key: "some-key", Operator: resource.DeviceTolerationOpExists}}, + }, + }, + }, + }, + }, + }, + }, +} + var ns1 = &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "default", @@ -206,9 +278,10 @@ func TestClaimTemplateStrategyCreate(t *testing.T) { testcases := map[string]struct { obj *resource.ResourceClaimTemplate adminAccess bool - expectValidationError string + deviceTaints bool prioritizedList bool consumableCapacity bool + expectValidationError string expectObj *resource.ResourceClaimTemplate verify func(*testing.T, []testclient.Action) }{ @@ -259,6 +332,48 @@ func TestClaimTemplateStrategyCreate(t *testing.T) { } }, }, + "drop-fields-device-taints": { + obj: objWithDeviceTaints, + deviceTaints: false, + expectObj: obj, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints": { + obj: objWithDeviceTaints, + deviceTaints: true, + expectObj: objWithDeviceTaints, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "drop-fields-device-taints-in-prioritized-list": { + obj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints-in-prioritized-list": { + obj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: true, + prioritizedList: true, + expectObj: objWithDeviceTaintsInPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, "drop-fields-prioritized-list": { obj: objWithPrioritizedList, prioritizedList: false, @@ -361,6 +476,7 @@ func TestClaimTemplateStrategyCreate(t *testing.T) { mockNSClient := fakeClient.CoreV1().Namespaces() featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ features.DRAAdminAccess: tc.adminAccess, + features.DRADeviceTaints: tc.deviceTaints, features.DRAPrioritizedList: tc.prioritizedList, }) strategy := NewStrategy(mockNSClient) @@ -450,3 +566,270 @@ func TestClaimTemplateStrategyUpdate(t *testing.T) { } }) } + +func TestStrategyUpdate(t *testing.T) { + ctx := genericapirequest.NewDefaultContext() + testcases := map[string]struct { + oldObj *resource.ResourceClaimTemplate + newObj *resource.ResourceClaimTemplate + adminAccess bool + deviceTaints bool + prioritizedList bool + expectValidationError string + expectObj *resource.ResourceClaimTemplate + verify func(*testing.T, []testclient.Action) + }{ + "no-changes-okay": { + oldObj: obj, + newObj: obj, + expectObj: obj, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "name-change-not-allowed": { + oldObj: obj, + newObj: func() *resource.ResourceClaimTemplate { + obj := obj.DeepCopy() + obj.Name += "-2" + return obj + }(), + expectValidationError: fieldImmutableError, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "drop-fields-admin-access": { + oldObj: obj, + newObj: objWithAdminAccess, + adminAccess: false, + expectObj: obj, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-admin-access": { + oldObj: obj, + newObj: objWithAdminAccess, + adminAccess: true, + expectValidationError: fieldImmutableError, // Spec is immutable. + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-admin-access": { + oldObj: objWithAdminAccess, + newObj: objWithAdminAccess, + adminAccess: true, + expectObj: objWithAdminAccess, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "admin-access-admin-namespace": { + oldObj: objWithAdminAccess, + newObj: objWithAdminAccess, + adminAccess: true, + expectObj: objWithAdminAccess, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "admin-access-non-admin-namespace": { + oldObj: objInNonAdminNamespace, + newObj: objWithAdminAccessInNonAdminNamespace, + adminAccess: true, + expectValidationError: fieldImmutableError, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "drop-fields-prioritized-list": { + oldObj: obj, + newObj: objWithPrioritizedList, + prioritizedList: false, + expectValidationError: deviceRequestError, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-prioritized-list": { + oldObj: obj, + newObj: objWithPrioritizedList, + prioritizedList: true, + expectValidationError: fieldImmutableError, // Spec is immutable. + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-prioritized-list": { + oldObj: objWithPrioritizedList, + newObj: objWithPrioritizedList, + prioritizedList: true, + expectObj: objWithPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-prioritized-list-disabled-feature": { + oldObj: objWithPrioritizedList, + newObj: objWithPrioritizedList, + prioritizedList: false, + expectObj: objWithPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "drop-fields-device-taints": { + oldObj: obj, + newObj: objWithDeviceTaints, + deviceTaints: false, + prioritizedList: true, + expectObj: obj, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints": { + oldObj: obj, + newObj: objWithDeviceTaints, + deviceTaints: true, + prioritizedList: true, + expectValidationError: fieldImmutableError, // Spec is immutable, cannot add tolerations. + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints": { + oldObj: objWithDeviceTaints, + newObj: objWithDeviceTaints, + deviceTaints: true, + prioritizedList: true, + expectObj: objWithDeviceTaints, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints-disabled-feature": { + oldObj: objWithDeviceTaints, + newObj: objWithDeviceTaints, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithDeviceTaints, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "drop-fields-device-taints-in-prioritized-list": { + oldObj: objWithPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-fields-device-taints-in-prioritized-list": { + oldObj: objWithPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: true, + prioritizedList: true, + expectValidationError: fieldImmutableError, // Spec is immutable, cannot add tolerations. + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints-in-prioritized-list": { + oldObj: objWithDeviceTaintsInPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: true, + prioritizedList: true, + expectObj: objWithDeviceTaintsInPrioritizedList, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "keep-existing-fields-device-taints-in-prioritized-list-disabled-feature": { + oldObj: objWithDeviceTaintsInPrioritizedList, + newObj: objWithDeviceTaintsInPrioritizedList, + deviceTaints: false, + prioritizedList: true, + expectObj: objWithDeviceTaintsInPrioritizedList, + 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 { + t.Run(name, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset(ns1, ns2) + mockNSClient := fakeClient.CoreV1().Namespaces() + + 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) + strategy := NewStrategy(mockNSClient) + + oldObj := tc.oldObj.DeepCopy() + newObj := tc.newObj.DeepCopy() + newObj.ResourceVersion = "4" + + strategy.PrepareForUpdate(ctx, newObj, oldObj) + if errs := strategy.ValidateUpdate(ctx, newObj, oldObj); len(errs) != 0 { + assert.ErrorContains(t, errs[0], tc.expectValidationError, "the error message should have contained the expected error message") + return + } + if tc.expectValidationError != "" { + t.Fatal("expected validation error(s), got none") + } + if warnings := strategy.WarningsOnUpdate(ctx, newObj, oldObj); len(warnings) != 0 { + t.Fatalf("unexpected warnings: %q", warnings) + } + strategy.Canonicalize(newObj) + expectObj := tc.expectObj.DeepCopy() + expectObj.ResourceVersion = "4" + assert.Equal(t, expectObj, newObj) + tc.verify(t, fakeClient.Actions()) + }) + } +} From da80b554a7bd4ca5aee1ccfd9f654a93429cacf6 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 12 Sep 2025 10:16:16 +0200 Subject: [PATCH 2/2] DRA API: unify dropped field logic for ResourceClaim and ResourceClaimTemplate The spec is the same in both, so those fields are now handled by common code. For ResourceClaim spec updates, the "in use" check now only considers the spec. Theoretically some features could be in use in an old ResourceClaim status and not in use in the spec. This can only occur in a spec update, which is currently prevented because the entire spec is immutable. Even if it was allowed, preventing adding disabled fields to the spec is the right thing to do regardless of what may have ended up in the status earlier. --- pkg/api/resourceclaimspec/util.go | 177 +++++++++++++++++ .../resource/resourceclaim/strategy.go | 184 +++--------------- .../resourceclaimtemplate/strategy.go | 155 +-------------- 3 files changed, 214 insertions(+), 302 deletions(-) create mode 100644 pkg/api/resourceclaimspec/util.go diff --git a/pkg/api/resourceclaimspec/util.go b/pkg/api/resourceclaimspec/util.go new file mode 100644 index 00000000000..989e27e82d5 --- /dev/null +++ b/pkg/api/resourceclaimspec/util.go @@ -0,0 +1,177 @@ +/* +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 ( + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/apis/resource" + "k8s.io/kubernetes/pkg/features" +) + +// DropDisabledFields removes disabled fields from the spec unless they were in +// use there before. +// +// Theoretically some features could be in use in an old ResourceClaim status +// and not in use in the spec. This can only occur in a spec update, which is +// currently prevented because the entire spec is immutable. Even if it was +// allowed, preventing adding disabled fields to the spec is the right thing to +// do regardless of what may have ended up in the status earlier. +func DropDisabledFields(new, old *resource.ResourceClaimSpec) { + dropDisabledDRAPrioritizedListFields(new, old) + dropDisabledDRADeviceTaintsFields(new, old) // Intentionally after dropDisabledDRAPrioritizedListFields to avoid iterating over FirstAvailable slice which needs to be dropped. + dropDisabledDRAAdminAccessFields(new, old) + dropDisabledDRAResourceClaimConsumableCapacityFields(new, old) +} + +func dropDisabledDRADeviceTaintsFields(new, old *resource.ResourceClaimSpec) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceTaints) || + draDeviceTaintsInUse(old) { + return + } + + for i, req := range new.Devices.Requests { + if exactly := req.Exactly; exactly != nil { + exactly.Tolerations = nil + } + for e := range req.FirstAvailable { + new.Devices.Requests[i].FirstAvailable[e].Tolerations = nil + } + } +} + +func draDeviceTaintsInUse(spec *resource.ResourceClaimSpec) bool { + if spec == nil { + return false + } + + for _, req := range spec.Devices.Requests { + if exactly := req.Exactly; exactly != nil && len(exactly.Tolerations) > 0 { + return true + } + for _, sub := range req.FirstAvailable { + if len(sub.Tolerations) > 0 { + return true + } + } + } + + return false +} + +func dropDisabledDRAPrioritizedListFields(new, old *resource.ResourceClaimSpec) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRAPrioritizedList) { + return + } + if draPrioritizedListFeatureInUse(old) { + return + } + + for i := range new.Devices.Requests { + new.Devices.Requests[i].FirstAvailable = nil + } +} + +func draPrioritizedListFeatureInUse(spec *resource.ResourceClaimSpec) bool { + if spec == nil { + return false + } + + for _, request := range spec.Devices.Requests { + if len(request.FirstAvailable) > 0 { + return true + } + } + + return false +} + +func dropDisabledDRAAdminAccessFields(new, old *resource.ResourceClaimSpec) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRAAdminAccess) || + DRAAdminAccessFeatureInUse(old) { + // No need to drop anything. + return + } + + for i := range new.Devices.Requests { + if new.Devices.Requests[i].Exactly != nil { + new.Devices.Requests[i].Exactly.AdminAccess = nil + } + } +} + +// DRAAdminAccessFeatureInUse checks whether the feature is in use in the spec. +func DRAAdminAccessFeatureInUse(spec *resource.ResourceClaimSpec) bool { + if spec == nil { + return false + } + + for _, request := range spec.Devices.Requests { + if request.Exactly != nil && request.Exactly.AdminAccess != nil { + return true + } + } + + return false +} + +func dropDisabledDRAResourceClaimConsumableCapacityFields(new, old *resource.ResourceClaimSpec) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRAConsumableCapacity) || + DRAConsumableCapacityFeatureInUse(old) { + // No need to drop anything. + return + } + + for _, constaint := range new.Devices.Constraints { + constaint.DistinctAttribute = nil + } + + for i := range new.Devices.Requests { + if new.Devices.Requests[i].Exactly != nil { + new.Devices.Requests[i].Exactly.Capacity = nil + } + request := new.Devices.Requests[i] + for j := range request.FirstAvailable { + new.Devices.Requests[i].FirstAvailable[j].Capacity = nil + } + } +} + +// DRAConsumableCapacityFeatureInUse checks whether the feature is in use in the spec. +func DRAConsumableCapacityFeatureInUse(spec *resource.ResourceClaimSpec) bool { + if spec == nil { + return false + } + + for _, constaint := range spec.Devices.Constraints { + if constaint.DistinctAttribute != nil { + return true + } + } + + for _, request := range spec.Devices.Requests { + if request.Exactly != nil && request.Exactly.Capacity != nil { + return true + } + for _, subRequest := range request.FirstAvailable { + if subRequest.Capacity != nil { + return true + } + } + } + + return false +} diff --git a/pkg/registry/resource/resourceclaim/strategy.go b/pkg/registry/resource/resourceclaim/strategy.go index 7a3f6dbd2ee..64b54b54272 100644 --- a/pkg/registry/resource/resourceclaim/strategy.go +++ b/pkg/registry/resource/resourceclaim/strategy.go @@ -39,6 +39,7 @@ import ( v1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/dynamic-resource-allocation/structured" "k8s.io/kubernetes/pkg/api/legacyscheme" + "k8s.io/kubernetes/pkg/api/resourceclaimspec" "k8s.io/kubernetes/pkg/apis/resource" "k8s.io/kubernetes/pkg/apis/resource/validation" "k8s.io/kubernetes/pkg/features" @@ -106,7 +107,7 @@ func (*resourceclaimStrategy) PrepareForCreate(ctx context.Context, obj runtime. // Status must not be set by user on create. claim.Status = resource.ResourceClaimStatus{} - dropDisabledFields(claim, nil) + dropDisabledSpecFields(claim, nil) } func (s *resourceclaimStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { @@ -133,7 +134,7 @@ func (*resourceclaimStrategy) PrepareForUpdate(ctx context.Context, obj, old run oldClaim := old.(*resource.ResourceClaim) newClaim.Status = oldClaim.Status - dropDisabledFields(newClaim, oldClaim) + dropDisabledSpecFields(newClaim, oldClaim) } func (s *resourceclaimStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { @@ -188,8 +189,8 @@ func (*resourceclaimStatusStrategy) PrepareForUpdate(ctx context.Context, obj, o newClaim.Spec = oldClaim.Spec metav1.ResetObjectMetaForStatus(&newClaim.ObjectMeta, &oldClaim.ObjectMeta) - dropDeallocatedStatusDevices(newClaim, oldClaim) - dropDisabledFields(newClaim, oldClaim) + dropDisabledStatusFields(newClaim, oldClaim) + dropDeallocatedStatusDevices(newClaim, oldClaim) // NOP if fields got dropped, so do this last. } func (r *resourceclaimStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { @@ -236,44 +237,23 @@ func toSelectableFields(claim *resource.ResourceClaim) fields.Set { return fields } -// dropDisabledFields removes fields which are covered by a feature gate. -func dropDisabledFields(newClaim, oldClaim *resource.ResourceClaim) { - dropDisabledDRAPrioritizedListFields(newClaim, oldClaim) - dropDisabledDRADeviceTaintsFields(newClaim, oldClaim) // Intentionally after dropDisabledDRAPrioritizedListFields to avoid iterating over FirstAvailable slice which needs to be dropped. - dropDisabledDRAAdminAccessFields(newClaim, oldClaim) +// dropDisabledSpecFields removes fields from the spec which are covered by a feature gate. +func dropDisabledSpecFields(newClaim, oldClaim *resource.ResourceClaim) { + var oldClaimSpec *resource.ResourceClaimSpec + if oldClaim != nil { + oldClaimSpec = &oldClaim.Spec + } + resourceclaimspec.DropDisabledFields(&newClaim.Spec, oldClaimSpec) +} + +// dropDisabledStatusFields removes fields from the status which are covered by a feature gate. +func dropDisabledStatusFields(newClaim, oldClaim *resource.ResourceClaim) { dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim) - dropDisabledDRADeviceBindingConditionsFields(newClaim, oldClaim) - dropDisabledDRAResourceClaimConsumableCapacityFields(newClaim, oldClaim) + dropDisabledDRAAdminAccessStatusFields(newClaim, oldClaim) + dropDisabledDRAResourceClaimConsumableCapacityStatusFields(newClaim, oldClaim) } -func dropDisabledDRAPrioritizedListFields(newClaim, oldClaim *resource.ResourceClaim) { - if utilfeature.DefaultFeatureGate.Enabled(features.DRAPrioritizedList) { - return - } - if draPrioritizedListFeatureInUse(oldClaim) { - return - } - - for i := range newClaim.Spec.Devices.Requests { - newClaim.Spec.Devices.Requests[i].FirstAvailable = nil - } -} - -func draPrioritizedListFeatureInUse(claim *resource.ResourceClaim) bool { - if claim == nil { - return false - } - - for _, request := range claim.Spec.Devices.Requests { - if len(request.FirstAvailable) > 0 { - return true - } - } - - return false -} - -func dropDisabledDRAAdminAccessFields(newClaim, oldClaim *resource.ResourceClaim) { +func dropDisabledDRAAdminAccessStatusFields(newClaim, oldClaim *resource.ResourceClaim) { if utilfeature.DefaultFeatureGate.Enabled(features.DRAAdminAccess) { // No need to drop anything. return @@ -288,15 +268,10 @@ func dropDisabledDRAAdminAccessFields(newClaim, oldClaim *resource.ResourceClaim return } - for i := range newClaim.Spec.Devices.Requests { - if newClaim.Spec.Devices.Requests[i].Exactly != nil { - newClaim.Spec.Devices.Requests[i].Exactly.AdminAccess = nil - } - } - if newClaim.Status.Allocation == nil { return } + for i := range newClaim.Status.Allocation.Devices.Results { newClaim.Status.Allocation.Devices.Results[i].AdminAccess = nil } @@ -307,10 +282,8 @@ func draAdminAccessFeatureInUse(claim *resource.ResourceClaim) bool { return false } - for _, request := range claim.Spec.Devices.Requests { - if request.Exactly != nil && request.Exactly.AdminAccess != nil { - return true - } + if resourceclaimspec.DRAAdminAccessFeatureInUse(&claim.Spec) { + return true } if allocation := claim.Status.Allocation; allocation != nil { @@ -324,41 +297,6 @@ func draAdminAccessFeatureInUse(claim *resource.ResourceClaim) bool { return false } -func dropDisabledDRADeviceTaintsFields(newClaim, oldClaim *resource.ResourceClaim) { - if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceTaints) || - draDeviceTaintsInUse(oldClaim) { - return - } - - for i, req := range newClaim.Spec.Devices.Requests { - if exactly := req.Exactly; exactly != nil { - exactly.Tolerations = nil - } - for e := range req.FirstAvailable { - newClaim.Spec.Devices.Requests[i].FirstAvailable[e].Tolerations = nil - } - } -} - -func draDeviceTaintsInUse(claim *resource.ResourceClaim) bool { - if claim == nil { - return false - } - - for _, req := range claim.Spec.Devices.Requests { - if exactly := req.Exactly; exactly != nil && len(exactly.Tolerations) > 0 { - return true - } - for _, sub := range req.FirstAvailable { - if len(sub.Tolerations) > 0 { - return true - } - } - } - - return false -} - func isDRAResourceClaimDeviceStatusInUse(claim *resource.ResourceClaim) bool { return claim != nil && len(claim.Status.Devices) > 0 } @@ -372,6 +310,10 @@ func dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim *resource // dropDeallocatedStatusDevices removes the status.devices that were allocated // in the oldClaim and that have been removed in the newClaim. +// +// In other words, it removes stale status entries after deallocation. Doing +// this in the apiserver avoids having to update clients which might be unaware +// of the status feature. func dropDeallocatedStatusDevices(newClaim, oldClaim *resource.ResourceClaim) { if !utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) && !isDRAResourceClaimDeviceStatusInUse(oldClaim) { return @@ -418,62 +360,13 @@ func dropDeallocatedStatusDevices(newClaim, oldClaim *resource.ResourceClaim) { } } -// dropDisabledDRADeviceBindingConditionsFields removes fields which are covered by a feature gate. -func dropDisabledDRADeviceBindingConditionsFields(newClaim, oldClaim *resource.ResourceClaim) { - if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceBindingConditions) && utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) || - draBindingConditionsFeatureInUse(oldClaim) { - return - } - - if newClaim.Status.Allocation == nil { - return - } - newClaim.Status.Allocation.AllocationTimestamp = nil - - for i := range newClaim.Status.Allocation.Devices.Results { - newClaim.Status.Allocation.Devices.Results[i].BindingConditions = nil - newClaim.Status.Allocation.Devices.Results[i].BindingFailureConditions = nil - } -} - -func draBindingConditionsFeatureInUse(claim *resource.ResourceClaim) bool { - if claim == nil || claim.Status.Allocation == nil { - return false - } - - if claim.Status.Allocation.AllocationTimestamp != nil { - return true - } - - for _, result := range claim.Status.Allocation.Devices.Results { - if len(result.BindingConditions) != 0 || len(result.BindingFailureConditions) != 0 { - return true - } - } - - return false -} - func draConsumableCapacityFeatureInUse(claim *resource.ResourceClaim) bool { if claim == nil { return false } - for _, constaint := range claim.Spec.Devices.Constraints { - if constaint.DistinctAttribute != nil { - return true - } - } - - for _, request := range claim.Spec.Devices.Requests { - if request.Exactly != nil && request.Exactly.Capacity != nil { - return true - } - for _, subRequest := range request.FirstAvailable { - if subRequest.Capacity != nil { - return true - } - } + if resourceclaimspec.DRAConsumableCapacityFeatureInUse(&claim.Spec) { + return true } if allocation := claim.Status.Allocation; allocation != nil { @@ -494,30 +387,15 @@ func draConsumableCapacityFeatureInUse(claim *resource.ResourceClaim) bool { return false } -// dropDisabledDRAResourceClaimConsumableCapacityFields drops any new feature fields -// from the newClaim if they were not used in the oldClaim. -func dropDisabledDRAResourceClaimConsumableCapacityFields(newClaim, oldClaim *resource.ResourceClaim) { +// dropDisabledDRAResourceClaimConsumableCapacityStatusFields drops any new feature fields +// from the newClaim status if they were not used in the oldClaim. +func dropDisabledDRAResourceClaimConsumableCapacityStatusFields(newClaim, oldClaim *resource.ResourceClaim) { if utilfeature.DefaultFeatureGate.Enabled(features.DRAConsumableCapacity) || draConsumableCapacityFeatureInUse(oldClaim) { // No need to drop anything. return } - for _, constaint := range newClaim.Spec.Devices.Constraints { - constaint.DistinctAttribute = nil - } - - // Drop any CapacityRequests newly added. - for i := range newClaim.Spec.Devices.Requests { - if newClaim.Spec.Devices.Requests[i].Exactly != nil { - newClaim.Spec.Devices.Requests[i].Exactly.Capacity = nil - } - request := newClaim.Spec.Devices.Requests[i] - for j := range request.FirstAvailable { - newClaim.Spec.Devices.Requests[i].FirstAvailable[j].Capacity = nil - } - } - if allocation := newClaim.Status.Allocation; allocation != nil { for i := range allocation.Devices.Results { newClaim.Status.Allocation.Devices.Results[i].ShareID = nil diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy.go b/pkg/registry/resource/resourceclaimtemplate/strategy.go index 1c551398e9a..f53e31ba753 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy.go +++ b/pkg/registry/resource/resourceclaimtemplate/strategy.go @@ -26,12 +26,11 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" v1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/kubernetes/pkg/api/legacyscheme" + "k8s.io/kubernetes/pkg/api/resourceclaimspec" "k8s.io/kubernetes/pkg/apis/resource" "k8s.io/kubernetes/pkg/apis/resource/validation" - "k8s.io/kubernetes/pkg/features" resourceutils "k8s.io/kubernetes/pkg/registry/resource" ) @@ -112,151 +111,9 @@ func toSelectableFields(template *resource.ResourceClaimTemplate) fields.Set { } func dropDisabledFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { - dropDisabledDRAPrioritizedListFields(newClaimTemplate, oldClaimTemplate) - dropDisabledDRADeviceTaintsFields(newClaimTemplate, oldClaimTemplate) // Intentionally after dropDisabledDRAPrioritizedListFields to avoid iterating over FirstAvailable slice which needs to be dropped. - dropDisabledDRAAdminAccessFields(newClaimTemplate, oldClaimTemplate) - dropDisabledDRAResourceClaimConsumableCapacityFields(newClaimTemplate, oldClaimTemplate) -} - -func dropDisabledDRADeviceTaintsFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { - if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceTaints) || - draDeviceTaintsInUse(oldClaimTemplate) { - return - } - - for i, req := range newClaimTemplate.Spec.Spec.Devices.Requests { - if exactly := req.Exactly; exactly != nil { - exactly.Tolerations = nil - } - for e := range req.FirstAvailable { - newClaimTemplate.Spec.Spec.Devices.Requests[i].FirstAvailable[e].Tolerations = nil - } - } -} - -func draDeviceTaintsInUse(claimTemplate *resource.ResourceClaimTemplate) bool { - if claimTemplate == nil { - return false - } - - for _, req := range claimTemplate.Spec.Spec.Devices.Requests { - if exactly := req.Exactly; exactly != nil && len(exactly.Tolerations) > 0 { - return true - } - for _, sub := range req.FirstAvailable { - if len(sub.Tolerations) > 0 { - return true - } - } - } - - return false -} - -func dropDisabledDRAPrioritizedListFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { - if utilfeature.DefaultFeatureGate.Enabled(features.DRAPrioritizedList) { - return - } - if draPrioritizedListFeatureInUse(oldClaimTemplate) { - return - } - - for i := range newClaimTemplate.Spec.Spec.Devices.Requests { - newClaimTemplate.Spec.Spec.Devices.Requests[i].FirstAvailable = nil - } -} - -func draPrioritizedListFeatureInUse(claimTemplate *resource.ResourceClaimTemplate) bool { - if claimTemplate == nil { - return false - } - - for _, request := range claimTemplate.Spec.Spec.Devices.Requests { - if len(request.FirstAvailable) > 0 { - return true - } - } - - return false -} - -func dropDisabledDRAAdminAccessFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { - if utilfeature.DefaultFeatureGate.Enabled(features.DRAAdminAccess) { - // No need to drop anything. - return - } - if draAdminAccessFeatureInUse(oldClaimTemplate) { - // If anything was set in the past, then fields must not get - // dropped on potentially unrelated updates. - return - } - - for i := range newClaimTemplate.Spec.Spec.Devices.Requests { - if newClaimTemplate.Spec.Spec.Devices.Requests[i].Exactly != nil { - newClaimTemplate.Spec.Spec.Devices.Requests[i].Exactly.AdminAccess = nil - } - } -} - -func draAdminAccessFeatureInUse(claimTemplate *resource.ResourceClaimTemplate) bool { - if claimTemplate == nil { - return false - } - - for _, request := range claimTemplate.Spec.Spec.Devices.Requests { - if request.Exactly != nil && request.Exactly.AdminAccess != nil { - return true - } - } - - return false -} - -func draConsumableCapacityFeatureInUse(claimTemplate *resource.ResourceClaimTemplate) bool { - if claimTemplate == nil { - return false - } - - for _, constaint := range claimTemplate.Spec.Spec.Devices.Constraints { - if constaint.DistinctAttribute != nil { - return true - } - } - - for _, request := range claimTemplate.Spec.Spec.Devices.Requests { - if request.Exactly != nil && request.Exactly.Capacity != nil { - return true - } - for _, subRequest := range request.FirstAvailable { - if subRequest.Capacity != nil { - return true - } - } - } - - return false -} - -// dropDisabledDRAResourceClaimConsumableCapacityFields drops any new feature field -// from the newClaimTemplate if they were not used in the oldClaimTemplate. -func dropDisabledDRAResourceClaimConsumableCapacityFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) { - if utilfeature.DefaultFeatureGate.Enabled(features.DRAConsumableCapacity) || - draConsumableCapacityFeatureInUse(oldClaimTemplate) { - // No need to drop anything. - return - } - - for _, constaint := range newClaimTemplate.Spec.Spec.Devices.Constraints { - constaint.DistinctAttribute = nil - } - - for i := range newClaimTemplate.Spec.Spec.Devices.Requests { - if newClaimTemplate.Spec.Spec.Devices.Requests[i].Exactly != nil { - newClaimTemplate.Spec.Spec.Devices.Requests[i].Exactly.Capacity = nil - } - request := newClaimTemplate.Spec.Spec.Devices.Requests[i] - for j := range request.FirstAvailable { - newClaimTemplate.Spec.Spec.Devices.Requests[i].FirstAvailable[j].Capacity = nil - } - } + var oldClaimSpec *resource.ResourceClaimSpec + if oldClaimTemplate != nil { + oldClaimSpec = &oldClaimTemplate.Spec.Spec + } + resourceclaimspec.DropDisabledFields(&newClaimTemplate.Spec.Spec, oldClaimSpec) }