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

sig-api-machinery: add "Graduate CustomResourceDefinitions to GA" #990

Merged
merged 1 commit into from
Apr 26, 2019

Conversation

jpbetz
Copy link
Contributor

@jpbetz jpbetz commented Apr 23, 2019

We'd like to graduate CRDs sub-features to beta in 1.15 and then graduate CRDs to GA in a following release, ideally 1.16.

The purpose of this KEP is to establish consensus for the remaining essential improvements needed to move CRDs to GA. In particular, we'd like to agree on the features and tests required.

A couple sections, particularly the test and scale goals sections, are not complete. Feedback on what we should include in those sections would be very helpful.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Apr 23, 2019
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 23, 2019
@jpbetz jpbetz force-pushed the extensibility-ga-kep branch 5 times, most recently from c3a91bc to 69a5269 Compare April 24, 2019 19:43
@jpbetz
Copy link
Contributor Author

jpbetz commented Apr 24, 2019

@lavalamp @deads2k, @liggitt @sttts @mbohlool @roycaihw Could we get a first past review? We'd like to get this in for the 1.15 KEP deadline on 4/30 since the Conversion webhooks to beta part of this targets 1.15. Note that we can continue to iterate on the other sections of this afterward since we're targeting 1.16 for CRDs to GA.

@jpbetz jpbetz force-pushed the extensibility-ga-kep branch from 69a5269 to 987c3e9 Compare April 24, 2019 19:56

See [umbrella issue](https://github.com/kubernetes/kubernetes/issues/58682) for status.

## Post-GA tasks
Copy link
Contributor

Choose a reason for hiding this comment

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

"Potential Post-GA tasks"

Some of them below are far from consensus or good enough understanding. We won't decide anything about those in this KEP.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should not try to list them comprehensibly at all or give detail below. A link to an umbrella github issue is enough IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I've replaced this section with a link to the umbrella issue where readers can find an up-to-date post GA task list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-added this per @lavalamp's request. Added a note that the list is a snapshot from when KEP was written and to see the umbrella issue for current status.

Copy link
Contributor

Choose a reason for hiding this comment

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

looks good.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the umbrella issue is not a good historical record. The purpose of this section is to inform people that we didn't forget about these things, we just decided they don't block v1 GA--you might want to make that a bit clearer in the text in a followup.

@jpbetz jpbetz force-pushed the extensibility-ga-kep branch from 987c3e9 to 6c53cc1 Compare April 24, 2019 21:01
keps/sig-api-machinery/20180415-crds-to-ga.md Outdated Show resolved Hide resolved
keps/sig-api-machinery/20180415-crds-to-ga.md Outdated Show resolved Hide resolved
keps/sig-api-machinery/20180415-crds-to-ga.md Outdated Show resolved Hide resolved
@jpbetz jpbetz force-pushed the extensibility-ga-kep branch from 6c53cc1 to 802b920 Compare April 25, 2019 04:11

See the [umbrella issue](https://github.com/kubernetes/kubernetes/issues/58682) for details on Post-GA tasks.

### Arbitrary subresources as Post-GA task
Copy link
Contributor

Choose a reason for hiding this comment

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

as nice as this is as a sketch, I would remove it from this KEP. This topic will move into its own KEP as soon as somebody seriously works on it. We have decided to do this post-GA though. So no need here yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. We wanted to think through the basics to make sure CRD API v1 would be compatible with adding arbitrary subresources later. But I agree that it doesn't need to be here in the KEP. We can discuss the details with those interested separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could start a gdoc at some point to work towards a 1.16 KEP for that.

@jpbetz jpbetz force-pushed the extensibility-ga-kep branch from 802b920 to 20096f0 Compare April 25, 2019 16:46
Copy link
Member

@lavalamp lavalamp left a comment

Choose a reason for hiding this comment

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

I think this is a fine start, but we need to work out all the TODOs before flipping it to implementable.

Currently custom resource subresources are v1beta1 and provide support for the
/status and /scale subresources. We plan to graduate this to GA as part of the
CRD to GA graduation. See [Arbitrary subresources as Post-GA
task](#arbitrary-subresources-as-post-ga-task) details on post GA tasks.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just call out arbitrary subresources as a non-goal / post-v1 task?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I've re-added the inline list of post GA tasks back to this document including this one.


TODO: complete this list

* Ensure v1beta1 and v1 CRDs are round trippable
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't sound like an e2e test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to "Custom resources should be readable and writable at all available versions (test for round trip-ability)"

keps/sig-api-machinery/20180415-crds-to-ga.md Outdated Show resolved Hide resolved
keps/sig-api-machinery/20180415-crds-to-ga.md Outdated Show resolved Hide resolved
@jpbetz jpbetz force-pushed the extensibility-ga-kep branch from 20096f0 to e6553ed Compare April 26, 2019 05:29
@jpbetz
Copy link
Contributor Author

jpbetz commented Apr 26, 2019

Thanks for the review @lavalamp. If it's okay, we'd like to merge this and finish the remaining TODOs via separate PRs, and then get 'implementable' approval after that.

I've opened #1004 to track CRD webhook conversion items that need to go into 1.15 separate from this KEP. So now this KEP should be targeting 1.16 only and doesn't need to be rushed.

@k8s-ci-robot
Copy link
Contributor

@jpbetz: You must be a member of the kubernetes/kubernetes-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone 1.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.


### CRD v1 schemas are restricted to a subset of the OpenAPI specification

See [Vanilla OpenAPI Subset Design](https://docs.google.com/document/d/1pcGlbmw-2Y0JJs9hsYnSBXamgG9TfWtHY6eh80zSTd8)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is #1002 now.

OpenAPI subset definition (next point). An update of the [old Pruning for
CustomResources KEP](https://github.com/kubernetes/enhancements/pull/709) and the implementation
([pruning PR](https://github.com/kubernetes/kubernetes/pull/64558), [defaulting
PR](https://github.com/kubernetes/kubernetes/pull/63604)), are follow-ups as soon as unblocked.
Copy link
Contributor

Choose a reason for hiding this comment

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

defaulting is KEP #1006 now


* See “Required for GA” issues tracked via the [CRD Project Board](https://github.com/orgs/kubernetes/projects/28).

For additional details on already completed features, see the [Umbrella Issue](https://github.com/kubernetes/kubernetes/issues/58682).
Copy link
Member

Choose a reason for hiding this comment

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

We need to spell these out in a followup, at least a list with bullet points; issues are mutable and therefore not a good historical reference. This document needs to collect what needed to be done at the time it was written.

@lavalamp
Copy link
Member

I'm OK with merging and building on this. Please fix the links @sttts pointed out in a followup :)

The TODOs need to TODONEs before this can be implementable, I think.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 26, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz, lavalamp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 26, 2019
@k8s-ci-robot k8s-ci-robot merged commit bd4c314 into kubernetes:master Apr 26, 2019
* Support proto encoding for custom resources (https://github.com/kubernetes/kubernetes/issues/63677)
* labelSelectorPath should be allowed not be under .status (https://github.com/kubernetes/kubernetes/issues/66688)
* Support arbitrary non-standard subresources for CRDs (https://github.com/kubernetes/kubernetes/issues/72637)
* OpenAPI v3 is published for all resources, including custom resources
Copy link
Contributor

Choose a reason for hiding this comment

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

We have found some client issues dealing with unexpected openapi conditions during our alpha stage. I'm concerned about promoting to GA without seeing this be functional because we're already talking about stripping back the set of supported features and I don't want to make GA objects invalid.

Copy link
Member

@liggitt liggitt Apr 26, 2019

Choose a reason for hiding this comment

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

This is talking about publishing openapi v3, which we don't do for any resources today, including in-tree ones.

The openapi subset KEP is intended to ensure we limit ourselves to what can be published for openapi v3, but exposing openapi/v3 endpoints is a distinct effort

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants