propose combining domain name & group, remove group from versions #14522

lavalamp · 2015-09-24T21:02:20Z

I think it's too many levels to have separate domain names and groups. This will regularize URLs for accessing 3rd party objects and any other object in our system.

k8s-bot · 2015-09-24T21:32:58Z

GCE e2e build/test passed for commit 0a72b09c9a44597d00f58b54a29a632c15edc71b.

k8s-github-robot · 2015-09-24T22:03:31Z

Labelling this PR as size/S

smarterclayton · 2015-09-25T14:07:56Z

I agree - was just having this conversation with @deads2k this morning. OpenShift would be "openshift.io/v1" for our primary API and we would expose other experimental API versions.

smarterclayton · 2015-09-25T14:08:42Z

Experimental should be experimental.k8s.io/v1 or similar

lavalamp · 2015-09-26T00:22:15Z

Yeah, we can talk about renaming all API groups to this format-- I think it probably makes sense, if slightly verbose.

lavalamp · 2015-09-26T00:24:37Z

Rebased.

k8s-bot · 2015-09-26T01:02:55Z

Unit, integration and GCE e2e test build/test passed for commit d6a72458a3ceb744272b655b377c9e078dfd6acf.

k8s-bot · 2015-09-28T16:11:40Z

Unit, integration and GCE e2e build/test failed for commit 91bad56e56dd2c1cebe06e1f0647f66bc7f27273.

lavalamp · 2015-09-29T00:12:03Z

@k8s-bot test this

k8s-bot · 2015-09-29T00:50:42Z

Unit, integration and GCE e2e test build/test passed for commit 91bad56e56dd2c1cebe06e1f0647f66bc7f27273.

lavalamp · 2015-09-29T21:25:07Z

@brendandburns ping?

brendandburns · 2015-10-01T03:13:57Z

Yeah, I guess this LGTM. I'm a little uncomfortable with this change because I think that it's important to be able to tie related APIs (e.g. v1, v2) in different apigroups (e.g. 'stable', 'experimental') together in the same object for introspection.

liggitt · 2015-10-01T03:22:22Z

this is no longer removing versions, right? can you update the PR title to reflect that?

k8s-github-robot · 2015-10-02T16:46:29Z

Automatic merge from submit-queue

lavalamp · 2015-10-02T17:46:54Z

Removing LGTM to give @bgrant0607 a chance to review

bgrant0607 · 2015-10-02T17:54:20Z

docs/design/extending-api.md

-versions:
-  - name: stable/v1
-  - name: experimental/v2
+version: v1


This makes it impossible to create multiple versions in the same group.

@lavalamp Was that intentional, since there is no conversion between versions?

I intended you to make another one of these entries for each version. I see I need to add the version to the name.

Hm, I guess actually allowing multiple versions is easier. I will add that back. But not multiple groups.

caesarxuchao · 2015-10-02T18:39:01Z

One nit, could you also fix the APIVersion here:

and get back:

{
   "apiVersion": "example.com/stable/v1",

lavalamp · 2015-10-02T19:36:09Z

@caesarxuchao that's not an error, the group is "stable.example.com" and the version is "v1".

lavalamp · 2015-10-02T19:38:04Z

@caesarxuchao sorry! I saw the error. I thought you were asking for it to be changed to that. Will fix.

lavalamp · 2015-10-02T19:39:59Z

Comments addressed, thanks for catching things.

lavalamp · 2015-10-02T19:44:33Z

And in response to @brendandburns

I think that it's important to be able to tie related APIs (e.g. v1, v2) in different apigroups (e.g. 'stable', 'experimental') together in the same object for introspection.

I think this is currently a non-goal, even for regular api groups. We explicitly have an upgrade path for different versions of the same object in the same api group, but not between apigroups. We haven't even talked about inter-group resource migration scenarios yet. I think we can safely let future-us worry about this one for these objects after we've figured it out for first class objects.

k8s-github-robot · 2015-10-02T19:57:14Z

Labelling this PR as size/M

k8s-bot · 2015-10-02T20:08:15Z

Unit, integration and GCE e2e test build/test passed for commit e08fad9298c90de1b651fc446388863ccd3996a8.

smarterclayton · 2015-10-02T20:09:55Z

Heh, that's something we have to go fix next week so we can ship
experimental APIs at a different rate than Kube :)

On Oct 2, 2015, at 3:44 PM, Daniel Smith notifications@github.com wrote:

And in response to @brendandburns https://github.com/brendandburns

I think that it's important to be able to tie related APIs (e.g. v1, v2) in
different apigroups (e.g. 'stable', 'experimental') together in the same
object for introspection.

I think this is currently a non-goal, even for regular api groups. We
explicitly have an upgrade path for different versions of the same object
in the same api group, but not between apigroups. We haven't even talked
about inter-group resource migration scenarios yet. I think we can safely
let future-us worry about this one for these objects after we've figured it
out for first class objects.

—
Reply to this email directly or view it on GitHub
#14522 (comment)
.

liggitt · 2015-10-02T20:47:17Z

I hope future-us is smart

bgrant0607 · 2015-10-02T21:37:46Z

docs/design/extending-api.md

 apiVersion: experimental/v1alpha1
 kind: ThirdPartyResource
 description: "A specification of a Pod to run on a cron style schedule"
 versions:
-  - name: stable/v1
-  - name: experimental/v2
+- v1


IIRC the nested struct was used in order to permit additional info to be added for each version in the future.

Another thing I noticed: there is an APIGroup field in versions, https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/experimental/types.go#L177

The comment says it specifies "the API group to add this object into, default 'experimental'." Does that make sense? Isn't the group already specified in "name: cron-tab.stable.example.com"?

@caesarxuchao: my change is mostly about declaring having a group there wrong :)

@bgrant0607 OK, I'll add it back.

then could you search and remove the reference to this field in the doc? Thank you.

The APIGroup this version should belong to.

We also need to delete that field from types.go, but that can be done after 1.1.

smarterclayton · 2015-10-02T21:40:40Z

Judging by past-us records....

On Oct 2, 2015, at 4:47 PM, Jordan Liggitt notifications@github.com wrote:

I hope future-us is smart

—
Reply to this email directly or view it on GitHub
#14522 (comment)
.

lavalamp · 2015-10-02T22:21:21Z

@smarterclayton

Heh, that's something we have to go fix next week so we can ship experimental APIs at a different rate than Kube :)

For now, I recommend just making an additional os-experimental group with the same SLO re: unexpected deprecations & deletions. :/

In the future we may want to start each group with an experimental version... but we still have some work to get there.

k8s-bot · 2015-10-02T23:08:49Z

Unit, integration and GCE e2e test build/test passed for commit 7d0989e7716a0e68a8b3d71b8e616adc050192e6.

bgrant0607 · 2015-10-03T05:55:42Z

docs/design/extending-api.md

-  - name: stable/v1
-  - name: experimental/v2
+- Name: v1
+- Name: v2


bgrant0607 · 2015-10-07T18:24:28Z

@brendandburns What is the current status of the ThirdPartyResource implementation? What works, what doesn't?

Also remove group from versions.

k8s-bot · 2015-10-07T23:28:04Z

GCE e2e test build/test passed for commit 72c1340.

bgrant0607 · 2015-10-08T20:20:15Z

Thanks. LGTM.

If ThirdPartyResource is a registered API, it more or less works according to the previous spec, and we don't make it conform to this spec, then we shouldn't cherrypick this.

If ThirdPartyResource doesn't really work, even according to the previous spec, then we should unregister it from the API.

If ThirdPartyResource isn't registered, then we should cherrypick this to let people know what our current thinking is.

bgrant0607 · 2015-10-08T20:21:32Z

Rerunning Shippable. It was an integration test flake.

k8s-github-robot · 2015-10-09T04:15:11Z

Automatic merge from submit-queue

Auto commit by PR queue bot

googlebot added the cla: yes label Sep 24, 2015

lavalamp assigned brendandburns Sep 24, 2015

erictune added the e2e-not-required label Sep 24, 2015

k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 24, 2015

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2015

lavalamp force-pushed the 3p-api-group branch from 0a72b09 to d6a7245 Compare September 26, 2015 00:24

lavalamp removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 26, 2015

lavalamp force-pushed the 3p-api-group branch from d6a7245 to 91bad56 Compare September 28, 2015 15:57

brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 1, 2015

lavalamp mentioned this pull request Oct 2, 2015

Support groups in third party resources #14438

Closed

lavalamp removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 2, 2015

bgrant0607 reviewed Oct 2, 2015
View reviewed changes

lavalamp changed the title ~~propose combining domain name & group, remove versions~~ propose combining domain name & group, remove group from versions Oct 2, 2015

lavalamp force-pushed the 3p-api-group branch from 91bad56 to e08fad9 Compare October 2, 2015 19:39

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 2, 2015

bgrant0607 reviewed Oct 2, 2015
View reviewed changes

lavalamp force-pushed the 3p-api-group branch from e08fad9 to 7d0989e Compare October 2, 2015 22:21

bgrant0607 reviewed Oct 3, 2015
View reviewed changes

bgrant0607 assigned bgrant0607 and unassigned brendandburns Oct 7, 2015

Propose combining domain name & group

72c1340

Also remove group from versions.

lavalamp force-pushed the 3p-api-group branch from 7d0989e to 72c1340 Compare October 7, 2015 23:01

bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 8, 2015

k8s-github-robot pushed a commit that referenced this pull request Oct 9, 2015

Merge pull request #14522 from lavalamp/3p-api-group

2192946

Auto commit by PR queue bot

k8s-github-robot merged commit 2192946 into kubernetes:master Oct 9, 2015

xingzhou pushed a commit to xingzhou/kubernetes that referenced this pull request Dec 15, 2016

Merge pull request kubernetes#14522 from lavalamp/3p-api-group

cea9f14

Auto commit by PR queue bot

lavalamp deleted the 3p-api-group branch September 1, 2020 22:57

propose combining domain name & group, remove group from versions #14522

propose combining domain name & group, remove group from versions #14522

Conversation

lavalamp commented Sep 24, 2015

k8s-bot commented Sep 24, 2015

k8s-github-robot commented Sep 24, 2015

smarterclayton commented Sep 25, 2015

smarterclayton commented Sep 25, 2015

lavalamp commented Sep 26, 2015

lavalamp commented Sep 26, 2015

k8s-bot commented Sep 26, 2015

k8s-bot commented Sep 28, 2015

lavalamp commented Sep 29, 2015

k8s-bot commented Sep 29, 2015

lavalamp commented Sep 29, 2015

brendandburns commented Oct 1, 2015

liggitt commented Oct 1, 2015

k8s-github-robot commented Oct 2, 2015

lavalamp commented Oct 2, 2015

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

caesarxuchao commented Oct 2, 2015

lavalamp commented Oct 2, 2015

lavalamp commented Oct 2, 2015

lavalamp commented Oct 2, 2015

lavalamp commented Oct 2, 2015

k8s-github-robot commented Oct 2, 2015

k8s-bot commented Oct 2, 2015

smarterclayton commented Oct 2, 2015

liggitt commented Oct 2, 2015

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

smarterclayton commented Oct 2, 2015

lavalamp commented Oct 2, 2015

k8s-bot commented Oct 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgrant0607 commented Oct 7, 2015

k8s-bot commented Oct 7, 2015

bgrant0607 commented Oct 8, 2015

bgrant0607 commented Oct 8, 2015

k8s-github-robot commented Oct 9, 2015