-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Mark storage capacity as GA + related updates #32008
Conversation
👷 Deploy Preview for kubernetes-io-vnext-staging processing. 🔨 Explore the source changes: e86725d 🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/622116f7f6bb9a0007e4e08d |
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 drafting this @pohly
I've added some inline feedback about showing what you need in order to use storage capacity tracking.
On top of the inline feedback, I suggest hyperlinking to https://kubernetes.io/docs/concepts/scheduling-eviction/ from the first mention of “scheduling”.
I also recommend omitting:
[Further work](https://github.com/kubernetes/enhancements/pull/1703) is needed to handle this automatically.
We actually avoid writing hints about future development in docs like this (one reason: as soon as that PR merges, the statement becomes partially incorrect). For beta and GA features, we try to get the docs reasonably close to our style guide.
Storage capacity tracking is a stable feature and always enabled in a | ||
Kubernetes cluster since Kubernetes 1.24. A CSI driver also has to support | ||
it. Please refer to the driver's documentation for details. |
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.
Storage capacity tracking is a stable feature and always enabled in a | |
Kubernetes cluster since Kubernetes 1.24. A CSI driver also has to support | |
it. Please refer to the driver's documentation for details. | |
Kubernetes v{{< skew currentVersion >}} includes cluster-level API support for storage | |
capacity tracking. To use this you must also be using a CSI driver that supports capacity | |
tracking. Consult the documentation for the CSI drivers that you use to find out whether | |
this support is available and, if so, how to use it. | |
If you are not running Kubernetes v{{< skew currentVersion >}}, check the documentation | |
for that version of Kubernetes. |
I would however also move this particular section to the start of the page (just before <!-- body -->
) and change the heading to read:
## {{% heading "prerequisites" %}}
You can then also reword or remove:
Tracking storage capacity is supported for {{< glossary_tooltip
text="Container Storage Interface" term_id="csi" >}} (CSI) drivers and
[needs to be enabled](#enabling-storage-capacity-tracking) when installing a CSI driver.
Done. There was a link to the scheduler under "further reading" which I removed because now it already gets introduced early on.
Done.
Done. I hope I understood correctly where the subheading needs to go. The previous split came about because enabling was more complicated. I agree that we can simplify that now. An unrelated change is a new link to the CSIStorageCapacity object. That link should become valid once the code PR is merged and the corresponding documentation is public. |
10ae184
to
950226b
Compare
## {{% heading "prerequisites" %}} | ||
|
||
Kubernetes v{{< skew currentVersion >}} includes cluster-level API support for | ||
storage capacity tracking. To use this you must also be using a CSI driver that | ||
supports capacity tracking. Consult the documentation for the CSI drivers that | ||
you use to find out whether this support is available and, if so, how to use | ||
it. If you are not running Kubernetes v{{< skew currentVersion >}}, check the | ||
documentation for that version of Kubernetes. |
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.
|
||
<!-- body --> | ||
|
||
## API | ||
|
||
There are two API extensions for this feature: | ||
- CSIStorageCapacity objects: | ||
- [CSIStorageCapacity](/docs/reference/generated/kubernetes-api/{{< param "version" >}}/#csistoragecapacity-v1-storage-k8s-io) objects: |
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 links to the legacy API reference. It'd be nice to link to a page within https://kubernetes.io/docs/reference/kubernetes-api/ instead.
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.
Looking at the other, already valid links there's a -v1
missing. Will add that.
Should the link have the https://kubernetes.io
prefix or start with /docs
?
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.
In the Markdown, start links to documentation with /docs/
https://kubernetes.io/docs/contribute/style/style-guide/#links has an 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.
Done. I also updated the link for CSIDriver (same bullet list).
/retitle Mark storage capacity as GA + related updates |
950226b
to
e86725d
Compare
/assign @PI-Victor |
/hold cancel The code PR was merged. Feel free to merge this one here. |
/lgtm |
LGTM label has been added. Git tree hash: 8cf3168dc34a4ee8e8ae33bb6b2d769933965363
|
/cc @nate-double-u |
/sig storage |
/lgtm |
/approve |
3 similar comments
/approve |
/approve |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, nate-double-u, tengqm, xing-yang 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 |
The feature is meant to graduate to GA in 1.24.
/hold
kubernetes/kubernetes#108445 needs to be merged first.