Retrieve machine spec from cAdvisor#491
Conversation
|
Renaming that in cAdvisor SGTM |
|
LGTM otherwise. |
|
OK. Hold on this PR. Let me do a rename in cAdvisor and send another commit to this PR. |
|
Just talked about this in google/cadvisor#92, we will only rename some functions in this PR.
PTAL. |
|
Rename looks good, still need a second review from someone. |
|
Thank you, @smarterclayton . I'll rebase it to solve the conflict |
|
Rebased and squashed all commits. ping @lavalamp @brendandburns |
There was a problem hiding this comment.
An idea: what do you think of this one-liner?
err = json.NewDecoder(response.Body).Decode(&minfo)
(appears multiple places)
There was a problem hiding this comment.
Yes. That would be better. Let me change it.
|
LGTM |
|
@lavalamp Done. PTAL. |
There was a problem hiding this comment.
Might as well change here, too.
|
@lavalamp Just did a grep and changed all places using json en/decoder. However, I noticed another issue: The unit test function names are not changed accordingly when we rename GetMachineInfo to GetRootInfo. I will do it in another commit. So please hold on this PR now. |
|
@lavalamp PTAL. (Remind me to do a squash when it's ready to merge.) |
|
LGTM; squash and let's merge |
|
@lavalamp Squashed. Thank you! |
|
CI passed. ping @lavalamp |
Retrieve machine spec from cAdvisor
|
Thanks for the change! |
WIP: Add utility to calculate derived stats.
Bug 1895329: UPSTREAM: 96751: Lower the frequency of volume plugin deprecation warning
This is the third piece of #448. It allows kubelet to retrieve machine spec (number of cores, total memory capacity.) from cAdvisor. The naming here is a bit confusing now: GetMachineInfo() and GetContainerInfo() returns info.ContainerInfo; while GetMachineSpec() returns info.MachineInfo. Probably we should rename MachineInfo to MachineSpec in cAdvisor? WDYT @vmarmol?