Scheduler: graduate DefaultPodTopologySpread to GA #107421

gavinfish · 2022-01-08T16:08:45Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

graduate DefaultPodTopologySpread to GA

Which issue(s) this PR fixes:

REF: kubernetes/enhancements#1258

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Feature of `DefaultPodTopologySpread` is graduated to GA

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

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-scheduling/1258-default-pod-topology-spread

k8s-ci-robot · 2022-01-08T16:08:53Z

@gavinfish: 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.

gavinfish · 2022-01-09T05:37:47Z

/assign @alculquicondor

k8s-triage-robot · 2022-01-09T06:31:11Z

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.

kerthcet · 2022-01-10T06:16:28Z

pkg/scheduler/apis/config/v1beta2/default_plugins_test.go

@@ -114,90 +113,6 @@ func TestApplyFeatureGates(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "DefaultPodTopologySpread disabled",


keep this test but change to another plugin, like JobTrackingWithFinalizers?

That's not a plugin. That's a feature for the job controller.

But there should be a test for VolumeCapacityPriority

I can create a separate PR to add more test for VolumeCapacityPriority.

sorry for wrong words, but shouldn't we change to a beta feature, and verify whether disable this feature-gate works? VolumeCapacityPriority is alpha now and disabled by default.

unit tests are independent of whether the feature is alpha or beta. The test itself sets the value of the gate, regardless of the graduation status.

kerthcet · 2022-01-10T06:44:19Z

pkg/features/kube_features.go

@@ -904,7 +904,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	HugePageStorageMediumSize:                      {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.24
 	DownwardAPIHugePages:                           {Default: true, PreRelease: featuregate.Beta},                    // on by default in 1.22
 	AnyVolumeDataSource:                            {Default: false, PreRelease: featuregate.Alpha},
-	DefaultPodTopologySpread:                       {Default: true, PreRelease: featuregate.Beta},


Since PodTopologySpread graduated to GA, should we remove SelectorSpread then? @alculquicondor

We can't, because it's a supported plugin in v1beta3. We can potentially remove it once we release a v1. Maybe we will do that in 1.25.

The feature gate itself should be removed two releases after graduation. That's 1.26.

Or we can add some related comments. Forget it if not necessary.

alculquicondor · 2022-01-10T15:38:38Z

/hold
for kubernetes/enhancements#3117

gavinfish · 2022-01-20T14:34:10Z

@alculquicondor PTAL

Left work for GA is updating documentation, right?

alculquicondor · 2022-01-20T16:45:43Z

/hold cancel

alculquicondor · 2022-01-20T16:48:36Z

cmd/kube-scheduler/app/server_test.go

@@ -221,7 +220,7 @@ leaderElection:
 			name: "default config with an alpha feature enabled and an beta feature disabled",
 			flags: []string{
 				"--kubeconfig", configKubeconfig,
-				"--feature-gates=VolumeCapacityPriority=true,DefaultPodTopologySpread=false",
+				"--feature-gates=VolumeCapacityPriority=true,JobMutableNodeSchedulingDirectives=false",


This is not a feature that affects kube-scheduler. Just get rid of the "and a beta feature disabled" part of the test. In this test, we should only care about features that affect the config.

alculquicondor · 2022-01-20T16:50:02Z

pkg/features/kube_features.go

@@ -904,7 +904,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	HugePageStorageMediumSize:                      {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.24
 	DownwardAPIHugePages:                           {Default: true, PreRelease: featuregate.Beta},                    // on by default in 1.22
 	AnyVolumeDataSource:                            {Default: false, PreRelease: featuregate.Alpha},
-	DefaultPodTopologySpread:                       {Default: true, PreRelease: featuregate.Beta},


The feature gate itself should be removed two releases after graduation. That's 1.26.

alculquicondor

Update the comments for:

kubernetes/pkg/scheduler/apis/config/types_pluginargs.go

Line 108 in e7e58de

// Defaults to "List" if feature gate DefaultPodTopologySpread is disabled
kubernetes/staging/src/k8s.io/kube-scheduler/config/v1beta3/types_pluginargs.go

Line 113 in e7e58de

// Defaults to "List" if feature gate DefaultPodTopologySpread is disabled
kubernetes/staging/src/k8s.io/kube-scheduler/config/v1beta2/types_pluginargs.go

Line 113 in e7e58de

// Defaults to "List" if feature gate DefaultPodTopologySpread is disabled

alculquicondor

/lgtm
/approve
/label api-review

k8s-ci-robot · 2022-01-24T18:21:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: alculquicondor, gavinfish
To complete the pull request process, please assign lavalamp after the PR has been reviewed.
You can assign the PR to them by writing /assign @lavalamp in a comment when ready.

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

Needs approval from an approver in each of these files:

~~cmd/kube-scheduler/OWNERS~~ [alculquicondor]
pkg/features/OWNERS
~~pkg/scheduler/apis/config/OWNERS~~ [alculquicondor]
staging/src/k8s.io/kube-scheduler/config/OWNERS

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

gavinfish · 2022-01-25T14:56:16Z

/assign @lavalamp

k8s-ci-robot · 2022-02-12T06:51:09Z

@gavinfish: PR needs rebase.

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.

kerthcet · 2022-02-16T07:30:01Z

ping @gavinfish please rebase this pr.

kerthcet · 2022-02-22T02:34:40Z

Is it possible by help rebasing this pr, I'll cherry pick @gavinfish 's commit as he does the most of the work. @alculquicondor

alculquicondor · 2022-02-22T15:40:30Z

Please do, it's been several days since @gavinfish last answered.

kerthcet · 2022-02-22T16:53:26Z

Create #108278 and cherry pick the commit already. Feel free to close this one.

alculquicondor · 2022-02-22T17:05:12Z

/close

k8s-ci-robot · 2022-02-22T17:05:33Z

@alculquicondor: Closed this PR.

In response to this:

/close

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.

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jan 8, 2022

k8s-ci-robot requested review from brendandburns and caesarxuchao January 8, 2022 16:09

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 Jan 8, 2022

gavinfish force-pushed the graduate-defaultpodtopologyspread branch from 27c5535 to f651af9 Compare January 9, 2022 04:10

gavinfish changed the title ~~[WIP] Scheduler: graduate DefaultPodTopologySpread to GA~~ Scheduler: graduate DefaultPodTopologySpread to GA Jan 9, 2022

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

k8s-ci-robot assigned alculquicondor Jan 9, 2022

kerthcet reviewed Jan 10, 2022

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 10, 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 Jan 20, 2022

alculquicondor reviewed Jan 20, 2022

View reviewed changes

gavinfish force-pushed the graduate-defaultpodtopologyspread branch from f651af9 to 451c03f Compare January 21, 2022 14:28

alculquicondor reviewed Jan 21, 2022

View reviewed changes

Scheduler: graduate DefaultPodTopologySpread to GA

fb96a68

gavinfish force-pushed the graduate-defaultpodtopologyspread branch from 451c03f to fb96a68 Compare January 23, 2022 03:16

alculquicondor reviewed Jan 24, 2022

View reviewed changes

k8s-ci-robot added api-review Categorizes an issue or PR as actively needing an API review. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jan 24, 2022

k8s-ci-robot assigned lavalamp Jan 25, 2022

liggitt added this to Assigned in API Reviews Jan 31, 2022

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

kerthcet mentioned this pull request Feb 18, 2022

Take taints/tolerations into consideration when calculating PodTopologySpread skew kubernetes/enhancements#3094

Open

8 tasks

kerthcet mentioned this pull request Feb 22, 2022

graduate default pod topology spread to ga #108278

Merged

k8s-ci-robot closed this Feb 22, 2022

liggitt removed this from Assigned in API Reviews Mar 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scheduler: graduate DefaultPodTopologySpread to GA #107421

Scheduler: graduate DefaultPodTopologySpread to GA #107421

gavinfish commented Jan 8, 2022

k8s-ci-robot commented Jan 8, 2022

gavinfish commented Jan 9, 2022

k8s-triage-robot commented Jan 9, 2022

kerthcet Jan 10, 2022

alculquicondor Jan 10, 2022

gavinfish Jan 10, 2022

kerthcet Jan 11, 2022

alculquicondor Jan 11, 2022

kerthcet Jan 10, 2022

alculquicondor Jan 20, 2022

alculquicondor Jan 20, 2022

kerthcet Jan 21, 2022

alculquicondor commented Jan 10, 2022

gavinfish commented Jan 20, 2022

alculquicondor commented Jan 20, 2022

alculquicondor Jan 20, 2022

gavinfish Jan 21, 2022

alculquicondor Jan 20, 2022

alculquicondor left a comment

alculquicondor left a comment

k8s-ci-robot commented Jan 24, 2022

gavinfish commented Jan 25, 2022

k8s-ci-robot commented Feb 12, 2022

kerthcet commented Feb 16, 2022

kerthcet commented Feb 22, 2022

alculquicondor commented Feb 22, 2022

kerthcet commented Feb 22, 2022

alculquicondor commented Feb 22, 2022

k8s-ci-robot commented Feb 22, 2022

Scheduler: graduate DefaultPodTopologySpread to GA #107421

Scheduler: graduate DefaultPodTopologySpread to GA #107421

Conversation

gavinfish commented Jan 8, 2022

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 Jan 8, 2022

gavinfish commented Jan 9, 2022

k8s-triage-robot commented Jan 9, 2022

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

Choose a reason for hiding this comment

alculquicondor commented Jan 10, 2022

gavinfish commented Jan 20, 2022

alculquicondor commented Jan 20, 2022

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

alculquicondor left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 24, 2022

gavinfish commented Jan 25, 2022

k8s-ci-robot commented Feb 12, 2022

kerthcet commented Feb 16, 2022

kerthcet commented Feb 22, 2022

alculquicondor commented Feb 22, 2022

kerthcet commented Feb 22, 2022

alculquicondor commented Feb 22, 2022

k8s-ci-robot commented Feb 22, 2022