Add a scheduler profile level parameter percentageOfNodesToScore #112521

yuanchen8911 · 2022-09-16T20:30:48Z

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

Add percentageOfNodesToScore as a per scheduler profile parameter in API version v1.

The PR makes the following changes:

Add profile level PercentageOfNodesToScore to API Version v1. v1beta2and v1beta3 will still use the old way (global value). In v1, if a profile level percentageOfNodesToScore is set, it will override the global (scheduler level) value. If the profile percentageOfNodesToScore is not set and the global percentageOfNodesToScore is set, the global value will be used. Otherwise, the default percentageOfNodesToScore is used.
Change both scheduler and profile level PercentageOfNodesToScore to pointers.

The goal is to support fine-grained per profile/workload scheduling customization and better balance scheduling performance and quality in multi-tenant Kubernetes clusters.

Which issue(s) this PR fixes:

Fixes # #93270

Reference:#95709 #95823
Previous PR: #97263 (incomplete)

Special notes for your reviewer:

We'll review and evaluate if the current global percentageOfNodesToScore should be deprecated and removed in the future versions.

TODO (future PRs)

Replace deprecated pointer.Int32Ptr/Int64Ptr with pointer.Int32/Int64 in v1.
Replace errorString by field.ErrorList so the coverage is more comprehensive.
Replace PluginConfig variable with defaults.PluginConfigs when it applies in apis/config/scheme/scheme_test.go.

Does this PR introduce a user-facing change?

Add percentageOfNodesToScore as a scheduler profile level parameter to API version v1. If a profile percentageOfNodesToScore is set, it will override global percentageOfNodesToScore.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

yuanchen8911 · 2022-09-16T20:35:11Z

/cc @Huang-Wei @ahg-g

yuanchen8911 · 2022-09-16T20:49:16Z

@Huang-Wei got a bunch of errors. Should I make changes to config/v1beta2 and v1beta3 too? Local testing and build worked fine. Thanks.

E0916 20:35:29.981766    6344 conversion.go:756] Warning: could not find nor generate a final Conversion function for k8s.io/kubernetes/pkg/scheduler/apis/config.KubeSchedulerProfile -> k8s.io/kube-scheduler/config/v1beta2.KubeSchedulerProfile
[33](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/112521/pull-kubernetes-verify-govet-levee/1570872862435184640#1:build-log.txt%3A33)
E0916 20:35:29.981863    6344 conversion.go:757]   the following fields need manual conversion:
[34](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/112521/pull-kubernetes-verify-govet-levee/1570872862435184640#1:build-log.txt%3A34)
E0916 20:35:29.981868    6344 conversion.go:759]       - PercentageOfNodesToScore
[35](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/112521/pull-kubernetes-verify-govet-levee/1570872862435184640#1:build-log.txt%3A35)
E0916 20:35:30.022813    6344 conversion.go:756] Warning: could not find nor generate a final Conversion function for k8s.io/kubernetes/pkg/scheduler/apis/config.KubeSchedulerProfile -> k8s.io/kube-scheduler/config/v1beta3.KubeSchedulerProfile
[36](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/112521/pull-kubernetes-verify-govet-levee/1570872862435184640#1:build-log.txt%3A36)
E0916 20:35:30.022849    6344 conversion.go:757]   the following fields need manual conversion:
[37](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/112521/pull-kubernetes-verify-govet-levee/1570872862435184640#1:build-log.txt%3A37)
E0916 20:35:30.022855    6344 conversion.go:759]       - PercentageOfNodesToScore

k8s-triage-robot · 2022-09-16T21:49:55Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

Huang-Wei · 2022-09-16T22:30:59Z

@yuanchen8911 you need to update the spec in versioned types.go as well: staging/src/k8s.io/kube-scheduler/config/v1/types.go. Then rerun codegen script, and then optionally, fill the conversion logic if the auto-generated is not smart enough to cover it for you.

staging/src/k8s.io/kube-scheduler/config/v1/types.go

yuanchen8911 · 2022-09-17T03:00:55Z

@yuanchen8911 you need to update the spec in versioned types.go as well: staging/src/k8s.io/kube-scheduler/config/v1/types.go. Then rerun codegen script, and then optionally, fill the conversion logic if the auto-generated is not smart enough to cover it for you.

Fixed manual conversions.

k8s-ci-robot · 2022-10-14T01:48:05Z

@yuanchen8911: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test pull-kubernetes-conformance-kind-ga-only-parallel
/test pull-kubernetes-coverage-unit
/test pull-kubernetes-dependencies
/test pull-kubernetes-dependencies-go-canary
/test pull-kubernetes-e2e-gce
/test pull-kubernetes-e2e-gce-100-performance
/test pull-kubernetes-e2e-gce-big-performance
/test pull-kubernetes-e2e-gce-canary
/test pull-kubernetes-e2e-gce-network-proxy-http-connect
/test pull-kubernetes-e2e-gce-no-stage
/test pull-kubernetes-e2e-gce-scale-performance-manual
/test pull-kubernetes-e2e-gce-ubuntu-containerd
/test pull-kubernetes-e2e-gce-ubuntu-containerd-canary
/test pull-kubernetes-e2e-kind
/test pull-kubernetes-e2e-kind-ipv6
/test pull-kubernetes-integration
/test pull-kubernetes-integration-go-canary
/test pull-kubernetes-kubemark-e2e-gce-scale
/test pull-kubernetes-node-e2e-containerd
/test pull-kubernetes-typecheck
/test pull-kubernetes-unit
/test pull-kubernetes-unit-go-canary
/test pull-kubernetes-update
/test pull-kubernetes-verify
/test pull-kubernetes-verify-go-canary
/test pull-kubernetes-verify-govet-levee

The following commands are available to trigger optional jobs:

/test check-dependency-stats
/test pull-ci-kubernetes-unit-windows
/test pull-e2e-gce-cloud-provider-disabled
/test pull-kubernetes-conformance-image-test
/test pull-kubernetes-conformance-kind-ga-only
/test pull-kubernetes-conformance-kind-ipv6-parallel
/test pull-kubernetes-cross
/test pull-kubernetes-e2e-capz-azure-disk
/test pull-kubernetes-e2e-capz-azure-disk-vmss
/test pull-kubernetes-e2e-capz-azure-file
/test pull-kubernetes-e2e-capz-azure-file-vmss
/test pull-kubernetes-e2e-capz-conformance
/test pull-kubernetes-e2e-capz-ha-control-plane
/test pull-kubernetes-e2e-capz-windows-containerd
/test pull-kubernetes-e2e-containerd-gce
/test pull-kubernetes-e2e-gce-alpha-features
/test pull-kubernetes-e2e-gce-correctness
/test pull-kubernetes-e2e-gce-csi-serial
/test pull-kubernetes-e2e-gce-device-plugin-gpu
/test pull-kubernetes-e2e-gce-kubetest2
/test pull-kubernetes-e2e-gce-network-proxy-grpc
/test pull-kubernetes-e2e-gce-storage-disruptive
/test pull-kubernetes-e2e-gce-storage-slow
/test pull-kubernetes-e2e-gce-storage-snapshot
/test pull-kubernetes-e2e-gce-ubuntu-containerd-serial
/test pull-kubernetes-e2e-gci-gce-autoscaling
/test pull-kubernetes-e2e-gci-gce-ingress
/test pull-kubernetes-e2e-gci-gce-ipvs
/test pull-kubernetes-e2e-kind-canary
/test pull-kubernetes-e2e-kind-dual-canary
/test pull-kubernetes-e2e-kind-ipv6-canary
/test pull-kubernetes-e2e-kind-ipvs-dual-canary
/test pull-kubernetes-e2e-kind-multizone
/test pull-kubernetes-e2e-kops-aws
/test pull-kubernetes-e2e-ubuntu-gce-network-policies
/test pull-kubernetes-kind-dra
/test pull-kubernetes-kubemark-e2e-gce-big
/test pull-kubernetes-local-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e
/test pull-kubernetes-node-crio-cgrpv2-e2e-kubetest2
/test pull-kubernetes-node-crio-e2e
/test pull-kubernetes-node-crio-e2e-kubetest2
/test pull-kubernetes-node-e2e-containerd-alpha-features
/test pull-kubernetes-node-e2e-containerd-features
/test pull-kubernetes-node-e2e-containerd-features-kubetest2
/test pull-kubernetes-node-e2e-containerd-kubetest2
/test pull-kubernetes-node-kubelet-credential-provider
/test pull-kubernetes-node-kubelet-serial-containerd
/test pull-kubernetes-node-kubelet-serial-containerd-kubetest2
/test pull-kubernetes-node-kubelet-serial-cpu-manager
/test pull-kubernetes-node-kubelet-serial-cpu-manager-kubetest2
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv1
/test pull-kubernetes-node-kubelet-serial-crio-cgroupv2
/test pull-kubernetes-node-kubelet-serial-hugepages
/test pull-kubernetes-node-kubelet-serial-memory-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager
/test pull-kubernetes-node-kubelet-serial-topology-manager-kubetest2
/test pull-kubernetes-node-memoryqos-cgrpv2
/test pull-kubernetes-node-swap-fedora
/test pull-kubernetes-node-swap-fedora-serial
/test pull-kubernetes-node-swap-ubuntu-serial
/test pull-kubernetes-unit-experimental
/test pull-publishing-bot-validate

Use /test all to run the following jobs that were automatically triggered:

pull-kubernetes-conformance-kind-ga-only-parallel
pull-kubernetes-dependencies
pull-kubernetes-e2e-gce-100-performance
pull-kubernetes-e2e-gce-ubuntu-containerd
pull-kubernetes-e2e-kind
pull-kubernetes-e2e-kind-ipv6
pull-kubernetes-integration
pull-kubernetes-node-e2e-containerd
pull-kubernetes-typecheck
pull-kubernetes-unit
pull-kubernetes-verify
pull-kubernetes-verify-govet-levee

In response to this:

/retest failed

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yuanchen8911 · 2022-10-14T01:49:17Z

/test pull-kubernetes-integration

kerthcet · 2022-10-14T03:11:26Z

pkg/scheduler/apis/config/types.go

-	// nodes will be scored.
-	PercentageOfNodesToScore int32
+	// nodes will be scored. It is overridden by profile level PercentageOfNodesToScore.
+	PercentageOfNodesToScore *int32


If we want to keep the API stable, why can we change this? Because no difference to end-users?
So the conclusion is if we can keep the consistent experience to users, no breaking change, then we can make some modifications, right?

Keep it consistent with the profile percentage. We need pointer to tell the difference between 0 and not set (nil) for profile percentage. If it's the latter, the global percentage will be used.

alculquicondor

just nits.

pkg/scheduler/apis/config/scheme/scheme_test.go

alculquicondor · 2022-10-14T13:26:59Z

pkg/scheduler/apis/config/scheme/scheme_test.go

+					SchedulerName:            "default-scheduler",
+					PercentageOfNodesToScore: nil,
+					Plugins:                  defaults.PluginsV1beta3,
+					PluginConfig: []config.PluginConfig{


defaults.PluginConfigV1beta3

It's PluginsV1beta3.

kubernetes/pkg/scheduler/apis/config/testing/defaults/defaults.go

Line 155 in 9bedff1

var PluginsV1beta3 = &config.Plugins{

I'm referring to this one

kubernetes/pkg/scheduler/apis/config/testing/defaults/defaults.go

Line 277 in 9bedff1

var PluginConfigsV1beta3 = []config.PluginConfig{

OK, replaced PluginConfig: ... with PluginConfig: defaults.PluginConfigsV1beta3

alculquicondor · 2022-10-14T13:31:17Z

pkg/scheduler/scheduler_test.go

+	tests := []struct {
+		name                           string
+		percentageOfNodesToScore       *int32
+		option                         *schedulerOptions


create this object inside the loop

Actually, forget about this suggestion.
Can you instead call New(..., WithPercentageOfNodesToScore) and then verify that the scheduler is created with the appropriate value?

schedulerOptions is an internal object, so it's not much worth testing at that level.

Rewrote the test function to create a scheduler using WithPercentageOfNodesToScore and to verify its percentageOfNodesToScore is set correctly.

alculquicondor · 2022-10-14T17:28:37Z

There are a couple of suggestions that are still not addressed

yuanchen8911 · 2022-10-14T19:15:12Z

There are a couple of suggestions that are still not addressed

Made the suggested changes. Please take another look. Thanks.

alculquicondor · 2022-10-14T19:38:45Z

pkg/scheduler/apis/config/scheme/scheme_test.go

+					SchedulerName:            "default-scheduler",
+					PercentageOfNodesToScore: nil,
+					Plugins:                  defaults.PluginsV1beta2,
+					PluginConfig: []config.PluginConfig{


defaults.PluginConfigsV1beta2 here

Replaced it here. Other PluginConfigs cannot use defaults.DefaultPluginConfigs unless we modify minCandidateNodesPercentage: 50 to 10 (default value) and/or bindTimeoutSeconds: 300 to 600 (default value).

There are a few places that can be improved. How about a separate PR for a complete cleanup?

yes, those sound tangential

Thanks, added it to TODO list.

alculquicondor

missed one comment :)

Fix conversion errors Changed the order update update fix manaul coversions keep the global parameter for backward compatibility Address Wei's comments Fix an error Fix issues Add unit tests for validation Fix a comment Address comments Update comments fix verifiation errors Add tests for scheme_test.go Convert percentageOfNodesToScore to pointer Fix errors Resolve conflicts Fix testing errors Address Wei's comments Revert IntPtr to Int changes Address comments Not overrite percentageOfNodesToScore Fix a bug Fix a bug change errs to err Fix a nit Remove duplication Address comments Fix lint warning Fix an issue Update comments Clean up Address comments Revert changes to defaults fix unit test error Update Fix tests Use default PluginConfigs

alculquicondor · 2022-10-14T20:17:36Z

/lgtm
/approve

yuanchen8911 · 2022-10-15T11:28:15Z

/assign @liggitt

liggitt · 2022-10-17T14:22:22Z

/approve
for config API changes

k8s-ci-robot · 2022-10-17T14:23:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, Huang-Wei, liggitt, yuanchen8911

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~cmd/kube-scheduler/OWNERS~~ [Huang-Wei,alculquicondor,liggitt]
~~pkg/generated/openapi/OWNERS~~ [liggitt]
~~pkg/scheduler/OWNERS~~ [Huang-Wei,alculquicondor,liggitt]
~~staging/src/k8s.io/kube-scheduler/config/OWNERS~~ [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Huang-Wei · 2022-10-17T16:46:47Z

/unhold

yuanchen8911 · 2022-10-17T23:06:20Z

Hi @alculquicondor, the PR was merged, but there's a red X with it. Is it related to pending/failed pull-kubernetes-e2e-gce-100-performance test. Just wanted to make sure everything is fine.

yuanchen8911 · 2022-10-17T23:58:42Z

/test pull-kubernetes-e2e-gce-100-performance

liggitt · 2022-10-18T00:22:11Z

That means the CI job passed on a batch merge that included this commit, and failed on this individual PR. That happens occasionally with flaky jobs

yuanchen8911 · 2022-10-18T01:09:37Z

That means the CI job passed on a batch merge that included this commit, and failed on this individual PR. That happens occasionally with flaky jobs

Thanks! So we don't need to rerun the test since it passed all tests as part of the batch merge.

liggitt · 2022-10-18T01:36:19Z

Right

k8s-ci-robot requested a review from caesarxuchao September 16, 2022 20:30

k8s-ci-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Sep 16, 2022

k8s-ci-robot requested a review from sanposhiho September 16, 2022 20:30

k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Sep 16, 2022

k8s-ci-robot requested review from ahg-g and Huang-Wei September 16, 2022 20:35

yuanchen8911 changed the title ~~Change percentageOfNodesToScore to a scheduler profile config parameter~~ Change percentageOfNodesToScore to a profile config parameter Sep 16, 2022

yuanchen8911 changed the title ~~Change percentageOfNodesToScore to a profile config parameter~~ Change percentageOfNodesToScore to a profile config parameter (WIP) Sep 16, 2022

yuanchen8911 force-pushed the profile-config branch from 74fdb48 to aa80bc4 Compare September 16, 2022 22:44

k8s-ci-robot added area/code-generation sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Sep 16, 2022

Huang-Wei reviewed Sep 16, 2022

View reviewed changes

staging/src/k8s.io/kube-scheduler/config/v1/types.go Outdated Show resolved Hide resolved

yuanchen8911 force-pushed the profile-config branch from aa80bc4 to 2586d82 Compare September 16, 2022 22:56

yuanchen8911 changed the title ~~Change percentageOfNodesToScore to a profile config parameter (WIP)~~ Make percentageOfNodesToScore a profile config parameter (WIP) Sep 16, 2022

yuanchen8911 force-pushed the profile-config branch 2 times, most recently from 4d791f4 to fb58e03 Compare September 17, 2022 02:13

yuanchen8911 changed the title ~~Make percentageOfNodesToScore a profile config parameter (WIP)~~ Make percentageOfNodesToScore a profile configuration parameter Sep 17, 2022

yuanchen8911 removed request for caesarxuchao and ahg-g September 17, 2022 03:00

kerthcet reviewed Oct 14, 2022

View reviewed changes

alculquicondor reviewed Oct 14, 2022

View reviewed changes

yuanchen8911 force-pushed the profile-config branch from 13d3b8a to 148aa16 Compare October 14, 2022 15:58

yuanchen8911 requested a review from alculquicondor October 14, 2022 17:06

yuanchen8911 force-pushed the profile-config branch from 148aa16 to d533d77 Compare October 14, 2022 19:14

alculquicondor reviewed Oct 14, 2022

View reviewed changes

yuanchen8911 force-pushed the profile-config branch from d533d77 to 7297f48 Compare October 14, 2022 20:01

yuanchen8911 requested a review from alculquicondor October 14, 2022 20:05

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2022

liggitt moved this from Changes requested to API review completed, 1.26 in API Reviews Oct 17, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 17, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 17, 2022

k8s-ci-robot merged commit 521fbd7 into kubernetes:master Oct 17, 2022

kerthcet mentioned this pull request Dec 15, 2022

scheduler: support scheduling profile-level configuration parameters #93270

Closed

Add a scheduler profile level parameter percentageOfNodesToScore #112521

Add a scheduler profile level parameter percentageOfNodesToScore #112521

Conversation

yuanchen8911 commented Sep 16, 2022 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

yuanchen8911 commented Sep 16, 2022

yuanchen8911 commented Sep 16, 2022 • edited

k8s-triage-robot commented Sep 16, 2022

Huang-Wei commented Sep 16, 2022

yuanchen8911 commented Sep 17, 2022

k8s-ci-robot commented Oct 14, 2022

yuanchen8911 commented Oct 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alculquicondor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuanchen8911 Oct 14, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuanchen8911 Oct 14, 2022 • edited

Choose a reason for hiding this comment

alculquicondor commented Oct 14, 2022

yuanchen8911 commented Oct 14, 2022 • edited

alculquicondor Oct 14, 2022 • edited

Choose a reason for hiding this comment

yuanchen8911 Oct 14, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alculquicondor left a comment • edited

Choose a reason for hiding this comment

alculquicondor commented Oct 14, 2022

yuanchen8911 commented Oct 15, 2022

liggitt commented Oct 17, 2022

k8s-ci-robot commented Oct 17, 2022

Huang-Wei commented Oct 17, 2022

yuanchen8911 commented Oct 17, 2022 • edited

yuanchen8911 commented Oct 17, 2022

liggitt commented Oct 18, 2022

yuanchen8911 commented Oct 18, 2022

liggitt commented Oct 18, 2022

yuanchen8911 commented Sep 16, 2022 •

edited

yuanchen8911 commented Sep 16, 2022 •

edited

yuanchen8911 Oct 14, 2022 •

edited

yuanchen8911 Oct 14, 2022 •

edited

yuanchen8911 commented Oct 14, 2022 •

edited

alculquicondor Oct 14, 2022 •

edited

yuanchen8911 Oct 14, 2022 •

edited

alculquicondor left a comment •

edited

yuanchen8911 commented Oct 17, 2022 •

edited