Scheduling v1beta3 #104251

ravisantoshgudimetla · 2021-08-09T19:50:17Z

What type of PR is this?

/kind api-change

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Introduce v1beta3 api for scheduler. This version 
- increases the weight of user specifiable priorities.
The weights of following priority plugins are increased
  - TaintTolerations to 3 - as leveraging node tainting to group nodes in the cluster is becoming a widely-adopted practice
  - NodeAffinity to 2
  - InterPodAffinity to 2

- Won't have HealthzBindAddress, MetricsBindAddress fields

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

KEP: https://github.com/kubernetes/enhancements/pull/2850/

k8s-ci-robot · 2021-08-09T19:50:24Z

@ravisantoshgudimetla: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

ravisantoshgudimetla · 2021-08-09T19:50:28Z

/sig scheduling

ravisantoshgudimetla · 2021-08-09T21:28:42Z

pkg/scheduler/apis/config/v1beta3/default_plugins.go

+				{Name: names.PodTopologySpread, Weight: pointer.Int32Ptr(2)},
+				// Weight is tripled because:
+				// - This is a score coming from user preference.
+				// - Usage of node tainting to group nodes in the cluster is increasing becoming a use-case


note to myself: typo: increasingly becoming

k8s-triage-robot · 2021-08-10T19:46:21Z

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.

ravisantoshgudimetla · 2021-08-10T21:40:57Z

/retest

damemi

@ravisantoshgudimetla this needs a release note, but like we talked about offline it's all pretty straightforward to me. The actual changes (increasing the default plugin weights) are fairly small but this sets up v1beta3 nicely for us to get in final changes before promoting to v1.

/cc @ahg-g @alculquicondor
one question I have is do we want to make this beta3 or just go ahead with starting work on v1, along with Abdullah's suggestion for the simplified config (#102303). what's the anticipated release time for v1, and does adding this new beta version add more work/time for us to eventually deprecate?

Staging changes for scheduler v1beta3 api. xref: kubernetes/enhancements#2850

Changes in pkg/apis/scheduler reflecting the changes from staging. The weights of following priority plugins were increased: - TaintTolerations - NodeAffinity - InterPodAffinity as they're user facing priorities and they should be more influential when compared to default plugins which don't have any user say. xref: kubernetes/enhancements#2850

ravisantoshgudimetla · 2021-10-05T17:38:42Z

/test pull-kubernetes-unit

alculquicondor · 2021-10-05T17:59:57Z

Please don't include the fix to PreferNominatedNode in this PR. Create a separate one.

ravisantoshgudimetla · 2021-10-06T13:17:57Z

Please don't include the fix to PreferNominatedNode in this PR. Create a separate one.

Done - #105509

Instead of changing the node names, I'd prefer to move L1343~1357 into t.Run().

Also nodeConfig seems redundant as node can be easily composed using st.Node().Cap()....Obj()

I don't have a strong preference one way or the other. I included your suggestions here - #105509

PTAL @alculquicondor @Huang-Wei

ravisantoshgudimetla · 2021-10-06T20:15:35Z

This is ready for review again.

alculquicondor · 2021-10-06T21:13:58Z

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

 	badRemovedPlugins1.Profiles[0].Plugins.Score.Enabled = append(badRemovedPlugins1.Profiles[0].Plugins.Score.Enabled, config.Plugin{Name: "ServiceAffinity", Weight: 2})

 	badRemovedPlugins2 := validConfig.DeepCopy()
-	badRemovedPlugins2.APIVersion = "kubescheduler.config.k8s.io/v1beta3" // hypothetical, v1beta3 doesn't exist
+	badRemovedPlugins2.APIVersion = "kubescheduler.config.k8s.io/v1beta3" // hypothetical, v1beta3


it's not hypothetical anymore.

alculquicondor · 2021-10-07T13:12:39Z

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

 	badRemovedPlugins1.Profiles[0].Plugins.Score.Enabled = append(badRemovedPlugins1.Profiles[0].Plugins.Score.Enabled, config.Plugin{Name: "ServiceAffinity", Weight: 2})

 	badRemovedPlugins2 := validConfig.DeepCopy()
-	badRemovedPlugins2.APIVersion = "kubescheduler.config.k8s.io/v1beta3" // hypothetical, v1beta3 doesn't exist
+	badRemovedPlugins2.APIVersion = "kubescheduler.config.k8s.io/v1beta3" //  v1beta3


No point having this test anymore.

I think we still need it, considering the validConfig points to v1beta2 here

yes, but you already have a test suite for v1beta3 below.

It's just not about having the same test. It is also about having the apiVersion to validation testing mapping. Say if we forget to copy some logic or files specific to validation, we can surface those errors in appropriate versions.

alculquicondor · 2021-10-07T13:13:15Z

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

+	badRemovedPlugins1.Profiles[0].Plugins.Score.Enabled = append(badRemovedPlugins1.Profiles[0].Plugins.Score.Enabled, config.Plugin{Name: "ServiceAffinity", Weight: 2})
+
+	badRemovedPlugins2 := validConfig.DeepCopy()
+	badRemovedPlugins2.APIVersion = "kubescheduler.config.k8s.io/v1beta3" //  v1beta3


Also no point for this one.

True, it may not add much value, considering v1beta3 is the api we're testing now. It was a copy-paste error. Will remove it.

Vendor changes

alculquicondor

/lgtm

ravisantoshgudimetla · 2021-10-07T16:28:22Z

/hold cancel

based on #104251 (review)

/retest

All the upgrade candidate nodes in a mcp would be applied `UpdateInProgress: PreferNoSchedule` taint. The taint will be removed MCD once the upgrade is complete. Since kubernetes/kubernetes#104251 landed, the nodes not having PreferNoSchedule taint will have higher score. Before the upgrade starts, MCC will taint all the nodes in the cluster that are supposed to be upgraded. Once the upgrade is complete since MCD will remove the taint, none of the nodes will have `UpdateInProgress: PreferNoSchedule` taint. This ensures the score of the nodes will be equal again. Why is this needed? This reduces the pod churn when the cluster upgrade is in progress. When the non-upgraded nodes in the cluster have `UpdateInProgress: PreferNoSchedule` taint, they would get lesser score and the pods would prefer to land onto untainted(upgraded) nodes there by reducing the chances of landing onto an unupgraded node which can cause one more reschedule

k8s-ci-robot added 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 Aug 9, 2021

k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 9, 2021

ravisantoshgudimetla changed the title ~~Scheduling v1beta3~~ [wip] Scheduling v1beta3 Aug 9, 2021

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2021

k8s-ci-robot requested review from adtac and sttts August 9, 2021 19:50

ravisantoshgudimetla force-pushed the scheduling-v1beta3 branch from 32ba5a1 to 0213510 Compare August 9, 2021 20:55

ravisantoshgudimetla commented Aug 9, 2021

View reviewed changes

ravisantoshgudimetla force-pushed the scheduling-v1beta3 branch from 0213510 to f4fb45c Compare August 10, 2021 18:14

ravisantoshgudimetla changed the title ~~[wip] Scheduling v1beta3~~ Scheduling v1beta3 Aug 10, 2021

k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 10, 2021

ravisantoshgudimetla force-pushed the scheduling-v1beta3 branch from f4fb45c to 27fdf6a Compare August 10, 2021 20:24

damemi reviewed Aug 11, 2021

View reviewed changes

k8s-ci-robot requested review from ahg-g and alculquicondor August 11, 2021 17:05

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 16, 2021

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2021

ravisantoshgudimetla added 2 commits October 4, 2021 13:37

[cc] [staging]:Introduce scheduler v1beta3 api

2743acf

Staging changes for scheduler v1beta3 api. xref: kubernetes/enhancements#2850

ravisantoshgudimetla force-pushed the scheduling-v1beta3 branch from bb84767 to d988387 Compare October 5, 2021 16:38

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 5, 2021

ravisantoshgudimetla force-pushed the scheduling-v1beta3 branch from d988387 to 3225d78 Compare October 6, 2021 13:16

alculquicondor reviewed Oct 6, 2021

View reviewed changes

ravisantoshgudimetla force-pushed the scheduling-v1beta3 branch 2 times, most recently from 78f680b to 079a7eb Compare October 6, 2021 21:50

alculquicondor reviewed Oct 7, 2021

View reviewed changes

ravisantoshgudimetla added 3 commits October 7, 2021 10:58

Make v1beta3 default

5c7f602

[scheduler][vendor]: add v1beta3 explicitly

1a484c1

Vendor changes

[test]: modify integration tests to use v1beta3

283b176

ravisantoshgudimetla force-pushed the scheduling-v1beta3 branch from 079a7eb to 283b176 Compare October 7, 2021 14:58

alculquicondor reviewed Oct 7, 2021

View reviewed changes

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

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 7, 2021

k8s-ci-robot merged commit 9b45983 into kubernetes:master Oct 7, 2021

k8s-ci-robot added this to the v1.23 milestone Oct 7, 2021

ravisantoshgudimetla deleted the scheduling-v1beta3 branch October 7, 2021 18:04

alculquicondor mentioned this pull request Oct 12, 2021

Migration guide between KubeSchedulerConfig versions kubernetes/website#30063

Closed

ravisantoshgudimetla mentioned this pull request Dec 2, 2021

[MCC][MCD]: Introduce in progress taint openshift/machine-config-operator#2686

Merged

rfranzke mentioned this pull request Jan 12, 2022

☂️-Issue for "Support for Kubernetes v1.23” gardener/gardener#5102

Closed

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scheduling v1beta3 #104251

Scheduling v1beta3 #104251

ravisantoshgudimetla commented Aug 9, 2021 •

edited

k8s-ci-robot commented Aug 9, 2021

ravisantoshgudimetla commented Aug 9, 2021

ravisantoshgudimetla Aug 9, 2021

k8s-triage-robot commented Aug 10, 2021

ravisantoshgudimetla commented Aug 10, 2021

damemi left a comment

ravisantoshgudimetla commented Oct 5, 2021

alculquicondor commented Oct 5, 2021

ravisantoshgudimetla commented Oct 6, 2021

ravisantoshgudimetla commented Oct 6, 2021

alculquicondor Oct 6, 2021

alculquicondor Oct 7, 2021

ravisantoshgudimetla Oct 7, 2021

alculquicondor Oct 7, 2021

ravisantoshgudimetla Oct 7, 2021

alculquicondor Oct 7, 2021

ravisantoshgudimetla Oct 7, 2021

ravisantoshgudimetla Oct 7, 2021

alculquicondor left a comment

ravisantoshgudimetla commented Oct 7, 2021

Scheduling v1beta3 #104251

Scheduling v1beta3 #104251

Conversation

ravisantoshgudimetla commented Aug 9, 2021 • 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.:

k8s-ci-robot commented Aug 9, 2021

ravisantoshgudimetla commented Aug 9, 2021

Choose a reason for hiding this comment

k8s-triage-robot commented Aug 10, 2021

ravisantoshgudimetla commented Aug 10, 2021

damemi left a comment

Choose a reason for hiding this comment

ravisantoshgudimetla commented Oct 5, 2021

alculquicondor commented Oct 5, 2021

ravisantoshgudimetla commented Oct 6, 2021

ravisantoshgudimetla commented Oct 6, 2021

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

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

ravisantoshgudimetla commented Oct 7, 2021

ravisantoshgudimetla commented Aug 9, 2021 •

edited