-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
KEP Proposal: API for PVC Namespace Transfer #1555
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,283 @@ | ||
--- | ||
title: Namespace Transfer for Storage Resources | ||
authors: | ||
- "@mhenriks" | ||
owning-sig: sig-storage | ||
participating-sigs: | ||
- sig-storage | ||
reviewers: | ||
- "@saad-ali" | ||
- "@j-griffith" | ||
approvers: | ||
- TBD | ||
editor: TBD | ||
creation-date: 2020-02-13 | ||
last-updated: 2020-02-13 | ||
status: provisional | ||
see-also: | ||
- "/keps/sig-storage/20190709-csi-snapshot.md" | ||
- "/keps/sig-storage/20181111-extend-datasource-field" | ||
replaces: | ||
superseded-by: | ||
--- | ||
|
||
# PVC Namespace Transfer | ||
|
||
## Table of Contents | ||
|
||
<!-- toc --> | ||
- [Release Signoff Checklist](#release-signoff-checklist) | ||
- [Summary](#summary) | ||
- [Motivation](#motivation) | ||
- [Goals](#goals) | ||
- [Non-Goals](#non-goals) | ||
- [Proposal](#proposal) | ||
- [User Stories](#user-stories) | ||
- [Story 1](#story-1) | ||
- [Story 2](#story-2) | ||
- [Implementation Details/Notes/Constraints](#implementation-detailsnotesconstraints) | ||
- [Risks and Mitigations](#risks-and-mitigations) | ||
- [Design Details](#design-details) | ||
- [Test Plan](#test-plan) | ||
- [Graduation Criteria](#graduation-criteria) | ||
- [Examples](#examples) | ||
- [Alpha -> Beta Graduation](#alpha---beta-graduation) | ||
- [Beta -> GA Graduation](#beta---ga-graduation) | ||
- [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) | ||
- [Version Skew Strategy](#version-skew-strategy) | ||
- [Implementation History](#implementation-history) | ||
<!-- /toc --> | ||
|
||
## Release Signoff Checklist | ||
|
||
**ACTION REQUIRED:** In order to merge code into a release, there must be an issue in [kubernetes/enhancements] referencing this KEP and targeting a release milestone **before [Enhancement Freeze](https://github.com/kubernetes/sig-release/tree/master/releases) | ||
of the targeted release**. | ||
|
||
For enhancements that make changes to code or processes/procedures in core Kubernetes i.e., [kubernetes/kubernetes], we require the following Release Signoff checklist to be completed. | ||
|
||
Check these off as they are completed for the Release Team to track. These checklist items _must_ be updated for the enhancement to be released. | ||
|
||
- [ ] kubernetes/enhancements issue in release milestone, which links to KEP (this should be a link to the KEP location in kubernetes/enhancements, not the initial KEP PR) | ||
- [ ] KEP approvers have set the KEP status to `implementable` | ||
- [ ] Design details are appropriately documented | ||
- [ ] Test plan is in place, giving consideration to SIG Architecture and SIG Testing input | ||
- [ ] Graduation criteria is in place | ||
- [ ] "Implementation History" section is up-to-date for milestone | ||
- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] | ||
- [ ] Supporting documentation e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes | ||
|
||
**Note:** Any PRs to move a KEP to `implementable` or significant changes once it is marked `implementable` should be approved by each of the KEP approvers. If any of those approvers is no longer appropriate than changes to that list should be approved by the remaining approvers and/or the owning SIG (or SIG-arch for cross cutting KEPs). | ||
|
||
**Note:** This checklist is iterative and should be reviewed and updated every time this enhancement is being considered for a milestone. | ||
|
||
[kubernetes.io]: https://kubernetes.io/ | ||
[kubernetes/enhancements]: https://github.com/kubernetes/enhancements/issues | ||
[kubernetes/kubernetes]: https://github.com/kubernetes/kubernetes | ||
[kubernetes/website]: https://github.com/kubernetes/website | ||
|
||
## Summary | ||
|
||
This KEP proposes an API to securely transfer Kubernetes storage resources between namespaces. | ||
The initial implementation will focus on PersistentVolumeClaims. But the API could be applied | ||
to other resource types as compelling use cases come up. | ||
|
||
`StorageTransferRequest` resources are created in the target namespace. | ||
`StorageTransferApproval` resources are created in the source namespace. | ||
When a controller detects matching request/approval resources, a transfer in initiated. | ||
|
||
When transferring a PersistentVolumeClaim, the associated PersistentVolume will be updated, | ||
the source PersistentVolumeClaim will be deleted, and the target PersistentVolumeClaim will be created. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The more I thought about this the last time around, the more I liked the idea of deleting the original claim; I think that's the right step FWIW. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In another comment here, you propose having the triggering the transfer via the pvc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that still aligns IMO, once the transfer process is completed then you'd delete the claim from the original namespace. |
||
The transfer simply deals with Kubernetes API resources. | ||
No data on the physical PersistentVolume is accessed/copied. | ||
|
||
## Motivation | ||
|
||
Give Kubernetes users the flexibility to easily and securely share data between namespaces. | ||
|
||
### Goals | ||
|
||
- Define an API for transferring persistent storage resources between namespaces. | ||
- The API should be compatible with as many existing storage provisioners as possible. | ||
|
||
### Non-Goals | ||
|
||
- Transfer of resources other than PersistentVolumeClaims will not be discussed. | ||
- Implementation details only discussed in the context of influencing the API definition. | ||
|
||
## Proposal | ||
|
||
`StorageTransferRequest` resource: | ||
|
||
```yaml | ||
apiVersion: v1alpha1 | ||
kind: StorageTransferRequest | ||
metadata: | ||
name: transfer-foo | ||
namespace: target-namespace | ||
spec: | ||
source: | ||
name: foo | ||
namespace: source-namespace | ||
kind: PersistentVolumeClaim | ||
approvalName: approve-foo | ||
targetName: bar | ||
``` | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know that it's necessary, but do we want to consider requiring a secret associated with this? It may be overkill because it's an active operation on both sides and it will only transfer to the destination specified by the source; but just something to think about (briefly). Maybe that's an implementation detail and we address it later. |
||
`StorageTransferApproval` resource: | ||
|
||
```yaml | ||
apiVersion: v1alpha1 | ||
kind: StorageTransferApproval | ||
metadata: | ||
name: approve-foo | ||
namespace: source-namespace | ||
spec: | ||
source: | ||
name: foo | ||
kind: PersistentVolumeClaim | ||
targetNamespace: target-namespace | ||
requestName: transfer-foo | ||
``` | ||
|
||
When matching `StorageTransferRequest` and `StorageTransferApproval` resources are detected, the transfer process is initiated. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing I thought might improve this, rather than immediately taking the action on this approval object, would it make sense to leverage the pvc datasource model that we've been using for other things? So for example what I'm thinking is that to import or accept a transfer the new namespace still issues a new PVC request, but they use a So the controller would wait for not only the matching requests, but the final even to kick this off would be a create pvc from that transfer datasource. Maybe that's more of an implementation detail. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The I agree that this would make the controller simpler. But decided against it because:
2 is obviously also an issue for the populators, so maybe shouldn't be too concerned about it now? But it could lead to some undesirable behavior in the meantime. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I often reverse who does the request and who does the accept... As far as the other points, I see both sides (mine and yours). Would be curious to see if anybody else has thoughts, I'm open to your ideas. Just want to make sure usability is good and if there's places to leverage existing work we do so. |
||
A `StorageTransferRequest` named `r` and `StorageTransferApproval` `a` are matched all of the following conditions are met: | ||
|
||
- `r.metadata.name == a.spec.requestName` | ||
- `r.metadata.namespace == a.spec.targetNamespace` | ||
- `r.spec.source.namespace == a.metadata.namespace` | ||
- `r.spec.source.name == a.spec.source.name` | ||
- `r.spec.source.kind == a.spec.source.kind` | ||
- `r.spec.approvalName == a.metadata.name` | ||
|
||
Once matching is complete, a controller will begin the transfer process which does the following (not necessarily in order): | ||
|
||
- Set the reclaim policy of the associated PersistentVolume `Retain` (if not already). | ||
- Delete the PVC `source-namespace\foo`. | ||
- Bind The PersistentVolume to `target-namespace\bar`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Earlier, targetName (with value of bar) is said to be optional. It is better to explain this in the procedure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I'd prefer not to be implicit, that's one of the reasons I proposed using the datasource model, we can easily enforce things like quota, limits and required claim parameters that way IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, with the datasource model, the user will directly see if creating the target PVC fails because of a quota limitation. Otherwise, it will happen in the background and be reported in the status of the What other required parameters are a concern? And does validating the new target PVC require referencing the source PVC to validate compatible parameters? If so, that would be complicated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My main philosophy has been just to make it as close to the existing create workflow as possible and avoid assumptions. It also means you don't have to duplicate parameters or testing for them. Instead you use the existing create implementation as your foundation and build on it rather than trying to make it better or offering a divergent path. Just my 2 cents though. |
||
- Create the PVC `target-namespace\bar` by copying the spec of `source-namespace\foo` and setting `spec.volumeName` appropriately. | ||
- Reset the PersistentVolume reclaim policy | ||
|
||
Users can monitor the status of a transfer by querying for the existence of the bound target PVC or checking the `status.complete` property of `StorageTransferRequest`. | ||
|
||
`StorageTransferRequest` object: | ||
|
||
```golang | ||
type StorageTransferRequest struct { | ||
metav1.TypeMeta | ||
metav1.ObjectMeta | ||
|
||
Spec StorageTransferRequestSpec | ||
Status *StorageTransferRequestStatus | ||
} | ||
``` | ||
|
||
```golang | ||
type StorageTransferRequestSpec struct { | ||
Source *corev1.ObjectReference | ||
|
||
ApprovalName string | ||
|
||
TargetName string | ||
} | ||
``` | ||
|
||
```golang | ||
type StorageTransferRequestStatus struct { | ||
Complete bool | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we change the bool field to int field where 100 (percent) means completion ?
This way, if the controller can estimate the progress, user would get better idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you suggest measuring progress? No data is transferred. The transfer simply involves updating metadata and should occur quickly after requests/approvals are matched. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another option is to use enum to represent each step as outlined starting line 147 above. Actually this field can be a string where 'done' or 'complete' marks the end of operation. |
||
Error *StorageTransferError | ||
} | ||
``` | ||
|
||
`StorageTransferApproval` object: | ||
|
||
```golang | ||
type StorageTransferApproval struct { | ||
metav1.TypeMeta | ||
metav1.ObjectMeta | ||
|
||
Spec StorageTransferApprovalSpec | ||
Status *StorageTransferApprovalStatus | ||
} | ||
``` | ||
|
||
```golang | ||
type StorageTransferApprovalSpec struct { | ||
Source *corev1.TypedLocalObjectReference | ||
|
||
TargetNamespace string | ||
|
||
RequestName string | ||
} | ||
``` | ||
|
||
```golang | ||
type StorageTransferApprovalStatus struct { | ||
Complete bool | ||
Error *StorageTransferError | ||
} | ||
``` | ||
|
||
### User Stories | ||
|
||
When combined with VolumeSnapshots and PVC cloning, users are given the power to easily share/experiment with persistent data. | ||
|
||
#### Story 1 | ||
|
||
The `prod` namespace contains the production database which is stored on the `db1` PersistentVolumeClaim. | ||
VolumeSnapshots of `db1` are taken at regular intervals. DBAs want to test a schema update script before running it in `prod`. | ||
A new PVC named `db1-test` is created in `prod` from a VolumeSnapshot of `db1`. The `db1-test` PVC is then transferred to the `stage` namespace where it can safely be mounted and modified in an isolated testing environment that mimics `prod`. | ||
|
||
#### Story 2 | ||
|
||
Someone wants to build an Infrastructure as a Service provider with Kubernetes and [KubeVirt](https://github.com/kubevirt). | ||
All the default virtual machine images can be stored on PersistentVolumeClaims in a single namespace called `golden-images`. | ||
When virtual machines are created, the PVC containing the appropriate virtual machine image is cloned and transferred to the target namespace. | ||
The virtual machine is booted from the cloned/transferred PVC. | ||
|
||
### Implementation Details/Notes/Constraints | ||
|
||
I believe the API can be implemented as CRDs and an external controller. If so, is a KEP really required? Would there be advantages to implementing in-tree? | ||
|
||
What is the best way to deal with unbound PVCs (WaitForFirstConsumer)? Transfer the unbound PVC? Wait until it's bound? May be race conditions with the former. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One option is to report error for unbound PVCs. |
||
|
||
Should there be a way (with the API) for a user to automatically approve all transfers from a namespace? | ||
|
||
### Risks and Mitigations | ||
|
||
Are all security concerns handled by request having separate request/approval resources? | ||
Should `sig-auth` be included in the review? | ||
|
||
I think the transfer phase (for bound PVCs) can be done by an external controller in a failure resistant way | ||
but issues may be discovered in the implementation phase. | ||
|
||
## Design Details | ||
|
||
### Test Plan | ||
|
||
Functionality should be tested/verified with all in-tree and multiple external provisioners. | ||
|
||
**Note:** *Section not required until targeted at a release.* | ||
|
||
### Graduation Criteria | ||
|
||
**Note:** *Section not required until targeted at a release.* | ||
|
||
#### Examples | ||
|
||
These are generalized examples to consider, in addition to the aforementioned [maturity levels][maturity-levels]. | ||
|
||
##### Alpha -> Beta Graduation | ||
|
||
##### Beta -> GA Graduation | ||
|
||
**Note:** Generally we also wait at least 2 releases between beta and GA/stable, since there's no opportunity for user feedback, or even bug reports, in back-to-back releases. | ||
|
||
### Upgrade / Downgrade Strategy | ||
|
||
### Version Skew Strategy | ||
|
||
## Implementation History | ||
|
||
- [Initial](https://github.com/kubernetes/enhancements/pull/643) PVC transfer proposal by John Griffith (@j-griffith) in Dec 2018 | ||
- [Updated](https://github.com/kubernetes/enhancements/pull/1112) PVC transfer proposal by John Griffith (@j-griffith) in Jun 2019 |
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.
@kubernetes/sig-auth-api-reviews FYI.