kube-scheduler: Use default predicates/prioritizers if they are unspecified in the policy config #59363

yguo0905 · 2018-02-05T19:03:44Z

What this PR does / why we need it:

The scheduler has built-in default sets of predicate/prioritizer that are applied on pod scheduling. It can also take a policy config file where predicate/prioritizer and extender settings can be specified. The current behavior is that if we want to configure an extender using the policy config, we have to also provide the default predicate/prioritizer settings. Otherwise, the empty predicate/prioritizer sets will be used.

This is inconvenient, and it's hard to keep the policy config up to date with the scheduler's defaults. This PR changes the scheduler to use the default predicate/prioritizer sets if they are unspecified in the policy config. But an empty list would bypass non-mandatory predicates/prioritizers.

This will change the behavior of a policy config that does not specify (but not empty list) predicate/prioritizer, but it's unlike someone is using such config in practice.

Special notes for your reviewer:

I think it makes sense to have this in 1.9 as well because

It's safe, given the scope of this change and the fact that it's very unlikely that someone is using a policy config with empty predicates/prioritizers.
Compared with the risk, asking users to provide the default predicate/prioritizer sets for is error-prone and may cause other issues.

Release note:

kube-scheduler: Use default predicates/prioritizers if they are unspecified in the policy config

/sig scheduling
/assign @bsalamat
/cc @vishh

bsalamat · 2018-02-05T22:08:38Z

pkg/scheduler/factory/factory.go

-	for _, predicate := range policy.Predicates {
-		glog.V(2).Infof("Registering predicate: %s", predicate.Name)
-		predicateKeys.Insert(RegisterCustomFitPredicate(predicate))
+	if len(policy.Predicates) == 0 {


Can you change it to use defaults only when the predicates and/or priorities are not provided? We would like to keep the possibility of bypassing all predicate or priority functions.

bsalamat · 2018-02-05T22:10:53Z

pkg/scheduler/factory/factory_test.go

+// Test configures a scheduler from a policy that does not contain any
+// predicates or priorities.
+// The predicates or priorities from DefaultProvider will be used.
+func TestCreateFromConfigWithEmptyPredicatesAndPriorities(t *testing.T) {


We need an integration test to cover all the logic, including the decoding of policy config. Coding an integration test is similar. Please see this.

yguo0905 · 2018-02-06T20:01:31Z

/retest

…does not specify them

bsalamat

Thanks, @yguo0905!

/lgtm

bsalamat · 2018-02-06T22:43:42Z

pkg/scheduler/factory/factory_test.go

@@ -541,3 +513,23 @@ func TestSkipPodUpdate(t *testing.T) {
 		}
 	}
 }
+
+func newConfigFactory(client *clientset.Clientset, hardPodAffinitySymmetricWeight int32) scheduler.Configurator {


Thanks for adding this!

k8s-ci-robot · 2018-02-06T22:51:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsalamat, yguo0905

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~pkg/scheduler/OWNERS~~ [bsalamat]
~~test/integration/scheduler/OWNERS~~ [bsalamat]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2018-02-07T05:34:44Z

Automatic merge from submit-queue (batch tested with PRs 59394, 58769, 59423, 59363, 59245). If you want to cherry-pick this change to another branch, please follow the instructions here.

We now use default predicates/priorities if they are unspecified in the policy config (kubernetes#59363). The other example in ./scheduler-policy-config.json shows how to use priorities and predicates. Let's use this example to show how to use extenders. When you are using extenders you don't necessarily want to mess with priorities and predicates (kubernetes#45188).

We now use default predicates/priorities if they are unspecified in the policy config (kubernetes/kubernetes#59363). The other example in ./scheduler-policy-config.json shows how to use priorities and predicates. Let's use this example to show how to use extenders. When you are using extenders you don't necessarily want to mess with priorities and predicates (kubernetes/kubernetes#45188).

We now use default predicates/priorities if they are unspecified in the policy config (kubernetes/kubernetes#59363). The other example in ./scheduler-policy-config.json shows how to use priorities and predicates. Let's use this example to show how to use extenders. When you are using extenders you don't necessarily want to mess with priorities and predicates (kubernetes/kubernetes#45188). Also remove duplicate file.

k8s-ci-robot assigned bsalamat Feb 5, 2018

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 5, 2018

k8s-ci-robot requested a review from vishh February 5, 2018 19:03

k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 5, 2018

yguo0905 force-pushed the sched-config branch from 0129075 to 010fbd4 Compare February 5, 2018 21:10

bsalamat reviewed Feb 5, 2018

View reviewed changes

yguo0905 force-pushed the sched-config branch from 010fbd4 to b9707ba Compare February 6, 2018 19:01

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 6, 2018

kube-scheduler: Use default predicates/prioritizers if policy config …

f69eaa3

…does not specify them

yguo0905 force-pushed the sched-config branch from b9707ba to f69eaa3 Compare February 6, 2018 21:01

bsalamat approved these changes Feb 6, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 6, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 6, 2018

k8s-github-robot merged commit 4b01601 into kubernetes:master Feb 7, 2018

yguo0905 deleted the sched-config branch February 7, 2018 17:59

yguo0905 mentioned this pull request Feb 7, 2018

Automated cherry pick of #59363: kube-scheduler: Use default predicates/prioritizers if they are unspecified in the policy config #59493

Closed

bsalamat mentioned this pull request Feb 13, 2018

Adding a scheduler extender requires duplicating the full scheduler policy config #45188

Closed

rohitagarwal003 mentioned this pull request Apr 6, 2018

Provide a more ideal example for scheduler extender policy. #62186

Closed

rohitagarwal003 mentioned this pull request Apr 9, 2018

Provide a more ideal example for scheduler extender policy. kubernetes/examples#224

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kube-scheduler: Use default predicates/prioritizers if they are unspecified in the policy config #59363

kube-scheduler: Use default predicates/prioritizers if they are unspecified in the policy config #59363

yguo0905 commented Feb 5, 2018 •

edited

Loading

bsalamat Feb 5, 2018 •

edited

Loading

yguo0905 Feb 6, 2018

bsalamat Feb 5, 2018

yguo0905 Feb 6, 2018

yguo0905 commented Feb 6, 2018

bsalamat left a comment

bsalamat Feb 6, 2018

k8s-ci-robot commented Feb 6, 2018

k8s-github-robot commented Feb 7, 2018

kube-scheduler: Use default predicates/prioritizers if they are unspecified in the policy config #59363

kube-scheduler: Use default predicates/prioritizers if they are unspecified in the policy config #59363

Conversation

yguo0905 commented Feb 5, 2018 • edited Loading

bsalamat Feb 5, 2018 • edited Loading

Choose a reason for hiding this comment

yguo0905 Feb 6, 2018

Choose a reason for hiding this comment

bsalamat Feb 5, 2018

Choose a reason for hiding this comment

yguo0905 Feb 6, 2018

Choose a reason for hiding this comment

yguo0905 commented Feb 6, 2018

bsalamat left a comment

Choose a reason for hiding this comment

bsalamat Feb 6, 2018

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 6, 2018

k8s-github-robot commented Feb 7, 2018

yguo0905 commented Feb 5, 2018 •

edited

Loading

bsalamat Feb 5, 2018 •

edited

Loading