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
Properly set object status #2726
Comments
We should be writing the status to etcd, and when the performance of accessing etcd becomes a bottleneck, we should serve the objects from an in-memory cache. |
Not only would this make it possible to WATCH status, but this would also address issues like #2882 and #2532 where the apiserver has different behavior from the object's implementation, and would facilitate plugins (#2594). The apiserver should be dumb. Even the registry shouldn't have object-specific business logic. Whatever component(s) implements the business logic for the object (e.g., Kubelet for pods, controller manager for replicationControllers) should proactively periodically compute the status and PUT it to the apiserver, which would update an in-memory cache and write through to etcd. GET should just return the latest cached value, not compute it on demand. This provides another motivation for exposing sub-parts of objects at their own endpoints (.../status, .../spec), or PATCH. |
In case another issue doesn't cover this: We also shouldn't let the user POST/PUT operations trash status. |
So PUT needs to know who is doing the PUTing? If user, only spec may change, if particular service account(s), only status may change? That sounds hard and would block on auth policies. Providing parts of objects at their own endpoints sounds hard, too... |
Parts of resources doesn't sound that hard - it lets you policy them separately and define clear semantics. For Derek's admission/resource controller he's separating the "spec" (this is what you're allowed) vs what you've observed in use "status", and status is set via a different endpoint. It seems cleaner so far than having it in spec and using policy to control attributes, because clients can reason about the rest resource as a unit rather than the attributes. My 0.02
|
I may be hallucinating but I thought at Kubercon we decided to store these things as separate objects and make them be separate endpoints and then have some kind of virtual endpoint that joined them? |
We certainly had a bunch of discussions to that end - probably want to lay it out and make sure everyone is happy
|
Ignoring auth, we wouldn't need to distinguish who was doing the PUTing if users only POSTed/PUT the Spec and components only POSTed/PUT the Status. The whole object could be a readonly view. Certainly this would be a major change. |
The decomposed PUT certainly is more consistent across resources, and definitely allows dumber clients. Most users would set spec for sure (if designed well). Setting status seems like the exceptional case - where mutating status might just always be a differentiator. ----- Original Message -----
|
/cc @satnam6502 |
I think one of the strong points of rest is that it's ok to have resources (identified by name) that return different representations of the same object and expose different mutation mechanisms. What complicates /pods/spec and /pods/status is that the vast majority of restful client libraries are simple - they can't handle things like nested sub resources, or if they do, they do it subtly different ways. Generally you gear your resources to what you expect users to transform. As an example, /pods works great because users mostly set the spec and mostly read the status. The intended users of /pods is pod consumers. The other use case for a pods endpoint is status owners (the people who own the status that the pod exposes). I don't know that there's as strong of a use case for /pod/spec (avoiding the cost to calculate status, maybe). Doing "/podspecs" and "/podstatus" simplifies client implementations a lot - you can more easily wire in "POST on /podstatus accepts PodStatus", whereas "POST on /pods accepts Pod" - you know what the type should be. It's harder to infer what "/pod/spec" returns in a generic fashion. None of those are blocking, just considerations. |
I was exactly thinking of different representations for the same resources. I'd be fine with /podstatus rather than /pod/status. I think the main motivation for /podspec would be to simplify read-modify-write sequences. PATCH and filtering/projection (#1459) would be alternatives to that. Internally, we use RPC rather than REST, and all read and write operations are inherently separate, as are reads of desired vs. current status (spec vs. status). I agree that separate operations are less amenable to generic meta-programming over the API, however, so probably we should just make slicing and dicing work easily and efficiently. |
I certainly have no objects to /podspec as an endpoint. Is the sliced up resource just PodSpec, or must it have metadata? Can I POST to PodSpec (needs metadata), or just PUT? Can I delete PodSpec? Seems like just allowing PUT/PATCH/GET would be 90% of the use cases you mention. ----- Original Message -----
|
Node status sync is blocked on this somehow. Agree that apiserver should be dumb, and I certainly want to do a PUT to /nodestatus or /node/status; however, this seems to be a larger change. I think the convention here is that things under root path is a k8s resource; so to use /podstatus, are we going to make it a |
Anything we accept or return probably needs to have an api version and kind embedded in it. We also at minimum need to have Name and Namespace somehow (unless we fix RESTStorage so that you can pass down the Name that the apiserver took off the URL). We probably need to return ResourceVersion in order to do atomic updates. We don't want to add those to PodStatus directly I don't think. One option is:
Is objectmeta useful in this context? Automated processes may want to set annotations or labels. On the other hand, if we defined very specific "facet" handlers (which is what /node/status /node/spec could be) you could define a different type of handler that is simpler and more targeted:
Changing semantics means clients have to be different, but clients could be more focused... ----- Original Message -----
|
I think I'm leaning towards '/node/status' approach. NodeStatus is not a standalone resource, it's a subobject of Node. Namespace, SelfLink, etc do not make sense for NodeStatus, and even if some of them do make sense, they are potentially duplidate with that of Node. Further, we'll need to connect Node with NodeStatus externally (other than struct embedding), which could be non-trivial work. Looking at our client implementation, it's not flexible to update status only. One option would be to change NodeInterface to include an extra status update field:
or create a new NodeStatusInterface.
The latter might be better as adding UpdateStatus directly to NodeInterface sounds like status is updatable for client. No matter what, I'd think the change is less intrusive and cleaner than a new object. As for server side, I think there is no need for another REST path. How about generalize context object? |
I wouldn't add |
Ah, yeah, not familiar with |
The challenge with PATCH is you still have define the type of object the server expects, and clients have to have a consistent object model as well. If that model is NodeStatus, the a separate resource path is appropriate because the path and resource is consistent. I assume we want to leave open the door in the future to use patch for single atomic field updates: "change pod status phase to foo". To do that, you really don't want to use the full pod status field, since you need to distinguish between "nil this field" and "do nothing for this field". But to do that, you have to define a third type of model that isn't Pod or PodStatus.
|
If we add another type for updating PodStatus, then we'll need another one for NodeStatus, then PodSpec, etc; is there a way we can avoid the pain? To make it more flexible, can we do something like this:
This may require a lot of reflection, not sure if this is doable. |
Filed #5000 for /binding. There are already issues for /size and stop, and this issue is about /status. If we want to do something similar for /events and /endpoints, those would be 2 new issues. |
Cool. Clone/instantiate is covered by #170. |
Can someone (maybe Clayton?) summarize the status of this issue? If this has become a meta-issue, let's file any necessary sub-issues and close this. Otherwise, what remains to be done here for 1.0? |
I don't know that we have a meta issue covering "cluster mutation is watchable, and the status of the cluster is observable from the API server without passthrough hacks" except this one. Will try to summarize if brian doesn't get here first. |
Quick status AIUI:
|
PodTemplate would support /instantiate. I need to do some prep work for /size to allow easy implementation. ----- Original Message -----
|
Namespace Status is posted via a subresource |
Thanks for the summary. So, concretely, what remains as a 1.0 blocker? Just the things from Brian's list minus the entries that say the item is done? |
I had hoped that all status would be posted via subresources in the v1 API. Right now the API is in an intermediate state. I think finishing the remaining resources is mainly additive and therefore wouldn't need to be done for 1.0. Whether via a subresource or via posting the full resource is mostly an implementation detail. What matters for clients is the ability to watch status changes, which just requires status to be posted by some means. Remaining things to be fixed for 1.0 can be driven by usability and performance problems. |
Using more specific issues for the remainder of the work. |
As per discussion #2665 (comment), we currently haven't set object status for watch command. The status should be set properly before returning.
The text was updated successfully, but these errors were encountered: