Add Schedulings Profiles to kubescheduler.config.k8s.io/v1alpha2 #88087

alculquicondor · 2020-02-12T18:26:59Z

What type of PR is this?

/kind api-change

What this PR does / why we need it:

Cluster operators can use profiles to offer different scheduling behaviors to pods. Each profile is associated to a scheduler name which pods can select through .spec.schedulerName.
If not defined, a profile is added with the name default-scheduler.

This PR only implements API changes, along with defaults, validation and conversion from v1alpha1. kube-scheduler will only use the first profile and preserve its behavior.

A follow up PR will add runtime support for the multiple profiles.

Which issue(s) this PR fixes:

Part of #87617, #85737 and kubernetes/enhancements#1451

Does this PR introduce a user-facing change?:

Add Scheduling Profiles to kubescheduler.config.k8s.io/v1alpha2

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

- [KEP]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-scheduling/20200114-multi-scheduling-profiles.md

fejta-bot · 2020-02-12T18:57:30Z

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.

alculquicondor · 2020-02-12T22:03:39Z

/assign @ahg-g @damemi

alculquicondor · 2020-02-12T22:03:57Z

/hold for the parent PR

cmd/kube-scheduler/app/options/deprecated.go

cmd/kube-scheduler/app/options/options_test.go

pkg/scheduler/apis/config/types.go

ahg-g · 2020-02-13T01:31:31Z

pkg/scheduler/apis/config/types.go

+	// SchedulerName is name of the scheduler, used to select which pods
+	// will be processed by this profile, based on pod's "spec.SchedulerName".


Name is name of the profile ...

pkg/scheduler/apis/config/v1alpha2/defaults.go

pkg/scheduler/apis/config/validation/validation.go

pkg/scheduler/apis/config/types.go

ahg-g · 2020-02-13T03:01:53Z

pkg/scheduler/apis/config/validation/validation.go

+func validateCommonQueueSort(path *field.Path, profiles []config.KubeSchedulerProfile) field.ErrorList {
+	allErrs := field.ErrorList{}
+	var canon *config.PluginSet
+	if profiles[0].Plugins != nil {


How about the PluginConfig? It should be the same for all profiles as well, but I guess this is harder to validate since we don't really know what exact QueueSort Plugin names we will end up with at this point, so perhaps we can delay this to Framework instantiation.

That is my thinking as well. I left a TODO here anyways.

pkg/scheduler/apis/config/types.go

ahg-g

looks good to me

For api review
/cc @liggitt

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

pkg/scheduler/apis/config/types.go

Huang-Wei

Some nits.

Huang-Wei · 2020-02-14T22:15:17Z

pkg/scheduler/apis/config/v1alpha1/conversion.go


+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"


An idiomatic alias is metav1.

pkg/scheduler/apis/config/v1alpha1/conversion.go

Huang-Wei · 2020-02-14T22:31:34Z

pkg/scheduler/apis/config/validation/validation.go

+	allErrs := field.ErrorList{}
+	var canon *config.PluginSet
+	if profiles[0].Plugins != nil {
+		canon = profiles[0].Plugins.QueueSort


Are the following cases valid?

some profiles are missing queuesort plugin, however, queuesort plugin is at least specified in one profile

the profile(s) with queuesort plugin specified is(are) not at profiles[0]

Ideally, this validation logic shouldn't know about valid plugin names or what's the default plugin. That means it can't deduce if two configurations are equivalent. So, we decided that the queuesort plugin specification must be strictly equal. Then, the cases you describe are considered invalid.

alculquicondor · 2020-02-18T21:35:27Z

@liggitt Friendly reminder

cmd/kube-scheduler/app/options/deprecated.go

liggitt · 2020-02-18T21:45:28Z

cmd/kube-scheduler/app/options/options.go

@@ -239,7 +242,8 @@ func (o *Options) Config() (*schedulerappconfig.Config, error) {
 	}

 	coreBroadcaster := record.NewBroadcaster()
-	coreRecorder := coreBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: c.ComponentConfig.SchedulerName})
+	// TODO(#85737): Support more than one profile.


in the meantime, fail if more than one is specified so we don't leave master in a should-not-ship state?

Actually, since this recorder is used for leader election, we can only use one name. The first profile's name makes the most sense.

I added the error in server.go

that definitely needs to be documented in the user guide for this... that is not intuitive

cmd/kube-scheduler/app/options/deprecated.go

pkg/scheduler/apis/config/types.go

liggitt · 2020-02-18T22:23:09Z

pkg/scheduler/apis/config/v1alpha1/conversion.go

+	} else {
+		profile.Plugins = nil
+	}
+	profile.PluginConfig = *(*[]config.PluginConfig)(unsafe.Pointer(&in.PluginConfig))


Prefer allocating an array and calling Convert_v1alpha1_PluginConfig_To_config_PluginConfig. In general, leave the unsafe casting to the generated conversion code (which does generate-time checks that the types are actually equivalent)

liggitt · 2020-02-18T23:10:53Z

pkg/scheduler/apis/config/v1alpha2/defaults.go

+		profile := &obj.Profiles[i]
+		if profile.SchedulerName == nil {
+			// Validation ensures that only one profile uses DefaultSchedulerName.
+			profile.SchedulerName = pointer.StringPtr(api.DefaultSchedulerName)


would it make more sense to only apply a default name if there's a single profile, and leave it undefaulted (and make validation fail with a "required" error) if there's more than a single profile?

Yes, this gives a more comprehensible error :)
Done

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

alculquicondor · 2020-02-19T16:04:17Z

/hold for squash

alculquicondor · 2020-02-19T20:40:41Z

Anything else to add? Note that the implementation PR is already up for review #88285

alculquicondor · 2020-02-21T11:20:18Z

/priority important-soon

liggitt · 2020-02-21T15:14:04Z

/approve

API change lgtm. @ahg-g can lgtm.

k8s-ci-robot · 2020-02-21T15:14:50Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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~~ [alculquicondor,liggitt]
~~pkg/scheduler/apis/config/OWNERS~~ [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

ahg-g · 2020-02-24T00:34:38Z

@alculquicondor please squash

Signed-off-by: Aldo Culquicondor <acondor@google.com>

alculquicondor · 2020-02-24T14:46:38Z

Rebased and squashed

ahg-g · 2020-02-24T15:08:36Z

/lgtm

alculquicondor · 2020-02-24T16:00:33Z

/retest

Huang-Wei · 2020-02-24T17:47:33Z

/unhold

alculquicondor · 2020-02-24T18:01:17Z

/hold cancel

alculquicondor · 2020-02-24T21:58:59Z

/retest

k8s-ci-robot requested review from draveness and hex108 February 12, 2020 18:28

k8s-ci-robot added area/test sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 12, 2020

alculquicondor force-pushed the mutiprofiles-api branch 2 times, most recently from 7dba86c to 71a5c2f Compare February 12, 2020 22:01

k8s-ci-robot assigned ahg-g and damemi Feb 12, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 12, 2020

alculquicondor force-pushed the mutiprofiles-api branch from 71a5c2f to 0dccc0b Compare February 12, 2020 22:59

ahg-g reviewed Feb 13, 2020

View reviewed changes

alculquicondor force-pushed the mutiprofiles-api branch 2 times, most recently from d93ade2 to 60237ed Compare February 13, 2020 17:47

alculquicondor mentioned this pull request Feb 13, 2020

Run multiple Scheduling Profiles kubernetes/enhancements#1451

Closed

ahg-g reviewed Feb 13, 2020

View reviewed changes

pkg/scheduler/apis/config/types.go Show resolved Hide resolved

alculquicondor force-pushed the mutiprofiles-api branch from 60237ed to 8cf0d2f Compare February 14, 2020 15:32

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 14, 2020

ahg-g reviewed Feb 14, 2020

View reviewed changes

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

pkg/scheduler/apis/config/types.go Outdated Show resolved Hide resolved

Huang-Wei reviewed Feb 14, 2020

View reviewed changes

alculquicondor force-pushed the mutiprofiles-api branch from 838a475 to 2afc7ab Compare February 18, 2020 14:46

alculquicondor mentioned this pull request Feb 18, 2020

Add support for multiple scheduling profiles #88285

Merged

liggitt reviewed Feb 18, 2020

View reviewed changes

cmd/kube-scheduler/app/options/deprecated.go Show resolved Hide resolved

liggitt reviewed Feb 18, 2020

View reviewed changes

alculquicondor force-pushed the mutiprofiles-api branch from 2afc7ab to 22071ad Compare February 19, 2020 15:29

alculquicondor mentioned this pull request Feb 21, 2020

Add compatibility tests for framework plugin configuration #88191

Merged

k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 21, 2020

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

Add Profiles to kubescheduler.config.k8s.io/v1alpha2

9e71741

Signed-off-by: Aldo Culquicondor <acondor@google.com>

alculquicondor force-pushed the mutiprofiles-api branch from 22071ad to 9e71741 Compare February 24, 2020 14:46

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

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

k8s-ci-robot merged commit f6525db into kubernetes:master Feb 24, 2020

k8s-ci-robot added this to the v1.18 milestone Feb 24, 2020

alculquicondor mentioned this pull request Feb 25, 2020

Add Extenders to v1alpha2 Component Config #88529

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Schedulings Profiles to kubescheduler.config.k8s.io/v1alpha2 #88087

Add Schedulings Profiles to kubescheduler.config.k8s.io/v1alpha2 #88087

alculquicondor commented Feb 12, 2020 •

edited

Loading

fejta-bot commented Feb 12, 2020

alculquicondor commented Feb 12, 2020

alculquicondor commented Feb 12, 2020

ahg-g Feb 13, 2020

ahg-g Feb 13, 2020

alculquicondor Feb 13, 2020

ahg-g left a comment

Huang-Wei left a comment

Huang-Wei Feb 14, 2020

alculquicondor Feb 18, 2020

Huang-Wei Feb 14, 2020

alculquicondor Feb 18, 2020

alculquicondor commented Feb 18, 2020

liggitt Feb 18, 2020

alculquicondor Feb 19, 2020

liggitt Feb 21, 2020 •

edited

Loading

alculquicondor Feb 24, 2020

liggitt Feb 18, 2020

liggitt Feb 18, 2020

alculquicondor Feb 19, 2020

alculquicondor commented Feb 19, 2020

alculquicondor commented Feb 19, 2020

alculquicondor commented Feb 21, 2020

liggitt commented Feb 21, 2020

k8s-ci-robot commented Feb 21, 2020

ahg-g commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

ahg-g commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

Huang-Wei commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

		// SchedulerName is name of the scheduler, used to select which pods
		// will be processed by this profile, based on pod's "spec.SchedulerName".

Add Schedulings Profiles to kubescheduler.config.k8s.io/v1alpha2 #88087

Add Schedulings Profiles to kubescheduler.config.k8s.io/v1alpha2 #88087

Conversation

alculquicondor commented Feb 12, 2020 • edited Loading

fejta-bot commented Feb 12, 2020

alculquicondor commented Feb 12, 2020

alculquicondor commented Feb 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahg-g left a comment

Choose a reason for hiding this comment

Huang-Wei left a 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

alculquicondor commented Feb 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Feb 21, 2020 • edited Loading

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

alculquicondor commented Feb 19, 2020

alculquicondor commented Feb 19, 2020

alculquicondor commented Feb 21, 2020

liggitt commented Feb 21, 2020

k8s-ci-robot commented Feb 21, 2020

ahg-g commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

ahg-g commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

Huang-Wei commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

alculquicondor commented Feb 24, 2020

alculquicondor commented Feb 12, 2020 •

edited

Loading

liggitt Feb 21, 2020 •

edited

Loading