Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Document SizeMemoryBackedVolumes feature for 1.20 #24906

Merged
merged 1 commit into from
Dec 1, 2020
Merged

Document SizeMemoryBackedVolumes feature for 1.20 #24906

merged 1 commit into from
Dec 1, 2020

Conversation

derekwaynecarr
Copy link
Member

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 5, 2020
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Nov 5, 2020 •
edited
Loading

Deploy preview for kubernetes-io-vnext-staging processing.

Building with commit af23941

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/5fc689ebacc0c000075800bc

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. language/en Issues or PRs related to English language labels Nov 5, 2020
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Nov 5, 2020
@annajung
Copy link
Contributor

annajung commented Nov 5, 2020

/milestone 1.20
/hold no k/k pr
/assign
/sig node

enhancement: kubernetes/enhancements#1967

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Nov 5, 2020
@k8s-ci-robot k8s-ci-robot added this to the 1.20 milestone Nov 5, 2020
sftim
sftim reviewed Nov 5, 2020
View reviewed changes
content/en/docs/concepts/storage/volumes.md Outdated
@@ -303,6 +303,12 @@ While tmpfs is very fast, be aware that unlike disks, tmpfs is cleared on
node reboot and any files you write count against your container's
memory limit.

{{< note >}}
If the `SizeMemoryBackedVolumes` feature gate is enabled in 1.20+, users may
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd write this:

Suggested change
If the `SizeMemoryBackedVolumes` feature gate is enabled in 1.20+, users may
If the `SizeMemoryBackedVolumes` [feature gate](/docs/reference/command-line-tools-reference/feature-gates/) is enabled, you can

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to update /docs/reference/command-line-tools-reference/feature-gates/

@annajung
Copy link
Contributor

/hold cancel
k/k pr merged

Hi @derekwaynecarr w/ k/k pr merged in, Sig Docs would appreciate it if you could fill the content of the docs early, ideally before kubecon to get it reviewed/merged on time. Thank you!

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 12, 2020
@sftim
Copy link
Contributor

sftim commented Nov 19, 2020

Hi @derekwaynecarr

Would you have time to make sure that /docs/reference/command-line-tools-reference/feature-gates/ is up to date for the changes this PR is documenting?

@annajung
Copy link
Contributor

Hi @derekwaynecarr today is the deadline for docs to be in ready for review state. Please address Tim's comment above and let us know if this is ready to be reviewed.

@derekwaynecarr
Copy link
Member Author

I made the recommended update, and believe this is ready for review.

annajung
annajung reviewed Nov 23, 2020
View reviewed changes
content/en/docs/reference/command-line-tools-reference/feature-gates.md
@@ -148,6 +148,7 @@ different Kubernetes components.
| `ServiceNodeExclusion` | `false` | Alpha | 1.8 | 1.18 |
| `ServiceNodeExclusion` | `true` | Beta | 1.19 | |
| `ServiceTopology` | `false` | Alpha | 1.17 | |
| `SizeMemoryBackedVolumes` | `false` | Alpha | 1.20 | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need a short description below describing this feature. Then, I think docs are good to merge

sftim
sftim reviewed Nov 24, 2020
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if https://kubernetes.io/docs/concepts/storage/ephemeral-volumes/ should also get a mention of the new feature gate. What do you think @derekwaynecarr ?

@sftim
Copy link
Contributor

sftim commented Nov 29, 2020

Hi @derekwaynecarr

Repeating #24906 (comment) - we'd like a summary of the new feature gate, similar to the other ones already in the list.

@reylejano
Copy link
Member

Hi @derekwaynecarr, just a reminder that the docs merge deadline is in a few days on Dec 2. If you can address @sftim 's comment about adding a summary of the new feature gate then this PR can merge after a quick review. Thank you

@derekwaynecarr derekwaynecarr force-pushed the memory-backed-size branch 2 times, most recently from 7b7a64c to 4a82d03 Compare December 1, 2020 16:02
@derekwaynecarr
Copy link
Member Author

@annajung addressed feedback

sftim
sftim requested changes Dec 1, 2020
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One fix needed; otherwise LGTM.

content/en/docs/reference/command-line-tools-reference/feature-gates.md Outdated
@@ -527,6 +528,7 @@ Each feature gate is designed for enabling/disabling a specific feature:
- `ServiceNodeExclusion`: Enable the exclusion of nodes from load balancers created by a cloud provider.
A node is eligible for exclusion if labelled with "`alpha.service-controller.kubernetes.io/exclude-balancer`" key or `node.kubernetes.io/exclude-from-external-load-balancers`.
- `ServiceTopology`: Enable service to route traffic based upon the Node topology of the cluster. See [ServiceTopology](/docs/concepts/services-networking/service-topology/) for more details.
- `SizeMemoryBackedVolumes`: Enables kubelet support to size memory backed volumes. See [volumes] (docs/concepts/storage/volumes) for more details.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CommonMark doesn't like the extra space

Suggested change
- `SizeMemoryBackedVolumes`: Enables kubelet support to size memory backed volumes. See [volumes] (docs/concepts/storage/volumes) for more details.
- `SizeMemoryBackedVolumes`: Enables kubelet support to size memory backed volumes. See [volumes](docs/concepts/storage/volumes) for more details.

@derekwaynecarr
Copy link
Member Author

@sftim fixed typo.

@annajung
Copy link
Contributor

annajung commented Dec 1, 2020

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 1, 2020
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 81f43f8178c62edce4a3b091e24f2c138136e3db

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: annajung

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 1, 2020
@k8s-ci-robot k8s-ci-robot merged commit 21c05e4 into kubernetes:dev-1.20 Dec 1, 2020
@ehashman ehashman mentioned this pull request May 14, 2021
Open
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annajung annajung annajung left review comments

@sftim sftim sftim requested changes

@jsafrane jsafrane Awaiting requested review from jsafrane

@thockin thockin Awaiting requested review from thockin

Assignees

@annajung annajung

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
1.20
Development

Successfully merging this pull request may close these issues.
6 participants
@derekwaynecarr @k8sio-netlify-preview-bot @annajung @sftim @reylejano @k8s-ci-robot