Skip to content

Proposal for alternate API representations of resources#33900

Closed
smarterclayton wants to merge 1 commit intokubernetes:masterfrom
smarterclayton:serverside
Closed

Proposal for alternate API representations of resources#33900
smarterclayton wants to merge 1 commit intokubernetes:masterfrom
smarterclayton:serverside

Conversation

@smarterclayton
Copy link
Copy Markdown
Contributor

@smarterclayton smarterclayton commented Oct 3, 2016

Allow servers to respond with different schemas for a given resource.

Addresses the following use cases with a common mechanism:

  • Retrieving a subset of object metadata in a list or watch of a resource, such as the
    metadata needed by the generic Garbage Collector or the Namespace Lifecycle Controller
  • Dealing with generic operations like Scale correctly from a client across multiple API
    groups, versions, or servers
  • Return a simple tabular representation of an object or list of objects for naive
    web or command-line clients to display (for kubectl get)
  • Return a simple description of an object that can be displayed in a wide range of clients
    (for kubectl describe)
  • Return the object with fields set by the server cleared (as kubectl export)

This change is Reviewable

@k8s-github-robot k8s-github-robot added kind/design Categorizes issue or PR as related to design. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Oct 3, 2016
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Jenkins Kubemark GCE e2e failed for commit 4f8df74c351eb766c4d728de86653de83caa4848. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Jenkins GCI GCE e2e failed for commit 4f8df74c351eb766c4d728de86653de83caa4848. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

Allow servers to respond with diffreent schemas for a given resource.
@smarterclayton smarterclayton changed the title WIP - Proposal for alternate API representations of resources Proposal for alternate API representations of resources Oct 3, 2016
@smarterclayton
Copy link
Copy Markdown
Contributor Author

@kubernetes/sig-api-machinery this is a draft for handling retrieving alternate representations of objects to solve the core use cases described in the proposal.

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Jenkins verification failed for commit dede520. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

Copy link
Copy Markdown
Contributor

@lavalamp lavalamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few suggestions. Overall looks good.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be big enough already without also tackling multi-resource returns at the same time :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to support list and watch of partial objects. That's not optional.

Copy link
Copy Markdown
Contributor

@lavalamp lavalamp Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but multiple objects in the same call at the same time? (I interpreted "one or more resources" as meaning different kinds of resources rather than multiple objects of the same resource type, which I think is what you meant.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct - will clarify this is access to a single resource type, singular or plural resources

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe leave items with fan-out to another proposal--that feels like step two.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to add an example to emphasize I don't believe it should be allowed to deviate from the pattern. Agree it doesn't have to be solved here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeleteOptions

UpdateOptions (if it was a thing)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/clietns/clients/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I want to move the legacy group from /api/v1 to /apis/core/v1 if I can talk @bgrant0607 into it. (Which really means serving it at both places for a really long time. But at least new components wouldn't have to build in an exception.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This group would only contain things that are API machinery or unversioned today, not pods or nodes.

| g | The group name of the desired response | Current group | The group the response is expected in. |
| v | The version of the desired response | Current version | The version the response is expected in. Note that this is separate from Group because `/` is not a valid character in Accept headers. |
| as | Kind name | None | If specified, transform the resource into the following kind (including the group and version parameters). |
| sv | The server group (`api.k8s.io`) version that should be applied to generic resources returned by this endpoint | Matching server version for the current group and version | If specified, the server should transform generic responses into this version of the server API group. |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think "server group" is a common term yet, perhaps you can define it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are sv and g separate?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you've called "api.k8s.io" I would call "core.k8s.io". We should probably figure out the canonical name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sv controls the schema I get back on errors (Status in api.k8s.io/v1). If a generic client asks for a response in a particular group version it may still want errors in a known schema, and for back compatibility we can't return Status in the server group by default.

the response the server provides, and over time the response may change and should therefore
be versioned. Our current API provides no way for a client to discover whether a `Scale`
response returned by `batch/v2alpha1` is the same as the `Scale` resource returned by
`autoscaling/v1`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When setting that up initially, we declared that clients had to know what the thing was, and that we'd version it via Accept/Content-Type. So what you're saying here was the original intent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and we need the separate group to give Scale a client discoverable schema.

| Name | Value | Default | Description |
| ---- | ----- | ------- | ----------- |
| g | The group name of the desired response | Current group | The group the response is expected in. |
| v | The version of the desired response | Current version | The version the response is expected in. Note that this is separate from Group because `/` is not a valid character in Accept headers. |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps do gv, and take the format version.group? Or is version intended to be optional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do slashes in Accept headers so we'd either need to do individual parts (g and v separate) or use the combined format from kubectl "resource.version.groupName". I prefer the decomposed version because the search rules for the combined format are complex.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think separating group and version where we can is good, except in cases where we can make an explicit argument that they should be consistent with apiVersion

in a tabular / descriptive format rather than raw JSON.

While the design of serverside tabular support is outside the scope of this proposal, a few
knows apply. The server must return a structured resource usable by both command line and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/knows/knowns/

| Name | Value | Default | Description |
| ---- | ----- | ------- | ----------- |
| kind | The kind of parameters being sent | `ListOptions` (GET), `DeleteOptions` (DELETE) | The kind of the serialized struct, defaults to ListOptions on GET and DeleteOptions on DELETE. |
| apiVersion | The API version of the parameter struct | `api.k8s.io/v1` | May be altered to match the expected version. Because we have not yet versioned ListOptions, this is safe to alter. |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

groupVersion? Would like to avoid making more things called apiVersion when they actually include the group, too.

also I'd argue again for v1.core.k8s.io.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going for strict compatibility with our core object schema. I'd rather use apiVersion consistently on resource operations rather than mix explanations (use apiVersion when you serialize an object to JSON but groupVersion when you serialize an object to query parameters seems wrong to me).

a predictable schema.

We also foresee increasing difficulty in building clients that must deal with extensions -
there are at least 6 known web-ui or CLI implementations that need to display some
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you list these 6 implementations that you have in mind?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kubectl, kompose, helm, dashboard, OpenShift web UI (developer console), OpenShift Eclipse client. Will create a small list here as a point in time capture

performant when controllers like the Garbage collector retrieve multiple objects.

GET /api/v1/namespaces
Accept: application/json;g=api.k8s.io,v=v1,as=PartialObjectMetadata, application/json
Copy link
Copy Markdown
Contributor

@caesarxuchao caesarxuchao Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I want to get multiple fields, e.g., ObjectMeta+Status.ObservedGeneration, do I need to define another partial API kind? That doesn't sound very flexible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice I believe it's safer and more correct to have a finite set of combinations targeted towards special use cases, rather than some sort of "meta retriever" that takes string JSONPath expressions. For instance, what do you need ObservedGeneration for? To properly implement GC on a subset of controllers that are reactive? The fields that are necessary for GC are just another variant of Scale - an interface for a particular use case. If ObservedGeneration means slightly different things per controller object, the right way is to declare the interface each object supports and expose the correct Og.

Specific schemes (like proto) can be optimized and generated. Generic ones can't. I would argue there are probably no more than 8 generic interfaces over the next year, and it is simpler and easier to generate the right documentation and outcomes for those interfaces to require concrete schemes than to assume field selectors can correctly identity the subset.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Garbage collector needs the ObservedGeneration to know if the controller of the resource (e.g., replicaset) has observed the resource is being deleted, and thus stops creating more dependents (e.g., pods).

I probably can find other approaches so that GC doesn't need to watch ObjectMeta and Status.ObservedGeneration at the same time. I'm asking a general question, and the reasons you listed are reasonable.

What is "Og"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Og = observed generation.

If we wanted to make controllers identify which generation they've observed formally we could always add it to PartialObjectMeta. But we would need to distinguish between controller knows and controller doesn't know (*int64 maybe) and what would GC do if the controller doesn't set OG?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the controller doesn't set OG, there will be race between GC and the controller, then GC can't guarantee to orphan/delete all the dependents.

Adding OG to PartialObjectMeta and typed it as *int64 sound good.

the server supports accepting that resource as well, and performs a PUT:

PUT /api/v1/namespace/example/replicasets/test/scale
Accept: application/json;g=api.k8s.io,v=v1,as=Scale, application/json
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the Accept head need "g=api.k8s.io,v=v1,as=Scale"? The kind and version in the body should be enough?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Controls the response from the server (show me the result of the PUT as Scale in server group).

Copy link
Copy Markdown
Contributor

@lavalamp lavalamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This group would only contain things that are API machinery or unversioned today, not pods or nodes.

Ah, what do you think about meta.k8s.io for that sort of thing? api is far too overloaded in our system and it'd be good to be more precise.

Copy link
Copy Markdown
Contributor

@lavalamp lavalamp Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but multiple objects in the same call at the same time? (I interpreted "one or more resources" as meaning different kinds of resources rather than multiple objects of the same resource type, which I think is what you meant.)

@bgrant0607
Copy link
Copy Markdown
Member

ref #1459

@bgrant0607
Copy link
Copy Markdown
Member

Another example: #17333

### Example: Partial metadata retrieval

The client may request to the server to return the list of namespaces as a
`PartialObjectMetadata` kind, which is an object containing only `ObjectMeta` and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just call it ObjectMetadata?
That way any client knows that if it requires a field X from namespaces, it can go a GET /api/v1/namespaces with Accept as=X.
How does it know that it needs to set as=PartialObjectMetadata for getting ObjectMetadata.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The distinction here is that the type has to have a schema, so the type returned has to have TypeMeta, which ObjectMetadata does not have.

Chai and I used Partial as the top level type for his PR - name can be changed, but prefer to have Go struct name match Kind.

Under this proposal, to scale a generic resource a client would perform the following
operations:

GET /api/v1/namespace/example/replicasets/test/scale
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should be /api/v1/namespace/example/replicationcontrollers or /apis/extensions/v1beta1/namespace/example/replicasets

@smarterclayton
Copy link
Copy Markdown
Contributor Author

I'm torn on "meta.k8s.io" - I understand the argument, but I also see the documentation:

"The API group for resources that span all API groups and are common to Kubernetes as a whole is: X "

I also considered "server.k8s.io" but that may overlap with other concepts.

I also think this group should hold discovery, and those are Kube API related things. I kind of feel meta is too... meta?

@smarterclayton
Copy link
Copy Markdown
Contributor Author

smarterclayton commented Oct 4, 2016 via email

"name": "foo",
"resourceVersion": "10",
...
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will also return other fields of Namespace, not only metadata, right? (I mean, spec and status).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I should have clarified. It would not:

type PartialObjectMetadata struct {
  TypeMeta `json:",inline"`
  ObjectMeta `json:"metadata"`
}

is the definition of the versioned PartialObjectMetadata. The client asks for a Kind and gets a Kind.

},
...
]
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to clarify one thing - IIUC, the value of "as" has to be understood by the server. This means that using this mechanism we will be able to get some well-defined parts of the objects (e.g. metadata), but not the random fields of objects (so in other words, it's not a generic mechanism to support sending only selected fields of objects to client). Right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, the design described here is "ask the server to return an object matching a known schema", not "ask the server to return parts of an existing schema". I will add that to alternatives to describe in more detail why I think the more generic option is worse for clients and for us in the near term.

@lavalamp
Copy link
Copy Markdown
Contributor

lavalamp commented Oct 4, 2016

How about universal.k8s.io?

On Mon, Oct 3, 2016 at 5:55 PM, Clayton Coleman notifications@github.com
wrote:

I'm torn on "meta.k8s.io" - I understand the argument, but I also see the
documentation:

"The API group for resources that span all API groups and are common to
Kubernetes as a whole is: X "

I also considered "server.k8s.io" but that may overlap with other
concepts.

I also think this group should hold discovery, and those are Kube API
related things. I kind of feel meta is too... meta?


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#33900 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAnglmOBq44Txx0gj1HlWexqXD-fkUaaks5qwaQQgaJpZM4KMNwi
.

@lavalamp
Copy link
Copy Markdown
Contributor

lavalamp commented Oct 4, 2016

re: observed generation: I think we may need a better system than putting
it in status. I think we may actually need a []struct{controller string; OG
int64}, for the case where multiple controllers are acting on an object.

On Tue, Oct 4, 2016 at 10:46 AM, Daniel Smith dbsmith@google.com wrote:

How about universal.k8s.io?

On Mon, Oct 3, 2016 at 5:55 PM, Clayton Coleman notifications@github.com
wrote:

I'm torn on "meta.k8s.io" - I understand the argument, but I also see
the documentation:

"The API group for resources that span all API groups and are common to
Kubernetes as a whole is: X "

I also considered "server.k8s.io" but that may overlap with other
concepts.

I also think this group should hold discovery, and those are Kube API
related things. I kind of feel meta is too... meta?


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#33900 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAnglmOBq44Txx0gj1HlWexqXD-fkUaaks5qwaQQgaJpZM4KMNwi
.

@smarterclayton
Copy link
Copy Markdown
Contributor Author

Will we get sued for using "universal"? "common.k8s.io" is another option.

On Tue, Oct 4, 2016 at 1:52 PM, Chao Xu notifications@github.com wrote:

@caesarxuchao commented on this pull request.

In docs/proposals/alternate-api-representations.md
#33900:

+For both export and the more complicated server side kubectl get cases, it's likely that
+more parameters are required and should be specified as query parameters. However, the core
+behavior is best represented as a variation on content-type. Supporting both is not limiting
+in the short term as long as we can validate correctly.
+
+
+### Example: Partial metadata retrieval
+
+The client may request to the server to return the list of namespaces as a
+PartialObjectMetadata kind, which is an object containing only ObjectMeta and
+can be serialized as protobuf or JSON. This is expected to be significantly more
+performant when controllers like the Garbage collector retrieve multiple objects.
+

  • GET /api/v1/namespaces
  • Accept: application/json;g=api.k8s.io,v=v1,as=PartialObjectMetadata, application/json

If the controller doesn't set OG, there will be race between GC and the
controller, then GC can't guarantee to orphan/delete all the dependents.

Adding OG to PartialObjectMeta and typed it as *int64 sound good.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#33900, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p6UUjinf4ukKM4xI92CGOupVW6-5ks5qwpJYgaJpZM4KMNwi
.

@k8s-github-robot k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 10, 2016
@apelisse apelisse removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 11, 2016
@soltysh
Copy link
Copy Markdown
Contributor

soltysh commented Nov 28, 2016

/sub

@smarterclayton
Copy link
Copy Markdown
Contributor Author

Started with meta.k8s.io / meta which seems fine. We use meta in enough places it doesn't feel terribly weird.

@k8s-github-robot
Copy link
Copy Markdown

Adding label:do-not-merge because PR changes docs prohibited to auto merge
See http://kubernetes.io/editdocs/ for information about editing docs

@k8s-github-robot k8s-github-robot added kind/design Categorizes issue or PR as related to design. kind/old-docs do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Dec 1, 2016
@smarterclayton
Copy link
Copy Markdown
Contributor Author

Moving to kubernetes/community#123, will close this once I update with latest comments.

@k8s-github-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files:

We suggest the following people:
cc @thockin
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@lavalamp
Copy link
Copy Markdown
Contributor

Is this superseded by kubernetes/community#123?

@smarterclayton
Copy link
Copy Markdown
Contributor Author

smarterclayton commented Jan 25, 2017 via email

@lavalamp lavalamp assigned smarterclayton and unassigned lavalamp Jan 26, 2017
@lavalamp
Copy link
Copy Markdown
Contributor

OK, switching assignment so my dashboard stops confusing me :)

@@ -0,0 +1,422 @@
<!-- BEGIN MUNGE: UNVERSIONED_WARNING -->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please strip the warning block.

@smarterclayton
Copy link
Copy Markdown
Contributor Author

All comments integrated in the latest version of the community proposal. Closing this.

k8s-github-robot pushed a commit to kubernetes/community that referenced this pull request Oct 12, 2017
Automatic merge from submit-queue.

Proposal: Alternate API representations for resources

Supports server side handling of transformation of resources.

From kubernetes/kubernetes#33900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. kind/design Categorizes issue or PR as related to design. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.