Proposal for alternate API representations of resources#33900
Proposal for alternate API representations of resources#33900smarterclayton wants to merge 1 commit intokubernetes:masterfrom
Conversation
|
Jenkins Kubemark GCE e2e failed for commit 4f8df74c351eb766c4d728de86653de83caa4848. Full PR test history. The magic incantation to run this job again is |
|
Jenkins GCI GCE e2e failed for commit 4f8df74c351eb766c4d728de86653de83caa4848. Full PR test history. The magic incantation to run this job again is |
4f8df74 to
398f592
Compare
Allow servers to respond with diffreent schemas for a given resource.
398f592 to
dede520
Compare
|
@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. |
|
Jenkins verification failed for commit dede520. Full PR test history. The magic incantation to run this job again is |
lavalamp
left a comment
There was a problem hiding this comment.
I have a few suggestions. Overall looks good.
There was a problem hiding this comment.
I think this will be big enough already without also tackling multi-resource returns at the same time :)
There was a problem hiding this comment.
We have to support list and watch of partial objects. That's not optional.
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
Correct - will clarify this is access to a single resource type, singular or plural resources
There was a problem hiding this comment.
Maybe leave items with fan-out to another proposal--that feels like step two.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
DeleteOptions
UpdateOptions (if it was a thing)
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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. | |
There was a problem hiding this comment.
I don't think "server group" is a common term yet, perhaps you can define it.
There was a problem hiding this comment.
why are sv and g separate?
There was a problem hiding this comment.
I think what you've called "api.k8s.io" I would call "core.k8s.io". We should probably figure out the canonical name.
There was a problem hiding this comment.
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`. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. | |
There was a problem hiding this comment.
perhaps do gv, and take the format version.group? Or is version intended to be optional?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
| | 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. | |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Can you list these 6 implementations that you have in mind?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Why does the Accept head need "g=api.k8s.io,v=v1,as=Scale"? The kind and version in the body should be enough?
There was a problem hiding this comment.
Controls the response from the server (show me the result of the PUT as Scale in server group).
lavalamp
left a comment
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
|
ref #1459 |
|
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
nit: should be /api/v1/namespace/example/replicationcontrollers or /apis/extensions/v1beta1/namespace/example/replicasets
|
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? |
|
After writing this I think #1459 is too generic - the advantage of this
approach is sticking to concrete schemas that can be represented in swagger
and optimized appropriately. I believe the actual selective use cases can
be solved without a partial field syntax (which is not portable across
proto and json) by judicious use of new objects. Also, uses cases like
spec and status can reuse the existing schema without needing full
flexibility. I'd argue that this gets 90% of the outcome of 1459 with
better control.
|
| "name": "foo", | ||
| "resourceVersion": "10", | ||
| ... | ||
| } |
There was a problem hiding this comment.
It will also return other fields of Namespace, not only metadata, right? (I mean, spec and status).
There was a problem hiding this comment.
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.
| }, | ||
| ... | ||
| ] | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
How about On Mon, Oct 3, 2016 at 5:55 PM, Clayton Coleman notifications@github.com
|
|
re: observed generation: I think we may need a better system than putting On Tue, Oct 4, 2016 at 10:46 AM, Daniel Smith dbsmith@google.com wrote:
|
|
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:
|
|
/sub |
|
Started with |
|
Adding label:do-not-merge because PR changes docs prohibited to auto merge |
|
Moving to kubernetes/community#123, will close this once I update with latest comments. |
|
[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files: We suggest the following people: |
|
Is this superseded by kubernetes/community#123? |
|
Yes but I still need to carry over comments.
On Jan 24, 2017, at 7:02 PM, Daniel Smith <notifications@github.com> wrote:
Is this superseded by kubernetes/community#123
<kubernetes/community#123>?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#33900 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p5kUxHfzYkCKuw1YbqwR3AevIpUpks5rVpDtgaJpZM4KMNwi>
.
|
|
OK, switching assignment so my dashboard stops confusing me :) |
| @@ -0,0 +1,422 @@ | |||
| <!-- BEGIN MUNGE: UNVERSIONED_WARNING --> | |||
There was a problem hiding this comment.
Please strip the warning block.
|
All comments integrated in the latest version of the community proposal. Closing this. |
Automatic merge from submit-queue. Proposal: Alternate API representations for resources Supports server side handling of transformation of resources. From kubernetes/kubernetes#33900
Allow servers to respond with different schemas for a given resource.
Addresses the following use cases with a common mechanism:
metadata needed by the generic Garbage Collector or the Namespace Lifecycle Controller
Scalecorrectly from a client across multiple APIgroups, versions, or servers
web or command-line clients to display (for
kubectl get)(for
kubectl describe)kubectl export)This change is