Skip to content

Publish CRD openapi#71192

Merged
k8s-ci-robot merged 7 commits intokubernetes:masterfrom
roycaihw:crd-publish-openapi
Mar 7, 2019
Merged

Publish CRD openapi#71192
k8s-ci-robot merged 7 commits intokubernetes:masterfrom
roycaihw:crd-publish-openapi

Conversation

@roycaihw
Copy link
Copy Markdown
Member

@roycaihw roycaihw commented Nov 19, 2018 •
edited by sttts
Loading

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

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/custom-resources needs-kind Indicates a PR lacks a `kind/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/apiserver sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Nov 19, 2018
@roycaihw roycaihw force-pushed the crd-publish-openapi branch from 9ca76e0 to 9289808 Compare November 19, 2018 03:24
@roycaihw
Copy link
Copy Markdown
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

This was referenced Nov 20, 2018
Merged
Closed
smarterclayton
smarterclayton requested changes Nov 20, 2018
View reviewed changes
@roycaihw roycaihw force-pushed the crd-publish-openapi branch 3 times, most recently from 12846b5 to 16ff635 Compare November 20, 2018 23:55
@nikhita nikhita mentioned this pull request Nov 26, 2018
Closed
54 tasks
@sttts
Copy link
Copy Markdown
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.

@ahmetb ahmetb mentioned this pull request Jan 2, 2019
Closed
liggitt
liggitt reviewed Jan 8, 2019
View reviewed changes
@MrHohn MrHohn mentioned this pull request Jan 9, 2019
Closed
@mingfang
Copy link
Copy Markdown

mingfang commented Jan 15, 2019

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

This was referenced Jan 16, 2019
Closed
Closed
@roycaihw
Copy link
Copy Markdown
Member Author

roycaihw commented Jan 17, 2019 •
edited
Loading

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 Experimental: publish CRD openapi schema quickly #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 "Looks good to me", indicates that a PR is ready to be merged. label Mar 6, 2019
@roycaihw
Copy link
Copy Markdown
Member Author

roycaihw commented Mar 6, 2019

@liggitt updated

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

@roycaihw
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Member

liggitt commented Mar 6, 2019

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

@roycaihw @sttts
apiextensions-apiserver: serve openapi spec
11cf310 
Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
@roycaihw roycaihw force-pushed the crd-publish-openapi branch from 64d66a4 to 5d0ba35 Compare March 7, 2019 00:35
@roycaihw
Copy link
Copy Markdown
Member Author

roycaihw commented Mar 7, 2019

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

@roycaihw
Copy link
Copy Markdown
Member Author

roycaihw commented Mar 7, 2019

/retest

roycaihw and others added 4 commits March 6, 2019 20:35
@roycaihw
apiextensions-apiserver: crd openapi spec builder
0ba4050
@roycaihw @sttts
kube-aggregator: periodically resync local specs
3079798 
Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
@roycaihw @soltysh @sttts
apiextensions: add openapi publishing unit+integration+e2e tests
867cdce 
Co-authored-by: Maciej Szulik <maszulik@redhat.com>
Co-authored-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
@roycaihw
generated
54b9941
@liggitt
Copy link
Copy Markdown
Member

liggitt commented Mar 7, 2019

/lgtm
/approve

@k8s-ci-robot
Copy link
Copy Markdown
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

Details 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
Copy link
Copy Markdown
Member Author

roycaihw commented Mar 7, 2019

/retest

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 7, 2019 •
edited
Loading

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

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 2c5e191b5af12bd65a93204afe5981a65580bcd8 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.

Details

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
Copy link
Copy Markdown
Member Author

roycaihw commented Mar 7, 2019

/retest

@mingfang
Copy link
Copy Markdown

mingfang commented Mar 7, 2019

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

@mariantalla
Copy link
Copy Markdown
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?

@mariantalla mariantalla mentioned this pull request Mar 8, 2019
Closed
@pmorie pmorie mentioned this pull request Mar 12, 2019
Merged
@kanatohodets kanatohodets mentioned this pull request Mar 13, 2019
Closed
This was referenced Mar 26, 2019
Closed
Closed
Closed
@roycaihw roycaihw mentioned this pull request Apr 4, 2019
Merged
@roycaihw roycaihw mentioned this pull request Apr 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soltysh soltysh soltysh requested changes

@sttts sttts sttts left review comments

@liggitt liggitt liggitt left review comments

@smarterclayton smarterclayton smarterclayton requested changes

@caesarxuchao caesarxuchao Awaiting requested review from caesarxuchao

@lavalamp lavalamp Awaiting requested review from lavalamp

+2 more reviewers

@mingfang mingfang mingfang left review comments

@mbohlool mbohlool mbohlool left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@soltysh soltysh

@sttts sttts

@liggitt liggitt

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/custom-resources area/kubectl area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

v1.14

Development

Successfully merging this pull request may close these issues.