From 599fe605f93be4d270a0058bbd7f08a5f2d4befb Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 11 Jul 2024 16:42:51 +0200 Subject: [PATCH] DRA scheduler: adapt to v1alpha3 API The structured parameter allocation logic was written from scratch in staging/src/k8s.io/dynamic-resource-allocation/structured where it might be useful for out-of-tree components. Besides the new features (amount, admin access) and API it now supports backtracking when the initial device selection doesn't lead to a complete allocation of all claims. Co-authored-by: Ed Bartosh Co-authored-by: John Belamaric --- pkg/scheduler/eventhandlers.go | 24 +- pkg/scheduler/eventhandlers_test.go | 30 +- .../dynamicresources/dynamicresources.go | 633 +++------ .../dynamicresources/dynamicresources_test.go | 518 +++---- .../namedresources/namedresourcesmodel.go | 153 -- .../namedresourcesmodel_test.go | 327 ----- .../dynamicresources/structuredparameters.go | 274 ---- .../structuredparameters_test.go | 1257 ----------------- pkg/scheduler/framework/types.go | 26 +- pkg/scheduler/scheduler_test.go | 32 +- pkg/scheduler/testing/wrappers.go | 194 +-- staging/publishing/import-restrictions.yaml | 1 + .../structured/allocator.go | 844 +++++++++++ .../structured/allocator_test.go | 971 +++++++++++++ .../structured/doc.go | 18 + .../structured/pools.go | 131 ++ test/e2e/dra/dra.go | 15 +- test/integration/scheduler/scheduler_test.go | 17 +- .../dra/another-resourceclaimtemplate.yaml | 8 +- .../config/dra/another-resourceclass.yaml | 5 - .../config/dra/claim-template.yaml | 7 - .../config/dra/deviceclass-structured.yaml | 8 + .../{resourceclass.yaml => deviceclass.yaml} | 3 +- .../config/dra/resourceclaim-structured.yaml | 9 +- .../config/dra/resourceclaim.yaml | 6 +- .../config/dra/resourceclaimparameters.yaml | 9 - .../dra/resourceclaimtemplate-structured.yaml | 9 +- .../config/dra/resourceclaimtemplate.yaml | 6 +- .../config/dra/resourceclass-structured.yaml | 6 - .../config/performance-config.yaml | 25 +- test/integration/scheduler_perf/dra.go | 21 +- 31 files changed, 2472 insertions(+), 3115 deletions(-) delete mode 100644 pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel.go delete mode 100644 pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go delete mode 100644 pkg/scheduler/framework/plugins/dynamicresources/structuredparameters.go delete mode 100644 pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go create mode 100644 staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go create mode 100644 staging/src/k8s.io/dynamic-resource-allocation/structured/allocator_test.go create mode 100644 staging/src/k8s.io/dynamic-resource-allocation/structured/doc.go create mode 100644 staging/src/k8s.io/dynamic-resource-allocation/structured/pools.go delete mode 100644 test/integration/scheduler_perf/config/dra/another-resourceclass.yaml delete mode 100644 test/integration/scheduler_perf/config/dra/claim-template.yaml create mode 100644 test/integration/scheduler_perf/config/dra/deviceclass-structured.yaml rename test/integration/scheduler_perf/config/dra/{resourceclass.yaml => deviceclass.yaml} (54%) delete mode 100644 test/integration/scheduler_perf/config/dra/resourceclaimparameters.yaml delete mode 100644 test/integration/scheduler_perf/config/dra/resourceclass-structured.yaml diff --git a/pkg/scheduler/eventhandlers.go b/pkg/scheduler/eventhandlers.go index 88f9f1fd43b..3b9b6ebb410 100644 --- a/pkg/scheduler/eventhandlers.go +++ b/pkg/scheduler/eventhandlers.go @@ -532,28 +532,10 @@ func addAllEventHandlers( ) handlers = append(handlers, handlerRegistration) } - case framework.ResourceClass: + case framework.DeviceClass: if utilfeature.DefaultFeatureGate.Enabled(features.DynamicResourceAllocation) { - if handlerRegistration, err = informerFactory.Resource().V1alpha3().ResourceClasses().Informer().AddEventHandler( - buildEvtResHandler(at, framework.ResourceClass, "ResourceClass"), - ); err != nil { - return err - } - handlers = append(handlers, handlerRegistration) - } - case framework.ResourceClaimParameters: - if utilfeature.DefaultFeatureGate.Enabled(features.DynamicResourceAllocation) { - if handlerRegistration, err = informerFactory.Resource().V1alpha3().ResourceClaimParameters().Informer().AddEventHandler( - buildEvtResHandler(at, framework.ResourceClaimParameters, "ResourceClaimParameters"), - ); err != nil { - return err - } - handlers = append(handlers, handlerRegistration) - } - case framework.ResourceClassParameters: - if utilfeature.DefaultFeatureGate.Enabled(features.DynamicResourceAllocation) { - if handlerRegistration, err = informerFactory.Resource().V1alpha3().ResourceClassParameters().Informer().AddEventHandler( - buildEvtResHandler(at, framework.ResourceClassParameters, "ResourceClassParameters"), + if handlerRegistration, err = informerFactory.Resource().V1alpha3().DeviceClasses().Informer().AddEventHandler( + buildEvtResHandler(at, framework.DeviceClass, "DeviceClass"), ); err != nil { return err } diff --git a/pkg/scheduler/eventhandlers_test.go b/pkg/scheduler/eventhandlers_test.go index f0c53715b4c..365c8dc7726 100644 --- a/pkg/scheduler/eventhandlers_test.go +++ b/pkg/scheduler/eventhandlers_test.go @@ -232,11 +232,9 @@ func TestAddAllEventHandlers(t *testing.T) { { name: "DRA events disabled", gvkMap: map[framework.GVK]framework.ActionType{ - framework.PodSchedulingContext: framework.Add, - framework.ResourceClaim: framework.Add, - framework.ResourceClass: framework.Add, - framework.ResourceClaimParameters: framework.Add, - framework.ResourceClassParameters: framework.Add, + framework.PodSchedulingContext: framework.Add, + framework.ResourceClaim: framework.Add, + framework.DeviceClass: framework.Add, }, expectStaticInformers: map[reflect.Type]bool{ reflect.TypeOf(&v1.Pod{}): true, @@ -248,22 +246,18 @@ func TestAddAllEventHandlers(t *testing.T) { { name: "DRA events enabled", gvkMap: map[framework.GVK]framework.ActionType{ - framework.PodSchedulingContext: framework.Add, - framework.ResourceClaim: framework.Add, - framework.ResourceClass: framework.Add, - framework.ResourceClaimParameters: framework.Add, - framework.ResourceClassParameters: framework.Add, + framework.PodSchedulingContext: framework.Add, + framework.ResourceClaim: framework.Add, + framework.DeviceClass: framework.Add, }, enableDRA: true, expectStaticInformers: map[reflect.Type]bool{ - reflect.TypeOf(&v1.Pod{}): true, - reflect.TypeOf(&v1.Node{}): true, - reflect.TypeOf(&v1.Namespace{}): true, - reflect.TypeOf(&resourceapi.PodSchedulingContext{}): true, - reflect.TypeOf(&resourceapi.ResourceClaim{}): true, - reflect.TypeOf(&resourceapi.ResourceClaimParameters{}): true, - reflect.TypeOf(&resourceapi.ResourceClass{}): true, - reflect.TypeOf(&resourceapi.ResourceClassParameters{}): true, + reflect.TypeOf(&v1.Pod{}): true, + reflect.TypeOf(&v1.Node{}): true, + reflect.TypeOf(&v1.Namespace{}): true, + reflect.TypeOf(&resourceapi.PodSchedulingContext{}): true, + reflect.TypeOf(&resourceapi.ResourceClaim{}): true, + reflect.TypeOf(&resourceapi.DeviceClass{}): true, }, expectDynamicInformers: map[schema.GroupVersionResource]bool{}, }, diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go index a0d2eccd3d4..d683e30cbb2 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go @@ -38,10 +38,10 @@ import ( resourceapiapply "k8s.io/client-go/applyconfigurations/resource/v1alpha3" "k8s.io/client-go/kubernetes" resourcelisters "k8s.io/client-go/listers/resource/v1alpha3" - "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/retry" "k8s.io/component-helpers/scheduling/corev1/nodeaffinity" "k8s.io/dynamic-resource-allocation/resourceclaim" + "k8s.io/dynamic-resource-allocation/structured" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" @@ -56,10 +56,6 @@ const ( Name = names.DynamicResources stateKey framework.StateKey = Name - - // generatedFromIndex is the lookup name for the index function - // which indexes by other resource which generated the parameters object. - generatedFromIndex = "generated-from-index" ) // The state is initialized in PreFilter phase. Because we save the pointer in @@ -82,9 +78,8 @@ type stateData struct { // (if one exists) and the changes made to it. podSchedulingState podSchedulingState - // resourceModel contains the information about available and allocated resources when using - // structured parameters and the pod needs this information. - resources resources + // Allocator handles claims with structured parameters. + allocator *structured.Allocator // mutex must be locked while accessing any of the fields below. mutex sync.Mutex @@ -99,6 +94,9 @@ type stateData struct { unavailableClaims sets.Set[int] informationsForClaim []informationForClaim + + // nodeAllocations caches the result of Filter for the nodes. + nodeAllocations map[string][]*resourceapi.AllocationResult } func (d *stateData) Clone() framework.StateData { @@ -106,24 +104,20 @@ func (d *stateData) Clone() framework.StateData { } type informationForClaim struct { - // The availableOnNode node filter of the claim converted from the - // v1 API to nodeaffinity.NodeSelector by PreFilter for repeated - // evaluation in Filter. Nil for claim which don't have it. - availableOnNode *nodeaffinity.NodeSelector + // Node selectors based on the claim status (single entry, key is empty) if allocated, + // otherwise the device class AvailableOnNodes selectors (potentially multiple entries, + // key is the device class name). + availableOnNodes map[string]*nodeaffinity.NodeSelector // The status of the claim got from the // schedulingCtx by PreFilter for repeated // evaluation in Filter. Nil for claim which don't have it. status *resourceapi.ResourceClaimSchedulingStatus - // structuredParameters is true if the claim is handled via the builtin - // controller. structuredParameters bool - controller *claimController // Set by Reserved, published by PreBind. - allocation *resourceapi.AllocationResult - allocationDriverName string + allocation *resourceapi.AllocationResult } type podSchedulingState struct { @@ -276,19 +270,9 @@ type dynamicResources struct { enabled bool fh framework.Handle clientset kubernetes.Interface - classLister resourcelisters.ResourceClassLister + classLister resourcelisters.DeviceClassLister podSchedulingContextLister resourcelisters.PodSchedulingContextLister - claimParametersLister resourcelisters.ResourceClaimParametersLister - classParametersLister resourcelisters.ResourceClassParametersLister - resourceSliceLister resourcelisters.ResourceSliceLister - claimNameLookup *resourceclaim.Lookup - - // claimParametersIndexer has the common claimParametersGeneratedFrom indexer installed to - // limit iteration over claimParameters to those of interest. - claimParametersIndexer cache.Indexer - // classParametersIndexer has the common classParametersGeneratedFrom indexer installed to - // limit iteration over classParameters to those of interest. - classParametersIndexer cache.Indexer + sliceLister resourcelisters.ResourceSliceLister // claimAssumeCache enables temporarily storing a newer claim object // while the scheduler has allocated it and the corresponding object @@ -357,61 +341,15 @@ func New(ctx context.Context, plArgs runtime.Object, fh framework.Handle, fts fe enabled: true, fh: fh, clientset: fh.ClientSet(), - classLister: fh.SharedInformerFactory().Resource().V1alpha3().ResourceClasses().Lister(), + classLister: fh.SharedInformerFactory().Resource().V1alpha3().DeviceClasses().Lister(), podSchedulingContextLister: fh.SharedInformerFactory().Resource().V1alpha3().PodSchedulingContexts().Lister(), - claimParametersLister: fh.SharedInformerFactory().Resource().V1alpha3().ResourceClaimParameters().Lister(), - claimParametersIndexer: fh.SharedInformerFactory().Resource().V1alpha3().ResourceClaimParameters().Informer().GetIndexer(), - classParametersLister: fh.SharedInformerFactory().Resource().V1alpha3().ResourceClassParameters().Lister(), - classParametersIndexer: fh.SharedInformerFactory().Resource().V1alpha3().ResourceClassParameters().Informer().GetIndexer(), - resourceSliceLister: fh.SharedInformerFactory().Resource().V1alpha3().ResourceSlices().Lister(), - claimNameLookup: resourceclaim.NewNameLookup(fh.ClientSet()), + sliceLister: fh.SharedInformerFactory().Resource().V1alpha3().ResourceSlices().Lister(), claimAssumeCache: fh.ResourceClaimCache(), } - if err := pl.claimParametersIndexer.AddIndexers(cache.Indexers{generatedFromIndex: claimParametersGeneratedFromIndexFunc}); err != nil { - return nil, fmt.Errorf("add claim parameters cache indexer: %w", err) - } - if err := pl.classParametersIndexer.AddIndexers(cache.Indexers{generatedFromIndex: classParametersGeneratedFromIndexFunc}); err != nil { - return nil, fmt.Errorf("add class parameters cache indexer: %w", err) - } - return pl, nil } -func claimParametersReferenceKeyFunc(namespace string, ref *resourceapi.ResourceClaimParametersReference) string { - return ref.APIGroup + "/" + ref.Kind + "/" + namespace + "/" + ref.Name -} - -// claimParametersGeneratedFromIndexFunc is an index function that returns other resource keys -// (= apiGroup/kind/namespace/name) for ResourceClaimParametersReference in a given claim parameters. -func claimParametersGeneratedFromIndexFunc(obj interface{}) ([]string, error) { - parameters, ok := obj.(*resourceapi.ResourceClaimParameters) - if !ok { - return nil, nil - } - if parameters.GeneratedFrom == nil { - return nil, nil - } - return []string{claimParametersReferenceKeyFunc(parameters.Namespace, parameters.GeneratedFrom)}, nil -} - -func classParametersReferenceKeyFunc(ref *resourceapi.ResourceClassParametersReference) string { - return ref.APIGroup + "/" + ref.Kind + "/" + ref.Namespace + "/" + ref.Name -} - -// classParametersGeneratedFromIndexFunc is an index function that returns other resource keys -// (= apiGroup/kind/namespace/name) for ResourceClassParametersReference in a given class parameters. -func classParametersGeneratedFromIndexFunc(obj interface{}) ([]string, error) { - parameters, ok := obj.(*resourceapi.ResourceClassParameters) - if !ok { - return nil, nil - } - if parameters.GeneratedFrom == nil { - return nil, nil - } - return []string{classParametersReferenceKeyFunc(parameters.GeneratedFrom)}, nil -} - var _ framework.PreEnqueuePlugin = &dynamicResources{} var _ framework.PreFilterPlugin = &dynamicResources{} var _ framework.FilterPlugin = &dynamicResources{} @@ -435,11 +373,6 @@ func (pl *dynamicResources) EventsToRegister(_ context.Context) ([]framework.Clu } events := []framework.ClusterEventWithHint{ - // Changes for claim or class parameters creation may make pods - // schedulable which depend on claims using those parameters. - {Event: framework.ClusterEvent{Resource: framework.ResourceClaimParameters, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterClaimParametersChange}, - {Event: framework.ClusterEvent{Resource: framework.ResourceClassParameters, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterClassParametersChange}, - // Allocation is tracked in ResourceClaims, so any changes may make the pods schedulable. {Event: framework.ClusterEvent{Resource: framework.ResourceClaim, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterClaimChange}, // When a driver has provided additional information, a pod waiting for that information @@ -458,7 +391,7 @@ func (pl *dynamicResources) EventsToRegister(_ context.Context) ([]framework.Clu // See: https://github.com/kubernetes/kubernetes/issues/110175 {Event: framework.ClusterEvent{Resource: framework.Node, ActionType: framework.Add | framework.UpdateNodeLabel | framework.UpdateNodeTaint}}, // A pod might be waiting for a class to get created or modified. - {Event: framework.ClusterEvent{Resource: framework.ResourceClass, ActionType: framework.Add | framework.Update}}, + {Event: framework.ClusterEvent{Resource: framework.DeviceClass, ActionType: framework.Add | framework.Update}}, } return events, nil } @@ -473,149 +406,6 @@ func (pl *dynamicResources) PreEnqueue(ctx context.Context, pod *v1.Pod) (status return nil } -// isSchedulableAfterClaimParametersChange is invoked for add and update claim parameters events reported by -// an informer. It checks whether that change made a previously unschedulable -// pod schedulable. It errs on the side of letting a pod scheduling attempt -// happen. The delete claim event will not invoke it, so newObj will never be nil. -func (pl *dynamicResources) isSchedulableAfterClaimParametersChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { - originalParameters, modifiedParameters, err := schedutil.As[*resourceapi.ResourceClaimParameters](oldObj, newObj) - if err != nil { - // Shouldn't happen. - return framework.Queue, fmt.Errorf("unexpected object in isSchedulableAfterClaimParametersChange: %w", err) - } - - usesParameters := false - if err := pl.foreachPodResourceClaim(pod, func(_ string, claim *resourceapi.ResourceClaim) { - ref := claim.Spec.ParametersRef - if ref == nil { - return - } - - // Using in-tree parameters directly? - if ref.APIGroup == resourceapi.SchemeGroupVersion.Group && - ref.Kind == "ResourceClaimParameters" { - if modifiedParameters.Name == ref.Name { - usesParameters = true - } - return - } - - // Need to look for translated parameters. - generatedFrom := modifiedParameters.GeneratedFrom - if generatedFrom == nil { - return - } - if generatedFrom.APIGroup == ref.APIGroup && - generatedFrom.Kind == ref.Kind && - generatedFrom.Name == ref.Name { - usesParameters = true - } - }); err != nil { - // This is not an unexpected error: we know that - // foreachPodResourceClaim only returns errors for "not - // schedulable". - logger.V(4).Info("pod is not schedulable", "pod", klog.KObj(pod), "claim", klog.KObj(modifiedParameters), "reason", err.Error()) - return framework.QueueSkip, nil - } - - if !usesParameters { - // This were not the parameters the pod was waiting for. - logger.V(6).Info("unrelated claim parameters got modified", "pod", klog.KObj(pod), "claimParameters", klog.KObj(modifiedParameters)) - return framework.QueueSkip, nil - } - - if originalParameters == nil { - logger.V(4).Info("claim parameters for pod got created", "pod", klog.KObj(pod), "claimParameters", klog.KObj(modifiedParameters)) - return framework.Queue, nil - } - - // Modifications may or may not be relevant. If the entire - // requests are as before, then something else must have changed - // and we don't care. - if apiequality.Semantic.DeepEqual(&originalParameters.DriverRequests, &modifiedParameters.DriverRequests) { - logger.V(6).Info("claim parameters for pod got modified where the pod doesn't care", "pod", klog.KObj(pod), "claimParameters", klog.KObj(modifiedParameters)) - return framework.QueueSkip, nil - } - - logger.V(4).Info("requests in claim parameters for pod got updated", "pod", klog.KObj(pod), "claimParameters", klog.KObj(modifiedParameters)) - return framework.Queue, nil -} - -// isSchedulableAfterClassParametersChange is invoked for add and update class parameters events reported by -// an informer. It checks whether that change made a previously unschedulable -// pod schedulable. It errs on the side of letting a pod scheduling attempt -// happen. The delete class event will not invoke it, so newObj will never be nil. -func (pl *dynamicResources) isSchedulableAfterClassParametersChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { - originalParameters, modifiedParameters, err := schedutil.As[*resourceapi.ResourceClassParameters](oldObj, newObj) - if err != nil { - // Shouldn't happen. - return framework.Queue, fmt.Errorf("unexpected object in isSchedulableAfterClassParametersChange: %w", err) - } - - usesParameters := false - if err := pl.foreachPodResourceClaim(pod, func(_ string, claim *resourceapi.ResourceClaim) { - class, err := pl.classLister.Get(claim.Spec.ResourceClassName) - if err != nil { - if !apierrors.IsNotFound(err) { - logger.Error(err, "look up resource class") - } - return - } - ref := class.ParametersRef - if ref == nil { - return - } - - // Using in-tree parameters directly? - if ref.APIGroup == resourceapi.SchemeGroupVersion.Group && - ref.Kind == "ResourceClassParameters" { - if modifiedParameters.Name == ref.Name { - usesParameters = true - } - return - } - - // Need to look for translated parameters. - generatedFrom := modifiedParameters.GeneratedFrom - if generatedFrom == nil { - return - } - if generatedFrom.APIGroup == ref.APIGroup && - generatedFrom.Kind == ref.Kind && - generatedFrom.Name == ref.Name { - usesParameters = true - } - }); err != nil { - // This is not an unexpected error: we know that - // foreachPodResourceClaim only returns errors for "not - // schedulable". - logger.V(4).Info("pod is not schedulable", "pod", klog.KObj(pod), "classParameters", klog.KObj(modifiedParameters), "reason", err.Error()) - return framework.QueueSkip, nil - } - - if !usesParameters { - // This were not the parameters the pod was waiting for. - logger.V(6).Info("unrelated class parameters got modified", "pod", klog.KObj(pod), "classParameters", klog.KObj(modifiedParameters)) - return framework.QueueSkip, nil - } - - if originalParameters == nil { - logger.V(4).Info("class parameters for pod got created", "pod", klog.KObj(pod), "class", klog.KObj(modifiedParameters)) - return framework.Queue, nil - } - - // Modifications may or may not be relevant. If the entire - // requests are as before, then something else must have changed - // and we don't care. - if apiequality.Semantic.DeepEqual(&originalParameters.Filters, &modifiedParameters.Filters) { - logger.V(6).Info("class parameters for pod got modified where the pod doesn't care", "pod", klog.KObj(pod), "classParameters", klog.KObj(modifiedParameters)) - return framework.QueueSkip, nil - } - - logger.V(4).Info("filters in class parameters for pod got updated", "pod", klog.KObj(pod), "classParameters", klog.KObj(modifiedParameters)) - return framework.Queue, nil -} - // isSchedulableAfterClaimChange is invoked for add and update claim events reported by // an informer. It checks whether that change made a previously unschedulable // pod schedulable. It errs on the side of letting a pod scheduling attempt @@ -641,7 +431,8 @@ func (pl *dynamicResources) isSchedulableAfterClaimChange(logger klog.Logger, po } if originalClaim != nil && - resourceclaim.IsAllocatedWithStructuredParameters(originalClaim) && + originalClaim.Status.Allocation != nil && + originalClaim.Status.Allocation.Controller == "" && modifiedClaim.Status.Allocation == nil { // A claim with structured parameters was deallocated. This might have made // resources available for other pods. @@ -823,7 +614,7 @@ func (pl *dynamicResources) podResourceClaims(pod *v1.Pod) ([]*resourceapi.Resou // It calls an optional handler for those claims that it finds. func (pl *dynamicResources) foreachPodResourceClaim(pod *v1.Pod, cb func(podResourceName string, claim *resourceapi.ResourceClaim)) error { for _, resource := range pod.Spec.ResourceClaims { - claimName, mustCheckOwner, err := pl.claimNameLookup.Name(pod, &resource) + claimName, mustCheckOwner, err := resourceclaim.Name(pod, &resource) if err != nil { return err } @@ -892,8 +683,10 @@ func (pl *dynamicResources) PreFilter(ctx context.Context, state *framework.Cycl return nil, statusError(logger, err) } + // All claims which the scheduler needs to allocate itself. + allocateClaims := make([]*resourceapi.ResourceClaim, 0, len(claims)) + s.informationsForClaim = make([]informationForClaim, len(claims)) - needResourceInformation := false for index, claim := range claims { if claim.Status.DeallocationRequested { // This will get resolved by the resource driver. @@ -907,44 +700,19 @@ func (pl *dynamicResources) PreFilter(ctx context.Context, state *framework.Cycl } if claim.Status.Allocation != nil { - if claim.Status.Allocation.AvailableOnNodes != nil { - nodeSelector, err := nodeaffinity.NewNodeSelector(claim.Status.Allocation.AvailableOnNodes) + s.informationsForClaim[index].structuredParameters = claim.Status.Allocation.Controller == "" + if claim.Status.Allocation.NodeSelector != nil { + nodeSelector, err := nodeaffinity.NewNodeSelector(claim.Status.Allocation.NodeSelector) if err != nil { return nil, statusError(logger, err) } - s.informationsForClaim[index].availableOnNode = nodeSelector + s.informationsForClaim[index].availableOnNodes = map[string]*nodeaffinity.NodeSelector{"": nodeSelector} } - - // The claim was allocated by the scheduler if it has the finalizer that is - // reserved for Kubernetes. - s.informationsForClaim[index].structuredParameters = slices.Contains(claim.Finalizers, resourceapi.Finalizer) } else { - // The ResourceClass might have a node filter. This is - // useful for trimming the initial set of potential - // nodes before we ask the driver(s) for information - // about the specific pod. - class, err := pl.classLister.Get(claim.Spec.ResourceClassName) - if err != nil { - // If the class cannot be retrieved, allocation cannot proceed. - if apierrors.IsNotFound(err) { - // Here we mark the pod as "unschedulable", so it'll sleep in - // the unscheduleable queue until a ResourceClass event occurs. - return nil, statusUnschedulable(logger, fmt.Sprintf("resource class %s does not exist", claim.Spec.ResourceClassName)) - } - // Other error, retry with backoff. - return nil, statusError(logger, fmt.Errorf("look up resource class: %v", err)) - } - if class.SuitableNodes != nil { - selector, err := nodeaffinity.NewNodeSelector(class.SuitableNodes) - if err != nil { - return nil, statusError(logger, err) - } - s.informationsForClaim[index].availableOnNode = selector - } - s.informationsForClaim[index].status = statusForClaim(s.podSchedulingState.schedulingCtx, pod.Spec.ResourceClaims[index].Name) - - if class.StructuredParameters != nil && *class.StructuredParameters { - s.informationsForClaim[index].structuredParameters = true + structuredParameters := claim.Spec.Controller == "" + s.informationsForClaim[index].structuredParameters = structuredParameters + if structuredParameters { + allocateClaims = append(allocateClaims, claim) // Allocation in flight? Better wait for that // to finish, see inFlightAllocations @@ -952,164 +720,93 @@ func (pl *dynamicResources) PreFilter(ctx context.Context, state *framework.Cycl if _, found := pl.inFlightAllocations.Load(claim.UID); found { return nil, statusUnschedulable(logger, fmt.Sprintf("resource claim %s is in the process of being allocated", klog.KObj(claim))) } + } else { + s.informationsForClaim[index].status = statusForClaim(s.podSchedulingState.schedulingCtx, pod.Spec.ResourceClaims[index].Name) + } - // We need the claim and class parameters. If - // they don't exist yet, the pod has to wait. - // - // TODO (https://github.com/kubernetes/kubernetes/issues/123697): - // check this already in foreachPodResourceClaim, together with setting up informationsForClaim. - // Then PreEnqueue will also check for existence of parameters. - classParameters, claimParameters, status := pl.lookupParameters(logger, class, claim) - if status != nil { - return nil, status + // Check all requests and device classes. If a class + // does not exist, scheduling cannot proceed, no matter + // how the claim is being allocated. + // + // When using a control plane controller, a class might + // have a node filter. This is useful for trimming the + // initial set of potential nodes before we ask the + // driver(s) for information about the specific pod. + for _, request := range claim.Spec.Devices.Requests { + if request.DeviceClassName == "" { + return nil, statusError(logger, fmt.Errorf("request %s: unsupported request type", request.Name)) } - controller, err := newClaimController(logger, class, classParameters, claimParameters) + + class, err := pl.classLister.Get(request.DeviceClassName) if err != nil { - return nil, statusError(logger, err) + // If the class cannot be retrieved, allocation cannot proceed. + if apierrors.IsNotFound(err) { + // Here we mark the pod as "unschedulable", so it'll sleep in + // the unscheduleable queue until a DeviceClass event occurs. + return nil, statusUnschedulable(logger, fmt.Sprintf("request %s: device class %s does not exist", request.Name, request.DeviceClassName)) + } + // Other error, retry with backoff. + return nil, statusError(logger, fmt.Errorf("request %s: look up device class: %w", request.Name, err)) + } + if class.Spec.SuitableNodes != nil && !structuredParameters { + selector, err := nodeaffinity.NewNodeSelector(class.Spec.SuitableNodes) + if err != nil { + return nil, statusError(logger, err) + } + if s.informationsForClaim[index].availableOnNodes == nil { + s.informationsForClaim[index].availableOnNodes = make(map[string]*nodeaffinity.NodeSelector) + } + s.informationsForClaim[index].availableOnNodes[class.Name] = selector } - s.informationsForClaim[index].controller = controller - needResourceInformation = true } } } - if needResourceInformation { + if len(allocateClaims) > 0 { + logger.V(5).Info("Preparing allocation with structured parameters", "pod", klog.KObj(pod), "resourceclaims", klog.KObjSlice(allocateClaims)) + // Doing this over and over again for each pod could be avoided - // by parsing once when creating the plugin and then updating - // that state in informer callbacks. But that would cause - // problems for using the plugin in the Cluster Autoscaler. If - // this step here turns out to be expensive, we may have to - // maintain and update state more persistently. + // by setting the allocator up once and then keeping it up-to-date + // as changes are observed. + // + // But that would cause problems for using the plugin in the + // Cluster Autoscaler. If this step here turns out to be + // expensive, we may have to maintain and update state more + // persistently. // // Claims are treated as "allocated" if they are in the assume cache // or currently their allocation is in-flight. - resources, err := newResourceModel(logger, pl.resourceSliceLister, pl.claimAssumeCache, &pl.inFlightAllocations) - logger.V(5).Info("Resource usage", "resources", klog.Format(resources)) + allocator, err := structured.NewAllocator(ctx, allocateClaims, &claimListerForAssumeCache{assumeCache: pl.claimAssumeCache, inFlightAllocations: &pl.inFlightAllocations}, pl.classLister, pl.sliceLister) if err != nil { return nil, statusError(logger, err) } - s.resources = resources + s.allocator = allocator + s.nodeAllocations = make(map[string][]*resourceapi.AllocationResult) } s.claims = claims return nil, nil } -func (pl *dynamicResources) lookupParameters(logger klog.Logger, class *resourceapi.ResourceClass, claim *resourceapi.ResourceClaim) (classParameters *resourceapi.ResourceClassParameters, claimParameters *resourceapi.ResourceClaimParameters, status *framework.Status) { - classParameters, status = pl.lookupClassParameters(logger, class) - if status != nil { - return - } - claimParameters, status = pl.lookupClaimParameters(logger, class, claim) - return +type claimListerForAssumeCache struct { + assumeCache *assumecache.AssumeCache + inFlightAllocations *sync.Map } -func (pl *dynamicResources) lookupClassParameters(logger klog.Logger, class *resourceapi.ResourceClass) (*resourceapi.ResourceClassParameters, *framework.Status) { - defaultClassParameters := resourceapi.ResourceClassParameters{} - - if class.ParametersRef == nil { - return &defaultClassParameters, nil - } - - if class.ParametersRef.APIGroup == resourceapi.SchemeGroupVersion.Group && - class.ParametersRef.Kind == "ResourceClassParameters" { - // Use the parameters which were referenced directly. - parameters, err := pl.classParametersLister.ResourceClassParameters(class.ParametersRef.Namespace).Get(class.ParametersRef.Name) - if err != nil { - if apierrors.IsNotFound(err) { - return nil, statusUnschedulable(logger, fmt.Sprintf("class parameters %s not found", klog.KRef(class.ParametersRef.Namespace, class.ParametersRef.Name))) - } - return nil, statusError(logger, fmt.Errorf("get class parameters %s: %v", klog.KRef(class.Namespace, class.ParametersRef.Name), err)) +func (cl *claimListerForAssumeCache) ListAllAllocated() ([]*resourceapi.ResourceClaim, error) { + // Probably not worth adding an index for? + objs := cl.assumeCache.List(nil) + allocated := make([]*resourceapi.ResourceClaim, 0, len(objs)) + for _, obj := range objs { + claim := obj.(*resourceapi.ResourceClaim) + if obj, ok := cl.inFlightAllocations.Load(claim.UID); ok { + claim = obj.(*resourceapi.ResourceClaim) } - return parameters, nil - } - - objs, err := pl.classParametersIndexer.ByIndex(generatedFromIndex, classParametersReferenceKeyFunc(class.ParametersRef)) - if err != nil { - return nil, statusError(logger, fmt.Errorf("listing class parameters failed: %v", err)) - } - switch len(objs) { - case 0: - return nil, statusUnschedulable(logger, fmt.Sprintf("generated class parameters for %s.%s %s not found", class.ParametersRef.Kind, class.ParametersRef.APIGroup, klog.KRef(class.ParametersRef.Namespace, class.ParametersRef.Name))) - case 1: - parameters, ok := objs[0].(*resourceapi.ResourceClassParameters) - if !ok { - return nil, statusError(logger, fmt.Errorf("unexpected object in class parameters index: %T", objs[0])) + if claim.Status.Allocation != nil { + allocated = append(allocated, claim) } - return parameters, nil - default: - sort.Slice(objs, func(i, j int) bool { - obj1, obj2 := objs[i].(*resourceapi.ResourceClassParameters), objs[j].(*resourceapi.ResourceClassParameters) - if obj1 == nil || obj2 == nil { - return false - } - return obj1.Name < obj2.Name - }) - return nil, statusError(logger, fmt.Errorf("multiple generated class parameters for %s.%s %s found: %s", class.ParametersRef.Kind, class.ParametersRef.APIGroup, klog.KRef(class.Namespace, class.ParametersRef.Name), klog.KObjSlice(objs))) - } -} - -func (pl *dynamicResources) lookupClaimParameters(logger klog.Logger, class *resourceapi.ResourceClass, claim *resourceapi.ResourceClaim) (*resourceapi.ResourceClaimParameters, *framework.Status) { - defaultClaimParameters := resourceapi.ResourceClaimParameters{ - DriverRequests: []resourceapi.DriverRequests{ - { - DriverName: class.DriverName, - Requests: []resourceapi.ResourceRequest{ - { - ResourceRequestModel: resourceapi.ResourceRequestModel{ - // TODO: This only works because NamedResources is - // the only model currently implemented. We need to - // match the default to how the resources of this - // class are being advertized in a ResourceSlice. - NamedResources: &resourceapi.NamedResourcesRequest{ - Selector: "true", - }, - }, - }, - }, - }, - }, - } - - if claim.Spec.ParametersRef == nil { - return &defaultClaimParameters, nil - } - if claim.Spec.ParametersRef.APIGroup == resourceapi.SchemeGroupVersion.Group && - claim.Spec.ParametersRef.Kind == "ResourceClaimParameters" { - // Use the parameters which were referenced directly. - parameters, err := pl.claimParametersLister.ResourceClaimParameters(claim.Namespace).Get(claim.Spec.ParametersRef.Name) - if err != nil { - if apierrors.IsNotFound(err) { - return nil, statusUnschedulable(logger, fmt.Sprintf("claim parameters %s not found", klog.KRef(claim.Namespace, claim.Spec.ParametersRef.Name))) - } - return nil, statusError(logger, fmt.Errorf("get claim parameters %s: %v", klog.KRef(claim.Namespace, claim.Spec.ParametersRef.Name), err)) - } - return parameters, nil - } - - objs, err := pl.claimParametersIndexer.ByIndex(generatedFromIndex, claimParametersReferenceKeyFunc(claim.Namespace, claim.Spec.ParametersRef)) - if err != nil { - return nil, statusError(logger, fmt.Errorf("listing claim parameters failed: %v", err)) - } - switch len(objs) { - case 0: - return nil, statusUnschedulable(logger, fmt.Sprintf("generated claim parameters for %s.%s %s not found", claim.Spec.ParametersRef.Kind, claim.Spec.ParametersRef.APIGroup, klog.KRef(claim.Namespace, claim.Spec.ParametersRef.Name))) - case 1: - parameters, ok := objs[0].(*resourceapi.ResourceClaimParameters) - if !ok { - return nil, statusError(logger, fmt.Errorf("unexpected object in claim parameters index: %T", objs[0])) - } - return parameters, nil - default: - sort.Slice(objs, func(i, j int) bool { - obj1, obj2 := objs[i].(*resourceapi.ResourceClaimParameters), objs[j].(*resourceapi.ResourceClaimParameters) - if obj1 == nil || obj2 == nil { - return false - } - return obj1.Name < obj2.Name - }) - return nil, statusError(logger, fmt.Errorf("multiple generated claim parameters for %s.%s %s found: %s", claim.Spec.ParametersRef.Kind, claim.Spec.ParametersRef.APIGroup, klog.KRef(claim.Namespace, claim.Spec.ParametersRef.Name), klog.KObjSlice(objs))) } + return allocated, nil } // PreFilterExtensions returns prefilter extensions, pod add and remove. @@ -1158,10 +855,11 @@ func (pl *dynamicResources) Filter(ctx context.Context, cs *framework.CycleState logger.V(10).Info("filtering based on resource claims of the pod", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaim", klog.KObj(claim)) if claim.Status.Allocation != nil { - if nodeSelector := state.informationsForClaim[index].availableOnNode; nodeSelector != nil { + for _, nodeSelector := range state.informationsForClaim[index].availableOnNodes { if !nodeSelector.Match(node) { logger.V(5).Info("AvailableOnNodes does not match", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaim", klog.KObj(claim)) unavailableClaims = append(unavailableClaims, index) + break } } continue @@ -1172,40 +870,61 @@ func (pl *dynamicResources) Filter(ctx context.Context, cs *framework.CycleState return statusUnschedulable(logger, "resourceclaim must be reallocated", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaim", klog.KObj(claim)) } - if selector := state.informationsForClaim[index].availableOnNode; selector != nil { - if matches := selector.Match(node); !matches { - return statusUnschedulable(logger, "excluded by resource class node filter", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclassName", claim.Spec.ResourceClassName) + for className, nodeSelector := range state.informationsForClaim[index].availableOnNodes { + if !nodeSelector.Match(node) { + return statusUnschedulable(logger, "excluded by device class node filter", "pod", klog.KObj(pod), "node", klog.KObj(node), "deviceclass", klog.KRef("", className)) } } - // Can the builtin controller tell us whether the node is suitable? - if state.informationsForClaim[index].structuredParameters { - suitable, err := state.informationsForClaim[index].controller.nodeIsSuitable(ctx, node.Name, state.resources) - if err != nil { - // An error indicates that something wasn't configured correctly, for example - // writing a CEL expression which doesn't handle a map lookup error. Normally - // this should never fail. We could return an error here, but then the pod - // would get retried. Instead we ignore the node. - return statusUnschedulable(logger, fmt.Sprintf("checking structured parameters failed: %v", err), "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaim", klog.KObj(claim)) - } - if !suitable { - return statusUnschedulable(logger, "resourceclaim cannot be allocated for the node (unsuitable)", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaim", klog.KObj(claim)) - } - } else { - if status := state.informationsForClaim[index].status; status != nil { - for _, unsuitableNode := range status.UnsuitableNodes { - if node.Name == unsuitableNode { - return statusUnschedulable(logger, "resourceclaim cannot be allocated for the node (unsuitable)", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaim", klog.KObj(claim), "unsuitablenodes", status.UnsuitableNodes) - } + + // Use information from control plane controller? + if status := state.informationsForClaim[index].status; status != nil { + for _, unsuitableNode := range status.UnsuitableNodes { + if node.Name == unsuitableNode { + return statusUnschedulable(logger, "resourceclaim cannot be allocated for the node (unsuitable)", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaim", klog.KObj(claim), "unsuitablenodes", status.UnsuitableNodes) } } } } + // Use allocator to check the node and cache the result in case that the node is picked. + var allocations []*resourceapi.AllocationResult + if state.allocator != nil { + allocCtx := ctx + if loggerV := logger.V(5); loggerV.Enabled() { + allocCtx = klog.NewContext(allocCtx, klog.LoggerWithValues(logger, "node", klog.KObj(node))) + } + + a, err := state.allocator.Allocate(allocCtx, node) + if err != nil { + // This should only fail if there is something wrong with the claim or class. + // Return an error to abort scheduling of it. + // + // This will cause retries. It would be slightly nicer to mark it as unschedulable + // *and* abort scheduling. Then only cluster event for updating the claim or class + // with the broken CEL expression would trigger rescheduling. + // + // But we cannot do both. As this shouldn't occur often, aborting like this is + // better than the more complicated alternative (return Unschedulable here, remember + // the error, then later raise it again later if needed). + return statusError(logger, err, "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaims", klog.KObjSlice(state.allocator.ClaimsToAllocate())) + } + // Check for exact length just to be sure. In practice this is all-or-nothing. + if len(a) != len(state.allocator.ClaimsToAllocate()) { + return statusUnschedulable(logger, "cannot allocate all claims", "pod", klog.KObj(pod), "node", klog.KObj(node), "resourceclaims", klog.KObjSlice(state.allocator.ClaimsToAllocate())) + } + // Reserve uses this information. + allocations = a + } + + // Store information in state while holding the mutex. + if state.allocator != nil || len(unavailableClaims) > 0 { + state.mutex.Lock() + defer state.mutex.Unlock() + } + if len(unavailableClaims) > 0 { // Remember all unavailable claims. This might be observed // concurrently, so we have to lock the state before writing. - state.mutex.Lock() - defer state.mutex.Unlock() if state.unavailableClaims == nil { state.unavailableClaims = sets.New[int]() @@ -1217,6 +936,10 @@ func (pl *dynamicResources) Filter(ctx context.Context, cs *framework.CycleState return statusUnschedulable(logger, "resourceclaim not available on the node", "pod", klog.KObj(pod)) } + if state.allocator != nil { + state.nodeAllocations[node.Name] = allocations + } + return nil } @@ -1266,7 +989,6 @@ func (pl *dynamicResources) PostFilter(ctx context.Context, cs *framework.CycleS claim := claim.DeepCopy() claim.Status.ReservedFor = nil if clearAllocation { - claim.Status.DriverName = "" claim.Status.Allocation = nil } else { claim.Status.DeallocationRequested = true @@ -1303,7 +1025,7 @@ func (pl *dynamicResources) PreScore(ctx context.Context, cs *framework.CycleSta pending := false for index, claim := range state.claims { if claim.Status.Allocation == nil && - state.informationsForClaim[index].controller == nil { + !state.informationsForClaim[index].structuredParameters { pending = true break } @@ -1379,10 +1101,11 @@ func (pl *dynamicResources) Reserve(ctx context.Context, cs *framework.CycleStat return nil } + logger := klog.FromContext(ctx) + numDelayedAllocationPending := 0 numClaimsWithStatusInfo := 0 - claimsWithBuiltinController := make([]int, 0, len(state.claims)) - logger := klog.FromContext(ctx) + numClaimsWithAllocator := 0 for index, claim := range state.claims { if claim.Status.Allocation != nil { // Allocated, but perhaps not reserved yet. We checked in PreFilter that @@ -1393,9 +1116,9 @@ func (pl *dynamicResources) Reserve(ctx context.Context, cs *framework.CycleStat continue } - // Do we have the builtin controller? - if state.informationsForClaim[index].controller != nil { - claimsWithBuiltinController = append(claimsWithBuiltinController, index) + // Do we use the allocator for it? + if state.informationsForClaim[index].structuredParameters { + numClaimsWithAllocator++ continue } @@ -1409,7 +1132,7 @@ func (pl *dynamicResources) Reserve(ctx context.Context, cs *framework.CycleStat } } - if numDelayedAllocationPending == 0 && len(claimsWithBuiltinController) == 0 { + if numDelayedAllocationPending == 0 && numClaimsWithAllocator == 0 { // Nothing left to do. return nil } @@ -1430,27 +1153,41 @@ func (pl *dynamicResources) Reserve(ctx context.Context, cs *framework.CycleStat } // Prepare allocation of claims handled by the schedulder. - for _, index := range claimsWithBuiltinController { - claim := state.claims[index] - driverName, allocation, err := state.informationsForClaim[index].controller.allocate(ctx, nodeName, state.resources) - if err != nil { + if state.allocator != nil { + // Entries in these two slices match each other. + claimsToAllocate := state.allocator.ClaimsToAllocate() + allocations, ok := state.nodeAllocations[nodeName] + if !ok { // We checked before that the node is suitable. This shouldn't have failed, // so treat this as an error. - return statusError(logger, fmt.Errorf("claim allocation failed unexpectedly: %v", err)) + return statusError(logger, errors.New("claim allocation not found for node")) } - state.informationsForClaim[index].allocation = allocation - state.informationsForClaim[index].allocationDriverName = driverName - // Strictly speaking, we don't need to store the full modified object. - // The allocation would be enough. The full object is useful for - // debugging and testing, so let's make it realistic. - claim = claim.DeepCopy() - if !slices.Contains(claim.Finalizers, resourceapi.Finalizer) { - claim.Finalizers = append(claim.Finalizers, resourceapi.Finalizer) + + // Sanity check: do we have results for all pending claims? + if len(allocations) != len(claimsToAllocate) || + len(allocations) != numClaimsWithAllocator { + return statusError(logger, fmt.Errorf("internal error, have %d allocations, %d claims to allocate, want %d claims", len(allocations), len(claimsToAllocate), numClaimsWithAllocator)) + } + + for i, claim := range claimsToAllocate { + index := slices.Index(state.claims, claim) + if index < 0 { + return statusError(logger, fmt.Errorf("internal error, claim %s with allocation not found", claim.Name)) + } + allocation := allocations[i] + state.informationsForClaim[index].allocation = allocation + + // Strictly speaking, we don't need to store the full modified object. + // The allocation would be enough. The full object is useful for + // debugging, testing and the allocator, so let's make it realistic. + claim = claim.DeepCopy() + if !slices.Contains(claim.Finalizers, resourceapi.Finalizer) { + claim.Finalizers = append(claim.Finalizers, resourceapi.Finalizer) + } + claim.Status.Allocation = allocation + pl.inFlightAllocations.Store(claim.UID, claim) + logger.V(5).Info("Reserved resource in allocation result", "claim", klog.KObj(claim), "allocation", klog.Format(allocation)) } - claim.Status.DriverName = driverName - claim.Status.Allocation = allocation - pl.inFlightAllocations.Store(claim.UID, claim) - logger.V(5).Info("Reserved resource in allocation result", "claim", klog.KObj(claim), "driver", driverName, "allocation", klog.Format(allocation)) } // When there is only one pending resource, we can go ahead with @@ -1460,8 +1197,8 @@ func (pl *dynamicResources) Reserve(ctx context.Context, cs *framework.CycleStat // // If all pending claims are handled with the builtin controller, // there is no need for a PodSchedulingContext change. - if numDelayedAllocationPending == 1 && len(claimsWithBuiltinController) == 0 || - numClaimsWithStatusInfo+len(claimsWithBuiltinController) == numDelayedAllocationPending && len(claimsWithBuiltinController) < numDelayedAllocationPending { + if numDelayedAllocationPending == 1 && numClaimsWithAllocator == 0 || + numClaimsWithStatusInfo+numClaimsWithAllocator == numDelayedAllocationPending && numClaimsWithAllocator < numDelayedAllocationPending { // TODO: can we increase the chance that the scheduler picks // the same node as before when allocation is on-going, // assuming that that node still fits the pod? Picking a @@ -1530,7 +1267,7 @@ func (pl *dynamicResources) Unreserve(ctx context.Context, cs *framework.CycleSt for index, claim := range state.claims { // If allocation was in-flight, then it's not anymore and we need to revert the // claim object in the assume cache to what it was before. - if state.informationsForClaim[index].controller != nil { + if state.informationsForClaim[index].structuredParameters { if _, found := pl.inFlightAllocations.LoadAndDelete(state.claims[index].UID); found { pl.claimAssumeCache.Restore(claim.Namespace + "/" + claim.Name) } @@ -1661,8 +1398,6 @@ func (pl *dynamicResources) bindClaim(ctx context.Context, state *stateData, ind } claim = updatedClaim } - - claim.Status.DriverName = state.informationsForClaim[index].allocationDriverName claim.Status.Allocation = allocation } diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go index cecc5211ae3..6b136300e94 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go @@ -51,6 +51,11 @@ import ( var ( podKind = v1.SchemeGroupVersion.WithKind("Pod") + nodeName = "worker" + node2Name = "worker-2" + node3Name = "worker-3" + controller = "some-driver" + driver = controller podName = "my-pod" podUID = "1234" resourceName = "my-resource" @@ -59,45 +64,12 @@ var ( claimName2 = podName + "-" + resourceName + "-2" className = "my-resource-class" namespace = "default" + attrName = resourceapi.QualifiedName("healthy") // device attribute only available on non-default node - resourceClass = &resourceapi.ResourceClass{ + deviceClass = &resourceapi.DeviceClass{ ObjectMeta: metav1.ObjectMeta{ Name: className, }, - DriverName: "some-driver", - } - structuredResourceClass = &resourceapi.ResourceClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: className, - }, - DriverName: "some-driver", - StructuredParameters: ptr.To(true), - } - structuredResourceClassWithParams = &resourceapi.ResourceClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: className, - }, - DriverName: "some-driver", - StructuredParameters: ptr.To(true), - ParametersRef: &resourceapi.ResourceClassParametersReference{ - Name: className, - Namespace: namespace, - Kind: "ResourceClassParameters", - APIGroup: "resource.k8s.io", - }, - } - structuredResourceClassWithCRD = &resourceapi.ResourceClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: className, - }, - DriverName: "some-driver", - StructuredParameters: ptr.To(true), - ParametersRef: &resourceapi.ResourceClassParametersReference{ - Name: className, - Namespace: namespace, - Kind: "ResourceClassParameters", - APIGroup: "example.com", - }, } podWithClaimName = st.MakePod().Name(podName).Namespace(namespace). @@ -124,39 +96,29 @@ var ( PodResourceClaims(v1.PodResourceClaim{Name: resourceName2, ResourceClaimName: &claimName2}). Obj() - workerNode = &st.MakeNode().Name("worker").Label("kubernetes.io/hostname", "worker").Node - workerNodeSlice = st.MakeResourceSlice("worker", "some-driver").NamedResourcesInstances("instance-1").Obj() + // Node with "instance-1" device and no device attributes. + workerNode = &st.MakeNode().Name(nodeName).Label("kubernetes.io/hostname", nodeName).Node + workerNodeSlice = st.MakeResourceSlice(nodeName, driver).Device("instance-1", nil).Obj() - claimParameters = st.MakeClaimParameters().Name(claimName).Namespace(namespace). - NamedResourcesRequests("some-driver", "true"). - GeneratedFrom(&resourceapi.ResourceClaimParametersReference{ - Name: claimName, - Kind: "ResourceClaimParameters", - APIGroup: "example.com", - }). - Obj() - claimParametersOtherNamespace = st.MakeClaimParameters().Name(claimName).Namespace(namespace+"-2"). - NamedResourcesRequests("some-driver", "true"). - GeneratedFrom(&resourceapi.ResourceClaimParametersReference{ - Name: claimName, - Kind: "ResourceClaimParameters", - APIGroup: "example.com", - }). - Obj() - classParameters = st.MakeClassParameters().Name(className).Namespace(namespace). - NamedResourcesFilters("some-driver", "true"). - GeneratedFrom(&resourceapi.ResourceClassParametersReference{ - Name: className, - Namespace: namespace, - Kind: "ResourceClassParameters", - APIGroup: "example.com", - }). - Obj() + // Node with same device, but now with a "healthy" boolean attribute. + workerNode2 = &st.MakeNode().Name(node2Name).Label("kubernetes.io/hostname", node2Name).Node + workerNode2Slice = st.MakeResourceSlice(node2Name, driver).Device("instance-1", map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{attrName: {BoolValue: ptr.To(true)}}).Obj() - claim = st.MakeResourceClaim(). + // Yet another node, same as the second one. + workerNode3 = &st.MakeNode().Name(node3Name).Label("kubernetes.io/hostname", node3Name).Node + workerNode3Slice = st.MakeResourceSlice(node3Name, driver).Device("instance-1", map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{attrName: {BoolValue: ptr.To(true)}}).Obj() + + brokenSelector = resourceapi.DeviceSelector{ + CEL: &resourceapi.CELDeviceSelector{ + // Not set for workerNode. + Expression: fmt.Sprintf(`device.attributes["%s"].%s`, driver, attrName), + }, + } + + claim = st.MakeResourceClaim(controller). Name(claimName). Namespace(namespace). - ResourceClassName(className). + Request(className). Obj() pendingClaim = st.FromResourceClaim(claim). OwnerReference(podName, podUID, podKind). @@ -164,44 +126,53 @@ var ( pendingClaim2 = st.FromResourceClaim(pendingClaim). Name(claimName2). Obj() + allocationResult = &resourceapi.AllocationResult{ + Controller: controller, + Devices: resourceapi.DeviceAllocationResult{ + Results: []resourceapi.DeviceRequestAllocationResult{{ + Driver: driver, + Pool: nodeName, + Device: "instance-1", + Request: "req-1", + }}, + }, + NodeSelector: func() *v1.NodeSelector { + // Label selector... + nodeSelector := st.MakeNodeSelector().In("metadata.name", []string{nodeName}).Obj() + // ... but we need a field selector, so let's swap. + nodeSelector.NodeSelectorTerms[0].MatchExpressions, nodeSelector.NodeSelectorTerms[0].MatchFields = nodeSelector.NodeSelectorTerms[0].MatchFields, nodeSelector.NodeSelectorTerms[0].MatchExpressions + return nodeSelector + }(), + } deallocatingClaim = st.FromResourceClaim(pendingClaim). - Allocation("some-driver", &resourceapi.AllocationResult{}). + Allocation(allocationResult). DeallocationRequested(true). Obj() inUseClaim = st.FromResourceClaim(pendingClaim). - Allocation("some-driver", &resourceapi.AllocationResult{}). + Allocation(allocationResult). ReservedForPod(podName, types.UID(podUID)). Obj() structuredInUseClaim = st.FromResourceClaim(inUseClaim). - Structured("worker", "instance-1"). + Structured(). Obj() allocatedClaim = st.FromResourceClaim(pendingClaim). - Allocation("some-driver", &resourceapi.AllocationResult{}). + Allocation(allocationResult). Obj() - pendingClaimWithParams = st.FromResourceClaim(pendingClaim).ParametersRef(claimName).Obj() - structuredAllocatedClaim = st.FromResourceClaim(allocatedClaim).Structured("worker", "instance-1").Obj() - structuredAllocatedClaimWithParams = st.FromResourceClaim(structuredAllocatedClaim).ParametersRef(claimName).Obj() - - otherStructuredAllocatedClaim = st.FromResourceClaim(structuredAllocatedClaim).Name(structuredAllocatedClaim.Name + "-other").Obj() - allocatedClaimWithWrongTopology = st.FromResourceClaim(allocatedClaim). - Allocation("some-driver", &resourceapi.AllocationResult{AvailableOnNodes: st.MakeNodeSelector().In("no-such-label", []string{"no-such-value"}).Obj()}). + Allocation(&resourceapi.AllocationResult{Controller: controller, NodeSelector: st.MakeNodeSelector().In("no-such-label", []string{"no-such-value"}).Obj()}). Obj() - structuredAllocatedClaimWithWrongTopology = st.FromResourceClaim(allocatedClaimWithWrongTopology). - Structured("worker-2", "instance-1"). - Obj() allocatedClaimWithGoodTopology = st.FromResourceClaim(allocatedClaim). - Allocation("some-driver", &resourceapi.AllocationResult{AvailableOnNodes: st.MakeNodeSelector().In("kubernetes.io/hostname", []string{"worker"}).Obj()}). + Allocation(&resourceapi.AllocationResult{Controller: controller, NodeSelector: st.MakeNodeSelector().In("kubernetes.io/hostname", []string{nodeName}).Obj()}). Obj() - structuredAllocatedClaimWithGoodTopology = st.FromResourceClaim(allocatedClaimWithGoodTopology). - Structured("worker", "instance-1"). - Obj() - otherClaim = st.MakeResourceClaim(). + otherClaim = st.MakeResourceClaim(controller). Name("not-my-claim"). Namespace(namespace). - ResourceClassName(className). + Request(className). Obj() + otherAllocatedClaim = st.FromResourceClaim(otherClaim). + Allocation(allocationResult). + Obj() scheduling = st.MakePodSchedulingContexts().Name(podName).Namespace(namespace). OwnerReference(podName, podUID, podKind). @@ -224,38 +195,37 @@ func reserve(claim *resourceapi.ResourceClaim, pod *v1.Pod) *resourceapi.Resourc Obj() } -// claimWithCRD replaces the in-tree group with "example.com". -func claimWithCRD(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { +func structuredClaim(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { + return st.FromResourceClaim(claim). + Structured(). + Obj() +} + +func breakCELInClaim(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { claim = claim.DeepCopy() - claim.Spec.ParametersRef.APIGroup = "example.com" + for i := range claim.Spec.Devices.Requests { + for e := range claim.Spec.Devices.Requests[i].Selectors { + claim.Spec.Devices.Requests[i].Selectors[e] = brokenSelector + } + if len(claim.Spec.Devices.Requests[i].Selectors) == 0 { + claim.Spec.Devices.Requests[i].Selectors = []resourceapi.DeviceSelector{brokenSelector} + } + } return claim } -// classWithCRD replaces the in-tree group with "example.com". -func classWithCRD(class *resourceapi.ResourceClass) *resourceapi.ResourceClass { +func breakCELInClass(class *resourceapi.DeviceClass) *resourceapi.DeviceClass { class = class.DeepCopy() - class.ParametersRef.APIGroup = "example.com" + for i := range class.Spec.Selectors { + class.Spec.Selectors[i] = brokenSelector + } + if len(class.Spec.Selectors) == 0 { + class.Spec.Selectors = []resourceapi.DeviceSelector{brokenSelector} + } + return class } -func breakCELInClaimParameters(parameters *resourceapi.ResourceClaimParameters) *resourceapi.ResourceClaimParameters { - parameters = parameters.DeepCopy() - for i := range parameters.DriverRequests { - for e := range parameters.DriverRequests[i].Requests { - parameters.DriverRequests[i].Requests[e].NamedResources.Selector = `attributes.bool["no-such-attribute"]` - } - } - return parameters -} - -func breakCELInClassParameters(parameters *resourceapi.ResourceClassParameters) *resourceapi.ResourceClassParameters { - parameters = parameters.DeepCopy() - for i := range parameters.Filters { - parameters.Filters[i].NamedResources.Selector = `attributes.bool["no-such-attribute"]` - } - return parameters -} - // result defines the expected outcome of some operation. It covers // operation's status and the state of the world (= objects). type result struct { @@ -337,7 +307,7 @@ func TestPlugin(t *testing.T) { nodes []*v1.Node // default if unset is workerNode pod *v1.Pod claims []*resourceapi.ResourceClaim - classes []*resourceapi.ResourceClass + classes []*resourceapi.DeviceClass schedulings []*resourceapi.PodSchedulingContext // objs get stored directly in the fake client, without passing @@ -378,7 +348,7 @@ func TestPlugin(t *testing.T) { }, "claim-reference-structured": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{structuredAllocatedClaim, otherClaim}, + claims: []*resourceapi.ResourceClaim{structuredClaim(allocatedClaim), otherClaim}, want: want{ prebind: result{ changes: change{ @@ -412,7 +382,7 @@ func TestPlugin(t *testing.T) { }, "claim-template-structured": { pod: podWithClaimTemplateInStatus, - claims: []*resourceapi.ResourceClaim{structuredAllocatedClaim, otherClaim}, + claims: []*resourceapi.ResourceClaim{structuredClaim(allocatedClaim), otherClaim}, want: want{ prebind: result{ changes: change{ @@ -464,12 +434,12 @@ func TestPlugin(t *testing.T) { }, "structured-no-resources": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - classes: []*resourceapi.ResourceClass{structuredResourceClass}, + claims: []*resourceapi.ResourceClaim{structuredClaim(pendingClaim)}, + classes: []*resourceapi.DeviceClass{deviceClass}, want: want{ filter: perNodeResult{ workerNode.Name: { - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `resourceclaim cannot be allocated for the node (unsuitable)`), + status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `cannot allocate all claims`), }, }, postfilter: result{ @@ -479,28 +449,28 @@ func TestPlugin(t *testing.T) { }, "structured-with-resources": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - classes: []*resourceapi.ResourceClass{structuredResourceClass}, + claims: []*resourceapi.ResourceClaim{structuredClaim(pendingClaim)}, + classes: []*resourceapi.DeviceClass{deviceClass}, objs: []apiruntime.Object{workerNodeSlice}, want: want{ reserve: result{ - inFlightClaim: structuredAllocatedClaim, + inFlightClaim: structuredClaim(allocatedClaim), }, prebind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), changes: change{ claim: func(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { if claim.Name == claimName { claim = claim.DeepCopy() - claim.Finalizers = structuredAllocatedClaim.Finalizers - claim.Status = structuredInUseClaim.Status + claim.Finalizers = structuredClaim(allocatedClaim).Finalizers + claim.Status = structuredClaim(inUseClaim).Status } return claim }, }, }, postbind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), }, }, }, @@ -509,18 +479,18 @@ func TestPlugin(t *testing.T) { // the scheduler got interrupted. pod: podWithClaimName, claims: func() []*resourceapi.ResourceClaim { - claim := pendingClaim.DeepCopy() - claim.Finalizers = structuredAllocatedClaim.Finalizers + claim := structuredClaim(pendingClaim) + claim.Finalizers = structuredClaim(allocatedClaim).Finalizers return []*resourceapi.ResourceClaim{claim} }(), - classes: []*resourceapi.ResourceClass{structuredResourceClass}, + classes: []*resourceapi.DeviceClass{deviceClass}, objs: []apiruntime.Object{workerNodeSlice}, want: want{ reserve: result{ - inFlightClaim: structuredAllocatedClaim, + inFlightClaim: structuredClaim(allocatedClaim), }, prebind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), changes: change{ claim: func(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { if claim.Name == claimName { @@ -532,7 +502,7 @@ func TestPlugin(t *testing.T) { }, }, postbind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), }, }, }, @@ -541,11 +511,11 @@ func TestPlugin(t *testing.T) { // removed before the scheduler reaches PreBind. pod: podWithClaimName, claims: func() []*resourceapi.ResourceClaim { - claim := pendingClaim.DeepCopy() - claim.Finalizers = structuredAllocatedClaim.Finalizers + claim := structuredClaim(pendingClaim) + claim.Finalizers = structuredClaim(allocatedClaim).Finalizers return []*resourceapi.ResourceClaim{claim} }(), - classes: []*resourceapi.ResourceClass{structuredResourceClass}, + classes: []*resourceapi.DeviceClass{deviceClass}, objs: []apiruntime.Object{workerNodeSlice}, prepare: prepare{ prebind: change{ @@ -557,15 +527,15 @@ func TestPlugin(t *testing.T) { }, want: want{ reserve: result{ - inFlightClaim: structuredAllocatedClaim, + inFlightClaim: structuredClaim(allocatedClaim), }, prebind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), changes: change{ claim: func(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { if claim.Name == claimName { claim = claim.DeepCopy() - claim.Finalizers = structuredAllocatedClaim.Finalizers + claim.Finalizers = structuredClaim(allocatedClaim).Finalizers claim.Status = structuredInUseClaim.Status } return claim @@ -573,7 +543,7 @@ func TestPlugin(t *testing.T) { }, }, postbind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), }, }, }, @@ -581,23 +551,23 @@ func TestPlugin(t *testing.T) { // No finalizer initially, then it gets added before // the scheduler reaches PreBind. Shouldn't happen? pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - classes: []*resourceapi.ResourceClass{structuredResourceClass}, + claims: []*resourceapi.ResourceClaim{structuredClaim(pendingClaim)}, + classes: []*resourceapi.DeviceClass{deviceClass}, objs: []apiruntime.Object{workerNodeSlice}, prepare: prepare{ prebind: change{ claim: func(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { - claim.Finalizers = structuredAllocatedClaim.Finalizers + claim.Finalizers = structuredClaim(allocatedClaim).Finalizers return claim }, }, }, want: want{ reserve: result{ - inFlightClaim: structuredAllocatedClaim, + inFlightClaim: structuredClaim(allocatedClaim), }, prebind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), changes: change{ claim: func(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { if claim.Name == claimName { @@ -609,31 +579,31 @@ func TestPlugin(t *testing.T) { }, }, postbind: result{ - assumedClaim: reserve(structuredAllocatedClaim, podWithClaimName), + assumedClaim: reserve(structuredClaim(allocatedClaim), podWithClaimName), }, }, }, "structured-skip-bind": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - classes: []*resourceapi.ResourceClass{structuredResourceClass}, + claims: []*resourceapi.ResourceClaim{structuredClaim(pendingClaim)}, + classes: []*resourceapi.DeviceClass{deviceClass}, objs: []apiruntime.Object{workerNodeSlice}, want: want{ reserve: result{ - inFlightClaim: structuredAllocatedClaim, + inFlightClaim: structuredClaim(allocatedClaim), }, unreserveBeforePreBind: &result{}, }, }, "structured-exhausted-resources": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim, otherStructuredAllocatedClaim}, - classes: []*resourceapi.ResourceClass{structuredResourceClass}, + claims: []*resourceapi.ResourceClaim{structuredClaim(pendingClaim), structuredClaim(otherAllocatedClaim)}, + classes: []*resourceapi.DeviceClass{deviceClass}, objs: []apiruntime.Object{workerNodeSlice}, want: want{ filter: perNodeResult{ workerNode.Name: { - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `resourceclaim cannot be allocated for the node (unsuitable)`), + status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `cannot allocate all claims`), }, }, postfilter: result{ @@ -642,182 +612,70 @@ func TestPlugin(t *testing.T) { }, }, - "with-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaimWithParams}, - classes: []*resourceapi.ResourceClass{structuredResourceClassWithParams}, - objs: []apiruntime.Object{claimParameters, classParameters, workerNodeSlice}, - want: want{ - reserve: result{ - inFlightClaim: structuredAllocatedClaimWithParams, - }, - prebind: result{ - assumedClaim: reserve(structuredAllocatedClaimWithParams, podWithClaimName), - changes: change{ - claim: func(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { - if claim.Name == claimName { - claim = claim.DeepCopy() - claim.Finalizers = structuredAllocatedClaim.Finalizers - claim.Status = structuredInUseClaim.Status - } - return claim - }, - }, - }, - postbind: result{ - assumedClaim: reserve(structuredAllocatedClaimWithParams, podWithClaimName), - }, - }, - }, - - "with-translated-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{claimWithCRD(pendingClaimWithParams)}, - classes: []*resourceapi.ResourceClass{classWithCRD(structuredResourceClassWithCRD)}, - objs: []apiruntime.Object{claimParameters, claimParametersOtherNamespace /* must be ignored */, classParameters, workerNodeSlice}, - want: want{ - reserve: result{ - inFlightClaim: claimWithCRD(structuredAllocatedClaimWithParams), - }, - prebind: result{ - assumedClaim: reserve(claimWithCRD(structuredAllocatedClaimWithParams), podWithClaimName), - changes: change{ - claim: func(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { - if claim.Name == claimName { - claim = claim.DeepCopy() - claim.Finalizers = structuredAllocatedClaim.Finalizers - claim.Status = structuredInUseClaim.Status - } - return claim - }, - }, - }, - postbind: result{ - assumedClaim: reserve(claimWithCRD(structuredAllocatedClaimWithParams), podWithClaimName), - }, - }, - }, - - "missing-class-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaimWithParams}, - classes: []*resourceapi.ResourceClass{structuredResourceClassWithParams}, - objs: []apiruntime.Object{claimParameters, workerNodeSlice}, - want: want{ - prefilter: result{ - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `class parameters default/my-resource-class not found`), - }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`), - }, - }, - }, - - "missing-claim-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaimWithParams}, - classes: []*resourceapi.ResourceClass{structuredResourceClassWithParams}, - objs: []apiruntime.Object{classParameters, workerNodeSlice}, - want: want{ - prefilter: result{ - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `claim parameters default/my-pod-my-resource not found`), - }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`), - }, - }, - }, - - "missing-translated-class-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{claimWithCRD(pendingClaimWithParams)}, - classes: []*resourceapi.ResourceClass{classWithCRD(structuredResourceClassWithCRD)}, - objs: []apiruntime.Object{claimParameters, workerNodeSlice}, - want: want{ - prefilter: result{ - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `generated class parameters for ResourceClassParameters.example.com default/my-resource-class not found`), - }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`), - }, - }, - }, - - "missing-translated-claim-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{claimWithCRD(pendingClaimWithParams)}, - classes: []*resourceapi.ResourceClass{classWithCRD(structuredResourceClassWithCRD)}, - objs: []apiruntime.Object{classParameters, workerNodeSlice}, - want: want{ - prefilter: result{ - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `generated claim parameters for ResourceClaimParameters.example.com default/my-pod-my-resource not found`), - }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`), - }, - }, - }, - - "too-many-translated-class-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{claimWithCRD(pendingClaimWithParams)}, - classes: []*resourceapi.ResourceClass{classWithCRD(structuredResourceClassWithCRD)}, - objs: []apiruntime.Object{claimParameters, classParameters, st.FromClassParameters(classParameters).Name("other").Obj() /* too many */, workerNodeSlice}, - want: want{ - prefilter: result{ - status: framework.AsStatus(errors.New(`multiple generated class parameters for ResourceClassParameters.example.com my-resource-class found: [default/my-resource-class default/other]`)), - }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`), - }, - }, - }, - - "too-many-translated-claim-parameters": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{claimWithCRD(pendingClaimWithParams)}, - classes: []*resourceapi.ResourceClass{classWithCRD(structuredResourceClassWithCRD)}, - objs: []apiruntime.Object{claimParameters, st.FromClaimParameters(claimParameters).Name("other").Obj() /* too many */, classParameters, workerNodeSlice}, - want: want{ - prefilter: result{ - status: framework.AsStatus(errors.New(`multiple generated claim parameters for ResourceClaimParameters.example.com default/my-pod-my-resource found: [default/my-pod-my-resource default/other]`)), - }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`), - }, - }, - }, - "claim-parameters-CEL-runtime-error": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaimWithParams}, - classes: []*resourceapi.ResourceClass{structuredResourceClassWithParams}, - objs: []apiruntime.Object{breakCELInClaimParameters(claimParameters), classParameters, workerNodeSlice}, + claims: []*resourceapi.ResourceClaim{breakCELInClaim(structuredClaim(pendingClaim))}, + classes: []*resourceapi.DeviceClass{deviceClass}, + objs: []apiruntime.Object{workerNodeSlice}, want: want{ filter: perNodeResult{ workerNode.Name: { - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `checking structured parameters failed: checking node "worker" and resources of driver "some-driver": evaluate request CEL expression: no such key: no-such-attribute`), + status: framework.AsStatus(errors.New(`claim default/my-pod-my-resource: selector #0: CEL runtime error: no such key: ` + string(attrName))), }, }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `still not schedulable`), - }, }, }, "class-parameters-CEL-runtime-error": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaimWithParams}, - classes: []*resourceapi.ResourceClass{structuredResourceClassWithParams}, - objs: []apiruntime.Object{claimParameters, breakCELInClassParameters(classParameters), workerNodeSlice}, + claims: []*resourceapi.ResourceClaim{structuredClaim(pendingClaim)}, + classes: []*resourceapi.DeviceClass{breakCELInClass(deviceClass)}, + objs: []apiruntime.Object{workerNodeSlice}, want: want{ filter: perNodeResult{ workerNode.Name: { - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `checking structured parameters failed: checking node "worker" and resources of driver "some-driver": evaluate filter CEL expression: no such key: no-such-attribute`), + status: framework.AsStatus(errors.New(`class my-resource-class: selector #0: CEL runtime error: no such key: ` + string(attrName))), }, }, - postfilter: result{ - status: framework.NewStatus(framework.Unschedulable, `still not schedulable`), + }, + }, + + // When pod scheduling encounters CEL runtime errors for some nodes, but not all, + // it should still not schedule the pod because there is something wrong with it. + // Scheduling it would make it harder to detect that there is a problem. + // + // This matches the "keeps pod pending because of CEL runtime errors" E2E test. + "CEL-runtime-error-for-one-of-two-nodes": { + nodes: []*v1.Node{workerNode, workerNode2}, + pod: podWithClaimName, + claims: []*resourceapi.ResourceClaim{breakCELInClaim(structuredClaim(pendingClaim))}, + classes: []*resourceapi.DeviceClass{deviceClass}, + objs: []apiruntime.Object{workerNodeSlice, workerNode2Slice}, + want: want{ + filter: perNodeResult{ + workerNode.Name: { + status: framework.AsStatus(errors.New(`claim default/my-pod-my-resource: selector #0: CEL runtime error: no such key: ` + string(attrName))), + }, + }, + }, + }, + + // When two nodes where found, PreScore gets called. + "CEL-runtime-error-for-one-of-three-nodes": { + nodes: []*v1.Node{workerNode, workerNode2, workerNode3}, + pod: podWithClaimName, + claims: []*resourceapi.ResourceClaim{breakCELInClaim(structuredClaim(pendingClaim))}, + classes: []*resourceapi.DeviceClass{deviceClass}, + objs: []apiruntime.Object{workerNodeSlice, workerNode2Slice, workerNode3Slice}, + want: want{ + filter: perNodeResult{ + workerNode.Name: { + status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `claim default/my-pod-my-resource: selector #0: CEL runtime error: no such key: `+string(attrName)), + }, + }, + prescore: result{ + // This is the error found during Filter. + status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `filter node worker: claim default/my-pod-my-resource: selector #0: CEL runtime error: no such key: healthy`), }, }, }, @@ -839,7 +697,7 @@ func TestPlugin(t *testing.T) { claims: []*resourceapi.ResourceClaim{pendingClaim}, want: want{ prefilter: result{ - status: framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("resource class %s does not exist", className)), + status: framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("request req-1: device class %s does not exist", className)), }, postfilter: result{ status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`), @@ -851,7 +709,7 @@ func TestPlugin(t *testing.T) { // and select a node. pod: podWithClaimName, claims: []*resourceapi.ResourceClaim{pendingClaim}, - classes: []*resourceapi.ResourceClass{resourceClass}, + classes: []*resourceapi.DeviceClass{deviceClass}, want: want{ prebind: result{ status: framework.NewStatus(framework.Pending, `waiting for resource driver`), @@ -865,7 +723,7 @@ func TestPlugin(t *testing.T) { // there are multiple claims. pod: podWithTwoClaimNames, claims: []*resourceapi.ResourceClaim{pendingClaim, pendingClaim2}, - classes: []*resourceapi.ResourceClass{resourceClass}, + classes: []*resourceapi.DeviceClass{deviceClass}, want: want{ prebind: result{ status: framework.NewStatus(framework.Pending, `waiting for resource driver`), @@ -879,7 +737,7 @@ func TestPlugin(t *testing.T) { pod: podWithClaimName, claims: []*resourceapi.ResourceClaim{pendingClaim}, schedulings: []*resourceapi.PodSchedulingContext{schedulingInfo}, - classes: []*resourceapi.ResourceClass{resourceClass}, + classes: []*resourceapi.DeviceClass{deviceClass}, want: want{ prebind: result{ status: framework.NewStatus(framework.Pending, `waiting for resource driver`), @@ -899,7 +757,7 @@ func TestPlugin(t *testing.T) { pod: podWithClaimName, claims: []*resourceapi.ResourceClaim{pendingClaim}, schedulings: []*resourceapi.PodSchedulingContext{schedulingInfo}, - classes: []*resourceapi.ResourceClass{resourceClass}, + classes: []*resourceapi.DeviceClass{deviceClass}, prepare: prepare{ prebind: change{ scheduling: func(in *resourceapi.PodSchedulingContext) *resourceapi.PodSchedulingContext { @@ -923,7 +781,7 @@ func TestPlugin(t *testing.T) { pod: podWithClaimName, claims: []*resourceapi.ResourceClaim{allocatedClaim}, schedulings: []*resourceapi.PodSchedulingContext{schedulingInfo}, - classes: []*resourceapi.ResourceClass{resourceClass}, + classes: []*resourceapi.DeviceClass{deviceClass}, want: want{ prebind: result{ changes: change{ @@ -967,7 +825,7 @@ func TestPlugin(t *testing.T) { // PostFilter tries to get the pod scheduleable by // deallocating the claim. pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{structuredAllocatedClaimWithWrongTopology}, + claims: []*resourceapi.ResourceClaim{structuredClaim(allocatedClaimWithWrongTopology)}, want: want{ filter: perNodeResult{ workerNode.Name: { @@ -979,7 +837,7 @@ func TestPlugin(t *testing.T) { changes: change{ claim: func(in *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { return st.FromResourceClaim(in). - Allocation("", nil). + Allocation(nil). Obj() }, }, @@ -1028,7 +886,7 @@ func TestPlugin(t *testing.T) { }, "bind-failure-structured": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{structuredAllocatedClaimWithGoodTopology}, + claims: []*resourceapi.ResourceClaim{structuredClaim(allocatedClaimWithGoodTopology)}, want: want{ prebind: result{ changes: change{ @@ -1109,15 +967,20 @@ func TestPlugin(t *testing.T) { t.Run(fmt.Sprintf("filter/%s", nodeInfo.Node().Name), func(t *testing.T) { testCtx.verify(t, tc.want.filter.forNode(nodeName), initialObjects, nil, status) }) - if status.Code() != framework.Success { - unschedulable = true - } else { + if status.Code() == framework.Success { potentialNodes = append(potentialNodes, nodeInfo) } + if status.Code() == framework.Error { + // An error aborts scheduling. + return + } + } + if len(potentialNodes) == 0 { + unschedulable = true } } - if !unschedulable && len(potentialNodes) > 0 { + if !unschedulable && len(potentialNodes) > 1 { initialObjects = testCtx.listAll(t) initialObjects = testCtx.updateAPIServer(t, initialObjects, tc.prepare.prescore) status := testCtx.p.PreScore(testCtx.ctx, testCtx.state, tc.pod, potentialNodes) @@ -1184,7 +1047,7 @@ func TestPlugin(t *testing.T) { }) } } - } else { + } else if len(potentialNodes) == 0 { initialObjects = testCtx.listAll(t) initialObjects = testCtx.updateAPIServer(t, initialObjects, tc.prepare.postfilter) result, status := testCtx.p.PostFilter(testCtx.ctx, testCtx.state, tc.pod, nil /* filteredNodeStatusMap not used by plugin */) @@ -1209,7 +1072,12 @@ type testContext struct { func (tc *testContext) verify(t *testing.T, expected result, initialObjects []metav1.Object, result interface{}, status *framework.Status) { t.Helper() - assert.Equal(t, expected.status, status) + if expectedErr := status.AsError(); expectedErr != nil { + // Compare only the error strings. + assert.ErrorContains(t, status.AsError(), expectedErr.Error()) + } else { + assert.Equal(t, expected.status, status) + } objects := tc.listAll(t) wantObjects := update(t, initialObjects, expected.changes) wantObjects = append(wantObjects, expected.added...) @@ -1351,7 +1219,7 @@ func update(t *testing.T, objects []metav1.Object, updates change) []metav1.Obje return updated } -func setup(t *testing.T, nodes []*v1.Node, claims []*resourceapi.ResourceClaim, classes []*resourceapi.ResourceClass, schedulings []*resourceapi.PodSchedulingContext, objs []apiruntime.Object) (result *testContext) { +func setup(t *testing.T, nodes []*v1.Node, claims []*resourceapi.ResourceClaim, classes []*resourceapi.DeviceClass, schedulings []*resourceapi.PodSchedulingContext, objs []apiruntime.Object) (result *testContext) { t.Helper() tc := &testContext{} @@ -1387,7 +1255,7 @@ func setup(t *testing.T, nodes []*v1.Node, claims []*resourceapi.ResourceClaim, require.NoError(t, err, "create resource claim") } for _, class := range classes { - _, err := tc.client.ResourceV1alpha3().ResourceClasses().Create(tc.ctx, class, metav1.CreateOptions{}) + _, err := tc.client.ResourceV1alpha3().DeviceClasses().Create(tc.ctx, class, metav1.CreateOptions{}) require.NoError(t, err, "create resource class") } for _, scheduling := range schedulings { @@ -1552,10 +1420,10 @@ func Test_isSchedulableAfterClaimChange(t *testing.T) { }, "structured-claim-deallocate": { pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim, otherStructuredAllocatedClaim}, - oldObj: otherStructuredAllocatedClaim, + claims: []*resourceapi.ResourceClaim{pendingClaim, structuredClaim(otherAllocatedClaim)}, + oldObj: structuredClaim(otherAllocatedClaim), newObj: func() *resourceapi.ResourceClaim { - claim := otherStructuredAllocatedClaim.DeepCopy() + claim := structuredClaim(otherAllocatedClaim).DeepCopy() claim.Status.Allocation = nil return claim }(), diff --git a/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel.go b/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel.go deleted file mode 100644 index cdc2654d9aa..00000000000 --- a/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel.go +++ /dev/null @@ -1,153 +0,0 @@ -/* -Copyright 2024 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 namedresources - -import ( - "context" - "errors" - "fmt" - "slices" - - resourceapi "k8s.io/api/resource/v1alpha3" - "k8s.io/apiserver/pkg/cel/environment" - "k8s.io/dynamic-resource-allocation/structured/namedresources/cel" -) - -// These types and fields are all exported to allow logging them with -// pretty-printed JSON. - -type Model struct { - Instances []InstanceAllocation -} - -type InstanceAllocation struct { - Allocated bool - Instance *resourceapi.NamedResourcesInstance -} - -// AddResources must be called first to create entries for all existing -// resource instances. The resources parameter may be nil. -func AddResources(m *Model, resources *resourceapi.NamedResourcesResources) { - if resources == nil { - return - } - - for i := range resources.Instances { - m.Instances = append(m.Instances, InstanceAllocation{Instance: &resources.Instances[i]}) - } -} - -// AddAllocation may get called after AddResources to mark some resource -// instances as allocated. The result parameter may be nil. -func AddAllocation(m *Model, result *resourceapi.NamedResourcesAllocationResult) { - if result == nil { - return - } - for i := range m.Instances { - if m.Instances[i].Instance.Name == result.Name { - m.Instances[i].Allocated = true - break - } - } -} - -func NewClaimController(filter *resourceapi.NamedResourcesFilter, requests []*resourceapi.NamedResourcesRequest) (*Controller, error) { - c := &Controller{} - if filter != nil { - compilation := cel.GetCompiler().CompileCELExpression(filter.Selector, environment.StoredExpressions) - if compilation.Error != nil { - // Shouldn't happen because of validation. - return nil, fmt.Errorf("compile class filter CEL expression: %w", compilation.Error) - } - c.filter = &compilation - } - for _, request := range requests { - compilation := cel.GetCompiler().CompileCELExpression(request.Selector, environment.StoredExpressions) - if compilation.Error != nil { - // Shouldn't happen because of validation. - return nil, fmt.Errorf("compile request CEL expression: %w", compilation.Error) - } - c.requests = append(c.requests, compilation) - } - return c, nil -} - -type Controller struct { - filter *cel.CompilationResult - requests []cel.CompilationResult -} - -func (c *Controller) NodeIsSuitable(ctx context.Context, model Model) (bool, error) { - indices, err := c.allocate(ctx, model) - return len(indices) == len(c.requests), err -} - -func (c *Controller) Allocate(ctx context.Context, model Model) ([]*resourceapi.NamedResourcesAllocationResult, error) { - indices, err := c.allocate(ctx, model) - if err != nil { - return nil, err - } - if len(indices) != len(c.requests) { - return nil, errors.New("insufficient resources") - } - results := make([]*resourceapi.NamedResourcesAllocationResult, len(c.requests)) - for i := range c.requests { - results[i] = &resourceapi.NamedResourcesAllocationResult{Name: model.Instances[indices[i]].Instance.Name} - } - return results, nil -} - -func (c *Controller) allocate(ctx context.Context, model Model) ([]int, error) { - // Shallow copy, we need to modify the allocated boolean. - instances := slices.Clone(model.Instances) - indices := make([]int, 0, len(c.requests)) - - for _, request := range c.requests { - for i, instance := range instances { - if instance.Allocated { - continue - } - if c.filter != nil { - okay, err := c.filter.Evaluate(ctx, instance.Instance.Attributes) - if err != nil { - return nil, fmt.Errorf("evaluate filter CEL expression: %w", err) - } - if !okay { - continue - } - } - okay, err := request.Evaluate(ctx, instance.Instance.Attributes) - if err != nil { - return nil, fmt.Errorf("evaluate request CEL expression: %w", err) - } - if !okay { - continue - } - // Found a matching, unallocated instance. Let's use it. - // - // A more thorough search would include backtracking because - // allocating one "large" instances for a "small" request may - // make a following "large" request impossible to satisfy when - // only "small" instances are left. - instances[i].Allocated = true - indices = append(indices, i) - break - } - } - return indices, nil - -} diff --git a/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go b/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go deleted file mode 100644 index d0d8ef60243..00000000000 --- a/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go +++ /dev/null @@ -1,327 +0,0 @@ -/* -Copyright 2024 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 namedresources - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - resourceapi "k8s.io/api/resource/v1alpha3" - "k8s.io/kubernetes/test/utils/ktesting" - "k8s.io/utils/ptr" -) - -func instance(allocated bool, name string, attributes ...resourceapi.NamedResourcesAttribute) InstanceAllocation { - return InstanceAllocation{ - Allocated: allocated, - Instance: &resourceapi.NamedResourcesInstance{ - Name: name, - Attributes: attributes, - }, - } -} - -func TestModel(t *testing.T) { - testcases := map[string]struct { - resources []*resourceapi.NamedResourcesResources - allocations []*resourceapi.NamedResourcesAllocationResult - - expectModel Model - }{ - "empty": {}, - - "nil": { - resources: []*resourceapi.NamedResourcesResources{nil}, - allocations: []*resourceapi.NamedResourcesAllocationResult{nil}, - }, - - "available": { - resources: []*resourceapi.NamedResourcesResources{ - { - Instances: []resourceapi.NamedResourcesInstance{ - {Name: "a"}, - {Name: "b"}, - }, - }, - { - Instances: []resourceapi.NamedResourcesInstance{ - {Name: "x"}, - {Name: "y"}, - }, - }, - }, - - expectModel: Model{Instances: []InstanceAllocation{instance(false, "a"), instance(false, "b"), instance(false, "x"), instance(false, "y")}}, - }, - - "allocated": { - resources: []*resourceapi.NamedResourcesResources{ - { - Instances: []resourceapi.NamedResourcesInstance{ - {Name: "a"}, - {Name: "b"}, - }, - }, - { - Instances: []resourceapi.NamedResourcesInstance{ - {Name: "x"}, - {Name: "y"}, - }, - }, - }, - allocations: []*resourceapi.NamedResourcesAllocationResult{ - { - Name: "something-else", - }, - { - Name: "a", - }, - }, - - expectModel: Model{Instances: []InstanceAllocation{instance(true, "a"), instance(false, "b"), instance(false, "x"), instance(false, "y")}}, - }, - } - - for name, tc := range testcases { - t.Run(name, func(t *testing.T) { - var actualModel Model - for _, resources := range tc.resources { - AddResources(&actualModel, resources) - } - for _, allocation := range tc.allocations { - AddAllocation(&actualModel, allocation) - } - - require.Equal(t, tc.expectModel, actualModel) - }) - } - -} - -func TestController(t *testing.T) { - filterAny := &resourceapi.NamedResourcesFilter{ - Selector: "true", - } - filterNone := &resourceapi.NamedResourcesFilter{ - Selector: "false", - } - filterBrokenType := &resourceapi.NamedResourcesFilter{ - Selector: "1", - } - filterBrokenEvaluation := &resourceapi.NamedResourcesFilter{ - Selector: `attributes.bool["no-such-attribute"]`, - } - filterAttribute := &resourceapi.NamedResourcesFilter{ - Selector: `attributes.bool["usable"]`, - } - - requestAny := &resourceapi.NamedResourcesRequest{ - Selector: "true", - } - requestNone := &resourceapi.NamedResourcesRequest{ - Selector: "false", - } - requestBrokenType := &resourceapi.NamedResourcesRequest{ - Selector: "1", - } - requestBrokenEvaluation := &resourceapi.NamedResourcesRequest{ - Selector: `attributes.bool["no-such-attribute"]`, - } - requestAttribute := &resourceapi.NamedResourcesRequest{ - Selector: `attributes.bool["usable"]`, - } - - instance1 := "instance-1" - oneInstance := Model{ - Instances: []InstanceAllocation{{ - Instance: &resourceapi.NamedResourcesInstance{ - Name: instance1, - }, - }}, - } - - instance2 := "instance-2" - twoInstances := Model{ - Instances: []InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{ - Name: instance1, - Attributes: []resourceapi.NamedResourcesAttribute{{ - Name: "usable", - NamedResourcesAttributeValue: resourceapi.NamedResourcesAttributeValue{ - BoolValue: ptr.To(false), - }, - }}, - }, - }, - { - Instance: &resourceapi.NamedResourcesInstance{ - Name: instance2, - Attributes: []resourceapi.NamedResourcesAttribute{{ - Name: "usable", - NamedResourcesAttributeValue: resourceapi.NamedResourcesAttributeValue{ - BoolValue: ptr.To(true), - }, - }}, - }, - }, - }, - } - - testcases := map[string]struct { - model Model - filter *resourceapi.NamedResourcesFilter - requests []*resourceapi.NamedResourcesRequest - - expectCreateErr bool - expectAllocation []string - expectAllocateErr bool - }{ - "empty": {}, - - "broken-filter": { - filter: filterBrokenType, - - expectCreateErr: true, - }, - - "broken-request": { - requests: []*resourceapi.NamedResourcesRequest{requestBrokenType}, - - expectCreateErr: true, - }, - - "no-resources": { - filter: filterAny, - requests: []*resourceapi.NamedResourcesRequest{requestAny}, - - expectAllocateErr: true, - }, - - "okay": { - model: oneInstance, - filter: filterAny, - requests: []*resourceapi.NamedResourcesRequest{requestAny}, - - expectAllocation: []string{instance1}, - }, - - "filter-mismatch": { - model: oneInstance, - filter: filterNone, - requests: []*resourceapi.NamedResourcesRequest{requestAny}, - - expectAllocateErr: true, - }, - - "request-mismatch": { - model: oneInstance, - filter: filterAny, - requests: []*resourceapi.NamedResourcesRequest{requestNone}, - - expectAllocateErr: true, - }, - - "many": { - model: twoInstances, - filter: filterAny, - requests: []*resourceapi.NamedResourcesRequest{requestAny, requestAny}, - - expectAllocation: []string{instance1, instance2}, - }, - - "too-many": { - model: oneInstance, - filter: filterAny, - requests: []*resourceapi.NamedResourcesRequest{requestAny, requestAny}, - - expectAllocateErr: true, - }, - - "filter-evaluation-error": { - model: oneInstance, - filter: filterBrokenEvaluation, - requests: []*resourceapi.NamedResourcesRequest{requestAny}, - - expectAllocateErr: true, - }, - - "request-evaluation-error": { - model: oneInstance, - filter: filterAny, - requests: []*resourceapi.NamedResourcesRequest{requestBrokenEvaluation}, - - expectAllocateErr: true, - }, - - "filter-attribute": { - model: twoInstances, - filter: filterAttribute, - requests: []*resourceapi.NamedResourcesRequest{requestAny}, - - expectAllocation: []string{instance2}, - }, - - "request-attribute": { - model: twoInstances, - filter: filterAny, - requests: []*resourceapi.NamedResourcesRequest{requestAttribute}, - - expectAllocation: []string{instance2}, - }, - } - - for name, tc := range testcases { - t.Run(name, func(t *testing.T) { - tCtx := ktesting.Init(t) - - controller, createErr := NewClaimController(tc.filter, tc.requests) - if createErr != nil { - if !tc.expectCreateErr { - tCtx.Fatalf("unexpected create error: %v", createErr) - } - return - } - if tc.expectCreateErr { - tCtx.Fatalf("did not get expected create error") - } - - allocation, createErr := controller.Allocate(tCtx, tc.model) - if createErr != nil { - if !tc.expectAllocateErr { - tCtx.Fatalf("unexpected allocate error: %v", createErr) - } - return - } - if tc.expectAllocateErr { - tCtx.Fatalf("did not get expected allocate error") - } - - expectAllocation := []*resourceapi.NamedResourcesAllocationResult{} - for _, name := range tc.expectAllocation { - expectAllocation = append(expectAllocation, &resourceapi.NamedResourcesAllocationResult{Name: name}) - } - require.Equal(tCtx, expectAllocation, allocation) - - isSuitable, isSuitableErr := controller.NodeIsSuitable(tCtx, tc.model) - assert.Equal(tCtx, len(expectAllocation) == len(tc.requests), isSuitable, "is suitable") - assert.Equal(tCtx, createErr, isSuitableErr) - }) - } -} diff --git a/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters.go b/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters.go deleted file mode 100644 index cd100db742a..00000000000 --- a/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters.go +++ /dev/null @@ -1,274 +0,0 @@ -/* -Copyright 2024 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 dynamicresources - -import ( - "context" - "fmt" - "sync" - - v1 "k8s.io/api/core/v1" - resourceapi "k8s.io/api/resource/v1alpha3" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/klog/v2" - namedresourcesmodel "k8s.io/kubernetes/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources" -) - -// resources is a map "node name" -> "driver name" -> available and -// allocated resources per structured parameter model. -type resources map[string]map[string]ResourceModels - -// ResourceModels may have more than one entry because it is valid for a driver to -// use more than one structured parameter model. -type ResourceModels struct { - NamedResources namedresourcesmodel.Model -} - -// resourceSliceLister is the subset of resourcelisters.ResourceSliceLister needed by -// newResourceModel. -type resourceSliceLister interface { - List(selector labels.Selector) (ret []*resourceapi.ResourceSlice, err error) -} - -// assumeCacheLister is the subset of volumebinding.AssumeCache needed by newResourceModel. -type assumeCacheLister interface { - List(indexObj interface{}) []interface{} -} - -// newResourceModel parses the available information about resources. Objects -// with an unknown structured parameter model silently ignored. An error gets -// logged later when parameters required for a pod depend on such an unknown -// model. -func newResourceModel(logger klog.Logger, resourceSliceLister resourceSliceLister, claimAssumeCache assumeCacheLister, inFlightAllocations *sync.Map) (resourceMap, error) { - model := make(resourceMap) - - slices, err := resourceSliceLister.List(labels.Everything()) - if err != nil { - return nil, fmt.Errorf("list node resource slices: %w", err) - } - for _, slice := range slices { - if slice.NamedResources == nil { - // Ignore unknown resource. We don't know what it is, - // so we cannot allocated anything depending on - // it. This is only an error if we actually see a claim - // which needs this unknown model. - continue - } - instances := slice.NamedResources.Instances - if model[slice.NodeName] == nil { - model[slice.NodeName] = make(map[string]Resources) - } - resources := model[slice.NodeName][slice.DriverName] - resources.Instances = make([]Instance, 0, len(instances)) - for i := range instances { - instance := Instance{ - NodeName: slice.NodeName, - DriverName: slice.DriverName, - NamedResourcesInstance: &instances[i], - } - resources.Instances = append(resources.Instances, instance) - } - model[slice.NodeName][slice.DriverName] = resources - } - - objs := claimAssumeCache.List(nil) - for _, obj := range objs { - claim, ok := obj.(*resourceapi.ResourceClaim) - if !ok { - return nil, fmt.Errorf("got unexpected object of type %T from claim assume cache", obj) - } - if obj, ok := inFlightAllocations.Load(claim.UID); ok { - // If the allocation is in-flight, then we have to use the allocation - // from that claim. - claim = obj.(*resourceapi.ResourceClaim) - } - if claim.Status.Allocation == nil { - continue - } - for _, handle := range claim.Status.Allocation.ResourceHandles { - structured := handle.StructuredData - if structured == nil { - continue - } - if model[structured.NodeName] == nil { - model[structured.NodeName] = make(map[string]Resources) - } - resources := model[structured.NodeName][handle.DriverName] - for _, result := range structured.Results { - // Same as above: if we don't know the allocation result model, ignore it. - if result.NamedResources == nil { - continue - } - instanceName := result.NamedResources.Name - for i := range resources.Instances { - if resources.Instances[i].NamedResourcesInstance.Name == instanceName { - resources.Instances[i].Allocated = true - break - } - } - // It could be that we don't know the instance. That's okay, - // we simply ignore the allocation result. - } - } - } - - return model, nil -} - -func newClaimController(logger klog.Logger, class *resourceapi.ResourceClass, classParameters *resourceapi.ResourceClassParameters, claimParameters *resourceapi.ResourceClaimParameters) (*claimController, error) { - // Each node driver is separate from the others. Each driver may have - // multiple requests which need to be allocated together, so here - // we have to collect them per model. - type perDriverRequests struct { - parameters []runtime.RawExtension - requests []*resourceapi.NamedResourcesRequest - } - namedresourcesRequests := make(map[string]perDriverRequests) - for i, request := range claimParameters.DriverRequests { - driverName := request.DriverName - p := namedresourcesRequests[driverName] - for e, request := range request.Requests { - switch { - case request.ResourceRequestModel.NamedResources != nil: - p.parameters = append(p.parameters, request.VendorParameters) - p.requests = append(p.requests, request.ResourceRequestModel.NamedResources) - default: - return nil, fmt.Errorf("claim parameters %s: driverRequests[%d].requests[%d]: no supported structured parameters found", klog.KObj(claimParameters), i, e) - } - } - if len(p.requests) > 0 { - namedresourcesRequests[driverName] = p - } - } - - c := &claimController{ - class: class, - classParameters: classParameters, - claimParameters: claimParameters, - namedresources: make(map[string]perDriverController, len(namedresourcesRequests)), - } - for driverName, perDriver := range namedresourcesRequests { - var filter *resourceapi.NamedResourcesFilter - for _, f := range classParameters.Filters { - if f.DriverName == driverName && f.ResourceFilterModel.NamedResources != nil { - filter = f.ResourceFilterModel.NamedResources - break - } - } - controller, err := namedresourcesmodel.NewClaimController(filter, perDriver.requests) - if err != nil { - return nil, fmt.Errorf("creating claim controller for named resources structured model: %w", err) - } - c.namedresources[driverName] = perDriverController{ - parameters: perDriver.parameters, - controller: controller, - } - } - return c, nil -} - -// claimController currently wraps exactly one structured parameter model. - -type claimController struct { - class *resourceapi.ResourceClass - classParameters *resourceapi.ResourceClassParameters - claimParameters *resourceapi.ResourceClaimParameters - namedresources map[string]perDriverController -} - -type perDriverController struct { - parameters []runtime.RawExtension - controller *namedresourcesmodel.Controller -} - -func (c claimController) nodeIsSuitable(ctx context.Context, nodeName string, resources resources) (bool, error) { - nodeResources := resources[nodeName] - for driverName, perDriver := range c.namedresources { - okay, err := perDriver.controller.NodeIsSuitable(ctx, nodeResources[driverName].NamedResources) - if err != nil { - // This is an error in the CEL expression which needs - // to be fixed. Better fail very visibly instead of - // ignoring the node. - return false, fmt.Errorf("checking node %q and resources of driver %q: %w", nodeName, driverName, err) - } - if !okay { - return false, nil - } - } - return true, nil -} - -func (c claimController) allocate(ctx context.Context, nodeName string, resources resources) (string, *resourceapi.AllocationResult, error) { - allocation := &resourceapi.AllocationResult{ - AvailableOnNodes: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - {Key: "kubernetes.io/hostname", Operator: v1.NodeSelectorOpIn, Values: []string{nodeName}}, - }, - }, - }, - }, - } - - nodeResources := resources[nodeName] - for driverName, perDriver := range c.namedresources { - // Must return one entry for each request. The entry may be nil. This way, - // the result can be correlated with the per-request parameters. - results, err := perDriver.controller.Allocate(ctx, nodeResources[driverName].NamedResources) - if err != nil { - return "", nil, fmt.Errorf("allocating via named resources structured model: %w", err) - } - handle := resourceapi.ResourceHandle{ - DriverName: driverName, - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: nodeName, - }, - } - for i, result := range results { - if result == nil { - continue - } - handle.StructuredData.Results = append(handle.StructuredData.Results, - resourceapi.DriverAllocationResult{ - VendorRequestParameters: perDriver.parameters[i], - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: result, - }, - }, - ) - } - if c.classParameters != nil { - for _, p := range c.classParameters.VendorParameters { - if p.DriverName == driverName { - handle.StructuredData.VendorClassParameters = p.Parameters - break - } - } - } - for _, request := range c.claimParameters.DriverRequests { - if request.DriverName == driverName { - handle.StructuredData.VendorClaimParameters = request.VendorParameters - break - } - } - allocation.ResourceHandles = append(allocation.ResourceHandles, handle) - } - - return c.class.DriverName, allocation, nil -} diff --git a/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go b/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go deleted file mode 100644 index 43b33198d19..00000000000 --- a/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go +++ /dev/null @@ -1,1257 +0,0 @@ -/* -Copyright 2024 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 dynamicresources - -import ( - "errors" - "sync" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - v1 "k8s.io/api/core/v1" - resourceapi "k8s.io/api/resource/v1alpha3" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - namedresourcesmodel "k8s.io/kubernetes/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources" - "k8s.io/kubernetes/test/utils/ktesting" - "k8s.io/utils/ptr" -) - -func TestModel(t *testing.T) { - testcases := map[string]struct { - slices resourceSliceLister - claims assumeCacheLister - inFlight map[types.UID]resourceapi.ResourceClaimStatus - - wantResources resources - wantErr bool - }{ - "empty": {}, - - "slice-list-error": { - slices: sliceError("slice list error"), - - wantErr: true, - }, - - "unknown-model": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node", - DriverName: "driver", - ResourceModel: resourceapi.ResourceModel{ /* empty! */ }, - }, - }, - - // Not an error. It is safe to ignore unknown resources until a claim requests them. - // The unknown model in that claim then triggers an error for that claim. - wantResources: resources{"node": map[string]ResourceModels{ - "driver": { - NamedResources: namedresourcesmodel.Model{}, - }, - }}, - }, - - "one-instance": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node", - DriverName: "driver", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{ - Name: "one", - Attributes: []resourceapi.NamedResourcesAttribute{{ - Name: "size", - NamedResourcesAttributeValue: resourceapi.NamedResourcesAttributeValue{ - IntValue: ptr.To(int64(1)), - }, - }}, - }}, - }, - }, - }, - }, - - wantResources: resources{"node": map[string]ResourceModels{ - "driver": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{ - Name: "one", - Attributes: []resourceapi.NamedResourcesAttribute{{ - Name: "size", - NamedResourcesAttributeValue: resourceapi.NamedResourcesAttributeValue{ - IntValue: ptr.To(int64(1)), - }, - }}, - }, - }, - }, - }, - }, - }}, - }, - - "two-instances": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node", - DriverName: "driver", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - }, - - wantResources: resources{"node": map[string]ResourceModels{ - "driver": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - }}, - }, - - "two-nodes": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - }, - - wantResources: resources{ - "node-a": map[string]ResourceModels{ - "driver": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - }, - "node-b": map[string]ResourceModels{ - "driver": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - }, - }, - }, - - "two-nodes-two-drivers": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - }, - - wantResources: resources{ - "node-a": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - }, - "node-b": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - }, - }, - }, - - "in-use-simple": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - }, - - claims: claimList{ - &resourceapi.ResourceClaim{ - /* not allocated */ - }, - &resourceapi.ResourceClaim{ - Status: resourceapi.ResourceClaimStatus{ - DriverName: "driver-a", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{{ - // Claims not allocated via structured parameters can be ignored. - }}, - }, - }, - }, - &resourceapi.ResourceClaim{ - Status: resourceapi.ResourceClaimStatus{ - DriverName: "driver-a", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{{ - DriverName: "driver-a", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-b", - // Unknown allocations can be ignored. - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{}, - }}, - }, - }}, - }, - }, - }, - &resourceapi.ResourceClaim{ - Status: resourceapi.ResourceClaimStatus{ - DriverName: "driver-a", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{{ - DriverName: "driver-a", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-a", - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "two", - }, - }, - }}, - }, - }}, - }, - }, - }, - }, - - wantResources: resources{ - "node-a": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Allocated: true, - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - }, - "node-b": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - }, - }, - }, - - "in-use-meta-driver": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - }, - - claims: claimList{ - &resourceapi.ResourceClaim{ - Status: resourceapi.ResourceClaimStatus{ - DriverName: "meta-driver", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{{ - DriverName: "driver-b", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-b", - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "X", - }, - }, - }}, - }, - }}, - }, - }, - }, - }, - - wantResources: resources{ - "node-a": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - }, - "node-b": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Allocated: true, - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - }, - }, - }, - - "in-use-many-results": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - }, - - claims: claimList{ - &resourceapi.ResourceClaim{ - Status: resourceapi.ResourceClaimStatus{ - DriverName: "driver-a", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{{ - DriverName: "driver-a", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-a", - Results: []resourceapi.DriverAllocationResult{ - { - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "one", - }, - }, - }, - { - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "two", - }, - }, - }, - }, - }, - }}, - }, - }, - }, - }, - - wantResources: resources{ - "node-a": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Allocated: true, - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Allocated: true, - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - }, - "node-b": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - }, - }, - }, - - "in-use-many-handles": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-a", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}, {Name: "two"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-a", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - &resourceapi.ResourceSlice{ - NodeName: "node-b", - DriverName: "driver-b", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "X"}, {Name: "Y"}}, - }, - }, - }, - }, - - claims: claimList{ - &resourceapi.ResourceClaim{ - Status: resourceapi.ResourceClaimStatus{ - DriverName: "meta-driver", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{ - { - DriverName: "driver-a", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-b", - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "X", - }, - }, - }}, - }, - }, - { - DriverName: "driver-b", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-b", - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "X", - }, - }, - }}, - }, - }, - }, - }, - }, - }, - }, - - wantResources: resources{ - "node-a": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "two"}, - }, - }, - }, - }, - }, - "node-b": map[string]ResourceModels{ - "driver-a": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Allocated: true, - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - "driver-b": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Allocated: true, - Instance: &resourceapi.NamedResourcesInstance{Name: "X"}, - }, - { - Instance: &resourceapi.NamedResourcesInstance{Name: "Y"}, - }, - }, - }, - }, - }, - }, - }, - - "orphaned-allocations": { - claims: claimList{ - &resourceapi.ResourceClaim{ - Status: resourceapi.ResourceClaimStatus{ - DriverName: "meta-driver", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{ - { - DriverName: "driver-a", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-b", - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "X", - }, - }, - }}, - }, - }, - { - DriverName: "driver-b", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node-b", - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "X", - }, - }, - }}, - }, - }, - }, - }, - }, - }, - }, - - wantResources: resources{ - "node-b": map[string]ResourceModels{}, - }, - }, - - "in-flight": { - slices: sliceList{ - &resourceapi.ResourceSlice{ - NodeName: "node", - DriverName: "driver", - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{ - Instances: []resourceapi.NamedResourcesInstance{{Name: "one"}}, - }, - }, - }, - }, - - claims: claimList{ - &resourceapi.ResourceClaim{ - ObjectMeta: metav1.ObjectMeta{ - UID: "abc", - }, - // Allocation not recorded yet. - }, - }, - - inFlight: map[types.UID]resourceapi.ResourceClaimStatus{ - "abc": { - DriverName: "driver", - Allocation: &resourceapi.AllocationResult{ - ResourceHandles: []resourceapi.ResourceHandle{{ - DriverName: "driver", - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: "node", - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: "one", - }, - }, - }}, - }, - }}, - }, - }, - }, - - wantResources: resources{"node": map[string]ResourceModels{ - "driver": { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{ - { - Allocated: true, - Instance: &resourceapi.NamedResourcesInstance{Name: "one"}, - }, - }, - }, - }, - }}, - }, - } - - for name, tc := range testcases { - t.Run(name, func(t *testing.T) { - tCtx := ktesting.Init(t) - - var inFlightAllocations sync.Map - for uid, claimStatus := range tc.inFlight { - inFlightAllocations.Store(uid, &resourceapi.ResourceClaim{Status: claimStatus}) - } - - slices := tc.slices - if slices == nil { - slices = sliceList{} - } - claims := tc.claims - if claims == nil { - claims = claimList{} - } - actualResources, actualErr := newResourceModel(tCtx.Logger(), slices, claims, &inFlightAllocations) - - if actualErr != nil { - if !tc.wantErr { - tCtx.Fatalf("unexpected error: %v", actualErr) - } - return - } - if tc.wantErr { - tCtx.Fatalf("did not get expected error") - } - - expectResources := tc.wantResources - if expectResources == nil { - expectResources = resources{} - } - require.Equal(tCtx, expectResources, actualResources) - }) - } -} - -type sliceList []*resourceapi.ResourceSlice - -func (l sliceList) List(selector labels.Selector) ([]*resourceapi.ResourceSlice, error) { - return l, nil -} - -type sliceError string - -func (l sliceError) List(selector labels.Selector) ([]*resourceapi.ResourceSlice, error) { - return nil, errors.New(string(l)) -} - -type claimList []any - -func (l claimList) List(indexObj any) []any { - return l -} - -func TestController(t *testing.T) { - driver1 := "driver-1" - class1 := &resourceapi.ResourceClass{ - DriverName: driver1, - } - - classParametersEmpty := &resourceapi.ResourceClassParameters{} - classParametersAny := &resourceapi.ResourceClassParameters{ - Filters: []resourceapi.ResourceFilter{{ - DriverName: driver1, - ResourceFilterModel: resourceapi.ResourceFilterModel{ - NamedResources: &resourceapi.NamedResourcesFilter{ - Selector: "true", - }, - }, - }}, - } - - claimParametersEmpty := &resourceapi.ResourceClaimParameters{} - claimParametersAny := &resourceapi.ResourceClaimParameters{ - DriverRequests: []resourceapi.DriverRequests{{ - DriverName: driver1, - }}, - } - claimParametersOne := &resourceapi.ResourceClaimParameters{ - DriverRequests: []resourceapi.DriverRequests{{ - DriverName: driver1, - Requests: []resourceapi.ResourceRequest{{ - ResourceRequestModel: resourceapi.ResourceRequestModel{ - NamedResources: &resourceapi.NamedResourcesRequest{ - Selector: "true", - }, - }, - }}, - }}, - } - claimParametersBroken := &resourceapi.ResourceClaimParameters{ - DriverRequests: []resourceapi.DriverRequests{{ - DriverName: driver1, - Requests: []resourceapi.ResourceRequest{{ - ResourceRequestModel: resourceapi.ResourceRequestModel{ - NamedResources: &resourceapi.NamedResourcesRequest{ - Selector: `attributes.bool["no-such-attribute"]`, - }, - }, - }}, - }}, - } - - instance1 := "instance-1" - - node1 := "node-1" - node1Selector := &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{{ - MatchExpressions: []v1.NodeSelectorRequirement{{ - Key: "kubernetes.io/hostname", - Operator: v1.NodeSelectorOpIn, - Values: []string{node1}, - }}, - }}, - } - node1Resources := resources{node1: map[string]ResourceModels{ - driver1: { - NamedResources: namedresourcesmodel.Model{ - Instances: []namedresourcesmodel.InstanceAllocation{{ - Instance: &resourceapi.NamedResourcesInstance{ - Name: instance1, - }, - }}, - }, - }, - }} - node1Allocation := &resourceapi.AllocationResult{ - AvailableOnNodes: node1Selector, - } - - instance1Allocation := &resourceapi.AllocationResult{ - AvailableOnNodes: node1Selector, - ResourceHandles: []resourceapi.ResourceHandle{{ - DriverName: driver1, - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: node1, - Results: []resourceapi.DriverAllocationResult{{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: instance1, - }, - }, - }}, - }, - }}, - } - - type nodeResult struct { - isSuitable bool - suitableErr string - - driverName string - allocation *resourceapi.AllocationResult - allocateErr string - } - type nodeResults map[string]nodeResult - - testcases := map[string]struct { - resources resources - class *resourceapi.ResourceClass - classParameters *resourceapi.ResourceClassParameters - claimParameters *resourceapi.ResourceClaimParameters - - expectCreateErr bool - expectNodeResults nodeResults - }{ - "empty": { - class: class1, - classParameters: classParametersEmpty, - claimParameters: claimParametersEmpty, - - expectNodeResults: nodeResults{ - node1: {isSuitable: true, driverName: driver1, allocation: node1Allocation}, - }, - }, - - "any": { - class: class1, - classParameters: classParametersEmpty, - claimParameters: claimParametersAny, - - expectNodeResults: nodeResults{ - node1: {isSuitable: true, driverName: driver1, allocation: node1Allocation}, - }, - }, - - "missing-model": { - class: class1, - classParameters: classParametersEmpty, - claimParameters: &resourceapi.ResourceClaimParameters{ - DriverRequests: []resourceapi.DriverRequests{{ - Requests: []resourceapi.ResourceRequest{{ /* empty model */ }}, - }}, - }, - - expectCreateErr: true, - }, - - "no-resources": { - class: class1, - classParameters: classParametersEmpty, - claimParameters: claimParametersOne, - - expectNodeResults: nodeResults{ - node1: {isSuitable: false, allocateErr: "allocating via named resources structured model: insufficient resources"}, - }, - }, - - "have-resources": { - resources: node1Resources, - class: class1, - classParameters: classParametersEmpty, - claimParameters: claimParametersOne, - - expectNodeResults: nodeResults{ - node1: {isSuitable: true, driverName: driver1, allocation: instance1Allocation}, - }, - }, - - "broken-cel": { - resources: node1Resources, - class: class1, - classParameters: classParametersEmpty, - claimParameters: claimParametersBroken, - - expectNodeResults: nodeResults{ - node1: {suitableErr: `checking node "node-1" and resources of driver "driver-1": evaluate request CEL expression: no such key: no-such-attribute`}, - }, - }, - - "class-filter": { - resources: node1Resources, - class: class1, - classParameters: classParametersAny, - claimParameters: claimParametersOne, - - expectNodeResults: nodeResults{ - node1: {isSuitable: true, driverName: driver1, allocation: instance1Allocation}, - }, - }, - - "vendor-parameters": { - resources: node1Resources, - class: class1, - classParameters: func() *resourceapi.ResourceClassParameters { - parameters := classParametersAny.DeepCopy() - parameters.VendorParameters = []resourceapi.VendorParameters{{ - DriverName: driver1, - Parameters: runtime.RawExtension{Raw: []byte("class-parameters")}, - }} - return parameters - }(), - - claimParameters: func() *resourceapi.ResourceClaimParameters { - parameters := claimParametersOne.DeepCopy() - parameters.DriverRequests[0].VendorParameters = runtime.RawExtension{Raw: []byte("claim-parameters")} - parameters.DriverRequests[0].Requests[0].VendorParameters = runtime.RawExtension{Raw: []byte("request-parameters")} - return parameters - }(), - - expectNodeResults: nodeResults{ - node1: {isSuitable: true, driverName: driver1, - allocation: func() *resourceapi.AllocationResult { - allocation := instance1Allocation.DeepCopy() - allocation.ResourceHandles[0].StructuredData.VendorClassParameters = runtime.RawExtension{Raw: []byte("class-parameters")} - allocation.ResourceHandles[0].StructuredData.VendorClaimParameters = runtime.RawExtension{Raw: []byte("claim-parameters")} - allocation.ResourceHandles[0].StructuredData.Results[0].VendorRequestParameters = runtime.RawExtension{Raw: []byte("request-parameters")} - return allocation - }(), - }, - }, - }, - } - - for name, tc := range testcases { - t.Run(name, func(t *testing.T) { - tCtx := ktesting.Init(t) - - controller, err := newClaimController(tCtx.Logger(), tc.class, tc.classParameters, tc.claimParameters) - if err != nil { - if !tc.expectCreateErr { - tCtx.Fatalf("unexpected error: %v", err) - } - return - } - if tc.expectCreateErr { - tCtx.Fatalf("did not get expected error") - } - - for nodeName, expect := range tc.expectNodeResults { - t.Run(nodeName, func(t *testing.T) { - tCtx := ktesting.Init(t) - - isSuitable, err := controller.nodeIsSuitable(tCtx, nodeName, tc.resources) - if err != nil { - if expect.suitableErr == "" { - tCtx.Fatalf("unexpected nodeIsSuitable error: %v", err) - } - require.Equal(tCtx, expect.suitableErr, err.Error()) - return - } - if expect.suitableErr != "" { - tCtx.Fatalf("did not get expected nodeIsSuitable error: %v", expect.suitableErr) - } - assert.Equal(tCtx, expect.isSuitable, isSuitable, "is suitable") - - driverName, allocation, err := controller.allocate(tCtx, nodeName, tc.resources) - if err != nil { - if expect.allocateErr == "" { - tCtx.Fatalf("unexpected allocate error: %v", err) - } - require.Equal(tCtx, expect.allocateErr, err.Error()) - return - } - if expect.allocateErr != "" { - tCtx.Fatalf("did not get expected allocate error: %v", expect.allocateErr) - } - assert.Equal(tCtx, expect.driverName, driverName, "driver name") - assert.Equal(tCtx, expect.allocation, allocation) - }) - } - }) - } -} diff --git a/pkg/scheduler/framework/types.go b/pkg/scheduler/framework/types.go index d066059276f..43cc16e15df 100644 --- a/pkg/scheduler/framework/types.go +++ b/pkg/scheduler/framework/types.go @@ -93,18 +93,16 @@ const ( // unschedulable pod pool. // This behavior will be removed when we remove the preCheck feature. // See: https://github.com/kubernetes/kubernetes/issues/110175 - Node GVK = "Node" - PersistentVolume GVK = "PersistentVolume" - PersistentVolumeClaim GVK = "PersistentVolumeClaim" - CSINode GVK = "storage.k8s.io/CSINode" - CSIDriver GVK = "storage.k8s.io/CSIDriver" - CSIStorageCapacity GVK = "storage.k8s.io/CSIStorageCapacity" - StorageClass GVK = "storage.k8s.io/StorageClass" - PodSchedulingContext GVK = "PodSchedulingContext" - ResourceClaim GVK = "ResourceClaim" - ResourceClass GVK = "ResourceClass" - ResourceClaimParameters GVK = "ResourceClaimParameters" - ResourceClassParameters GVK = "ResourceClassParameters" + Node GVK = "Node" + PersistentVolume GVK = "PersistentVolume" + PersistentVolumeClaim GVK = "PersistentVolumeClaim" + CSINode GVK = "storage.k8s.io/CSINode" + CSIDriver GVK = "storage.k8s.io/CSIDriver" + CSIStorageCapacity GVK = "storage.k8s.io/CSIStorageCapacity" + StorageClass GVK = "storage.k8s.io/StorageClass" + PodSchedulingContext GVK = "PodSchedulingContext" + ResourceClaim GVK = "ResourceClaim" + DeviceClass GVK = "DeviceClass" // WildCard is a special GVK to match all resources. // e.g., If you register `{Resource: "*", ActionType: All}` in EventsToRegister, @@ -197,9 +195,7 @@ func UnrollWildCardResource() []ClusterEventWithHint { {Event: ClusterEvent{Resource: StorageClass, ActionType: All}}, {Event: ClusterEvent{Resource: PodSchedulingContext, ActionType: All}}, {Event: ClusterEvent{Resource: ResourceClaim, ActionType: All}}, - {Event: ClusterEvent{Resource: ResourceClass, ActionType: All}}, - {Event: ClusterEvent{Resource: ResourceClaimParameters, ActionType: All}}, - {Event: ClusterEvent{Resource: ResourceClassParameters, ActionType: All}}, + {Event: ClusterEvent{Resource: DeviceClass, ActionType: All}}, } } diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 3c9c07d960f..ed342ae7635 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -646,13 +646,7 @@ func Test_buildQueueingHintMap(t *testing.T) { {Resource: framework.ResourceClaim, ActionType: framework.All}: { {PluginName: filterWithoutEnqueueExtensions, QueueingHintFn: defaultQueueingHintFn}, }, - {Resource: framework.ResourceClass, ActionType: framework.All}: { - {PluginName: filterWithoutEnqueueExtensions, QueueingHintFn: defaultQueueingHintFn}, - }, - {Resource: framework.ResourceClaimParameters, ActionType: framework.All}: { - {PluginName: filterWithoutEnqueueExtensions, QueueingHintFn: defaultQueueingHintFn}, - }, - {Resource: framework.ResourceClassParameters, ActionType: framework.All}: { + {Resource: framework.DeviceClass, ActionType: framework.All}: { {PluginName: filterWithoutEnqueueExtensions, QueueingHintFn: defaultQueueingHintFn}, }, }, @@ -803,19 +797,17 @@ func Test_UnionedGVKs(t *testing.T) { Disabled: []schedulerapi.Plugin{{Name: "*"}}, // disable default plugins }, want: map[framework.GVK]framework.ActionType{ - framework.Pod: framework.All, - framework.Node: framework.All, - framework.CSINode: framework.All, - framework.CSIDriver: framework.All, - framework.CSIStorageCapacity: framework.All, - framework.PersistentVolume: framework.All, - framework.PersistentVolumeClaim: framework.All, - framework.StorageClass: framework.All, - framework.PodSchedulingContext: framework.All, - framework.ResourceClaim: framework.All, - framework.ResourceClass: framework.All, - framework.ResourceClaimParameters: framework.All, - framework.ResourceClassParameters: framework.All, + framework.Pod: framework.All, + framework.Node: framework.All, + framework.CSINode: framework.All, + framework.CSIDriver: framework.All, + framework.CSIStorageCapacity: framework.All, + framework.PersistentVolume: framework.All, + framework.PersistentVolumeClaim: framework.All, + framework.StorageClass: framework.All, + framework.PodSchedulingContext: framework.All, + framework.ResourceClaim: framework.All, + framework.DeviceClass: framework.All, }, }, { diff --git a/pkg/scheduler/testing/wrappers.go b/pkg/scheduler/testing/wrappers.go index a5bbffaf948..ecce0833aeb 100644 --- a/pkg/scheduler/testing/wrappers.go +++ b/pkg/scheduler/testing/wrappers.go @@ -900,8 +900,8 @@ func (p *PersistentVolumeWrapper) NodeAffinityIn(key string, vals []string) *Per type ResourceClaimWrapper struct{ resourceapi.ResourceClaim } // MakeResourceClaim creates a ResourceClaim wrapper. -func MakeResourceClaim() *ResourceClaimWrapper { - return &ResourceClaimWrapper{resourceapi.ResourceClaim{}} +func MakeResourceClaim(controller string) *ResourceClaimWrapper { + return &ResourceClaimWrapper{resourceapi.ResourceClaim{Spec: resourceapi.ResourceClaimSpec{Controller: controller}}} } // FromResourceClaim creates a ResourceClaim wrapper from some existing object. @@ -946,72 +946,34 @@ func (wrapper *ResourceClaimWrapper) OwnerReference(name, uid string, gvk schema return wrapper } -// ParametersRef sets a reference to a ResourceClaimParameters.resource.k8s.io. -func (wrapper *ResourceClaimWrapper) ParametersRef(name string) *ResourceClaimWrapper { - wrapper.ResourceClaim.Spec.ParametersRef = &resourceapi.ResourceClaimParametersReference{ - Name: name, - Kind: "ResourceClaimParameters", - APIGroup: "resource.k8s.io", - } - return wrapper -} - -// ResourceClassName sets the resource class name of the inner object. -func (wrapper *ResourceClaimWrapper) ResourceClassName(name string) *ResourceClaimWrapper { - wrapper.ResourceClaim.Spec.ResourceClassName = name +// Request adds one device request for the given device class. +func (wrapper *ResourceClaimWrapper) Request(deviceClassName string) *ResourceClaimWrapper { + wrapper.Spec.Devices.Requests = append(wrapper.Spec.Devices.Requests, + resourceapi.DeviceRequest{ + Name: fmt.Sprintf("req-%d", len(wrapper.Spec.Devices.Requests)+1), + // Cannot rely on defaulting here, this is used in unit tests. + AllocationMode: resourceapi.DeviceAllocationModeExactCount, + Count: 1, + DeviceClassName: deviceClassName, + }, + ) return wrapper } // Allocation sets the allocation of the inner object. -func (wrapper *ResourceClaimWrapper) Allocation(driverName string, allocation *resourceapi.AllocationResult) *ResourceClaimWrapper { - wrapper.ResourceClaim.Status.DriverName = driverName +func (wrapper *ResourceClaimWrapper) Allocation(allocation *resourceapi.AllocationResult) *ResourceClaimWrapper { wrapper.ResourceClaim.Status.Allocation = allocation return wrapper } // Structured turns a "normal" claim into one which was allocated via structured parameters. -// This modifies the allocation result and adds the reserved finalizer if the claim -// is allocated. The claim has to become local to a node. The assumption is that -// "named resources" are used. -func (wrapper *ResourceClaimWrapper) Structured(nodeName string, namedResourcesInstances ...string) *ResourceClaimWrapper { +// The only difference is that there is no controller name and the special finalizer +// gets added. +func (wrapper *ResourceClaimWrapper) Structured() *ResourceClaimWrapper { + wrapper.Spec.Controller = "" if wrapper.ResourceClaim.Status.Allocation != nil { wrapper.ResourceClaim.Finalizers = append(wrapper.ResourceClaim.Finalizers, resourceapi.Finalizer) - for i, resourceHandle := range wrapper.ResourceClaim.Status.Allocation.ResourceHandles { - resourceHandle.Data = "" - resourceHandle.StructuredData = &resourceapi.StructuredResourceHandle{ - NodeName: nodeName, - } - wrapper.ResourceClaim.Status.Allocation.ResourceHandles[i] = resourceHandle - } - if len(wrapper.ResourceClaim.Status.Allocation.ResourceHandles) == 0 { - wrapper.ResourceClaim.Status.Allocation.ResourceHandles = []resourceapi.ResourceHandle{{ - DriverName: wrapper.ResourceClaim.Status.DriverName, - StructuredData: &resourceapi.StructuredResourceHandle{ - NodeName: nodeName, - }, - }} - } - for _, resourceHandle := range wrapper.ResourceClaim.Status.Allocation.ResourceHandles { - for _, name := range namedResourcesInstances { - result := resourceapi.DriverAllocationResult{ - AllocationResultModel: resourceapi.AllocationResultModel{ - NamedResources: &resourceapi.NamedResourcesAllocationResult{ - Name: name, - }, - }, - } - resourceHandle.StructuredData.Results = append(resourceHandle.StructuredData.Results, result) - } - } - wrapper.ResourceClaim.Status.Allocation.AvailableOnNodes = &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{{ - MatchExpressions: []v1.NodeSelectorRequirement{{ - Key: "kubernetes.io/hostname", - Operator: v1.NodeSelectorOpIn, - Values: []string{nodeName}, - }}, - }}, - } + wrapper.ResourceClaim.Status.Allocation.Controller = "" } return wrapper } @@ -1120,8 +1082,9 @@ type ResourceSliceWrapper struct { func MakeResourceSlice(nodeName, driverName string) *ResourceSliceWrapper { wrapper := new(ResourceSliceWrapper) wrapper.Name = nodeName + "-" + driverName - wrapper.NodeName = nodeName - wrapper.DriverName = driverName + wrapper.Spec.NodeName = nodeName + wrapper.Spec.Pool.Name = nodeName + wrapper.Spec.Driver = driverName return wrapper } @@ -1129,119 +1092,14 @@ func (wrapper *ResourceSliceWrapper) Obj() *resourceapi.ResourceSlice { return &wrapper.ResourceSlice } -func (wrapper *ResourceSliceWrapper) NamedResourcesInstances(names ...string) *ResourceSliceWrapper { - wrapper.ResourceModel = resourceapi.ResourceModel{NamedResources: &resourceapi.NamedResourcesResources{}} +func (wrapper *ResourceSliceWrapper) Devices(names ...string) *ResourceSliceWrapper { for _, name := range names { - wrapper.ResourceModel.NamedResources.Instances = append(wrapper.ResourceModel.NamedResources.Instances, - resourceapi.NamedResourcesInstance{Name: name}, - ) + wrapper.Spec.Devices = append(wrapper.Spec.Devices, resourceapi.Device{Name: name}) } return wrapper } -type ClaimParametersWrapper struct { - resourceapi.ResourceClaimParameters -} - -func MakeClaimParameters() *ClaimParametersWrapper { - return &ClaimParametersWrapper{} -} - -// FromClaimParameters creates a ResourceClaimParameters wrapper from an existing object. -func FromClaimParameters(other *resourceapi.ResourceClaimParameters) *ClaimParametersWrapper { - return &ClaimParametersWrapper{*other.DeepCopy()} -} - -func (wrapper *ClaimParametersWrapper) Obj() *resourceapi.ResourceClaimParameters { - return &wrapper.ResourceClaimParameters -} - -func (wrapper *ClaimParametersWrapper) Name(s string) *ClaimParametersWrapper { - wrapper.SetName(s) - return wrapper -} - -func (wrapper *ClaimParametersWrapper) UID(s string) *ClaimParametersWrapper { - wrapper.SetUID(types.UID(s)) - return wrapper -} - -func (wrapper *ClaimParametersWrapper) Namespace(s string) *ClaimParametersWrapper { - wrapper.SetNamespace(s) - return wrapper -} - -func (wrapper *ClaimParametersWrapper) GeneratedFrom(value *resourceapi.ResourceClaimParametersReference) *ClaimParametersWrapper { - wrapper.ResourceClaimParameters.GeneratedFrom = value - return wrapper -} - -func (wrapper *ClaimParametersWrapper) NamedResourcesRequests(driverName string, selectors ...string) *ClaimParametersWrapper { - requests := resourceapi.DriverRequests{ - DriverName: driverName, - } - for _, selector := range selectors { - request := resourceapi.ResourceRequest{ - ResourceRequestModel: resourceapi.ResourceRequestModel{ - NamedResources: &resourceapi.NamedResourcesRequest{ - Selector: selector, - }, - }, - } - requests.Requests = append(requests.Requests, request) - } - wrapper.DriverRequests = append(wrapper.DriverRequests, requests) - return wrapper -} - -type ClassParametersWrapper struct { - resourceapi.ResourceClassParameters -} - -func MakeClassParameters() *ClassParametersWrapper { - return &ClassParametersWrapper{} -} - -// FromClassParameters creates a ResourceClassParameters wrapper from an existing object. -func FromClassParameters(other *resourceapi.ResourceClassParameters) *ClassParametersWrapper { - return &ClassParametersWrapper{*other.DeepCopy()} -} - -func (wrapper *ClassParametersWrapper) Obj() *resourceapi.ResourceClassParameters { - return &wrapper.ResourceClassParameters -} - -func (wrapper *ClassParametersWrapper) Name(s string) *ClassParametersWrapper { - wrapper.SetName(s) - return wrapper -} - -func (wrapper *ClassParametersWrapper) UID(s string) *ClassParametersWrapper { - wrapper.SetUID(types.UID(s)) - return wrapper -} - -func (wrapper *ClassParametersWrapper) Namespace(s string) *ClassParametersWrapper { - wrapper.SetNamespace(s) - return wrapper -} - -func (wrapper *ClassParametersWrapper) GeneratedFrom(value *resourceapi.ResourceClassParametersReference) *ClassParametersWrapper { - wrapper.ResourceClassParameters.GeneratedFrom = value - return wrapper -} - -func (wrapper *ClassParametersWrapper) NamedResourcesFilters(driverName string, selectors ...string) *ClassParametersWrapper { - for _, selector := range selectors { - filter := resourceapi.ResourceFilter{ - DriverName: driverName, - ResourceFilterModel: resourceapi.ResourceFilterModel{ - NamedResources: &resourceapi.NamedResourcesFilter{ - Selector: selector, - }, - }, - } - wrapper.Filters = append(wrapper.Filters, filter) - } +func (wrapper *ResourceSliceWrapper) Device(name string, attrs map[resourceapi.QualifiedName]resourceapi.DeviceAttribute) *ResourceSliceWrapper { + wrapper.Spec.Devices = append(wrapper.Spec.Devices, resourceapi.Device{Name: name, Basic: &resourceapi.BasicDevice{Attributes: attrs}}) return wrapper } diff --git a/staging/publishing/import-restrictions.yaml b/staging/publishing/import-restrictions.yaml index ad48080bc87..79133f0425a 100644 --- a/staging/publishing/import-restrictions.yaml +++ b/staging/publishing/import-restrictions.yaml @@ -256,6 +256,7 @@ - k8s.io/apiserver/pkg/cel - k8s.io/apiserver/pkg/cel/environment - k8s.io/client-go + - k8s.io/component-helpers/scheduling/corev1/nodeaffinity - k8s.io/dynamic-resource-allocation - k8s.io/klog - k8s.io/kubelet diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go new file mode 100644 index 00000000000..61334e00ba4 --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator.go @@ -0,0 +1,844 @@ +/* +Copyright 2024 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 structured + +import ( + "context" + "errors" + "fmt" + "math" + "strings" + + v1 "k8s.io/api/core/v1" + resourceapi "k8s.io/api/resource/v1alpha3" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/cel/environment" + resourcelisters "k8s.io/client-go/listers/resource/v1alpha3" + "k8s.io/dynamic-resource-allocation/cel" + "k8s.io/klog/v2" +) + +// ClaimLister returns a subset of the claims that a +// resourcelisters.ResourceClaimLister would return. +type ClaimLister interface { + // ListAllAllocated returns only claims which are allocated. + ListAllAllocated() ([]*resourceapi.ResourceClaim, error) +} + +// Allocator calculates how to allocate a set of unallocated claims which use +// structured parameters. +// +// It needs as input the node where the allocated claims are meant to be +// available and the current state of the cluster (claims, classes, resource +// slices). +type Allocator struct { + claimsToAllocate []*resourceapi.ResourceClaim + claimLister ClaimLister + classLister resourcelisters.DeviceClassLister + sliceLister resourcelisters.ResourceSliceLister +} + +// NewAllocator returns an allocator for a certain set of claims or an error if +// some problem was detected which makes it impossible to allocate claims. +func NewAllocator(ctx context.Context, + claimsToAllocate []*resourceapi.ResourceClaim, + claimLister ClaimLister, + classLister resourcelisters.DeviceClassLister, + sliceLister resourcelisters.ResourceSliceLister, +) (*Allocator, error) { + return &Allocator{ + claimsToAllocate: claimsToAllocate, + claimLister: claimLister, + classLister: classLister, + sliceLister: sliceLister, + }, nil +} + +// ClaimsToAllocate returns the claims that the allocated was created for. +func (a *Allocator) ClaimsToAllocate() []*resourceapi.ResourceClaim { + return a.claimsToAllocate +} + +// Allocate calculates the allocation(s) for one particular node. +// +// It returns an error only if some fatal problem occurred. These are errors +// caused by invalid input data, like for example errors in CEL selectors, so a +// scheduler should abort and report that problem instead of trying to find +// other nodes where the error doesn't occur. +// +// In the future, special errors will be defined which enable the caller to +// identify which object (like claim or class) caused the problem. This will +// enable reporting the problem as event for those objects. +// +// If the claims cannot be allocated, it returns nil. This includes the +// situation where the resource slices are incomplete at the moment. +// +// If the claims can be allocated, then it prepares one allocation result for +// each unallocated claim. It is the responsibility of the caller to persist +// those allocations, if desired. +// +// Allocate is thread-safe. If the caller wants to get the node name included +// in log output, it can use contextual logging and add the node as an +// additional value. A name can also be useful because log messages do not +// have a common prefix. V(5) is used for one-time log entries, V(6) for important +// progress reports, and V(7) for detailed debug output. +func (a *Allocator) Allocate(ctx context.Context, node *v1.Node) (finalResult []*resourceapi.AllocationResult, finalErr error) { + alloc := &allocator{ + Allocator: a, + ctx: ctx, // all methods share the same a and thus ctx + logger: klog.FromContext(ctx), + deviceMatchesRequest: make(map[matchKey]bool), + constraints: make([][]constraint, len(a.claimsToAllocate)), + requestData: make(map[requestIndices]requestData), + allocated: make(map[DeviceID]bool), + result: make([]*resourceapi.AllocationResult, len(a.claimsToAllocate)), + } + alloc.logger.V(5).Info("Starting allocation", "numClaims", len(alloc.claimsToAllocate)) + defer alloc.logger.V(5).Info("Done with allocation", "success", len(finalResult) == len(alloc.claimsToAllocate), "err", finalErr) + + // First determine all eligible pools. + pools, err := GatherPools(ctx, alloc.sliceLister, node) + if err != nil { + return nil, fmt.Errorf("gather pool information: %w", err) + } + alloc.pools = pools + if loggerV := alloc.logger.V(7); loggerV.Enabled() { + loggerV.Info("Gathered pool information", "numPools", len(pools), "pools", pools) + } else { + alloc.logger.V(5).Info("Gathered pool information", "numPools", len(pools)) + } + + // We allocate one claim after the other and for each claim, all of + // its requests. For each individual device we pick one possible + // candidate after the other, checking constraints as we go. + // Each chosen candidate is marked as "in use" and the process + // continues, recursively. This way, all requests get matched against + // all candidates in all possible orders. + // + // The first full solution is chosen. + // + // In other words, this is an exhaustive search. This is okay because + // it aborts early. Once scoring gets added, more intelligence may be + // needed to avoid trying "equivalent" solutions (two identical + // requests, two identical devices, two solutions that are the same in + // practice). + + // This is where we sanity check that we can actually handle the claims + // and their requests. For each claim we determine how many devices + // need to be allocated. If not all can be stored in the result, the + // claim cannot be allocated. + for claimIndex, claim := range alloc.claimsToAllocate { + numDevices := 0 + + // If we have any any request that wants "all" devices, we need to + // figure out how much "all" is. If some pool is incomplete, we stop + // here because allocation cannot succeed. Once we do scoring, we should + // stop in all cases, not just when "all" devices are needed, because + // pulling from an incomplete might not pick the best solution and it's + // better to wait. This does not matter yet as long the incomplete pool + // has some matching device. + for requestIndex := range claim.Spec.Devices.Requests { + request := &claim.Spec.Devices.Requests[requestIndex] + for i, selector := range request.Selectors { + if selector.CEL == nil { + // Unknown future selector type! + return nil, fmt.Errorf("claim %s, request %s, selector #%d: CEL expression empty (unsupported selector type?)", klog.KObj(claim), request.Name, i) + } + } + + // Should be set. If it isn't, something changed and we should refuse to proceed. + if request.DeviceClassName == "" { + return nil, fmt.Errorf("claim %s, request %s: missing device class name (unsupported request type?)", klog.KObj(claim), request.Name) + } + class, err := alloc.classLister.Get(request.DeviceClassName) + if err != nil { + return nil, fmt.Errorf("claim %s, request %s: could not retrieve device class %s: %w", klog.KObj(claim), request.Name, request.DeviceClassName, err) + } + + requestData := requestData{ + class: class, + } + + switch request.AllocationMode { + case resourceapi.DeviceAllocationModeExactCount: + numDevices := request.Count + if numDevices > math.MaxInt { + // Allowed by API validation, but doesn't make sense. + return nil, fmt.Errorf("claim %s, request %s: exact count %d is too large", klog.KObj(claim), request.Name, numDevices) + } + requestData.numDevices = int(numDevices) + case resourceapi.DeviceAllocationModeAll: + requestData.allDevices = make([]deviceWithID, 0, resourceapi.AllocationResultsMaxSize) + for _, pool := range pools { + if pool.IsIncomplete { + return nil, fmt.Errorf("claim %s, request %s: asks for all devices, but resource pool %s is currently being updated", klog.KObj(claim), request.Name, pool.PoolID) + } + + for _, slice := range pool.Slices { + for deviceIndex := range slice.Spec.Devices { + selectable, err := alloc.isSelectable(requestIndices{claimIndex: claimIndex, requestIndex: requestIndex}, slice, deviceIndex) + if err != nil { + return nil, err + } + if selectable { + requestData.allDevices = append(requestData.allDevices, deviceWithID{device: slice.Spec.Devices[deviceIndex].Basic, DeviceID: DeviceID{Driver: slice.Spec.Driver, Pool: slice.Spec.Pool.Name, Device: slice.Spec.Devices[deviceIndex].Name}}) + } + } + } + } + requestData.numDevices = len(requestData.allDevices) + alloc.logger.V(6).Info("Request for 'all' devices", "claim", klog.KObj(claim), "request", request.Name, "numDevicesPerRequest", requestData.numDevices) + default: + return nil, fmt.Errorf("claim %s, request %s: unsupported count mode %s", klog.KObj(claim), request.Name, request.AllocationMode) + } + alloc.requestData[requestIndices{claimIndex: claimIndex, requestIndex: requestIndex}] = requestData + numDevices += requestData.numDevices + } + alloc.logger.Info("Checked claim", "claim", klog.KObj(claim), "numDevices", numDevices) + + // Check that we don't end up with too many results. + if numDevices > resourceapi.AllocationResultsMaxSize { + return nil, fmt.Errorf("claim %s: number of requested devices %d exceeds the claim limit of %d", klog.KObj(claim), numDevices, resourceapi.AllocationResultsMaxSize) + } + + // If we don't, then we can pre-allocate the result slices for + // appending the actual results later. + alloc.result[claimIndex] = &resourceapi.AllocationResult{ + Devices: resourceapi.DeviceAllocationResult{ + Results: make([]resourceapi.DeviceRequestAllocationResult, 0, numDevices), + }, + } + + // Constraints are assumed to be monotonic: once a constraint returns + // false, adding more devices will not cause it to return true. This + // allows the search to stop early once a constraint returns false. + var constraints = make([]constraint, len(claim.Spec.Devices.Constraints)) + for i, constraint := range claim.Spec.Devices.Constraints { + switch { + case constraint.MatchAttribute != nil: + logger := alloc.logger + if loggerV := alloc.logger.V(6); loggerV.Enabled() { + logger = klog.LoggerWithName(logger, "matchAttributeConstraint") + logger = klog.LoggerWithValues(logger, "matchAttribute", *constraint.MatchAttribute) + } + m := &matchAttributeConstraint{ + logger: logger, + requestNames: sets.New(constraint.Requests...), + attributeName: *constraint.MatchAttribute, + } + constraints[i] = m + default: + // Unknown constraint type! + return nil, fmt.Errorf("claim %s, constraint #%d: empty constraint (unsupported constraint type?)", klog.KObj(claim), i) + } + } + alloc.constraints[claimIndex] = constraints + } + + // Selecting a device for a request is independent of what has been + // allocated already. Therefore the result of checking a request against + // a device instance in the pool can be cached. The pointer to both + // can serve as key because they are static for the duration of + // the Allocate call and can be compared in Go. + alloc.deviceMatchesRequest = make(map[matchKey]bool) + + // Some of the existing devices are probably already allocated by + // claims... + claims, err := alloc.claimLister.ListAllAllocated() + numAllocated := 0 + if err != nil { + return nil, fmt.Errorf("list allocated claims: %w", err) + } + for _, claim := range claims { + // Sanity check.. + if claim.Status.Allocation == nil { + continue + } + for _, result := range claim.Status.Allocation.Devices.Results { + deviceID := DeviceID{Driver: result.Driver, Pool: result.Pool, Device: result.Device} + alloc.allocated[deviceID] = true + numAllocated++ + } + } + alloc.logger.V(6).Info("Gathered information about allocated devices", "numAllocated", numAllocated) + + // In practice, there aren't going to be many different CEL + // expressions. Most likely, there is going to be handful of different + // device classes that get used repeatedly. Different requests may all + // use the same selector. Therefore compiling CEL expressions on demand + // could be a useful performance enhancement. It's not implemented yet + // because the key is more complex (just the string?) and the memory + // for both key and cached content is larger than for device matches. + // + // We may also want to cache this in the shared [Allocator] instance, + // which implies adding locking. + + // All errors get created such that they can be returned by Allocate + // without further wrapping. + done, err := alloc.allocateOne(deviceIndices{}) + if err != nil { + return nil, err + } + if errors.Is(err, errStop) || !done { + return nil, nil + } + + for claimIndex, allocationResult := range alloc.result { + claim := alloc.claimsToAllocate[claimIndex] + + // Populate configs. + for requestIndex := range claim.Spec.Devices.Requests { + class := alloc.requestData[requestIndices{claimIndex: claimIndex, requestIndex: requestIndex}].class + if class != nil { + for _, config := range class.Spec.Config { + allocationResult.Devices.Config = append(allocationResult.Devices.Config, resourceapi.DeviceAllocationConfiguration{ + Source: resourceapi.AllocationConfigSourceClass, + Requests: nil, // All of them... + DeviceConfiguration: config.DeviceConfiguration, + }) + } + } + } + for _, config := range claim.Spec.Devices.Config { + allocationResult.Devices.Config = append(allocationResult.Devices.Config, resourceapi.DeviceAllocationConfiguration{ + Source: resourceapi.AllocationConfigSourceClaim, + Requests: config.Requests, + DeviceConfiguration: config.DeviceConfiguration, + }) + } + + // Determine node selector. + nodeSelector, err := alloc.createNodeSelector(allocationResult) + if err != nil { + return nil, fmt.Errorf("create NodeSelector for claim %s: %w", claim.Name, err) + } + allocationResult.NodeSelector = nodeSelector + } + + return alloc.result, nil +} + +// errStop is a special error that gets returned by allocateOne if it detects +// that allocation cannot succeed. +var errStop = errors.New("stop allocation") + +// allocator is used while an [Allocator.Allocate] is running. Only a single +// goroutine works with it, so there is no need for locking. +type allocator struct { + *Allocator + ctx context.Context + logger klog.Logger + pools []*Pool + deviceMatchesRequest map[matchKey]bool + constraints [][]constraint // one list of constraints per claim + requestData map[requestIndices]requestData // one entry per request + allocated map[DeviceID]bool + skippedUnknownDevice bool + result []*resourceapi.AllocationResult +} + +// matchKey identifies a device/request pair. +type matchKey struct { + DeviceID + requestIndices +} + +// requestIndices identifies one specific request by its +// claim and request index. +type requestIndices struct { + claimIndex, requestIndex int +} + +// deviceIndices identifies one specific required device inside +// a request of a certain claim. +type deviceIndices struct { + claimIndex, requestIndex, deviceIndex int +} + +type requestData struct { + class *resourceapi.DeviceClass + numDevices int + + // pre-determined set of devices for allocating "all" devices + allDevices []deviceWithID +} + +type deviceWithID struct { + DeviceID + device *resourceapi.BasicDevice +} + +type constraint interface { + // add is called whenever a device is about to be allocated. It must + // check whether the device matches the constraint and if yes, + // track that it is allocated. + add(requestName string, device *resourceapi.BasicDevice, deviceID DeviceID) bool + + // For every successful add there is exactly one matching removed call + // with the exact same parameters. + remove(requestName string, device *resourceapi.BasicDevice, deviceID DeviceID) +} + +// matchAttributeConstraint compares an attribute value across devices. +// All devices must share the same value. When the set of devices is +// empty, any device that has the attribute can be added. After that, +// only matching devices can be added. +// +// We don't need to track *which* devices are part of the set, only +// how many. +type matchAttributeConstraint struct { + logger klog.Logger // Includes name and attribute name, so no need to repeat in log messages. + requestNames sets.Set[string] + attributeName resourceapi.FullyQualifiedName + + attribute *resourceapi.DeviceAttribute + numDevices int +} + +func (m *matchAttributeConstraint) add(requestName string, device *resourceapi.BasicDevice, deviceID DeviceID) bool { + if m.requestNames.Len() > 0 && !m.requestNames.Has(requestName) { + // Device not affected by constraint. + m.logger.V(7).Info("Constraint does not apply to request", "request", requestName) + return true + } + + attribute := lookupAttribute(device, deviceID, m.attributeName) + if attribute == nil { + // Doesn't have the attribute. + m.logger.V(7).Info("Constraint not satisfied, attribute not set") + return false + } + + if m.numDevices == 0 { + // The first device can always get picked. + m.attribute = attribute + m.numDevices = 1 + m.logger.V(7).Info("First in set") + return true + } + + switch { + case attribute.StringValue != nil: + if m.attribute.StringValue == nil || *attribute.StringValue != *m.attribute.StringValue { + m.logger.V(7).Info("String values different") + return false + } + case attribute.IntValue != nil: + if m.attribute.IntValue == nil || *attribute.IntValue != *m.attribute.IntValue { + m.logger.V(7).Info("Int values different") + return false + } + case attribute.BoolValue != nil: + if m.attribute.BoolValue == nil || *attribute.BoolValue != *m.attribute.BoolValue { + m.logger.V(7).Info("Bool values different") + return false + } + case attribute.VersionValue != nil: + // semver 2.0.0 requires that version strings are in their + // minimal form (in particular, no leading zeros). Therefore a + // strict "exact equal" check can do a string comparison. + if m.attribute.VersionValue == nil || *attribute.VersionValue != *m.attribute.VersionValue { + m.logger.V(7).Info("Version values different") + return false + } + default: + // Unknown value type, cannot match. + m.logger.V(7).Info("Match attribute type unknown") + return false + } + + m.numDevices++ + m.logger.V(7).Info("Constraint satisfied by device", "device", deviceID, "numDevices", m.numDevices) + return true +} + +func (m *matchAttributeConstraint) remove(requestName string, device *resourceapi.BasicDevice, deviceID DeviceID) { + if m.requestNames.Len() > 0 && !m.requestNames.Has(requestName) { + // Device not affected by constraint. + return + } + + m.numDevices-- + m.logger.V(7).Info("Device removed from constraint set", "device", deviceID, "numDevices", m.numDevices) +} + +func lookupAttribute(device *resourceapi.BasicDevice, deviceID DeviceID, attributeName resourceapi.FullyQualifiedName) *resourceapi.DeviceAttribute { + // Fully-qualified match? + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName)]; ok { + return &attr + } + index := strings.Index(string(attributeName), "/") + if index < 0 { + // Should not happen for a valid fully qualified name. + return nil + } + + if string(attributeName[0:index]) != deviceID.Driver { + // Not an attribute of the driver and not found above, + // so it is not available. + return nil + } + + // Domain matches the driver, so let's check just the ID. + if attr, ok := device.Attributes[resourceapi.QualifiedName(attributeName[index+1:])]; ok { + return &attr + } + + return nil +} + +// allocateOne iterates over all eligible devices (not in use, match selector, +// satisfy constraints) for a specific required device. It returns true if +// everything got allocated, an error if allocation needs to stop. +func (alloc *allocator) allocateOne(r deviceIndices) (bool, error) { + if r.claimIndex >= len(alloc.claimsToAllocate) { + // Done! If we were doing scoring, we would compare the current allocation result + // against the previous one, keep the best, and continue. Without scoring, we stop + // and use the first solution. + alloc.logger.V(6).Info("Allocation result found") + return true, nil + } + + claim := alloc.claimsToAllocate[r.claimIndex] + if r.requestIndex >= len(claim.Spec.Devices.Requests) { + // Done with the claim, continue with the next one. + return alloc.allocateOne(deviceIndices{claimIndex: r.claimIndex + 1}) + } + + // We already know how many devices per request are needed. + // Ready to move on to the next request? + requestData := alloc.requestData[requestIndices{claimIndex: r.claimIndex, requestIndex: r.requestIndex}] + if r.deviceIndex >= requestData.numDevices { + return alloc.allocateOne(deviceIndices{claimIndex: r.claimIndex, requestIndex: r.requestIndex + 1}) + } + + request := &alloc.claimsToAllocate[r.claimIndex].Spec.Devices.Requests[r.requestIndex] + doAllDevices := request.AllocationMode == resourceapi.DeviceAllocationModeAll + alloc.logger.V(6).Info("Allocating one device", "currentClaim", r.claimIndex, "totalClaims", len(alloc.claimsToAllocate), "currentRequest", r.requestIndex, "totalRequestsPerClaim", len(claim.Spec.Devices.Requests), "currentDevice", r.deviceIndex, "devicesPerRequest", requestData.numDevices, "allDevices", doAllDevices, "adminAccess", request.AdminAccess) + + if doAllDevices { + // For "all" devices we already know which ones we need. We + // just need to check whether we can use them. + deviceWithID := requestData.allDevices[r.deviceIndex] + _, _, err := alloc.allocateDevice(r, deviceWithID.device, deviceWithID.DeviceID, true) + if err != nil { + return false, err + } + done, err := alloc.allocateOne(deviceIndices{claimIndex: r.claimIndex, requestIndex: r.requestIndex, deviceIndex: r.deviceIndex + 1}) + if err != nil { + return false, err + } + + // The order in which we allocate "all" devices doesn't matter, + // so we only try with the one which was up next. If we couldn't + // get all of them, then there is no solution and we have to stop. + if !done { + return false, errStop + } + return done, nil + } + + // We need to find suitable devices. + for _, pool := range alloc.pools { + for _, slice := range pool.Slices { + for deviceIndex := range slice.Spec.Devices { + deviceID := DeviceID{Driver: pool.Driver, Pool: pool.Pool, Device: slice.Spec.Devices[deviceIndex].Name} + + // Checking for "in use" is cheap and thus gets done first. + if !request.AdminAccess && alloc.allocated[deviceID] { + alloc.logger.V(7).Info("Device in use", "device", deviceID) + continue + } + + // Next check selectors. + selectable, err := alloc.isSelectable(requestIndices{claimIndex: r.claimIndex, requestIndex: r.requestIndex}, slice, deviceIndex) + if err != nil { + return false, err + } + if !selectable { + alloc.logger.V(7).Info("Device not selectable", "device", deviceID) + continue + } + + // Finally treat as allocated and move on to the next device. + allocated, deallocate, err := alloc.allocateDevice(r, slice.Spec.Devices[deviceIndex].Basic, deviceID, false) + if err != nil { + return false, err + } + if !allocated { + // In use or constraint violated... + alloc.logger.V(7).Info("Device not usable", "device", deviceID) + continue + } + done, err := alloc.allocateOne(deviceIndices{claimIndex: r.claimIndex, requestIndex: r.requestIndex, deviceIndex: r.deviceIndex + 1}) + if err != nil { + return false, err + } + + // If we found a solution, then we can stop. + if done { + return done, nil + } + + // Otherwise try some other device after rolling back. + deallocate() + } + } + } + + // If we get here without finding a solution, then there is none. + return false, nil +} + +// isSelectable checks whether a device satisfies the request and class selectors. +func (alloc *allocator) isSelectable(r requestIndices, slice *resourceapi.ResourceSlice, deviceIndex int) (bool, error) { + // This is the only supported device type at the moment. + device := slice.Spec.Devices[deviceIndex].Basic + if device == nil { + // Must be some future, unknown device type. We cannot select it. + // If we don't find anything else, then this will get reported + // in the final result, so remember that we skipped some device. + alloc.skippedUnknownDevice = true + return false, nil + } + + deviceID := DeviceID{Driver: slice.Spec.Driver, Pool: slice.Spec.Pool.Name, Device: slice.Spec.Devices[deviceIndex].Name} + matchKey := matchKey{DeviceID: deviceID, requestIndices: r} + if matches, ok := alloc.deviceMatchesRequest[matchKey]; ok { + // No need to check again. + return matches, nil + } + + requestData := alloc.requestData[r] + if requestData.class != nil { + match, err := alloc.selectorsMatch(r, device, deviceID, requestData.class, requestData.class.Spec.Selectors) + if err != nil { + return false, err + } + if !match { + alloc.deviceMatchesRequest[matchKey] = false + return false, nil + } + } + + request := &alloc.claimsToAllocate[r.claimIndex].Spec.Devices.Requests[r.requestIndex] + match, err := alloc.selectorsMatch(r, device, deviceID, nil, request.Selectors) + if err != nil { + return false, err + } + if !match { + alloc.deviceMatchesRequest[matchKey] = false + return false, nil + } + + alloc.deviceMatchesRequest[matchKey] = true + return true, nil + +} + +func (alloc *allocator) selectorsMatch(r requestIndices, device *resourceapi.BasicDevice, deviceID DeviceID, class *resourceapi.DeviceClass, selectors []resourceapi.DeviceSelector) (bool, error) { + for i, selector := range selectors { + expr := cel.GetCompiler().CompileCELExpression(selector.CEL.Expression, environment.StoredExpressions) + if expr.Error != nil { + // Could happen if some future apiserver accepted some + // future expression and then got downgraded. Normally + // the "stored expression" mechanism prevents that, but + // this code here might be more than one release older + // than the cluster it runs in. + if class != nil { + return false, fmt.Errorf("class %s: selector #%d: CEL compile error: %w", class.Name, i, expr.Error) + } + return false, fmt.Errorf("claim %s: selector #%d: CEL compile error: %w", klog.KObj(alloc.claimsToAllocate[r.claimIndex]), i, expr.Error) + } + + matches, err := expr.DeviceMatches(alloc.ctx, cel.Device{Driver: deviceID.Driver, Attributes: device.Attributes, Capacity: device.Capacity}) + if class != nil { + alloc.logger.V(7).Info("CEL result", "device", deviceID, "class", klog.KObj(class), "selector", i, "expression", selector.CEL.Expression, "matches", matches, "err", err) + } else { + alloc.logger.V(7).Info("CEL result", "device", deviceID, "claim", klog.KObj(alloc.claimsToAllocate[r.claimIndex]), "selector", i, "expression", selector.CEL.Expression, "matches", matches, "err", err) + } + + if err != nil { + // TODO (future): more detailed errors which reference class resp. claim. + if class != nil { + return false, fmt.Errorf("class %s: selector #%d: CEL runtime error: %w", class.Name, i, err) + } + return false, fmt.Errorf("claim %s: selector #%d: CEL runtime error: %w", klog.KObj(alloc.claimsToAllocate[r.claimIndex]), i, err) + } + if !matches { + return false, nil + } + } + + // All of them match. + return true, nil +} + +// allocateDevice checks device availability and constraints for one +// candidate. The device must be selectable. +// +// If that candidate works out okay, the shared state gets updated +// as if that candidate had been allocated. If allocation cannot continue later +// and must try something else, then the rollback function can be invoked to +// restore the previous state. +func (alloc *allocator) allocateDevice(r deviceIndices, device *resourceapi.BasicDevice, deviceID DeviceID, must bool) (bool, func(), error) { + claim := alloc.claimsToAllocate[r.claimIndex] + request := &claim.Spec.Devices.Requests[r.requestIndex] + adminAccess := request.AdminAccess + if !adminAccess && alloc.allocated[deviceID] { + alloc.logger.V(7).Info("Device in use", "device", deviceID) + return false, nil, nil + } + + // It's available. Now check constraints. + for i, constraint := range alloc.constraints[r.claimIndex] { + added := constraint.add(request.Name, device, deviceID) + if !added { + if must { + // It does not make sense to declare a claim where a constraint prevents getting + // all devices. Treat this as an error. + return false, nil, fmt.Errorf("claim %s, request %s: cannot add device %s because a claim constraint would not be satisfied", klog.KObj(claim), request.Name, deviceID) + } + + // Roll back for all previous constraints before we return. + for e := 0; e < i; e++ { + alloc.constraints[r.claimIndex][e].remove(request.Name, device, deviceID) + } + return false, nil, nil + } + } + + // All constraints satisfied. Mark as in use (unless we do admin access) + // and record the result. + alloc.logger.V(7).Info("Device allocated", "device", deviceID) + if !adminAccess { + alloc.allocated[deviceID] = true + } + result := resourceapi.DeviceRequestAllocationResult{ + Request: request.Name, + Driver: deviceID.Driver, + Pool: deviceID.Pool, + Device: deviceID.Device, + } + previousNumResults := len(alloc.result[r.claimIndex].Devices.Results) + alloc.result[r.claimIndex].Devices.Results = append(alloc.result[r.claimIndex].Devices.Results, result) + + return true, func() { + for _, constraint := range alloc.constraints[r.claimIndex] { + constraint.remove(request.Name, device, deviceID) + } + if !adminAccess { + alloc.allocated[deviceID] = false + } + // Truncate, but keep the underlying slice. + alloc.result[r.claimIndex].Devices.Results = alloc.result[r.claimIndex].Devices.Results[:previousNumResults] + alloc.logger.V(7).Info("Device deallocated", "device", deviceID) + }, nil +} + +// createNodeSelector constructs a node selector for the allocation, if needed, +// otherwise it returns nil. +func (alloc *allocator) createNodeSelector(allocation *resourceapi.AllocationResult) (*v1.NodeSelector, error) { + // Selector with one term. That term gets extended with additional + // requirements from the different devices. + nodeSelector := &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{{}}, + } + + for _, deviceAllocation := range allocation.Devices.Results { + slice := alloc.findSlice(deviceAllocation) + if slice == nil { + return nil, fmt.Errorf("internal error: device %+v not found in pools", deviceAllocation) + } + if slice.Spec.NodeName != "" { + // At least one device is local to one node. This + // restricts the allocation to that node. + return &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{{ + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{slice.Spec.NodeName}, + }}, + }}, + }, nil + } + if slice.Spec.NodeSelector != nil { + switch len(slice.Spec.NodeSelector.NodeSelectorTerms) { + case 0: + // Nothing? + case 1: + // Add all terms if they are not present already. + addNewNodeSelectorRequirements(slice.Spec.NodeSelector.NodeSelectorTerms[0].MatchFields, &nodeSelector.NodeSelectorTerms[0].MatchFields) + addNewNodeSelectorRequirements(slice.Spec.NodeSelector.NodeSelectorTerms[0].MatchExpressions, &nodeSelector.NodeSelectorTerms[0].MatchExpressions) + default: + // This shouldn't occur, validation must prevent creation of such slices. + return nil, fmt.Errorf("unsupported ResourceSlice.NodeSelector with %d terms", len(slice.Spec.NodeSelector.NodeSelectorTerms)) + } + } + } + + if len(nodeSelector.NodeSelectorTerms[0].MatchFields) > 0 || len(nodeSelector.NodeSelectorTerms[0].MatchExpressions) > 0 { + // We have a valid node selector. + return nodeSelector, nil + } + + // Available everywhere. + return nil, nil +} + +func (alloc *allocator) findSlice(deviceAllocation resourceapi.DeviceRequestAllocationResult) *resourceapi.ResourceSlice { + for _, pool := range alloc.pools { + if pool.Driver != deviceAllocation.Driver || + pool.Pool != deviceAllocation.Pool { + continue + } + for _, slice := range pool.Slices { + for _, device := range slice.Spec.Devices { + if device.Name == deviceAllocation.Device { + return slice + } + } + } + } + return nil +} + +func addNewNodeSelectorRequirements(from []v1.NodeSelectorRequirement, to *[]v1.NodeSelectorRequirement) { + for _, requirement := range from { + if !containsNodeSelectorRequirement(*to, requirement) { + *to = append(*to, requirement) + } + } +} + +func containsNodeSelectorRequirement(requirements []v1.NodeSelectorRequirement, requirement v1.NodeSelectorRequirement) bool { + values := sets.New(requirement.Values...) + for _, existingRequirement := range requirements { + if existingRequirement.Key != requirement.Key { + continue + } + if existingRequirement.Operator != requirement.Operator { + continue + } + if !sets.New(existingRequirement.Values...).Equal(values) { + continue + } + return true + } + return false +} diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator_test.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator_test.go new file mode 100644 index 00000000000..bbb249faf2b --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/allocator_test.go @@ -0,0 +1,971 @@ +/* +Copyright 2024 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 structured + +import ( + "errors" + "flag" + "fmt" + "testing" + + "github.com/onsi/gomega" + "github.com/onsi/gomega/gstruct" + "github.com/onsi/gomega/types" + + v1 "k8s.io/api/core/v1" + resourceapi "k8s.io/api/resource/v1alpha3" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/klog/v2/ktesting" + "k8s.io/utils/ptr" +) + +const ( + region1 = "region-1" + region2 = "region-2" + node1 = "node-1" + node2 = "node-2" + classA = "class-a" + classB = "class-b" + driverA = "driver-a" + driverB = "driver-b" + pool1 = "pool-1" + pool2 = "pool-2" + pool3 = "pool-3" + pool4 = "pool-4" + req0 = "req-0" + req1 = "req-1" + req2 = "req-2" + req3 = "req-3" + claim0 = "claim-0" + claim1 = "claim-1" + slice1 = "slice-1" + slice2 = "slice-2" + device1 = "device-1" + device2 = "device-2" +) + +func init() { + // Bump up the default verbosity for testing. Allocate uses very + // high thresholds because it is used in the scheduler's per-node + // filter operation. + ktesting.DefaultConfig = ktesting.NewConfig(ktesting.Verbosity(7)) + ktesting.DefaultConfig.AddFlags(flag.CommandLine) +} + +// Test objects generators + +const ( + fieldNameKey = "metadata.name" + regionKey = "region" + planetKey = "planet" + planetValueEarth = "earth" +) + +// generate a node object given a name and a region +func node(name, region string) *v1.Node { + return &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + regionKey: region, + planetKey: planetValueEarth, + }, + }, + } +} + +// generate a DeviceClass object with the given name and a driver CEL selector. +// driver name is assumed to be the same as the class name. +func class(name, driver string) *resourceapi.DeviceClass { + return &resourceapi.DeviceClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: resourceapi.DeviceClassSpec{ + Selectors: []resourceapi.DeviceSelector{ + { + CEL: &resourceapi.CELDeviceSelector{ + Expression: fmt.Sprintf(`device.driver == "%s"`, driver), + }, + }, + }, + }, + } +} + +// generate a DeviceConfiguration object with the given driver and attribute. +func deviceConfiguration(driver, attribute string) resourceapi.DeviceConfiguration { + return resourceapi.DeviceConfiguration{ + Opaque: &resourceapi.OpaqueDeviceConfiguration{ + Driver: driver, + Parameters: runtime.RawExtension{ + Raw: []byte(fmt.Sprintf("{\"%s\":\"%s\"}", attribute, attribute+"Value")), + }, + }, + } +} + +// generate a DeviceClass object with the given name and attribute. +// attribute is used to generate device configuration parameters in a form of JSON {attribute: attributeValue}. +func classWithConfig(name, driver, attribute string) *resourceapi.DeviceClass { + class := class(name, driver) + class.Spec.Config = []resourceapi.DeviceClassConfiguration{ + { + DeviceConfiguration: deviceConfiguration(driver, attribute), + }, + } + return class +} + +// generate a DeviceClass object with the given name and the node selector +// that selects nodes with the region label set to either "west" or "east". +func classWithSuitableNodes(name, driver string) *resourceapi.DeviceClass { + class := class(name, driver) + class.Spec.SuitableNodes = &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: regionKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{region1, region2}, + }, + }, + }, + }, + } + return class +} + +// generate a ResourceClaim object with the given name and device requests. +func claimWithRequests(name string, constraints []resourceapi.DeviceConstraint, requests ...resourceapi.DeviceRequest) *resourceapi.ResourceClaim { + return &resourceapi.ResourceClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: resourceapi.ResourceClaimSpec{ + Devices: resourceapi.DeviceClaim{ + Requests: requests, + Constraints: constraints, + }, + }, + } +} + +// generate a DeviceRequest object with the given name, class and selectors. +func request(name, class string, count int64, selectors ...resourceapi.DeviceSelector) resourceapi.DeviceRequest { + return resourceapi.DeviceRequest{ + Name: name, + Count: count, + AllocationMode: resourceapi.DeviceAllocationModeExactCount, + DeviceClassName: class, + Selectors: selectors, + } +} + +// generate a ResourceClaim object with the given name, request and class. +func claim(name, req, class string, constraints ...resourceapi.DeviceConstraint) *resourceapi.ResourceClaim { + claim := claimWithRequests(name, constraints, request(req, class, 1)) + return claim +} + +// generate a ResourceClaim object with the given name, request, class, and attribute. +// attribute is used to generate parameters in a form of JSON {attribute: attributeValue}. +func claimWithDeviceConfig(name, request, class, driver, attribute string) *resourceapi.ResourceClaim { + claim := claim(name, request, class) + claim.Spec.Devices.Config = []resourceapi.DeviceClaimConfiguration{ + { + DeviceConfiguration: deviceConfiguration(driver, attribute), + }, + } + return claim +} + +// generate allocated ResourceClaim object +func allocatedClaim(name, request, class string, results ...resourceapi.DeviceRequestAllocationResult) *resourceapi.ResourceClaim { + claim := claim(name, request, class) + claim.Status.Allocation = &resourceapi.AllocationResult{ + Devices: resourceapi.DeviceAllocationResult{ + Results: results, + }, + } + return claim +} + +// generate a Device object with the given name, capacity and attributes. +func device(name string, capacity map[resourceapi.QualifiedName]resource.Quantity, attributes map[resourceapi.QualifiedName]resourceapi.DeviceAttribute) resourceapi.Device { + return resourceapi.Device{ + Name: name, + Basic: &resourceapi.BasicDevice{ + Attributes: attributes, + Capacity: capacity, + }, + } +} + +// generate a ResourceSlice object with the given name, node, +// driver and pool names, generation and a list of devices. +// The nodeSelection parameter may be a string (= node name), +// true (= all nodes), or a node selector (= specific nodes). +func slice(name string, nodeSelection any, pool, driver string, devices ...resourceapi.Device) *resourceapi.ResourceSlice { + slice := &resourceapi.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: resourceapi.ResourceSliceSpec{ + Driver: driver, + Pool: resourceapi.ResourcePool{ + Name: pool, + ResourceSliceCount: 1, + Generation: 1, + }, + Devices: devices, + }, + } + + switch nodeSelection := nodeSelection.(type) { + case *v1.NodeSelector: + slice.Spec.NodeSelector = nodeSelection + case bool: + if !nodeSelection { + panic("nodeSelection == false is not valid") + } + slice.Spec.AllNodes = true + case string: + slice.Spec.NodeName = nodeSelection + default: + panic(fmt.Sprintf("unexpected nodeSelection type %T: %+v", nodeSelection, nodeSelection)) + } + + return slice +} + +func deviceAllocationResult(request, driver, pool, device string) resourceapi.DeviceRequestAllocationResult { + return resourceapi.DeviceRequestAllocationResult{ + Request: request, + Driver: driver, + Pool: pool, + Device: device, + } +} + +// nodeLabelSelector creates a node selector with a label match for "key" in "values". +func nodeLabelSelector(key string, values ...string) *v1.NodeSelector { + requirements := []v1.NodeSelectorRequirement{{ + Key: key, + Operator: v1.NodeSelectorOpIn, + Values: values, + }} + selector := &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{{MatchExpressions: requirements}}} + return selector +} + +// localNodeSelector returns a node selector for a specific node. +func localNodeSelector(nodeName string) *v1.NodeSelector { + selector := nodeLabelSelector(fieldNameKey, nodeName) + // Swap the requirements: we need to select by field, not label. + selector.NodeSelectorTerms[0].MatchFields, selector.NodeSelectorTerms[0].MatchExpressions = selector.NodeSelectorTerms[0].MatchExpressions, selector.NodeSelectorTerms[0].MatchFields + return selector +} + +// allocationResult returns a matcher for one AllocationResult pointer with a list of +// embedded device allocation results. The order of those results doesn't matter. +func allocationResult(selector *v1.NodeSelector, results ...resourceapi.DeviceRequestAllocationResult) types.GomegaMatcher { + return gstruct.PointTo(gstruct.MatchFields(0, gstruct.Fields{ + "Devices": gstruct.MatchFields(0, gstruct.Fields{ + "Results": gomega.ConsistOf(results), // Order is irrelevant. + "Config": gomega.BeNil(), + }), + "NodeSelector": matchNodeSelector(selector), + "Controller": gomega.BeEmpty(), + })) +} + +// matchNodeSelector returns a matcher for a node selector. The order +// of terms, requirements, and values is irrelevant. +func matchNodeSelector(selector *v1.NodeSelector) types.GomegaMatcher { + if selector == nil { + return gomega.BeNil() + } + return gomega.HaveField("NodeSelectorTerms", matchNodeSelectorTerms(selector.NodeSelectorTerms)) +} + +func matchNodeSelectorTerms(terms []v1.NodeSelectorTerm) types.GomegaMatcher { + var matchTerms []types.GomegaMatcher + for _, term := range terms { + matchTerms = append(matchTerms, matchNodeSelectorTerm(term)) + } + return gomega.ConsistOf(matchTerms) +} + +func matchNodeSelectorTerm(term v1.NodeSelectorTerm) types.GomegaMatcher { + return gstruct.MatchFields(0, gstruct.Fields{ + "MatchExpressions": matchNodeSelectorRequirements(term.MatchExpressions), + "MatchFields": matchNodeSelectorRequirements(term.MatchFields), + }) +} + +func matchNodeSelectorRequirements(requirements []v1.NodeSelectorRequirement) types.GomegaMatcher { + var matchRequirements []types.GomegaMatcher + for _, requirement := range requirements { + matchRequirements = append(matchRequirements, matchNodeSelectorRequirement(requirement)) + } + return gomega.ConsistOf(matchRequirements) +} + +func matchNodeSelectorRequirement(requirement v1.NodeSelectorRequirement) types.GomegaMatcher { + return gstruct.MatchFields(0, gstruct.Fields{ + "Key": gomega.Equal(requirement.Key), + "Operator": gomega.Equal(requirement.Operator), + "Values": gomega.ConsistOf(requirement.Values), + }) +} + +func allocationResultWithConfig(selector *v1.NodeSelector, driver string, source resourceapi.AllocationConfigSource, attribute string, results ...resourceapi.DeviceRequestAllocationResult) *resourceapi.AllocationResult { + return &resourceapi.AllocationResult{ + Devices: resourceapi.DeviceAllocationResult{ + Results: results, + Config: []resourceapi.DeviceAllocationConfiguration{ + { + Source: source, + DeviceConfiguration: deviceConfiguration(driver, attribute), + }, + }, + }, + NodeSelector: selector, + } +} + +// Helpers + +// convert a list of objects to a slice +func objects[T any](objs ...T) []T { + return objs +} + +// generate a ResourceSlice object with the given parameters and one device "device-1" +func sliceWithOneDevice(name string, nodeSelection any, pool, driver string) *resourceapi.ResourceSlice { + return slice(name, nodeSelection, pool, driver, device(device1, nil, nil)) +} + +func TestAllocator(t *testing.T) { + nonExistentAttribute := resourceapi.FullyQualifiedName("NonExistentAttribute") + boolAttribute := resourceapi.FullyQualifiedName("boolAttribute") + stringAttribute := resourceapi.FullyQualifiedName("stringAttribute") + versionAttribute := resourceapi.FullyQualifiedName("driverVersion") + intAttribute := resourceapi.FullyQualifiedName("numa") + + testcases := map[string]struct { + claimsToAllocate []*resourceapi.ResourceClaim + allocatedClaims []*resourceapi.ResourceClaim + classes []*resourceapi.DeviceClass + slices []*resourceapi.ResourceSlice + node *v1.Node + + expectResults []any + expectError types.GomegaMatcher // can be used to check for no error or match specific error types + }{ + + "empty": {}, + "simple": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: []any{allocationResult( + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + )}, + }, + "other-node": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: objects(class(classA, driverA)), + slices: objects( + sliceWithOneDevice(slice1, node1, pool1, driverB), + sliceWithOneDevice(slice2, node2, pool2, driverA), + ), + node: node(node2, region2), + + expectResults: []any{allocationResult( + localNodeSelector(node2), + deviceAllocationResult(req0, driverA, pool2, device1), + )}, + }, + "small-and-large": { + claimsToAllocate: objects(claimWithRequests( + claim0, + nil, + request(req0, classA, 1, resourceapi.DeviceSelector{ + CEL: &resourceapi.CELDeviceSelector{ + Expression: fmt.Sprintf(`device.capacity["%s"].memory.compareTo(quantity("1Gi")) >= 0`, driverA), + }}), + request(req1, classA, 1, resourceapi.DeviceSelector{ + CEL: &resourceapi.CELDeviceSelector{ + Expression: fmt.Sprintf(`device.capacity["%s"].memory.compareTo(quantity("2Gi")) >= 0`, driverA), + }}), + )), + classes: objects(class(classA, driverA)), + slices: objects(slice(slice1, node1, pool1, driverA, + device(device1, map[resourceapi.QualifiedName]resource.Quantity{ + "memory": resource.MustParse("1Gi"), + }, nil), + device(device2, map[resourceapi.QualifiedName]resource.Quantity{ + "memory": resource.MustParse("2Gi"), + }, nil), + )), + node: node(node1, region1), + + expectResults: []any{allocationResult( + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + deviceAllocationResult(req1, driverA, pool1, device2), + )}, + }, + "small-and-large-backtrack-requests": { + claimsToAllocate: objects(claimWithRequests( + claim0, + nil, + request(req0, classA, 1, resourceapi.DeviceSelector{ + CEL: &resourceapi.CELDeviceSelector{ + Expression: fmt.Sprintf(`device.capacity["%s"].memory.compareTo(quantity("1Gi")) >= 0`, driverA), + }}), + request(req1, classA, 1, resourceapi.DeviceSelector{ + CEL: &resourceapi.CELDeviceSelector{ + Expression: fmt.Sprintf(`device.capacity["%s"].memory.compareTo(quantity("2Gi")) >= 0`, driverA), + }}), + )), + classes: objects(class(classA, driverA)), + // Reversing the order in which the devices are listed causes the "large" device to + // be allocated for the "small" request, leaving the "large" request unsatisfied. + // The initial decision needs to be undone before a solution is found. + slices: objects(slice(slice1, node1, pool1, driverA, + device(device2, map[resourceapi.QualifiedName]resource.Quantity{ + "memory": resource.MustParse("2Gi"), + }, nil), + device(device1, map[resourceapi.QualifiedName]resource.Quantity{ + "memory": resource.MustParse("1Gi"), + }, nil), + )), + node: node(node1, region1), + + expectResults: []any{allocationResult( + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + deviceAllocationResult(req1, driverA, pool1, device2), + )}, + }, + "small-and-large-backtrack-claims": { + claimsToAllocate: objects( + claimWithRequests( + claim0, + nil, + request(req0, classA, 1, resourceapi.DeviceSelector{ + CEL: &resourceapi.CELDeviceSelector{ + Expression: fmt.Sprintf(`device.capacity["%s"].memory.compareTo(quantity("1Gi")) >= 0`, driverA), + }})), + claimWithRequests( + claim1, + nil, + request(req1, classA, 1, resourceapi.DeviceSelector{ + CEL: &resourceapi.CELDeviceSelector{ + Expression: fmt.Sprintf(`device.capacity["%s"].memory.compareTo(quantity("2Gi")) >= 0`, driverA), + }}), + )), + classes: objects(class(classA, driverA)), + // Reversing the order in which the devices are listed causes the "large" device to + // be allocated for the "small" request, leaving the "large" request unsatisfied. + // The initial decision needs to be undone before a solution is found. + slices: objects(slice(slice1, node1, pool1, driverA, + device(device2, map[resourceapi.QualifiedName]resource.Quantity{ + "memory": resource.MustParse("2Gi"), + }, nil), + device(device1, map[resourceapi.QualifiedName]resource.Quantity{ + "memory": resource.MustParse("1Gi"), + }, nil), + )), + node: node(node1, region1), + + expectResults: []any{ + allocationResult(localNodeSelector(node1), deviceAllocationResult(req0, driverA, pool1, device1)), + allocationResult(localNodeSelector(node1), deviceAllocationResult(req1, driverA, pool1, device2)), + }, + }, + "devices-split-across-different-slices": { + claimsToAllocate: objects(claimWithRequests(claim0, nil, resourceapi.DeviceRequest{ + Name: req0, + Count: 2, + AllocationMode: resourceapi.DeviceAllocationModeExactCount, + DeviceClassName: classA, + })), + classes: objects(class(classA, driverA)), + slices: objects( + sliceWithOneDevice(slice1, node1, pool1, driverA), + sliceWithOneDevice(slice2, node1, pool2, driverA), + ), + node: node(node1, region1), + + expectResults: []any{allocationResult( + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + deviceAllocationResult(req0, driverA, pool2, device1), + )}, + }, + "obsolete-slice": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: objects(class(classA, driverA)), + slices: objects( + sliceWithOneDevice("slice-1-obsolete", node1, pool1, driverA), + func() *resourceapi.ResourceSlice { + slice := sliceWithOneDevice(slice1, node1, pool1, driverA) + // This makes the other slice obsolete. + slice.Spec.Pool.Generation++ + return slice + }(), + ), + node: node(node1, region1), + + expectResults: []any{allocationResult( + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + )}, + }, + "no-slices": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: objects(class(classA, driverA)), + slices: nil, + node: node(node1, region1), + + expectResults: nil, + }, + "not-enough-suitable-devices": { + claimsToAllocate: objects(claim(claim0, req0, classA), claim(claim0, req1, classA)), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + + node: node(node1, region1), + + expectResults: nil, + }, + "no-classes": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: nil, + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: nil, + expectError: gomega.MatchError(gomega.ContainSubstring("could not retrieve device class class-a")), + }, + "unknown-class": { + claimsToAllocate: objects(claim(claim0, req0, "unknown-class")), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: nil, + expectError: gomega.MatchError(gomega.ContainSubstring("could not retrieve device class unknown-class")), + }, + "empty-class": { + claimsToAllocate: objects(claim(claim0, req0, "")), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: nil, + expectError: gomega.MatchError(gomega.ContainSubstring("claim claim-0, request req-0: missing device class name (unsupported request type?)")), + }, + "no-claims-to-allocate": { + claimsToAllocate: nil, + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: nil, + }, + "all-devices": { + claimsToAllocate: objects(claimWithRequests(claim0, nil, resourceapi.DeviceRequest{ + Name: req0, + AllocationMode: resourceapi.DeviceAllocationModeAll, + Count: 1, + DeviceClassName: classA, + })), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: []any{allocationResult( + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + )}, + }, + "all-devices-of-the-incomplete-pool": { + claimsToAllocate: objects(claimWithRequests(claim0, nil, resourceapi.DeviceRequest{ + Name: req0, + AllocationMode: resourceapi.DeviceAllocationModeAll, + Count: 1, + DeviceClassName: classA, + })), + classes: objects(class(classA, driverA)), + slices: objects( + func() *resourceapi.ResourceSlice { + slice := sliceWithOneDevice(slice1, node1, pool1, driverA) + // This makes the pool incomplete, one other slice is missing. + slice.Spec.Pool.ResourceSliceCount++ + return slice + }(), + ), + node: node(node1, region1), + + expectResults: nil, + expectError: gomega.MatchError(gomega.ContainSubstring("claim claim-0, request req-0: asks for all devices, but resource pool driver-a/pool-1 is currently being updated")), + }, + "network-attached-device": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, nodeLabelSelector(regionKey, region1), pool1, driverA)), + node: node(node1, region1), + + expectResults: []any{allocationResult( + nodeLabelSelector(regionKey, region1), + deviceAllocationResult(req0, driverA, pool1, device1), + )}, + }, + "unsuccessful-allocation-network-attached-device": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, nodeLabelSelector(regionKey, region1), pool1, driverA)), + // Wrong region, no devices available. + node: node(node2, region2), + + expectResults: nil, + }, + "many-network-attached-devices": { + claimsToAllocate: objects(claimWithRequests(claim0, nil, request(req0, classA, 4))), + classes: objects(class(classA, driverA)), + slices: objects( + sliceWithOneDevice(slice1, nodeLabelSelector(regionKey, region1), pool1, driverA), + sliceWithOneDevice(slice1, true, pool2, driverA), + sliceWithOneDevice(slice1, nodeLabelSelector(planetKey, planetValueEarth), pool3, driverA), + sliceWithOneDevice(slice1, localNodeSelector(node1), pool4, driverA), + ), + node: node(node1, region1), + + expectResults: []any{allocationResult( + // A union of the individual selectors. + &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{{ + MatchExpressions: []v1.NodeSelectorRequirement{ + {Key: regionKey, Operator: v1.NodeSelectorOpIn, Values: []string{region1}}, + {Key: planetKey, Operator: v1.NodeSelectorOpIn, Values: []string{planetValueEarth}}, + }, + MatchFields: []v1.NodeSelectorRequirement{ + {Key: fieldNameKey, Operator: v1.NodeSelectorOpIn, Values: []string{node1}}, + }, + }}, + }, + deviceAllocationResult(req0, driverA, pool1, device1), + deviceAllocationResult(req0, driverA, pool2, device1), + deviceAllocationResult(req0, driverA, pool3, device1), + deviceAllocationResult(req0, driverA, pool4, device1), + )}, + }, + "local-and-network-attached-devices": { + claimsToAllocate: objects(claimWithRequests(claim0, nil, request(req0, classA, 2))), + classes: objects(class(classA, driverA)), + slices: objects( + sliceWithOneDevice(slice1, nodeLabelSelector(regionKey, region1), pool1, driverA), + sliceWithOneDevice(slice1, node1, pool2, driverA), + ), + node: node(node1, region1), + + expectResults: []any{allocationResult( + // Once there is any node-local device, the selector is for that node. + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + deviceAllocationResult(req0, driverA, pool2, device1), + )}, + }, + "several-different-drivers": { + claimsToAllocate: objects(claim(claim0, req0, classA), claim(claim0, req0, classB)), + classes: objects(class(classA, driverA), class(classB, driverB)), + slices: objects( + slice(slice1, node1, pool1, driverA, + device(device1, nil, nil), + device(device2, nil, nil), + ), + sliceWithOneDevice(slice1, node1, pool1, driverB), + ), + node: node(node1, region1), + + expectResults: []any{ + allocationResult(localNodeSelector(node1), deviceAllocationResult(req0, driverA, pool1, device1)), + allocationResult(localNodeSelector(node1), deviceAllocationResult(req0, driverB, pool1, device1)), + }, + }, + "already-allocated-devices": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + allocatedClaims: objects( + allocatedClaim(claim0, req0, classA, + deviceAllocationResult(req0, driverA, pool1, device1), + deviceAllocationResult(req1, driverA, pool1, device2), + ), + ), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: nil, + }, + "with-constraint": { + claimsToAllocate: objects(claimWithRequests( + claim0, + []resourceapi.DeviceConstraint{ + {MatchAttribute: &intAttribute}, + {MatchAttribute: &versionAttribute}, + {MatchAttribute: &stringAttribute}, + {MatchAttribute: &boolAttribute}, + }, + request(req0, classA, 1), + request(req1, classA, 1), + ), + ), + classes: objects(class(classA, driverA)), + slices: objects(slice(slice1, node1, pool1, driverA, + device(device1, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "driverVersion": {VersionValue: ptr.To("1.0.0")}, + "numa": {IntValue: ptr.To(int64(1))}, + "stringAttribute": {StringValue: ptr.To("stringAttributeValue")}, + "boolAttribute": {BoolValue: ptr.To(true)}, + }), + device(device2, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "driverVersion": {VersionValue: ptr.To("1.0.0")}, + "numa": {IntValue: ptr.To(int64(1))}, + "stringAttribute": {StringValue: ptr.To("stringAttributeValue")}, + "boolAttribute": {BoolValue: ptr.To(true)}, + }), + )), + node: node(node1, region1), + + expectResults: []any{allocationResult( + localNodeSelector(node1), + deviceAllocationResult(req0, driverA, pool1, device1), + deviceAllocationResult(req1, driverA, pool1, device2), + )}, + }, + "with-constraint-non-existent-attribute": { + claimsToAllocate: objects(claim(claim0, req0, classA, resourceapi.DeviceConstraint{ + MatchAttribute: &nonExistentAttribute, + })), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: nil, + }, + "with-constraint-not-matching-int-attribute": { + claimsToAllocate: objects(claimWithRequests( + claim0, + []resourceapi.DeviceConstraint{{MatchAttribute: &intAttribute}}, + request(req0, classA, 3)), + ), + classes: objects(class(classA, driverA), class(classB, driverB)), + slices: objects(slice(slice1, node1, pool1, driverA, + device(device1, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "numa": {IntValue: ptr.To(int64(1))}, + }), + device(device2, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "numa": {IntValue: ptr.To(int64(2))}, + }), + )), + node: node(node1, region1), + + expectResults: nil, + }, + "with-constraint-not-matching-version-attribute": { + claimsToAllocate: objects(claimWithRequests( + claim0, + []resourceapi.DeviceConstraint{{MatchAttribute: &versionAttribute}}, + request(req0, classA, 3)), + ), + classes: objects(class(classA, driverA), class(classB, driverB)), + slices: objects(slice(slice1, node1, pool1, driverA, + device(device1, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "driverVersion": {VersionValue: ptr.To("1.0.0")}, + }), + device(device2, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "driverVersion": {VersionValue: ptr.To("2.0.0")}, + }), + )), + node: node(node1, region1), + + expectResults: nil, + }, + "with-constraint-not-matching-string-attribute": { + claimsToAllocate: objects(claimWithRequests( + claim0, + []resourceapi.DeviceConstraint{{MatchAttribute: &stringAttribute}}, + request(req0, classA, 3)), + ), + classes: objects(class(classA, driverA), class(classB, driverB)), + slices: objects(slice(slice1, node1, pool1, driverA, + device(device1, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "stringAttribute": {StringValue: ptr.To("stringAttributeValue")}, + }), + device(device2, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "stringAttribute": {StringValue: ptr.To("stringAttributeValue2")}, + }), + )), + node: node(node1, region1), + + expectResults: nil, + }, + "with-constraint-not-matching-bool-attribute": { + claimsToAllocate: objects(claimWithRequests( + claim0, + []resourceapi.DeviceConstraint{{MatchAttribute: &boolAttribute}}, + request(req0, classA, 3)), + ), + classes: objects(class(classA, driverA), class(classB, driverB)), + slices: objects(slice(slice1, node1, pool1, driverA, + device(device1, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "boolAttribute": {BoolValue: ptr.To(true)}, + }), + device(device2, nil, map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{ + "boolAttribute": {BoolValue: ptr.To(false)}, + }), + )), + node: node(node1, region1), + + expectResults: nil, + }, + "with-class-device-config": { + claimsToAllocate: objects(claim(claim0, req0, classA)), + classes: objects(classWithConfig(classA, driverA, "classAttribute")), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: []any{ + allocationResultWithConfig( + localNodeSelector(node1), + driverA, + resourceapi.AllocationConfigSourceClass, + "classAttribute", + deviceAllocationResult(req0, driverA, pool1, device1), + ), + }, + }, + "claim-with-device-config": { + claimsToAllocate: objects(claimWithDeviceConfig(claim0, req0, classA, driverA, "deviceAttribute")), + classes: objects(class(classA, driverA)), + slices: objects(sliceWithOneDevice(slice1, node1, pool1, driverA)), + node: node(node1, region1), + + expectResults: []any{ + allocationResultWithConfig( + localNodeSelector(node1), + driverA, + resourceapi.AllocationConfigSourceClaim, + "deviceAttribute", + deviceAllocationResult(req0, driverA, pool1, device1), + ), + }, + }, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + _, ctx := ktesting.NewTestContext(t) + g := gomega.NewWithT(t) + + // Listing objects is deterministic and returns them in the same + // order as in the test case. That makes the allocation result + // also deterministic. + var allocated, toAllocate claimLister + var classLister informerLister[resourceapi.DeviceClass] + var sliceLister informerLister[resourceapi.ResourceSlice] + for _, claim := range tc.claimsToAllocate { + toAllocate.claims = append(toAllocate.claims, claim.DeepCopy()) + } + for _, claim := range tc.allocatedClaims { + allocated.claims = append(allocated.claims, claim.DeepCopy()) + } + for _, slice := range tc.slices { + sliceLister.objs = append(sliceLister.objs, slice.DeepCopy()) + } + for _, class := range tc.classes { + classLister.objs = append(classLister.objs, class.DeepCopy()) + } + + allocator, err := NewAllocator(ctx, toAllocate.claims, allocated, classLister, sliceLister) + g.Expect(err).ToNot(gomega.HaveOccurred()) + + results, err := allocator.Allocate(ctx, tc.node) + matchError := tc.expectError + if matchError == nil { + matchError = gomega.Not(gomega.HaveOccurred()) + } + g.Expect(err).To(matchError) + g.Expect(results).To(gomega.ConsistOf(tc.expectResults...)) + + // Objects that the allocator had access to should not have been modified. + g.Expect(toAllocate.claims).To(gomega.HaveExactElements(tc.claimsToAllocate)) + g.Expect(allocated.claims).To(gomega.HaveExactElements(tc.allocatedClaims)) + g.Expect(sliceLister.objs).To(gomega.ConsistOf(tc.slices)) + g.Expect(classLister.objs).To(gomega.ConsistOf(tc.classes)) + }) + } +} + +type claimLister struct { + claims []*resourceapi.ResourceClaim + err error +} + +func (l claimLister) ListAllAllocated() ([]*resourceapi.ResourceClaim, error) { + return l.claims, l.err +} + +type informerLister[T any] struct { + objs []*T + err error +} + +func (l informerLister[T]) List(selector labels.Selector) (ret []*T, err error) { + if selector.String() != labels.Everything().String() { + return nil, errors.New("labels selector not implemented") + } + return l.objs, l.err +} + +func (l informerLister[T]) Get(name string) (*T, error) { + for _, obj := range l.objs { + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + if accessor.GetName() == name { + return obj, nil + } + } + return nil, apierrors.NewNotFound(schema.GroupResource{}, "not found") +} diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/doc.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/doc.go new file mode 100644 index 00000000000..ed9a2ae7e0d --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2024 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 structured contains code for working with structured parameters. +package structured diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/pools.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/pools.go new file mode 100644 index 00000000000..4c1f8e68bbb --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/structured/pools.go @@ -0,0 +1,131 @@ +/* +Copyright 2024 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 structured + +import ( + "context" + "fmt" + + v1 "k8s.io/api/core/v1" + resourceapi "k8s.io/api/resource/v1alpha3" + "k8s.io/apimachinery/pkg/labels" + resourcelisters "k8s.io/client-go/listers/resource/v1alpha3" + "k8s.io/component-helpers/scheduling/corev1/nodeaffinity" +) + +// GatherPools collects information about all resource pools which provide +// devices that are accessible from the given node. +// +// Out-dated slices are silently ignored. Pools may be incomplete, which is +// recorded in the result. +func GatherPools(ctx context.Context, sliceLister resourcelisters.ResourceSliceLister, node *v1.Node) ([]*Pool, error) { + pools := make(map[PoolID]*Pool) + + // TODO (future): use a custom lister interface and implement it with + // and indexer on the node name field. Then here we can ask for only + // slices with the right node name and those with no node name. + slices, err := sliceLister.List(labels.Everything()) + if err != nil { + return nil, fmt.Errorf("list resource slices: %w", err) + } + for _, slice := range slices { + switch { + case slice.Spec.NodeName != "": + if slice.Spec.NodeName == node.Name { + addSlice(pools, slice) + } + case slice.Spec.AllNodes: + addSlice(pools, slice) + case slice.Spec.NodeSelector != nil: + selector, err := nodeaffinity.NewNodeSelector(slice.Spec.NodeSelector) + if err != nil { + return nil, fmt.Errorf("node selector in resource slice %s: %w", slice.Name, err) + } + if selector.Match(node) { + addSlice(pools, slice) + } + default: + // Nothing known was set. This must be some future, unknown extension, + // so we don't know how to handle it. We may still be able to allocated from + // other pools, so we continue. + // + // TODO (eventually): let caller decide how to report this to the user. Warning + // about it for every slice on each scheduling attempt would be too noisy, but + // perhaps once per run would be useful? + continue + } + + } + + // Find incomplete pools and flatten into a single slice. + result := make([]*Pool, 0, len(pools)) + for _, pool := range pools { + pool.IsIncomplete = int64(len(pool.Slices)) != pool.Slices[0].Spec.Pool.ResourceSliceCount + result = append(result, pool) + } + + return result, nil +} + +func addSlice(pools map[PoolID]*Pool, slice *resourceapi.ResourceSlice) { + id := PoolID{Driver: slice.Spec.Driver, Pool: slice.Spec.Pool.Name} + pool := pools[id] + if pool == nil { + // New pool. + pool = &Pool{ + PoolID: id, + Slices: []*resourceapi.ResourceSlice{slice}, + } + pools[id] = pool + return + } + + if slice.Spec.Pool.Generation < pool.Slices[0].Spec.Pool.Generation { + // Out-dated. + return + } + + if slice.Spec.Pool.Generation > pool.Slices[0].Spec.Pool.Generation { + // Newer, replaces all old slices. + pool.Slices = []*resourceapi.ResourceSlice{slice} + } + + // Add to pool. + pool.Slices = append(pool.Slices, slice) +} + +type Pool struct { + PoolID + IsIncomplete bool + Slices []*resourceapi.ResourceSlice +} + +type PoolID struct { + Driver, Pool string +} + +func (p PoolID) String() string { + return p.Driver + "/" + p.Pool +} + +type DeviceID struct { + Driver, Pool, Device string +} + +func (d DeviceID) String() string { + return d.Driver + "/" + d.Pool + "/" + d.Device +} diff --git a/test/e2e/dra/dra.go b/test/e2e/dra/dra.go index 68b11e1de16..70bc6aa6fab 100644 --- a/test/e2e/dra/dra.go +++ b/test/e2e/dra/dra.go @@ -772,7 +772,20 @@ var _ = framework.SIGDescribe("node")("DRA", feature.DynamicResourceAllocation, ) b.create(ctx, pod, template) - framework.ExpectNoError(e2epod.WaitForPodNameUnschedulableInNamespace(ctx, f.ClientSet, pod.Name, pod.Namespace), "pod must not get scheduled because of a CEL runtime error") + framework.ExpectNoError(e2epod.WaitForPodCondition(ctx, f.ClientSet, pod.Namespace, pod.Name, "scheduling failure", f.Timeouts.PodStartShort, func(pod *v1.Pod) (bool, error) { + for _, condition := range pod.Status.Conditions { + if condition.Type == "PodScheduled" { + if condition.Status != "False" { + gomega.StopTrying("pod got scheduled unexpectedly").Now() + } + if strings.Contains(condition.Message, "CEL runtime error") { + // This is what we are waiting for. + return true, nil + } + } + } + return false, nil + }), "pod must not get scheduled because of a CEL runtime error") }) }) case parameterModeClassicDRA: diff --git a/test/integration/scheduler/scheduler_test.go b/test/integration/scheduler/scheduler_test.go index f47e64ad63f..2e86191f4af 100644 --- a/test/integration/scheduler/scheduler_test.go +++ b/test/integration/scheduler/scheduler_test.go @@ -678,28 +678,13 @@ func TestPodSchedulingContextSSA(t *testing.T) { } } - defer func() { - if err := testCtx.ClientSet.ResourceV1alpha3().ResourceClasses().DeleteCollection(testCtx.Ctx, metav1.DeleteOptions{}, metav1.ListOptions{}); err != nil { - t.Errorf("Unexpected error deleting ResourceClasses: %v", err) - } - }() - class := &resourceapi.ResourceClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-class", - }, - DriverName: "does-not-matter", - } - if _, err := testCtx.ClientSet.ResourceV1alpha3().ResourceClasses().Create(testCtx.Ctx, class, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create class: %v", err) - } - claim := &resourceapi.ResourceClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "my-claim", Namespace: testCtx.NS.Name, }, Spec: resourceapi.ResourceClaimSpec{ - ResourceClassName: class.Name, + Controller: "dra.example.com", }, } if _, err := testCtx.ClientSet.ResourceV1alpha3().ResourceClaims(claim.Namespace).Create(testCtx.Ctx, claim, metav1.CreateOptions{}); err != nil { diff --git a/test/integration/scheduler_perf/config/dra/another-resourceclaimtemplate.yaml b/test/integration/scheduler_perf/config/dra/another-resourceclaimtemplate.yaml index f68127b67a8..9ac0b32715f 100644 --- a/test/integration/scheduler_perf/config/dra/another-resourceclaimtemplate.yaml +++ b/test/integration/scheduler_perf/config/dra/another-resourceclaimtemplate.yaml @@ -1,7 +1,11 @@ -apiVersion: resource.k8s.io/v1alpha1 +apiVersion: resource.k8s.io/v1alpha3 kind: ResourceClaimTemplate metadata: name: another-test-claim-template spec: spec: - resourceClassName: another-test-class + controller: another-test-driver.cdi.k8s.io + devices: + requests: + - name: req-0 + deviceClassName: test-class diff --git a/test/integration/scheduler_perf/config/dra/another-resourceclass.yaml b/test/integration/scheduler_perf/config/dra/another-resourceclass.yaml deleted file mode 100644 index 52eb55698b8..00000000000 --- a/test/integration/scheduler_perf/config/dra/another-resourceclass.yaml +++ /dev/null @@ -1,5 +0,0 @@ -apiVersion: resource.k8s.io/v1alpha1 -kind: ResourceClass -metadata: - name: another-test-class -driverName: another-test-driver.cdi.k8s.io diff --git a/test/integration/scheduler_perf/config/dra/claim-template.yaml b/test/integration/scheduler_perf/config/dra/claim-template.yaml deleted file mode 100644 index 2ccc57e478c..00000000000 --- a/test/integration/scheduler_perf/config/dra/claim-template.yaml +++ /dev/null @@ -1,7 +0,0 @@ -apiVersion: resource.k8s.io/v1alpha1 -kind: ResourceClaimTemplate -metadata: - name: claim-template -spec: - spec: - resourceClassName: scheduler-performance diff --git a/test/integration/scheduler_perf/config/dra/deviceclass-structured.yaml b/test/integration/scheduler_perf/config/dra/deviceclass-structured.yaml new file mode 100644 index 00000000000..873fd282bd2 --- /dev/null +++ b/test/integration/scheduler_perf/config/dra/deviceclass-structured.yaml @@ -0,0 +1,8 @@ +apiVersion: resource.k8s.io/v1alpha3 +kind: DeviceClass +metadata: + name: test-class +spec: + selectors: + - cel: + expression: device.driver == "test-driver.cdi.k8s.io" diff --git a/test/integration/scheduler_perf/config/dra/resourceclass.yaml b/test/integration/scheduler_perf/config/dra/deviceclass.yaml similarity index 54% rename from test/integration/scheduler_perf/config/dra/resourceclass.yaml rename to test/integration/scheduler_perf/config/dra/deviceclass.yaml index 87801ed6584..62417231b9a 100644 --- a/test/integration/scheduler_perf/config/dra/resourceclass.yaml +++ b/test/integration/scheduler_perf/config/dra/deviceclass.yaml @@ -1,5 +1,4 @@ apiVersion: resource.k8s.io/v1alpha3 -kind: ResourceClass +kind: DeviceClass metadata: name: test-class -driverName: test-driver.cdi.k8s.io diff --git a/test/integration/scheduler_perf/config/dra/resourceclaim-structured.yaml b/test/integration/scheduler_perf/config/dra/resourceclaim-structured.yaml index 12b1af5f944..0207afc2a51 100644 --- a/test/integration/scheduler_perf/config/dra/resourceclaim-structured.yaml +++ b/test/integration/scheduler_perf/config/dra/resourceclaim-structured.yaml @@ -3,8 +3,7 @@ kind: ResourceClaim metadata: name: test-claim-{{.Index}} spec: - resourceClassName: test-class - parametersRef: - apiGroup: resource.k8s.io - kind: ResourceClaimParameters - name: test-claim-parameters + devices: + requests: + - name: req-0 + deviceClassName: test-class diff --git a/test/integration/scheduler_perf/config/dra/resourceclaim.yaml b/test/integration/scheduler_perf/config/dra/resourceclaim.yaml index 0f11096d299..510a5a1ba2c 100644 --- a/test/integration/scheduler_perf/config/dra/resourceclaim.yaml +++ b/test/integration/scheduler_perf/config/dra/resourceclaim.yaml @@ -3,4 +3,8 @@ kind: ResourceClaim metadata: name: test-claim-{{.Index}} spec: - resourceClassName: test-class + controller: test-driver.cdi.k8s.io + devices: + requests: + - name: req-0 + deviceClassName: test-class diff --git a/test/integration/scheduler_perf/config/dra/resourceclaimparameters.yaml b/test/integration/scheduler_perf/config/dra/resourceclaimparameters.yaml deleted file mode 100644 index abd395b7b3d..00000000000 --- a/test/integration/scheduler_perf/config/dra/resourceclaimparameters.yaml +++ /dev/null @@ -1,9 +0,0 @@ -apiVersion: resource.k8s.io/v1alpha3 -kind: ResourceClaimParameters -metadata: - name: test-claim-parameters -driverRequests: -- driverName: test-driver.cdi.k8s.io - requests: - - namedResources: - selector: "true" diff --git a/test/integration/scheduler_perf/config/dra/resourceclaimtemplate-structured.yaml b/test/integration/scheduler_perf/config/dra/resourceclaimtemplate-structured.yaml index c98f320592f..77aa178afa5 100644 --- a/test/integration/scheduler_perf/config/dra/resourceclaimtemplate-structured.yaml +++ b/test/integration/scheduler_perf/config/dra/resourceclaimtemplate-structured.yaml @@ -4,8 +4,7 @@ metadata: name: test-claim-template spec: spec: - resourceClassName: test-class - parametersRef: - apiGroup: resource.k8s.io - kind: ResourceClaimParameters - name: test-claim-parameters + devices: + requests: + - name: req-0 + deviceClassName: test-class diff --git a/test/integration/scheduler_perf/config/dra/resourceclaimtemplate.yaml b/test/integration/scheduler_perf/config/dra/resourceclaimtemplate.yaml index 79e66f49ee0..9b94a70b987 100644 --- a/test/integration/scheduler_perf/config/dra/resourceclaimtemplate.yaml +++ b/test/integration/scheduler_perf/config/dra/resourceclaimtemplate.yaml @@ -4,4 +4,8 @@ metadata: name: test-claim-template spec: spec: - resourceClassName: test-class + controller: test-driver.cdi.k8s.io + devices: + requests: + - name: req-0 + deviceClassName: test-class diff --git a/test/integration/scheduler_perf/config/dra/resourceclass-structured.yaml b/test/integration/scheduler_perf/config/dra/resourceclass-structured.yaml deleted file mode 100644 index 8fd4a83267d..00000000000 --- a/test/integration/scheduler_perf/config/dra/resourceclass-structured.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: resource.k8s.io/v1alpha3 -kind: ResourceClass -metadata: - name: test-class -driverName: test-driver.cdi.k8s.io -structuredParameters: true diff --git a/test/integration/scheduler_perf/config/performance-config.yaml b/test/integration/scheduler_perf/config/performance-config.yaml index 8c256a65fc2..4b6905a70ff 100644 --- a/test/integration/scheduler_perf/config/performance-config.yaml +++ b/test/integration/scheduler_perf/config/performance-config.yaml @@ -758,7 +758,7 @@ nodes: scheduler-perf-dra-* maxClaimsPerNodeParam: $maxClaimsPerNode - opcode: createAny - templatePath: config/dra/resourceclass.yaml + templatePath: config/dra/deviceclass.yaml - opcode: createAny templatePath: config/dra/resourceclaimtemplate.yaml namespace: init @@ -829,9 +829,7 @@ nodes: scheduler-perf-dra-* maxClaimsPerNodeParam: $maxClaimsPerNode - opcode: createAny - templatePath: config/dra/resourceclass.yaml - - opcode: createAny - templatePath: config/dra/another-resourceclass.yaml + templatePath: config/dra/deviceclass.yaml - opcode: createAny templatePath: config/dra/resourceclaimtemplate.yaml namespace: init @@ -855,6 +853,7 @@ collectMetrics: true workloads: - name: fast + labels: [integration-test, fast] params: # This testcase runs through all code paths without # taking too long overall. @@ -902,10 +901,7 @@ maxClaimsPerNodeParam: $maxClaimsPerNode structuredParameters: true - opcode: createAny - templatePath: config/dra/resourceclass-structured.yaml - - opcode: createAny - templatePath: config/dra/resourceclaimparameters.yaml - namespace: init + templatePath: config/dra/deviceclass-structured.yaml - opcode: createAny templatePath: config/dra/resourceclaimtemplate-structured.yaml namespace: init @@ -913,9 +909,6 @@ namespace: init countParam: $initPods podTemplatePath: config/dra/pod-with-claim-template.yaml - - opcode: createAny - templatePath: config/dra/resourceclaimparameters.yaml - namespace: test - opcode: createAny templatePath: config/dra/resourceclaimtemplate-structured.yaml namespace: test @@ -979,10 +972,7 @@ maxClaimsPerNodeParam: $maxClaimsPerNode structuredParameters: true - opcode: createAny - templatePath: config/dra/resourceclass-structured.yaml - - opcode: createAny - templatePath: config/dra/resourceclaimparameters.yaml - namespace: init + templatePath: config/dra/deviceclass-structured.yaml - opcode: createAny templatePath: config/dra/resourceclaim-structured.yaml namespace: init @@ -991,9 +981,6 @@ namespace: init countParam: $initPods podTemplatePath: config/dra/pod-with-claim-ref.yaml - - opcode: createAny - templatePath: config/dra/resourceclaimparameters.yaml - namespace: test - opcode: createAny templatePath: config/dra/resourceclaim-structured.yaml namespace: test @@ -1100,4 +1087,4 @@ labels: [performance, fast] params: gatedPods: 10000 - measurePods: 20000 \ No newline at end of file + measurePods: 20000 diff --git a/test/integration/scheduler_perf/dra.go b/test/integration/scheduler_perf/dra.go index 7f2ab2dcd2d..d76cfd6b2ac 100644 --- a/test/integration/scheduler_perf/dra.go +++ b/test/integration/scheduler_perf/dra.go @@ -202,7 +202,7 @@ func (op *createResourceDriverOp) run(tCtx ktesting.TContext) { tCtx.CleanupCtx(func(tCtx ktesting.TContext) { err := tCtx.Client().ResourceV1alpha3().ResourceSlices().DeleteCollection(tCtx, metav1.DeleteOptions{}, - metav1.ListOptions{FieldSelector: "driverName=" + op.DriverName}, + metav1.ListOptions{FieldSelector: resourceapi.ResourceSliceSelectorDriver + "=" + op.DriverName}, ) tCtx.ExpectNoError(err, "delete node resource slices") }) @@ -234,18 +234,21 @@ func resourceSlice(driverName, nodeName string, capacity int) *resourceapi.Resou Name: nodeName, }, - NodeName: nodeName, - DriverName: driverName, - - ResourceModel: resourceapi.ResourceModel{ - NamedResources: &resourceapi.NamedResourcesResources{}, + Spec: resourceapi.ResourceSliceSpec{ + Driver: driverName, + NodeName: nodeName, + Pool: resourceapi.ResourcePool{ + Name: nodeName, + ResourceSliceCount: 1, + }, }, } for i := 0; i < capacity; i++ { - slice.ResourceModel.NamedResources.Instances = append(slice.ResourceModel.NamedResources.Instances, - resourceapi.NamedResourcesInstance{ - Name: fmt.Sprintf("instance-%d", i), + slice.Spec.Devices = append(slice.Spec.Devices, + resourceapi.Device{ + Name: fmt.Sprintf("instance-%d", i), + Basic: &resourceapi.BasicDevice{}, }, ) }