From c1f8dd3a48172aaa779508918448392279243e1b Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Wed, 14 Sep 2022 14:27:33 -0700 Subject: [PATCH 01/23] add kubectl explain: openapi v3 upgrade kep --- .../3515-kubectl-explain-openapiv3/README.md | 862 ++++++++++++++++++ .../3515-kubectl-explain-openapiv3/kep.yaml | 23 + 2 files changed, 885 insertions(+) create mode 100644 keps/sig-cli/3515-kubectl-explain-openapiv3/README.md create mode 100755 keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md new file mode 100644 index 00000000000..848a9f40b41 --- /dev/null +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -0,0 +1,862 @@ + +# KEP-3515: OpenAPI v3 for kubectl explain + + + + + + +- [Release Signoff Checklist](#release-signoff-checklist) +- [Summary](#summary) +- [Motivation](#motivation) + - [Richer OpenAPI V3 Data](#richer-openapi-v3-data) + - [CRD schemas expressed as OpenAPI v2 are lossy](#crd-schemas-expressed-as-openapi-v2-are-lossy) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) + - [Basic Usage](#basic-usage) + - [Built-in Template Options](#built-in-template-options) + - [Custom Templates](#custom-templates) + - [Example template](#example-template) + - [Risks and Mitigations](#risks-and-mitigations) + - [OpenAPI V3 Not Available](#openapi-v3-not-available) + - [Risk](#risk) + - [Mitigation](#mitigation) + - [Template API Stability](#template-api-stability) + - [Risk](#risk-1) + - [Mitigation](#mitigation-1) + - [OpenAPI serialization time](#openapi-serialization-time) + - [Risk](#risk-2) + - [Mitigation](#mitigation-2) +- [Design Details](#design-details) + - [Current High-level Approach](#current-high-level-approach) + - [Proposed High-level Approach](#proposed-high-level-approach) + - [Template rendering](#template-rendering) + - [Test Plan](#test-plan) + - [Prerequisite testing updates](#prerequisite-testing-updates) + - [Unit tests](#unit-tests) + - [Integration tests](#integration-tests) + - [e2e tests](#e2e-tests) + - [Graduation Criteria](#graduation-criteria) + - [Alpha](#alpha) + - [Beta](#beta) + - [GA](#ga) + - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) + - [Version Skew Strategy](#version-skew-strategy) +- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) + - [Feature Enablement and Rollback](#feature-enablement-and-rollback) + - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning) + - [Monitoring Requirements](#monitoring-requirements) + - [Dependencies](#dependencies) + - [Scalability](#scalability) + - [Troubleshooting](#troubleshooting) +- [Implementation History](#implementation-history) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) + - [Implement proto.Models for OpenAPI V3 data](#implement-protomodels-for-openapi-v3-data) + + +## Release Signoff Checklist + + + +Items marked with (R) are required *prior to targeting to a milestone / release*. + +- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR) +- [ ] (R) KEP approvers have approved the KEP status as `implementable` +- [ ] (R) Design details are appropriately documented +- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input (including test refactors) + - [ ] e2e Tests for all Beta API Operations (endpoints) + - [ ] (R) Ensure GA e2e tests meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) + - [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free +- [ ] (R) Graduation criteria is in place + - [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) +- [ ] (R) Production readiness review completed +- [ ] (R) Production readiness review approved +- [ ] "Implementation History" section is up-to-date for milestone +- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] +- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes + + + +[kubernetes.io]: https://kubernetes.io/ +[kubernetes/enhancements]: https://git.k8s.io/enhancements +[kubernetes/kubernetes]: https://git.k8s.io/kubernetes +[kubernetes/website]: https://git.k8s.io/website + +## Summary + +This KEP proposes an enhancement to kubectl explain: + +1. Switch data source from OpenAPI v2 to OpenAPI v3 +2. Replace the hand-written `kubectl explain` printer with a go/template implementation. + +## Motivation + + + +### Richer OpenAPI V3 Data + +OpenAPI V3 support in Kubernetes is planned to go GA in 1.26. OpenAPI V3 is a richer +representation of the kubernetes API to our users, who have been asking for visibility +into things like: + +1. nullable +2. default +2. validation fields like oneOf, anyOf +2. + +### CRD schemas expressed as OpenAPI v2 are lossy + +Today CRDs specify their schemas in OpenAPI v3 format. To serve the `/openapi/v2` +document used today by kubectl, there is an expensive conversion from the v3 down +to v2 format. + +This process is [very lossy](https://github.com/kubernetes/kubernetes/blob/6e0de20fbb4c127d2e45c7a22347c08545fc7a86/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion.go#L56-L66), so `kubectl explain` when used against CRDs +making use of v3 features does not have a good experience with inaccurate information, or fields removed altogther. + + +### Goals + +1. Provide the new richer type information specified by OpenAPI v3 within kubectl explain +2. Have a more maintainable `text/template` based approach to printing +3. Fallback to old `explain` implementation if cluster does not expose OpenAPI v3 data. +4. Provide multiple new output formats for kubectl explain: + * human-readable plaintext + * markdown + * maybe others +5. (Optional?) Allow users to specify their own templates for use with kubectl + explain + +### Non-Goals + +any? + +## Proposal + + +### Basic Usage + +The following user experience should be possible with `kubectl explain` + +```shell +kubectl explain pods +``` + +Output should be familiar to users of today's `kubectl explain`, except new +information is populated: + +```console + +``` + +### Built-in Template Options + +Users should be able to specify built-in templates to use instead of the +human-readable plaintext form: +```shell +kubectl explain pods --template md +``` + +When using the `md` template, a markdown document is printed to stdout, so it +might be saved and used for a documentation website, for example. + +```console + +``` + +### Custom Templates +Users should also be able to specify a path to a custom template file for +the resource information to be written to: + +human-readable plaintext form: +```shell +kubectl explain pods --template /path/to/template.tmpl +``` + +#### Example template + +TBD + +### Risks and Mitigations + +#### OpenAPI V3 Not Available + +##### Risk + +OpenAPI v3 data is not available in the current cluster. + +##### Mitigation + +If the initial request for OpenAPI V3 fails, the old implementation using +OpenAPI v2 should be used instead. There is another risk that this fallback process +takes too long for interactive speed, so timeout should be carefully considered +to prevent user from waiting too long to see their results. + +#### Template API Stability +##### Risk + +If templates are written to target a specific OpenAPI version, then changes to +the format would case templates to be broken. It is a goal of this KEP to be more +maintainable than the current verison. If we risk templates breaking due to format +changes, that voids the goals of the KEP. + +This is only a risk of users are allowed to specify their own templates. If all +templates are maintained internally, then API stability it is something to consider +but not necessarily a requirement. + +##### Mitigation + +TBD + +#### OpenAPI serialization time +##### Risk + +Today there is no interactive-speed way to deserialize protobuf or JSON openapi +v3 data into the kube-openapi format. + +##### Mitigation + +There has been recent progress in this area. To unmarshal kube-OpenAPI v3 is now able +to be done in a performant enough way to do it in the CLI. This KEP's beta release +should be blocked on the merging of this optimization. + +## Design Details + +#### Current High-level Approach + +TODO: fill with links to referred implementation + +1. User types `kubectl explain pods` +2. kubectl resolves 'pods' to GVR core v1 pods using cluster discovery information +3. kubectl resolves GVR to its GVK using restmapper +4. kubectl fetches `/openapi/v2` as protobuf +5. kubectl parses the protobuf into `gnostic_v2.Document` +6. kubectl converts `gnostic_v2.Document` into `proto.Models` +7. kubectl searches the document's `Definitions` for a schema with the +extension `x-kubernetes-group-version-kind` matching the interested GVK +8. kubectl renders the definition using its hardcoded printer +9. If `--recursive` was used, repeat step 8 for the transitive closure of + object-typed fields of the top-level object. Concat the results together. + +#### Proposed High-level Approach + +1. User types `kubectl explain pods` +2. kubectl resolves 'pods' to GVR core v1 pods using cluster discovery information +3. kubectl fetches `/openapi/v3//` +4. kubectl parses the result as kube-openapi spec3 +5. kubectl locates the return type of the `/apis///` OpenAPI Path +6. kubectl renders the type using its built-in template for human-readable plaintext +7. If `--recursive` was used, repeat step 6 for the transitive closure of + object-typed fields of the top-level object. Concat the results together. + +### Template rendering + +Go's text/template will be used due to its familiarity, stability, and virtue of being in stdlib. + + +The following library functions will be provided to templates: + +TBD + +### Test Plan + + + +[ ] I/we understand the owners of the involved components may require updates to +existing tests to make this code solid enough prior to committing the changes necessary +to implement this enhancement. + +##### Prerequisite testing updates + + + +##### Unit tests + + + + + +- ``: `` - `` + +##### Integration tests + + + +- : + +##### e2e tests + + + +- : + +### Graduation Criteria + +Defined using feature gate + +#### Alpha + +- Feature implemented behind an envar? +- Existing explain tests are working or adapted for new implementation + +#### Beta + +- kube-OpenAPI JSON deserialization is optimized to take less than 150ms on + most machines + +#### GA + + + +### Upgrade / Downgrade Strategy + + + +### Version Skew Strategy + + + +## Production Readiness Review Questionnaire + + + +### Feature Enablement and Rollback + + + +###### How can this feature be enabled / disabled in a live cluster? + + + +- [ ] Feature gate (also fill in values in `kep.yaml`) + - Feature gate name: + - Components depending on the feature gate: +- [ ] Other + - Describe the mechanism: + - Will enabling / disabling the feature require downtime of the control + plane? + - Will enabling / disabling the feature require downtime or reprovisioning + of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled). + +###### Does enabling the feature change any default behavior? + + + +###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)? + +Yes. The old hand-written printer for OpenAPI v2 data in use today can be used as +a fallback. + +###### What happens if we reenable the feature if it was previously rolled back? + +Exactly what you would expect. Since the feature only changes how data is displayed, +there is no lasting effect from toggling it. + +###### Are there any tests for feature enablement/disablement? + + + +### Rollout, Upgrade and Rollback Planning + + + +###### How can a rollout or rollback fail? Can it impact already running workloads? + + + +###### What specific metrics should inform a rollback? + + + +###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested? + + + +###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.? + + + +### Monitoring Requirements + + + +###### How can an operator determine if the feature is in use by workloads? + + + +###### How can someone using this feature know that it is working for their instance? + +```shell +kubectl explain pods -v +``` + +TBD exact output. Should see successful OpenAPI v3 request or something + +###### What are the reasonable SLOs (Service Level Objectives) for the enhancement? + + + +###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service? + + + +- [ ] Metrics + - Metric name: + - [Optional] Aggregation method: + - Components exposing the metric: +- [ ] Other (treat as last resort) + - Details: + +###### Are there any missing metrics that would be useful to have to improve observability of this feature? + + + +### Dependencies + + + +###### Does this feature depend on any specific services running in the cluster? + + + +To reap the benefits of this feature, OpenAPI v3 is required, however OpenAPI v2 +data can be used as a fallback. + +### Scalability + + + +###### Will enabling / using this feature result in any new API calls? + +Yes, up feature replaces a single GET of `/openapi/v2` which returns a large (megabytes) +openapi document for all types with a more targeted call to `/openapi/v3//` + +The `/openapi/v3` endpoing implements E-Tag caching so that if the document has +not changed the server incurs a cheap, almost neglible cost to serving the request. + + + +###### Will enabling / using this feature result in introducing new API types? + +No. + +###### Will enabling / using this feature result in any new calls to the cloud provider? + +No. + +###### Will enabling / using this feature result in increasing size or count of the existing API objects? + +No. + +###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs? + +No. + +###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components? + +No, would expect generally same amount of resource usage for kubectl. + +### Troubleshooting + + + +###### How does this feature react if the API server and/or etcd is unavailable? + + + +###### What are other known failure modes? + + + +###### What steps should be taken if SLOs are not being met to determine the problem? + +## Implementation History + + + +## Drawbacks + + + +## Alternatives + +### Implement proto.Models for OpenAPI V3 data + +The current hard-coded printer is capable of printing any objects in `proto.Models` form. + +[We already have a way to express OpenAPI v3 data as `proto.Models`, so this can be +seen as a path of least resistence for plugging OpenAPI v3 into `kubectl explain`. + +We decide against this approach due to our desire to deprecate `proto.Models`. +We see`proto.Models` conversion as unnecessary and costly buraucracy in the code, +and we are seeking to deprecate the type in favor of the kube-openapi types. diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml b/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml new file mode 100755 index 00000000000..27569805bb6 --- /dev/null +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml @@ -0,0 +1,23 @@ +id: "3515" +prnumber: "" +name: kubectl-explain-openapiv3 +title: Kubectl Explain OpenAPIv3 +kep-number: draft +authors: ['@alexzielenski'] +owning-sig: sig-cli +participating-sigs: [sig-api-machinery, sig-cli] +reviewers: [] +approvers: ['@TBD'] +prr-approvers: [] +creation-date: "2022-09-14" +last-updated: v1.19 +status: provisional +stage: alpha +latest-milestone: "" +milestone: + alpha: "" + beta: "" + stable: "" +feature-gates: [] +disable-supported: false +metrics: [] From a5814d16e23eb817ba583bde6f1f76a9bfaee299 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 20 Sep 2022 16:09:43 -0700 Subject: [PATCH 02/23] spruce up wording --- .../3515-kubectl-explain-openapiv3/README.md | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index 848a9f40b41..c88d801e544 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -187,16 +187,19 @@ demonstrate the interest in a KEP within the wider Kubernetes community. [experience reports]: https://github.com/golang/go/wiki/ExperienceReports --> -### Richer OpenAPI V3 Data +### OpenAPI v3 is a richer API description than OpenAPI v2 -OpenAPI V3 support in Kubernetes is planned to go GA in 1.26. OpenAPI V3 is a richer -representation of the kubernetes API to our users, who have been asking for visibility +OpenAPI v3 support in Kubernetes is currently beta since version 1.24. +OpenAPI V3 is a richer representation of the kubernetes API to our users, who have been asking for visibility into things like: 1. nullable 2. default -2. validation fields like oneOf, anyOf -2. +3. validation fields like oneOf, anyOf, etc. + +To show each of these additional data points by themselves is a strong reason +to switch to using OpenAPI v3. + ### CRD schemas expressed as OpenAPI v2 are lossy @@ -207,6 +210,9 @@ to v2 format. This process is [very lossy](https://github.com/kubernetes/kubernetes/blob/6e0de20fbb4c127d2e45c7a22347c08545fc7a86/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion.go#L56-L66), so `kubectl explain` when used against CRDs making use of v3 features does not have a good experience with inaccurate information, or fields removed altogther. +This transformation causes bugs, for example, when attempting to `explain` a field +that is `nullable`, kubectl instead shows nothing, due to the lossy conversion +wiping nullable fields. ### Goals @@ -218,11 +224,16 @@ making use of v3 features does not have a good experience with inaccurate inform * markdown * maybe others 5. (Optional?) Allow users to specify their own templates for use with kubectl - explain + explain (there may be interesting use cases for this) ### Non-Goals -any? +1. "Fix" openapi v3 to openapi v2 conversion + This is a non-goal for two reasons: + * These formats are not compatible, and there WILL be data loss and inaccuracy + * This negates the benefits of using OpenAPI v3 for the richer type information +2. Provide general-purpose OpenAPI visualization. + ## Proposal @@ -258,7 +269,7 @@ might be saved and used for a documentation website, for example. ``` ### Custom Templates -Users should also be able to specify a path to a custom template file for +Users might also like to be able to specify a path to a custom template file for the resource information to be written to: human-readable plaintext form: @@ -266,7 +277,7 @@ human-readable plaintext form: kubectl explain pods --template /path/to/template.tmpl ``` -#### Example template +#### Example template Format TBD @@ -857,6 +868,12 @@ The current hard-coded printer is capable of printing any objects in `proto.Mode [We already have a way to express OpenAPI v3 data as `proto.Models`, so this can be seen as a path of least resistence for plugging OpenAPI v3 into `kubectl explain`. -We decide against this approach due to our desire to deprecate `proto.Models`. -We see`proto.Models` conversion as unnecessary and costly buraucracy in the code, -and we are seeking to deprecate the type in favor of the kube-openapi types. +This approach is undesirable for a few different reasons: + +1.) We would like to update the explain printer to include new OpenAPI v3 information, +the current design makes that time consuming and not maintainable. + +2.) API-Machinery has desire to deprecate `proto.Models`. We see`proto.Models` +conversion as unnecessary and costly buraucracy, that contributes to high +OpenAPI overhead. We are seeking to deprecate the type in favor of the +kube-openapi types for future usage. From d03ed2131ad9b0cc6fdc5817c58f0a9b3d11cb84 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Wed, 28 Sep 2022 13:33:53 -0700 Subject: [PATCH 03/23] reduce scope and take feedback into account --- keps/prod-readiness/sig-cli/3515.yaml | 3 + .../3515-kubectl-explain-openapiv3/README.md | 120 +++++++++++------- .../3515-kubectl-explain-openapiv3/kep.yaml | 4 +- 3 files changed, 80 insertions(+), 47 deletions(-) create mode 100644 keps/prod-readiness/sig-cli/3515.yaml diff --git a/keps/prod-readiness/sig-cli/3515.yaml b/keps/prod-readiness/sig-cli/3515.yaml new file mode 100644 index 00000000000..b2bb14812b0 --- /dev/null +++ b/keps/prod-readiness/sig-cli/3515.yaml @@ -0,0 +1,3 @@ +kep-number: 3515 +alpha: + approver: "@johnbelamaric" diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index c88d801e544..dcd4d28dab8 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -253,33 +253,63 @@ information is populated: ``` +Note: Feature during development will be gated by an experimental flag. The commands +shown here elide the experimental flag for clarity. + ### Built-in Template Options +#### Plaintext + -Users should be able to specify built-in templates to use instead of the -human-readable plaintext form: ```shell -kubectl explain pods --template md +kubectl explain pods ``` +or -When using the `md` template, a markdown document is printed to stdout, so it -might be saved and used for a documentation website, for example. +```shell +kubectl explain pods --output plaintext +``` -```console - +The plaintext output format is the default and should be crafted to be as close +as the existing `explain` output in use before this KEP. + +#### Raw + +```shell +kubectl explain pods --output raw ``` -### Custom Templates -Users might also like to be able to specify a path to a custom template file for -the resource information to be written to: +To get raw OpenAPI v3 data for a certain resource today involves: +1.) setting up kubectl proxy +2.) fetching the correct path at `/openapi/v3//` +3.) filtering out unwanted results + +This command is useful not only for its convenience, but also other visualizations +may be built upon the raw output if we opt not to support a first-class custom +template solution in the future. + +#### HTML -human-readable plaintext form: ```shell -kubectl explain pods --template /path/to/template.tmpl +kubectl explain pods --output html ``` -#### Example template Format +Similarly to [godoc](https://pkg.go.dev), the we suggest to provide a searchable, +navigable, generated webpage for the kubernetes types of whatever cluster kubectl +is talking to. + +#### Markdown + +```shell +kubectl explain pods --output md +``` + +When using the `md` template, a markdown document is printed to stdout, so it +might be saved and used for a documentation website, for example. + +```console + +``` -TBD ### Risks and Mitigations @@ -296,21 +326,6 @@ OpenAPI v2 should be used instead. There is another risk that this fallback proc takes too long for interactive speed, so timeout should be carefully considered to prevent user from waiting too long to see their results. -#### Template API Stability -##### Risk - -If templates are written to target a specific OpenAPI version, then changes to -the format would case templates to be broken. It is a goal of this KEP to be more -maintainable than the current verison. If we risk templates breaking due to format -changes, that voids the goals of the KEP. - -This is only a risk of users are allowed to specify their own templates. If all -templates are maintained internally, then API stability it is something to consider -but not necessarily a requirement. - -##### Mitigation - -TBD #### OpenAPI serialization time ##### Risk @@ -357,11 +372,6 @@ extension `x-kubernetes-group-version-kind` matching the interested GVK Go's text/template will be used due to its familiarity, stability, and virtue of being in stdlib. - -The following library functions will be provided to templates: - -TBD - ### Test Plan ## Release Signoff Checklist @@ -783,7 +783,7 @@ Yes, up feature replaces a single GET of `/openapi/v2` which returns a large (me openapi document for all types with a more targeted call to `/openapi/v3//` The `/openapi/v3//` endpoint implements E-Tag caching so that if the document has -not changed the server incurs a cheap, almost neglible cost to serving the request. +not changed the server incurs a cheap, almost negligible cost to serving the request. The document returned by calls to `/openapi/v3/...` is expected to be far smaller than the megabytes-scale openapi v2 document, since it only includes information @@ -882,7 +882,7 @@ Why should this KEP _not_ be implemented? The current hard-coded printer is capable of printing any objects in `proto.Models` form. [We already have a way to express OpenAPI v3 data as `proto.Models`, so this can be -seen as a path of least resistence for plugging OpenAPI v3 into `kubectl explain`. +seen as a path of least resistance for plugging OpenAPI v3 into `kubectl explain`. This approach is undesirable for a few different reasons: From 9fcaad4753840c64012b367e5f35cad3f7bd6937 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Thu, 29 Sep 2022 10:22:22 -0700 Subject: [PATCH 05/23] fixx out kep yaml --- .../3515-kubectl-explain-openapiv3/kep.yaml | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml b/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml index 382f06e5225..1b1b5faca62 100755 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml @@ -1,23 +1,27 @@ id: "3515" -prnumber: "" +prnumber: "3516" name: kubectl-explain-openapiv3 title: Kubectl Explain OpenAPIv3 kep-number: 3515 authors: ['@alexzielenski'] owning-sig: sig-cli participating-sigs: [sig-api-machinery, sig-cli] -reviewers: [] -approvers: ['@TBD'] -prr-approvers: [] +reviewers: + - "@KnVerey" + - "@seans3" + - "@apelisse" +approvers: + - "@KnVerey" + - "@seans3" creation-date: "2022-09-14" last-updated: v1.19 status: implementable stage: alpha -latest-milestone: "" +latest-milestone: "1.26" milestone: - alpha: "" - beta: "" - stable: "" + alpha: "1.26" + beta: "1.28" + stable: "1.29" feature-gates: [] disable-supported: false metrics: [] From 677ccacc7ea94bd7b9d7ffc6d9b312d234615662 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Thu, 29 Sep 2022 10:31:08 -0700 Subject: [PATCH 06/23] remove prnumber what is this used for --- keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml b/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml index 1b1b5faca62..e9103663d7c 100755 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/kep.yaml @@ -1,5 +1,4 @@ id: "3515" -prnumber: "3516" name: kubectl-explain-openapiv3 title: Kubectl Explain OpenAPIv3 kep-number: 3515 From 6fa9647c6e0b6676b241f480285e61a4a4c8512f Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Thu, 29 Sep 2022 16:06:44 -0700 Subject: [PATCH 07/23] fill in testing plan a bit more --- .../3515-kubectl-explain-openapiv3/README.md | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index 5920349a024..1071f107d96 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -385,7 +385,7 @@ when drafting this test plan. [testing-guidelines]: https://git.k8s.io/community/contributors/devel/sig-testing/testing.md --> -[ ] I/we understand the owners of the involved components may require updates to +[x] I/we understand the owners of the involved components may require updates to existing tests to make this code solid enough prior to committing the changes necessary to implement this enhancement. @@ -417,7 +417,7 @@ This can inform certain test coverage improvements that we want to do before extending the production code to implement this enhancement. --> -- ``: `` - `` +- `k8s.io/kubectl/pkg/explain`: `09/29/2022`-`75.6` ##### Integration tests @@ -431,6 +431,13 @@ https://storage.googleapis.com/k8s-triage/index.html - : +Tests should include + +- Expected Output tests +- Show correct OpenAPI v3 endpoints are hit +- Tests that show default/nullability information is being included in plaintext output +- Tests that update the backing openapi in between calls to explain + ##### e2e tests +Existing e2e tests should be adapted for the new system. +E2E test that shows every definition in OpenAPI document can be retrieved via explain + + + - : ### Graduation Criteria @@ -453,10 +465,14 @@ Defined using feature gate - Feature implemented behind a command line flag `--experimental-openapiv3` - Existing explain tests are working or adapted for new implementation +- Plaintext output roughly matches explain output +- Raw output implemented #### Beta -- kube-OpenAPI v3 JSON deserialization is optimized to take less than 150ms on +- md output implemented +- basic html output +- kube-OpenAPI v3 JSON deserialization is optimized to take less than 150ms on most machines #### GA From e8382b4ebe70d92370eba1a7a859ad1953a4bde7 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Fri, 30 Sep 2022 10:04:18 -0700 Subject: [PATCH 08/23] rename raw to openapiv3 --- keps/sig-cli/3515-kubectl-explain-openapiv3/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index 1071f107d96..d269a0b0941 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -272,10 +272,10 @@ kubectl explain pods --output plaintext The plaintext output format is the default and should be crafted to be as close as the existing `explain` output in use before this KEP. -#### Raw +#### OpenAPIV3 (raw json) ```shell -kubectl explain pods --output raw +kubectl explain pods --output openapiv3 ``` To get raw OpenAPI v3 data for a certain resource today involves: @@ -466,7 +466,7 @@ Defined using feature gate - Feature implemented behind a command line flag `--experimental-openapiv3` - Existing explain tests are working or adapted for new implementation - Plaintext output roughly matches explain output -- Raw output implemented +- OpenAPIV3 (raw json) output implemented #### Beta @@ -713,10 +713,10 @@ logs or events for this purpose. ###### How can someone using this feature know that it is working for their instance? ```shell -kubectl explain pods --output raw +kubectl explain pods --output openapiv3 ``` -TBD exact output. User should see OpenAPI v3 JSON Schema for `pods` type printed to console. +User should see OpenAPI v3 JSON Schema for `pods` type printed to console. ###### What are the reasonable SLOs (Service Level Objectives) for the enhancement? From 9f744f3adbc5a512144149ea253a432d983df654 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Fri, 30 Sep 2022 11:33:32 -0700 Subject: [PATCH 09/23] update with feedback --- .../3515-kubectl-explain-openapiv3/README.md | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index d269a0b0941..3329f0f7fe5 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -321,11 +321,26 @@ OpenAPI v3 data is not available in the current cluster. ##### Mitigation +# If the user does not provide an --output argument + +This is a "compatibility mode" where users will see OpenAPI v3 plaintext template +if the data is available, or fallback to old `kubectl explain` behavior for openapi v2. + If the initial request for OpenAPI V3 fails, the old implementation using OpenAPI v2 should be used instead. There is another risk that this fallback process takes too long for interactive speed, so timeout should be carefully considered -to prevent user from waiting too long to see their results. +to prevent user from waiting too long to see their results. + +# If the user does provider an --output argument +If a user specifies an `--output` argument and the server 404's attempting to +fetch the correct openapi version for the template, a new error message should +be thrown to the effect of server missing openapi data for version: %v.%v.%v`. + +Internal templates should strive to support all OpenAPI versions supported by +versoins of kubernetes within their skew. + +Other network errors should be handled using normal kubectl error handling. #### OpenAPI serialization time ##### Risk @@ -353,8 +368,10 @@ TODO: fill with links to referred implementation 6. kubectl converts `gnostic_v2.Document` into `proto.Models` 7. kubectl searches the document's `Definitions` for a schema with the extension `x-kubernetes-group-version-kind` matching the interested GVK -8. kubectl renders the definition using its hardcoded printer -9. If `--recursive` was used, repeat step 8 for the transitive closure of +8. If a field path was used, kubectl traveres the definition's fields to the subschema +specified by the user's path. +9. kubectl renders the definition using its hardcoded printer +10. If `--recursive` was used, repeat step 9 for the transitive closure of object-typed fields of the top-level object. Concat the results together. #### Proposed High-level Approach @@ -363,10 +380,11 @@ extension `x-kubernetes-group-version-kind` matching the interested GVK 2. kubectl resolves 'pods' to GVR core v1 pods using cluster discovery information 3. kubectl fetches `/openapi/v3//` 4. kubectl parses the result as kube-openapi spec3 -5. kubectl locates the return type of the `/apis///` OpenAPI Path -6. kubectl renders the type using its built-in template for human-readable plaintext -7. If `--recursive` was used, repeat step 6 for the transitive closure of - object-typed fields of the top-level object. Concat the results together. +5. kubectl locates the schema of the return type for the Path `/apis///` in kube-openapi +6. If a field path was used, kubectl traveres the definition's fields to the subschema +specified by the user's path. +8. kubectl renders the type using its built-in template for human-readable plaintext +9. If `--recursive` was used, repeat step 8 for the transitive closure of object-typed fields of the top-level object. Concat the results together. ### Template rendering From 0316076bf98b7d5ae10977c193a9a8af532794da Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Fri, 30 Sep 2022 11:39:21 -0700 Subject: [PATCH 10/23] fix typo --- keps/sig-cli/3515-kubectl-explain-openapiv3/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index 3329f0f7fe5..bf8e2ec1e7f 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -368,7 +368,7 @@ TODO: fill with links to referred implementation 6. kubectl converts `gnostic_v2.Document` into `proto.Models` 7. kubectl searches the document's `Definitions` for a schema with the extension `x-kubernetes-group-version-kind` matching the interested GVK -8. If a field path was used, kubectl traveres the definition's fields to the subschema +8. If a field path was used, kubectl traverses the definition's fields to the subschema specified by the user's path. 9. kubectl renders the definition using its hardcoded printer 10. If `--recursive` was used, repeat step 9 for the transitive closure of From 15fb8cdf592c90cafb2aed66bf7008daf62b8f9e Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Fri, 30 Sep 2022 12:00:15 -0700 Subject: [PATCH 11/23] fill in version skew strategy --- .../3515-kubectl-explain-openapiv3/README.md | 49 ++++++++++++++----- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index bf8e2ec1e7f..0b5cfa7d7a2 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -243,7 +243,7 @@ wiping nullable fields. The following user experience should be possible with `kubectl explain` ```shell -kubectl explain pods +kubectl explain pods.spec ``` Output should be familiar to users of today's `kubectl explain`, except new @@ -321,7 +321,7 @@ OpenAPI v3 data is not available in the current cluster. ##### Mitigation -# If the user does not provide an --output argument +###### If the user does not provide an --output argument This is a "compatibility mode" where users will see OpenAPI v3 plaintext template if the data is available, or fallback to old `kubectl explain` behavior for openapi v2. @@ -331,14 +331,14 @@ OpenAPI v2 should be used instead. There is another risk that this fallback proc takes too long for interactive speed, so timeout should be carefully considered to prevent user from waiting too long to see their results. -# If the user does provider an --output argument +###### If the user does provider an --output argument If a user specifies an `--output` argument and the server 404's attempting to fetch the correct openapi version for the template, a new error message should be thrown to the effect of server missing openapi data for version: %v.%v.%v`. Internal templates should strive to support all OpenAPI versions supported by -versoins of kubernetes within their skew. +versions of kubernetes within their skew. Other network errors should be handled using normal kubectl error handling. @@ -472,7 +472,6 @@ Existing e2e tests should be adapted for the new system. E2E test that shows every definition in OpenAPI document can be retrieved via explain - - : ### Graduation Criteria @@ -482,15 +481,24 @@ Defined using feature gate #### Alpha - Feature implemented behind a command line flag `--experimental-openapiv3` +- `--output` flag added - Existing explain tests are working or adapted for new implementation - Plaintext output roughly matches explain output - OpenAPIV3 (raw json) output implemented +- HTML and MD outputs are not target for alpha #### Beta -- md output implemented -- basic html output -- kube-OpenAPI v3 JSON deserialization is optimized to take less than 150ms on +- md output implemented (or dropped from design due to continued debate) + - Table of contents all GVKs grouped by Group then Version. + - Section for each individual GVK + - All types hyperlink to specific section +- basic html output (or dropped from design due to continued debate) + - Table of contents all GVKs grouped by Group then Version. + - Page for each individual GVK. + - All types hyperlink to their specific page + - Searchable by name, description, field name. +- kube-openAPI v3 JSON deserialization is optimized to take less than 150ms on most machines #### GA @@ -571,6 +579,8 @@ enhancement: cluster required to make on upgrade, in order to make use of the enhancement? --> +N/A + ### Version Skew Strategy +This feature only requires the target cluster has enabled The OpenAPIV3 feature. + +OpenAPIV3 is Beta as of Kubernetes 1.24. This feature should not be on-by-default +until it is GA. + +Users of the `--output` flag who attempt to use it against a cluster for which +OpenAPI v3 is not enabled will be shown an error informing them of missing openapi +version upon 404. + +Built-in templates supported by kubectl should aim to support any OpenAPI +version which is GA. + ## Production Readiness Review Questionnaire -###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)? +Enabling the feature changes the data source of `kubectl explain` to use openapiv3. +The output optimally should be familiar to users, who may be delighted to see new +information populated. -Yes. The old hand-written printer for OpenAPI v2 data in use today can be used as -a fallback. +###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)? Until the feature is stable it will only be enabled when the `--experimental-openapiv3` flag is used. +It has no persistent effect on data that is viewewd. ###### What happens if we reenable the feature if it was previously rolled back? -There is no persistence to using the `--experimental-openapiv3` flag. It is only -used for viewing data. +There is no persistence to using the feature. It is only used for viewing data. ###### Are there any tests for feature enablement/disablement? From f2d6ffd443176d8d79e14eb4f2d456bdb3345a9b Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Fri, 30 Sep 2022 12:03:44 -0700 Subject: [PATCH 12/23] update toc --- keps/sig-cli/3515-kubectl-explain-openapiv3/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index 0b5cfa7d7a2..fd3888ff8a3 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -88,7 +88,7 @@ tags, and then generate with `hack/update-toc.sh`. - [Basic Usage](#basic-usage) - [Built-in Template Options](#built-in-template-options) - [Plaintext](#plaintext) - - [Raw](#raw) + - [OpenAPIV3 (raw json)](#openapiv3-raw-json) - [HTML](#html) - [Markdown](#markdown) - [Risks and Mitigations](#risks-and-mitigations) From 9422a7dde2c613a352d21ae6f7d6547e7df9f73b Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Fri, 30 Sep 2022 12:06:02 -0700 Subject: [PATCH 13/23] formatting fixes --- keps/sig-cli/3515-kubectl-explain-openapiv3/README.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md index fd3888ff8a3..d1dd9b85ed5 100644 --- a/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md +++ b/keps/sig-cli/3515-kubectl-explain-openapiv3/README.md @@ -331,11 +331,11 @@ OpenAPI v2 should be used instead. There is another risk that this fallback proc takes too long for interactive speed, so timeout should be carefully considered to prevent user from waiting too long to see their results. -###### If the user does provider an --output argument +###### If the user does provide an --output argument If a user specifies an `--output` argument and the server 404's attempting to fetch the correct openapi version for the template, a new error message should -be thrown to the effect of server missing openapi data for version: %v.%v.%v`. +be thrown to the effect of: `server missing openapi data for version: %v.%v.%v`. Internal templates should strive to support all OpenAPI versions supported by versions of kubernetes within their skew. @@ -381,7 +381,7 @@ specified by the user's path. 3. kubectl fetches `/openapi/v3//` 4. kubectl parses the result as kube-openapi spec3 5. kubectl locates the schema of the return type for the Path `/apis///` in kube-openapi -6. If a field path was used, kubectl traveres the definition's fields to the subschema +6. If a field path was used, kubectl traverses the definition's fields to the subschema specified by the user's path. 8. kubectl renders the type using its built-in template for human-readable plaintext 9. If `--recursive` was used, repeat step 8 for the transitive closure of object-typed fields of the top-level object. Concat the results together. @@ -894,7 +894,8 @@ details). For now, we leave it here. ###### How does this feature react if the API server and/or etcd is unavailable? - +Using kubectl's normal error handling. There is no lasting effect to data or the +user. ###### What are other known failure modes? @@ -913,6 +914,8 @@ For each of them, fill in the following information by copying the below templat ###### What steps should be taken if SLOs are not being met to determine the problem? +N/A + ## Implementation History