-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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.
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
Recommend runtimeclasses for scheduling #29062
Recommend runtimeclasses for scheduling #29062
Conversation
In future, we will enforce runtimeclasses to be used. Pods having just nodeSelector and tolerations are going to be rejected during apiserver admission. This commit ensures we recommend using runtimeclasses in the current release so that the transition will be smooth for end-users when we make it mandatory in future releases
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Please note that this is the recommended way to schedule pods onto Windows nodes instead of using nodeSelector and | ||
tolerations as they can be added to pod spec by any user. In future, we will enforce runtimeclasses to be used. Pods | ||
having just nodeSelector and tolerations are going to be rejected during apiserver admission. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest calling this out earlier under Taints and Tolerations
. Otherwise the reader is going think this might come in only if they are using multiple Windows versions in the same cluster. IOW, move RuntimeClasses
to the same level as Taints and Tolerations
and reword appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree to move RuntimeClasses
to be a peer of Taints and Tolerations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the "In future, ..." statement. We don't predict/document future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's the style guide on future statements.
Maybe the heading should change? RuntimeClass stops being a simplification and is instead the primary, recommended way to define the OS-level scheduling constraints for Pods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think should say anything about In future, we will enforce runtimeclasses to be used. Pods having just nodeSelector and tolerations are going to be rejected during apiserver admission. In this PR.
We should get approval of kubernetes/enhancements#2803 from all participating SIGs before saying anything definitive.
That's a fair argument. I'll remove that statement.
Also - because of this maybe we shouldn't say using runtime classes are recommended over nodeSelector generally.
If we can provide specific reasons why it is better (that we can commit to) great.
If not should we just document it as an alternative right now?
I think the argument is other way around, we know that the runtimeclasses are better, so we are proposing to make them authoritative in the linked enhancement.
I'll go with @sftim's suggestion that using runtimeclasses is recommended and better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I more meant that we shouldn't just say they are better but we should list some reasons why they are better.
Right now nodeSelectors are ubiquitous for identifying Windows pods I can see a lot of people asking why they should switch.
Since the KEP isn't approved and we cannot say nodeSelectors won't work in the future can we give some examples of why people should use them today?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried doing that here. I also mentioned that runtimeclasses are created by cluster admin. That is the reason for the switch. Do you want me to add more? Something from historical context that runtimeclasses didn't exist back then...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you also need to be a cluster admin to register a Windows node / configure automation to allow admission, and this is the production Windows setup guide, I think it's not only OK to recommend using a RuntimeClass for scheduling, I strongly think we should.
✔️ Deploy Preview for kubernetes-io-main-staging ready! 🔨 Explore the source changes: ecafc37 🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/60faf8066a259f00070e6f24 😎 Browse the preview: https://deploy-preview-29062--kubernetes-io-main-staging.netlify.app/docs/setup/production-environment/windows/user-guide-windows-containers |
The example at https://kubernetes.io/docs/setup/production-environment/windows/user-guide-windows-containers/#simplifying-with-runtimeclass uses the docker runtime class handler. Can we add an example for containerd that uses the default runtime class handler |
How are we going to handle something like the getting started section where node selectors are used: https://kubernetes.io/docs/setup/production-environment/windows/user-guide-windows-containers/#getting-started-deploying-a-windows-container? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to make sure we address my query about deprecation of nodeSelector
.
/hold
to highlight that
|
||
```yaml | ||
apiVersion: node.k8s.io/v1 | ||
kind: RuntimeClass | ||
metadata: | ||
name: windows-2019 | ||
handler: 'docker' | ||
handler: 'runhcs-wcow-process' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use tabs to show handles for docker and the containerd default handler similar to OS tabs at https://kubernetes.io/docs/setup/production-environment/container-runtimes/#cri-o?
A lot of windows users are still running with docker (unfortunately) and this can be a pretty big change for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need some help from @sftim or someone from familiar with that <tab>
tag. I am not sure if I am having correct tags.
I feel like we should address this at the same time? If it's windows and someone is just getting started, it would be good to set them on the RuntimeClass path in the getting started guide |
nodeSelector: | ||
kubernetes.io/os: windows | ||
``` | ||
Create a file named `win-webserver.yaml` with the contents from [here](#Use RuntimeClass(Recommended approach)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a file named `win-webserver.yaml` with the contents from [here](#Use RuntimeClass(Recommended approach)) | |
Create a file named `win-webserver.yaml` with the contents from [using RuntimeClasses](#Use RuntimeClass(Recommended approach)) |
I think it is a better to use descriptions over here
for screen readers
|
||
### Ensuring OS-specific workloads land on the appropriate container host | ||
For example: `--register-with-taints='os=windows:NoSchedule'` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For example: `--register-with-taints='os=windows:NoSchedule'` | |
For example adding the flag `--register-with-taints='os=windows:NoSchedule'` to kubelet parameters cuases kubelet to register the taint. |
326664c
to
bbb4681
Compare
bbb4681
to
ecafc37
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsturtevant - I addressed your comments
|
||
```yaml | ||
apiVersion: node.k8s.io/v1 | ||
kind: RuntimeClass | ||
metadata: | ||
name: windows-2019 | ||
handler: 'docker' | ||
handler: 'runhcs-wcow-process' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need some help from @sftim or someone from familiar with that <tab>
tag. I am not sure if I am having correct tags.
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
@ravisantoshgudimetla: 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. |
Hi @ravisantoshgudimetla, this PR has had no action and the bot labelled this PR as |
@reylejano: Closed this PR. In response to this:
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. |
In future, we will enforce runtimeclasses to be used. Pods having just nodeSelector
and tolerations are going to be rejected during apiserver admission. This commit
ensures we recommend using runtimeclasses in the current release so that the
transition will be smooth for end-users when we make it mandatory in future releases