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.

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

generic ephemeral volumes: beta #26801

Merged

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Mar 2, 2021

The feature is scheduled for becoming beta in 1.21.

@k8s-ci-robot k8s-ci-robot added this to the 1.21 milestone Mar 2, 2021
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Mar 2, 2021

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

Building with commit 4021005

https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/605a248dc3c93a000732bc88

@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Mar 2, 2021
@k8s-ci-robot k8s-ci-robot requested review from msau42 and xing-yang March 2, 2021 19:28
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 2, 2021
@pohly
Copy link
Contributor Author

pohly commented Mar 2, 2021

/hold

The actual change in Kubernetes is currently pending for review in kubernetes/kubernetes#99643 and some other PRs.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 2, 2021
@pohly
Copy link
Contributor Author

pohly commented Mar 2, 2021

cc: @reylejano

@fejta-bot
Copy link

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.
/check-cla

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 2, 2021
@reylejano
Copy link
Member

/milestone 1.21
/assign @mvortizr
/sig storage
hold until kubernetes/kubernetes#99643 merges

@k8s-ci-robot k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Mar 2, 2021
@pohly pohly force-pushed the generic-ephemeral-volumes-beta branch from 40ffe96 to 6319a96 Compare March 4, 2021 07:23
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 4, 2021
@pohly pohly mentioned this pull request Mar 4, 2021
@pohly
Copy link
Contributor Author

pohly commented Mar 10, 2021

/hold cancel

All k/k PRs were merged.

@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 Mar 10, 2021
- Explicitly disable the feature through the feature gate, to avoid
being surprised when some future Kubernetes version enables it
by default.
- Explicitly disable the feature through the feature gate.
- Use a [Pod Security
Copy link
Member

Choose a reason for hiding this comment

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

@pohly Pod Security Policy is tracked to be deprecated in 1.21. I recommend removing the option to use a Pod Security Policy on this page. What are @kubernetes/sig-docs-en-reviews and @kubernetes/sig-storage-pr-reviews opinions in removing recommending a PSP on this page .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know about the deprecation. But it's the only available option for per-pod control of this feature, so IMHO it is worth mentioning.

Copy link
Contributor

Choose a reason for hiding this comment

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

SIG Auth might have an opinion here too.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are definitely relevant third party solutions in this area; we don't need to list them, but we can mention that various kinds of admission-time restriction on Pods can help restrict PVC creation.

Copy link
Member

Choose a reason for hiding this comment

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

Is there already a page with PSP alternatives? If so, then we can point to that as additional options.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is OK to merge, and it's also good to highlight this to SIG Security / SIG Auth as an area to consider revising for the v1.21 docs.

Copy link
Member

Choose a reason for hiding this comment

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

#26629 is adding PSP to the deprecation guide for 1.21.

Granular control over specific volume types is not planned for the in-tree replacement for PSP, so pointing people who want that towards the admission webhook documentation seems like a more durable recommendation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a "(deprecated in Kubernetes 1.21)" remark for the PSP bullet item and added another one for admission webhook:

* Use an [admission webhook](/docs/reference/access-authn-authz/extensible-admission-controllers/)
  which rejects objects like Pods that have a generic ephemeral
  volume.

- Explicitly disable the feature through the feature gate, to avoid
being surprised when some future Kubernetes version enables it
by default.
- Explicitly disable the feature through the feature gate.
- Use a [Pod Security
Copy link
Member

Choose a reason for hiding this comment

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

Is there already a page with PSP alternatives? If so, then we can point to that as additional options.


This feature requires the `GenericEphemeralVolume` [feature gate](/docs/reference/command-line-tools-reference/feature-gates/) to be
enabled. Because this is an alpha feature, it is disabled by default.
enabled. Because this is a beta feature, it is enabled by default.

Generic ephemeral volumes are similar to `emptyDir` volumes, just more
Copy link
Member

Choose a reason for hiding this comment

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

Similar to emptyDir in what way? I can only think of lifetime as the only similarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are also empty by default because they get provisioned anew for each pod startup. That they don't have to be empty falls under "more flexible"

Copy link
Member

@msau42 msau42 Mar 17, 2021

Choose a reason for hiding this comment

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

I would explicitly state what aspects are similar if we're going to be comparing the two types. There's many assumptions and features around emptydir that do not apply to generic ephemeral volumes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed into:

Generic ephemeral volumes are similar to emptyDir volumes in the
sense that they provide a per-pod directory for scratch data that is
usually empty after provisioning. But they may also have additional
features:

- Explicitly disable the feature through the feature gate, to avoid
being surprised when some future Kubernetes version enables it
by default.
- Explicitly disable the feature through the feature gate.
- Use a [Pod Security
Policy](/docs/concepts/policy/pod-security-policy/) where the
`volumes` list does not contain the `ephemeral` volume type.
Copy link
Member

Choose a reason for hiding this comment

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

For the quota discussion below, I would link to our pages for storageclass quota.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reylejano
Copy link
Member

Hi @pohly , thank you for having your Doc PR ready for review. Please squash your commits.

Just wanted to share the upcoming doc related dates for the 1.21 release:

  • Docs Ready for Review deadline is March 24 done
  • Docs Ready to Merge deadline is March 31

Just need a few lgtms after the updates:
@kubernetes/sig-storage-pr-reviews Can you provide a tech lgtm after reviewing the updates?
@kubernetes/sig-docs-pr-reviews Can you provide a lgtm after reviewing the updates?

@pohly pohly force-pushed the generic-ephemeral-volumes-beta branch from 476a6b1 to 4247374 Compare March 22, 2021 20:44
@pohly
Copy link
Contributor Author

pohly commented Mar 22, 2021

Please squash your commits.

There were logically separate, but okay, I squashed them.

@msau42
Copy link
Member

msau42 commented Mar 23, 2021

Mostly lgtm. There's still one open question on psp alternatives.

@pohly
Copy link
Contributor Author

pohly commented Mar 23, 2021

There's still one open question on psp alternatives.

What if there is none that we can link to? Is the text then okay?

The feature is scheduled for becoming beta in 1.21.

In addition, the commit addresses some of the review feedback.
@pohly pohly force-pushed the generic-ephemeral-volumes-beta branch from 4247374 to 4021005 Compare March 23, 2021 17:25
@reylejano
Copy link
Member

There's still one open question on psp alternatives.

What if there is none that we can link to? Is the text then okay?

After reviewing comments from liggit and thumbsup from sftim , the text looks good

/lgtm

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

LGTM label has been added.

Git tree hash: 5a6735bf303657f055853b93af6531100e383e7c

@reylejano
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: reylejano

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 Mar 25, 2021
@k8s-ci-robot k8s-ci-robot merged commit d53aef4 into kubernetes:dev-1.21 Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/storage Categorizes an issue or PR as relevant to SIG Storage. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants