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

WIP: Proposal for moving "printers" (kubectl get) to the server #29472

Closed
smarterclayton opened this issue Jul 22, 2016 · 12 comments
Closed

WIP: Proposal for moving "printers" (kubectl get) to the server #29472

smarterclayton opened this issue Jul 22, 2016 · 12 comments
Labels
area/kubectl sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@smarterclayton
Copy link
Contributor

smarterclayton commented Jul 22, 2016

Still WIP

As mentioned in #12143 (and #11617) we want to move printers to the server so that all clients can benefit from them, the naive client implementation is simpler, and so we can avoid having to perform external -> internal conversions in the client.

Considerations

  • A kubectl get is an alternative representation of the underlying API object or objects - in HTTP and REST this is most commonly treated as another representation of the same resource.
    • We need to support alternate representations of both the list (kubectl get pods) and the individual item (kubectl get pod).
  • The representation returned must be structured (an API object) and we need to potentially handle the desire to change that API over time.
  • We want to support both command line clients and web clients, and that requires an internal structure
    • command line clients may add additional value like wrapping within the terminal window boundaries, or showing and hiding headers
    • web clients will wish to transform the response in HTML and need a structured format
  • We already enable clients dealing with JSON and YAML - a serialization in either of those formats would be easily consumed (with a preference for JSON)
  • We will need to support additional parameters to these representations, parameters that only make sense when viewing this representation (for instance, wide vs narrow). Other parameters may be common (paging, sorting, filtering).
  • There are two primary mechanisms for conveying "alternate return type of content" for RESTful objects.
    • One is a query parameter (?table=1) and one is the content type (Accept: application/json;tabular=1).
    • We have support for both mechanisms - the former is easier for naive clients, the latter is more "correct" in HTTP vocabulary.
    • We support Accept: application/json;pretty=1 and ?pretty=1 today, which are roughly equivalent.
  • We already have export, which is implemented as a query parameter. Export always returns the same object schema as the normal GET request.
  • We have watch, which is implemented as a query parameter, but which returns a different content-type and object schema as a normal GET request.
  • We want the serialization choice to be obvious to a swagger API consumer (which means it can either be content type or query parameter, although query parameters have more doc).
  • Since this is a variant of the object, I believe we should avoid a subresource (since we would have to create a subresource of a list, which would conflict with resource names).

Proposed:

Support both content type and query parameter for selecting an alternate representation of the object. Supporting content-type means we can in the future add new representations AND allow the user to request multiple serializations in order.

Example:

$ curl -H "Accept: application/json;table=wide" https://server/v1/namespaces/default/pods?table=wide
Content-Type: application/json;table=wide
{
  "kind": "Table",
  "apiVersion": "printers.k8s.io/v1",
  "metadata": {
    "resourceVersion": "395",
    "selfLink": "https://server/v1/namespaces/default/pods"
  },
  "columns": [
    {"name": "Name"},
    {"name": "Age"},
  ],
  "items": [
    {
      "values": ["hello", "3d"],
    }
  ]
}

ISSUE: Do we want to continue the inlining of schemas (return v1 for v1 resources), or explicitly return a different schema type? The latter is easier for generic clients. The former is easier for versioned clients.

It would be valid to specify ?watch=1 or ?export=1 with ?table= in the future. Tabular output is a transformation of the resulting object from a Get.

This is a reasonably simple table schema (it's possible to extend values in the future) but in the near term additional descriptive metadata could be hung off "columns". The use of the table parameter aligns with the requested schema. For instance, the "default filter" concept needs to be implemented server side, which means each row needs a descriptive field (filtered or not).

Implementation (more details needed)

  • Server
    • Create a new API group printers.k8s.io with a v1 version that accepts this object.
    • Add a new method TransformTable(context api.Context, obj runtime.Object) (api.Table, error) to RESTStorage
    • Provide a default implementation of TransformTable on generic.Etcd that handles fields supported by ObjectMeta
  • Client
    • Move kubectl.Printers into a package outside of pkg/kubectl where it can be reused by both
    • Add functionality to resource.Helper to request tabular output.
      • Invoke DecodeInto(..., &v1.Table{}) to decode the response (which should error if v1.Table is not recognized)
    • Alter kubectl get to invoke the generic helper.
@smarterclayton
Copy link
Contributor Author

Still in progress

@adohe-zz
Copy link

@smarterclayton I think it would be better to list some work items here so other people can pick and help. wdyt?

@k8s-github-robot k8s-github-robot added area/kubectl sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Aug 3, 2016
@smarterclayton
Copy link
Contributor Author

Going to write up a proposal shortly for the API part, then start suggesting changes into smaller work items. Joe Beda had a good suggestion for describe that resolves some of the issues we had with it.

@adohe-zz
Copy link

Really look forward to this :)

@errordeveloper
Copy link
Member

I like this approach a lot. Given that this has been open for a while and hasn't seen much activity since, would it make sense to attempt building this as a 3rd-party resource first? Firstly, this approach would allow more room for experimentation without much in the way, like it can be implemented by a novice to API machinery and agains a released version of Kubernetes. Also, I can imagine that re-mapping can be done in lots of clever ways, so it'd be fun to work on.
Additionally, I see how there is some benefit for having this triggered with Accept header and/or query parameter, but it would make the endpoint handler more complex and presumably make API spec more complex as well as require changes to API machinery and all that stuff, so having it more external by design would be easier, wouldn't it?

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Dec 31, 2016 via email

@brendandburns
Copy link
Contributor

Is there a more concrete proposal/design for this somewhere? I'm a little concerned we're rushing into a fairly significant change without much discussion or feedback beyond what is in this issue.

@errordeveloper
Copy link
Member

errordeveloper commented Feb 7, 2017

I just reminded myself that Elasticsearch has a similar things, it's called "cat API", I think it's rather quite neat and we should consider it as an example.

@brendandburns
Copy link
Contributor

A couple other things we need to make certain of:

  1. I'm adding internationalization support, if we're pretty printing on the server, it needs to take language as a parameter so that it can translate appropriately.

  2. I'm adding colorization support, again, we likely want the format here to work for colorization too.

I suspect there will be a number of these kinds of features, where we need to pass a great deal of information from the client back to the server just so the server can render something properly.

In general, this means I have a bunch of concerns about this. It's fine to add a simple table API if we want, but I'm quite worried that we will sacrifice user experience in the client in exchange for server side rendering.

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Feb 8, 2017 via email

@pwittrock
Copy link
Member

After syncing up offline, this makes sense to me and is complementary with the using the approach described here as a fallback.

@smarterclayton
Copy link
Contributor Author

Is almost merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubectl sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

No branches or pull requests

6 participants