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

Publish CRD openapi #71192

Merged
merged 7 commits into from Mar 7, 2019

Conversation

@roycaihw
Copy link
Member

roycaihw commented Nov 19, 2018

Fixes #71159, #71142. Updated e2e test to exercise kubectl explain and client-side validation.

Special notes for your reviewer:
The first seven commits are from #67205 and #71137.

TODO:

Does this PR introduce a user-facing change?:

kube-apiserver now serves OpenAPI specs for registered CRDs with defined 
validation schemata as an alpha feature, to be enabled via the "CustomResourcePublishOpenAPI" feature gate. Kubectl will validate client-side using those. Note that in
future, client-side validation in 1.14 kubectl against a 1.15 cluster will reject
unknown fields for CRDs with validation schema defined. 

/sig api-machinery
/area custom-resources
cc @liggitt @mbohlool @smarterclayton @sttts

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Nov 20, 2018

I put together a summary of potential solutions for #71143. Please take a look and leave any comments/suggestions: https://docs.google.com/document/d/13lBj8Stdwku8BgL0fbT__4Iw97NRh77loJ_MoZuCGwQ/edit#

/test pull-kubernetes-integration

@roycaihw roycaihw force-pushed the roycaihw:crd-publish-openapi branch 3 times, most recently from 12846b5 to 16ff635 Nov 20, 2018

@nikhita nikhita referenced this pull request Nov 26, 2018

Open

Umbrella issue for CRDs moving to GA #58682

19 of 55 tasks complete
@sttts

This comment has been minimized.

Copy link
Contributor

sttts commented Jan 2, 2019

What is the roadmap for this feature? Which work is still open?

Do we have a feature issue in the feature tracking repo for this? We probably should.

@mingfang

This comment has been minimized.

Copy link

mingfang commented Jan 15, 2019

This didn't make it to the v1.14.0-alpha1 release.
How can we make progress on this?

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Jan 17, 2019

Do we have a feature issue in the feature tracking repo for this?

Created: kubernetes/enhancements#692

Which work is still open?

  • 1. For schema -> swagger, reuse more of kube-openapi builder rather than writing the entire spec constructor
  • 2. Wire up crd_openapi_controller in aggregator-apiserver instead of apiextensions-apiserver, to speed up publishing (experimented in #72979)
  • 3. For CRDs without schema, construct the swagger in a way that allows any unknown property

@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 6, 2019

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Mar 6, 2019

@liggitt updated

/test pull-kubernetes-e2e-gce-alpha-features

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Mar 6, 2019

removes definition from spec when one versin gets changed to not be served green in https://gubernator.k8s.io/build/kubernetes-jenkins/pr-logs/pull/71192/pull-kubernetes-e2e-gce-alpha-features/127

/retest

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Mar 6, 2019

lgtm, squash in the review comments commit, then will tag

apiextensions-apiserver: serve openapi spec
Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>

@roycaihw roycaihw force-pushed the roycaihw:crd-publish-openapi branch from 64d66a4 to 5d0ba35 Mar 7, 2019

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Mar 7, 2019

/test pull-kubernetes-e2e-gce-alpha-features

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Mar 7, 2019

/retest

roycaihw and others added some commits Mar 7, 2019

kube-aggregator: periodically resync local specs
Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
apiextensions: add openapi publishing unit+integration+e2e tests
Co-authored-by: Maciej Szulik <maszulik@redhat.com>
Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>

@roycaihw roycaihw force-pushed the roycaihw:crd-publish-openapi branch from 5d0ba35 to 54b9941 Mar 7, 2019

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Mar 7, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Mar 7, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 7, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, roycaihw, sttts

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

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Mar 7, 2019

/retest

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 7, 2019

@roycaihw: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 2c5e191 link /test pull-kubernetes-e2e-kops-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@roycaihw

This comment has been minimized.

Copy link
Member Author

roycaihw commented Mar 7, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 0b48018 into kubernetes:master Mar 7, 2019

17 checks passed

cla/linuxfoundation roycaihw authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
@mingfang

This comment has been minimized.

Copy link

mingfang commented Mar 7, 2019

@roycaihw Thank you very much. This is a critical feature for me.

@mariantalla

This comment has been minimized.

Copy link
Contributor

mariantalla commented Mar 8, 2019

Hello, this PR may be related to this failing test: #75125 (it started failing right after this PR was merged). Could you have a look please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.