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

KEP Proposal: API for PVC Namespace Transfer #1555

Closed
wants to merge 3 commits into from
Closed
Changes from 2 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
283 changes: 283 additions & 0 deletions keps/sig-storage/20200217-namespace-transfer.md
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 -&gt; Beta Graduation](#alpha---beta-graduation)
- [Beta -&gt; 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.
Copy link
Member

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.

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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The 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 spec.dataSource field. In that case do you think it would still be preferable to delete the original claim? And if so, when? When requests/approvals are matched or the target PVC is created?

Copy link
Contributor

Choose a reason for hiding this comment

The 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
targetName: bar #this is optional
```

Copy link
Contributor

Choose a reason for hiding this comment

The 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
```

When matching `StorageTransferRequest` and `StorageTransferApproval` resources are detected, the transfer process is initiated.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 StorageTransferApproval object like you have here as the DataSource.

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.

Copy link
Author

Choose a reason for hiding this comment

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

The StorageTransferApproval is not in the same namespace as the target PVC, so the DataSource would have to refer to the StorageTransferRequest.

I agree that this would make the controller simpler. But decided against it because:

  1. its one more thing for the user to do. And what if the target spec doesn't match the source?
  2. the DataSource field is not appropriately ignored/handled for all provisioners at current.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Matching requests/approvals is similar to the process for binding PersistentVolumes with PersistentVolumeClaims.
If they are not explicitly bound by the user, a controller will attempt to pair them by looking for matching resource types/names.

Once matching is complete, a controller will begin the transfer process which does the following (not necessarily in order):

- Make sure the associated PersistentVolume is not deleted/recycled during the transfer by setting reclaim policy to `Retain` (if not already).
- Delete the PVC `source-namespace\foo`.
- Bind The PersistentVolume to `target-namespace\bar`.
Copy link

@tedyu tedyu Feb 20, 2020

Choose a reason for hiding this comment

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

Earlier, targetName (with value of bar) is said to be optional.
If the value is omitted, the transfer would use existing name from the source.

It is better to explain this in the procedure.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The 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 StorageTransferRequest.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

The last step makes it possible for the API to be compatible with the most provisioners
as they will ignore a PVC that is already bound. In [previous](https://github.com/kubernetes/enhancements/pull/1112) namespace transfer KEPs, the target PVC is created by the user. Although, this will probably work with the CSI external provisioner, it will not work with others (like static local volume).

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 is set by the controller when requests/approvals are matched
// or by the user when manual binding is preferred
ApprovalName string

// TargetName allows for the source and target resources to have different names
// It is optional
TargetName *string
Copy link

Choose a reason for hiding this comment

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

It may be better to always keep this field populated (which would simplify checking in the code)

Copy link
Author

Choose a reason for hiding this comment

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

Hi @tedyu - thanks for taking a look. Yes, your suggestion would definitely make the implementation simpler. But I wonder about the case in which the user doesn't have access to the source namespace and some coordination with an admin is necessary. Now one more piece of information is required to be communicated. Maybe it's not a big deal, but at this point I kind of feel like the benefit outweighs the cost.

Copy link

Choose a reason for hiding this comment

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

Since the transfer is secure, I wonder under what scenario the user doesn't have access to the source namespace.

Copy link
Author

Choose a reason for hiding this comment

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

A hypothetical case could be something like: Department A needs a copy of a database (PVC) owned by department B. Each department operates only in their own namespace. The admins for A and B can work together to facilitate the transfer rather than getting a cluster admin involved.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe it makes sense to assume that a typical user of this API has access to both namespaces. If so, binding requests/responses is simpler. Dynamic binding can be a later feature.

}
```

```golang
type StorageTransferRequestStatus struct {
Complete bool
Copy link

Choose a reason for hiding this comment

The 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 ?

  Progress int

This way, if the controller can estimate the progress, user would get better idea.

Copy link
Author

Choose a reason for hiding this comment

The 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.

Copy link

@tedyu tedyu Feb 21, 2020

Choose a reason for hiding this comment

The 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.
It is clearer than a single bool.

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 is set by the controller when requests/approvals are matched
// or by the user when manual binding is preferred
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.
Copy link

Choose a reason for hiding this comment

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

One option is to report error for unbound PVCs.
When the PVC is bound, the transfer process can start again.


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