Add check to skip PodTopologySpread PreFilter if no constraints are specified #117683

utam0k · 2023-04-29T09:28:53Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds a check in the PodTopologySpread PreFilter function to return a Skip status if there are no topology spread constraints specified in the pod spec and no default constraints. This prevents unnecessary processing and filtering for pods that don't have any topology spread constraints.

Which issue(s) this PR fixes:

This change is a part of the work for issue #114399.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

The scheduler skips the PodTopologySpread Filter plugin if no spread constraints.
It will affect some metrics values related to the PodTopologySpread Filter plugin.

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

NONE

k8s-ci-robot · 2023-04-29T09:29:01Z

Welcome @utam0k!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-04-29T09:29:01Z

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.

k8s-ci-robot · 2023-04-29T09:29:02Z

Hi @utam0k. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

utam0k · 2023-04-29T09:29:55Z

There may be other places to skip in another PR,
It would not be a bad idea to put out a PR of this size for once.

sanposhiho

/ok-to-test

pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go

sanposhiho · 2023-04-29T10:02:41Z

Please change the release note to something like:

The scheduler skips the PodTopologySpread Filter plugin if no spread constraints on the Pod.
It will affect some metrics values related to the PodTopologySpread Filter plugin.

utam0k · 2023-04-29T10:21:21Z

@sanposhiho Thanks for your review and support🙏 I have updated this PR following what you pointed out. PTAL 👀

sanposhiho

only several nits

pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go

utam0k · 2023-05-01T04:43:48Z

@sanposhiho I appreciate your kind review 🙏 I have fixed them. PTAL

utam0k · 2023-05-02T10:27:54Z

@Huang-Wei @sanposhiho I appreciate you pointing it out 🙏 I have made the necessary updates to the PR based on your comments.

utam0k · 2023-05-02T11:53:22Z

/retest-required

alculquicondor · 2023-05-02T14:56:20Z

cc @tosi3k

alculquicondor

/approve

pkg/scheduler/framework/plugins/podtopologyspread/filtering.go

Huang-Wei · 2023-05-02T21:22:45Z

/lgtm
Thanks @utam0k !

k8s-ci-robot · 2023-05-02T21:22:56Z

LGTM label has been added.

Git tree hash: 29c4c5389715fa9298fdd175f4e77d7d3366e500

Huang-Wei · 2023-05-02T23:02:01Z

/retest

…pecified This commit adds a check in the PodTopologySpread PreFilter function to return a Skip status if there are no topology spread constraints specified This prevents unnecessary processing and filtering for pods that don't have any topology spread constraints. This change is a part of the work for issue kubernetes#114399. Signed-off-by: utam0k <[email protected]>

utam0k · 2023-05-03T04:41:17Z

@Huang-Wei, I apologize for repeatedly requesting reviews. Could you please take a look whenever you have a moment? Thank you.

Huang-Wei · 2023-05-03T05:23:40Z

/lgtm

k8s-ci-robot · 2023-05-03T05:23:46Z

LGTM label has been added.

Git tree hash: eba210f9eeda753b44a403fdd3c2826c56418964

alculquicondor

/hold cancel

k8s-ci-robot · 2023-05-03T12:09:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, sanposhiho, utam0k

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:

~~pkg/scheduler/OWNERS~~ [alculquicondor]

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

…pecified Cherry-pick of kubernetes#117683 Backport request approved in b/286005251 Change-Id: Ieee7d7e6c50823b71d6e21f6cc2071f81bea7a32

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 29, 2023

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Apr 29, 2023

k8s-ci-robot requested review from denkensk and sanposhiho April 29, 2023 09:29

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 Apr 29, 2023

sanposhiho reviewed Apr 29, 2023

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 29, 2023

sanposhiho reviewed Apr 29, 2023

View reviewed changes

pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go Outdated Show resolved Hide resolved

utam0k force-pushed the skip-topologyspread-empty branch from b90ef80 to 1b4aa5f Compare April 29, 2023 10:18

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 29, 2023

utam0k requested a review from sanposhiho April 30, 2023 04:22

sanposhiho reviewed Apr 30, 2023

View reviewed changes

utam0k force-pushed the skip-topologyspread-empty branch from 1b4aa5f to 9840409 Compare May 1, 2023 04:36

utam0k requested a review from sanposhiho May 1, 2023 04:42

utam0k force-pushed the skip-topologyspread-empty branch from 1d86925 to 37c7cd5 Compare May 2, 2023 10:29

utam0k requested a review from Huang-Wei May 2, 2023 10:30

alculquicondor reviewed May 2, 2023

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 2, 2023

Huang-Wei reviewed May 2, 2023

View reviewed changes

pkg/scheduler/framework/plugins/podtopologyspread/filtering.go Show resolved Hide resolved

k8s-ci-robot assigned Huang-Wei May 2, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 2, 2023

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

utam0k force-pushed the skip-topologyspread-empty branch from 37c7cd5 to d82684e Compare May 3, 2023 04:39

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2023

k8s-ci-robot requested a review from Huang-Wei May 3, 2023 04:39

utam0k requested a review from alculquicondor May 3, 2023 04:40

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2023

alculquicondor approved these changes May 3, 2023

View reviewed changes

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

k8s-ci-robot merged commit 0d67dd6 into kubernetes:master May 3, 2023
12 checks passed

k8s-ci-robot added this to the v1.28 milestone May 3, 2023

jaskaransarkaria mentioned this pull request Jun 5, 2024

Planning upgrade to EKS 1.28 ministryofjustice/cloud-platform#5570

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add check to skip PodTopologySpread PreFilter if no constraints are specified #117683

Add check to skip PodTopologySpread PreFilter if no constraints are specified #117683

utam0k commented Apr 29, 2023 •

edited

k8s-ci-robot commented Apr 29, 2023

k8s-ci-robot commented Apr 29, 2023

k8s-ci-robot commented Apr 29, 2023

utam0k commented Apr 29, 2023

sanposhiho left a comment

sanposhiho commented Apr 29, 2023 •

edited

utam0k commented Apr 29, 2023

sanposhiho left a comment

utam0k commented May 1, 2023

utam0k commented May 2, 2023

utam0k commented May 2, 2023

alculquicondor commented May 2, 2023

alculquicondor left a comment

Huang-Wei commented May 2, 2023

k8s-ci-robot commented May 2, 2023

Huang-Wei commented May 2, 2023

utam0k commented May 3, 2023

Huang-Wei commented May 3, 2023

k8s-ci-robot commented May 3, 2023

alculquicondor left a comment

k8s-ci-robot commented May 3, 2023

Add check to skip PodTopologySpread PreFilter if no constraints are specified #117683

Add check to skip PodTopologySpread PreFilter if no constraints are specified #117683

Conversation

utam0k commented Apr 29, 2023 • 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 Apr 29, 2023

k8s-ci-robot commented Apr 29, 2023

k8s-ci-robot commented Apr 29, 2023

utam0k commented Apr 29, 2023

sanposhiho left a comment

Choose a reason for hiding this comment

sanposhiho commented Apr 29, 2023 • edited

utam0k commented Apr 29, 2023

sanposhiho left a comment

Choose a reason for hiding this comment

utam0k commented May 1, 2023

utam0k commented May 2, 2023

utam0k commented May 2, 2023

alculquicondor commented May 2, 2023

alculquicondor left a comment

Choose a reason for hiding this comment

Huang-Wei commented May 2, 2023

k8s-ci-robot commented May 2, 2023

Huang-Wei commented May 2, 2023

utam0k commented May 3, 2023

Huang-Wei commented May 3, 2023

k8s-ci-robot commented May 3, 2023

alculquicondor left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented May 3, 2023

utam0k commented Apr 29, 2023 •

edited

sanposhiho commented Apr 29, 2023 •

edited