diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 423f5525b23..542782e7890 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -16564,7 +16564,7 @@ "additionalProperties": { "$ref": "#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim's status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", "type": "object" } }, @@ -17079,7 +17079,7 @@ "additionalProperties": { "$ref": "#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", "type": "object" }, "device": { @@ -18450,7 +18450,7 @@ "additionalProperties": { "$ref": "#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", "type": "object" } }, @@ -18965,7 +18965,7 @@ "additionalProperties": { "$ref": "#/definitions/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", "type": "object" }, "device": { diff --git a/api/openapi-spec/v3/apis__resource.k8s.io__v1_openapi.json b/api/openapi-spec/v3/apis__resource.k8s.io__v1_openapi.json index b6d9556a68e..6a638d3c391 100644 --- a/api/openapi-spec/v3/apis__resource.k8s.io__v1_openapi.json +++ b/api/openapi-spec/v3/apis__resource.k8s.io__v1_openapi.json @@ -242,7 +242,7 @@ "additionalProperties": { "$ref": "#/components/schemas/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim's status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", "type": "object" } }, @@ -811,7 +811,7 @@ "additionalProperties": { "$ref": "#/components/schemas/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", "type": "object" }, "device": { diff --git a/api/openapi-spec/v3/apis__resource.k8s.io__v1beta2_openapi.json b/api/openapi-spec/v3/apis__resource.k8s.io__v1beta2_openapi.json index d7573dc857c..555d29e917d 100644 --- a/api/openapi-spec/v3/apis__resource.k8s.io__v1beta2_openapi.json +++ b/api/openapi-spec/v3/apis__resource.k8s.io__v1beta2_openapi.json @@ -242,7 +242,7 @@ "additionalProperties": { "$ref": "#/components/schemas/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + "description": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", "type": "object" } }, @@ -811,7 +811,7 @@ "additionalProperties": { "$ref": "#/components/schemas/io.k8s.apimachinery.pkg.api.resource.Quantity" }, - "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + "description": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", "type": "object" }, "device": { diff --git a/pkg/apis/resource/types.go b/pkg/apis/resource/types.go index e992d68d56e..2b557a15ba5 100644 --- a/pkg/apis/resource/types.go +++ b/pkg/apis/resource/types.go @@ -1015,6 +1015,9 @@ const ( DeviceConstraintsMaxSize = 32 DeviceConfigMaxSize = 32 DeviceDerivedAttributesMaxSize = 32 + // MaxCapacityRequirements is the maximum number of entries allowed in + // CapacityRequirements.Requests. + MaxCapacityRequirements = 32 ) // DRAAdminNamespaceLabelKey is a label key used to grant administrative access @@ -1381,7 +1384,9 @@ type CapacityRequirements struct { // If the device allows multiple allocation, // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, - // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // is recorded in the resource claim's status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. // // +optional Requests map[QualifiedName]resource.Quantity @@ -1992,6 +1997,8 @@ type DeviceRequestAllocationResult struct { // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. + // // +optional // +featureGate=DRAConsumableCapacity ConsumedCapacity map[QualifiedName]resource.Quantity diff --git a/pkg/apis/resource/v1/zz_generated.validations.go b/pkg/apis/resource/v1/zz_generated.validations.go index 8a9c7bd49c4..b2463c76ea9 100644 --- a/pkg/apis/resource/v1/zz_generated.validations.go +++ b/pkg/apis/resource/v1/zz_generated.validations.go @@ -284,6 +284,53 @@ func Validate_AllocationResult( return errs } +// Validate_CapacityRequirements validates an instance of CapacityRequirements according +// to declarative validation rules in the API schema. +func Validate_CapacityRequirements( + ctx context.Context, op operation.Operation, fldPath *field.Path, + obj, oldObj *resourcev1.CapacityRequirements) (errs field.ErrorList) { + + { // field resourcev1.CapacityRequirements.Requests + fn := func( + fldPath *field.Path, + obj, oldObj map[resourcev1.QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1.CapacityRequirements) map[resourcev1.QualifiedName]resource.Quantity { + return oldObj.Requests + }) + errs = append(errs, fn(fldPath.Child("requests"), obj.Requests, oldVal, oldObj != nil)...) + } + + return errs +} + // Validate_CounterSet validates an instance of CounterSet according // to declarative validation rules in the API schema. func Validate_CounterSet( @@ -2086,7 +2133,43 @@ func Validate_DeviceRequestAllocationResult( errs = append(errs, fn(fldPath.Child("shareID"), obj.ShareID, oldVal, oldObj != nil)...) } - // field resourcev1.DeviceRequestAllocationResult.ConsumedCapacity has no validation + { // field resourcev1.DeviceRequestAllocationResult.ConsumedCapacity + fn := func( + fldPath *field.Path, + obj, oldObj map[resourcev1.QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1.DeviceRequestAllocationResult) map[resourcev1.QualifiedName]resource.Quantity { + return oldObj.ConsumedCapacity + }) + errs = append(errs, fn(fldPath.Child("consumedCapacity"), obj.ConsumedCapacity, oldVal, oldObj != nil)...) + } { // field resourcev1.DeviceRequestAllocationResult.SkipNodeOperations fn := func( @@ -2263,7 +2346,35 @@ func Validate_DeviceSubRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field resourcev1.DeviceSubRequest.Capacity has no validation + { // field resourcev1.DeviceSubRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *resourcev1.CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1.DeviceSubRequest) *resourcev1.CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field resourcev1.DeviceSubRequest.DerivedAttributes fn := func( @@ -2745,7 +2856,35 @@ func Validate_ExactDeviceRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field resourcev1.ExactDeviceRequest.Capacity has no validation + { // field resourcev1.ExactDeviceRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *resourcev1.CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1.ExactDeviceRequest) *resourcev1.CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field resourcev1.ExactDeviceRequest.DerivedAttributes fn := func( diff --git a/pkg/apis/resource/v1beta1/zz_generated.validations.go b/pkg/apis/resource/v1beta1/zz_generated.validations.go index ca928e4df6e..56d735edd10 100644 --- a/pkg/apis/resource/v1beta1/zz_generated.validations.go +++ b/pkg/apis/resource/v1beta1/zz_generated.validations.go @@ -494,6 +494,53 @@ func Validate_BasicDevice( return errs } +// Validate_CapacityRequirements validates an instance of CapacityRequirements according +// to declarative validation rules in the API schema. +func Validate_CapacityRequirements( + ctx context.Context, op operation.Operation, fldPath *field.Path, + obj, oldObj *resourcev1beta1.CapacityRequirements) (errs field.ErrorList) { + + { // field resourcev1beta1.CapacityRequirements.Requests + fn := func( + fldPath *field.Path, + obj, oldObj map[resourcev1beta1.QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta1.CapacityRequirements) map[resourcev1beta1.QualifiedName]resource.Quantity { + return oldObj.Requests + }) + errs = append(errs, fn(fldPath.Child("requests"), obj.Requests, oldVal, oldObj != nil)...) + } + + return errs +} + // Validate_CounterSet validates an instance of CounterSet according // to declarative validation rules in the API schema. func Validate_CounterSet( @@ -1974,7 +2021,35 @@ func Validate_DeviceRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field resourcev1beta1.DeviceRequest.Capacity has no validation + { // field resourcev1beta1.DeviceRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *resourcev1beta1.CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta1.DeviceRequest) *resourcev1beta1.CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field resourcev1beta1.DeviceRequest.DerivedAttributes fn := func( @@ -2220,7 +2295,43 @@ func Validate_DeviceRequestAllocationResult( errs = append(errs, fn(fldPath.Child("shareID"), obj.ShareID, oldVal, oldObj != nil)...) } - // field resourcev1beta1.DeviceRequestAllocationResult.ConsumedCapacity has no validation + { // field resourcev1beta1.DeviceRequestAllocationResult.ConsumedCapacity + fn := func( + fldPath *field.Path, + obj, oldObj map[resourcev1beta1.QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta1.DeviceRequestAllocationResult) map[resourcev1beta1.QualifiedName]resource.Quantity { + return oldObj.ConsumedCapacity + }) + errs = append(errs, fn(fldPath.Child("consumedCapacity"), obj.ConsumedCapacity, oldVal, oldObj != nil)...) + } { // field resourcev1beta1.DeviceRequestAllocationResult.SkipNodeOperations fn := func( @@ -2397,7 +2508,35 @@ func Validate_DeviceSubRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field resourcev1beta1.DeviceSubRequest.Capacity has no validation + { // field resourcev1beta1.DeviceSubRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *resourcev1beta1.CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta1.DeviceSubRequest) *resourcev1beta1.CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field resourcev1beta1.DeviceSubRequest.DerivedAttributes fn := func( diff --git a/pkg/apis/resource/v1beta2/zz_generated.validations.go b/pkg/apis/resource/v1beta2/zz_generated.validations.go index fb1bac6e984..da87f1f998e 100644 --- a/pkg/apis/resource/v1beta2/zz_generated.validations.go +++ b/pkg/apis/resource/v1beta2/zz_generated.validations.go @@ -284,6 +284,53 @@ func Validate_AllocationResult( return errs } +// Validate_CapacityRequirements validates an instance of CapacityRequirements according +// to declarative validation rules in the API schema. +func Validate_CapacityRequirements( + ctx context.Context, op operation.Operation, fldPath *field.Path, + obj, oldObj *resourcev1beta2.CapacityRequirements) (errs field.ErrorList) { + + { // field resourcev1beta2.CapacityRequirements.Requests + fn := func( + fldPath *field.Path, + obj, oldObj map[resourcev1beta2.QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta2.CapacityRequirements) map[resourcev1beta2.QualifiedName]resource.Quantity { + return oldObj.Requests + }) + errs = append(errs, fn(fldPath.Child("requests"), obj.Requests, oldVal, oldObj != nil)...) + } + + return errs +} + // Validate_CounterSet validates an instance of CounterSet according // to declarative validation rules in the API schema. func Validate_CounterSet( @@ -2090,7 +2137,43 @@ func Validate_DeviceRequestAllocationResult( errs = append(errs, fn(fldPath.Child("shareID"), obj.ShareID, oldVal, oldObj != nil)...) } - // field resourcev1beta2.DeviceRequestAllocationResult.ConsumedCapacity has no validation + { // field resourcev1beta2.DeviceRequestAllocationResult.ConsumedCapacity + fn := func( + fldPath *field.Path, + obj, oldObj map[resourcev1beta2.QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta2.DeviceRequestAllocationResult) map[resourcev1beta2.QualifiedName]resource.Quantity { + return oldObj.ConsumedCapacity + }) + errs = append(errs, fn(fldPath.Child("consumedCapacity"), obj.ConsumedCapacity, oldVal, oldObj != nil)...) + } { // field resourcev1beta2.DeviceRequestAllocationResult.SkipNodeOperations fn := func( @@ -2267,7 +2350,35 @@ func Validate_DeviceSubRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field resourcev1beta2.DeviceSubRequest.Capacity has no validation + { // field resourcev1beta2.DeviceSubRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *resourcev1beta2.CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta2.DeviceSubRequest) *resourcev1beta2.CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field resourcev1beta2.DeviceSubRequest.DerivedAttributes fn := func( @@ -2749,7 +2860,35 @@ func Validate_ExactDeviceRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field resourcev1beta2.ExactDeviceRequest.Capacity has no validation + { // field resourcev1beta2.ExactDeviceRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *resourcev1beta2.CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *resourcev1beta2.ExactDeviceRequest) *resourcev1beta2.CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field resourcev1beta2.ExactDeviceRequest.DerivedAttributes fn := func( diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 65bdfdead20..8ad6672cb95 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -61,12 +61,12 @@ import ( var ResourceNormalizationRules = []field.NormalizationRule{ { // ResourceClaim - Regexp: regexp.MustCompile(`spec\.devices\.requests\[(\d+)\]\.(deviceClassName|selectors|allocationMode|count|adminAccess|tolerations|derivedAttributes)`), + Regexp: regexp.MustCompile(`spec\.devices\.requests\[(\d+)\]\.(deviceClassName|selectors|allocationMode|count|adminAccess|tolerations|derivedAttributes|capacity)`), Replacement: "spec.devices.requests[$1].exactly.$2", }, { // ResourceClaimTemplate (RCT.Spec.Spec adds an extra "spec." prefix) - Regexp: regexp.MustCompile(`spec\.spec\.devices\.requests\[(\d+)\]\.(deviceClassName|selectors|allocationMode|count|adminAccess|tolerations|derivedAttributes)`), + Regexp: regexp.MustCompile(`spec\.spec\.devices\.requests\[(\d+)\]\.(deviceClassName|selectors|allocationMode|count|adminAccess|tolerations|derivedAttributes|capacity)`), Replacement: "spec.spec.devices.requests[$1].exactly.$2", }, { @@ -118,7 +118,7 @@ func validateUID(uid string, fldPath *field.Path) field.ErrorList { // ValidateResourceClaim validates a ResourceClaim. func ValidateResourceClaim(resourceClaim *resource.ResourceClaim) field.ErrorList { allErrs := corevalidation.ValidateObjectMeta(&resourceClaim.ObjectMeta, true, corevalidation.ValidateResourceClaimName, field.NewPath("metadata")) - allErrs = append(allErrs, validateResourceClaimSpec(&resourceClaim.Spec, field.NewPath("spec"), false)...) + allErrs = append(allErrs, validateResourceClaimSpec(&resourceClaim.Spec, nil, field.NewPath("spec"), false)...) return allErrs } @@ -141,9 +141,12 @@ func ValidateResourceClaimStatusUpdate(resourceClaim, oldClaim *resource.Resourc return allErrs } -func validateResourceClaimSpec(spec *resource.ResourceClaimSpec, fldPath *field.Path, stored bool) field.ErrorList { +// validateResourceClaimSpec gets oldSpec only when a ResourceClaimTemplate is updated. +// Checks which were added after the spec could be stored must accept unchanged old +// values, because the spec is immutable and the user cannot fix them. +func validateResourceClaimSpec(spec, oldSpec *resource.ResourceClaimSpec, fldPath *field.Path, stored bool) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validateDeviceClaim(&spec.Devices, fldPath.Child("devices"), stored)...) + allErrs = append(allErrs, validateDeviceClaim(&spec.Devices, oldSpec, fldPath.Child("devices"), stored)...) return allErrs } @@ -204,7 +207,7 @@ func gatherConstraintAttributes(constraints []resource.DeviceConstraint) map[str return attrs } -func validateDeviceClaim(deviceClaim *resource.DeviceClaim, fldPath *field.Path, stored bool) field.ErrorList { +func validateDeviceClaim(deviceClaim *resource.DeviceClaim, oldSpec *resource.ResourceClaimSpec, fldPath *field.Path, stored bool) field.ErrorList { allErrs := field.ErrorList{} requestNames := gatherRequestNames(deviceClaim) @@ -216,7 +219,7 @@ func validateDeviceClaim(deviceClaim *resource.DeviceClaim, fldPath *field.Path, } allErrs = append(allErrs, validateSet(deviceClaim.Requests, resource.DeviceRequestsMaxSize, func(request resource.DeviceRequest, fldPath *field.Path) field.ErrorList { - return validateDeviceRequest(request, fldPath, opts) + return validateDeviceRequest(request, oldDeviceRequest(oldSpec, request.Name), fldPath, opts) }, func(request resource.DeviceRequest) string { return request.Name @@ -237,6 +240,37 @@ func validateDeviceClaim(deviceClaim *resource.DeviceClaim, fldPath *field.Path, return allErrs } +func oldDeviceRequest(oldSpec *resource.ResourceClaimSpec, name string) *resource.DeviceRequest { + if oldSpec == nil { + return nil + } + for i := range oldSpec.Devices.Requests { + if oldSpec.Devices.Requests[i].Name == name { + return &oldSpec.Devices.Requests[i] + } + } + return nil +} + +func oldExactCapacity(oldRequest *resource.DeviceRequest) *resource.CapacityRequirements { + if oldRequest == nil || oldRequest.Exactly == nil { + return nil + } + return oldRequest.Exactly.Capacity +} + +func oldSubRequestCapacity(oldRequest *resource.DeviceRequest, name string) *resource.CapacityRequirements { + if oldRequest == nil { + return nil + } + for i := range oldRequest.FirstAvailable { + if oldRequest.FirstAvailable[i].Name == name { + return oldRequest.FirstAvailable[i].Capacity + } + } + return nil +} + type requestNames map[string]sets.Set[string] func (r requestNames) Has(s string) bool { @@ -287,7 +321,7 @@ func gatherAllocatedDevices(allocationResult *resource.DeviceAllocationResult) s return allocatedDevices } -func validateDeviceRequest(request resource.DeviceRequest, fldPath *field.Path, opts deviceValidationOptions) field.ErrorList { +func validateDeviceRequest(request resource.DeviceRequest, oldRequest *resource.DeviceRequest, fldPath *field.Path, opts deviceValidationOptions) field.ErrorList { allErrs := validateRequestName(request.Name, fldPath.Child("name")) numDeviceRequestType := 0 @@ -309,19 +343,19 @@ func validateDeviceRequest(request resource.DeviceRequest, fldPath *field.Path, case hasFirstAvailable: allErrs = append(allErrs, validateSet(request.FirstAvailable, resource.FirstAvailableDeviceRequestMaxSize, func(subRequest resource.DeviceSubRequest, fldPath *field.Path) field.ErrorList { - return validateDeviceSubRequest(subRequest, fldPath, opts, request.Name) + return validateDeviceSubRequest(subRequest, oldSubRequestCapacity(oldRequest, subRequest.Name), fldPath, opts, request.Name) }, func(subRequest resource.DeviceSubRequest) string { return subRequest.Name }, fldPath.Child("firstAvailable"), sizeCovered, uniquenessCovered)...) case hasExactly: - allErrs = append(allErrs, validateExactDeviceRequest(*request.Exactly, fldPath.Child("exactly"), opts, request.Name)...) + allErrs = append(allErrs, validateExactDeviceRequest(*request.Exactly, oldExactCapacity(oldRequest), fldPath.Child("exactly"), opts, request.Name)...) } return allErrs } -func validateDeviceSubRequest(subRequest resource.DeviceSubRequest, fldPath *field.Path, opts deviceValidationOptions, parentRequestName string) field.ErrorList { +func validateDeviceSubRequest(subRequest resource.DeviceSubRequest, oldCapacity *resource.CapacityRequirements, fldPath *field.Path, opts deviceValidationOptions, parentRequestName string) field.ErrorList { allErrs := validateRequestName(subRequest.Name, fldPath.Child("name")) allErrs = append(allErrs, validateDeviceClass(subRequest.DeviceClassName, fldPath.Child("deviceClassName")).MarkCoveredByDeclarative()...) allErrs = append(allErrs, validateSelectorSlice(subRequest.Selectors, fldPath.Child("selectors"), opts)...) @@ -330,10 +364,11 @@ func validateDeviceSubRequest(subRequest resource.DeviceSubRequest, fldPath *fie for i, toleration := range subRequest.Tolerations { allErrs = append(allErrs, validateDeviceToleration(toleration, fldPath.Child("tolerations").Index(i))...) } + allErrs = append(allErrs, validateCapacityRequirements(subRequest.Capacity, oldCapacity, fldPath.Child("capacity"))...) return allErrs } -func validateExactDeviceRequest(request resource.ExactDeviceRequest, fldPath *field.Path, opts deviceValidationOptions, requestName string) field.ErrorList { +func validateExactDeviceRequest(request resource.ExactDeviceRequest, oldCapacity *resource.CapacityRequirements, fldPath *field.Path, opts deviceValidationOptions, requestName string) field.ErrorList { var allErrs field.ErrorList allErrs = append(allErrs, validateDeviceClass(request.DeviceClassName, fldPath.Child("deviceClassName"))...) allErrs = append(allErrs, validateSelectorSlice(request.Selectors, fldPath.Child("selectors"), opts)...) @@ -342,9 +377,29 @@ func validateExactDeviceRequest(request resource.ExactDeviceRequest, fldPath *fi for i, toleration := range request.Tolerations { allErrs = append(allErrs, validateDeviceToleration(toleration, fldPath.Child("tolerations").Index(i))...) } + allErrs = append(allErrs, validateCapacityRequirements(request.Capacity, oldCapacity, fldPath.Child("capacity"))...) return allErrs } +func validateCapacityRequirements(capacity, oldCapacity *resource.CapacityRequirements, fldPath *field.Path) field.ErrorList { + if capacity == nil { + return nil + } + // Capacity request names were not validated before, so stored names + // which did not change remain valid. + if oldCapacity != nil && apiequality.Semantic.DeepEqual(capacity.Requests, oldCapacity.Requests) { + return nil + } + return validateCapacityNames(capacity.Requests, fldPath.Child("requests")) +} + +// validateCapacityNames only checks the keys. The map size and the values are +// validated declaratively. +func validateCapacityNames(capacities map[resource.QualifiedName]apiresource.Quantity, fldPath *field.Path) field.ErrorList { + return validateMap(capacities, -1, attributeAndCapacityMaxKeyLength, validateQualifiedName, + func(apiresource.Quantity, *field.Path) field.ErrorList { return nil }, fldPath) +} + func validateDeviceAllocationMode(deviceAllocationMode resource.DeviceAllocationMode, count int64, allocModeFldPath, countFldPath *field.Path) field.ErrorList { var allErrs field.ErrorList switch deviceAllocationMode { @@ -670,6 +725,7 @@ func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocati allErrs = append(allErrs, validateUID(string(*result.ShareID), fldPath.Child("shareID")).MarkCoveredByDeclarative()...) } allErrs = append(allErrs, validateSkipNodeOperations(result.SkipNodeOperations, fldPath.Child("skipNodeOperations"))...) + allErrs = append(allErrs, validateCapacityNames(result.ConsumedCapacity, fldPath.Child("consumedCapacity"))...) return allErrs } @@ -757,23 +813,27 @@ func validateDeviceClassConfiguration(config resource.DeviceClassConfiguration, // ValidateResourceClaimTemplate validates a ResourceClaimTemplate. func ValidateResourceClaimTemplate(template *resource.ResourceClaimTemplate) field.ErrorList { + return validateResourceClaimTemplate(template, nil) +} + +func validateResourceClaimTemplate(template *resource.ResourceClaimTemplate, oldSpec *resource.ResourceClaimSpec) field.ErrorList { allErrs := corevalidation.ValidateObjectMeta(&template.ObjectMeta, true, corevalidation.ValidateResourceClaimTemplateName, field.NewPath("metadata")) - allErrs = append(allErrs, validateResourceClaimTemplateSpec(&template.Spec, field.NewPath("spec"), false)...) + allErrs = append(allErrs, validateResourceClaimTemplateSpec(&template.Spec, oldSpec, field.NewPath("spec"), false)...) return allErrs } -func validateResourceClaimTemplateSpec(spec *resource.ResourceClaimTemplateSpec, fldPath *field.Path, stored bool) field.ErrorList { +func validateResourceClaimTemplateSpec(spec *resource.ResourceClaimTemplateSpec, oldSpec *resource.ResourceClaimSpec, fldPath *field.Path, stored bool) field.ErrorList { allErrs := corevalidation.ValidateTemplateObjectMeta(&spec.ObjectMeta, fldPath.Child("metadata")) - allErrs = append(allErrs, validateResourceClaimSpec(&spec.Spec, fldPath.Child("spec"), stored)...) + allErrs = append(allErrs, validateResourceClaimSpec(&spec.Spec, oldSpec, fldPath.Child("spec"), stored)...) return allErrs } // ValidateResourceClaimTemplateUpdate tests if an update to template is valid. func ValidateResourceClaimTemplateUpdate(template, oldTemplate *resource.ResourceClaimTemplate) field.ErrorList { - allErrs := corevalidation.ValidateObjectMetaUpdate(&template.ObjectMeta, &oldTemplate.ObjectMeta, field.NewPath("metadata")) - // The spec is immutable. On update, we only check for immutability. - // Re-validating other fields is skipped because the user cannot change them; - // the only actionable error is for the immutability violation. + // Unlike for ResourceClaim, the spec gets validated again. Checks which were + // added later must accept the old values, so the old spec is passed down. + allErrs := validateResourceClaimTemplate(template, &oldTemplate.Spec.Spec) + allErrs = append(allErrs, corevalidation.ValidateObjectMetaUpdate(&template.ObjectMeta, &oldTemplate.ObjectMeta, field.NewPath("metadata"))...) allErrs = append(allErrs, apimachineryvalidation.ValidateImmutableField(template.Spec, oldTemplate.Spec, field.NewPath("spec"))...) return allErrs } diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index 2d4b7f82cc3..45589c99204 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -1275,6 +1275,46 @@ func TestValidateClaim(t *testing.T) { return claim }(), }, + "capacity-requests-bad-key": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("exactly", "capacity", "requests"), "1bad", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"), + }, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpec) + claim.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{ + "1bad": apiresource.MustParse("1"), + }, + } + return claim + }(), + }, + "capacity-requests-bad-key-in-subrequest": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("capacity", "requests"), "1bad", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"), + }, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpecWithFirstAvailable) + claim.Spec.Devices.Requests[0].FirstAvailable[0].Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{ + "1bad": apiresource.MustParse("1"), + }, + } + return claim + }(), + }, + "capacity-requests-valid": { + wantFailures: nil, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, validClaimSpec) + claim.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{ + "dra.example.com/cores": apiresource.MustParse("4"), + }, + } + return claim + }(), + }, } for name, scenario := range scenarios { @@ -2601,6 +2641,33 @@ func TestValidateClaimStatusUpdate(t *testing.T) { }, consumableCapacityFeatureGate: true, }, + "invalid-add-consumed-capacity-bad-key": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("consumedCapacity"), "1bad", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"), + }, + oldClaim: testClaim(goodName, goodNS, validClaimSpec), + update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { + claim.Status.Allocation = &resource.AllocationResult{ + Devices: resource.DeviceAllocationResult{ + Results: []resource.DeviceRequestAllocationResult{ + { + Request: goodName, + Driver: goodName, + Pool: goodName, + Device: goodName, + ShareID: ptr.To(goodShareID), + AdminAccess: ptr.To(false), + ConsumedCapacity: map[resource.QualifiedName]apiresource.Quantity{ + "1bad": apiresource.MustParse("1"), + }, + }, + }, + }, + } + return claim + }, + consumableCapacityFeatureGate: true, + }, "valid-add-enable-both-device-status-and-consumable-capacity-features": { oldClaim: testClaim(goodName, goodNS, validClaimSpec), update: func(claim *resource.ResourceClaim) *resource.ResourceClaim { diff --git a/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go b/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go index 7cf5b185064..346d7a8d971 100644 --- a/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go @@ -19,6 +19,7 @@ package validation import ( "testing" + apiresource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/apis/resource" @@ -210,6 +211,42 @@ func TestValidateClaimTemplate(t *testing.T) { return template }(), }, + "capacity-valid": { + wantFailures: nil, + template: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, validClaimSpec) + template.Spec.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{ + "dra.example.com/cores": apiresource.MustParse("2"), + }, + } + return template + }(), + }, + "capacity-bad-key": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("exactly", "capacity", "requests"), "1bad", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')")}, + template: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, validClaimSpec) + template.Spec.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{ + "1bad": apiresource.MustParse("1"), + }, + } + return template + }(), + }, + "capacity-bad-key-in-subrequest": { + wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("capacity", "requests"), "1bad", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')")}, + template: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, validClaimSpecWithFirstAvailable) + template.Spec.Spec.Devices.Requests[0].FirstAvailable[0].Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{ + "1bad": apiresource.MustParse("1"), + }, + } + return template + }(), + }, } for name, scenario := range scenarios { @@ -256,6 +293,51 @@ func TestValidateClaimTemplateUpdate(t *testing.T) { return template }, }, + "valid-metadata-update-with-stored-invalid-capacity-name": { + oldClaimTemplate: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, validClaimSpec) + template.Spec.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{"test-capacity": apiresource.MustParse("1")}, + } + return template + }(), + update: func(template *resource.ResourceClaimTemplate) *resource.ResourceClaimTemplate { + template.Labels = map[string]string{"foo": "bar"} + return template + }, + }, + "valid-metadata-update-with-stored-invalid-capacity-name-in-subrequest": { + oldClaimTemplate: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, validClaimSpecWithFirstAvailable) + template.Spec.Spec.Devices.Requests[0].FirstAvailable[0].Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{"test-capacity": apiresource.MustParse("1")}, + } + return template + }(), + update: func(template *resource.ResourceClaimTemplate) *resource.ResourceClaimTemplate { + template.Labels = map[string]string{"foo": "bar"} + return template + }, + }, + "invalid-update-to-invalid-capacity-name": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("exactly", "capacity", "requests"), "test-capacity", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"), + field.Invalid(field.NewPath("spec"), func() resource.ResourceClaimTemplateSpec { + spec := validClaimTemplate.Spec.DeepCopy() + spec.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{"test-capacity": apiresource.MustParse("1")}, + } + return *spec + }(), "field is immutable"), + }, + oldClaimTemplate: validClaimTemplate, + update: func(template *resource.ResourceClaimTemplate) *resource.ResourceClaimTemplate { + template.Spec.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{ + Requests: map[resource.QualifiedName]apiresource.Quantity{"test-capacity": apiresource.MustParse("1")}, + } + return template + }, + }, } for name, scenario := range scenarios { diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 0d4272c6c6f..15b449aa3f1 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -48285,7 +48285,7 @@ func schema_k8sio_api_resource_v1_CapacityRequirements(ref common.ReferenceCallb Properties: map[string]spec.Schema{ "requests": { SchemaProps: spec.SchemaProps{ - Description: "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + Description: "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim's status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", Type: []string{"object"}, AdditionalProperties: &spec.SchemaOrBool{ Allows: true, @@ -49379,7 +49379,7 @@ func schema_k8sio_api_resource_v1_DeviceRequestAllocationResult(ref common.Refer }, "consumedCapacity": { SchemaProps: spec.SchemaProps{ - Description: "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + Description: "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", Type: []string{"object"}, AdditionalProperties: &spec.SchemaOrBool{ Allows: true, @@ -51857,7 +51857,7 @@ func schema_k8sio_api_resource_v1beta1_CapacityRequirements(ref common.Reference Properties: map[string]spec.Schema{ "requests": { SchemaProps: spec.SchemaProps{ - Description: "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + Description: "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim's status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", Type: []string{"object"}, AdditionalProperties: &spec.SchemaOrBool{ Allows: true, @@ -52891,7 +52891,7 @@ func schema_k8sio_api_resource_v1beta1_DeviceRequestAllocationResult(ref common. }, "consumedCapacity": { SchemaProps: spec.SchemaProps{ - Description: "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + Description: "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", Type: []string{"object"}, AdditionalProperties: &spec.SchemaOrBool{ Allows: true, @@ -54155,7 +54155,7 @@ func schema_k8sio_api_resource_v1beta2_CapacityRequirements(ref common.Reference Properties: map[string]spec.Schema{ "requests": { SchemaProps: spec.SchemaProps{ - Description: "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + Description: "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", Type: []string{"object"}, AdditionalProperties: &spec.SchemaOrBool{ Allows: true, @@ -55249,7 +55249,7 @@ func schema_k8sio_api_resource_v1beta2_DeviceRequestAllocationResult(ref common. }, "consumedCapacity": { SchemaProps: spec.SchemaProps{ - Description: "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + Description: "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", Type: []string{"object"}, AdditionalProperties: &spec.SchemaOrBool{ Allows: true, diff --git a/pkg/registry/resource/resourceclaim/strategy_test.go b/pkg/registry/resource/resourceclaim/strategy_test.go index 1f4674b2b14..7eaae26193a 100644 --- a/pkg/registry/resource/resourceclaim/strategy_test.go +++ b/pkg/registry/resource/resourceclaim/strategy_test.go @@ -419,7 +419,7 @@ var objWithCapacityRequests = &resource.ResourceClaim{ AllocationMode: resource.DeviceAllocationModeAll, Capacity: &resource.CapacityRequirements{ Requests: map[resource.QualifiedName]apiresource.Quantity{ - resource.QualifiedName("test-capacity"): apiresource.MustParse("1"), + resource.QualifiedName("test_capacity"): apiresource.MustParse("1"), }, }, }, @@ -536,7 +536,7 @@ var ( ) var testCapacity = map[resource.QualifiedName]apiresource.Quantity{ - resource.QualifiedName("test-capacity"): apiresource.MustParse("1"), + resource.QualifiedName("test_capacity"): apiresource.MustParse("1"), } func TestStrategy(t *testing.T) { diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy.go b/pkg/registry/resource/resourceclaimtemplate/strategy.go index 8204af8ef40..ba0bb8913e1 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy.go +++ b/pkg/registry/resource/resourceclaimtemplate/strategy.go @@ -92,8 +92,7 @@ func (*resourceClaimTemplateStrategy) PrepareForUpdate(ctx context.Context, obj, func (s *resourceClaimTemplateStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { // AuthorizedForAdmin isn't needed here because the spec is immutable. - errorList := validation.ValidateResourceClaimTemplate(obj.(*resource.ResourceClaimTemplate)) - return append(errorList, validation.ValidateResourceClaimTemplateUpdate(obj.(*resource.ResourceClaimTemplate), old.(*resource.ResourceClaimTemplate))...) + return validation.ValidateResourceClaimTemplateUpdate(obj.(*resource.ResourceClaimTemplate), old.(*resource.ResourceClaimTemplate)) } func (*resourceClaimTemplateStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { diff --git a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go b/pkg/registry/resource/resourceclaimtemplate/strategy_test.go index 237ecdff6bc..9e98b7170cc 100644 --- a/pkg/registry/resource/resourceclaimtemplate/strategy_test.go +++ b/pkg/registry/resource/resourceclaimtemplate/strategy_test.go @@ -248,7 +248,7 @@ var objWithDerivedAttributesInPrioritizedList = &resource.ResourceClaimTemplate{ } var testCapacity = map[resource.QualifiedName]apiresource.Quantity{ - resource.QualifiedName("test-capacity"): apiresource.MustParse("1"), + resource.QualifiedName("test_capacity"): apiresource.MustParse("1"), } var objWithCapacityRequests = func() *resource.ResourceClaimTemplate { @@ -257,6 +257,17 @@ var objWithCapacityRequests = func() *resource.ResourceClaimTemplate { return obj }() +// objWithStoredInvalidCapacityName has a capacity request name which was +// accepted before capacity request names were validated. +func objWithStoredInvalidCapacityName(prioritizedList bool, finalizers ...string) *resource.ResourceClaimTemplate { + obj := obj.DeepCopy() + obj.Finalizers = finalizers + addSpecDeviceRequestWithCapacityRequests(obj, map[resource.QualifiedName]apiresource.Quantity{ + "test-capacity": apiresource.MustParse("1"), + }, prioritizedList) + return obj +} + func addSpecDeviceRequestWithCapacityRequests(resourceClaimTemplate *resource.ResourceClaimTemplate, capacity map[resource.QualifiedName]apiresource.Quantity, prioritizedListFeature bool) { r := resource.DeviceRequest{ @@ -978,6 +989,32 @@ func TestStrategyUpdate(t *testing.T) { } }, }, + "remove-finalizer-with-stored-invalid-capacity-name": { + oldObj: objWithStoredInvalidCapacityName(false, "example.com/finalizer"), + newObj: objWithStoredInvalidCapacityName(false), + featureOverrides: featuregatetesting.FeatureOverrides{ + features.DRAConsumableCapacity: true, + }, + expectObj: objWithStoredInvalidCapacityName(false), + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + "remove-finalizer-with-stored-invalid-capacity-name-in-subrequest": { + oldObj: objWithStoredInvalidCapacityName(true, "example.com/finalizer"), + newObj: objWithStoredInvalidCapacityName(true), + featureOverrides: featuregatetesting.FeatureOverrides{ + features.DRAConsumableCapacity: true, + }, + expectObj: objWithStoredInvalidCapacityName(true), + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, } for name, tc := range testcases { diff --git a/pkg/scheduler/framework/plugins/dynamicresources/allocateddevices.go b/pkg/scheduler/framework/plugins/dynamicresources/allocateddevices.go index c0ccc61055b..3bf96eb1d73 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/allocateddevices.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/allocateddevices.go @@ -17,9 +17,14 @@ limitations under the License. package dynamicresources import ( + "fmt" + "maps" + "slices" + "strings" "sync" resourceapi "k8s.io/api/resource/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/cache" @@ -81,17 +86,37 @@ func foreachAllocatedDevice(claim *resourceapi.ResourceClaim, } } +// consumedCapacityError checks the consumed capacity of each allocation result. +// Checking only the aggregated capacity would not be enough, because a positive +// value from another claim for the same device can hide a negative value. +// The stored names get checked, because NewDeviceConsumedCapacity drops an +// unqualified name when the qualified name is also set. +func consumedCapacityError(claim *resourceapi.ResourceClaim) error { + if claim.Status.Allocation == nil { + return nil + } + for _, result := range claim.Status.Allocation.Devices.Results { + for _, name := range slices.Sorted(maps.Keys(result.ConsumedCapacity)) { + if quantity := result.ConsumedCapacity[name]; quantity.Sign() < 0 { + deviceID := structured.MakeDeviceID(result.Driver, result.Pool, result.Device) + return fmt.Errorf("claim %s, device %s: consumed capacity %q must not be negative, got %s", klog.KObj(claim), deviceID, name, quantity.String()) + } + } + } + return nil +} + // allocatedDevices reacts to events in a cache and maintains a set of all allocated devices. // This is cheaper than repeatedly calling List, making strings unique, and building the set // each time PreFilter is called. // // To simplify detecting concurrent changes, each modification bumps a revision counter, -// similar to ResourceVersion in the apiserver. Get and Capacities include the +// similar to ResourceVersion in the apiserver. get and getCapacities include the // current value in their result. A caller than can compare againt the current value // to determine whether some prior results are still up-to-date, without having to get // and compare them. // -// All methods are thread-safe. Get returns a cloned set. +// All methods are thread-safe. get returns a cloned set. type allocatedDevices struct { logger klog.Logger @@ -100,6 +125,7 @@ type allocatedDevices struct { ids sets.Set[structured.DeviceID] shareIDs sets.Set[structured.SharedDeviceID] capacities structured.ConsumedCapacityCollection + invalidCapacities map[types.UID]error enabledConsumableCapacity bool } @@ -109,18 +135,19 @@ func newAllocatedDevices(logger klog.Logger) *allocatedDevices { ids: sets.New[structured.DeviceID](), shareIDs: sets.New[structured.SharedDeviceID](), capacities: structured.NewConsumedCapacityCollection(), + invalidCapacities: make(map[types.UID]error), enabledConsumableCapacity: utilfeature.DefaultFeatureGate.Enabled(features.DRAConsumableCapacity), } } -func (a *allocatedDevices) Get() (sets.Set[structured.DeviceID], int64) { +func (a *allocatedDevices) get() (sets.Set[structured.DeviceID], int64) { a.mutex.RLock() defer a.mutex.RUnlock() return a.ids.Clone(), a.revision } -func (a *allocatedDevices) GetSharedDeviceIDs() (sets.Set[structured.DeviceID], int64) { +func (a *allocatedDevices) getSharedDeviceIDs() (sets.Set[structured.DeviceID], int64) { a.mutex.RLock() defer a.mutex.RUnlock() @@ -131,14 +158,26 @@ func (a *allocatedDevices) GetSharedDeviceIDs() (sets.Set[structured.DeviceID], return result, a.revision } -func (a *allocatedDevices) Capacities() (structured.ConsumedCapacityCollection, int64) { +// getCapacities returns an error instead of the capacities if some allocated claim +// has a negative consumed capacity, because the aggregated capacities then +// under-count what is in use. The error remains until all such claims are removed. +func (a *allocatedDevices) getCapacities() (structured.ConsumedCapacityCollection, int64, error) { a.mutex.RLock() defer a.mutex.RUnlock() - return a.capacities.Clone(), a.revision + if len(a.invalidCapacities) > 0 { + errs := slices.SortedFunc(maps.Values(a.invalidCapacities), func(x, y error) int { + return strings.Compare(x.Error(), y.Error()) + }) + if len(errs) > 1 { + return nil, a.revision, fmt.Errorf("%w (and %d more claims with invalid consumed capacity)", errs[0], len(errs)-1) + } + return nil, a.revision, errs[0] + } + return a.capacities.Clone(), a.revision, nil } -func (a *allocatedDevices) Revision() int64 { +func (a *allocatedDevices) getRevision() int64 { a.mutex.RLock() defer a.mutex.RUnlock() @@ -205,9 +244,11 @@ func (a *allocatedDevices) addDevices(claim *resourceapi.ResourceClaim) { deviceIDs := make([]structured.DeviceID, 0, 20) var shareIDs []structured.SharedDeviceID var deviceCapacities []structured.DeviceConsumedCapacity + var capacityErr error if a.enabledConsumableCapacity { shareIDs = make([]structured.SharedDeviceID, 0, 20) deviceCapacities = make([]structured.DeviceConsumedCapacity, 0, 20) + capacityErr = consumedCapacityError(claim) } foreachAllocatedDevice(claim, func(deviceID structured.DeviceID) { @@ -221,11 +262,15 @@ func (a *allocatedDevices) addDevices(claim *resourceapi.ResourceClaim) { }, func(capacity structured.DeviceConsumedCapacity) { a.logger.V(6).Info("Observed consumed capacity", "device", capacity.DeviceID, "consumed capacity", capacity.ConsumedCapacity, "claim", klog.KObj(claim)) - deviceCapacities = append(deviceCapacities, capacity) + // A negative value can bring the aggregated capacity of a device to zero + // while valid claims still consume it, and Remove then deletes the entry. + if capacityErr == nil { + deviceCapacities = append(deviceCapacities, capacity) + } }, ) - if len(deviceIDs) == 0 && len(shareIDs) == 0 && len(deviceCapacities) == 0 { + if len(deviceIDs) == 0 && len(shareIDs) == 0 && len(deviceCapacities) == 0 && capacityErr == nil { return } @@ -241,6 +286,9 @@ func (a *allocatedDevices) addDevices(claim *resourceapi.ResourceClaim) { for _, capacity := range deviceCapacities { a.capacities.Insert(capacity) } + if capacityErr != nil { + a.invalidCapacities[claim.UID] = capacityErr + } } func (a *allocatedDevices) removeDevices(claim *resourceapi.ResourceClaim) { @@ -253,9 +301,11 @@ func (a *allocatedDevices) removeDevices(claim *resourceapi.ResourceClaim) { deviceIDs := make([]structured.DeviceID, 0, 20) var shareIDs []structured.SharedDeviceID var deviceCapacities []structured.DeviceConsumedCapacity + var capacityErr error if a.enabledConsumableCapacity { shareIDs = make([]structured.SharedDeviceID, 0, 20) deviceCapacities = make([]structured.DeviceConsumedCapacity, 0, 20) + capacityErr = consumedCapacityError(claim) } foreachAllocatedDevice(claim, func(deviceID structured.DeviceID) { @@ -269,10 +319,13 @@ func (a *allocatedDevices) removeDevices(claim *resourceapi.ResourceClaim) { }, func(capacity structured.DeviceConsumedCapacity) { a.logger.V(6).Info("Observed consumed capacity release", "device id", capacity.DeviceID, "consumed capacity", capacity.ConsumedCapacity, "claim", klog.KObj(claim)) - deviceCapacities = append(deviceCapacities, capacity) + // addDevices did not insert the capacities of an invalid claim. + if capacityErr == nil { + deviceCapacities = append(deviceCapacities, capacity) + } }) - if len(deviceIDs) == 0 && len(shareIDs) == 0 && len(deviceCapacities) == 0 { + if len(deviceIDs) == 0 && len(shareIDs) == 0 && len(deviceCapacities) == 0 && capacityErr == nil { return } @@ -288,4 +341,5 @@ func (a *allocatedDevices) removeDevices(claim *resourceapi.ResourceClaim) { for _, capacity := range deviceCapacities { a.capacities.Remove(capacity) } + delete(a.invalidCapacities, claim.UID) } diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dra_manager.go b/pkg/scheduler/framework/plugins/dynamicresources/dra_manager.go index b693e3864e3..29fb5187c9e 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dra_manager.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dra_manager.go @@ -326,7 +326,7 @@ func (c *claimTracker) ListAllAllocatedDevices() (a sets.Set[structured.DeviceID // Start with a fresh set that matches the current known state of the // world according to the informers. - allocated, revision := c.allocatedDevices.Get() + allocated, revision := c.allocatedDevices.get() // Whatever is in flight also has to be checked. for _, inFlight := range c.allInFlightAllocationsRLocked() { @@ -337,7 +337,7 @@ func (c *claimTracker) ListAllAllocatedDevices() (a sets.Set[structured.DeviceID }, false, func(structured.SharedDeviceID) {}, func(structured.DeviceConsumedCapacity) {}) } - if revision == c.allocatedDevices.Revision() { + if revision == c.allocatedDevices.getRevision() { // Our current result is valid, nothing changed in the meantime. return allocated, nil } @@ -378,9 +378,12 @@ func (c *claimTracker) GatherAllocatedState() (s *structured.AllocatedState, err // world according to the informers. enabledConsumableCapacity := utilfeature.DefaultFeatureGate.Enabled(features.DRAConsumableCapacity) - allocated, revision1 := c.allocatedDevices.Get() - allocatedSharedDeviceIDs, revision2 := c.allocatedDevices.GetSharedDeviceIDs() - aggregatedCapacity, revision3 := c.allocatedDevices.Capacities() + allocated, revision1 := c.allocatedDevices.get() + allocatedSharedDeviceIDs, revision2 := c.allocatedDevices.getSharedDeviceIDs() + aggregatedCapacity, revision3, err := c.allocatedDevices.getCapacities() + if err != nil { + return nil, err + } if revision1 != revision2 || revision2 != revision3 { // Already not consistent. Try again. @@ -403,6 +406,11 @@ func (c *claimTracker) GatherAllocatedState() (s *structured.AllocatedState, err // Whatever is in flight also has to be checked. for _, inFlight := range c.allInFlightAllocationsRLocked() { claim := inFlight.claim + if enabledConsumableCapacity { + if err = consumedCapacityError(claim); err != nil { + return nil, err + } + } foreachAllocatedDevice(claim, func(deviceID structured.DeviceID) { // dedicatedDeviceCallback c.logger.V(6).Info("Device is in flight for allocation", "device", deviceID, "claim", klog.KObj(claim)) @@ -418,7 +426,7 @@ func (c *claimTracker) GatherAllocatedState() (s *structured.AllocatedState, err aggregatedCapacity.Insert(capacity) }) } - if revision1 == c.allocatedDevices.Revision() { + if revision1 == c.allocatedDevices.getRevision() { // Our current result is valid, nothing changed in the meantime. return &structured.AllocatedState{ AllocatedDevices: allocated, diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dra_manager_test.go b/pkg/scheduler/framework/plugins/dynamicresources/dra_manager_test.go index 395b11a72b6..54ec1dde882 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dra_manager_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dra_manager_test.go @@ -22,9 +22,15 @@ import ( "github.com/onsi/gomega" resourceapi "k8s.io/api/resource/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/cache" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/dynamic-resource-allocation/structured" "k8s.io/ktesting" + "k8s.io/kubernetes/pkg/features" st "k8s.io/kubernetes/pkg/scheduler/testing" "k8s.io/kubernetes/pkg/scheduler/util/assumecache" ) @@ -264,3 +270,158 @@ func testMaybeRemoveClaimPendingAllocation(tCtx ktesting.TContext) { }) } } + +const ( + sharedDriver = "driver.example.com" + sharedPool = "pool-0" + sharedDevice = "device-0" +) + +// claimWithConsumedCapacity returns a claim with one share of the same device. +func claimWithConsumedCapacity(uid types.UID, memory string) *resourceapi.ResourceClaim { + return &resourceapi.ResourceClaim{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: string(uid), UID: uid}, + Status: resourceapi.ResourceClaimStatus{ + Allocation: &resourceapi.AllocationResult{ + Devices: resourceapi.DeviceAllocationResult{ + Results: []resourceapi.DeviceRequestAllocationResult{{ + Request: "req-0", + Driver: sharedDriver, + Pool: sharedPool, + Device: sharedDevice, + ShareID: new(uid + "-share"), + ConsumedCapacity: map[resourceapi.QualifiedName]resource.Quantity{"memory": resource.MustParse(memory)}, + }}, + }, + }, + }, + } +} + +func TestGatherAllocatedStateConsumedCapacity(t *testing.T) { + testGatherAllocatedStateConsumedCapacity(ktesting.Init(t)) +} +func testGatherAllocatedStateConsumedCapacity(tCtx ktesting.TContext) { + featuregatetesting.SetFeatureGateDuringTest(tCtx, utilfeature.DefaultFeatureGate, features.DRAConsumableCapacity, true) + + valid := claimWithConsumedCapacity("valid", "5") + otherValid := claimWithConsumedCapacity("other-valid", "1") + negative := claimWithConsumedCapacity("negative", "-2") + otherNegative := claimWithConsumedCapacity("other-negative", "-3") + small := claimWithConsumedCapacity("small", "2") + negativeLarge := claimWithConsumedCapacity("negative-large", "-5") + // The qualified name replaces the unqualified one in the aggregated capacity. + negativeUnqualified := claimWithConsumedCapacity("negative-unqualified", "-2") + negativeUnqualified.Status.Allocation.Devices.Results[0].ConsumedCapacity[sharedDriver+"/memory"] = resource.MustParse("5") + negativeDedicated := claimWithConsumedCapacity("negative-dedicated", "-2") + negativeDedicated.Status.Allocation.Devices.Results[0].ShareID = nil + negativeAdminAccess := claimWithConsumedCapacity("negative-admin-access", "-2") + negativeAdminAccess.Status.Allocation.Devices.Results[0].AdminAccess = new(true) + + tests := map[string]struct { + allocated []*resourceapi.ResourceClaim + removed []*resourceapi.ResourceClaim + inFlight []*resourceapi.ResourceClaim + // expectErr is a substring of the expected error. If empty, + // expectMemory is the aggregated memory of the shared device. + expectErr string + expectMemory string + }{ + "valid": { + allocated: []*resourceapi.ResourceClaim{valid, otherValid}, + expectMemory: "6", + }, + "negative-alone": { + allocated: []*resourceapi.ResourceClaim{negative}, + expectErr: "claim default/negative, device driver.example.com/pool-0/device-0: consumed capacity \"memory\" must not be negative, got -2", + }, + "negative-after-positive": { + allocated: []*resourceapi.ResourceClaim{valid, negative}, + expectErr: "claim default/negative", + }, + // The aggregated capacity would be 3, which is not negative. + "negative-hidden-by-later-positive": { + allocated: []*resourceapi.ResourceClaim{negative, valid}, + expectErr: "claim default/negative", + }, + "several-negative": { + allocated: []*resourceapi.ResourceClaim{negative, otherNegative}, + expectErr: "claim default/negative, device driver.example.com/pool-0/device-0: consumed capacity \"memory\" must not be negative, got -2 (and 1 more claims with invalid consumed capacity)", + }, + "negative-in-flight": { + allocated: []*resourceapi.ResourceClaim{valid}, + inFlight: []*resourceapi.ResourceClaim{negative}, + expectErr: "claim default/negative", + }, + "negative-unqualified": { + allocated: []*resourceapi.ResourceClaim{negativeUnqualified}, + expectErr: "claim default/negative-unqualified, device driver.example.com/pool-0/device-0: consumed capacity \"memory\" must not be negative, got -2", + }, + "negative-unqualified-in-flight": { + inFlight: []*resourceapi.ResourceClaim{negativeUnqualified}, + expectErr: "claim default/negative-unqualified", + }, + "negative-dedicated": { + allocated: []*resourceapi.ResourceClaim{negativeDedicated}, + expectErr: "claim default/negative-dedicated", + }, + "negative-admin-access": { + allocated: []*resourceapi.ResourceClaim{negativeAdminAccess}, + expectErr: "claim default/negative-admin-access", + }, + "negative-admin-access-removed": { + allocated: []*resourceapi.ResourceClaim{negativeAdminAccess, valid}, + removed: []*resourceapi.ResourceClaim{negativeAdminAccess}, + expectMemory: "5", + }, + "negative-removed": { + allocated: []*resourceapi.ResourceClaim{negative, valid}, + removed: []*resourceapi.ResourceClaim{negative}, + expectMemory: "5", + }, + // 5 + 2 - 5 becomes zero when the small claim is removed, which must + // not drop the device while the valid claim still consumes it. + "negative-removed-after-sum-is-zero": { + allocated: []*resourceapi.ResourceClaim{valid, small, negativeLarge}, + removed: []*resourceapi.ResourceClaim{small, negativeLarge}, + expectMemory: "5", + }, + "one-of-several-negative-removed": { + allocated: []*resourceapi.ResourceClaim{negative, otherNegative, valid}, + removed: []*resourceapi.ResourceClaim{negative}, + expectErr: "claim default/other-negative", + }, + } + + for name, test := range tests { + tCtx.Run(name, func(tCtx ktesting.TContext) { + c := &claimTracker{ + logger: tCtx.Logger(), + inFlightAllocations: make(map[types.UID]inFlightAllocation), + allocatedDevices: newAllocatedDevices(tCtx.Logger()), + } + for _, claim := range test.allocated { + c.allocatedDevices.onAdd(claim) + } + for _, claim := range test.removed { + c.allocatedDevices.onDelete(claim) + } + for _, claim := range test.inFlight { + c.inFlightAllocations[claim.UID] = inFlightAllocation{claim: claim, sharers: 1} + } + + state, err := c.GatherAllocatedState() + if test.expectErr != "" { + tCtx.Expect(err).To(gomega.MatchError(gomega.ContainSubstring(test.expectErr))) + return + } + tCtx.ExpectNoError(err) + deviceID := structured.MakeDeviceID(sharedDriver, sharedPool, sharedDevice) + capacity := state.AggregatedCapacity[deviceID] + tCtx.Expect(capacity).To(gomega.HaveLen(1)) + for _, memory := range capacity { + tCtx.Expect(memory.Cmp(resource.MustParse(test.expectMemory))).To(gomega.BeZero(), "aggregated memory %s", memory) + } + }) + } +} diff --git a/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources.go b/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources.go index 51c4b5a117b..6e5954162ee 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources.go @@ -218,6 +218,9 @@ func addDeviceMapping( return fmt.Errorf("claim %s/%s, device %s: ConsumedCapacity is nil, but Capacity key '%s' is set in NodeAllocatableResources for resource %s", key.Namespace, key.Name, result.Device, capacityKey, resourceName) } if consumed, exists := draapi.LookupByQualifiedName(result.ConsumedCapacity, capacityKey, result.Driver); exists { + if consumed.Sign() < 0 { + return fmt.Errorf("claim %s/%s, device %s: ConsumedCapacity for capacity key '%s' must not be negative, got %s", key.Namespace, key.Name, result.Device, capacityKey, consumed.String()) + } // If !exists - the capacityKey is not in ConsumedCapacity, this mapping is not relevant for this allocation consumedQuantity := consumed.DeepCopy() quantityOne := resource.MustParse("1") @@ -227,6 +230,9 @@ func addDeviceMapping( qDec.Mul(qDec, multiplier.AsDec()) consumedQuantity = *resource.NewDecimalQuantity(*qDec, consumedQuantity.Format) } + if consumedQuantity.Sign() < 0 { + return fmt.Errorf("claim %s/%s, device %s: computed node-allocatable footprint for resource %s must not be negative, got %s", key.Namespace, key.Name, result.Device, resourceName, consumedQuantity.String()) + } current := totalResources[resourceName] current.Add(consumedQuantity) totalResources[resourceName] = current diff --git a/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources_test.go b/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources_test.go index e2ce491128e..0eaac250203 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/nodeallocatabledynamicresources_test.go @@ -1310,6 +1310,16 @@ func TestBuildNodeAllocatableDRAInfo(t *testing.T) { }, wantErr: true, }, + { + name: "Invalid - Negative ConsumedCapacity", + pod: st.MakePod().Name("test-pod").Namespace(claimNameSpace).UID("test-uid").Containers([]v1.Container{{Name: "c1", Resources: v1.ResourceRequirements{Claims: []v1.ResourceClaim{{Name: "claim1"}}}}}).Obj(), + claims: []*resourceapi.ResourceClaim{makeClaim("claim1", "claim1-uid")}, + resourceSlices: []*resourceapi.ResourceSlice{makeSlice("slice1", cpuDeviceCapacity)}, + nodeAllocatableClaimAllocations: map[v1.ObjectReference]*resourceapi.AllocationResult{ + {Name: "claim1", UID: "claim1-uid"}: allocResult("pool1", "cpu0", map[resourceapi.QualifiedName]resource.Quantity{"dra.example.com/cpu": resource.MustParse("-8")}), + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/staging/src/k8s.io/api/resource/v1/generated.proto b/staging/src/k8s.io/api/resource/v1/generated.proto index 1c9c0f28699..6fae24e2073 100644 --- a/staging/src/k8s.io/api/resource/v1/generated.proto +++ b/staging/src/k8s.io/api/resource/v1/generated.proto @@ -318,9 +318,14 @@ message CapacityRequirements { // If the device allows multiple allocation, // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, - // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // is recorded in the resource claim's status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. // // +optional + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 map requests = 1; } @@ -1175,8 +1180,13 @@ message DeviceRequestAllocationResult { // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. + // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 map consumedCapacity = 10; // skipNodeOperations lists node-local resource operations (gRPC calls) @@ -1316,6 +1326,7 @@ message DeviceSubRequest { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional optional CapacityRequirements capacity = 7; // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1662,6 +1673,7 @@ message ExactDeviceRequest { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional optional CapacityRequirements capacity = 7; // derivedAttributes defines a set of virtual attributes computed via CEL expressions diff --git a/staging/src/k8s.io/api/resource/v1/types.go b/staging/src/k8s.io/api/resource/v1/types.go index 1984918b564..7139f7c0e83 100644 --- a/staging/src/k8s.io/api/resource/v1/types.go +++ b/staging/src/k8s.io/api/resource/v1/types.go @@ -1277,6 +1277,7 @@ const ( DeviceRequestsMaxSize = AllocationResultsMaxSize DeviceConstraintsMaxSize = 32 DeviceConfigMaxSize = 32 + MaxCapacityRequirements = 32 ) // DRAAdminNamespaceLabelKey is a label key used to grant administrative access @@ -1453,6 +1454,7 @@ type ExactDeviceRequest struct { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional Capacity *CapacityRequirements `json:"capacity,omitempty" protobuf:"bytes,7,opt,name=capacity"` // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1598,6 +1600,7 @@ type DeviceSubRequest struct { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional Capacity *CapacityRequirements `json:"capacity,omitempty" protobuf:"bytes,7,opt,name=capacity"` // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1663,9 +1666,14 @@ type CapacityRequirements struct { // If the device allows multiple allocation, // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, - // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // is recorded in the resource claim's status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. // // +optional + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 Requests map[QualifiedName]resource.Quantity `json:"requests,omitempty" protobuf:"bytes,1,rep,name=requests,castkey=QualifiedName"` } @@ -2338,8 +2346,13 @@ type DeviceRequestAllocationResult struct { // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. + // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 ConsumedCapacity map[QualifiedName]resource.Quantity `json:"consumedCapacity,omitempty" protobuf:"bytes,10,rep,name=consumedCapacity"` // skipNodeOperations lists node-local resource operations (gRPC calls) diff --git a/staging/src/k8s.io/api/resource/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/resource/v1/types_swagger_doc_generated.go index fa682e866d4..dc5d7b49ebe 100644 --- a/staging/src/k8s.io/api/resource/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/resource/v1/types_swagger_doc_generated.go @@ -86,7 +86,7 @@ func (CapacityRequestPolicyRange) SwaggerDoc() map[string]string { var map_CapacityRequirements = map[string]string{ "": "CapacityRequirements defines the capacity requirements for a specific device request.", - "requests": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + "requests": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim's status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", } func (CapacityRequirements) SwaggerDoc() map[string]string { @@ -301,7 +301,7 @@ var map_DeviceRequestAllocationResult = map[string]string{ "bindingConditions": "bindingConditions contains a copy of the BindingConditions from the corresponding ResourceSlice at the time of allocation.\n\nThis is a beta field and requires enabling the DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates.", "bindingFailureConditions": "bindingFailureConditions contains a copy of the BindingFailureConditions from the corresponding ResourceSlice at the time of allocation.\n\nThis is a beta field and requires enabling the DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates.", "shareID": "shareID uniquely identifies an individual allocation share of the device, used when the device supports multiple simultaneous allocations. It serves as an additional map key to differentiate concurrent shares of the same device.", - "consumedCapacity": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + "consumedCapacity": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", "skipNodeOperations": "skipNodeOperations lists node-local resource operations (gRPC calls) that will be skipped for this allocated device when determining whether operations are necessary on the node. If all allocated devices for a driver in a claim skip an operation, that gRPC call will be skipped. It is a copy of the ResourceSlice.spec.skipNodeOperations value at the time when the device was allocated.", } diff --git a/staging/src/k8s.io/api/resource/v1/zz_generated.validations.go b/staging/src/k8s.io/api/resource/v1/zz_generated.validations.go index ea37def86b9..b69a8947dfc 100644 --- a/staging/src/k8s.io/api/resource/v1/zz_generated.validations.go +++ b/staging/src/k8s.io/api/resource/v1/zz_generated.validations.go @@ -198,6 +198,53 @@ func Validate_AllocationResult( return errs } +// Validate_CapacityRequirements validates an instance of CapacityRequirements according +// to declarative validation rules in the API schema. +func Validate_CapacityRequirements( + ctx context.Context, op operation.Operation, fldPath *field.Path, + obj, oldObj *CapacityRequirements) (errs field.ErrorList) { + + { // field CapacityRequirements.Requests + fn := func( + fldPath *field.Path, + obj, oldObj map[QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *CapacityRequirements) map[QualifiedName]resource.Quantity { + return oldObj.Requests + }) + errs = append(errs, fn(fldPath.Child("requests"), obj.Requests, oldVal, oldObj != nil)...) + } + + return errs +} + // Validate_CounterSet validates an instance of CounterSet according // to declarative validation rules in the API schema. func Validate_CounterSet( @@ -1930,7 +1977,43 @@ func Validate_DeviceRequestAllocationResult( errs = append(errs, fn(fldPath.Child("shareID"), obj.ShareID, oldVal, oldObj != nil)...) } - // field DeviceRequestAllocationResult.ConsumedCapacity has no validation + { // field DeviceRequestAllocationResult.ConsumedCapacity + fn := func( + fldPath *field.Path, + obj, oldObj map[QualifiedName]resource.Quantity, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.MaxProperties(ctx, op, fldPath, obj, oldObj, 32).MarkShortCircuit(); len(e) != 0 { + errs = append(errs, e...) + earlyReturn = true + } + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + if e := validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, + func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *resource.Quantity) field.ErrorList { + return validate.MinimumQuantity(ctx, op, fldPath, obj, oldObj, resource.MustParse("0")) + }); len(e) != 0 { + errs = append(errs, e...) + } + return + } + oldVal := safe.Field(oldObj, + func(oldObj *DeviceRequestAllocationResult) map[QualifiedName]resource.Quantity { + return oldObj.ConsumedCapacity + }) + errs = append(errs, fn(fldPath.Child("consumedCapacity"), obj.ConsumedCapacity, oldVal, oldObj != nil)...) + } { // field DeviceRequestAllocationResult.SkipNodeOperations fn := func( @@ -2107,7 +2190,35 @@ func Validate_DeviceSubRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field DeviceSubRequest.Capacity has no validation + { // field DeviceSubRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *DeviceSubRequest) *CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field DeviceSubRequest.DerivedAttributes fn := func( @@ -2512,7 +2623,35 @@ func Validate_ExactDeviceRequest( errs = append(errs, fn(fldPath.Child("tolerations"), obj.Tolerations, oldVal, oldObj != nil)...) } - // field ExactDeviceRequest.Capacity has no validation + { // field ExactDeviceRequest.Capacity + fn := func( + fldPath *field.Path, + obj, oldObj *CapacityRequirements, + oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update { + if validate.SemanticDeepEqual(obj, oldObj) { + return nil + } + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalPointer(ctx, op, fldPath, obj, oldObj).MarkShortCircuit(); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + // call the type's validation function + errs = append(errs, Validate_CapacityRequirements(ctx, op, fldPath, obj, oldObj)...) + return + } + oldVal := safe.Field(oldObj, + func(oldObj *ExactDeviceRequest) *CapacityRequirements { + return oldObj.Capacity + }) + errs = append(errs, fn(fldPath.Child("capacity"), obj.Capacity, oldVal, oldObj != nil)...) + } { // field ExactDeviceRequest.DerivedAttributes fn := func( diff --git a/staging/src/k8s.io/api/resource/v1beta1/generated.proto b/staging/src/k8s.io/api/resource/v1beta1/generated.proto index 0de4759d014..f9d1582e447 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/generated.proto +++ b/staging/src/k8s.io/api/resource/v1beta1/generated.proto @@ -477,9 +477,14 @@ message CapacityRequirements { // If the device allows multiple allocation, // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, - // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // is recorded in the resource claim's status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. // // +optional + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 map requests = 1; } @@ -1177,6 +1182,7 @@ message DeviceRequest { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional optional CapacityRequirements capacity = 9; // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1325,8 +1331,13 @@ message DeviceRequestAllocationResult { // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. + // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 map consumedCapacity = 10; // skipNodeOperations lists node-local resource operations (gRPC calls) @@ -1467,6 +1478,7 @@ message DeviceSubRequest { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional optional CapacityRequirements capacity = 8; // derivedAttributes defines a set of virtual attributes computed via CEL expressions diff --git a/staging/src/k8s.io/api/resource/v1beta1/types.go b/staging/src/k8s.io/api/resource/v1beta1/types.go index c5aaa9997bd..d1ff4870d33 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/types.go +++ b/staging/src/k8s.io/api/resource/v1beta1/types.go @@ -1289,6 +1289,7 @@ type DeviceRequest struct { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional Capacity *CapacityRequirements `json:"capacity,omitempty" protobuf:"bytes,9,opt,name=capacity"` // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1435,6 +1436,7 @@ type DeviceSubRequest struct { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional Capacity *CapacityRequirements `json:"capacity,omitempty" protobuf:"bytes,8,opt,name=capacity"` // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1500,9 +1502,14 @@ type CapacityRequirements struct { // If the device allows multiple allocation, // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, - // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // is recorded in the resource claim's status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. // // +optional + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 Requests map[QualifiedName]resource.Quantity `json:"requests,omitempty" protobuf:"bytes,1,rep,name=requests,castkey=QualifiedName"` } @@ -1566,6 +1573,9 @@ const ( FirstAvailableDeviceRequestMaxSize = 8 DeviceTolerationsMaxLength = 16 DeviceDerivedAttributesMaxSize = 32 + // MaxCapacityRequirements is the maximum number of entries allowed in + // CapacityRequirements.Requests. + MaxCapacityRequirements = 32 ) // +enum @@ -2176,8 +2186,13 @@ type DeviceRequestAllocationResult struct { // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. + // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 ConsumedCapacity map[QualifiedName]resource.Quantity `json:"consumedCapacity,omitempty" protobuf:"bytes,10,rep,name=consumedCapacity"` // skipNodeOperations lists node-local resource operations (gRPC calls) diff --git a/staging/src/k8s.io/api/resource/v1beta1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/resource/v1beta1/types_swagger_doc_generated.go index 6643b4b449a..6a0a0a16b20 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/resource/v1beta1/types_swagger_doc_generated.go @@ -106,7 +106,7 @@ func (CapacityRequestPolicyRange) SwaggerDoc() map[string]string { var map_CapacityRequirements = map[string]string{ "": "CapacityRequirements defines the capacity requirements for a specific device request.", - "requests": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + "requests": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim's status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", } func (CapacityRequirements) SwaggerDoc() map[string]string { @@ -317,7 +317,7 @@ var map_DeviceRequestAllocationResult = map[string]string{ "bindingConditions": "bindingConditions contains a copy of the BindingConditions from the corresponding ResourceSlice at the time of allocation.\n\nThis is a beta field and requires enabling the DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates.", "bindingFailureConditions": "bindingFailureConditions contains a copy of the BindingFailureConditions from the corresponding ResourceSlice at the time of allocation.\n\nThis is a beta field and requires enabling the DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates.", "shareID": "shareID uniquely identifies an individual allocation share of the device, used when the device supports multiple simultaneous allocations. It serves as an additional map key to differentiate concurrent shares of the same device.", - "consumedCapacity": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + "consumedCapacity": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", "skipNodeOperations": "skipNodeOperations lists node-local resource operations (gRPC calls) that will be skipped for this allocated device when determining whether operations are necessary on the node. If all allocated devices for a driver in a claim skip an operation, that gRPC call will be skipped. It is a copy of the ResourceSlice.spec.skipNodeOperations value at the time when the device was allocated.", } diff --git a/staging/src/k8s.io/api/resource/v1beta2/generated.proto b/staging/src/k8s.io/api/resource/v1beta2/generated.proto index 6e338f2b8a9..f67423ed7a7 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/generated.proto +++ b/staging/src/k8s.io/api/resource/v1beta2/generated.proto @@ -320,7 +320,12 @@ message CapacityRequirements { // The consumed capacity, which may be adjusted based on the requestPolicy if defined, // is recorded in the resource claim’s status.devices[*].consumedCapacity field. // + // Requests may have at most 32 entries. The values must not be negative. + // // +optional + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 map requests = 1; } @@ -1176,8 +1181,13 @@ message DeviceRequestAllocationResult { // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. + // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 map consumedCapacity = 10; // skipNodeOperations lists node-local resource operations (gRPC calls) @@ -1317,6 +1327,7 @@ message DeviceSubRequest { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional optional CapacityRequirements capacity = 7; // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1664,6 +1675,7 @@ message ExactDeviceRequest { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional optional CapacityRequirements capacity = 7; // derivedAttributes defines a set of virtual attributes computed via CEL expressions diff --git a/staging/src/k8s.io/api/resource/v1beta2/types.go b/staging/src/k8s.io/api/resource/v1beta2/types.go index 9531bff46a2..25a22c2825d 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/types.go +++ b/staging/src/k8s.io/api/resource/v1beta2/types.go @@ -1259,6 +1259,9 @@ const ( DeviceRequestsMaxSize = AllocationResultsMaxSize DeviceConstraintsMaxSize = 32 DeviceConfigMaxSize = 32 + // MaxCapacityRequirements is the maximum number of entries allowed in + // CapacityRequirements.Requests. + MaxCapacityRequirements = 32 ) // DRAAdminNamespaceLabelKey is a label key used to grant administrative access @@ -1436,6 +1439,7 @@ type ExactDeviceRequest struct { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional Capacity *CapacityRequirements `json:"capacity,omitempty" protobuf:"bytes,7,opt,name=capacity"` // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1581,6 +1585,7 @@ type DeviceSubRequest struct { // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional Capacity *CapacityRequirements `json:"capacity,omitempty" protobuf:"bytes,7,opt,name=capacity"` // derivedAttributes defines a set of virtual attributes computed via CEL expressions @@ -1648,7 +1653,12 @@ type CapacityRequirements struct { // The consumed capacity, which may be adjusted based on the requestPolicy if defined, // is recorded in the resource claim’s status.devices[*].consumedCapacity field. // + // Requests may have at most 32 entries. The values must not be negative. + // // +optional + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 Requests map[QualifiedName]resource.Quantity `json:"requests,omitempty" protobuf:"bytes,1,rep,name=requests,castkey=QualifiedName"` } @@ -2322,8 +2332,13 @@ type DeviceRequestAllocationResult struct { // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. + // // +optional // +featureGate=DRAConsumableCapacity + // +k8s:optional + // +k8s:maxProperties=32 + // +k8s:eachVal=+k8s:minimum=0 ConsumedCapacity map[QualifiedName]resource.Quantity `json:"consumedCapacity,omitempty" protobuf:"bytes,10,rep,name=consumedCapacity"` // skipNodeOperations lists node-local resource operations (gRPC calls) diff --git a/staging/src/k8s.io/api/resource/v1beta2/types_swagger_doc_generated.go b/staging/src/k8s.io/api/resource/v1beta2/types_swagger_doc_generated.go index 6a4a094154c..0d9160ce03b 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/resource/v1beta2/types_swagger_doc_generated.go @@ -86,7 +86,7 @@ func (CapacityRequestPolicyRange) SwaggerDoc() map[string]string { var map_CapacityRequirements = map[string]string{ "": "CapacityRequirements defines the capacity requirements for a specific device request.", - "requests": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.", + "requests": "requests represent individual device resource requests for distinct resources, all of which must be provided by the device.\n\nThis value is used as an additional filtering condition against the available capacity on the device. This is semantically equivalent to a CEL selector with `device.capacity[]..compareTo(quantity()) >= 0`. For example, device.capacity['test-driver.cdi.k8s.io'].counters.compareTo(quantity('2')) >= 0.\n\nThe domain prefix can be omitted, in which case it defaults to the driver of the device under evaluation. For example, \"bandwidth: 1Gi\" is equivalent to \"/bandwidth: 1Gi\" for a device published by driver \"\", regardless of which other, differently-domained \"bandwidth\" capacities that device might also have. To request one of those, the domain must be given explicitly, for example \"example.com/bandwidth\". Requesting the same driver capacity with and without the driver name as domain, for example \"bandwidth: 1Gi\" and \"/bandwidth: 2Gi\", is ambiguous and causes scheduling to fail with an error. For the sake of consistency there's no exception for such a conflict where the value is the same - that is still an error.\n\nWhen a requestPolicy is defined, the requested amount is adjusted upward to the nearest valid value based on the policy. If the requested amount cannot be adjusted to a valid value—because it exceeds what the requestPolicy allows— the device is considered ineligible for allocation.\n\nFor any capacity that is not explicitly requested: - If no requestPolicy is set, the default consumed capacity is equal to the full device capacity\n (i.e., the whole device is claimed).\n- If a requestPolicy is set, the default consumed capacity is determined according to that policy.\n\nIf the device allows multiple allocation, the aggregated amount across all requests must not exceed the capacity value. The consumed capacity, which may be adjusted based on the requestPolicy if defined, is recorded in the resource claim’s status.devices[*].consumedCapacity field.\n\nRequests may have at most 32 entries. The values must not be negative.", } func (CapacityRequirements) SwaggerDoc() map[string]string { @@ -301,7 +301,7 @@ var map_DeviceRequestAllocationResult = map[string]string{ "bindingConditions": "bindingConditions contains a copy of the BindingConditions from the corresponding ResourceSlice at the time of allocation.\n\nThis is a beta field and requires enabling the DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates.", "bindingFailureConditions": "bindingFailureConditions contains a copy of the BindingFailureConditions from the corresponding ResourceSlice at the time of allocation.\n\nThis is a beta field and requires enabling the DRADeviceBindingConditions and DRAResourceClaimDeviceStatus feature gates.", "shareID": "shareID uniquely identifies an individual allocation share of the device, used when the device supports multiple simultaneous allocations. It serves as an additional map key to differentiate concurrent shares of the same device.", - "consumedCapacity": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.", + "consumedCapacity": "consumedCapacity tracks the amount of capacity consumed per device as part of the claim request. The consumed amount may differ from the requested amount: it is rounded up to the nearest valid value based on the device’s requestPolicy if applicable (i.e., may not be less than the requested amount).\n\nThe total consumed capacity for each device must not exceed the DeviceCapacity's Value.\n\nThis field is populated only for devices that allow multiple allocations. All capacity entries are included, even if the consumed amount is zero. The domain prefix in the capacity name may be omitted if it is the same as the driver name.\n\nConsumedCapacity may have at most 32 entries. The values must not be negative.", "skipNodeOperations": "skipNodeOperations lists node-local resource operations (gRPC calls) that will be skipped for this allocated device when determining whether operations are necessary on the node. If all allocated devices for a driver in a claim skip an operation, that gRPC call will be skipped. It is a copy of the ResourceSlice.spec.skipNodeOperations value at the time when the device was allocated.", } diff --git a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/capacityrequirements.go b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/capacityrequirements.go index 6c21b1da8c0..27fd5fbb507 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/capacityrequirements.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/capacityrequirements.go @@ -60,7 +60,9 @@ type CapacityRequirementsApplyConfiguration struct { // If the device allows multiple allocation, // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, - // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // is recorded in the resource claim's status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. Requests map[resourcev1.QualifiedName]resource.Quantity `json:"requests,omitempty"` } diff --git a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/devicerequestallocationresult.go b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/devicerequestallocationresult.go index e89f2309280..be7b56ef70d 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/devicerequestallocationresult.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1/devicerequestallocationresult.go @@ -95,6 +95,8 @@ type DeviceRequestAllocationResultApplyConfiguration struct { // All capacity entries are included, even if the consumed amount is zero. // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. + // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. ConsumedCapacity map[resourcev1.QualifiedName]resource.Quantity `json:"consumedCapacity,omitempty"` // skipNodeOperations lists node-local resource operations (gRPC calls) // that will be skipped for this allocated device when determining whether diff --git a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/capacityrequirements.go b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/capacityrequirements.go index fa4b2da5495..59ee7abf04d 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/capacityrequirements.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/capacityrequirements.go @@ -60,7 +60,9 @@ type CapacityRequirementsApplyConfiguration struct { // If the device allows multiple allocation, // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, - // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // is recorded in the resource claim's status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. Requests map[resourcev1beta1.QualifiedName]resource.Quantity `json:"requests,omitempty"` } diff --git a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/devicerequestallocationresult.go b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/devicerequestallocationresult.go index 6cb11537015..701fe0f8973 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/devicerequestallocationresult.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta1/devicerequestallocationresult.go @@ -96,6 +96,8 @@ type DeviceRequestAllocationResultApplyConfiguration struct { // All capacity entries are included, even if the consumed amount is zero. // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. + // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. ConsumedCapacity map[resourcev1beta1.QualifiedName]resource.Quantity `json:"consumedCapacity,omitempty"` // skipNodeOperations lists node-local resource operations (gRPC calls) // that will be skipped for this allocated device when determining whether diff --git a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/capacityrequirements.go b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/capacityrequirements.go index fdc7339d46a..299154b17a4 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/capacityrequirements.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/capacityrequirements.go @@ -61,6 +61,8 @@ type CapacityRequirementsApplyConfiguration struct { // the aggregated amount across all requests must not exceed the capacity value. // The consumed capacity, which may be adjusted based on the requestPolicy if defined, // is recorded in the resource claim’s status.devices[*].consumedCapacity field. + // + // Requests may have at most 32 entries. The values must not be negative. Requests map[resourcev1beta2.QualifiedName]resource.Quantity `json:"requests,omitempty"` } diff --git a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/devicerequestallocationresult.go b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/devicerequestallocationresult.go index b9869dfec49..b278d1c1b5b 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/devicerequestallocationresult.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/resource/v1beta2/devicerequestallocationresult.go @@ -96,6 +96,8 @@ type DeviceRequestAllocationResultApplyConfiguration struct { // All capacity entries are included, even if the consumed amount is zero. // The domain prefix in the capacity name may be omitted if it is // the same as the driver name. + // + // ConsumedCapacity may have at most 32 entries. The values must not be negative. ConsumedCapacity map[resourcev1beta2.QualifiedName]resource.Quantity `json:"consumedCapacity,omitempty"` // skipNodeOperations lists node-local resource operations (gRPC calls) // that will be skipped for this allocated device when determining whether diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go index 6fc2921b42f..2da9581cdb4 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/allocatortesting/allocator_testing.go @@ -6423,6 +6423,41 @@ func TestAllocator(t *testing.T, node: node(node1, region1), expectError: g.MatchError(g.ContainSubstring("negative")), }, + "consumable-capacity-negative-request-with-range-policy-aborts": { + features: Features{ + ConsumableCapacity: true, + }, + // The range policy would round the negative request up to Min, a valid value. + claimsToAllocate: objects( + claim(claim0).withRequests(deviceRequest(req0, classA, 1).withCapacityRequest(capacity0, resource.MustParse("-1"))), + ), + classes: objects(classWithAllowMultipleAllocations(classA, driverA, true)), + slices: unwrapResourceSlices( + sliceWithDevices(slice1, node1, pool1, driverA, + device(device1).withAllowMultipleAllocations().withCapacityRequestPolicyRange(map[resourceapi.QualifiedName]resource.Quantity{capacity0: four}), + ), + ), + node: node(node1, region1), + expectError: g.MatchError(g.ContainSubstring("negative")), + }, + "consumable-capacity-negative-request-with-valid-values-policy-aborts": { + features: Features{ + ConsumableCapacity: true, + }, + // The valid values policy would round the negative request up to the smallest valid value. + claimsToAllocate: objects( + claim(claim0).withRequests(deviceRequest(req0, classA, 1).withCapacityRequest(capacity0, resource.MustParse("-1"))), + ), + classes: objects(classWithAllowMultipleAllocations(classA, driverA, true)), + slices: unwrapResourceSlices( + sliceWithDevices(slice1, node1, pool1, driverA, + device(device1).withAllowMultipleAllocations().withCapacityRequestPolicyValidValues(one, map[resourceapi.QualifiedName]resource.Quantity{capacity0: four}, + []resource.Quantity{two}), + ), + ), + node: node(node1, region1), + expectError: g.MatchError(g.ContainSubstring("negative")), + }, "consumable-capacity-multi-allocatable-device-with-valid-values-policy": { features: Features{ ConsumableCapacity: true, diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity.go index 1ab1a5f08f0..2117fd5d83f 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity.go @@ -35,7 +35,7 @@ import ( // request is rejected outright instead of driving a device-by-device retry. var errCapacityRequestNotRepresentable = errors.New("capacity request cannot be represented in the device's range arithmetic") -// errNegativeCapacity signals that a resolved consumed capacity is negative. A negative +// errNegativeCapacity signals that a capacity request or a resolved consumed capacity is negative. A negative // value is not valid capacity and would under-count the device and over-allocate it. It // can reach the allocator from a stored ResourceSlice or ResourceClaim created before // stricter validation, which admission does not re-check, so the allocator rejects it @@ -112,7 +112,7 @@ func cmpRequestOverCapacity(currentConsumedCapacity ConsumedCapacity, requestedC } consumedCapacity, err := calculateConsumedCapacity(requestedValPtr, cap, fractionalCapacityRange) if err != nil { - // The request is not representable in this capacity's range arithmetic. + // The request is negative or not representable in this capacity's range arithmetic. // Surface a fatal error so the allocator aborts instead of retrying other // devices, which a user-controlled out-of-range request could abuse. return nil, false, fmt.Errorf("capacity %q: %w", name, err) @@ -270,6 +270,11 @@ func calculateConsumedCapacity(requestedVal *resource.Quantity, capacity resourc if requestedVal == nil { return fillEmptyRequest(capacity), nil } + // Must be checked before rounding: the request policy can round a negative + // request up to a valid value, which would hide it from the caller's check. + if requestedVal.Sign() < 0 { + return resource.Quantity{}, fmt.Errorf("%w: request %s", errNegativeCapacity, requestedVal.String()) + } if capacity.RequestPolicy == nil { return *requestedVal, nil } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity_test.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity_test.go index a183aa4d012..62c773a1897 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/consumable_capacity_test.go @@ -379,9 +379,40 @@ func testCalculateConsumedCapacity(tCtx ktesting.TContext) { fractionalCapacityRange bool expectResult resource.Quantity expectErr bool + expectErrIs error // errCapacityRequestNotRepresentable if nil expectErrMessage string }{ "empty": {requestedVal: nil, capacityValue: one, requestPolicy: &resourceapi.CapacityRequestPolicy{}, expectResult: one}, + "negative-request-without-policy-is-rejected": { + requestedVal: &negativeOne, + capacityValue: one, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, + // Rounding up to Min would turn the negative request into a valid one. + "negative-request-below-range-min-is-rejected": { + requestedVal: &negativeOne, + capacityValue: two, + requestPolicy: &resourceapi.CapacityRequestPolicy{Default: &one, ValidRange: &resourceapi.CapacityRequestPolicyRange{Min: &one}}, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, + "negative-request-below-fractional-range-min-is-rejected": { + requestedVal: &negativeOne, + capacityValue: one, + requestPolicy: &resourceapi.CapacityRequestPolicy{Default: &pointTwo, ValidRange: &resourceapi.CapacityRequestPolicyRange{Min: &pointTwo, Max: &one, Step: &pointOne}}, + fractionalCapacityRange: true, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, + // Rounding up to the first valid value would turn the negative request into a valid one. + "negative-request-below-valid-values-is-rejected": { + requestedVal: &negativeOne, + capacityValue: two, + requestPolicy: &resourceapi.CapacityRequestPolicy{Default: &one, ValidValues: []resource.Quantity{one, two}}, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, // A request above MaxInt64 cannot be read with Value() without wrapping, so // calculateConsumedCapacity returns a fatal error rather than acting on a // wrapped read (#140441). @@ -619,7 +650,11 @@ func testCalculateConsumedCapacity(tCtx ktesting.TContext) { } consumedCapacity, err := calculateConsumedCapacity(tc.requestedVal, capacity, tc.fractionalCapacityRange) if tc.expectErr { - tCtx.Require(err).To(g.MatchError(errCapacityRequestNotRepresentable)) + expectErrIs := tc.expectErrIs + if expectErrIs == nil { + expectErrIs = errCapacityRequestNotRepresentable + } + tCtx.Require(err).To(g.MatchError(expectErrIs)) if tc.expectErrMessage != "" { tCtx.Require(err).To(g.MatchError(g.ContainSubstring(tc.expectErrMessage))) } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity.go index a2231104598..903109f31e1 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity.go @@ -35,7 +35,7 @@ import ( // request is rejected outright instead of driving a device-by-device retry. var errCapacityRequestNotRepresentable = errors.New("capacity request cannot be represented in the device's range arithmetic") -// errNegativeCapacity signals that a resolved consumed capacity is negative. A negative +// errNegativeCapacity signals that a capacity request or a resolved consumed capacity is negative. A negative // value is not valid capacity and would under-count the device and over-allocate it. It // can reach the allocator from a stored ResourceSlice or ResourceClaim created before // stricter validation, which admission does not re-check, so the allocator rejects it @@ -112,7 +112,7 @@ func cmpRequestOverCapacity(currentConsumedCapacity ConsumedCapacity, requestedC } consumedCapacity, err := calculateConsumedCapacity(requestedValPtr, cap, fractionalCapacityRange) if err != nil { - // The request is not representable in this capacity's range arithmetic. + // The request is negative or not representable in this capacity's range arithmetic. // Surface a fatal error so the allocator aborts instead of retrying other // devices, which a user-controlled out-of-range request could abuse. return nil, false, fmt.Errorf("capacity %q: %w", name, err) @@ -270,6 +270,11 @@ func calculateConsumedCapacity(requestedVal *resource.Quantity, capacity resourc if requestedVal == nil { return fillEmptyRequest(capacity), nil } + // Must be checked before rounding: the request policy can round a negative + // request up to a valid value, which would hide it from the caller's check. + if requestedVal.Sign() < 0 { + return resource.Quantity{}, fmt.Errorf("%w: request %s", errNegativeCapacity, requestedVal.String()) + } if capacity.RequestPolicy == nil { return *requestedVal, nil } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity_test.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity_test.go index 4de28ea901f..e8c060c0c91 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/consumable_capacity_test.go @@ -379,9 +379,40 @@ func testCalculateConsumedCapacity(tCtx ktesting.TContext) { fractionalCapacityRange bool expectResult resource.Quantity expectErr bool + expectErrIs error // errCapacityRequestNotRepresentable if nil expectErrMessage string }{ "empty": {requestedVal: nil, capacityValue: one, requestPolicy: &resourceapi.CapacityRequestPolicy{}, expectResult: one}, + "negative-request-without-policy-is-rejected": { + requestedVal: &negativeOne, + capacityValue: one, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, + // Rounding up to Min would turn the negative request into a valid one. + "negative-request-below-range-min-is-rejected": { + requestedVal: &negativeOne, + capacityValue: two, + requestPolicy: &resourceapi.CapacityRequestPolicy{Default: &one, ValidRange: &resourceapi.CapacityRequestPolicyRange{Min: &one}}, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, + "negative-request-below-fractional-range-min-is-rejected": { + requestedVal: &negativeOne, + capacityValue: one, + requestPolicy: &resourceapi.CapacityRequestPolicy{Default: &pointTwo, ValidRange: &resourceapi.CapacityRequestPolicyRange{Min: &pointTwo, Max: &one, Step: &pointOne}}, + fractionalCapacityRange: true, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, + // Rounding up to the first valid value would turn the negative request into a valid one. + "negative-request-below-valid-values-is-rejected": { + requestedVal: &negativeOne, + capacityValue: two, + requestPolicy: &resourceapi.CapacityRequestPolicy{Default: &one, ValidValues: []resource.Quantity{one, two}}, + expectErr: true, + expectErrIs: errNegativeCapacity, + }, // A request above MaxInt64 cannot be read with Value() without wrapping, so // calculateConsumedCapacity returns a fatal error rather than acting on a // wrapped read (#140441). @@ -619,7 +650,11 @@ func testCalculateConsumedCapacity(tCtx ktesting.TContext) { } consumedCapacity, err := calculateConsumedCapacity(tc.requestedVal, capacity, tc.fractionalCapacityRange) if tc.expectErr { - tCtx.Require(err).To(g.MatchError(errCapacityRequestNotRepresentable)) + expectErrIs := tc.expectErrIs + if expectErrIs == nil { + expectErrIs = errCapacityRequestNotRepresentable + } + tCtx.Require(err).To(g.MatchError(expectErrIs)) if tc.expectErrMessage != "" { tCtx.Require(err).To(g.MatchError(g.ContainSubstring(tc.expectErrMessage))) } diff --git a/test/declarative_validation/resource/resourceclaim/declarative_validation_test.go b/test/declarative_validation/resource/resourceclaim/declarative_validation_test.go index 3befaaf1cfc..8e859027447 100644 --- a/test/declarative_validation/resource/resourceclaim/declarative_validation_test.go +++ b/test/declarative_validation/resource/resourceclaim/declarative_validation_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/google/uuid" + apiresource "k8s.io/apimachinery/pkg/api/resource" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -594,6 +595,39 @@ func testDeclarativeValidate(t *testing.T, apiVersion string) { "valid firstAvailable derived attributes, max allowed": { input: mkValidResourceClaim(tweakFirstAvailableDerivedAttributesCount(32)), }, + "valid exactly capacity requests, max allowed": { + input: mkValidResourceClaim(tweakExactlyCapacityRequests(mkCapacityRequests(32, "1"))), + }, + "valid exactly capacity requests, zero": { + input: mkValidResourceClaim(tweakExactlyCapacityRequests(mkCapacityRequests(1, "0"))), + }, + "invalid exactly capacity requests, too many": { + input: mkValidResourceClaim(tweakExactlyCapacityRequests(mkCapacityRequests(33, "1"))), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests").Index(0).Child("exactly", "capacity", "requests"), 33, 32).WithOrigin("maxProperties"), + }, + }, + "invalid exactly capacity requests, negative": { + input: mkValidResourceClaim(tweakExactlyCapacityRequests(mkCapacityRequests(1, "-1"))), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("exactly", "capacity", "requests").Key("dra.example.com/cap0"), "-1", "").WithOrigin("minimum"), + }, + }, + "valid firstAvailable capacity requests, max allowed": { + input: mkValidResourceClaim(tweakFirstAvailableCapacityRequests(mkCapacityRequests(32, "1"))), + }, + "invalid firstAvailable capacity requests, too many": { + input: mkValidResourceClaim(tweakFirstAvailableCapacityRequests(mkCapacityRequests(33, "1"))), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("capacity", "requests"), 33, 32).WithOrigin("maxProperties"), + }, + }, + "invalid firstAvailable capacity requests, negative": { + input: mkValidResourceClaim(tweakFirstAvailableCapacityRequests(mkCapacityRequests(1, "-1"))), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("capacity", "requests").Key("dra.example.com/cap0"), "-1", "").WithOrigin("minimum"), + }, + }, // TODO: Add more test cases } for k, tc := range testCases { @@ -744,6 +778,33 @@ func tweakFirstAvailable(items int) func(*resource.ResourceClaim) { } } +func mkCapacityRequests(count int, value string) map[resource.QualifiedName]apiresource.Quantity { + requests := make(map[resource.QualifiedName]apiresource.Quantity, count) + for i := range count { + requests[resource.QualifiedName(fmt.Sprintf("dra.example.com/cap%d", i))] = apiresource.MustParse(value) + } + return requests +} + +func tweakExactlyCapacityRequests(requests map[resource.QualifiedName]apiresource.Quantity) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + rc.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{Requests: requests} + } +} + +func tweakFirstAvailableCapacityRequests(requests map[resource.QualifiedName]apiresource.Quantity) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + tweakFirstAvailable(1)(rc) + rc.Spec.Devices.Requests[0].FirstAvailable[0].Capacity = &resource.CapacityRequirements{Requests: requests} + } +} + +func tweakLabels(labels map[string]string) func(*resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + rc.Labels = labels + } +} + func tweakExactlyAllocationMode(mode resource.DeviceAllocationMode, count int64) func(*resource.ResourceClaim) { return func(rc *resource.ResourceClaim) { if len(rc.Spec.Devices.Requests) > 0 && rc.Spec.Devices.Requests[0].Exactly != nil { @@ -832,6 +893,10 @@ func testDeclarativeValidateUpdate(t *testing.T, apiVersion string) { update: validClaim, old: validClaim, }, + "valid update with unchanged negative capacity requests": { + update: mkValidResourceClaim(tweakExactlyCapacityRequests(mkCapacityRequests(1, "-1")), tweakLabels(map[string]string{"foo": "bar"})), + old: mkValidResourceClaim(tweakExactlyCapacityRequests(mkCapacityRequests(1, "-1"))), + }, "spec immutable: modify request class name": { update: mkValidResourceClaim(tweakSpecChangeClassName("another-class")), old: validClaim, @@ -1170,6 +1235,32 @@ func testDeclarativeValidateStatusUpdate(t *testing.T, apiVersion string) { old: mkResourceClaimWithStatus(), update: mkValidResourceClaim(), }, + "valid status.allocation.devices.results[].consumedCapacity, max allowed": { + old: mkValidResourceClaim(), + update: mkResourceClaimWithStatus(tweakStatusConsumedCapacity(mkCapacityRequests(32, "1"))), + }, + "valid status.allocation.devices.results[].consumedCapacity, zero": { + old: mkValidResourceClaim(), + update: mkResourceClaimWithStatus(tweakStatusConsumedCapacity(mkCapacityRequests(1, "0"))), + }, + "valid status.allocation.devices.results[].consumedCapacity, negative but unchanged": { + old: mkResourceClaimWithStatus(tweakStatusConsumedCapacity(mkCapacityRequests(1, "-1"))), + update: mkResourceClaimWithStatus(tweakStatusConsumedCapacity(mkCapacityRequests(1, "-1"))), + }, + "invalid status.allocation.devices.results[].consumedCapacity, too many": { + old: mkValidResourceClaim(), + update: mkResourceClaimWithStatus(tweakStatusConsumedCapacity(mkCapacityRequests(33, "1"))), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("consumedCapacity"), 33, 32).WithOrigin("maxProperties"), + }, + }, + "invalid status.allocation.devices.results[].consumedCapacity, negative": { + old: mkValidResourceClaim(), + update: mkResourceClaimWithStatus(tweakStatusConsumedCapacity(mkCapacityRequests(1, "-1"))), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("consumedCapacity").Key("dra.example.com/cap0"), "-1", "").WithOrigin("minimum"), + }, + }, "invalid status.allocation changed device (NoModify)": { old: mkResourceClaimWithStatus(), update: tweakStatusAllocationDevice(mkResourceClaimWithStatus(), "device-different"), @@ -1734,6 +1825,14 @@ func tweakStatusDeviceRequestAllocationResultDriver(driver string) func(rc *reso } } +func tweakStatusConsumedCapacity(consumed map[resource.QualifiedName]apiresource.Quantity) func(rc *resource.ResourceClaim) { + return func(rc *resource.ResourceClaim) { + for i := range rc.Status.Allocation.Devices.Results { + rc.Status.Allocation.Devices.Results[i].ConsumedCapacity = consumed + } + } +} + func tweakStatusDeviceRequestAllocationResultShareID(shareID types.UID) func(rc *resource.ResourceClaim) { return func(rc *resource.ResourceClaim) { for i := range rc.Status.Allocation.Devices.Results { diff --git a/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1_test.go b/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1_test.go index e32bccf0042..79ccabac4c2 100644 --- a/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1_test.go +++ b/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1_test.go @@ -109,6 +109,12 @@ func init() { "spec.devices.requests[*].exactly.allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.devices.requests[*].exactly.capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.devices.requests[*].exactly.capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.devices.requests[*].exactly.derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -142,6 +148,12 @@ func init() { "spec.devices.requests[*].firstAvailable[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.devices.requests[*].firstAvailable[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.devices.requests[*].firstAvailable[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.devices.requests[*].firstAvailable[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -200,6 +212,12 @@ func init() { "status.allocation.devices.results[*].bindingFailureConditions": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, + "status.allocation.devices.results[*].consumedCapacity": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "status.allocation.devices.results[*].consumedCapacity[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "status.allocation.devices.results[*].driver": { {ErrorType: "FieldValueInvalid", Origin: "format=k8s-long-name-caseless"}, {ErrorType: "FieldValueRequired"}, diff --git a/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta1_test.go b/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta1_test.go index 86877748ba6..31760275739 100644 --- a/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta1_test.go +++ b/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta1_test.go @@ -109,6 +109,12 @@ func init() { "spec.devices.requests[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.devices.requests[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.devices.requests[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.devices.requests[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -129,6 +135,12 @@ func init() { "spec.devices.requests[*].firstAvailable[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.devices.requests[*].firstAvailable[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.devices.requests[*].firstAvailable[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.devices.requests[*].firstAvailable[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -200,6 +212,12 @@ func init() { "status.allocation.devices.results[*].bindingFailureConditions": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, + "status.allocation.devices.results[*].consumedCapacity": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "status.allocation.devices.results[*].consumedCapacity[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "status.allocation.devices.results[*].driver": { {ErrorType: "FieldValueInvalid", Origin: "format=k8s-long-name-caseless"}, {ErrorType: "FieldValueRequired"}, diff --git a/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta2_test.go b/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta2_test.go index cd71ef35914..f70f5b43686 100644 --- a/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta2_test.go +++ b/test/declarative_validation/resource/resourceclaim/zz_generated.validations.v1beta2_test.go @@ -109,6 +109,12 @@ func init() { "spec.devices.requests[*].exactly.allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.devices.requests[*].exactly.capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.devices.requests[*].exactly.capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.devices.requests[*].exactly.derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -142,6 +148,12 @@ func init() { "spec.devices.requests[*].firstAvailable[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.devices.requests[*].firstAvailable[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.devices.requests[*].firstAvailable[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.devices.requests[*].firstAvailable[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -200,6 +212,12 @@ func init() { "status.allocation.devices.results[*].bindingFailureConditions": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, + "status.allocation.devices.results[*].consumedCapacity": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "status.allocation.devices.results[*].consumedCapacity[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "status.allocation.devices.results[*].driver": { {ErrorType: "FieldValueInvalid", Origin: "format=k8s-long-name-caseless"}, {ErrorType: "FieldValueRequired"}, diff --git a/test/declarative_validation/resource/resourceclaimtemplate/declarative_validation_test.go b/test/declarative_validation/resource/resourceclaimtemplate/declarative_validation_test.go index b1d15a41af9..30fa23ec6e6 100644 --- a/test/declarative_validation/resource/resourceclaimtemplate/declarative_validation_test.go +++ b/test/declarative_validation/resource/resourceclaimtemplate/declarative_validation_test.go @@ -21,6 +21,7 @@ import ( "strings" "testing" + apiresource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" @@ -500,6 +501,39 @@ func testDeclarativeValidate(t *testing.T, apiVersion string) { "valid firstAvailable derived attributes, max allowed": { input: mkValidResourceClaimTemplate(tweakFirstAvailableDerivedAttributesCount(32)), }, + "valid exactly capacity requests, max allowed": { + input: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(mkCapacityRequests(32, "1"))), + }, + "valid exactly capacity requests, zero": { + input: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(mkCapacityRequests(1, "0"))), + }, + "invalid exactly capacity requests, too many": { + input: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(mkCapacityRequests(33, "1"))), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("exactly", "capacity", "requests"), 33, 32).WithOrigin("maxProperties"), + }, + }, + "invalid exactly capacity requests, negative": { + input: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(mkCapacityRequests(1, "-1"))), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("exactly", "capacity", "requests").Key("dra.example.com/cap0"), "-1", "").WithOrigin("minimum"), + }, + }, + "valid firstAvailable capacity requests, max allowed": { + input: mkValidResourceClaimTemplate(tweakFirstAvailableCapacityRequests(mkCapacityRequests(32, "1"))), + }, + "invalid firstAvailable capacity requests, too many": { + input: mkValidResourceClaimTemplate(tweakFirstAvailableCapacityRequests(mkCapacityRequests(33, "1"))), + expectedErrs: field.ErrorList{ + field.TooMany(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("capacity", "requests"), 33, 32).WithOrigin("maxProperties"), + }, + }, + "invalid firstAvailable capacity requests, negative": { + input: mkValidResourceClaimTemplate(tweakFirstAvailableCapacityRequests(mkCapacityRequests(1, "-1"))), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("spec", "spec", "devices", "requests").Index(0).Child("firstAvailable").Index(0).Child("capacity", "requests").Key("dra.example.com/cap0"), "-1", "").WithOrigin("minimum"), + }, + }, // TODO: Add more test cases } @@ -542,6 +576,18 @@ func testDeclarativeValidateUpdate(t *testing.T, apiVersion string) { old: mkValidResourceClaimTemplate(), update: mkValidResourceClaimTemplate(), }, + "valid update with unchanged negative capacity requests": { + old: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(mkCapacityRequests(1, "-1"))), + update: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(mkCapacityRequests(1, "-1")), tweakLabels(map[string]string{"foo": "bar"})), + }, + "valid update with unchanged invalid capacity request name": { + old: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(invalidCapacityRequestName)), + update: mkValidResourceClaimTemplate(tweakExactlyCapacityRequests(invalidCapacityRequestName), tweakLabels(map[string]string{"foo": "bar"})), + }, + "valid update with unchanged invalid capacity request name in subrequest": { + old: mkValidResourceClaimTemplate(tweakFirstAvailableCapacityRequests(invalidCapacityRequestName)), + update: mkValidResourceClaimTemplate(tweakFirstAvailableCapacityRequests(invalidCapacityRequestName), tweakLabels(map[string]string{"foo": "bar"})), + }, // TODO: Add more test cases } @@ -748,6 +794,38 @@ func tweakDuplicateConfigRequest(name string) func(*resource.ResourceClaimTempla } } +// invalidCapacityRequestName could be stored before capacity request names were validated. +var invalidCapacityRequestName = map[resource.QualifiedName]apiresource.Quantity{ + "test-capacity": apiresource.MustParse("1"), +} + +func mkCapacityRequests(count int, value string) map[resource.QualifiedName]apiresource.Quantity { + requests := make(map[resource.QualifiedName]apiresource.Quantity, count) + for i := range count { + requests[resource.QualifiedName(fmt.Sprintf("dra.example.com/cap%d", i))] = apiresource.MustParse(value) + } + return requests +} + +func tweakExactlyCapacityRequests(requests map[resource.QualifiedName]apiresource.Quantity) func(*resource.ResourceClaimTemplate) { + return func(rct *resource.ResourceClaimTemplate) { + rct.Spec.Spec.Devices.Requests[0].Exactly.Capacity = &resource.CapacityRequirements{Requests: requests} + } +} + +func tweakFirstAvailableCapacityRequests(requests map[resource.QualifiedName]apiresource.Quantity) func(*resource.ResourceClaimTemplate) { + return func(rct *resource.ResourceClaimTemplate) { + tweakFirstAvailable(1)(rct) + rct.Spec.Spec.Devices.Requests[0].FirstAvailable[0].Capacity = &resource.CapacityRequirements{Requests: requests} + } +} + +func tweakLabels(labels map[string]string) func(*resource.ResourceClaimTemplate) { + return func(rct *resource.ResourceClaimTemplate) { + rct.Labels = labels + } +} + func tweakExactlyAllocationMode(mode resource.DeviceAllocationMode, count int64) func(*resource.ResourceClaimTemplate) { return func(rct *resource.ResourceClaimTemplate) { if len(rct.Spec.Spec.Devices.Requests) > 0 && rct.Spec.Spec.Devices.Requests[0].Exactly != nil { diff --git a/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1_test.go b/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1_test.go index 973b1e2823f..486027070b3 100644 --- a/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1_test.go +++ b/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1_test.go @@ -106,6 +106,12 @@ func init() { "spec.spec.devices.requests[*].exactly.allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.spec.devices.requests[*].exactly.capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.spec.devices.requests[*].exactly.capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.spec.devices.requests[*].exactly.derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -139,6 +145,12 @@ func init() { "spec.spec.devices.requests[*].firstAvailable[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.spec.devices.requests[*].firstAvailable[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.spec.devices.requests[*].firstAvailable[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.spec.devices.requests[*].firstAvailable[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, diff --git a/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta1_test.go b/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta1_test.go index 3d87f33c085..cca91589d42 100644 --- a/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta1_test.go +++ b/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta1_test.go @@ -106,6 +106,12 @@ func init() { "spec.spec.devices.requests[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.spec.devices.requests[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.spec.devices.requests[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.spec.devices.requests[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -126,6 +132,12 @@ func init() { "spec.spec.devices.requests[*].firstAvailable[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.spec.devices.requests[*].firstAvailable[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.spec.devices.requests[*].firstAvailable[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.spec.devices.requests[*].firstAvailable[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, diff --git a/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta2_test.go b/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta2_test.go index 69ede70d5ef..88f70063472 100644 --- a/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta2_test.go +++ b/test/declarative_validation/resource/resourceclaimtemplate/zz_generated.validations.v1beta2_test.go @@ -106,6 +106,12 @@ func init() { "spec.spec.devices.requests[*].exactly.allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.spec.devices.requests[*].exactly.capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.spec.devices.requests[*].exactly.capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.spec.devices.requests[*].exactly.derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, }, @@ -139,6 +145,12 @@ func init() { "spec.spec.devices.requests[*].firstAvailable[*].allocationMode": { {ErrorType: "FieldValueNotSupported"}, }, + "spec.spec.devices.requests[*].firstAvailable[*].capacity.requests": { + {ErrorType: "FieldValueTooMany", Origin: "maxProperties"}, + }, + "spec.spec.devices.requests[*].firstAvailable[*].capacity.requests[*]": { + {ErrorType: "FieldValueInvalid", Origin: "minimum"}, + }, "spec.spec.devices.requests[*].firstAvailable[*].derivedAttributes": { {ErrorType: "FieldValueTooMany", Origin: "maxItems"}, },