-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Update InPlacePodVerticalScaling docs for v1.33 beta #50290
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
Conversation
|
@tallclair: You must be a member of the kubernetes/website-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your Website milestone maintainers and have them propose you as an additional delegate for this responsibility. 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-sigs/prow repository. |
👷 Deploy Preview for kubernetes-io-vnext-staging processing.
|
|
/assign @natasha41575 |
✅ Pull request preview available for checking
To edit notification comments on pull requests, go to your Netlify site configuration. |
sftim
left a comment
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.
Thanks for the PR. I'm afraid we need a wider change, given the graduation to beta.
We must also update the Pod documentation at https://kubernetes.io/docs/concepts/workloads/pods/
For beta, and given that this will be enabled by default, I judge that revising the Pod concept docs is a requirement.
content/en/docs/tasks/configure-pod-container/resize-container-resources.md
Outdated
Show resolved
Hide resolved
content/en/docs/tasks/configure-pod-container/resize-container-resources.md
Show resolved
Hide resolved
content/en/docs/tasks/configure-pod-container/resize-container-resources.md
Outdated
Show resolved
Hide resolved
content/en/docs/tasks/configure-pod-container/resize-container-resources.md
Outdated
Show resolved
Hide resolved
|
|
||
| Below example shows a Pod whose Container's CPU can be resized without restart, but | ||
| resizing memory requires the container to be restarted. | ||
| You can control whether a container should be restarted when resizing by setting `resizePolicy` within the container specification. This allows fine-grained control based on resource type (CPU or memory). |
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.
What about extended resources (eg from device plugins)? The docs should make the story clear for those.
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.
This is covered under key-concepts (only CPU & Memory are mutable) and limitations. Do you have any suggestions of how to raise the visibility of this?
| * **Resource Removal:** Resource requests and limits cannot be entirely removed once set; they can only be changed to different values. | ||
| * **Operating System:** Windows pods do not support in-place resize. | ||
| * **Node Policies:** Pods managed by [static CPU or Memory manager policies](/docs/tasks/administer-cluster/cpu-management-policies/) cannot be resized in-place. | ||
| * **Swap:** Pods utilizing [swap memory](/docs/concepts/architecture/nodes/#swap-memory) cannot resize memory requests unless the `resizePolicy` for memory is `RestartContainer`. |
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.
Is this true?
| * **Swap:** Pods utilizing [swap memory](/docs/concepts/architecture/nodes/#swap-memory) cannot resize memory requests unless the `resizePolicy` for memory is `RestartContainer`. | |
| * **Swap:** Pods utilizing [swap memory](/docs/concepts/architecture/nodes/#swap-memory) can only resize memory requests if the `resizePolicy` for memory is `RestartContainer`. If the `resizePolicy` is set to something else, the API server accepts the resize request but it will not succeed. |
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.
Again, do you think it's necessary to describe the enforcement mechanism here? Here's the full list:
Enforced in API admission:
- Resource type
- QoS class
- Container types
- Windows pods (only if OS name is windows)
- Resource removal
Enforced at allocation time (resize marked Infeasible):
- Static CPU / memory
- Swap
- node Capacity (not listed here)
- Windows (anything that slipped through admission)
| ## Example 1: Resizing CPU without Restart | ||
|
|
||
| ## Create a pod with resource requests and limits | ||
| First, create a Pod designed for in-place CPU resize and restart-required memory resize. |
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.
What namespace is this Pod in? Do you need to create a namespace first?
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 was thinking they could just use the default namespace, since it's just a single resource being created. Is it standard practice to create a dedicated namespace for all tutorials?
|
|
||
| ```shell | ||
| kubectl get pod qos-demo-5 --output=yaml --namespace=qos-example | ||
| kubectl patch pod resize-demo --subresource resize --patch \ |
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.
What namespace is change happening in? Do you need to create a namespace first?
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.
(see above)
content/en/docs/tasks/configure-pod-container/resize-container-resources.md
Outdated
Show resolved
Hide resolved
| Delete the pod: | ||
|
|
||
| ```shell | ||
| kubectl delete namespace qos-example |
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.
Does the namespace need removing or not?
|
@tallclair if you want to bring in another contributor to work on some of these docs, that's fine; it doesn't have to all land on you. |
|
/milestone 1.33 |
|
Actually, we can hold this PR whilst we get the Pod concept changes prepared and then unhold all the related PRs once it's clear that the docs will be ready. @tallclair would you prefer that? |
|
/lgtm |
|
LGTM label has been added. Git tree hash: ed1d44ec9aaa194d56a02393fc0f8d4c022438c4
|
|
I decided to just add the auxilary pages to this PR. /assign @sftim |
|
Thanks for the review @vinaykul ! |
|
/assign @vinaykul |
vinaykul
left a comment
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.
/lgtm
|
LGTM label has been added. Git tree hash: a3ea774aa818bd5beeba4aba152c50030208bf54
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tengqm, vinaykul 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 |
|
@vinaykul Could you please give another |
|
/lgtm |
|
LGTM label has been added. Git tree hash: 43a8798ccc6541b76c15fe76ce55bb0156497766
|
Description
InPlacePodVerticalScaling is graduating to beta in v1.33. I rewrote the primary doc page to capture v1.33 changes and hopefully generally add clarity and detail.
Issue
Fixes #50022
/sig node
/priority important-soon
/milestone v1.33