Conversation
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
bfe252a to
9b0f634
Compare
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
9b0f634 to
8ddcad7
Compare
| You may also be able to mutate the incoming Pod, at admission time, to unset | ||
| the `.spec.nodeName` field and to use a node selector instead. | ||
|
|
||
| ## DRA beta features {#beta-features} |
There was a problem hiding this comment.
I think this separation into "GA/beta/alpha" features is not useful and leads to unnecessary churn when features graduate from alpha to beta. It's also misleading because not all beta features are on by default, or on-by-default features could be turned off. "Optional DRA features" looks like a better description.
We still need to move features out of this section when they graduation to GA, though. So perhaps we should instead use "Additional DRA features", which then can include GA features?
/cc @ritazh
There was a problem hiding this comment.
Thanks for the suggestion @pohly.
- I personally +1 and appreciated the Alpha/Beta sections because for a team/user that can only use GA things, that section makes it easy to skip. And for a team/user who is an early adopter, it's easier to find and pay more attention to those to ensure these alpha/beta features work in their environment before the features are GAed.
- A generic "Optional and Additional DRA features" section is hard to gate and scale in the future. e.g. which features are considered optional and additional? who makes that decision?
- I'm also +1 if we want each feature to just have their own section as long as their graduation status is right below it, it's easier to understand how to use it and easier for the feature owners to go back and update.
Thoughts?
There was a problem hiding this comment.
If we go with "additional", we can add an introduction like this:
Additional features add advanced functionality to core DRA; usage of them is optional and/or may only be relevant with certain DRA drivers.
Some of the features are in the Alpha or Beta
feature stage.
...
I still find that better than categorizing them by their state. An explicit "this is an alpha feature" in the section is clearer than having to remember how far down the page one has scrolled.
There was a problem hiding this comment.
Are you suggesting something like the following?
...
## Additional DRA features
Additional features add advanced functionality to core DRA; usage of them is optional and/or may only be relevant with certain DRA drivers. Some of the features are in the Alpha or Beta feature stage.
### Feature one (GA)
### Feature two (alpha)
### Feature three (beta)
...
There was a problem hiding this comment.
How about a tabular form?
DRA Features Status
| Feature | Status | Kubernetes Version | FG Default | Notes |
|---|---|---|---|---|
| Structured Parameters | Alpha | v1.30+ | Off | Moves parameter logic from external drivers into the scheduler. |
| Device Taints | Beta | v1.36+ | On | Allows nodes with specific devices to be tainted dynamically. |
There was a problem hiding this comment.
Looking at current content makes it clear that "Beta Features" and "Alpha Features" are not useful sub-sections because not all alpha/beta features are described there. For example, prioritized list is described further up in the section about requesting devices. I think we should follow that pattern and describe features where it makes sense, not based on their status.
There was a problem hiding this comment.
Feature one (GA)
That can work, as long as we explicitly set the anchor to not include the GA part.
We don't need to include the (GA/Alpha/Beta) part because it gets rendered for us automatically:
https://kubernetes.io/docs/concepts/scheduling-eviction/dynamic-resource-allocation/#admin-access
I don't think we need to be even more explicit.
There was a problem hiding this comment.
I switched to "Additional features" and did a pass over how the other features are described. Linking to feature gates was inconsistent or even broken. I'm now linking to the specific feature gate anchor.
There was a problem hiding this comment.
I agree with the change. As features graduate to stable, we should try to find a way to include information about it into the regular flow of the doc, so we don't end up with a long list of "additional features" separate from the overall structure.
There was a problem hiding this comment.
If there is a better place than "Additional features", then a new feature should be described there immediately. We did that for prioritized list.
"Additional features" then are the things whose usage is less common (the "advanced use cases").
9bf6a45 to
ebf3053
Compare
|
@pohly I’m +1 on this format. It removes the need to move items from the alpha section to the beta section, and avoids the conflict resolution work that comes with such moves. However, if we adopt this format, I agree with @ritazh that each feature should have its alpha/beta/GA status clearly indicated right below the feature name, in a consistent manner. Regarding the idea of calling them “Optional” or “Additional” features, I feel those terms give an impression of being extra. Since this change affects the overall structure and impacts the work of each feature author, I’d like to get it merged sooner rather than later. |
content/en/docs/concepts/scheduling-eviction/dynamic-resource-allocation.md
Outdated
Show resolved
Hide resolved
| the `.spec.nodeName` field and to use a node selector instead. | ||
|
|
||
| ## DRA beta features {#beta-features} | ||
| ## Optional DRA features |
There was a problem hiding this comment.
+1 to Additional.
To me, additional sounds layered and not extra, a feature that is built on top of the core DRA framework and is a powerful extension. Especially, device taints, partitiontable devices etc is not an optional feature, atleast for GPUs. And as I suggested below, may be put all the features in a table.
There was a problem hiding this comment.
I switched to "Additional" with:
The following sections describe DRA features that support advanced use
case. Usage of them is optional and/or may only be relevant with DRA
drivers that support them.
|
/wg device-management |
content/en/docs/concepts/scheduling-eviction/dynamic-resource-allocation.md
Outdated
Show resolved
Hide resolved
content/en/docs/concepts/scheduling-eviction/dynamic-resource-allocation.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Should we update this to v1beta2? And also the kubectl describe output? That might help illustrate the new timeAdded field.
There was a problem hiding this comment.
Yes. I also noticed that the header had something about DeviceTaintRule in v1alpha3. Added v1beta2 there.
The kubectl describe output doesn't change. The timeAdded was already set on create before, it shows up there as Time Added.
content/en/docs/concepts/scheduling-eviction/dynamic-resource-allocation.md
Outdated
Show resolved
Hide resolved
ebf3053 to
5724c70
Compare
| For stateful applications running on specialized hardware, it is critical to know when a device has failed or become unhealthy. | ||
| It is also helpful to find out if the device recovers. | ||
|
|
||
| To enable this functionality, the `ResourceHealthStatus` [feature gate](/docs/reference/command-line-tools-reference/feature-gates/ResourceHealthStatus/) |
|
Hello @pohly 👋! |
|
LGTM |
| You may also be able to mutate the incoming Pod, at admission time, to unset | ||
| the `.spec.nodeName` field and to use a node selector instead. | ||
|
|
||
| ## DRA beta features {#beta-features} |
There was a problem hiding this comment.
I agree with the change. As features graduate to stable, we should try to find a way to include information about it into the regular flow of the doc, so we don't end up with a long list of "additional features" separate from the overall structure.
|
|
||
| The following sections describe DRA features that are available in the Beta | ||
| The following sections describe DRA features that support advanced use | ||
| case. Usage of them is optional and/or may only be relevant with DRA |
There was a problem hiding this comment.
Do we need the and/or here? I think just using and is sufficient.
|
|
||
| The following sections describe DRA features that are available in the Beta | ||
| The following sections describe DRA features that support advanced use | ||
| case. Usage of them is optional and/or may only be relevant with DRA |
There was a problem hiding this comment.
nit: "features that support advanced use cases"
| case. Usage of them is optional and/or may only be relevant with DRA | |
| cases. Usage of them is optional and/or may only be relevant with DRA |
Changes since 1.34: - Updating the time stamp on updates. - Beta graduation. To simplify feature graduation, the explicit "alpha/beta features" sections get replaced with "Additional features". Linking to feature gates gets harmonized.
5724c70 to
cab8aa6
Compare
|
Is this ready for a final It would be good to merge this soon because it affects other docs PRs which graduate features. |
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: b296ac8dde366724a307a955ad53bca62dce1d45 |
|
/assign @tengqm Ready for approval. /priority important-soon Blocks other docs PRs. |
Description
DRA Device Taints and Tolerations is beta in 1.36.
Issue
KEP: kubernetes/enhancements#5055