From fee14ffca2099b2d7a0bfc5761691b363eac4de1 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 18 Sep 2025 19:44:06 +0200 Subject: [PATCH] DRA API: device taints 1.35 This raises the number of allowed taints per device to 16 by lowering the number of allowed devices to 64 per ResourceSlice if (and only if!) taints are used. "effect: None" and DeviceTaintRule status with conditions get added to support giving feedback to admins. Instead of merely adding the new effect value, this also changes validation of the enum so that unknown values are valid if they were already stored. This will simplify adding new effects in the future because validation won't fail for them after a downgrade. Consumers must treat them like this new None effect, i.e. ignore them. --- pkg/apis/resource/types.go | 75 +++++- pkg/apis/resource/validation/validation.go | 83 +++++- .../validation_devicetaintrule_test.go | 35 ++- .../validation_resourceclaim_test.go | 2 +- .../validation_resourceslice_test.go | 59 ++++- .../devicetaintrule/storage/storage.go | 48 +++- .../devicetaintrule/storage/storage_test.go | 59 ++++- .../resource/devicetaintrule/strategy.go | 92 +++++-- .../resource/devicetaintrule/strategy_test.go | 250 +++++++++++++++--- .../resource/resourceclaim/strategy.go | 4 + .../resource/rest/storage_resource.go | 3 +- staging/src/k8s.io/api/resource/v1/types.go | 28 +- .../src/k8s.io/api/resource/v1alpha3/types.go | 66 ++++- .../src/k8s.io/api/resource/v1beta1/types.go | 28 +- .../src/k8s.io/api/resource/v1beta2/types.go | 28 +- .../client-go/applyconfigurations/utils.go | 2 + .../apiserver/apply/status_test.go | 1 + test/integration/dra/dra_test.go | 37 +-- test/integration/dra/objects.go | 43 +-- 19 files changed, 793 insertions(+), 150 deletions(-) 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