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

Notice of deprecation for recycler. #36760

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions pkg/controller/volume/persistentvolume/deprecated-recycle-api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
## Deprecation of Persistent Volume Recycler Behavior
Copy link
Member

Choose a reason for hiding this comment

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

I don't know where this file should live, but the code is probably not the right place. Maybe in the API docs repo, linked from the docs about the feature?

@devin-donnelly

Copy link
Member

Choose a reason for hiding this comment

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

@kubernetes/docs I know the release is imminent, but I want to put this into the queue. Do we have a doc or index of docs that cover deprecated features?

The function of the persistent volume recycler will change in future kubernetes releases. The current implementation will remain and function as is for the short term but is expected to be disabled or significantly changed in future releases.

## Summary
The current recycler design only allows for user space volume erasure operations. This is inadequate for control plane or erasure that requires elevated privileges. Future redesign is anticipated and this document serves as warning of future API change.

**USE RECYCLE API AT YOUR OWN RISK AS IT MAY BE REMOVED OR SIGNIFICANTLY CHANGED IN FUTURE KUBERNETES RELEASES**.

## Problem Scenarios
**Scenario 1**: User attaches a volume to their POD and is assigned a supplemental group but their default group is some other group. New files are created as the default group and for the recycler to remove them it would have to run as the default group. The scenario is similar and problimatic for chown'ed and chgroup'ed files. The recycler has no way to determine at launch which group to run as to remove every file on a recycled volume.

**Scenario 2**: Malicious user writes a POD that uses debugfs to inspect attached volumes for previously deleted files. Malicious user would gain access to "recycled" files from other users.

To work around either of these scenarios kubernetes will need to delete the partition on the storage volume and re-create. In the current architecture the volume attached to the recycler is unavailable for deletion as its in use.

Copy link
Contributor

Choose a reason for hiding this comment

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

spelling - unavailable

Additionally, delete/create is an admin control plane operation and the node hosting the recycle POD will need network access to the storage control plane and the admin credentials. Its desirable to isolate storage control plan from the user network for security.

## Rationale
With the inclusion of configurable dynamic provisioning and storage service classes its expected that most storage volumes will be created dynamically. In situations where the volumes can be created dynamically its more reliable and secure to "recycle" by deleting a volume and recreating it.

Additionally most filesystems do not remove block data when a user space delete operation occurs. The next user of a recycled volume could gain access to the previous users data.

In the short term the user space volume scrub will remain for backwards compatibility. With the recent feature additions to dynamic provisioning the prefered route is to delete and create volumes instead of re-using old ones. The recycler function will be removed from support as soon as the deprecation schedule allows. Users are encouraged to write dynamic provisioners and deleters and use that instead of recycling.

## Deprecated Design
As of Kubernetes 1.2 if a volume is marked for recycle when it is deleted a special recycle container is attached and 'rm -rf's the data on the volume. This behavior is expected to change or be removed.


## Timeline

* Kube 1.5 : Notice of deprecation
* Kube 1.5 + 1 year : Remove old recycler API
Copy link
Member

Choose a reason for hiding this comment

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

This is, unfortunately, not my interpretation of the API compat guarantees. I think we have to keep this for all v1 installations. When we launch API v2, we can remove this from v2 after now + 1 year (which is likely to happen before v2) launches.

@bgrant0607 - have we any precedent for API deprecations yet?

Copy link
Member

Choose a reason for hiding this comment

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

@thockin pod.spec.serviceAccount has been deprecated since 1.0, but has not been removed. node.status.phase has been deprecated and is not populated, but has not been removed.

Technically, we're not supposed to remove fields in the current API version. As I recall, we had planned to remove deprecated fields in the next major API version:
https://github.com/kubernetes/kubernetes/blob/master/docs/design/versioning.md

However, I could be persuaded that 1 year beyond deprecation is sufficient, assuming that we add fine-grain versioning (#34508).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, Brian - that was my comprehension, too.

If we add finer-grained API versioning, I could likewise be convinced. Absent that, we can mark this as deprecated but we can't remove it. Removing the functionality but leaving the API is pretty sketchy, even.

Copy link
Member

Choose a reason for hiding this comment

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

@childsb Can we change the language here to indicate that this will be removed from the next API version, but will remain in the v1 API



[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/pkg/controller/volume/persistentvolume/deprecated-recycle-api.md?pixel)]()