New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Feb 7, 2018

Conversation

Projects
None yet
4 participants
@yguo0905
Contributor

yguo0905 commented Feb 5, 2018

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

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

This comment has been minimized.

@bsalamat

bsalamat Feb 5, 2018

Contributor

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.

This comment has been minimized.

@yguo0905

yguo0905 Feb 6, 2018

Contributor

Done.

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

This comment has been minimized.

@bsalamat

bsalamat Feb 5, 2018

Contributor

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.

This comment has been minimized.

@yguo0905

yguo0905 Feb 6, 2018

Contributor

Done.

@k8s-ci-robot k8s-ci-robot added size/L and removed size/M labels Feb 6, 2018

@yguo0905

This comment has been minimized.

Contributor

yguo0905 commented Feb 6, 2018

/retest

@bsalamat

Thanks, @yguo0905!

/lgtm

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

This comment has been minimized.

@bsalamat

bsalamat Feb 6, 2018

Contributor

Thanks for adding this!

@k8s-ci-robot k8s-ci-robot added the lgtm label Feb 6, 2018

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Feb 6, 2018

[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:

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

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Feb 7, 2018

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.

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

13 checks passed

Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation yguo0905 authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-unit Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@yguo0905 yguo0905 deleted the yguo0905:sched-config branch Feb 7, 2018

mindprince added a commit to mindprince/kubernetes that referenced this pull request Apr 6, 2018

Provide a more ideal example for scheduler extender policy.
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).

mindprince added a commit to mindprince/examples that referenced this pull request Apr 9, 2018

Provide a more ideal example for scheduler extender policy.
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).

mindprince added a commit to mindprince/examples that referenced this pull request Apr 9, 2018

Provide a more ideal example for scheduler extender policy.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment