From 77eae7c34f99c434631673fa93d0019ce64c9f86 Mon Sep 17 00:00:00 2001 From: googs1025 Date: Wed, 8 Jan 2025 16:24:28 +0800 Subject: [PATCH] feature(scheduler): remove dra plugin resourceslice QueueingHintFn --- .../dynamicresources/dynamicresources.go | 34 +----- .../dynamicresources/dynamicresources_test.go | 105 ------------------ 2 files changed, 1 insertion(+), 138 deletions(-) diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go index 886a9b9c849..bc8537e8118 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go @@ -176,7 +176,7 @@ func (pl *DynamicResources) EventsToRegister(_ context.Context) ([]framework.Clu // A pod might be waiting for a class to get created or modified. {Event: framework.ClusterEvent{Resource: framework.DeviceClass, ActionType: framework.Add | framework.Update}}, // Adding or updating a ResourceSlice might make a pod schedulable because new resources became available. - {Event: framework.ClusterEvent{Resource: framework.ResourceSlice, ActionType: framework.Add | framework.Update}, QueueingHintFn: pl.isSchedulableAfterResourceSliceChange}, + {Event: framework.ClusterEvent{Resource: framework.ResourceSlice, ActionType: framework.Add | framework.Update}}, } return events, nil @@ -288,38 +288,6 @@ func (pl *DynamicResources) isSchedulableAfterPodChange(logger klog.Logger, pod return framework.Queue, nil } -// isSchedulableAfterResourceSliceChange is invoked for add and update slice events reported by -// an informer. Such changes can make an unschedulable pod schedulable when the pod requests a device -// and the change adds a suitable device. -// -// For the sake of faster execution and avoiding code duplication, isSchedulableAfterResourceSliceChange -// only checks whether the pod uses claims. All of the more detailed checks are done in the scheduling -// attempt. -// -// The delete claim event will not invoke it, so newObj will never be nil. -func (pl *DynamicResources) isSchedulableAfterResourceSliceChange(logger klog.Logger, pod *v1.Pod, oldObj, newObj interface{}) (framework.QueueingHint, error) { - _, modifiedSlice, err := schedutil.As[*resourceapi.ResourceSlice](oldObj, newObj) - if err != nil { - // Shouldn't happen. - return framework.Queue, fmt.Errorf("unexpected object in isSchedulableAfterResourceSliceChange: %w", err) - } - - if err := pl.foreachPodResourceClaim(pod, nil); err != nil { - // This is not an unexpected error: we know that - // foreachPodResourceClaim only returns errors for "not - // schedulable". - logger.V(6).Info("pod is not schedulable after resource slice change", "pod", klog.KObj(pod), "resourceSlice", klog.KObj(modifiedSlice), "reason", err.Error()) - return framework.QueueSkip, nil - } - - // We could check what got changed in the slice, but right now that's likely to be - // about the spec (there's no status yet...). - // We could check whether all claims use classic DRA, but that doesn't seem worth it. - // Let's assume that changing the slice may make the pod schedulable. - logger.V(5).Info("ResourceSlice change might make pod schedulable", "pod", klog.KObj(pod), "resourceSlice", klog.KObj(modifiedSlice)) - return framework.Queue, nil -} - // podResourceClaims returns the ResourceClaims for all pod.Spec.PodResourceClaims. func (pl *DynamicResources) podResourceClaims(pod *v1.Pod) ([]*resourceapi.ResourceClaim, error) { claims := make([]*resourceapi.ResourceClaim, 0, len(pod.Spec.ResourceClaims)) diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go index 71d347f2c82..5896b72509d 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go @@ -1412,108 +1412,3 @@ func Test_isSchedulableAfterPodChange(t *testing.T) { }) } } - -func Test_isSchedulableAfterResourceSliceChange(t *testing.T) { - testcases := map[string]struct { - pod *v1.Pod - claims []*resourceapi.ResourceClaim - oldObj, newObj interface{} - wantHint framework.QueueingHint - wantErr bool - }{ - "queue-new-resource-slice": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - newObj: resourceSlice, - wantHint: framework.Queue, - }, - "queue1-update-resource-slice-with-claim-is-allocated": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{allocatedClaim}, - oldObj: resourceSlice, - newObj: resourceSliceUpdated, - wantHint: framework.Queue, - }, - "queue-update-resource-slice-with-claim-is-deleting": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{deleteClaim}, - oldObj: resourceSlice, - newObj: resourceSliceUpdated, - wantHint: framework.QueueSkip, - }, - "queue-new-resource-slice-with-two-claim": { - pod: podWithTwoClaimNames, - claims: []*resourceapi.ResourceClaim{pendingClaim, pendingClaim2}, - oldObj: resourceSlice, - newObj: resourceSliceUpdated, - wantHint: framework.Queue, - }, - "queue-update-resource-slice-with-two-claim-but-one-hasn't-been-created": { - pod: podWithTwoClaimNames, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - oldObj: resourceSlice, - newObj: resourceSliceUpdated, - wantHint: framework.QueueSkip, - }, - "queue-update-resource-slice": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - oldObj: resourceSlice, - newObj: resourceSliceUpdated, - wantHint: framework.Queue, - }, - "skip-not-find-resource-claim": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{}, - oldObj: resourceSlice, - newObj: resourceSliceUpdated, - wantHint: framework.QueueSkip, - }, - "backoff-unexpected-object-with-oldObj-newObj": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - oldObj: pendingClaim, - newObj: pendingClaim, - wantErr: true, - }, - "backoff-unexpected-object-with-oldObj": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - oldObj: pendingClaim, - newObj: resourceSlice, - wantErr: true, - }, - "backoff-unexpected-object-with-newObj": { - pod: podWithClaimName, - claims: []*resourceapi.ResourceClaim{pendingClaim}, - oldObj: resourceSlice, - newObj: pendingClaim, - wantErr: true, - }, - } - for name, tc := range testcases { - tc := tc - t.Run(name, func(t *testing.T) { - t.Parallel() - logger, _ := ktesting.NewTestContext(t) - features := feature.Features{ - EnableDynamicResourceAllocation: true, - } - testCtx := setup(t, nil, tc.claims, nil, nil, features) - gotHint, err := testCtx.p.isSchedulableAfterResourceSliceChange(logger, tc.pod, tc.oldObj, tc.newObj) - if tc.wantErr { - if err == nil { - t.Fatal("want an error, got none") - } - return - } - - if err != nil { - t.Fatalf("want no error, got: %v", err) - } - if tc.wantHint != gotHint { - t.Fatalf("want %#v, got %#v", tc.wantHint.String(), gotHint.String()) - } - }) - } -}