Add API for StorageClasses #29694

childsb · 2016-07-27T18:53:48Z

This is the API objects only required for dynamic provisioning picked apart from the controller logic.

Entire feature is here: #29006

pmorie · 2016-07-27T19:45:11Z

@childsb test-cmd.sh changes and integration test for REST api should be in this PR too.

childsb · 2016-07-27T19:51:26Z

i'll add them, thanks @pmorie

thockin · 2016-07-28T05:39:20Z

pkg/apis/extensions/v1beta1/types.go

+	v1.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
+
+	// Provisioner indicates the type of the provisioner.
+	Provisioner string `json:"provisioner,omitempty" protobuf:"bytes,2,opt,name=provisioner"`


should not be omitempty

thockin · 2016-07-28T05:44:50Z

API parts LGTM except omitempty. Adding @wojtek-t to take a glance at the registry logic and @krousey for the client (please delegate as needed).

wojtek-t · 2016-07-28T20:01:21Z

pkg/registry/storageclass/etcd/etcd.go

+		ObjectNameFunc: func(obj runtime.Object) (string, error) {
+			return obj.(*extensions.StorageClass).Name, nil
+		},
+		PredicateFunc: func(label labels.Selector, field fields.Selector) generic.Matcher {


PredicateFunc: storageclass.MatchStorageClasses,

wojtek-t · 2016-07-28T20:04:13Z

I looked into:

pkg/master
pkg/registry
test/integration
files.

I added one nit, other than that lgtm.

pmorie · 2016-07-28T22:00:10Z

test/integration/storageclasses/storage_classes_test.go

+
+	client := client.NewOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}})
+
+	ns := framework.CreateTestingNamespace("storageclass", s, t)


You actually do not need to create a namespace for this test -- storage class is a global resource.

The namespace is used for a PVC in the test.

thockin · 2016-07-28T22:54:46Z

I am LGTM on this. Paul? If you're good, go ahead and apply the label.

wojtek-t · 2016-07-28T22:57:34Z

@childsb - please squash commits before merging

childsb · 2016-07-28T23:04:06Z

Squashed and ready to go. Thanks @wojtek-t @pmorie @thockin

k8s-bot · 2016-07-28T23:34:35Z

GCE e2e build/test passed for commit f5bd7d4.

pmorie · 2016-07-29T02:17:40Z

LGTM.

k8s-github-robot · 2016-07-29T04:26:55Z

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

k8s-bot · 2016-07-29T04:59:06Z

GCE e2e build/test passed for commit f5bd7d4.

k8s-github-robot · 2016-07-29T05:00:05Z

Automatic merge from submit-queue

Automatic merge from submit-queue make reousrce prefix consistent with other registries Storage class registry was added recently in #29694. In other registries, resource prefix was changed to a more generic way using RESTOptions.ResourcePrefix. This PR is an attempt to make both consistent.

deads2k · 2016-08-26T14:21:37Z

@childsb @lavalamp @kubernetes/sig-api-machinery Why did these types end up in extensions? I thought that when we started reslicing groups for things like autoscaling and batch, we had agreed that we were going make new API groups for new categories and extensions should probably end up holding only thirdpartyresources.

We tagged an alpha of 1.4, but I think we should consider moving these. I suspect they'll want new versions at a cadence to match the storage controllers, not a cadence locked to the rest of this API.

thockin · 2016-08-26T16:23:16Z

Brian and I discussed this at length. The idea is that we need to be able
to "incubate" new Kinds. ThirdParty is insufficient for Beta because we
need to experience validation and defaulting and admission control and ...
We don't want to add beta object to a non-beta API (not applicable here,
but in general) and we don't want to rev the whole api group just to add a
beta object.

So we agreed we need an incubator group for beta types. We already have
extensions, so...

@bgrant0607

On Fri, Aug 26, 2016 at 7:22 AM, David Eads notifications@github.com
wrote:

@childsb https://github.com/childsb @lavalamp
https://github.com/lavalamp @kubernetes/sig-api-machinery
https://github.com/orgs/kubernetes/teams/sig-api-machinery Why did
these types end up in extensions? I thought that when we started
reslicing groups for things like autoscaling and batch, we had agreed
that we were going make new API groups for new categories and extensions
should probably end up holding only thirdpartyresources.

We tagged an alpha of 1.4, but I think we should consider moving these. I
suspect they'll want new versions at a cadence to match the storage
controllers, not a cadence locked to the rest of this API.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29694 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFVgVByGGWR6WrHCBwSy_WjT_m3z0bjxks5qjvalgaJpZM4JWgnZ
.

deads2k · 2016-08-26T16:41:34Z

Brian and I discussed this at length. The idea is that we need to be able
to "incubate" new Kinds. ThirdParty is insufficient for Beta because we
need to experience validation and defaulting and admission control and ...
We don't want to add beta object to a non-beta API (not applicable here,
but in general) and we don't want to rev the whole api group just to add a
beta object.

Why couldn't a new kind be incubated in its own API group which can be separately versioned so that when you're done or if you want to make changes, you can do so at your own cadence? Adding API objects to extensions has effectively locked the version of this object. You can't change it in a breaking way at all. If you had your own API group, you'd be able to.

Doing it like this has not only locked this object immediately (no breaking changes for an incubating type?), but you'll now experience migration pain as you try to move it out of incubation to its own group.

deads2k · 2016-08-26T16:42:04Z

@thockin also, I opened #31521 so we don't have to keep bumping a dead thread.

thockin · 2016-08-26T16:57:11Z

On Fri, Aug 26, 2016 at 9:42 AM, David Eads notifications@github.com wrote:

Brian and I discussed this at length. The idea is that we need to be able
to "incubate" new Kinds. ThirdParty is insufficient for Beta because we
need to experience validation and defaulting and admission control and ...
We don't want to add beta object to a non-beta API (not applicable here,
but in general) and we don't want to rev the whole api group just to add a
beta object.

Why couldn't a new kind be incubated in its own API group which can be separately versioned so that when you're done or if you want to make changes, you can do so at your own cadence? Adding API objects to extensions has effectively locked the version of this object. You can't change it in a breaking way at all. If you had your own API group, you'd be able to.

In this particular case, we could make a new group. In general we
will sometimes want to add a new beta Kind to an extant v1 group. We
are not going to revive v1beta1 for the group, nor do we want to bump
to v2beta1 for every new Kind. So we need a place to grow things.

Doing it like this has not only locked this object immediately (no breaking changes for an incubating type?), but you'll now experience migration pain as you try to move it out of incubation to its own group.

We need an answer to this whole problem in general. I am not happy
with the way apigroups has turned out - it's far less flexible than I
think we wanted, and far too cumbersome to use.

deads2k · 2016-08-26T17:22:18Z

In this particular case, we could make a new group. In general we
will sometimes want to add a new beta Kind to an extant v1 group. We
are not going to revive v1beta1 for the group, nor do we want to bump
to v2beta1 for every new Kind. So we need a place to grow things.

My objection isn't to adding a type to an existing version of an API group. I'm concerned about using this particular group as a catchall for "I'm going to want a new API group, but this is a convenient place at the moment for a type that doesn't belong in this group in the end".

thockin · 2016-08-26T17:24:38Z

so if we had "incubator" instead of "extensions" you'd be less unhappy?

On Fri, Aug 26, 2016 at 10:23 AM, David Eads notifications@github.com
wrote:

In this particular case, we could make a new group. In general we
will sometimes want to add a new beta Kind to an extant v1 group. We
are not going to revive v1beta1 for the group, nor do we want to bump
to v2beta1 for every new Kind. So we need a place to grow things.

My objection isn't to adding a type to an existing version of an API
group. I'm concerned about using this particular group as a catchall
for "I'm going to want a new API group, but this is a convenient place at
the moment for a type that doesn't belong in this group in the end".

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29694 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFVgVKSfPbkMWrU8otfXfkHZt0agHx7Rks5qjyD9gaJpZM4JWgnZ
.

lavalamp · 2016-08-26T23:15:27Z

IMO extensions is a bit of an incubator, since we turn resources on and off
individually. Or at least we should be. I agree that extensions should not
be the permanent home for new api types.

On Fri, Aug 26, 2016 at 9:57 AM, Tim Hockin notifications@github.com
wrote:

On Fri, Aug 26, 2016 at 9:42 AM, David Eads notifications@github.com
wrote:

Brian and I discussed this at length. The idea is that we need to be able
to "incubate" new Kinds. ThirdParty is insufficient for Beta because we
need to experience validation and defaulting and admission control and
...
We don't want to add beta object to a non-beta API (not applicable here,
but in general) and we don't want to rev the whole api group just to add
a
beta object.

Why couldn't a new kind be incubated in its own API group which can be
separately versioned so that when you're done or if you want to make
changes, you can do so at your own cadence? Adding API objects to
extensions has effectively locked the version of this object. You can't
change it in a breaking way at all. If you had your own API group, you'd be
able to.

In this particular case, we could make a new group. In general we
will sometimes want to add a new beta Kind to an extant v1 group. We
are not going to revive v1beta1 for the group, nor do we want to bump
to v2beta1 for every new Kind. So we need a place to grow things.

Doing it like this has not only locked this object immediately (no
breaking changes for an incubating type?), but you'll now experience
migration pain as you try to move it out of incubation to its own group.

We need an answer to this whole problem in general. I am not happy
with the way apigroups has turned out - it's far less flexible than I
think we wanted, and far too cumbersome to use.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29694 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAngltRxYbfzT8ANktyGHi56hvIPVwAkks5qjxrxgaJpZM4JWgnZ
.

googlebot added the cla: yes label Jul 27, 2016

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

childsb added this to the v1.4 milestone Jul 27, 2016

childsb assigned thockin Jul 27, 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 27, 2016

k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. retest-not-required-docs-only and removed retest-not-required-docs-only labels Jul 28, 2016

thockin reviewed Jul 28, 2016
View reviewed changes

thockin added release-note-none Denotes a PR that doesn't merit a release note. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 28, 2016

thockin changed the title ~~API Changes for StorageClasses~~ Add API for StorageClasses Jul 28, 2016

thockin removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Jul 28, 2016

thockin assigned krousey and wojtek-t Jul 28, 2016

wojtek-t reviewed Jul 28, 2016
View reviewed changes

pmorie reviewed Jul 28, 2016
View reviewed changes

API Changes for StorageClass

f5bd7d4

childsb force-pushed the dynprov2-apionly branch from b666892 to f5bd7d4 Compare July 28, 2016 23:02

pmorie self-assigned this Jul 29, 2016

pmorie added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 29, 2016

k8s-github-robot merged commit 7abc3de into kubernetes:master Jul 29, 2016

jsafrane mentioned this pull request Aug 2, 2016

UPSTREAM: 29006: Dynamic provisioning v2beta openshift/origin#10160

Closed

hongchaodeng mentioned this pull request Aug 3, 2016

make reousrce prefix consistent with other registries #30020

Merged

deads2k mentioned this pull request Aug 26, 2016

StorageClass should not be in the extensions group #31521

Closed

deads2k mentioned this pull request Sep 2, 2016

Move StorageClass to a storage group #31886

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API for StorageClasses #29694

Add API for StorageClasses #29694

childsb commented Jul 27, 2016

pmorie commented Jul 27, 2016

childsb commented Jul 27, 2016

thockin Jul 28, 2016

thockin commented Jul 28, 2016

wojtek-t Jul 28, 2016

wojtek-t commented Jul 28, 2016

pmorie Jul 28, 2016

childsb Jul 28, 2016

thockin commented Jul 28, 2016

wojtek-t commented Jul 28, 2016

childsb commented Jul 28, 2016

k8s-bot commented Jul 28, 2016

pmorie commented Jul 29, 2016

k8s-github-robot commented Jul 29, 2016

k8s-bot commented Jul 29, 2016

k8s-github-robot commented Jul 29, 2016

deads2k commented Aug 26, 2016

thockin commented Aug 26, 2016

deads2k commented Aug 26, 2016

deads2k commented Aug 26, 2016

thockin commented Aug 26, 2016

deads2k commented Aug 26, 2016

thockin commented Aug 26, 2016

lavalamp commented Aug 26, 2016


		client := client.NewOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}})

		ns := framework.CreateTestingNamespace("storageclass", s, t)

Add API for StorageClasses #29694

Add API for StorageClasses #29694

Conversation

childsb commented Jul 27, 2016

pmorie commented Jul 27, 2016

childsb commented Jul 27, 2016

thockin Jul 28, 2016

Choose a reason for hiding this comment

thockin commented Jul 28, 2016

wojtek-t Jul 28, 2016

Choose a reason for hiding this comment

wojtek-t commented Jul 28, 2016

pmorie Jul 28, 2016

Choose a reason for hiding this comment

childsb Jul 28, 2016

Choose a reason for hiding this comment

thockin commented Jul 28, 2016

wojtek-t commented Jul 28, 2016

childsb commented Jul 28, 2016

k8s-bot commented Jul 28, 2016

pmorie commented Jul 29, 2016

k8s-github-robot commented Jul 29, 2016

k8s-bot commented Jul 29, 2016

k8s-github-robot commented Jul 29, 2016

deads2k commented Aug 26, 2016

thockin commented Aug 26, 2016

deads2k commented Aug 26, 2016

deads2k commented Aug 26, 2016

thockin commented Aug 26, 2016

deads2k commented Aug 26, 2016

thockin commented Aug 26, 2016

lavalamp commented Aug 26, 2016