Skip to content

Commit ed826dd

Browse files
committed
fix(dra plugin): when there is no resourceclaim, return directly
1 parent 27ec5de commit ed826dd

File tree

2 files changed

+36
-9
lines changed

2 files changed

+36
-9
lines changed

pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,11 @@ func (pl *DynamicResources) PostFilter(ctx context.Context, cs *framework.CycleS
576576
if !pl.enabled {
577577
return nil, framework.NewStatus(framework.Unschedulable, "plugin disabled")
578578
}
579+
// If a Pod doesn't have any resource claims attached to it, there is no need for further processing.
580+
// Thus we provide a fast path for this case to avoid unnecessary computations.
581+
if len(pod.Spec.ResourceClaims) == 0 {
582+
return nil, framework.NewStatus(framework.Unschedulable)
583+
}
579584
logger := klog.FromContext(ctx)
580585
state, err := getStateData(cs)
581586
if err != nil {

pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,8 @@ func TestPlugin(t *testing.T) {
290290
prepare prepare
291291
want want
292292

293+
// enableDRAAdminAccess is set to true if the DRAAdminAccess feature gate is enabled.
294+
enableDRAAdminAccess bool
293295
// Feature gates. False is chosen so that the uncommon case
294296
// doesn't need to be set.
295297
disableDRA bool
@@ -301,7 +303,7 @@ func TestPlugin(t *testing.T) {
301303
status: framework.NewStatus(framework.Skip),
302304
},
303305
postfilter: result{
304-
status: framework.NewStatus(framework.Unschedulable, `no new claims to deallocate`),
306+
status: framework.NewStatus(framework.Unschedulable),
305307
},
306308
},
307309
},
@@ -554,9 +556,10 @@ func TestPlugin(t *testing.T) {
554556
},
555557
},
556558

557-
"request-admin-access": {
558-
// Because the pending claim asks for admin access, allocation succeeds despite resources
559-
// being exhausted.
559+
"request-admin-access-with-DRAAdminAccess-featuregate": {
560+
// When the DRAAdminAccess feature gate is enabled,
561+
// Because the pending claim asks for admin access,
562+
// allocation succeeds despite resources being exhausted.
560563
pod: podWithClaimName,
561564
claims: []*resourceapi.ResourceClaim{adminAccess(pendingClaim), otherAllocatedClaim},
562565
classes: []*resourceapi.DeviceClass{deviceClass},
@@ -582,6 +585,24 @@ func TestPlugin(t *testing.T) {
582585
assumedClaim: reserve(adminAccess(allocatedClaim), podWithClaimName),
583586
},
584587
},
588+
enableDRAAdminAccess: true,
589+
},
590+
"request-admin-access-without-DRAAdminAccess-featuregate": {
591+
// When the DRAAdminAccess feature gate is disabled,
592+
// even though the pending claim requests admin access,
593+
// the scheduler returns an unschedulable status.
594+
pod: podWithClaimName,
595+
claims: []*resourceapi.ResourceClaim{adminAccess(pendingClaim), otherAllocatedClaim},
596+
classes: []*resourceapi.DeviceClass{deviceClass},
597+
objs: []apiruntime.Object{workerNodeSlice},
598+
want: want{
599+
filter: perNodeResult{
600+
workerNode.Name: {
601+
status: framework.NewStatus(framework.UnschedulableAndUnresolvable, `claim default/my-pod-my-resource, request req-1: admin access is requested, but the feature is disabled`),
602+
},
603+
},
604+
},
605+
enableDRAAdminAccess: false,
585606
},
586607

587608
"structured-ignore-allocated-admin-access": {
@@ -768,6 +789,9 @@ func TestPlugin(t *testing.T) {
768789
prefilter: result{
769790
status: framework.NewStatus(framework.Skip),
770791
},
792+
postfilter: result{
793+
status: framework.NewStatus(framework.Unschedulable, `plugin disabled`),
794+
},
771795
},
772796
disableDRA: true,
773797
},
@@ -783,6 +807,7 @@ func TestPlugin(t *testing.T) {
783807
nodes = []*v1.Node{workerNode}
784808
}
785809
features := feature.Features{
810+
EnableDRAAdminAccess: tc.enableDRAAdminAccess,
786811
EnableDynamicResourceAllocation: !tc.disableDRA,
787812
}
788813
testCtx := setup(t, nodes, tc.claims, tc.classes, tc.objs, features)
@@ -801,9 +826,6 @@ func TestPlugin(t *testing.T) {
801826
assert.Equal(t, tc.want.preFilterResult, result)
802827
testCtx.verify(t, tc.want.prefilter, initialObjects, result, status)
803828
})
804-
if status.IsSkip() {
805-
return
806-
}
807829
unschedulable := status.Code() != framework.Success
808830

809831
var potentialNodes []*framework.NodeInfo
@@ -912,9 +934,9 @@ type testContext struct {
912934

913935
func (tc *testContext) verify(t *testing.T, expected result, initialObjects []metav1.Object, result interface{}, status *framework.Status) {
914936
t.Helper()
915-
if expectedErr := status.AsError(); expectedErr != nil {
937+
if actualErr := status.AsError(); actualErr != nil {
916938
// Compare only the error strings.
917-
assert.ErrorContains(t, status.AsError(), expectedErr.Error())
939+
assert.ErrorContains(t, actualErr, expected.status.AsError().Error())
918940
} else {
919941
assert.Equal(t, expected.status, status)
920942
}

0 commit comments

Comments
 (0)