WIP: Adding pod compute resource stats to kubelet API. #4685
Conversation
The same API will also expose stats for all the pods known to the kubelet.
|
@dchen1107: Posting this WIP for feedback before I add unit tests and e2e tests. |
| // ResourceStats represents compute resource usage metrics associated with a pod or a container. | ||
| type ResourceStats struct { | ||
| // Detailed instantaneous metrics. | ||
| Detailed *cadvisor_api.ContainerInfo `json:"detailed,omitempty"` |
There was a problem hiding this comment.
Should we decouple what we expose in the master API from the cadvisor API? The downside is you would end up with two data structures and a bunch of stupid copying between them, but the advantage is that you could evolve the cadvisor API without impacting the master API.
There was a problem hiding this comment.
The intent is to not expose all the stats via the master since it will be a lot of data. The kubelet API will be extended soon to expose historical usage distribution, which is what @rjnagal is working on. That will be something we will end up exposing via the master API. The copying overhead should be minimal I hope.
There was a problem hiding this comment.
cc/ @bgrant0607 we need to make the final decision on how to introduce resource related types to our API?
@vishh I am not comfortable to use cadvisor's API directly here, since it is introduced an extra dependency to cadvisor. But up to Brian to make the final call.
There was a problem hiding this comment.
FYI: Please ping me directly in the future. Mentioning me on github is no longer sufficient to rise about the noise.
Will discuss offline.
There was a problem hiding this comment.
IIUC, the only reason you touched pkg/api/types.go here was because Kubelet is reusing the types from master. Should we introduce a new API types file specifically for the kubelet, like @abhgupta did in #4674 for scheduler? Or maybe we can have three types files, one for master-only, one for kubelet-only, and one for shared (similar to how we factored the proto bufs for our internal system)? I think it's fine to share some data structures between master and kubelet as long as we think carefully about upgrades/evolution. The two are going to be talking to each other anyway so there's no escaping thinking about backward compatibility no matter what you do.
There was a problem hiding this comment.
What this current PR does is wrong in multiple ways -- and so are the current Kubelet APIs.
For now, I proposed that Vish expose the actual cadvisor API at a new API prefix. The cadvisor API has its own independent versioning. This would effectively make Kubelet a caching proxy for cadvisor.
This is a bigger discussion, which we should have in another forum.
|
ping, any new status on this PR? Should I take another look at it? |
|
Not yet Dawn. I will re-work this PR as per Brian's comments and ping you. On Tue, Mar 3, 2015 at 4:33 PM, Dawn Chen notifications@github.com wrote:
|
|
Is this PR active? If not please close it according to https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/devel/pull-requests.md |
|
Closing according to the mentioned policy. |
The same API will also expose stats for all the pods known to the kubelet.
DO NOT MERGE. Testing pending.