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
Topology Manager docs update for 1.19 #21607
Topology Manager docs update for 1.19 #21607
Conversation
Deploy preview for kubernetes-io-vnext-staging processing. Building with commit 7b270b4 https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5f0c75caa7f90c0007719e0a |
/milestone 1.19 |
b21a40b
to
2af48ec
Compare
Hi @klueska, docs shadow here 👋 just a friendly reminder this PR needs tech and doc review by July 16th. |
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.
One small nit. LGTM otherwise.
content/en/docs/concepts/extend-kubernetes/compute-storage-net/device-plugins.md
Outdated
Show resolved
Hide resolved
2af48ec
to
6fbd89d
Compare
/lgtm |
Hi @klueska, you can reach out to docs team at #sig-docs channel to request a review, please let me know if you need any help |
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.
Hi @klueska
Here's some feedback.
- I recommend using a note shortcode (see inline suggestion)
- Does https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/ require any changes as part of this PR?
**Note:** Plugins are not required to provide useful implementations for | ||
`GetPreferredAllocation()` or `PreStartContainer()`. Flags indicating which | ||
(if any) of these calls are available should be set in the `DevicePluginOptions` | ||
message sent back by a call to `GetDevicePluginOptions()`. The `kubelet` will | ||
always call `GetDevicePluginOptions()` to see which optional functions are | ||
available, before calling any of them directly. |
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.
**Note:** Plugins are not required to provide useful implementations for | |
`GetPreferredAllocation()` or `PreStartContainer()`. Flags indicating which | |
(if any) of these calls are available should be set in the `DevicePluginOptions` | |
message sent back by a call to `GetDevicePluginOptions()`. The `kubelet` will | |
always call `GetDevicePluginOptions()` to see which optional functions are | |
available, before calling any of them directly. | |
{{% note %}} | |
Plugins are not required to provide useful implementations for | |
`GetPreferredAllocation()` or `PreStartContainer()`. Flags indicating which | |
(if any) of these calls are available should be set in the `DevicePluginOptions` | |
message sent back by a call to `GetDevicePluginOptions()`. The kubelet | |
always calls `GetDevicePluginOptions()` to see which optional functions are | |
available, before calling any of them directly. | |
{{% /note %}} |
Use a Note shortcode (untested).
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 per your suggestion with 1 minor difference. I used
{{< note >}}
not{{% note %}}
as the link you provided suggested. - No, this actually moved to beta in 1.18, but the documentation was never updated to reflect that. The feature gate was toggled to "always-on" back before 1.18 was released.
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.
On detail 1, I did also file #22488 about the advice being not quite right for callout shortcodes. Anyway, it renders well enough as is.
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.
Signed-off-by: Kevin Klues <kklues@nvidia.com>
6fbd89d
to
7b270b4
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.
Looks good, thanks for updating the docs.
/lgtm
Hello, @kubernetes/sig-docs-en-owners I am looking for an approval on this PR. It's already got a LGTM from a technical perspective. |
/approve |
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: irvifa, savitharaghunathan 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 |
Signed-off-by: Kevin Klues kklues@nvidia.com