Merge pull request kubernetes#129517 from googs1025/feature/remove/dra_resourceslice_qhint

feature(scheduler): remove dra plugin resourceslice QueueingHintFn

Author: k8s-ci-robot

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))

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())
-			}
-		})
-	}
-}