From f64797580a8462ede9b43b62f2e5b4974028b55d Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 24 Oct 2025 21:19:33 +0200 Subject: [PATCH 1/2] DRA internal API: remove unnecessary types The purpose of the internal API is to replace certain strings with UniqueString where it makes sense for performance. We don't need to duplicated types which don't contain such a change. Also, some types were already not used anywhere (dead code). This removal gets rid of some conversion code and simplifies the consumable capacity implementation. --- .../dynamic-resource-allocation/api/types.go | 56 +---- .../api/zz_generated.conversion.go | 211 +----------------- .../experimental/allocator_experimental.go | 34 +-- .../internal/experimental/constraint.go | 7 +- .../experimental/consumable_capacity.go | 33 +-- .../incubating/allocator_incubating.go | 18 +- .../internal/stable/allocator_stable.go | 18 +- 7 files changed, 54 insertions(+), 323 deletions(-) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/api/types.go b/staging/src/k8s.io/dynamic-resource-allocation/api/types.go index b73cf0a53fa..183fd289164 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/api/types.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/api/types.go @@ -19,7 +19,6 @@ package api import ( v1 "k8s.io/api/core/v1" resourceapi "k8s.io/api/resource/v1" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -42,7 +41,7 @@ type ResourceSliceSpec struct { type CounterSet struct { Name UniqueString - Counters map[string]Counter + Counters map[string]resourceapi.Counter } type ResourcePool struct { @@ -52,8 +51,8 @@ type ResourcePool struct { } type Device struct { Name UniqueString - Attributes map[QualifiedName]DeviceAttribute - Capacity map[QualifiedName]DeviceCapacity + Attributes map[resourceapi.QualifiedName]resourceapi.DeviceAttribute + Capacity map[resourceapi.QualifiedName]resourceapi.DeviceCapacity ConsumesCounters []DeviceCounterConsumption NodeName *string NodeSelector *v1.NodeSelector @@ -67,52 +66,5 @@ type Device struct { type DeviceCounterConsumption struct { CounterSet UniqueString - Counters map[string]Counter + Counters map[string]resourceapi.Counter } - -type QualifiedName string - -type FullyQualifiedName string - -type DeviceAttribute struct { - IntValue *int64 - BoolValue *bool - StringValue *string - VersionValue *string -} - -type DeviceCapacity struct { - Value resource.Quantity - RequestPolicy *CapacityRequestPolicy -} - -type CapacityRequestPolicy struct { - Default *resource.Quantity - ValidValues []resource.Quantity - ValidRange *CapacityRequestPolicyRange -} - -type CapacityRequestPolicyRange struct { - Min *resource.Quantity - Max *resource.Quantity - Step *resource.Quantity -} - -type Counter struct { - Value resource.Quantity -} - -type DeviceTaint struct { - Key string - Value string - Effect DeviceTaintEffect - TimeAdded *metav1.Time -} - -type DeviceTaintEffect string - -const ( - DeviceTaintEffectNoSchedule DeviceTaintEffect = "NoSchedule" - - DeviceTaintEffectNoExecute DeviceTaintEffect = "NoExecute" -) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/api/zz_generated.conversion.go b/staging/src/k8s.io/dynamic-resource-allocation/api/zz_generated.conversion.go index 7c356fdf517..cac65c36c31 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/api/zz_generated.conversion.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/api/zz_generated.conversion.go @@ -26,7 +26,6 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/resource/v1" - resource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" conversion "k8s.io/apimachinery/pkg/conversion" runtime "k8s.io/apimachinery/pkg/runtime" @@ -39,36 +38,6 @@ func init() { // RegisterConversions adds conversion functions to the given scheme. // Public to allow building arbitrary schemes. func RegisterConversions(s *runtime.Scheme) error { - if err := s.AddGeneratedConversionFunc((*CapacityRequestPolicy)(nil), (*v1.CapacityRequestPolicy)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_api_CapacityRequestPolicy_To_v1_CapacityRequestPolicy(a.(*CapacityRequestPolicy), b.(*v1.CapacityRequestPolicy), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1.CapacityRequestPolicy)(nil), (*CapacityRequestPolicy)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1_CapacityRequestPolicy_To_api_CapacityRequestPolicy(a.(*v1.CapacityRequestPolicy), b.(*CapacityRequestPolicy), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*CapacityRequestPolicyRange)(nil), (*v1.CapacityRequestPolicyRange)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_api_CapacityRequestPolicyRange_To_v1_CapacityRequestPolicyRange(a.(*CapacityRequestPolicyRange), b.(*v1.CapacityRequestPolicyRange), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1.CapacityRequestPolicyRange)(nil), (*CapacityRequestPolicyRange)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1_CapacityRequestPolicyRange_To_api_CapacityRequestPolicyRange(a.(*v1.CapacityRequestPolicyRange), b.(*CapacityRequestPolicyRange), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*Counter)(nil), (*v1.Counter)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_api_Counter_To_v1_Counter(a.(*Counter), b.(*v1.Counter), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1.Counter)(nil), (*Counter)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1_Counter_To_api_Counter(a.(*v1.Counter), b.(*Counter), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*CounterSet)(nil), (*v1.CounterSet)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_api_CounterSet_To_v1_CounterSet(a.(*CounterSet), b.(*v1.CounterSet), scope) }); err != nil { @@ -89,26 +58,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*DeviceAttribute)(nil), (*v1.DeviceAttribute)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_api_DeviceAttribute_To_v1_DeviceAttribute(a.(*DeviceAttribute), b.(*v1.DeviceAttribute), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1.DeviceAttribute)(nil), (*DeviceAttribute)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1_DeviceAttribute_To_api_DeviceAttribute(a.(*v1.DeviceAttribute), b.(*DeviceAttribute), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*DeviceCapacity)(nil), (*v1.DeviceCapacity)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_api_DeviceCapacity_To_v1_DeviceCapacity(a.(*DeviceCapacity), b.(*v1.DeviceCapacity), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1.DeviceCapacity)(nil), (*DeviceCapacity)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1_DeviceCapacity_To_api_DeviceCapacity(a.(*v1.DeviceCapacity), b.(*DeviceCapacity), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*DeviceCounterConsumption)(nil), (*v1.DeviceCounterConsumption)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_api_DeviceCounterConsumption_To_v1_DeviceCounterConsumption(a.(*DeviceCounterConsumption), b.(*v1.DeviceCounterConsumption), scope) }); err != nil { @@ -119,16 +68,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*DeviceTaint)(nil), (*v1.DeviceTaint)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_api_DeviceTaint_To_v1_DeviceTaint(a.(*DeviceTaint), b.(*v1.DeviceTaint), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1.DeviceTaint)(nil), (*DeviceTaint)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1_DeviceTaint_To_api_DeviceTaint(a.(*v1.DeviceTaint), b.(*DeviceTaint), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*ResourcePool)(nil), (*v1.ResourcePool)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_api_ResourcePool_To_v1_ResourcePool(a.(*ResourcePool), b.(*v1.ResourcePool), scope) }); err != nil { @@ -172,74 +111,6 @@ func RegisterConversions(s *runtime.Scheme) error { return nil } -func autoConvert_api_CapacityRequestPolicy_To_v1_CapacityRequestPolicy(in *CapacityRequestPolicy, out *v1.CapacityRequestPolicy, s conversion.Scope) error { - out.Default = (*resource.Quantity)(unsafe.Pointer(in.Default)) - out.ValidValues = *(*[]resource.Quantity)(unsafe.Pointer(&in.ValidValues)) - out.ValidRange = (*v1.CapacityRequestPolicyRange)(unsafe.Pointer(in.ValidRange)) - return nil -} - -// Convert_api_CapacityRequestPolicy_To_v1_CapacityRequestPolicy is an autogenerated conversion function. -func Convert_api_CapacityRequestPolicy_To_v1_CapacityRequestPolicy(in *CapacityRequestPolicy, out *v1.CapacityRequestPolicy, s conversion.Scope) error { - return autoConvert_api_CapacityRequestPolicy_To_v1_CapacityRequestPolicy(in, out, s) -} - -func autoConvert_v1_CapacityRequestPolicy_To_api_CapacityRequestPolicy(in *v1.CapacityRequestPolicy, out *CapacityRequestPolicy, s conversion.Scope) error { - out.Default = (*resource.Quantity)(unsafe.Pointer(in.Default)) - out.ValidValues = *(*[]resource.Quantity)(unsafe.Pointer(&in.ValidValues)) - out.ValidRange = (*CapacityRequestPolicyRange)(unsafe.Pointer(in.ValidRange)) - return nil -} - -// Convert_v1_CapacityRequestPolicy_To_api_CapacityRequestPolicy is an autogenerated conversion function. -func Convert_v1_CapacityRequestPolicy_To_api_CapacityRequestPolicy(in *v1.CapacityRequestPolicy, out *CapacityRequestPolicy, s conversion.Scope) error { - return autoConvert_v1_CapacityRequestPolicy_To_api_CapacityRequestPolicy(in, out, s) -} - -func autoConvert_api_CapacityRequestPolicyRange_To_v1_CapacityRequestPolicyRange(in *CapacityRequestPolicyRange, out *v1.CapacityRequestPolicyRange, s conversion.Scope) error { - out.Min = (*resource.Quantity)(unsafe.Pointer(in.Min)) - out.Max = (*resource.Quantity)(unsafe.Pointer(in.Max)) - out.Step = (*resource.Quantity)(unsafe.Pointer(in.Step)) - return nil -} - -// Convert_api_CapacityRequestPolicyRange_To_v1_CapacityRequestPolicyRange is an autogenerated conversion function. -func Convert_api_CapacityRequestPolicyRange_To_v1_CapacityRequestPolicyRange(in *CapacityRequestPolicyRange, out *v1.CapacityRequestPolicyRange, s conversion.Scope) error { - return autoConvert_api_CapacityRequestPolicyRange_To_v1_CapacityRequestPolicyRange(in, out, s) -} - -func autoConvert_v1_CapacityRequestPolicyRange_To_api_CapacityRequestPolicyRange(in *v1.CapacityRequestPolicyRange, out *CapacityRequestPolicyRange, s conversion.Scope) error { - out.Min = (*resource.Quantity)(unsafe.Pointer(in.Min)) - out.Max = (*resource.Quantity)(unsafe.Pointer(in.Max)) - out.Step = (*resource.Quantity)(unsafe.Pointer(in.Step)) - return nil -} - -// Convert_v1_CapacityRequestPolicyRange_To_api_CapacityRequestPolicyRange is an autogenerated conversion function. -func Convert_v1_CapacityRequestPolicyRange_To_api_CapacityRequestPolicyRange(in *v1.CapacityRequestPolicyRange, out *CapacityRequestPolicyRange, s conversion.Scope) error { - return autoConvert_v1_CapacityRequestPolicyRange_To_api_CapacityRequestPolicyRange(in, out, s) -} - -func autoConvert_api_Counter_To_v1_Counter(in *Counter, out *v1.Counter, s conversion.Scope) error { - out.Value = in.Value - return nil -} - -// Convert_api_Counter_To_v1_Counter is an autogenerated conversion function. -func Convert_api_Counter_To_v1_Counter(in *Counter, out *v1.Counter, s conversion.Scope) error { - return autoConvert_api_Counter_To_v1_Counter(in, out, s) -} - -func autoConvert_v1_Counter_To_api_Counter(in *v1.Counter, out *Counter, s conversion.Scope) error { - out.Value = in.Value - return nil -} - -// Convert_v1_Counter_To_api_Counter is an autogenerated conversion function. -func Convert_v1_Counter_To_api_Counter(in *v1.Counter, out *Counter, s conversion.Scope) error { - return autoConvert_v1_Counter_To_api_Counter(in, out, s) -} - func autoConvert_api_CounterSet_To_v1_CounterSet(in *CounterSet, out *v1.CounterSet, s conversion.Scope) error { if err := Convert_api_UniqueString_To_string(&in.Name, &out.Name, s); err != nil { return err @@ -257,7 +128,7 @@ func autoConvert_v1_CounterSet_To_api_CounterSet(in *v1.CounterSet, out *Counter if err := Convert_string_To_api_UniqueString(&in.Name, &out.Name, s); err != nil { return err } - out.Counters = *(*map[string]Counter)(unsafe.Pointer(&in.Counters)) + out.Counters = *(*map[string]v1.Counter)(unsafe.Pointer(&in.Counters)) return nil } @@ -305,8 +176,8 @@ func autoConvert_v1_Device_To_api_Device(in *v1.Device, out *Device, s conversio if err := Convert_string_To_api_UniqueString(&in.Name, &out.Name, s); err != nil { return err } - out.Attributes = *(*map[QualifiedName]DeviceAttribute)(unsafe.Pointer(&in.Attributes)) - out.Capacity = *(*map[QualifiedName]DeviceCapacity)(unsafe.Pointer(&in.Capacity)) + out.Attributes = *(*map[v1.QualifiedName]v1.DeviceAttribute)(unsafe.Pointer(&in.Attributes)) + out.Capacity = *(*map[v1.QualifiedName]v1.DeviceCapacity)(unsafe.Pointer(&in.Capacity)) if in.ConsumesCounters != nil { in, out := &in.ConsumesCounters, &out.ConsumesCounters *out = make([]DeviceCounterConsumption, len(*in)) @@ -336,54 +207,6 @@ func Convert_v1_Device_To_api_Device(in *v1.Device, out *Device, s conversion.Sc return autoConvert_v1_Device_To_api_Device(in, out, s) } -func autoConvert_api_DeviceAttribute_To_v1_DeviceAttribute(in *DeviceAttribute, out *v1.DeviceAttribute, s conversion.Scope) error { - out.IntValue = (*int64)(unsafe.Pointer(in.IntValue)) - out.BoolValue = (*bool)(unsafe.Pointer(in.BoolValue)) - out.StringValue = (*string)(unsafe.Pointer(in.StringValue)) - out.VersionValue = (*string)(unsafe.Pointer(in.VersionValue)) - return nil -} - -// Convert_api_DeviceAttribute_To_v1_DeviceAttribute is an autogenerated conversion function. -func Convert_api_DeviceAttribute_To_v1_DeviceAttribute(in *DeviceAttribute, out *v1.DeviceAttribute, s conversion.Scope) error { - return autoConvert_api_DeviceAttribute_To_v1_DeviceAttribute(in, out, s) -} - -func autoConvert_v1_DeviceAttribute_To_api_DeviceAttribute(in *v1.DeviceAttribute, out *DeviceAttribute, s conversion.Scope) error { - out.IntValue = (*int64)(unsafe.Pointer(in.IntValue)) - out.BoolValue = (*bool)(unsafe.Pointer(in.BoolValue)) - out.StringValue = (*string)(unsafe.Pointer(in.StringValue)) - out.VersionValue = (*string)(unsafe.Pointer(in.VersionValue)) - return nil -} - -// Convert_v1_DeviceAttribute_To_api_DeviceAttribute is an autogenerated conversion function. -func Convert_v1_DeviceAttribute_To_api_DeviceAttribute(in *v1.DeviceAttribute, out *DeviceAttribute, s conversion.Scope) error { - return autoConvert_v1_DeviceAttribute_To_api_DeviceAttribute(in, out, s) -} - -func autoConvert_api_DeviceCapacity_To_v1_DeviceCapacity(in *DeviceCapacity, out *v1.DeviceCapacity, s conversion.Scope) error { - out.Value = in.Value - out.RequestPolicy = (*v1.CapacityRequestPolicy)(unsafe.Pointer(in.RequestPolicy)) - return nil -} - -// Convert_api_DeviceCapacity_To_v1_DeviceCapacity is an autogenerated conversion function. -func Convert_api_DeviceCapacity_To_v1_DeviceCapacity(in *DeviceCapacity, out *v1.DeviceCapacity, s conversion.Scope) error { - return autoConvert_api_DeviceCapacity_To_v1_DeviceCapacity(in, out, s) -} - -func autoConvert_v1_DeviceCapacity_To_api_DeviceCapacity(in *v1.DeviceCapacity, out *DeviceCapacity, s conversion.Scope) error { - out.Value = in.Value - out.RequestPolicy = (*CapacityRequestPolicy)(unsafe.Pointer(in.RequestPolicy)) - return nil -} - -// Convert_v1_DeviceCapacity_To_api_DeviceCapacity is an autogenerated conversion function. -func Convert_v1_DeviceCapacity_To_api_DeviceCapacity(in *v1.DeviceCapacity, out *DeviceCapacity, s conversion.Scope) error { - return autoConvert_v1_DeviceCapacity_To_api_DeviceCapacity(in, out, s) -} - func autoConvert_api_DeviceCounterConsumption_To_v1_DeviceCounterConsumption(in *DeviceCounterConsumption, out *v1.DeviceCounterConsumption, s conversion.Scope) error { if err := Convert_api_UniqueString_To_string(&in.CounterSet, &out.CounterSet, s); err != nil { return err @@ -401,7 +224,7 @@ func autoConvert_v1_DeviceCounterConsumption_To_api_DeviceCounterConsumption(in if err := Convert_string_To_api_UniqueString(&in.CounterSet, &out.CounterSet, s); err != nil { return err } - out.Counters = *(*map[string]Counter)(unsafe.Pointer(&in.Counters)) + out.Counters = *(*map[string]v1.Counter)(unsafe.Pointer(&in.Counters)) return nil } @@ -410,32 +233,6 @@ func Convert_v1_DeviceCounterConsumption_To_api_DeviceCounterConsumption(in *v1. return autoConvert_v1_DeviceCounterConsumption_To_api_DeviceCounterConsumption(in, out, s) } -func autoConvert_api_DeviceTaint_To_v1_DeviceTaint(in *DeviceTaint, out *v1.DeviceTaint, s conversion.Scope) error { - out.Key = in.Key - out.Value = in.Value - out.Effect = v1.DeviceTaintEffect(in.Effect) - out.TimeAdded = (*metav1.Time)(unsafe.Pointer(in.TimeAdded)) - return nil -} - -// Convert_api_DeviceTaint_To_v1_DeviceTaint is an autogenerated conversion function. -func Convert_api_DeviceTaint_To_v1_DeviceTaint(in *DeviceTaint, out *v1.DeviceTaint, s conversion.Scope) error { - return autoConvert_api_DeviceTaint_To_v1_DeviceTaint(in, out, s) -} - -func autoConvert_v1_DeviceTaint_To_api_DeviceTaint(in *v1.DeviceTaint, out *DeviceTaint, s conversion.Scope) error { - out.Key = in.Key - out.Value = in.Value - out.Effect = DeviceTaintEffect(in.Effect) - out.TimeAdded = (*metav1.Time)(unsafe.Pointer(in.TimeAdded)) - return nil -} - -// Convert_v1_DeviceTaint_To_api_DeviceTaint is an autogenerated conversion function. -func Convert_v1_DeviceTaint_To_api_DeviceTaint(in *v1.DeviceTaint, out *DeviceTaint, s conversion.Scope) error { - return autoConvert_v1_DeviceTaint_To_api_DeviceTaint(in, out, s) -} - func autoConvert_api_ResourcePool_To_v1_ResourcePool(in *ResourcePool, out *v1.ResourcePool, s conversion.Scope) error { if err := Convert_api_UniqueString_To_string(&in.Name, &out.Name, s); err != nil { return err diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go index 1b3f3999d9f..cf8e0621f98 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/allocator_experimental.go @@ -277,7 +277,7 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node, claims []*resou for i, constraint := range claim.Spec.Devices.Constraints { switch { case constraint.MatchAttribute != nil: - matchAttribute := draapi.FullyQualifiedName(*constraint.MatchAttribute) + matchAttribute := resourceapi.FullyQualifiedName(*constraint.MatchAttribute) logger := alloc.logger if loggerV := alloc.logger.V(6); loggerV.Enabled() { logger = klog.LoggerWithName(logger, "matchAttributeConstraint") @@ -290,7 +290,7 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node, claims []*resou } constraints[i] = m case constraint.DistinctAttribute != nil: - distinctAttribute := draapi.FullyQualifiedName(*constraint.DistinctAttribute) + distinctAttribute := resourceapi.FullyQualifiedName(*constraint.DistinctAttribute) logger := alloc.logger if loggerV := alloc.logger.V(6); loggerV.Enabled() { logger = klog.LoggerWithName(logger, "distinctAttributeConstraint") @@ -300,7 +300,7 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node, claims []*resou logger: logger, requestNames: sets.New(constraint.Requests...), attributeName: distinctAttribute, - attributes: make(map[string]draapi.DeviceAttribute), + attributes: make(map[string]resourceapi.DeviceAttribute), } constraints[i] = m default: @@ -602,7 +602,7 @@ type allocator struct { // counterSets is a map with the name of counter sets to the counters in // the set. -type counterSets map[draapi.UniqueString]map[string]draapi.Counter +type counterSets map[draapi.UniqueString]map[string]resourceapi.Counter // matchKey identifies a device/request pair. type matchKey struct { @@ -721,9 +721,9 @@ type constraint interface { type matchAttributeConstraint struct { logger klog.Logger // Includes name and attribute name, so no need to repeat in log messages. requestNames sets.Set[string] - attributeName draapi.FullyQualifiedName + attributeName resourceapi.FullyQualifiedName - attribute *draapi.DeviceAttribute + attribute *resourceapi.DeviceAttribute numDevices int } @@ -803,9 +803,9 @@ func (m *matchAttributeConstraint) matches(requestName, subRequestName string) b } } -func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName draapi.FullyQualifiedName) *draapi.DeviceAttribute { +func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName resourceapi.FullyQualifiedName) *resourceapi.DeviceAttribute { // Fully-qualified match? - if attr, ok := device.Attributes[draapi.QualifiedName(attributeName)]; ok { + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName)]; ok { return &attr } index := strings.Index(string(attributeName), "/") @@ -821,7 +821,7 @@ func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName dra } // Domain matches the driver, so let's check just the ID. - if attr, ok := device.Attributes[draapi.QualifiedName(attributeName[index+1:])]; ok { + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName[index+1:])]; ok { return &attr } @@ -1311,19 +1311,9 @@ func (alloc *allocator) allocateDevice(r deviceIndices, device deviceWithID, mus } if allowMultipleAllocations { - convertedCapacities := make(map[resourceapi.QualifiedName]resourceapi.DeviceCapacity) - for key, value := range device.Capacity { - var convertedCapacity resourceapi.DeviceCapacity - err := draapi.Convert_api_DeviceCapacity_To_v1_DeviceCapacity(&value, &convertedCapacity, nil) - if err != nil { - return false, nil, fmt.Errorf("convert DeviceCapacity: %w", err) - } - convertedCapacities[resourceapi.QualifiedName(key)] = convertedCapacity - } - consumedCapacity = GetConsumedCapacityFromRequest(request.capacities(), convertedCapacities) + consumedCapacity = GetConsumedCapacityFromRequest(request.capacities(), device.Capacity) shareID = GenerateNewShareID() alloc.logger.V(7).Info("Device capacity allocated", "device", device.id, - "converted capacity", klog.Format(convertedCapacities), "consumed capacity", klog.Format(consumedCapacity)) alloc.allocatingCapacity.Insert(NewDeviceConsumedCapacity(device.id, consumedCapacity)) } @@ -1408,7 +1398,7 @@ func (alloc *allocator) checkAvailableCounters(device deviceWithID) (bool, error if !found { availableCountersForSlice = make(counterSets, len(slice.Spec.SharedCounters)) for _, counterSet := range slice.Spec.SharedCounters { - availableCountersForCounterSet := make(map[string]draapi.Counter, len(counterSet.Counters)) + availableCountersForCounterSet := make(map[string]resourceapi.Counter, len(counterSet.Counters)) for name, c := range counterSet.Counters { availableCountersForCounterSet[name] = c } @@ -1467,7 +1457,7 @@ func (alloc *allocator) checkAvailableCounters(device deviceWithID) (bool, error for _, deviceCounterConsumption := range device.ConsumesCounters { consumedCountersForCounterSet, found := consumedCountersForSlice[deviceCounterConsumption.CounterSet] if !found { - consumedCountersForCounterSet = make(map[string]draapi.Counter) + consumedCountersForCounterSet = make(map[string]resourceapi.Counter) consumedCountersForSlice[deviceCounterConsumption.CounterSet] = consumedCountersForCounterSet } for name, c := range deviceCounterConsumption.Counters { diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go index 5ca115e0ef4..94c44103478 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/experimental/constraint.go @@ -19,6 +19,7 @@ package experimental import ( "fmt" + resourceapi "k8s.io/api/resource/v1" "k8s.io/apimachinery/pkg/util/sets" draapi "k8s.io/dynamic-resource-allocation/api" "k8s.io/klog/v2" @@ -34,9 +35,9 @@ import ( type distinctAttributeConstraint struct { logger klog.Logger // Includes name and attribute name, so no need to repeat in log messages. requestNames sets.Set[string] - attributeName draapi.FullyQualifiedName + attributeName resourceapi.FullyQualifiedName - attributes map[string]draapi.DeviceAttribute + attributes map[string]resourceapi.DeviceAttribute numDevices int } @@ -91,7 +92,7 @@ func (m *distinctAttributeConstraint) matches(requestName, subRequestName string } } -func (m *distinctAttributeConstraint) matchesAttribute(attribute draapi.DeviceAttribute) bool { +func (m *distinctAttributeConstraint) matchesAttribute(attribute resourceapi.DeviceAttribute) bool { for _, attr := range m.attributes { switch { case attribute.StringValue != nil: 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 8b663fddeec..1df17186c27 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 @@ -18,51 +18,43 @@ package experimental import ( "errors" - "fmt" resourceapi "k8s.io/api/resource/v1" "k8s.io/apimachinery/pkg/api/resource" - draapi "k8s.io/dynamic-resource-allocation/api" "k8s.io/utils/ptr" ) // CmpRequestOverCapacity checks whether the new capacity request can be added within the given capacity, // and checks whether the requested value is against the capacity requestPolicy. func CmpRequestOverCapacity(currentConsumedCapacity ConsumedCapacity, deviceRequestCapacity *resourceapi.CapacityRequirements, - allowMultipleAllocations *bool, capacity map[draapi.QualifiedName]draapi.DeviceCapacity, allocatingCapacity ConsumedCapacity) (bool, error) { + allowMultipleAllocations *bool, capacity map[resourceapi.QualifiedName]resourceapi.DeviceCapacity, allocatingCapacity ConsumedCapacity) (bool, error) { if requestsContainNonExistCapacity(deviceRequestCapacity, capacity) { return false, errors.New("some requested capacity has not been defined") } clone := currentConsumedCapacity.Clone() for name, cap := range capacity { - convertedName := resourceapi.QualifiedName(name) - var convertedCapacity resourceapi.DeviceCapacity - err := draapi.Convert_api_DeviceCapacity_To_v1_DeviceCapacity(&cap, &convertedCapacity, nil) - if err != nil { - return false, fmt.Errorf("failed to convert DeviceCapacity %w", err) - } var requestedValPtr *resource.Quantity if deviceRequestCapacity != nil && deviceRequestCapacity.Requests != nil { - if requestedVal, requestedFound := deviceRequestCapacity.Requests[convertedName]; requestedFound { + if requestedVal, requestedFound := deviceRequestCapacity.Requests[name]; requestedFound { requestedValPtr = &requestedVal } } - consumedCapacity := calculateConsumedCapacity(requestedValPtr, convertedCapacity) - if violatesPolicy(consumedCapacity, convertedCapacity.RequestPolicy) { + consumedCapacity := calculateConsumedCapacity(requestedValPtr, cap) + if violatesPolicy(consumedCapacity, cap.RequestPolicy) { return false, nil } // If the current clone already contains an entry for this capacity, add the consumedCapacity to it. // Otherwise, initialize it with calculated consumedCapacity. - if _, allocatedFound := clone[convertedName]; allocatedFound { - clone[convertedName].Add(consumedCapacity) + if _, allocatedFound := clone[name]; allocatedFound { + clone[name].Add(consumedCapacity) } else { - clone[convertedName] = ptr.To(consumedCapacity) + clone[name] = ptr.To(consumedCapacity) } // If allocatingCapacity contains an entry for this capacity, add its value to clone as well. - if allocatingVal, allocatingFound := allocatingCapacity[convertedName]; allocatingFound { - clone[convertedName].Add(*allocatingVal) + if allocatingVal, allocatingFound := allocatingCapacity[name]; allocatingFound { + clone[name].Add(*allocatingVal) } - if clone[convertedName].Cmp(cap.Value) > 0 { + if clone[name].Cmp(cap.Value) > 0 { return false, nil } } @@ -71,13 +63,12 @@ func CmpRequestOverCapacity(currentConsumedCapacity ConsumedCapacity, deviceRequ // requestsNonExistCapacity returns true if requests contain non-exist capacity. func requestsContainNonExistCapacity(deviceRequestCapacity *resourceapi.CapacityRequirements, - capacity map[draapi.QualifiedName]draapi.DeviceCapacity) bool { + capacity map[resourceapi.QualifiedName]resourceapi.DeviceCapacity) bool { if deviceRequestCapacity == nil || deviceRequestCapacity.Requests == nil { return false } for name := range deviceRequestCapacity.Requests { - convertedName := draapi.QualifiedName(name) - if _, found := capacity[convertedName]; !found { + if _, found := capacity[name]; !found { return true } } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/allocator_incubating.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/allocator_incubating.go index 8f4d87ac334..0677a0cea29 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/allocator_incubating.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/incubating/allocator_incubating.go @@ -222,7 +222,7 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node, claims []*resou for i, constraint := range claim.Spec.Devices.Constraints { switch { case constraint.MatchAttribute != nil: - matchAttribute := draapi.FullyQualifiedName(*constraint.MatchAttribute) + matchAttribute := resourceapi.FullyQualifiedName(*constraint.MatchAttribute) logger := alloc.logger if loggerV := alloc.logger.V(6); loggerV.Enabled() { logger = klog.LoggerWithName(logger, "matchAttributeConstraint") @@ -491,7 +491,7 @@ type allocator struct { // counterSets is a map with the name of counter sets to the counters in // the set. -type counterSets map[draapi.UniqueString]map[string]draapi.Counter +type counterSets map[draapi.UniqueString]map[string]resourceapi.Counter // matchKey identifies a device/request pair. type matchKey struct { @@ -608,9 +608,9 @@ type constraint interface { type matchAttributeConstraint struct { logger klog.Logger // Includes name and attribute name, so no need to repeat in log messages. requestNames sets.Set[string] - attributeName draapi.FullyQualifiedName + attributeName resourceapi.FullyQualifiedName - attribute *draapi.DeviceAttribute + attribute *resourceapi.DeviceAttribute numDevices int } @@ -690,9 +690,9 @@ func (m *matchAttributeConstraint) matches(requestName, subRequestName string) b } } -func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName draapi.FullyQualifiedName) *draapi.DeviceAttribute { +func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName resourceapi.FullyQualifiedName) *resourceapi.DeviceAttribute { // Fully-qualified match? - if attr, ok := device.Attributes[draapi.QualifiedName(attributeName)]; ok { + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName)]; ok { return &attr } index := strings.Index(string(attributeName), "/") @@ -708,7 +708,7 @@ func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName dra } // Domain matches the driver, so let's check just the ID. - if attr, ok := device.Attributes[draapi.QualifiedName(attributeName[index+1:])]; ok { + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName[index+1:])]; ok { return &attr } @@ -1206,7 +1206,7 @@ func (alloc *allocator) checkAvailableCounters(device deviceWithID) (bool, error if !found { availableCountersForSlice = make(counterSets, len(slice.Spec.SharedCounters)) for _, counterSet := range slice.Spec.SharedCounters { - availableCountersForCounterSet := make(map[string]draapi.Counter, len(counterSet.Counters)) + availableCountersForCounterSet := make(map[string]resourceapi.Counter, len(counterSet.Counters)) for name, c := range counterSet.Counters { availableCountersForCounterSet[name] = c } @@ -1265,7 +1265,7 @@ func (alloc *allocator) checkAvailableCounters(device deviceWithID) (bool, error for _, deviceCounterConsumption := range device.ConsumesCounters { consumedCountersForCounterSet, found := consumedCountersForSlice[deviceCounterConsumption.CounterSet] if !found { - consumedCountersForCounterSet = make(map[string]draapi.Counter) + consumedCountersForCounterSet = make(map[string]resourceapi.Counter) consumedCountersForSlice[deviceCounterConsumption.CounterSet] = consumedCountersForCounterSet } for name, c := range deviceCounterConsumption.Counters { diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/stable/allocator_stable.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/stable/allocator_stable.go index b31c5351fb2..797f9715e0d 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/stable/allocator_stable.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/internal/stable/allocator_stable.go @@ -213,7 +213,7 @@ func (a *Allocator) Allocate(ctx context.Context, node *v1.Node, claims []*resou for i, constraint := range claim.Spec.Devices.Constraints { switch { case constraint.MatchAttribute != nil: - matchAttribute := draapi.FullyQualifiedName(*constraint.MatchAttribute) + matchAttribute := resourceapi.FullyQualifiedName(*constraint.MatchAttribute) logger := alloc.logger if loggerV := alloc.logger.V(6); loggerV.Enabled() { logger = klog.LoggerWithName(logger, "matchAttributeConstraint") @@ -472,7 +472,7 @@ type allocator struct { // counterSets is a map with the name of counter sets to the counters in // the set. -type counterSets map[draapi.UniqueString]map[string]draapi.Counter +type counterSets map[draapi.UniqueString]map[string]resourceapi.Counter // matchKey identifies a device/request pair. type matchKey struct { @@ -564,9 +564,9 @@ type constraint interface { type matchAttributeConstraint struct { logger klog.Logger // Includes name and attribute name, so no need to repeat in log messages. requestNames sets.Set[string] - attributeName draapi.FullyQualifiedName + attributeName resourceapi.FullyQualifiedName - attribute *draapi.DeviceAttribute + attribute *resourceapi.DeviceAttribute numDevices int } @@ -646,9 +646,9 @@ func (m *matchAttributeConstraint) matches(requestName, subRequestName string) b } } -func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName draapi.FullyQualifiedName) *draapi.DeviceAttribute { +func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName resourceapi.FullyQualifiedName) *resourceapi.DeviceAttribute { // Fully-qualified match? - if attr, ok := device.Attributes[draapi.QualifiedName(attributeName)]; ok { + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName)]; ok { return &attr } index := strings.Index(string(attributeName), "/") @@ -664,7 +664,7 @@ func lookupAttribute(device *draapi.Device, deviceID DeviceID, attributeName dra } // Domain matches the driver, so let's check just the ID. - if attr, ok := device.Attributes[draapi.QualifiedName(attributeName[index+1:])]; ok { + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName[index+1:])]; ok { return &attr } @@ -1112,7 +1112,7 @@ func (alloc *allocator) checkAvailableCounters(device deviceWithID) (bool, error if !found { availableCountersForSlice = make(counterSets, len(slice.Spec.SharedCounters)) for _, counterSet := range slice.Spec.SharedCounters { - availableCountersForCounterSet := make(map[string]draapi.Counter, len(counterSet.Counters)) + availableCountersForCounterSet := make(map[string]resourceapi.Counter, len(counterSet.Counters)) for name, c := range counterSet.Counters { availableCountersForCounterSet[name] = c } @@ -1171,7 +1171,7 @@ func (alloc *allocator) checkAvailableCounters(device deviceWithID) (bool, error for _, deviceCounterConsumption := range device.ConsumesCounters { consumedCountersForCounterSet, found := consumedCountersForSlice[deviceCounterConsumption.CounterSet] if !found { - consumedCountersForCounterSet = make(map[string]draapi.Counter) + consumedCountersForCounterSet = make(map[string]resourceapi.Counter) consumedCountersForSlice[deviceCounterConsumption.CounterSet] = consumedCountersForCounterSet } for name, c := range deviceCounterConsumption.Counters { From aad6cec749bf13a720993616f2219b0879417d48 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 22 Oct 2025 19:23:23 +0200 Subject: [PATCH 2/2] DRA internal API: add JSON tags This is useful for klog.Format() and diff.Diff() which both use JSON to render structs in a more readable manner. In particular the `omitempty` is important to keep that output short. It's intentionally not compatible with normal encoding to avoid accidentally using it with a real API request. --- .../api/type_test.go | 43 ++++++++++++++++ .../dynamic-resource-allocation/api/types.go | 50 +++++++++++-------- 2 files changed, 73 insertions(+), 20 deletions(-) create mode 100644 staging/src/k8s.io/dynamic-resource-allocation/api/type_test.go diff --git a/staging/src/k8s.io/dynamic-resource-allocation/api/type_test.go b/staging/src/k8s.io/dynamic-resource-allocation/api/type_test.go new file mode 100644 index 00000000000..6e41bcab604 --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/api/type_test.go @@ -0,0 +1,43 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package api + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" +) + +var slice = ResourceSlice{ + TypeMeta: metav1.TypeMeta{ + Kind: "ResourceSlice", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "slice", + }, + Spec: ResourceSliceSpec{ + Driver: MakeUniqueString("driver-name"), + Devices: []Device{{ + Name: MakeUniqueString("device-name"), + }}, + }, +} + +func TestKlog(t *testing.T) { + t.Logf("slice:\n%s", klog.Format(slice)) +} diff --git a/staging/src/k8s.io/dynamic-resource-allocation/api/types.go b/staging/src/k8s.io/dynamic-resource-allocation/api/types.go index 183fd289164..1d26a81e019 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/api/types.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/api/types.go @@ -22,26 +22,35 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// JSON tags exist to make the output more readable (klog, diff.Diff). +// They are intentionally not compatible with the normal encoding +// of a ResourceSlice to avoid accidentally using them with an apiserver +// request: +// - TypeMeta does not get encoded. +// - Fields from this package use upper case whereas types from the +// real API use lower case. + type ResourceSlice struct { - metav1.TypeMeta + metav1.TypeMeta `json:"-"` // Not needed, not set consistently. metav1.ObjectMeta + Spec ResourceSliceSpec } type ResourceSliceSpec struct { Driver UniqueString Pool ResourcePool - NodeName *string - NodeSelector *v1.NodeSelector - AllNodes bool - Devices []Device - PerDeviceNodeSelection *bool - SharedCounters []CounterSet + NodeName *string `json:",omitempty"` + NodeSelector *v1.NodeSelector `json:",omitempty"` + AllNodes bool `json:",omitempty"` + Devices []Device `json:",omitempty"` + PerDeviceNodeSelection *bool `json:",omitempty"` + SharedCounters []CounterSet `json:",omitempty"` } type CounterSet struct { Name UniqueString - Counters map[string]resourceapi.Counter + Counters map[string]resourceapi.Counter `json:",omitempty"` } type ResourcePool struct { @@ -49,22 +58,23 @@ type ResourcePool struct { Generation int64 ResourceSliceCount int64 } + type Device struct { Name UniqueString - Attributes map[resourceapi.QualifiedName]resourceapi.DeviceAttribute - Capacity map[resourceapi.QualifiedName]resourceapi.DeviceCapacity - ConsumesCounters []DeviceCounterConsumption - NodeName *string - NodeSelector *v1.NodeSelector - AllNodes *bool - Taints []resourceapi.DeviceTaint - BindsToNode bool - BindingConditions []string - BindingFailureConditions []string - AllowMultipleAllocations *bool + Attributes map[resourceapi.QualifiedName]resourceapi.DeviceAttribute `json:",omitempty"` + Capacity map[resourceapi.QualifiedName]resourceapi.DeviceCapacity `json:",omitempty"` + ConsumesCounters []DeviceCounterConsumption `json:",omitempty"` + NodeName *string `json:",omitempty"` + NodeSelector *v1.NodeSelector `json:",omitempty"` + AllNodes *bool `json:",omitempty"` + Taints []resourceapi.DeviceTaint `json:",omitempty"` + BindsToNode bool `json:",omitempty"` + BindingConditions []string `json:",omitempty"` + BindingFailureConditions []string `json:",omitempty"` + AllowMultipleAllocations *bool `json:",omitempty"` } type DeviceCounterConsumption struct { CounterSet UniqueString - Counters map[string]resourceapi.Counter + Counters map[string]resourceapi.Counter `json:",omitempty"` }