From a1cf9516fdde3a88725fbfc384f2f1bd497eb5b1 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Mon, 30 Jan 2017 13:36:54 -0800 Subject: No feature proposal for `kubectl get` and `kubectl describe` extensions for non-compiled in types. --- .../sig-cli/get-describe-apiserver-extensions.md | 89 ++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md diff --git a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md new file mode 100644 index 00000000..5de90e62 --- /dev/null +++ b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md @@ -0,0 +1,89 @@ +# Provide open-api extensions for kubectl get / kubectl describe columns + +Status: Pending + +## Motivation + +`kubectl get` and `kubectl describe` do not provide a rich experience +for resources retrieved through federated apiservers and types not +compiled into the kubectl binary. Kubectl should support printing +columns configured per-type without having the types compiled in. + +## Proposal + +Allow the apiserver to define the type specific columns that will be +printed using the open-api swagger.json spec already fetched by kubectl. +This provides a limited describe to only print out fields on the object +and related events. + +## User Experience + +### Use Cases + +- As a user, when I run `kubectl get` on sig-service-catalog resources + defined in a federated apiserver, I want to see more than just the + name and the type of the resource. +- As a user, when I run `kubectl describe` on sig-service-catalog + resources defined in a federated apiserver, I want the command + to succeed, and to see events for the resource along with important + fields of the resource. + +## Implementation + +Define the open-api extensions `x-kubernetes-kubectl-get-columns` and +`x-kubernetes-kubectl-describe-columns`. These extensions have a +string value containing the columns to be printed by kubectl. The +string format is the same as the `--custom-columns` for `kubectl get`. + +### Apiserver + +- Populate the open-api extension value for resource types. + +### Kubectl + +- In `kubectl get` use the `x-kubernetes-kubectl-get-columns` value + when printing an object iff 1) it is defined and 2) the output type + is "" (empty string) or "wide". + +- In `kubectl describe` use the `x-kubernetes-kubectl-describe-columns` value + when printing an object iff 1) it is defined + +### Client/Server Backwards/Forwards compatibility + +#### Newer client + +Client doesn't find the open-api extensions. Fallback on 1.5 behavior. + +#### Newer server + +Client doesn't respect open-api extensions. Uses 1.5 behavior. + +## Alternatives considered + +### Kubectl describe fully implemented in the server + +Implement a sub-resource "/describe" in the apiserver. This executes +the describe business logic for the object and returns either a string +or json blob for kubectl to print. + +**Pros:** Higher fidelity. Can aggregate data and fetch other objects. + +**Cons:** Higher complexity. Requires more api changes. + +### Write per-type columns to kubectl.config or another local file + +Support checking a local file containing per-type information including +the columns to print. + +**Pros:** Simplest solution. Easy for user to override values. + +**Cons:** Requires manual configuration on user side. Does not provide a consistent experience across clients. + +### Write per-type go templates to kubectl.config or another local file + +Support checking a local file containing per-type information including +the go template. + +**Pros:** Higher fidelity. Easy for user to override values. + +**Cons:** Higher complexity. Requires manual configuration on user side. Does not provide a consistent experience across clients. -- cgit v1.2.3 From 1aa92825b68af4800c76c6f360b3ccf92d581681 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Jan 2017 09:06:22 -0800 Subject: Add implementation details --- .../sig-cli/get-describe-apiserver-extensions.md | 86 ++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md index 5de90e62..03bb026a 100644 --- a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md +++ b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md @@ -16,6 +16,11 @@ printed using the open-api swagger.json spec already fetched by kubectl. This provides a limited describe to only print out fields on the object and related events. +**Note:** This solution will only work for types compiled into the apiserver +providing the open-api swagger.json to kubectl. This solution will +not work for TPR, though TPR could possibly be solved in a similar +way by apply an annotation with the same key / value to the TPR. + ## User Experience ### Use Cases @@ -39,8 +44,14 @@ string format is the same as the `--custom-columns` for `kubectl get`. - Populate the open-api extension value for resource types. +This is done by hardcoding the extension for types compiled into +the api server. As such this is only a solution for types +implemented using federated apiservers. + ### Kubectl +Overview: + - In `kubectl get` use the `x-kubernetes-kubectl-get-columns` value when printing an object iff 1) it is defined and 2) the output type is "" (empty string) or "wide". @@ -48,6 +59,69 @@ string format is the same as the `--custom-columns` for `kubectl get`. - In `kubectl describe` use the `x-kubernetes-kubectl-describe-columns` value when printing an object iff 1) it is defined +If no open-api extension is present for a type, fallback on the 1.5 +behavior. + +Details: + +#### Option 1: Re-parse the open-api swagger.json in a kubectl library + +Re-parse the open-api swagger.json schema and build a map of group version kind -> columns +parsed from the schema. For this would look similar to validation/schema.go + +In get.go and describe.go: After fetching the "Infos" from the +resource builder, lookup the group version kind from the populated map. + +**Pros:** + - Simple and straightforward solution + - Scope of impacted Kubernetes components is minimal + - Doable in 1.6 + +**Cons:** + - Hacky solution + - Can not be cleanly extended to support TPR + +#### Option 2: Modify api-machinery RestMapper + +Modify the api-machinery RestMapper to parse the column tags and +include them in the *RestMapping* used by the resource builder. + +```go +type RESTMapping struct { + // Resource is a string representing the name of this resource as a REST client would see it + Resource string + + GroupVersionKind schema.GroupVersionKind + + // Scope contains the information needed to deal with REST Resources that are in a resource hierarchy + Scope RESTScope + + runtime.ObjectConvertor + MetadataAccessor + + // New for kubectl get / describe + DisplayOptions DisplayOptions +} + +type DisplayOptions struct { + GetColumns []string + DescribeColumns []string +} +``` + +The tags would then be easily accessible from the kubectl get / describe +functions through: `resource.Builder -> Infos -> Mapping -> DisplayOptions` + +**Pros:** + - Clean + generalized solution + - The same strategy can be applied to support TPR + - Can be used by other clients / tools + +**Cons:** + - Fields are only loosely tied to rest + - Complicated due to the broad scope and impact + - May not be doable in 1.6 + ### Client/Server Backwards/Forwards compatibility #### Newer client @@ -60,6 +134,18 @@ Client doesn't respect open-api extensions. Uses 1.5 behavior. ## Alternatives considered +### Fork Kubectl and compile in go types + +Fork kubectl and compile in the go types. Implement get / describe +for the new types in the forked version. + +**Pros:** *This is what will happen for sig-service catalog if we take no action in 1.6* + +**Cons:** Bad user experience. No clear solution for patching forked kubectl. +User has to use a separate kubectl binary per-apiserver. Bad president. + +I really don't want this solution to be used. + ### Kubectl describe fully implemented in the server Implement a sub-resource "/describe" in the apiserver. This executes -- cgit v1.2.3 From 39dbe85d62d2811a8835a3ac488d266ad0cb6eec Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Jan 2017 09:28:54 -0800 Subject: Add information about what to do when extensions are available for compiled in types --- .../sig-cli/get-describe-apiserver-extensions.md | 23 ++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md index 03bb026a..29adca75 100644 --- a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md +++ b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md @@ -59,10 +59,6 @@ Overview: - In `kubectl describe` use the `x-kubernetes-kubectl-describe-columns` value when printing an object iff 1) it is defined -If no open-api extension is present for a type, fallback on the 1.5 -behavior. - -Details: #### Option 1: Re-parse the open-api swagger.json in a kubectl library @@ -122,12 +118,31 @@ functions through: `resource.Builder -> Infos -> Mapping -> DisplayOptions` - Complicated due to the broad scope and impact - May not be doable in 1.6 +#### Considerations + +What should be used for oth an open-api extension columns tag AND a +compiled in printer exist for a type? + +- Apiserver only provides `describe` for types that are never compiled in + - Compiled in `describe` is much more rich - aggregating data across many other types. + e.g. Node describe aggregating Pod data + - kubectl will not be able to provide any `describe` information for new types when version skewed against a newer server +- Always use the extensions if present + - Allows server to control columns. Adds new columns for types on old clients that maybe missing the columns. +- Always use the compiled in commands if present + - The compiled in `describe` is richer and provides aggregated information about many types. +- Always use the `get` extension if present. Always use the `describe` compiled in code if present. + - Inconsistent behavior across how extensions are handled + ### Client/Server Backwards/Forwards compatibility #### Newer client Client doesn't find the open-api extensions. Fallback on 1.5 behavior. +In the future, this will provide stronger backwards / forwards compability +as it will allow clients to print objects + #### Newer server Client doesn't respect open-api extensions. Uses 1.5 behavior. -- cgit v1.2.3 From f3d5c99509697d902096ebca82de3f1d3a3cd190 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Jan 2017 09:38:54 -0800 Subject: Add version --- .../design-proposals/sig-cli/get-describe-apiserver-extensions.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md index 29adca75..b9b27ca0 100644 --- a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md +++ b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md @@ -2,6 +2,8 @@ Status: Pending +Version: Alpha + ## Motivation `kubectl get` and `kubectl describe` do not provide a rich experience -- cgit v1.2.3 From ff39a627f7f55e1f4823d88821c75e7563578698 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Jan 2017 09:46:07 -0800 Subject: Update proposal to use generalize extensions --- .../sig-cli/get-describe-apiserver-extensions.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md index b9b27ca0..698b5cf3 100644 --- a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md +++ b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md @@ -81,8 +81,8 @@ resource builder, lookup the group version kind from the populated map. #### Option 2: Modify api-machinery RestMapper -Modify the api-machinery RestMapper to parse the column tags and -include them in the *RestMapping* used by the resource builder. +Modify the api-machinery RestMapper to parse extensions prefixed +with `x-kubernetes` and include them in the *RestMapping* used by the resource builder. ```go type RESTMapping struct { @@ -97,13 +97,12 @@ type RESTMapping struct { runtime.ObjectConvertor MetadataAccessor - // New for kubectl get / describe - DisplayOptions DisplayOptions + // Extensions + OpenApiExtensions OpenApiExtensions } type DisplayOptions struct { - GetColumns []string - DescribeColumns []string + OpenApiExtensions map[string]interface{} } ``` @@ -113,6 +112,7 @@ functions through: `resource.Builder -> Infos -> Mapping -> DisplayOptions` **Pros:** - Clean + generalized solution - The same strategy can be applied to support TPR + - Can support exposing future extensions such as patchStrategy and mergeKey - Can be used by other clients / tools **Cons:** -- cgit v1.2.3 From af507712b9160ed43f47d378fe9505e35783c51d Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Jan 2017 10:06:52 -0800 Subject: fix typo --- .../design-proposals/sig-cli/get-describe-apiserver-extensions.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md index 698b5cf3..42884990 100644 --- a/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md +++ b/contributors/design-proposals/sig-cli/get-describe-apiserver-extensions.md @@ -98,11 +98,11 @@ type RESTMapping struct { MetadataAccessor // Extensions - OpenApiExtensions OpenApiExtensions + ApiExtensions ApiExtensions } -type DisplayOptions struct { - OpenApiExtensions map[string]interface{} +type ApiExtensions struct { + Extensions map[string]interface{} } ``` -- cgit v1.2.3