Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add a doc for QueueingHint #43813

Merged
merged 4 commits into from
Nov 22, 2023
Merged

add a doc for QueueingHint #43813

merged 4 commits into from
Nov 22, 2023

Conversation

sanposhiho
Copy link
Member

ref: https://github.com/kubernetes/enhancements/tree/master/keps/sig-scheduling/4247-queueinghint

It's not a mistake that I try to push this to main branch, not to v1.29 branch.
This KEP is under a unique situation where we assume this feature has been existing as beta since v1.28.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 4, 2023
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 4, 2023
@sanposhiho
Copy link
Member Author

/cc @alculquicondor

@netlify Netlify
Copy link

netlify bot commented Nov 4, 2023 •
edited
Loading

✅ Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit de6a641
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/655d414d5eb73600085e164b
😎 Deploy Preview https://deploy-preview-43813--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dipesh-rawat
Copy link
Member

/sig scheduling

@k8s-ci-robot k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Nov 4, 2023
@sanposhiho
Copy link
Member Author

@tengqm Thanks, fixed based on your reviews.

sftim
sftim reviewed Nov 9, 2023
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for docs.

Ready for tech review (by / on behalf of SIG Scheduling)

alculquicondor
alculquicondor reviewed Nov 13, 2023
View reviewed changes
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated
@@ -222,6 +222,26 @@ This is an informational extension point. Post-bind plugins are called after a
Pod is successfully bound. This is the end of a binding cycle, and can be used
to clean up associated resources.

### EnqueueExtension
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go after PreEnqueue.

I wonder if we should rename ## Extension points to ## Interfaces, so that it's not confused with https://kubernetes.io/docs/reference/scheduling/config/#extension-points, which actually form part of the API.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I rename this along with some wording changes in the doc.

content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated Show resolved Hide resolved
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated Show resolved Hide resolved
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated Show resolved Hide resolved
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated Show resolved Hide resolved
@k8s-ci-robot 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 Nov 18, 2023
@sanposhiho
Copy link
Member Author

@alculquicondor Thanks, modified the PR based on your reviews.

alculquicondor
alculquicondor reviewed Nov 21, 2023
View reviewed changes
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated Show resolved Hide resolved
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated Show resolved Hide resolved
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated Show resolved Hide resolved
content/en/docs/concepts/scheduling-eviction/scheduling-framework.md Outdated
so that the scheduler will retry the scheduling of the Pod.

{{< note >}}
QueueingHint is a beta-level field and is enabled by default in 1.28.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
QueueingHint is a beta-level field and is enabled by default in 1.28.
QueueingHint is a beta-level feature and is enabled by default in 1.28.

(it's not an API field)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite right either. Try something like “Queuing hint evaluation during scheduling is a beta-level feature…”, or “the QueueingHint scheduler frobnication is a beta feature…” (change frobnication to a suitable term).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I go with:
QueueingHint evaluation during scheduling is a beta-level feature ..

@sanposhiho sanposhiho mentioned this pull request Nov 22, 2023
Closed
@alculquicondor
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 22, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 587e9b976d7b755c73489cbd120869c03674eeb7

@divya-mohan0209
Copy link
Contributor

Taking Tim's comment as approval for merge.
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: divya-mohan0209

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 22, 2023
@k8s-ci-robot k8s-ci-robot merged commit dad8686 into kubernetes:main Nov 22, 2023
6 checks passed
@windsonsea windsonsea mentioned this pull request Nov 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sftim sftim sftim left review comments

@tengqm tengqm tengqm left review comments

@alculquicondor alculquicondor alculquicondor left review comments

@ahg-g ahg-g Awaiting requested review from ahg-g

@divya-mohan0209 divya-mohan0209 Awaiting requested review from divya-mohan0209

Assignees

@alculquicondor alculquicondor

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
SIG Scheduling
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sanposhiho @dipesh-rawat @alculquicondor @k8s-ci-robot @divya-mohan0209 @tengqm @sftim