Implement dynamic provisioning (beta) of PersistentVolumes via StorageClass #29006

jsafrane · 2016-07-15T14:39:10Z

Implemented according to PR #26908. There are several patches in this PR with one huge code regen inside.

Please review the API changes (the first patch) carefully, sometimes I don't know what the code is doing...
PV.Spec.Class and PVC.Spec.Class is not implemented, use annotation volume.alpha.kubernetes.io/storage-class
See e2e test and integration test changes - Kubernetes won't provision a thing without explicit configuration of at least one StorageClass instance!
Multiple provisioning volume plugins can coexist together, e.g. HostPath and AWS EBS. This is important for Gluster and RBD provisioners in [WIP] Adding dynamic provisioner and deleter interface for glusterfs #25026
Contradicting the proposal, claim.Selector and volume.alpha.kubernetes.io/storage-class annotation are not mutually exclusive. They're both used for matching existing PVs. However, only volume.alpha.kubernetes.io/storage-class is used for provisioning, configuration of provisioning with Selector is left for (near) future.
Documentation is missing. Can please someone write some while I am out?

For now, AWS volume plugin accepts classes with these parameters:

kind: StorageClass
metadata:
  name: slow
provisionerType: kubernetes.io/aws-ebs
provisionerParameters:
  type: io1
  zone: us-east-1d
  iopsPerGB: 10

parameters are case-insensitive
type: io1, gp2, sc1, st1. See AWS docs for details
iopsPerGB: only for io1 volumes. I/O operations per second per GiB. AWS volume plugin multiplies this with size of requested volume to compute IOPS of the volume and caps it at 20 000 IOPS (maximum supported by AWS, see AWS docs).
of course, the plugin will use some defaults when a parameter is omitted in a StorageClass instance (gp2 in the same zone as in 1.3).

GCE:

apiVersion: extensions/v1beta1
kind: StorageClass
metadata:
  name: slow
provisionerType: kubernetes.io/gce-pd
provisionerParameters:
  type: pd-standard
  zone: us-central1-a

type: pd-standard or pd-ssd
zone: GCE zone
of course, the plugin will use some defaults when a parameter is omitted in a StorageClass instance (SSD in the same zone as in 1.3 ?).

No OpenStack/Cinder yet

@kubernetes/sig-storage

This change is

jsafrane · 2016-07-15T14:48:33Z

Yay, XXL, I am sorry. Generated code + lot of unit test changes. The actual logic is pretty straightforward though.

jsafrane · 2016-07-15T14:53:55Z

Also, something tells me that extensions/storageclasses API should be enabled by default. I expect some integration/e2e tests will fail because of this. What's the official procedure of enabling it?

jsafrane · 2016-07-15T15:34:40Z

as expected, e2e tests fail:

E0715 15:06:59.173246       5 reflector.go:216] pkg/controller/volume/persistentvolume/controller_base.go:148: Failed to list *extensions.StorageClass: the server could not find the requested resource

matchstick · 2016-07-15T16:10:47Z

@kubernetes/sig-storage Throwing this on the 1.4 milestone and also I am going to take a stab at reviewing it but realize with my noobness I should not be one to LGTM.

On another note. Jan you are pure liquid awesome.

jsafrane · 2016-07-15T16:37:39Z

I am on PTO for next two weeks, @childsb will cover this PR on my behalf. I hope I did not mess it up too much.

rootfs · 2016-07-18T18:58:51Z

pkg/apis/extensions/types.go

+
+	// ProvisionerParameters holds the parameters for the provisioner that should
+	// create volumes of this storage class.
+	ProvisionerParameters map[string]string `json:"provisionerParameters,omitempty"`


what if a provisioner needs secret?

Since none of our intree provisioners need secrets right now, I think we should keep that for the second phase.

The Ceph Volume Plugin needs secrets. We'll need it if we are to add a Ceph
provisioner.

On Tue, Jul 19, 2016 at 10:19 AM, bradley childs notifications@github.com
wrote:

In pkg/apis/extensions/types.go
#29006 (comment)
:

+// StorageClass describes the parameters for a class of storage for
+// which PersistentVolumes can be dynamically provisioned.
+//
+// StorageClasses are non-namespaced; the name of the storage class
+// according to etcd is in ObjectMeta.Name.
+type StorageClass struct {

unversioned.TypeMeta json:",inline"

api.ObjectMeta json:"metadata,omitempty"

// ProvisionerType indicates the type of the provisioner.

ProvisionerType string json:"provisionerType,omitempty"

// ProvisionerParameters holds the parameters for the provisioner that should

// create volumes of this storage class.

ProvisionerParameters map[string]string json:"provisionerParameters,omitempty"

Since none of our intree provisioners need secrets right now, I think we
should keep that for the second phase.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubernetes/pull/29006/files/c28e599c704a45c55eb83426fb761329adcc6ed4#r71359954,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADDZzDn9kq-e3mauq2kRGO8XoLtNsPCaks5qXOsWgaJpZM4JNdmW
.

Secret is needed once Azure cloudprovider #28821 is ready and we are going to support dynamic provisioning on Azure. Azure storage service access account and keys should be treated as secret.

cc @colemickens

Hi @rootfs, if you're going to piggy-back on the cloudprovider's auth, you might not need your own Secret, right? Since the cloudprovider's service account could be used to retrieve the storage account keys.

Allowing a secret is more flexible though, you could separate privileges better potentially.

@rootfs @colemickens

If you need a secret, the coordinates of the secret should be a parameter to the provisioner. Let me note -- this would be the first case in which we allowed secrets to be referenced from outside their namespace and will need to be thought through from an auth/ACL/permissions perspective.

@pmorie @jsafrane if we make secrets a parameter, we better document the usage, for dev and end user.

The provisioner should create the secret needed to use the provisioned PV as a part of provisioning the storage.

If i'm creating a RBD volume the PV will need a secret (generated by the provisioner) to connect. each volume secret may/should be different. I would expect secrets needed as a param to provisioner a much smaller use case.

Agree to leave Secrets until we have a concrete case. It is a scary precedent to descend into namespaces.

"ProvisionerParameters" is a mouthful, and I think "Provisioner" adds no value. How about "Parameters" or "Arguments"?

rootfs · 2016-07-19T20:44:06Z

I'll add cinder volume type.

pmorie · 2016-07-19T20:48:25Z

pkg/apis/extensions/types.go

+	ProvisionerParameters map[string]string `json:"provisionerParameters,omitempty"`
+}
+
+// ProvisionerType describes the type of a provisioner.


Forgive my ignorance -- I had thought that we were not going to formalize any provisioner types in the API. Did that change while I was out?

Agree with Paul. Let's not. Just strike this and the list

justinsb · 2016-08-18T15:52:28Z

pkg/volume/gce_pd/gce_util.go

+
+	if zone == "" {
+		// No zone specified, choose one randomly in the same region as the
+		// node is running.


Just FYI it isn't strictly random. It will spread across the zones, realiably so for PetSets

justinsb · 2016-08-18T15:53:19Z

AWS stuff LGTM. Not sure why we are forcing users to do math for IOPS, but it's fine :-)

thockin · 2016-08-18T16:04:28Z

Jan & Brad,

We have not been updating kubernetes/enhancements#36 :(

Reviewed 7 of 8 files at r11.
Review status: 30 of 31 files reviewed at latest revision, 2 unresolved discussions.

Comments from Reviewable

thockin · 2016-08-18T16:09:26Z

Reviewed 1 of 8 files at r11.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

examples/experimental/persistent-volume-provisioning/README.md, line 46 [r8] (raw file):

Previously, thockin (Tim Hockin) wrote…

We also need to update docs/user-guide/persistent-volumes/ and docs/user-guide/volumes.md in kubernetes/kubernetes.github.io

user docs is the last checkbox on https://github.com/kubernetes/enhancements/issues/36

Comments from Reviewable

thockin · 2016-08-18T16:09:49Z

+lgtm

Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

thockin · 2016-08-18T16:14:22Z

re-titled for relnote. The release not tag says "action required". Can you update teh first comment with notes for cluster admins on what action they need to take (I think none, in which case just change the label)

k8s-github-robot · 2016-08-18T16:17:27Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

jsafrane · 2016-08-18T16:49:36Z

I'll update docs after the freeze... How do we document "beta" features? There needs to be a big exclamation mark somewhere that "beta" annotation will be probably replaced by something else in future.

jsafrane · 2016-08-18T16:52:43Z

IMO, no admin action is needed. Alpha annotation is still working, for Beta it's just any other new feature, i.e. read (non-existing) docs.

jsafrane · 2016-08-18T16:53:21Z

@thockin, and thanks for the review!

k8s-bot · 2016-08-18T16:56:01Z

GCE e2e build/test passed for commit bb5d562.

k8s-github-robot · 2016-08-18T16:56:16Z

Automatic merge from submit-queue

wattsteve · 2016-08-18T16:59:30Z

W00t! Nice work @jsafrane and @kubernetes/sig-storage !

jingxu97 · 2016-08-19T18:17:54Z

pkg/cloudprovider/providers/aws/aws.go

+			iops = 100
+		}
+		if iops > 20000 {
+			iops = 20000


why those numbers are hardcoded? Isn't it better to put constants somewhere?
What if they change in the future?

Actually, they already changed :-)

Filled #31115 with constants + a link to AWS docs.

Automatic merge from submit-queue Add encryption to EBS dynamic provisioner Resolves #30792 Adds encryption to the EBS cloud provider and provisioner. Follow up to #29006 (all commits but the one in this PR will drop out). @kubernetes/sig-storage ```release-note ```

@colemickens

Automatic merge from submit-queue support Azure data disk volume This is a WIP of supporting azure data disk volume. Will add test and dynamic provisioning support once #29006 is merged replace #25915 fix #23259 @kubernetes/sig-storage @colemickens @brendandburns

@jsafrane

…oning Automatic merge from submit-queue Dynamic provisioning for flocker volume plugin Refactor flocker volume plugin * [x] Support provisioning beta (#29006) * [x] Support deletion * [x] Use bind mounts instead of /flocker in containers * [x] support ownership management or SELinux relabeling. * [x] adds volume specification via datasetUUID (this is guranted to be unique) I based my refactor work to replicate pretty much GCE-PD behaviour **Related issues**: #29006 #26908 @jsafrane @mattbates @wallrj @wallnerryan

Automatic merge from submit-queue Implement dynamic provisioning (beta) of PersistentVolumes via StorageClass Implemented according to PR kubernetes#26908. There are several patches in this PR with one huge code regen inside. * Please review the API changes (the first patch) carefully, sometimes I don't know what the code is doing... * `PV.Spec.Class` and `PVC.Spec.Class` is not implemented, use annotation `volume.alpha.kubernetes.io/storage-class` * See e2e test and integration test changes - Kubernetes won't provision a thing without explicit configuration of at least one `StorageClass` instance! * Multiple provisioning volume plugins can coexist together, e.g. HostPath and AWS EBS. This is important for Gluster and RBD provisioners in kubernetes#25026 * Contradicting the proposal, `claim.Selector` and `volume.alpha.kubernetes.io/storage-class` annotation are **not** mutually exclusive. They're both used for matching existing PVs. However, only `volume.alpha.kubernetes.io/storage-class` is used for provisioning, configuration of provisioning with `Selector` is left for (near) future. * Documentation is missing. Can please someone write some while I am out? For now, AWS volume plugin accepts classes with these parameters: ``` kind: StorageClass metadata: name: slow provisionerType: kubernetes.io/aws-ebs provisionerParameters: type: io1 zone: us-east-1d iopsPerGB: 10 ``` * parameters are case-insensitive * `type`: `io1`, `gp2`, `sc1`, `st1`. See AWS docs for details * `iopsPerGB`: only for `io1` volumes. I/O operations per second per GiB. AWS volume plugin multiplies this with size of requested volume to compute IOPS of the volume and caps it at 20 000 IOPS (maximum supported by AWS, see AWS docs). * of course, the plugin will use some defaults when a parameter is omitted in a `StorageClass` instance (`gp2` in the same zone as in 1.3). GCE: ``` apiVersion: extensions/v1beta1 kind: StorageClass metadata: name: slow provisionerType: kubernetes.io/gce-pd provisionerParameters: type: pd-standard zone: us-central1-a ``` * `type`: `pd-standard` or `pd-ssd` * `zone`: GCE zone * of course, the plugin will use some defaults when a parameter is omitted in a `StorageClass` instance (SSD in the same zone as in 1.3 ?). No OpenStack/Cinder yet @kubernetes/sig-storage

jsafrane added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Jul 15, 2016

googlebot added the cla: yes label Jul 15, 2016

k8s-github-robot assigned lavalamp Jul 15, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 15, 2016

jsafrane force-pushed the dynprov2 branch from 4ceeca4 to f84855d Compare July 15, 2016 14:48

jsafrane force-pushed the dynprov2 branch from f84855d to 8c82c73 Compare July 15, 2016 14:52

jsafrane force-pushed the dynprov2 branch from 8c82c73 to dfdc26b Compare July 15, 2016 15:28

jsafrane force-pushed the dynprov2 branch from dfdc26b to c28e599 Compare July 15, 2016 15:56

matchstick self-assigned this Jul 15, 2016

matchstick added this to the v1.4 milestone Jul 15, 2016

matchstick added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jul 15, 2016

matchstick added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note-label-needed labels Jul 15, 2016

rootfs mentioned this pull request Jul 17, 2016

Add support for libStorage as a Kubernetes volume plugin. #28599

Closed

rootfs reviewed Jul 18, 2016
View reviewed changes

pmorie changed the title ~~Dynamic provisioning v2aplha~~ Dynamic provisioning v2alpha Jul 19, 2016

pmorie reviewed Jul 19, 2016
View reviewed changes

justinsb reviewed Aug 18, 2016
View reviewed changes

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 18, 2016

thockin changed the title ~~Dynamic provisioning v2beta~~ Implement dynamic provisioning (beta) of PersistentVolumes via StorageClass Aug 18, 2016

thockin removed the kind/design Categorizes issue or PR as related to design. label Aug 18, 2016

k8s-github-robot added the kind/design Categorizes issue or PR as related to design. label Aug 18, 2016

jsafrane added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Aug 18, 2016

k8s-github-robot merged commit 9d2a5fe into kubernetes:master Aug 18, 2016

simonswine mentioned this pull request Aug 19, 2016

Dynamic provisioning for flocker volume plugin #31005

Merged

5 tasks

jingxu97 reviewed Aug 19, 2016
View reviewed changes

wongma7 mentioned this pull request Aug 19, 2016

Add section on storage classes to PV doc kubernetes/website#1064

Merged

wallrj mentioned this pull request Oct 14, 2016

Flocker volume driver should use StorageClass parameters to determine the Flocker Storage profile of dynamically provisioned volumes #34810

Closed

verult mentioned this pull request Sep 12, 2017

Fix failure to load volume plugins for #52048 #52049

Merged

LittleChimera mentioned this pull request May 21, 2021

[WIP] Set default storage class on PVC sync if default StorageClass is present #100031

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement dynamic provisioning (beta) of PersistentVolumes via StorageClass #29006

Implement dynamic provisioning (beta) of PersistentVolumes via StorageClass #29006

jsafrane commented Jul 15, 2016 •

edited by thockin

Loading

jsafrane commented Jul 15, 2016

jsafrane commented Jul 15, 2016

jsafrane commented Jul 15, 2016

matchstick commented Jul 15, 2016

jsafrane commented Jul 15, 2016

rootfs Jul 18, 2016

childsb Jul 19, 2016

wattsteve Jul 19, 2016

rootfs Jul 19, 2016

colemickens Jul 19, 2016

pmorie Jul 19, 2016

rootfs Jul 19, 2016

childsb Jul 21, 2016

thockin Jul 26, 2016

thockin Jul 26, 2016

rootfs commented Jul 19, 2016

pmorie Jul 19, 2016

thockin Jul 26, 2016

justinsb Aug 18, 2016

justinsb commented Aug 18, 2016

thockin commented Aug 18, 2016

thockin commented Aug 18, 2016

thockin commented Aug 18, 2016

thockin commented Aug 18, 2016

k8s-github-robot commented Aug 18, 2016

jsafrane commented Aug 18, 2016

jsafrane commented Aug 18, 2016

jsafrane commented Aug 18, 2016

k8s-bot commented Aug 18, 2016

k8s-github-robot commented Aug 18, 2016

wattsteve commented Aug 18, 2016

jingxu97 Aug 19, 2016

jsafrane Aug 22, 2016

Implement dynamic provisioning (beta) of PersistentVolumes via StorageClass #29006

Implement dynamic provisioning (beta) of PersistentVolumes via StorageClass #29006

Conversation

jsafrane commented Jul 15, 2016 • edited by thockin Loading

jsafrane commented Jul 15, 2016

jsafrane commented Jul 15, 2016

jsafrane commented Jul 15, 2016

matchstick commented Jul 15, 2016

jsafrane commented Jul 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rootfs commented Jul 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinsb commented Aug 18, 2016

thockin commented Aug 18, 2016

thockin commented Aug 18, 2016

thockin commented Aug 18, 2016

thockin commented Aug 18, 2016

k8s-github-robot commented Aug 18, 2016

jsafrane commented Aug 18, 2016

jsafrane commented Aug 18, 2016

jsafrane commented Aug 18, 2016

k8s-bot commented Aug 18, 2016

k8s-github-robot commented Aug 18, 2016

wattsteve commented Aug 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsafrane commented Jul 15, 2016 •

edited by thockin

Loading