Show openapi custom columns in kubectl get
#53483
Conversation
|
I'd like to see this also applied to the server side code so we're not only working on half the solution. |
|
I'm probably missing some context. Could you elaborate? How is this half the solution, since it does exactly what I expect? |
831bf30
to
dc67a41
Compare
|
Updated to fix the broken tests. I don't disagree with you but:
|
dc67a41
to
4a47318
Compare
+1 on this. We already have the custom column syntax documented in the service-catalog walkthrough docs. Being able to remove it and have the custom columns "just work" (from the user perspective) would be tremendous from a UX point of view. |
|
@smarterclayton I don't follow the argument that we should hold off on UX improvements in the client so the server doesn't fall behind. Isn't needing to pairing client / server features the opposite of what we want since we can expect to work in client / server version skewed environments and will never be able to transactionally update both. Failing to provide an adequate solution for printing extensions will likely result in folks doing something much worse - such as forking kubectl and compiling in custom logic (yes this was proposed), or writing plugins to print resources. |
|
I'm trying to land server side for 1.9 (although it's pretty close to the
wire). We have to have that to support any realistic extension use case.
The server is ultimately the one who controls output, not the client,
because the client will always be field skewed with the server. I'd rather
focus on finishing server side than continuing to poke at it from the CLI
side. Are you saying you don't agree with the move to have it on the
server side?
…On Tue, Nov 14, 2017 at 11:52 PM, Phillip Wittrock ***@***.*** > wrote:
@smarterclayton <https://github.com/smarterclayton> I don't follow the
argument that we should hold off on UX improvements in the client so the
server doesn't fall behind. Isn't needing to pairing client / server
features the opposite of what we want since we can expect to work in client
/ server version skewed environments and will never be able to
transactionally update both.
Failing to provide an adequate solution for printing extensions will
likely result in folks doing something much worse - such as forking kubectl
and compiling in custom logic (yes this was proposed), or writing plugins
to print resources.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#53483 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p7Gzd5sHDHtym4F10ZbDqLEZ_OWlks5s2m4JgaJpZM4PuiAz>
.
|
|
@smarterclayton Thanks for all you responses.
I believe the implementation here is still driven by the server. As I understand it, this approach and the server-side-printing approach are very similar - in both cases the server provides to the client a list of columns + the structured data to populate those columns, and the client then formats the data for printing in a generic way without compiling in any information about the type. In neither case should version skew occur since the server controls the data is printed by the client. (round-tripping through structs aside, which won't happen for extension types and really shouldn't happen for non-extension types) The difference with this implementation vs server-side-printing, is 1) that the server attaches the columns metadata to the object schema instead of including it in the response body, and 2) the server cannot pre-process the information in the object configuration.
Not entirely. Btw, do you have a link to the latests design for server-side-printing? I read the initial design, but that was some time ago, and things have probably changed since I read it. The information currently in The information in I am wary of having the server do much pre-processing of the data in the object configuration for a simple
I don't think this will reduce focus on server side printing except that it reduces the negative impact of server side printing failing to land in 1.9, and reduces the impact of unknown-unknowns discovered with server-side-printing post code freeze. The feature enabled in this PR was completed and merged in 1.8, but kept as opt-in. This PR is limited to changing the default flag value so it is opt-out, and updating some tests. My own bias'ed analysis: Risks of not enabling this feature in 1.9
Risks of enabling this feature
Thoughts? |
Add a new EmptyResource class that implements openapi.Resources, but just doesn't have any resources. That is useful for tests that expect the OpenAPISchema to be working, but don't expect any actual result. Also implement CreateOpenAPISchemaFunc to simply plug in the TestFactory code when you want to use an actual APISchema.
Change the name of the flag to something slightly more user-friendly, while making it default true. The flag is kept so that someone can revert it temporarly if things go wrong.
4a47318
to
1d99b76
Compare
|
/retest |
|
@smarterclayton Planning on lgtming this today if you have no other comments. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: apelisse, pwittrock No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/hold cancel |
|
Automatic merge from submit-queue (batch tested with PRs 50457, 55558, 53483, 55731, 52842). If you want to cherry-pick this change to another branch, please follow the instructions here. |
Automatic merge from submit-queue (batch tested with PRs 65319, 64513, 65474, 65601, 65634). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. deprecate --use-openapi-print-columns in favor of --server-print server-side printing has been supported since 1.10 with identical output for core kubernetes types, support is available for extension API servers since 1.10, and for CRDs since 1.11. openapi printing is mutually exclusive with server-side printing (you have to fetch full objects to do openapi printing, and table row output to do server side printing) openapi printing has many downsides: * it requires fetching/parsing a very large schema on every get request * it requires complex object extraction logic be built into every client * it is limited to literal values that appear in the objects see discussion of long-term direction between these two approaches in #53483 /sig cli @kubernetes/sig-cli-pr-reviews /assign @pwittrock @soltysh ```release-note kubectl: --use-openapi-print-columns is deprecated in favor of --server-print ```
Change the name of the flag to something slightly more user-friendly,
while making it default true. The flag is kept so that someone can
revert it temporarly if things go wrong.
What this PR does / why we need it: OpenAPI can specify for some columns to be used in
kubectl get. That's very useful for non-core/hard-coded type in order to print more valuable data in get.Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Closes kubernetes/kubectl#143
Release note: