diff --git a/pkg/apis/resource/types.go b/pkg/apis/resource/types.go index dd9e76d8bdc..016e7c236de 100644 --- a/pkg/apis/resource/types.go +++ b/pkg/apis/resource/types.go @@ -145,7 +145,7 @@ type ResourceSliceSpec struct { // Devices lists some or all of the devices in this pool. // - // Must not have more than 128 entries. + // Must not have more than 128 entries. If any device uses taints the limit is 64. // // +optional // +listType=atomic @@ -243,6 +243,7 @@ type ResourcePool struct { const ResourceSliceMaxSharedCapacity = 128 const ResourceSliceMaxDevices = 128 +const ResourceSliceMaxDevicesWithTaints = 64 const PoolNameMaxLength = validation.DNS1123SubdomainMaxLength // Same as for a single node name. const BindingConditionsMaxSize = 4 const BindingFailureConditionsMaxSize = 4 @@ -326,7 +327,9 @@ type Device struct { // If specified, these are the driver-defined taints. // - // The maximum number of taints is 4. + // The maximum number of taints is 16. If taints are set for + // any device in a ResourceSlice, then the maximum number of + // allowed devices per ResourceSlice is 64 instead of 128. // // This is an alpha field and requires enabling the DRADeviceTaints // feature gate. @@ -601,8 +604,8 @@ type DeviceAttribute struct { // DeviceAttributeMaxValueLength is the maximum length of a string or version attribute value. const DeviceAttributeMaxValueLength = 64 -// DeviceTaintsMaxLength is the maximum number of taints per device. -const DeviceTaintsMaxLength = 4 +// DeviceTaintsMaxLength is the maximum number of taints per Device. +const DeviceTaintsMaxLength = 16 // The device this taint is attached to has the "effect" on // any claim which does not tolerate the taint and, through the claim, @@ -622,8 +625,10 @@ type DeviceTaint struct { // The effect of the taint on claims that do not tolerate the taint // and through such claims on the pods using them. - // Valid effects are NoSchedule and NoExecute. PreferNoSchedule as used for - // nodes is not valid here. + // + // Valid effects are None, NoSchedule and NoExecute. PreferNoSchedule as used for + // nodes is not valid here. More effects may get added in the future. + // Consumers must treat unknown effects like None. // // +required Effect DeviceTaintEffect @@ -632,6 +637,14 @@ type DeviceTaint struct { // // Implementing PreferNoSchedule would depend on a scoring solution for DRA. // It might get added as part of that. + // + // A possible future new effect is NoExecuteWithPodDisruptionBudget: + // honor the pod disruption budget instead of simply deleting pods. + // This is currently undecided, it could also be a separate field. + // + // Validation must be prepared to allow unknown enums in stored objects, + // which will enable adding new enums within a single release without + // ratcheting. // TimeAdded represents the time at which the taint was added. // Added automatically during create or update if not set. @@ -650,6 +663,9 @@ type DeviceTaint struct { type DeviceTaintEffect string const ( + // No effect, the taint is purely informational. + DeviceTaintEffectNone DeviceTaintEffect = "None" + // Do not allow new pods to schedule which use a tainted device unless they tolerate the taint, // but allow all pods submitted to Kubelet without going through the scheduler // to start, and allow all already-running pods to continue running. @@ -1876,18 +1892,16 @@ type DeviceTaintRule struct { // Changing the spec automatically increments the metadata.generation number. Spec DeviceTaintRuleSpec - // ^^^ - // A spec gets added because adding a status seems likely. - // Such a status could provide feedback on applying the - // eviction and/or statistics (number of matching devices, - // affected allocated claims, pods remaining to be evicted, - // etc.). + // Status provides information about what was requested in the spec. + // + // +optional + Status DeviceTaintRuleStatus } // DeviceTaintRuleSpec specifies the selector and one taint. type DeviceTaintRuleSpec struct { // DeviceSelector defines which device(s) the taint is applied to. - // All selector criteria must be satified for a device to + // All selector criteria must be satisfied for a device to // match. The empty selector matches all devices. Without // a selector, no devices are matches. // @@ -1947,6 +1961,41 @@ type DeviceTaintSelector struct { Selectors []DeviceSelector } +// DeviceTaintRuleStatus provides information about an on-going pod eviction. +type DeviceTaintRuleStatus struct { + // Conditions provide information about the state of the DeviceTaintRule + // and the cluster at some point in time, + // in a machine-readable and human-readable format. + // + // The following condition is currently defined as part of this API, more may + // get added: + // - Type: EvictionInProgress + // - Status: True if there are currently pods which need to be evicted, False otherwise + // (includes the effects which don't cause eviction). + // - Reason: not specified, may change + // - Message: includes information about number of pending pods and already evicted pods + // in a human-readable format, updated periodically, may change + // + // For `effect: None`, the condition above gets set once for each change to + // the spec, with the message containing information about what would happen + // if the effect was `NoExecute`. This feedback can be used to decide whether + // changing the effect to `NoExecute` will work as intended. It only gets + // set once to avoid having to constantly update the status. + // + // Must have 8 or less entries. + // + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition +} + +// DeviceTaintRuleStatusMaxConditions is the maximum number of conditions in DeviceTaintRuleStatus. +const DeviceTaintRuleStatusMaxConditions = 8 + +// DeviceTaintConditionEvictionInProgress is the publicly documented condition type for the DeviceTaintRuleStatus. +const DeviceTaintConditionEvictionInProgress = "EvictionInProgress" + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // DeviceTaintRuleList is a collection of DeviceTaintRules. diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 2d5529c9532..313f829c37e 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -704,9 +704,14 @@ func validateResourceSliceSpec(spec, oldSpec *resource.ResourceSliceSpec, fldPat } sharedCounterToCounterNames := gatherSharedCounterCounterNames(spec.SharedCounters) - allErrs = append(allErrs, validateSet(spec.Devices, resource.ResourceSliceMaxDevices, + maxDevices := resource.ResourceSliceMaxDevices + if haveDeviceTaints(spec) { + maxDevices = resource.ResourceSliceMaxDevicesWithTaints + } + allErrs = append(allErrs, validateSet(spec.Devices, maxDevices, func(device resource.Device, fldPath *field.Path) field.ErrorList { - return validateDevice(device, fldPath, sharedCounterToCounterNames, spec.PerDeviceNodeSelection) + oldDevice := lookupDevice(oldSpec, device.Name) + return validateDevice(device, oldDevice, fldPath, sharedCounterToCounterNames, spec.PerDeviceNodeSelection) }, func(device resource.Device) string { return device.Name @@ -740,6 +745,32 @@ func validateResourceSliceSpec(spec, oldSpec *resource.ResourceSliceSpec, fldPat return allErrs } +func haveDeviceTaints(spec *resource.ResourceSliceSpec) bool { + if spec == nil { + return false + } + + for _, device := range spec.Devices { + if len(device.Taints) > 0 { + return true + } + } + return false +} + +func lookupDevice(spec *resource.ResourceSliceSpec, deviceName string) *resource.Device { + if spec == nil { + return nil + } + for i := range spec.Devices { + device := &spec.Devices[i] + if device.Name == deviceName { + return device + } + } + return nil +} + func validateCounterSet(counterSet resource.CounterSet, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList if counterSet.Name == "" { @@ -782,7 +813,7 @@ func validateResourcePool(pool resource.ResourcePool, fldPath *field.Path) field return allErrs } -func validateDevice(device resource.Device, fldPath *field.Path, sharedCounterToCounterNames map[string]sets.Set[string], perDeviceNodeSelection *bool) field.ErrorList { +func validateDevice(device resource.Device, oldDevice *resource.Device, fldPath *field.Path, sharedCounterToCounterNames map[string]sets.Set[string], perDeviceNodeSelection *bool) field.ErrorList { var allErrs field.ErrorList allowMultipleAllocations := device.AllowMultipleAllocations != nil && *device.AllowMultipleAllocations allErrs = append(allErrs, validateDeviceName(device.Name, fldPath.Child("name"))...) @@ -799,7 +830,15 @@ func validateDevice(device resource.Device, fldPath *field.Path, sharedCounterTo } else { allErrs = append(allErrs, validateMap(device.Capacity, -1, attributeAndCapacityMaxKeyLength, validateQualifiedName, validateSingleAllocatableDeviceCapacity, fldPath.Child("capacity"))...) } - allErrs = append(allErrs, validateSlice(device.Taints, resource.DeviceTaintsMaxLength, validateDeviceTaint, fldPath.Child("taints"))...) + // If the entire set is the same as before then validation can be skipped. + // We could also do the DeepEqual on the entire spec, but here it is a bit cheaper. + if oldDevice == nil || !apiequality.Semantic.DeepEqual(oldDevice.Taints, device.Taints) { + allErrs = append(allErrs, validateSlice(device.Taints, resource.DeviceTaintsMaxLength, + func(taint resource.DeviceTaint, fldPath *field.Path) field.ErrorList { + return validateDeviceTaint(taint, nil, fldPath) + }, + fldPath.Child("taints"))...) + } allErrs = append(allErrs, validateSet(device.ConsumesCounters, -1, validateDeviceCounterConsumption, @@ -1342,7 +1381,11 @@ func validateDeviceTaintRuleSpec(spec, oldSpec *resource.DeviceTaintRuleSpec, fl oldFilter = oldSpec.DeviceSelector // +k8s:verify-mutation:reason=clone } allErrs = append(allErrs, validateDeviceTaintSelector(spec.DeviceSelector, oldFilter, fldPath.Child("deviceSelector"))...) - allErrs = append(allErrs, validateDeviceTaint(spec.Taint, fldPath.Child("taint"))...) + var oldTaint *resource.DeviceTaint + if oldSpec != nil { + oldTaint = &oldSpec.Taint // +k8s:verify-mutation:reason=clone + } + allErrs = append(allErrs, validateDeviceTaint(spec.Taint, oldTaint, fldPath.Child("taint"))...) return allErrs } @@ -1382,20 +1425,22 @@ func validateDeviceTaintSelector(filter, oldFilter *resource.DeviceTaintSelector } var validDeviceTolerationOperators = []resource.DeviceTolerationOperator{resource.DeviceTolerationOpEqual, resource.DeviceTolerationOpExists} -var validDeviceTaintEffects = sets.New(resource.DeviceTaintEffectNoSchedule, resource.DeviceTaintEffectNoExecute) +var validDeviceTaintEffects = sets.New(resource.DeviceTaintEffectNoSchedule, resource.DeviceTaintEffectNoExecute, resource.DeviceTaintEffectNone) -func validateDeviceTaint(taint resource.DeviceTaint, fldPath *field.Path) field.ErrorList { +func validateDeviceTaint(taint resource.DeviceTaint, oldTaint *resource.DeviceTaint, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, metav1validation.ValidateLabelName(taint.Key, fldPath.Child("key"))...) // Includes checking for non-empty. if taint.Value != "" { allErrs = append(allErrs, validateLabelValue(taint.Value, fldPath.Child("value"))...) } - switch { - case taint.Effect == "": - allErrs = append(allErrs, field.Required(fldPath.Child("effect"), "").MarkCoveredByDeclarative()) // Required in a taint. - case !validDeviceTaintEffects.Has(taint.Effect): - allErrs = append(allErrs, field.NotSupported(fldPath.Child("effect"), taint.Effect, sets.List(validDeviceTaintEffects)).MarkCoveredByDeclarative()) + if oldTaint == nil || oldTaint.Effect != taint.Effect { + switch { + case taint.Effect == "": + allErrs = append(allErrs, field.Required(fldPath.Child("effect"), "").MarkCoveredByDeclarative()) // Required in a taint. + case !validDeviceTaintEffects.Has(taint.Effect): + allErrs = append(allErrs, field.NotSupported(fldPath.Child("effect"), taint.Effect, sets.List(validDeviceTaintEffects)).MarkCoveredByDeclarative()) + } } return allErrs @@ -1478,3 +1523,17 @@ func validateDeviceBindingParameters(bindingConditions, bindingFailureConditions return allErrs } + +// ValidateDeviceTaintRuleStatusUpdate tests if a DeviceTaintRule status update is valid. +func ValidateDeviceTaintRuleStatusUpdate(rule, oldRule *resource.DeviceTaintRule) field.ErrorList { + var allErrs field.ErrorList + + fldPath := field.NewPath("status") + allErrs = corevalidation.ValidateObjectMetaUpdate(&rule.ObjectMeta, &oldRule.ObjectMeta, field.NewPath("metadata")) // Covers invalid name changes. + allErrs = append(allErrs, metav1validation.ValidateConditions(rule.Status.Conditions, fldPath.Child("conditions"))...) + if len(rule.Status.Conditions) > resource.DeviceTaintRuleStatusMaxConditions { + allErrs = append(allErrs, field.TooMany(fldPath.Child("conditions"), len(rule.Status.Conditions), resource.DeviceTaintRuleStatusMaxConditions)) + } + + return allErrs +} diff --git a/pkg/apis/resource/validation/validation_devicetaintrule_test.go b/pkg/apis/resource/validation/validation_devicetaintrule_test.go index 089f738cfc5..48c606c388b 100644 --- a/pkg/apis/resource/validation/validation_devicetaintrule_test.go +++ b/pkg/apis/resource/validation/validation_devicetaintrule_test.go @@ -283,6 +283,7 @@ func TestValidateDeviceTaint(t *testing.T) { return claim }(), }, + // Minimal tests for DeviceTaint. Full coverage of validateDeviceTaint is in ResourceSlice test. "valid-taint": { taintRule: func() *resourceapi.DeviceTaintRule { claim := testDeviceTaintRule(goodName, validDeviceTaintRuleSpec) @@ -294,20 +295,33 @@ func TestValidateDeviceTaint(t *testing.T) { return claim }(), }, - "invalid-taint": { + "required-taint": { wantFailures: field.ErrorList{ field.Required(field.NewPath("spec", "taint", "effect"), "").MarkCoveredByDeclarative(), }, taintRule: func() *resourceapi.DeviceTaintRule { claim := testDeviceTaintRule(goodName, validDeviceTaintRuleSpec) claim.Spec.Taint = resourceapi.DeviceTaint{ - // Minimal test. Full coverage of validateDeviceTaint is in ResourceSlice test. Key: goodName, Value: goodName, } return claim }(), }, + "invalid-taint": { + wantFailures: field.ErrorList{ + field.NotSupported(field.NewPath("spec", "taint", "effect"), resourceapi.DeviceTaintEffect("some-other-effect"), []resourceapi.DeviceTaintEffect{resourceapi.DeviceTaintEffectNoExecute, resourceapi.DeviceTaintEffectNoSchedule, resourceapi.DeviceTaintEffectNone}).MarkCoveredByDeclarative(), + }, + taintRule: func() *resourceapi.DeviceTaintRule { + claim := testDeviceTaintRule(goodName, validDeviceTaintRuleSpec) + claim.Spec.Taint = resourceapi.DeviceTaint{ + Effect: "some-other-effect", + Key: goodName, + Value: goodName, + } + return claim + }(), + }, } for name, scenario := range scenarios { @@ -321,6 +335,8 @@ func TestValidateDeviceTaint(t *testing.T) { func TestValidateDeviceTaintUpdate(t *testing.T) { name := "valid" validTaintRule := testDeviceTaintRule(name, validDeviceTaintRuleSpec) + invalidTaintEffectRule := validTaintRule.DeepCopy() + invalidTaintEffectRule.Spec.Taint.Effect = "some-other-effect" scenarios := map[string]struct { old *resourceapi.DeviceTaintRule @@ -339,6 +355,21 @@ func TestValidateDeviceTaintUpdate(t *testing.T) { return taintRule }, }, + "valid-existing-unknown-effect": { + old: invalidTaintEffectRule, + update: func(taintRule *resourceapi.DeviceTaintRule) *resourceapi.DeviceTaintRule { + taintRule.Labels = map[string]string{"a": "b"} + return taintRule + }, + }, + "invalid-new-unknown-effect": { + wantFailures: field.ErrorList{field.NotSupported(field.NewPath("spec", "taint", "effect"), resourceapi.DeviceTaintEffect("some-other-effect"), []resourceapi.DeviceTaintEffect{resourceapi.DeviceTaintEffectNoExecute, resourceapi.DeviceTaintEffectNoSchedule, resourceapi.DeviceTaintEffectNone})}.MarkCoveredByDeclarative(), + old: validTaintRule, + update: func(taintRule *resourceapi.DeviceTaintRule) *resourceapi.DeviceTaintRule { + taintRule.Spec.Taint.Effect = "some-other-effect" + return taintRule + }, + }, } for name, scenario := range scenarios { diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index 598cfd6f2b0..8f90f8b381a 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -793,7 +793,7 @@ func TestValidateClaim(t *testing.T) { field.Invalid(fldPath.Index(5).Child("key"), badName, "name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')").MarkCoveredByDeclarative(), field.Invalid(fldPath.Index(5).Child("value"), badName, "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"), - field.NotSupported(fldPath.Index(5).Child("effect"), resource.DeviceTaintEffect("some-other-effect"), []resource.DeviceTaintEffect{resource.DeviceTaintEffectNoExecute, resource.DeviceTaintEffectNoSchedule}).MarkCoveredByDeclarative(), + field.NotSupported(fldPath.Index(5).Child("effect"), resource.DeviceTaintEffect("some-other-effect"), []resource.DeviceTaintEffect{resource.DeviceTaintEffectNoExecute, resource.DeviceTaintEffectNoSchedule, resource.DeviceTaintEffectNone}).MarkCoveredByDeclarative(), ) return allErrs }(), diff --git a/pkg/apis/resource/validation/validation_resourceslice_test.go b/pkg/apis/resource/validation/validation_resourceslice_test.go index 5bb9a316163..b14dc7f3298 100644 --- a/pkg/apis/resource/validation/validation_resourceslice_test.go +++ b/pkg/apis/resource/validation/validation_resourceslice_test.go @@ -108,6 +108,25 @@ func TestValidateResourceSlice(t *testing.T) { wantFailures: field.ErrorList{field.TooMany(field.NewPath("spec", "devices"), resourceapi.ResourceSliceMaxDevices+1, resourceapi.ResourceSliceMaxDevices)}, slice: testResourceSlice(goodName, goodName, goodName, resourceapi.ResourceSliceMaxDevices+1), }, + "good-taints": { + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSlice(goodName, goodName, goodName, resourceapi.ResourceSliceMaxDevicesWithTaints) + for i := range slice.Spec.Devices { + slice.Spec.Devices[i].Taints = []resourceapi.DeviceTaint{{Key: "example.com/taint", Effect: resourceapi.DeviceTaintEffectNoExecute}} + } + return slice + }(), + }, + "too-large-taints": { + wantFailures: field.ErrorList{field.TooMany(field.NewPath("spec", "devices"), resourceapi.ResourceSliceMaxDevicesWithTaints+1, resourceapi.ResourceSliceMaxDevicesWithTaints)}, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSlice(goodName, goodName, goodName, resourceapi.ResourceSliceMaxDevicesWithTaints+1) + for i := range slice.Spec.Devices { + slice.Spec.Devices[i].Taints = []resourceapi.DeviceTaint{{Key: "example.com/taint", Effect: resourceapi.DeviceTaintEffectNoExecute}} + } + return slice + }(), + }, "missing-name": { wantFailures: field.ErrorList{field.Required(field.NewPath("metadata", "name"), "name or generateName is required")}, slice: testResourceSlice("", goodName, driverName, 1), @@ -497,7 +516,7 @@ func TestValidateResourceSlice(t *testing.T) { field.Invalid(fldPath.Index(3).Child("key"), badName, "name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"), field.Invalid(fldPath.Index(3).Child("value"), badName, "a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"), - field.NotSupported(fldPath.Index(3).Child("effect"), resourceapi.DeviceTaintEffect("some-other-op"), []resourceapi.DeviceTaintEffect{resourceapi.DeviceTaintEffectNoExecute, resourceapi.DeviceTaintEffectNoSchedule}).MarkCoveredByDeclarative(), + field.NotSupported(fldPath.Index(3).Child("effect"), resourceapi.DeviceTaintEffect("some-other-effect"), []resourceapi.DeviceTaintEffect{resourceapi.DeviceTaintEffectNoExecute, resourceapi.DeviceTaintEffectNoSchedule, resourceapi.DeviceTaintEffectNone}).MarkCoveredByDeclarative(), } }(), slice: func() *resourceapi.ResourceSlice { @@ -522,7 +541,7 @@ func TestValidateResourceSlice(t *testing.T) { // Invalid strings. Key: badName, Value: badName, - Effect: "some-other-op", + Effect: "some-other-effect", }, } return slice @@ -867,6 +886,17 @@ func TestValidateResourceSlice(t *testing.T) { func TestValidateResourceSliceUpdate(t *testing.T) { name := "valid" validResourceSlice := testResourceSlice(name, name, name, 1) + invalidResourceSliceWithTaints := validResourceSlice.DeepCopy() + invalidResourceSliceWithTaints.Spec.Devices[0].Taints = []resourceapi.DeviceTaint{ + { + Key: "unhealthy-power", + Effect: resourceapi.DeviceTaintEffectNoExecute, + }, + { + Key: "unhealthy-mem", + Effect: "some-other-effect", + }, + } scenarios := map[string]struct { oldResourceSlice *resourceapi.ResourceSlice @@ -938,6 +968,31 @@ func TestValidateResourceSliceUpdate(t *testing.T) { return slice }, }, + "invalid-new-effect-in-old-device": { + wantFailures: field.ErrorList{field.NotSupported(field.NewPath("spec", "devices").Index(0).Child("taints").Index(1).Child("effect"), resourceapi.DeviceTaintEffect("some-other-effect"), []resourceapi.DeviceTaintEffect{resourceapi.DeviceTaintEffectNoExecute, resourceapi.DeviceTaintEffectNoSchedule, resourceapi.DeviceTaintEffectNone})}.MarkCoveredByDeclarative(), + oldResourceSlice: validResourceSlice, + update: func(slice *resourceapi.ResourceSlice) *resourceapi.ResourceSlice { + slice.Spec.Devices[0].Taints = invalidResourceSliceWithTaints.Spec.Devices[0].Taints + return slice + }, + }, + "valid-old-effect": { + oldResourceSlice: invalidResourceSliceWithTaints, + update: func(slice *resourceapi.ResourceSlice) *resourceapi.ResourceSlice { + slice.Spec.Devices[0].Attributes["foo"] = resourceapi.DeviceAttribute{StringValue: ptr.To("bar")} + return slice + }, + }, + "invalid-new-effect-in-new-device": { + wantFailures: field.ErrorList{field.NotSupported(field.NewPath("spec", "devices").Index(1).Child("taints").Index(1).Child("effect"), resourceapi.DeviceTaintEffect("some-other-effect"), []resourceapi.DeviceTaintEffect{resourceapi.DeviceTaintEffectNoExecute, resourceapi.DeviceTaintEffectNoSchedule, resourceapi.DeviceTaintEffectNone})}.MarkCoveredByDeclarative(), + oldResourceSlice: invalidResourceSliceWithTaints, + update: func(slice *resourceapi.ResourceSlice) *resourceapi.ResourceSlice { + device := slice.Spec.Devices[0].DeepCopy() + device.Name += "-other" + slice.Spec.Devices = append(slice.Spec.Devices, *device) + return slice + }, + }, } for name, scenario := range scenarios { diff --git a/pkg/registry/resource/devicetaintrule/storage/storage.go b/pkg/registry/resource/devicetaintrule/storage/storage.go index 64ff2fb3972..9919426979e 100644 --- a/pkg/registry/resource/devicetaintrule/storage/storage.go +++ b/pkg/registry/resource/devicetaintrule/storage/storage.go @@ -17,14 +17,19 @@ limitations under the License. package storage import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic" genericregistry "k8s.io/apiserver/pkg/registry/generic/registry" + "k8s.io/apiserver/pkg/registry/rest" "k8s.io/kubernetes/pkg/apis/resource" "k8s.io/kubernetes/pkg/printers" printersinternal "k8s.io/kubernetes/pkg/printers/internalversion" printerstorage "k8s.io/kubernetes/pkg/printers/storage" "k8s.io/kubernetes/pkg/registry/resource/devicetaintrule" + "sigs.k8s.io/structured-merge-diff/v6/fieldpath" ) // REST implements a RESTStorage for DeviceTaintRule. @@ -33,7 +38,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against DeviceTaintRule. -func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { +func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &resource.DeviceTaintRule{} }, NewListFunc: func() runtime.Object { return &resource.DeviceTaintRuleList{} }, @@ -44,13 +49,50 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, error) { UpdateStrategy: devicetaintrule.Strategy, DeleteStrategy: devicetaintrule.Strategy, ReturnDeletedObject: true, + ResetFieldsStrategy: devicetaintrule.Strategy, TableConvertor: printerstorage.TableConvertor{TableGenerator: printers.NewTableGenerator().With(printersinternal.AddHandlers)}, } options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { - return nil, err + return nil, nil, err } - return &REST{store}, nil + statusStore := *store + statusStore.UpdateStrategy = devicetaintrule.StatusStrategy + statusStore.ResetFieldsStrategy = devicetaintrule.StatusStrategy + + return &REST{store}, &StatusREST{store: &statusStore}, nil +} + +// StatusREST implements the REST endpoint for changing the status of a DeviceTaintRule. +type StatusREST struct { + store *genericregistry.Store +} + +// New creates a new DeviceTaintRule object. +func (r *StatusREST) New() runtime.Object { + return &resource.DeviceTaintRule{} +} + +func (r *StatusREST) Destroy() { + // Given that underlying store is shared with REST, + // we don't destroy it here explicitly. +} + +// Get retrieves the object from the storage. It is required to support Patch. +func (r *StatusREST) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { + return r.store.Get(ctx, name, options) +} + +// Update alters the status subset of an object. +func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { + // We are explicitly setting forceAllowCreate to false in the call to the underlying storage because + // subresources should never allow create on update. + return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, false, options) +} + +// GetResetFields implements rest.ResetFieldsStrategy +func (r *StatusREST) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { + return r.store.GetResetFields() } diff --git a/pkg/registry/resource/devicetaintrule/storage/storage_test.go b/pkg/registry/resource/devicetaintrule/storage/storage_test.go index c45701a17cc..fa4a88d3434 100644 --- a/pkg/registry/resource/devicetaintrule/storage/storage_test.go +++ b/pkg/registry/resource/devicetaintrule/storage/storage_test.go @@ -20,19 +20,24 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + + apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing" + "k8s.io/apiserver/pkg/registry/rest" etcd3testing "k8s.io/apiserver/pkg/storage/etcd3/testing" "k8s.io/kubernetes/pkg/apis/resource" _ "k8s.io/kubernetes/pkg/apis/resource/install" "k8s.io/kubernetes/pkg/registry/registrytest" ) -func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { +func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) { etcdStorage, server := registrytest.NewEtcdStorageForResource(t, resource.Resource("devicetaintrules")) restOptions := generic.RESTOptions{ StorageConfig: etcdStorage, @@ -40,11 +45,11 @@ func newStorage(t *testing.T) (*REST, *etcd3testing.EtcdTestServer) { DeleteCollectionWorkers: 1, ResourcePrefix: "devicetaintrules", } - deviceTaintStorage, err := NewREST(restOptions) + deviceTaintStorage, statusStorage, err := NewREST(restOptions) if err != nil { t.Fatalf("unexpected error from REST storage: %v", err) } - return deviceTaintStorage, server + return deviceTaintStorage, statusStorage, server } func validNewDeviceTaint(name string) *resource.DeviceTaintRule { @@ -63,7 +68,7 @@ func validNewDeviceTaint(name string) *resource.DeviceTaintRule { } func TestCreate(t *testing.T) { - storage, server := newStorage(t) + storage, _, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store).ClusterScope() @@ -80,7 +85,7 @@ func TestCreate(t *testing.T) { } func TestUpdate(t *testing.T) { - storage, server := newStorage(t) + storage, _, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store).ClusterScope() @@ -98,7 +103,7 @@ func TestUpdate(t *testing.T) { } func TestDelete(t *testing.T) { - storage, server := newStorage(t) + storage, _, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store).ClusterScope().ReturnDeletedObject() @@ -106,7 +111,7 @@ func TestDelete(t *testing.T) { } func TestGet(t *testing.T) { - storage, server := newStorage(t) + storage, _, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store).ClusterScope() @@ -114,7 +119,7 @@ func TestGet(t *testing.T) { } func TestList(t *testing.T) { - storage, server := newStorage(t) + storage, _, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store).ClusterScope() @@ -122,7 +127,7 @@ func TestList(t *testing.T) { } func TestWatch(t *testing.T) { - storage, server := newStorage(t) + storage, _, server := newStorage(t) defer server.Terminate(t) defer storage.Store.DestroyFunc() test := genericregistrytest.New(t, storage.Store).ClusterScope() @@ -144,3 +149,39 @@ func TestWatch(t *testing.T) { }, ) } + +func TestUpdateStatus(t *testing.T) { + storage, statusStorage, server := newStorage(t) + defer server.Terminate(t) + defer storage.Store.DestroyFunc() + ctx := genericapirequest.NewDefaultContext() + + key, _ := storage.KeyFunc(ctx, "foo") + deviceTaintStart := validNewDeviceTaint("foo") + err := storage.Storage.Create(ctx, key, deviceTaintStart, nil, 0, false) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + deviceTaint := deviceTaintStart.DeepCopy() + deviceTaint.Status.Conditions = []metav1.Condition{{ + Type: "EvicitionInProgress", + Status: metav1.ConditionTrue, + Reason: "PodsLeft", + Message: "100 pods left", + LastTransitionTime: metav1.Time{Time: time.Now().Truncate(time.Second)}, + }} + _, _, err = statusStorage.Update(ctx, deviceTaint.Name, rest.DefaultUpdatedObjectInfo(deviceTaint), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{}) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + obj, err := storage.Get(ctx, "foo", &metav1.GetOptions{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + deviceTaintOut := obj.(*resource.DeviceTaintRule) + // only compare relevant changes b/c of difference in metadata + if !apiequality.Semantic.DeepEqual(deviceTaint.Status, deviceTaintOut.Status) { + t.Errorf("unexpected object: %s", cmp.Diff(deviceTaint.Status, deviceTaintOut.Status)) + } +} diff --git a/pkg/registry/resource/devicetaintrule/strategy.go b/pkg/registry/resource/devicetaintrule/strategy.go index d458fc7d538..dc8325ea051 100644 --- a/pkg/registry/resource/devicetaintrule/strategy.go +++ b/pkg/registry/resource/devicetaintrule/strategy.go @@ -20,12 +20,14 @@ import ( "context" apiequality "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/resource" "k8s.io/kubernetes/pkg/apis/resource/validation" + "sigs.k8s.io/structured-merge-diff/v6/fieldpath" ) // deviceTaintRuleStrategy implements behavior for DeviceTaintRule objects @@ -34,51 +36,105 @@ type deviceTaintRuleStrategy struct { names.NameGenerator } -var Strategy = deviceTaintRuleStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} +var ( + Strategy = &deviceTaintRuleStrategy{legacyscheme.Scheme, names.SimpleNameGenerator} + StatusStrategy = &deviceTaintRuleStatusStrategy{deviceTaintRuleStrategy: Strategy} +) func (deviceTaintRuleStrategy) NamespaceScoped() bool { return false } -func (deviceTaintRuleStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { - patch := obj.(*resource.DeviceTaintRule) - patch.Generation = 1 +// GetResetFields returns the set of fields that get reset by the strategy and +// should not be modified by the user. For a new DeviceTaintRule that is the +// status. +func (*deviceTaintRuleStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { + fields := map[fieldpath.APIVersion]*fieldpath.Set{ + "resource.k8s.io/v1alpha3": fieldpath.NewSet( + fieldpath.MakePathOrDie("status"), + ), + } + + return fields } -func (deviceTaintRuleStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { - patch := obj.(*resource.DeviceTaintRule) - return validation.ValidateDeviceTaintRule(patch) +func (*deviceTaintRuleStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { + rule := obj.(*resource.DeviceTaintRule) + // Status must not be set by user on create. + rule.Status = resource.DeviceTaintRuleStatus{} + rule.Generation = 1 } -func (deviceTaintRuleStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { +func (*deviceTaintRuleStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { + rule := obj.(*resource.DeviceTaintRule) + return validation.ValidateDeviceTaintRule(rule) +} + +func (*deviceTaintRuleStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { return nil } -func (deviceTaintRuleStrategy) Canonicalize(obj runtime.Object) { +func (*deviceTaintRuleStrategy) Canonicalize(obj runtime.Object) { } -func (deviceTaintRuleStrategy) AllowCreateOnUpdate() bool { +func (*deviceTaintRuleStrategy) AllowCreateOnUpdate() bool { return false } -func (deviceTaintRuleStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { - patch := obj.(*resource.DeviceTaintRule) - oldPatch := old.(*resource.DeviceTaintRule) +func (*deviceTaintRuleStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + rule := obj.(*resource.DeviceTaintRule) + oldRule := old.(*resource.DeviceTaintRule) + rule.Status = oldRule.Status // Any changes to the spec increment the generation number. - if !apiequality.Semantic.DeepEqual(oldPatch.Spec, patch.Spec) { - patch.Generation = oldPatch.Generation + 1 + if !apiequality.Semantic.DeepEqual(oldRule.Spec, rule.Spec) { + rule.Generation = oldRule.Generation + 1 } } -func (deviceTaintRuleStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { +func (*deviceTaintRuleStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { return validation.ValidateDeviceTaintRuleUpdate(obj.(*resource.DeviceTaintRule), old.(*resource.DeviceTaintRule)) } -func (deviceTaintRuleStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { +func (*deviceTaintRuleStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { return nil } -func (deviceTaintRuleStrategy) AllowUnconditionalUpdate() bool { +func (*deviceTaintRuleStrategy) AllowUnconditionalUpdate() bool { return true } + +type deviceTaintRuleStatusStrategy struct { + *deviceTaintRuleStrategy +} + +// GetResetFields returns the set of fields that get reset by the strategy and +// should not be modified by the user. For a status update that is the spec. +func (*deviceTaintRuleStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { + fields := map[fieldpath.APIVersion]*fieldpath.Set{ + "resource.k8s.io/v1alpha3": fieldpath.NewSet( + fieldpath.MakePathOrDie("metadata"), + fieldpath.MakePathOrDie("spec"), + ), + } + + return fields +} + +func (*deviceTaintRuleStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + newRule := obj.(*resource.DeviceTaintRule) + oldRule := old.(*resource.DeviceTaintRule) + newRule.Spec = oldRule.Spec + metav1.ResetObjectMetaForStatus(&newRule.ObjectMeta, &oldRule.ObjectMeta) +} + +func (r *deviceTaintRuleStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { + newRule := obj.(*resource.DeviceTaintRule) + oldRule := old.(*resource.DeviceTaintRule) + return validation.ValidateDeviceTaintRuleStatusUpdate(newRule, oldRule) +} + +// WarningsOnUpdate returns warnings for the given update. +func (*deviceTaintRuleStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { + return nil +} diff --git a/pkg/registry/resource/devicetaintrule/strategy_test.go b/pkg/registry/resource/devicetaintrule/strategy_test.go index 7923a8e981c..19993748c09 100644 --- a/pkg/registry/resource/devicetaintrule/strategy_test.go +++ b/pkg/registry/resource/devicetaintrule/strategy_test.go @@ -19,14 +19,17 @@ package devicetaintrule import ( "testing" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/kubernetes/pkg/apis/resource" ) -var patch = &resource.DeviceTaintRule{ +var obj = &resource.DeviceTaintRule{ ObjectMeta: metav1.ObjectMeta{ - Name: "valid-patch", + Name: "valid-patch", + Generation: 1, }, Spec: resource.DeviceTaintRuleSpec{ Taint: resource.DeviceTaint{ @@ -36,6 +39,31 @@ var patch = &resource.DeviceTaintRule{ }, } +var objWithStatus = &resource.DeviceTaintRule{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-patch", + Generation: 1, + }, + Spec: resource.DeviceTaintRuleSpec{ + Taint: resource.DeviceTaint{ + Key: "example.com/tainted", + Effect: resource.DeviceTaintEffectNoExecute, + }, + }, + Status: resource.DeviceTaintRuleStatus{ + Conditions: []metav1.Condition{{ + Type: "foo", + Status: metav1.ConditionFalse, + LastTransitionTime: metav1.Now(), + Reason: "something", + Message: "else", + }}, + }, +} + +var fieldImmutableError = "field is immutable" +var metadataError = "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters" + func TestDeviceTaintRuleStrategy(t *testing.T) { if Strategy.NamespaceScoped() { t.Errorf("DeviceTaintRule must not be namespace scoped") @@ -47,40 +75,202 @@ func TestDeviceTaintRuleStrategy(t *testing.T) { func TestDeviceTaintRuleStrategyCreate(t *testing.T) { ctx := genericapirequest.NewDefaultContext() - patch := patch.DeepCopy() + testcases := map[string]struct { + obj *resource.DeviceTaintRule + expectValidationError string + expectObj *resource.DeviceTaintRule + }{ + "simple": { + obj: obj, + expectObj: obj, + }, + "validation-error": { + obj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Name = "%#@$%$" + return obj + }(), + expectValidationError: metadataError, + }, + "drop-status": { + obj: objWithStatus, + expectObj: obj, + }, + "set-generation": { + obj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Generation = 42 // Cannot be set by client on create, overwritten with 1. + return obj + }(), + expectObj: obj, + }, + } - Strategy.PrepareForCreate(ctx, patch) - errs := Strategy.Validate(ctx, patch) - if len(errs) != 0 { - t.Errorf("unexpected error validating for create %v", errs) + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + obj := tc.obj.DeepCopy() + Strategy.PrepareForCreate(ctx, obj) + if errs := Strategy.Validate(ctx, obj); len(errs) != 0 { + if tc.expectValidationError == "" { + t.Fatalf("unexpected error(s): %v", errs) + } + 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.WarningsOnCreate(ctx, obj); len(warnings) != 0 { + t.Fatalf("unexpected warnings: %q", warnings) + } + Strategy.Canonicalize(obj) + assert.Equal(t, tc.expectObj, obj) + }) } } func TestDeviceTaintRuleStrategyUpdate(t *testing.T) { - t.Run("no-changes-okay", func(t *testing.T) { - ctx := genericapirequest.NewDefaultContext() - patch := patch.DeepCopy() - newPatch := patch.DeepCopy() - newPatch.ResourceVersion = "4" + ctx := genericapirequest.NewDefaultContext() - Strategy.PrepareForUpdate(ctx, newPatch, patch) - errs := Strategy.ValidateUpdate(ctx, newPatch, patch) - if len(errs) != 0 { - t.Errorf("unexpected validation errors: %v", errs) - } - }) + testcases := map[string]struct { + oldObj *resource.DeviceTaintRule + newObj *resource.DeviceTaintRule + expectValidationError string + expectObj *resource.DeviceTaintRule + }{ + "no-changes-okay": { + oldObj: obj, + newObj: obj, + expectObj: obj, + }, + "name-change-not-allowed": { + oldObj: obj, + newObj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Name += "-2" + return obj + }(), + expectValidationError: fieldImmutableError, + }, + "drop-status": { + oldObj: obj, + newObj: objWithStatus, + expectObj: obj, + }, + "bump-generation": { + oldObj: obj, + newObj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Spec.Taint.Effect = resource.DeviceTaintEffectNone + return obj + }(), + expectObj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Spec.Taint.Effect = resource.DeviceTaintEffectNone + obj.Generation++ + return obj + }(), + }, + } - t.Run("name-change-not-allowed", func(t *testing.T) { - ctx := genericapirequest.NewDefaultContext() - patch := patch.DeepCopy() - newPatch := patch.DeepCopy() - newPatch.Name = "valid-patch-2" - newPatch.ResourceVersion = "4" + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + oldObj := tc.oldObj.DeepCopy() + newObj := tc.newObj.DeepCopy() + newObj.ResourceVersion = "4" - Strategy.PrepareForUpdate(ctx, newPatch, patch) - errs := Strategy.ValidateUpdate(ctx, newPatch, patch) - if len(errs) == 0 { - t.Errorf("expected a validation error") - } - }) + Strategy.PrepareForUpdate(ctx, newObj, oldObj) + if errs := Strategy.ValidateUpdate(ctx, newObj, oldObj); len(errs) != 0 { + if tc.expectValidationError == "" { + t.Fatalf("unexpected error(s): %v", errs) + } + 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) + }) + } +} + +func TestStatusStrategyUpdate(t *testing.T) { + ctx := genericapirequest.NewDefaultContext() + testcases := map[string]struct { + oldObj *resource.DeviceTaintRule + newObj *resource.DeviceTaintRule + expectValidationError string + expectObj *resource.DeviceTaintRule + }{ + "no-changes-okay": { + oldObj: obj, + newObj: obj, + expectObj: obj, + }, + "name-change-not-allowed": { + oldObj: obj, + newObj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Name += "-2" + return obj + }(), + expectValidationError: fieldImmutableError, + }, + // Cannot add finalizers, annotations and labels during status update. + "drop-meta-changes": { + oldObj: obj, + newObj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Finalizers = []string{"foo"} + obj.Annotations = map[string]string{"foo": "bar"} + obj.Labels = map[string]string{"foo": "bar"} + return obj + }(), + expectObj: obj, + }, + "drop-spec": { + oldObj: obj, + newObj: func() *resource.DeviceTaintRule { + obj := obj.DeepCopy() + obj.Spec.Taint.Effect = resource.DeviceTaintEffectNone + return obj + }(), + expectObj: obj, + }, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + oldObj := tc.oldObj.DeepCopy() + newObj := tc.newObj.DeepCopy() + newObj.ResourceVersion = "4" + + StatusStrategy.PrepareForUpdate(ctx, newObj, oldObj) + if errs := StatusStrategy.ValidateUpdate(ctx, newObj, oldObj); len(errs) != 0 { + if tc.expectValidationError == "" { + t.Fatalf("unexpected error(s): %v", errs) + } + 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 := StatusStrategy.WarningsOnUpdate(ctx, newObj, oldObj); len(warnings) != 0 { + t.Fatalf("unexpected warnings: %q", warnings) + } + StatusStrategy.Canonicalize(newObj) + + expectObj := tc.expectObj.DeepCopy() + expectObj.ResourceVersion = "4" + assert.Equal(t, expectObj, newObj) + }) + } } diff --git a/pkg/registry/resource/resourceclaim/strategy.go b/pkg/registry/resource/resourceclaim/strategy.go index 5dd6db1ba34..ca6d25237fc 100644 --- a/pkg/registry/resource/resourceclaim/strategy.go +++ b/pkg/registry/resource/resourceclaim/strategy.go @@ -153,15 +153,19 @@ func NewStatusStrategy(resourceclaimStrategy *resourceclaimStrategy) *resourcecl func (*resourceclaimStatusStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.Set { fields := map[fieldpath.APIVersion]*fieldpath.Set{ "resource.k8s.io/v1alpha3": fieldpath.NewSet( + fieldpath.MakePathOrDie("metadata"), fieldpath.MakePathOrDie("spec"), ), "resource.k8s.io/v1beta1": fieldpath.NewSet( + fieldpath.MakePathOrDie("metadata"), fieldpath.MakePathOrDie("spec"), ), "resource.k8s.io/v1beta2": fieldpath.NewSet( + fieldpath.MakePathOrDie("metadata"), fieldpath.MakePathOrDie("spec"), ), "resource.k8s.io/v1": fieldpath.NewSet( + fieldpath.MakePathOrDie("metadata"), fieldpath.MakePathOrDie("spec"), ), } diff --git a/pkg/registry/resource/rest/storage_resource.go b/pkg/registry/resource/rest/storage_resource.go index 27660fea478..f960b6a6462 100644 --- a/pkg/registry/resource/rest/storage_resource.go +++ b/pkg/registry/resource/rest/storage_resource.go @@ -118,11 +118,12 @@ func (p RESTStorageProvider) v1alpha3Storage(apiResourceConfigSource serverstora storage := map[string]rest.Storage{} if resource := "devicetaintrules"; apiResourceConfigSource.ResourceEnabled(resourcev1alpha3.SchemeGroupVersion.WithResource(resource)) { - deviceTaintStorage, err := devicetaintrulestore.NewREST(restOptionsGetter) + deviceTaintStorage, deviceTaintStatusStorage, err := devicetaintrulestore.NewREST(restOptionsGetter) if err != nil { return nil, err } storage[resource] = deviceTaintStorage + storage[resource+"/status"] = deviceTaintStatusStorage } return storage, nil diff --git a/staging/src/k8s.io/api/resource/v1/types.go b/staging/src/k8s.io/api/resource/v1/types.go index f698be61969..15b897e5c0d 100644 --- a/staging/src/k8s.io/api/resource/v1/types.go +++ b/staging/src/k8s.io/api/resource/v1/types.go @@ -149,7 +149,7 @@ type ResourceSliceSpec struct { // Devices lists some or all of the devices in this pool. // - // Must not have more than 128 entries. + // Must not have more than 128 entries. If any device uses taints the limit is 64. // // +optional // +listType=atomic @@ -250,6 +250,7 @@ type ResourcePool struct { const ResourceSliceMaxSharedCapacity = 128 const ResourceSliceMaxDevices = 128 +const ResourceSliceMaxDevicesWithTaints = 64 const PoolNameMaxLength = validation.DNS1123SubdomainMaxLength // Same as for a single node name. const BindingConditionsMaxSize = 4 const BindingFailureConditionsMaxSize = 4 @@ -333,7 +334,9 @@ type Device struct { // If specified, these are the driver-defined taints. // - // The maximum number of taints is 4. + // The maximum number of taints is 16. If taints are set for + // any device in a ResourceSlice, then the maximum number of + // allowed devices per ResourceSlice is 64 instead of 128. // // This is an alpha field and requires enabling the DRADeviceTaints // feature gate. @@ -618,8 +621,8 @@ type DeviceAttribute struct { // DeviceAttributeMaxValueLength is the maximum length of a string or version attribute value. const DeviceAttributeMaxValueLength = 64 -// DeviceTaintsMaxLength is the maximum number of taints per device. -const DeviceTaintsMaxLength = 4 +// DeviceTaintsMaxLength is the maximum number of taints per Device. +const DeviceTaintsMaxLength = 16 // The device this taint is attached to has the "effect" on // any claim which does not tolerate the taint and, through the claim, @@ -641,8 +644,10 @@ type DeviceTaint struct { // The effect of the taint on claims that do not tolerate the taint // and through such claims on the pods using them. - // Valid effects are NoSchedule and NoExecute. PreferNoSchedule as used for - // nodes is not valid here. + // + // Valid effects are None, NoSchedule and NoExecute. PreferNoSchedule as used for + // nodes is not valid here. More effects may get added in the future. + // Consumers must treat unknown effects like None. // // +required // +k8s:required @@ -652,6 +657,14 @@ type DeviceTaint struct { // // Implementing PreferNoSchedule would depend on a scoring solution for DRA. // It might get added as part of that. + // + // A possible future new effect is NoExecuteWithPodDisruptionBudget: + // honor the pod disruption budget instead of simply deleting pods. + // This is currently undecided, it could also be a separate field. + // + // Validation must be prepared to allow unknown enums in stored objects, + // which will enable adding new enums within a single release without + // ratcheting. // TimeAdded represents the time at which the taint was added. // Added automatically during create or update if not set. @@ -671,6 +684,9 @@ type DeviceTaint struct { type DeviceTaintEffect string const ( + // No effect, the taint is purely informational. + DeviceTaintEffectNone DeviceTaintEffect = "None" + // Do not allow new pods to schedule which use a tainted device unless they tolerate the taint, // but allow all pods submitted to Kubelet without going through the scheduler // to start, and allow all already-running pods to continue running. diff --git a/staging/src/k8s.io/api/resource/v1alpha3/types.go b/staging/src/k8s.io/api/resource/v1alpha3/types.go index da9a9ca286b..3ea3e7bafd5 100644 --- a/staging/src/k8s.io/api/resource/v1alpha3/types.go +++ b/staging/src/k8s.io/api/resource/v1alpha3/types.go @@ -134,8 +134,10 @@ type DeviceTaint struct { // The effect of the taint on claims that do not tolerate the taint // and through such claims on the pods using them. - // Valid effects are NoSchedule and NoExecute. PreferNoSchedule as used for - // nodes is not valid here. + // + // Valid effects are None, NoSchedule and NoExecute. PreferNoSchedule as used for + // nodes is not valid here. More effects may get added in the future. + // Consumers must treat unknown effects like None. // // +required Effect DeviceTaintEffect `json:"effect" protobuf:"bytes,3,name=effect,casttype=DeviceTaintEffect"` @@ -144,6 +146,14 @@ type DeviceTaint struct { // // Implementing PreferNoSchedule would depend on a scoring solution for DRA. // It might get added as part of that. + // + // A possible future new effect is NoExecuteWithPodDisruptionBudget: + // honor the pod disruption budget instead of simply deleting pods. + // This is currently undecided, it could also be a separate field. + // + // Validation must be prepared to allow unknown enums in stored objects, + // which will enable adding new enums within a single release without + // ratcheting. // TimeAdded represents the time at which the taint was added. // Added automatically during create or update if not set. @@ -162,6 +172,9 @@ type DeviceTaint struct { type DeviceTaintEffect string const ( + // No effect, the taint is purely informational. + DeviceTaintEffectNone DeviceTaintEffect = "None" + // Do not allow new pods to schedule which use a tainted device unless they tolerate the taint, // but allow all pods submitted to Kubelet without going through the scheduler // to start, and allow all already-running pods to continue running. @@ -190,18 +203,16 @@ type DeviceTaintRule struct { // Changing the spec automatically increments the metadata.generation number. Spec DeviceTaintRuleSpec `json:"spec" protobuf:"bytes,2,name=spec"` - // ^^^ - // A spec gets added because adding a status seems likely. - // Such a status could provide feedback on applying the - // eviction and/or statistics (number of matching devices, - // affected allocated claims, pods remaining to be evicted, - // etc.). + // Status provides information about what was requested in the spec. + // + // +optional + Status DeviceTaintRuleStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"` } // DeviceTaintRuleSpec specifies the selector and one taint. type DeviceTaintRuleSpec struct { // DeviceSelector defines which device(s) the taint is applied to. - // All selector criteria must be satified for a device to + // All selector criteria must be satisfied for a device to // match. The empty selector matches all devices. Without // a selector, no devices are matches. // @@ -261,6 +272,43 @@ type DeviceTaintSelector struct { Selectors []DeviceSelector `json:"selectors,omitempty" protobuf:"bytes,5,rep,name=selectors"` } +// DeviceTaintRuleStatus provides information about an on-going pod eviction. +type DeviceTaintRuleStatus struct { + // Conditions provide information about the state of the DeviceTaintRule + // and the cluster at some point in time, + // in a machine-readable and human-readable format. + // + // The following condition is currently defined as part of this API, more may + // get added: + // - Type: EvictionInProgress + // - Status: True if there are currently pods which need to be evicted, False otherwise + // (includes the effects which don't cause eviction). + // - Reason: not specified, may change + // - Message: includes information about number of pending pods and already evicted pods + // in a human-readable format, updated periodically, may change + // + // For `effect: None`, the condition above gets set once for each change to + // the spec, with the message containing information about what would happen + // if the effect was `NoExecute`. This feedback can be used to decide whether + // changing the effect to `NoExecute` will work as intended. It only gets + // set once to avoid having to constantly update the status. + // + // Must have 8 or fewer entries. + // + // +optional + // +listType=map + // +listMapKey=type + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` +} + +// DeviceTaintRuleStatusMaxConditions is the maximum number of conditions in DeviceTaintRuleStatus. +const DeviceTaintRuleStatusMaxConditions = 8 + +// DeviceTaintConditionEvictionInProgress is the publicly documented condition type for the DeviceTaintRuleStatus. +const DeviceTaintConditionEvictionInProgress = "EvictionInProgress" + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +k8s:prerelease-lifecycle-gen:introduced=1.33 diff --git a/staging/src/k8s.io/api/resource/v1beta1/types.go b/staging/src/k8s.io/api/resource/v1beta1/types.go index e63de0d1bcc..358923fb3be 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/types.go +++ b/staging/src/k8s.io/api/resource/v1beta1/types.go @@ -149,7 +149,7 @@ type ResourceSliceSpec struct { // Devices lists some or all of the devices in this pool. // - // Must not have more than 128 entries. + // Must not have more than 128 entries. If any device uses taints the limit is 64. // // +optional // +listType=atomic @@ -258,6 +258,7 @@ type ResourcePool struct { const ResourceSliceMaxSharedCapacity = 128 const ResourceSliceMaxDevices = 128 +const ResourceSliceMaxDevicesWithTaints = 64 const PoolNameMaxLength = validation.DNS1123SubdomainMaxLength // Same as for a single node name. const BindingConditionsMaxSize = 4 const BindingFailureConditionsMaxSize = 4 @@ -345,7 +346,9 @@ type BasicDevice struct { // If specified, these are the driver-defined taints. // - // The maximum number of taints is 4. + // The maximum number of taints is 16. If taints are set for + // any device in a ResourceSlice, then the maximum number of + // allowed devices per ResourceSlice is 64 instead of 128. // // This is an alpha field and requires enabling the DRADeviceTaints // feature gate. @@ -622,8 +625,8 @@ type DeviceAttribute struct { // DeviceAttributeMaxValueLength is the maximum length of a string or version attribute value. const DeviceAttributeMaxValueLength = 64 -// DeviceTaintsMaxLength is the maximum number of taints per device. -const DeviceTaintsMaxLength = 4 +// DeviceTaintsMaxLength is the maximum number of taints per Device. +const DeviceTaintsMaxLength = 16 // The device this taint is attached to has the "effect" on // any claim which does not tolerate the taint and, through the claim, @@ -645,8 +648,10 @@ type DeviceTaint struct { // The effect of the taint on claims that do not tolerate the taint // and through such claims on the pods using them. - // Valid effects are NoSchedule and NoExecute. PreferNoSchedule as used for - // nodes is not valid here. + // + // Valid effects are None, NoSchedule and NoExecute. PreferNoSchedule as used for + // nodes is not valid here. More effects may get added in the future. + // Consumers must treat unknown effects like None. // // +required // +k8s:required @@ -656,6 +661,14 @@ type DeviceTaint struct { // // Implementing PreferNoSchedule would depend on a scoring solution for DRA. // It might get added as part of that. + // + // A possible future new effect is NoExecuteWithPodDisruptionBudget: + // honor the pod disruption budget instead of simply deleting pods. + // This is currently undecided, it could also be a separate field. + // + // Validation must be prepared to allow unknown enums in stored objects, + // which will enable adding new enums within a single release without + // ratcheting. // TimeAdded represents the time at which the taint was added. // Added automatically during create or update if not set. @@ -675,6 +688,9 @@ type DeviceTaint struct { type DeviceTaintEffect string const ( + // No effect, the taint is purely informational. + DeviceTaintEffectNone DeviceTaintEffect = "None" + // Do not allow new pods to schedule which use a tainted device unless they tolerate the taint, // but allow all pods submitted to Kubelet without going through the scheduler // to start, and allow all already-running pods to continue running. diff --git a/staging/src/k8s.io/api/resource/v1beta2/types.go b/staging/src/k8s.io/api/resource/v1beta2/types.go index c3e06f3edb5..65934ced465 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/types.go +++ b/staging/src/k8s.io/api/resource/v1beta2/types.go @@ -149,7 +149,7 @@ type ResourceSliceSpec struct { // Devices lists some or all of the devices in this pool. // - // Must not have more than 128 entries. + // Must not have more than 128 entries. If any device uses taints the limit is 64. // // +optional // +listType=atomic @@ -250,6 +250,7 @@ type ResourcePool struct { const ResourceSliceMaxSharedCapacity = 128 const ResourceSliceMaxDevices = 128 +const ResourceSliceMaxDevicesWithTaints = 64 const PoolNameMaxLength = validation.DNS1123SubdomainMaxLength // Same as for a single node name. const BindingConditionsMaxSize = 4 const BindingFailureConditionsMaxSize = 4 @@ -333,7 +334,9 @@ type Device struct { // If specified, these are the driver-defined taints. // - // The maximum number of taints is 4. + // The maximum number of taints is 16. If taints are set for + // any device in a ResourceSlice, then the maximum number of + // allowed devices per ResourceSlice is 64 instead of 128. // // This is an alpha field and requires enabling the DRADeviceTaints // feature gate. @@ -618,8 +621,8 @@ type DeviceAttribute struct { // DeviceAttributeMaxValueLength is the maximum length of a string or version attribute value. const DeviceAttributeMaxValueLength = 64 -// DeviceTaintsMaxLength is the maximum number of taints per device. -const DeviceTaintsMaxLength = 4 +// DeviceTaintsMaxLength is the maximum number of taints per Device. +const DeviceTaintsMaxLength = 16 // The device this taint is attached to has the "effect" on // any claim which does not tolerate the taint and, through the claim, @@ -641,8 +644,10 @@ type DeviceTaint struct { // The effect of the taint on claims that do not tolerate the taint // and through such claims on the pods using them. - // Valid effects are NoSchedule and NoExecute. PreferNoSchedule as used for - // nodes is not valid here. + // + // Valid effects are None, NoSchedule and NoExecute. PreferNoSchedule as used for + // nodes is not valid here. More effects may get added in the future. + // Consumers must treat unknown effects like None. // // +required // +k8s:required @@ -652,6 +657,14 @@ type DeviceTaint struct { // // Implementing PreferNoSchedule would depend on a scoring solution for DRA. // It might get added as part of that. + // + // A possible future new effect is NoExecuteWithPodDisruptionBudget: + // honor the pod disruption budget instead of simply deleting pods. + // This is currently undecided, it could also be a separate field. + // + // Validation must be prepared to allow unknown enums in stored objects, + // which will enable adding new enums within a single release without + // ratcheting. // TimeAdded represents the time at which the taint was added. // Added automatically during create or update if not set. @@ -671,6 +684,9 @@ type DeviceTaint struct { type DeviceTaintEffect string const ( + // No effect, the taint is purely informational. + DeviceTaintEffectNone DeviceTaintEffect = "None" + // Do not allow new pods to schedule which use a tainted device unless they tolerate the taint, // but allow all pods submitted to Kubelet without going through the scheduler // to start, and allow all already-running pods to continue running. diff --git a/staging/src/k8s.io/client-go/applyconfigurations/utils.go b/staging/src/k8s.io/client-go/applyconfigurations/utils.go index aafb1ac1946..5c3df50c7c4 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/utils.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/utils.go @@ -1720,6 +1720,8 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &resourcev1alpha3.DeviceTaintRuleApplyConfiguration{} case v1alpha3.SchemeGroupVersion.WithKind("DeviceTaintRuleSpec"): return &resourcev1alpha3.DeviceTaintRuleSpecApplyConfiguration{} + case v1alpha3.SchemeGroupVersion.WithKind("DeviceTaintRuleStatus"): + return &resourcev1alpha3.DeviceTaintRuleStatusApplyConfiguration{} case v1alpha3.SchemeGroupVersion.WithKind("DeviceTaintSelector"): return &resourcev1alpha3.DeviceTaintSelectorApplyConfiguration{} diff --git a/test/integration/apiserver/apply/status_test.go b/test/integration/apiserver/apply/status_test.go index 15aee66dafa..ba0e11337c9 100644 --- a/test/integration/apiserver/apply/status_test.go +++ b/test/integration/apiserver/apply/status_test.go @@ -58,6 +58,7 @@ var statusData = map[schema.GroupVersionResource]string{ gvr("storage.k8s.io", "v1", "volumeattachments"): `{"status": {"attached": true}}`, gvr("policy", "v1", "poddisruptionbudgets"): `{"status": {"currentHealthy": 5}}`, gvr("policy", "v1beta1", "poddisruptionbudgets"): `{"status": {"currentHealthy": 5}}`, + gvr("resource.k8s.io", "v1alpha3", "devicetaintrules"): `{"status": {"conditions": [{"type": "EvictionInProgress", "status": "True", "reason: "PodsLeft", "message: "100 pods left", "lastTransitionTime": "2020-01-01T00:00:00Z"}]}}`, gvr("resource.k8s.io", "v1beta1", "resourceclaims"): `{"status": {"allocation": {"nodeSelector": {"nodeSelectorTerms": [{"matchExpressions": [{"key": "some-label", "operator": "In", "values": ["some-value"]}] }]}}}}`, gvr("resource.k8s.io", "v1beta2", "resourceclaims"): `{"status": {"allocation": {"nodeSelector": {"nodeSelectorTerms": [{"matchExpressions": [{"key": "some-label", "operator": "In", "values": ["some-value"]}] }]}}}}`, gvr("resource.k8s.io", "v1", "resourceclaims"): `{"status": {"allocation": {"nodeSelector": {"nodeSelectorTerms": [{"matchExpressions": [{"key": "some-label", "operator": "In", "values": ["some-value"]}] }]}}}}`, diff --git a/test/integration/dra/dra_test.go b/test/integration/dra/dra_test.go index b788332e73a..4f13474019b 100644 --- a/test/integration/dra/dra_test.go +++ b/test/integration/dra/dra_test.go @@ -1142,25 +1142,28 @@ func testResourceClaimDeviceStatus(tCtx ktesting.TContext, enabled bool) { require.Equal(tCtx, deviceStatus, claim.Status.Devices, "after removing device status three") } -// testMaxResourceSlice creates a ResourceSlice that is as large as possible +// testMaxResourceSlice creates ResourceSlices that are as large as possible // and prints some information about it. func testMaxResourceSlice(tCtx ktesting.TContext) { - slice := NewMaxResourceSlice() - createdSlice := createSlice(tCtx, slice) - totalSize := createdSlice.Size() - var managedFieldsSize int - for _, f := range createdSlice.ManagedFields { - managedFieldsSize += f.Size() - } - specSize := createdSlice.Spec.Size() - tCtx.Logf("\n\nTotal size: %s\nManagedFields size: %s (%.0f%%)\nSpec size: %s (%.0f)%%\n\nManagedFields:\n%s", - resource.NewQuantity(int64(totalSize), resource.BinarySI), - resource.NewQuantity(int64(managedFieldsSize), resource.BinarySI), float64(managedFieldsSize)*100/float64(totalSize), - resource.NewQuantity(int64(specSize), resource.BinarySI), float64(specSize)*100/float64(totalSize), - klog.Format(createdSlice.ManagedFields), - ) - if diff := cmp.Diff(slice.Spec, createdSlice.Spec); diff != "" { - tCtx.Errorf("ResourceSliceSpec got modified during Create (- want, + got):\n%s", diff) + for name, slice := range NewMaxResourceSlices() { + tCtx.Run(name, func(tCtx ktesting.TContext) { + createdSlice := createSlice(tCtx, slice) + totalSize := createdSlice.Size() + var managedFieldsSize int + for _, f := range createdSlice.ManagedFields { + managedFieldsSize += f.Size() + } + specSize := createdSlice.Spec.Size() + tCtx.Logf("\n\nTotal size: %s\nManagedFields size: %s (%.0f%%)\nSpec size: %s (%.0f)%%\n\nManagedFields:\n%s", + resource.NewQuantity(int64(totalSize), resource.BinarySI), + resource.NewQuantity(int64(managedFieldsSize), resource.BinarySI), float64(managedFieldsSize)*100/float64(totalSize), + resource.NewQuantity(int64(specSize), resource.BinarySI), float64(specSize)*100/float64(totalSize), + klog.Format(createdSlice.ManagedFields), + ) + if diff := cmp.Diff(slice.Spec, createdSlice.Spec); diff != "" { + tCtx.Errorf("ResourceSliceSpec got modified during Create (- want, + got):\n%s", diff) + } + }) } } diff --git a/test/integration/dra/objects.go b/test/integration/dra/objects.go index aef0001e14c..63de5aa5c20 100644 --- a/test/integration/dra/objects.go +++ b/test/integration/dra/objects.go @@ -29,8 +29,33 @@ import ( "k8s.io/utils/ptr" ) -// NewMaxResourceSlice creates a slice that is as large as possible given the current validation constraints. -func NewMaxResourceSlice() *resourceapi.ResourceSlice { +// NewMaxResourceSlices creates slices that are as large as possible given the current validation constraints. +func NewMaxResourceSlices() map[string]*resourceapi.ResourceSlice { + slices := map[string]*resourceapi.ResourceSlice{ + "basic": newBasicResourceSlice(resourceapi.ResourceSliceMaxDevices), + "with-taints": newTaintedResourceSlice(), + } + return slices +} + +func newTaintedResourceSlice() *resourceapi.ResourceSlice { + slice := newBasicResourceSlice(resourceapi.ResourceSliceMaxDevicesWithTaints) + for i := range slice.Spec.Devices { + for j := 0; j < resourceapi.DeviceTaintsMaxLength; j++ { + slice.Spec.Devices[i].Taints = append(slice.Spec.Devices[i].Taints, + resourceapi.DeviceTaint{ + Key: maxLabelName(i), + Value: maxLabelValue(i), + Effect: resourceapi.DeviceTaintEffectNoSchedule, + TimeAdded: &metav1.Time{Time: time.Now().Truncate(time.Second)}, + }, + ) + } + } + return slice +} + +func newBasicResourceSlice(numDevices int) *resourceapi.ResourceSlice { slice := &resourceapi.ResourceSlice{ ObjectMeta: metav1.ObjectMeta{ Name: maxSubDomain(0), @@ -70,7 +95,7 @@ func NewMaxResourceSlice() *resourceapi.ResourceSlice { }(), Devices: func() []resourceapi.Device { var devices []resourceapi.Device - for i := 0; i < resourceapi.ResourceSliceMaxDevices; i++ { + for i := 0; i < numDevices; i++ { devices = append(devices, resourceapi.Device{ Name: maxDNSLabel(i), // Use attributes rather than capacity since it is more expensive. @@ -98,18 +123,6 @@ func NewMaxResourceSlice() *resourceapi.ResourceSlice { return consumesCounters }(), NodeName: ptr.To(maxSubDomain(0)), - Taints: func() []resourceapi.DeviceTaint { - var taints []resourceapi.DeviceTaint - for i := 0; i < resourceapi.DeviceTaintsMaxLength; i++ { - taints = append(taints, resourceapi.DeviceTaint{ - Key: maxLabelName(i), - Value: maxLabelValue(i), - Effect: resourceapi.DeviceTaintEffectNoSchedule, - TimeAdded: &metav1.Time{Time: time.Now().Truncate(time.Second)}, - }) - } - return taints - }(), }) } return devices