From 7f041297364f927cf1a6b91ecdc43e027b38ae90 Mon Sep 17 00:00:00 2001 From: Harry Zhang Date: Mon, 19 Mar 2018 12:15:24 -0700 Subject: [PATCH] Add Ignorable flag to extender Ignore extender in generic scheduler Add test to verify the ignorable flag Fix warning msg --- .../algorithm/scheduler_interface.go | 4 +++ pkg/scheduler/api/types.go | 3 ++ pkg/scheduler/api/v1/types.go | 3 ++ pkg/scheduler/core/extender.go | 13 +++++++-- pkg/scheduler/core/extender_test.go | 27 ++++++++++++++++++ pkg/scheduler/core/generic_scheduler.go | 28 +++++++++++++++---- pkg/scheduler/factory/factory_test.go | 5 ++++ 7 files changed, 76 insertions(+), 7 deletions(-) diff --git a/pkg/scheduler/algorithm/scheduler_interface.go b/pkg/scheduler/algorithm/scheduler_interface.go index 61c740c60a..e43077151f 100644 --- a/pkg/scheduler/algorithm/scheduler_interface.go +++ b/pkg/scheduler/algorithm/scheduler_interface.go @@ -64,6 +64,10 @@ type SchedulerExtender interface { // SupportsPreemption returns if the scheduler extender support preemption or not. SupportsPreemption() bool + + // IsIgnorable returns true indicates scheduling should not fail when this extender + // is unavailable. This gives scheduler ability to fail fast and tolerate non-critical extenders as well. + IsIgnorable() bool } // ScheduleAlgorithm is an interface implemented by things that know how to schedule pods diff --git a/pkg/scheduler/api/types.go b/pkg/scheduler/api/types.go index 7b31dfe7fe..8ce5d205bb 100644 --- a/pkg/scheduler/api/types.go +++ b/pkg/scheduler/api/types.go @@ -192,6 +192,9 @@ type ExtenderConfig struct { // will skip checking the resource in predicates. // +optional ManagedResources []ExtenderManagedResource + // Ignorable specifies if the extender is ignorable, i.e. scheduling should not + // fail when the extender returns an error or is not reachable. + Ignorable bool } // ExtenderPreemptionResult represents the result returned by preemption phase of extender. diff --git a/pkg/scheduler/api/v1/types.go b/pkg/scheduler/api/v1/types.go index 80ad839a03..21412c3ea1 100644 --- a/pkg/scheduler/api/v1/types.go +++ b/pkg/scheduler/api/v1/types.go @@ -174,6 +174,9 @@ type ExtenderConfig struct { // will skip checking the resource in predicates. // +optional ManagedResources []ExtenderManagedResource `json:"managedResources,omitempty"` + // Ignorable specifies if the extender is ignorable, i.e. scheduling should not + // fail when the extender returns an error or is not reachable. + Ignorable bool `json:"ignorable,omitempty"` } // ExtenderArgs represents the arguments needed by the extender to filter/prioritize diff --git a/pkg/scheduler/core/extender.go b/pkg/scheduler/core/extender.go index 8664ba8b52..a654905e28 100644 --- a/pkg/scheduler/core/extender.go +++ b/pkg/scheduler/core/extender.go @@ -49,6 +49,7 @@ type HTTPExtender struct { client *http.Client nodeCacheCapable bool managedResources sets.String + ignorable bool } func makeTransport(config *schedulerapi.ExtenderConfig) (http.RoundTripper, error) { @@ -102,9 +103,16 @@ func NewHTTPExtender(config *schedulerapi.ExtenderConfig) (algorithm.SchedulerEx client: client, nodeCacheCapable: config.NodeCacheCapable, managedResources: managedResources, + ignorable: config.Ignorable, }, nil } +// IsIgnorable returns true indicates scheduling should not fail when this extender +// is unavailable +func (h *HTTPExtender) IsIgnorable() bool { + return h.ignorable +} + // SupportsPreemption returns if a extender support preemption. // A extender should have preempt verb defined and enabled its own node cache. func (h *HTTPExtender) SupportsPreemption() bool { @@ -147,11 +155,12 @@ func (h *HTTPExtender) ProcessPreemption( // Extender will always return NodeNameToMetaVictims. // So let's convert it to NodeToVictims by using NodeNameToInfo. - nodeToVictims, err := h.convertToNodeToVictims(result.NodeNameToMetaVictims, nodeNameToInfo) + newNodeToVictims, err := h.convertToNodeToVictims(result.NodeNameToMetaVictims, nodeNameToInfo) if err != nil { return nil, err } - return nodeToVictims, nil + // Do not override nodeToVictims + return newNodeToVictims, nil } // convertToNodeToVictims converts "nodeNameToMetaVictims" from object identifiers, diff --git a/pkg/scheduler/core/extender_test.go b/pkg/scheduler/core/extender_test.go index 6064ba04e3..99855a4f28 100644 --- a/pkg/scheduler/core/extender_test.go +++ b/pkg/scheduler/core/extender_test.go @@ -113,12 +113,17 @@ type FakeExtender struct { nodeCacheCapable bool filteredNodes []*v1.Node unInterested bool + ignorable bool // Cached node information for fake extender cachedNodeNameToInfo map[string]*schedulercache.NodeInfo cachedPDBs []*policy.PodDisruptionBudget } +func (f *FakeExtender) IsIgnorable() bool { + return f.ignorable +} + func (f *FakeExtender) SupportsPreemption() bool { // Assume preempt verb is always defined. return true @@ -456,6 +461,28 @@ func TestGenericSchedulerWithExtenders(t *testing.T) { expectedHost: "machine2", // machine2 has higher score name: "test 8", }, + { + // Scheduling is expected to not fail in + // Filter/Prioritize phases if the extender is not available and ignorable. + // + // If scheduler did not ignore the extender, the test will fail + // because of the errors from errorPredicateExtender. + predicates: map[string]algorithm.FitPredicate{"true": truePredicate}, + prioritizers: []algorithm.PriorityConfig{{Map: EqualPriorityMap, Weight: 1}}, + extenders: []FakeExtender{ + { + predicates: []fitPredicate{errorPredicateExtender}, + ignorable: true, + }, + { + predicates: []fitPredicate{machine1PredicateExtender}, + }, + }, + nodes: []string{"machine1", "machine2"}, + expectsErr: false, + expectedHost: "machine1", + name: "test 9", + }, } for _, test := range tests { diff --git a/pkg/scheduler/core/generic_scheduler.go b/pkg/scheduler/core/generic_scheduler.go index 7571b2cf69..365fc79ffe 100644 --- a/pkg/scheduler/core/generic_scheduler.go +++ b/pkg/scheduler/core/generic_scheduler.go @@ -269,12 +269,24 @@ func (g *genericScheduler) processPreemptionWithExtenders( if len(nodeToVictims) > 0 { for _, extender := range g.extenders { if extender.SupportsPreemption() { - var err error - // Replace nodeToVictims with result after preemption from extender. - if nodeToVictims, err = extender.ProcessPreemption(pod, nodeToVictims, g.cachedNodeInfoMap); err != nil { + newNodeToVictims, err := extender.ProcessPreemption( + pod, + nodeToVictims, + g.cachedNodeInfoMap, + ) + if err != nil { + if extender.IsIgnorable() { + glog.Warningf("Skipping extender %v as it returned error %v and has ignorable flag set", + extender, err) + continue + } return nil, err } - // If node list is empty, no preemption will happen, skip other extenders. + // Replace nodeToVictims with new result after preemption. So the + // rest of extenders can continue use it as parameter. + nodeToVictims = newNodeToVictims + + // If node list becomes empty, no preemption can happen regardless of other extenders. if len(nodeToVictims) == 0 { break } @@ -384,7 +396,13 @@ func findNodesThatFit( } filteredList, failedMap, err := extender.Filter(pod, filtered, nodeNameToInfo) if err != nil { - return []*v1.Node{}, FailedPredicateMap{}, err + if extender.IsIgnorable() { + glog.Warningf("Skipping extender %v as it returned error %v and has ignorable flag set", + extender, err) + continue + } else { + return []*v1.Node{}, FailedPredicateMap{}, err + } } for failedNodeName, failedMsg := range failedMap { diff --git a/pkg/scheduler/factory/factory_test.go b/pkg/scheduler/factory/factory_test.go index 924e48bb4d..a49e2e0198 100644 --- a/pkg/scheduler/factory/factory_test.go +++ b/pkg/scheduler/factory/factory_test.go @@ -539,6 +539,11 @@ func newConfigFactory(client *clientset.Clientset, hardPodAffinitySymmetricWeigh type fakeExtender struct { isBinder bool interestedPodName string + ignorable bool +} + +func (f *fakeExtender) IsIgnorable() bool { + return f.ignorable } func (f *fakeExtender) ProcessPreemption(