Skip to content

Commit cfdc365

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#86133 from Huang-Wei/cleanup-predicate-path
Eliminate running paths of Predicates in scheduler
2 parents 9f2ae40 + dc3d1bd commit cfdc365

File tree

17 files changed

+720
-505
lines changed

17 files changed

+720
-505
lines changed

‎pkg/scheduler/BUILD‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,17 @@ go_test(
7777
"//pkg/scheduler/core:go_default_library",
7878
"//pkg/scheduler/framework/plugins:go_default_library",
7979
"//pkg/scheduler/framework/plugins/nodelabel:go_default_library",
80+
"//pkg/scheduler/framework/plugins/nodeports:go_default_library",
81+
"//pkg/scheduler/framework/plugins/noderesources:go_default_library",
8082
"//pkg/scheduler/framework/plugins/serviceaffinity:go_default_library",
83+
"//pkg/scheduler/framework/plugins/volumebinding:go_default_library",
8184
"//pkg/scheduler/framework/v1alpha1:go_default_library",
8285
"//pkg/scheduler/internal/cache:go_default_library",
8386
"//pkg/scheduler/internal/cache/fake:go_default_library",
8487
"//pkg/scheduler/internal/queue:go_default_library",
8588
"//pkg/scheduler/nodeinfo:go_default_library",
8689
"//pkg/scheduler/nodeinfo/snapshot:go_default_library",
90+
"//pkg/scheduler/testing:go_default_library",
8791
"//pkg/scheduler/volumebinder:go_default_library",
8892
"//staging/src/k8s.io/api/core/v1:go_default_library",
8993
"//staging/src/k8s.io/api/events/v1beta1:go_default_library",

‎pkg/scheduler/algorithm_factory.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ var (
7171
schedulerFactoryMutex sync.RWMutex
7272

7373
// maps that hold registered algorithm types
74+
// TODO(Huang-Wei): remove this.
7475
fitPredicateMap = make(map[string]FitPredicateFactory)
7576
mandatoryFitPredicates = sets.NewString()
7677
priorityFunctionMap = make(map[string]PriorityConfigFactory)
@@ -143,6 +144,7 @@ func ApplyPredicatesAndPriorities(s *Snapshot) {
143144

144145
// RegisterFitPredicate registers a fit predicate with the algorithm
145146
// registry. Returns the name with which the predicate was registered.
147+
// TODO(Huang-Wei): remove this.
146148
func RegisterFitPredicate(name string, predicate predicates.FitPredicate) string {
147149
return RegisterFitPredicateFactory(name, func(AlgorithmFactoryArgs) predicates.FitPredicate { return predicate })
148150
}

‎pkg/scheduler/core/BUILD‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,17 @@ go_test(
5555
"//pkg/scheduler/algorithm/priorities/util:go_default_library",
5656
"//pkg/scheduler/apis/config:go_default_library",
5757
"//pkg/scheduler/apis/extender/v1:go_default_library",
58+
"//pkg/scheduler/framework/plugins/interpodaffinity:go_default_library",
59+
"//pkg/scheduler/framework/plugins/noderesources:go_default_library",
60+
"//pkg/scheduler/framework/plugins/podtopologyspread:go_default_library",
5861
"//pkg/scheduler/framework/v1alpha1:go_default_library",
5962
"//pkg/scheduler/internal/cache:go_default_library",
6063
"//pkg/scheduler/internal/queue:go_default_library",
6164
"//pkg/scheduler/listers:go_default_library",
6265
"//pkg/scheduler/listers/fake:go_default_library",
6366
"//pkg/scheduler/nodeinfo:go_default_library",
6467
"//pkg/scheduler/nodeinfo/snapshot:go_default_library",
68+
"//pkg/scheduler/testing:go_default_library",
6569
"//pkg/scheduler/util:go_default_library",
6670
"//staging/src/k8s.io/api/core/v1:go_default_library",
6771
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",

‎pkg/scheduler/core/extender_test.go‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
internalcache "k8s.io/kubernetes/pkg/scheduler/internal/cache"
4343
internalqueue "k8s.io/kubernetes/pkg/scheduler/internal/queue"
4444
schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
45+
st "k8s.io/kubernetes/pkg/scheduler/testing"
4546
"k8s.io/kubernetes/pkg/scheduler/util"
4647
)
4748

@@ -348,16 +349,16 @@ var _ algorithm.SchedulerExtender = &FakeExtender{}
348349

349350
func TestGenericSchedulerWithExtenders(t *testing.T) {
350351
tests := []struct {
351-
name string
352-
predicates map[string]predicates.FitPredicate
353-
prioritizers []priorities.PriorityConfig
354-
extenders []FakeExtender
355-
nodes []string
356-
expectedResult ScheduleResult
357-
expectsErr bool
352+
name string
353+
registerFilterPlugin st.RegisterFilterPluginFunc
354+
prioritizers []priorities.PriorityConfig
355+
extenders []FakeExtender
356+
nodes []string
357+
expectedResult ScheduleResult
358+
expectsErr bool
358359
}{
359360
{
360-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
361+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
361362
extenders: []FakeExtender{
362363
{
363364
predicates: []fitPredicate{truePredicateExtender},
@@ -371,7 +372,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
371372
name: "test 1",
372373
},
373374
{
374-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
375+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
375376
extenders: []FakeExtender{
376377
{
377378
predicates: []fitPredicate{truePredicateExtender},
@@ -385,7 +386,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
385386
name: "test 2",
386387
},
387388
{
388-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
389+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
389390
extenders: []FakeExtender{
390391
{
391392
predicates: []fitPredicate{truePredicateExtender},
@@ -403,7 +404,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
403404
name: "test 3",
404405
},
405406
{
406-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
407+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
407408
extenders: []FakeExtender{
408409
{
409410
predicates: []fitPredicate{machine2PredicateExtender},
@@ -417,7 +418,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
417418
name: "test 4",
418419
},
419420
{
420-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
421+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
421422
extenders: []FakeExtender{
422423
{
423424
predicates: []fitPredicate{truePredicateExtender},
@@ -434,7 +435,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
434435
name: "test 5",
435436
},
436437
{
437-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
438+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
438439
extenders: []FakeExtender{
439440
{
440441
predicates: []fitPredicate{truePredicateExtender},
@@ -456,8 +457,8 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
456457
name: "test 6",
457458
},
458459
{
459-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
460-
prioritizers: []priorities.PriorityConfig{{Map: machine2Prioritizer, Weight: 20}},
460+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
461+
prioritizers: []priorities.PriorityConfig{{Map: machine2Prioritizer, Weight: 20}},
461462
extenders: []FakeExtender{
462463
{
463464
predicates: []fitPredicate{truePredicateExtender},
@@ -481,8 +482,8 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
481482
// If scheduler sends the pod by mistake, the test would fail
482483
// because of the errors from errorPredicateExtender and/or
483484
// errorPrioritizerExtender.
484-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
485-
prioritizers: []priorities.PriorityConfig{{Map: machine2Prioritizer, Weight: 1}},
485+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
486+
prioritizers: []priorities.PriorityConfig{{Map: machine2Prioritizer, Weight: 1}},
486487
extenders: []FakeExtender{
487488
{
488489
predicates: []fitPredicate{errorPredicateExtender},
@@ -505,7 +506,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
505506
//
506507
// If scheduler did not ignore the extender, the test would fail
507508
// because of the errors from errorPredicateExtender.
508-
predicates: map[string]predicates.FitPredicate{"true": truePredicate},
509+
registerFilterPlugin: st.RegisterFilterPlugin("TrueFilter", NewTrueFilterPlugin),
509510
extenders: []FakeExtender{
510511
{
511512
predicates: []fitPredicate{errorPredicateExtender},
@@ -540,15 +541,24 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
540541
cache.AddNode(createNode(name))
541542
}
542543
queue := internalqueue.NewSchedulingQueue(nil)
544+
545+
registry := framework.Registry{}
546+
plugins := &schedulerapi.Plugins{
547+
Filter: &schedulerapi.PluginSet{},
548+
}
549+
var pluginConfigs []schedulerapi.PluginConfig
550+
test.registerFilterPlugin(&registry, plugins, pluginConfigs)
551+
fwk, _ := framework.NewFramework(registry, plugins, pluginConfigs)
552+
543553
scheduler := NewGenericScheduler(
544554
cache,
545555
queue,
546-
test.predicates,
556+
nil,
547557
predicates.EmptyMetadataProducer,
548558
test.prioritizers,
549559
priorities.EmptyMetadataProducer,
550560
emptySnapshot,
551-
emptyFramework,
561+
fwk,
552562
extenders,
553563
nil,
554564
informerFactory.Core().V1().PersistentVolumeClaims().Lister(),

‎pkg/scheduler/core/generic_scheduler.go‎

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ type FailedPredicateMap map[string][]predicates.PredicateFailureReason
7272

7373
// FitError describes a fit error of a pod.
7474
type FitError struct {
75-
Pod *v1.Pod
76-
NumAllNodes int
75+
Pod *v1.Pod
76+
NumAllNodes int
77+
// TODO(Huang-Wei): remove 'FailedPredicates'
7778
FailedPredicates FailedPredicateMap
7879
FilteredNodesStatuses framework.NodeToStatusMap
7980
}
@@ -476,12 +477,13 @@ func (g *genericScheduler) numFeasibleNodesToFind(numAllNodes int32) (numNodes i
476477

477478
// Filters the nodes to find the ones that fit based on the given predicate functions
478479
// Each node is passed through the predicate functions to determine if it is a fit
480+
// TODO(Huang-Wei): remove 'FailedPredicateMap' from the return parameters.
479481
func (g *genericScheduler) findNodesThatFit(ctx context.Context, state *framework.CycleState, pod *v1.Pod) ([]*v1.Node, FailedPredicateMap, framework.NodeToStatusMap, error) {
480482
var filtered []*v1.Node
481483
failedPredicateMap := FailedPredicateMap{}
482484
filteredNodesStatuses := framework.NodeToStatusMap{}
483485

484-
if len(g.predicates) == 0 && !g.framework.HasFilterPlugins() {
486+
if !g.framework.HasFilterPlugins() {
485487
filtered = g.nodeInfoSnapshot.ListNodes()
486488
} else {
487489
allNodes := len(g.nodeInfoSnapshot.NodeInfoList)
@@ -506,7 +508,7 @@ func (g *genericScheduler) findNodesThatFit(ctx context.Context, state *framewor
506508
// We check the nodes starting from where we left off in the previous scheduling cycle,
507509
// this is to make sure all nodes have the same chance of being examined across pods.
508510
nodeInfo := g.nodeInfoSnapshot.NodeInfoList[(g.nextStartNodeIndex+i)%allNodes]
509-
fits, failedPredicates, status, err := g.podFitsOnNode(
511+
fits, _, status, err := g.podFitsOnNode(
510512
ctx,
511513
state,
512514
pod,
@@ -531,9 +533,6 @@ func (g *genericScheduler) findNodesThatFit(ctx context.Context, state *framewor
531533
if !status.IsSuccess() {
532534
filteredNodesStatuses[nodeInfo.Node().Name] = status
533535
}
534-
if len(failedPredicates) != 0 {
535-
failedPredicateMap[nodeInfo.Node().Name] = failedPredicates
536-
}
537536
predicateResultLock.Unlock()
538537
}
539538
}
@@ -566,6 +565,7 @@ func (g *genericScheduler) findNodesThatFit(ctx context.Context, state *framewor
566565
return []*v1.Node{}, FailedPredicateMap{}, framework.NodeToStatusMap{}, err
567566
}
568567

568+
// TODO(Huang-Wei): refactor this to fill 'filteredNodesStatuses' instead of 'failedPredicateMap'.
569569
for failedNodeName, failedMsg := range failedMap {
570570
if _, found := failedPredicateMap[failedNodeName]; !found {
571571
failedPredicateMap[failedNodeName] = []predicates.PredicateFailureReason{}
@@ -584,6 +584,7 @@ func (g *genericScheduler) findNodesThatFit(ctx context.Context, state *framewor
584584
// addNominatedPods adds pods with equal or greater priority which are nominated
585585
// to run on the node given in nodeInfo to meta and nodeInfo. It returns 1) whether
586586
// any pod was added, 2) augmented metadata, 3) augmented CycleState 4) augmented nodeInfo.
587+
// TODO(Huang-Wei): remove 'meta predicates.Metadata' from the signature.
587588
func (g *genericScheduler) addNominatedPods(ctx context.Context, pod *v1.Pod, meta predicates.Metadata, state *framework.CycleState,
588589
nodeInfo *schedulernodeinfo.NodeInfo) (bool, predicates.Metadata,
589590
*framework.CycleState, *schedulernodeinfo.NodeInfo, error) {
@@ -662,46 +663,18 @@ func (g *genericScheduler) podFitsOnNode(
662663
// nominated pods are running because they are not running right now and in fact,
663664
// they may end up getting scheduled to a different node.
664665
for i := 0; i < 2; i++ {
665-
metaToUse := meta
666666
stateToUse := state
667667
nodeInfoToUse := info
668668
if i == 0 {
669669
var err error
670-
podsAdded, metaToUse, stateToUse, nodeInfoToUse, err = g.addNominatedPods(ctx, pod, meta, state, info)
670+
podsAdded, _, stateToUse, nodeInfoToUse, err = g.addNominatedPods(ctx, pod, meta, state, info)
671671
if err != nil {
672672
return false, []predicates.PredicateFailureReason{}, nil, err
673673
}
674674
} else if !podsAdded || len(failedPredicates) != 0 || !status.IsSuccess() {
675675
break
676676
}
677677

678-
for _, predicateKey := range predicates.Ordering() {
679-
var (
680-
fit bool
681-
reasons []predicates.PredicateFailureReason
682-
err error
683-
)
684-
685-
if predicate, exist := g.predicates[predicateKey]; exist {
686-
fit, reasons, err = predicate(pod, metaToUse, nodeInfoToUse)
687-
if err != nil {
688-
return false, []predicates.PredicateFailureReason{}, nil, err
689-
}
690-
691-
if !fit {
692-
// eCache is available and valid, and predicates result is unfit, record the fail reasons
693-
failedPredicates = append(failedPredicates, reasons...)
694-
// if alwaysCheckAllPredicates is false, short circuit all predicates when one predicate fails.
695-
if !alwaysCheckAllPredicates {
696-
klog.V(5).Infoln("since alwaysCheckAllPredicates has not been set, the predicate " +
697-
"evaluation is short circuited and there are chances " +
698-
"of other predicates failing as well.")
699-
break
700-
}
701-
}
702-
}
703-
}
704-
705678
status = g.framework.RunFilterPlugins(ctx, stateToUse, pod, nodeInfoToUse)
706679
if !status.IsSuccess() && !status.IsUnschedulable() {
707680
return false, failedPredicates, status, status.AsError()
@@ -1270,6 +1243,7 @@ func podPassesBasicChecks(pod *v1.Pod, pvcLister corelisters.PersistentVolumeCla
12701243
}
12711244

12721245
// NewGenericScheduler creates a genericScheduler object.
1246+
// TODO(Huang-Wei): remove 'predicates' and 'alwaysCheckAllPredicates'.
12731247
func NewGenericScheduler(
12741248
cache internalcache.Cache,
12751249
podQueue internalqueue.SchedulingQueue,

0 commit comments

Comments
 (0)