Merge pull request #132927 from pohly/dra-api-strategy-todo

DRA API: implement ResourceClaim strategy for DRADeviceTaints

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
+}

pkg/registry/resource/resourceclaim/strategy.go

@@ -38,6 +38,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"
@@ -92,7 +93,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 {
@@ -119,7 +120,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 {
@@ -174,8 +175,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 {
@@ -222,43 +223,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)
-	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
@@ -273,15 +254,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
 	}
@@ -292,10 +268,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 {
@@ -322,6 +296,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
@@ -368,64 +346,13 @@ 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) ||
-		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 {
@@ -446,30 +373,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

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,
 			})

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,115 +111,9 @@ func toSelectableFields(template *resource.ResourceClaimTemplate) fields.Set {
 }
 
 func dropDisabledFields(newClaimTemplate, oldClaimTemplate *resource.ResourceClaimTemplate) {
-	dropDisabledDRAPrioritizedListFields(newClaimTemplate, oldClaimTemplate)
-	dropDisabledDRAAdminAccessFields(newClaimTemplate, oldClaimTemplate)
-	dropDisabledDRAResourceClaimConsumableCapacityFields(newClaimTemplate, oldClaimTemplate)
-}
-
-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)
 }

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())
+		})
+	}
+}