Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Kubelet: support retrieving stats using UID of mirror pods #5730

Merged
merged 1 commit into from
Mar 21, 2015
Merged

Kubelet: support retrieving stats using UID of mirror pods #5730

merged 1 commit into from
Mar 21, 2015
+240 −47

Conversation

yujuhong
Copy link
Contributor

Kubelet supports retrieving stats for pods/containers with and without UID.
This does not always work for the static pods because users may get the UIDs of
the mirror pods from the API server, and use them to query Kubelet. In this
case, Kubelet would fail to locate the containers due to mismatched UIDs.

This change adds a intenral mirror to static pod UID mapping and teaches all
public-facing functions to perform UID lookup before proceeding. This allows
users to use either mirror or static pod's UID to retrieve stats.

This fixes #5688

@vmarmol vmarmol self-assigned this Mar 20, 2015
@yujuhong
Copy link
Contributor Author

This PR is a bit cumbersome. The pod manager that I'm supposedly working on will help a lot.

vmarmol
vmarmol reviewed Mar 20, 2015
View reviewed changes
pkg/kubelet/kubelet.go
@@ -2176,8 +2200,23 @@ func (kl *Kubelet) StreamingConnectionIdleTimeout() time.Duration {
return kl.streamingConnectionIdleTimeout
}

func (kl *Kubelet) getStaticPodUID(uid types.UID) (types.UID, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets document why this exists in case future readers get confused :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@vmarmol
Copy link
Contributor

vmarmol commented Mar 20, 2015

Yeah it does look a bit brittle, but I think we knew that coming into it. Looks good overall, minor comments. Thanks @yujuhong!

@vmarmol
Copy link
Contributor

vmarmol commented Mar 20, 2015

LGTM, squash commits and we'll merge.

@yujuhong
Kubelet: support retrieving stats using UID of mirror pod
15e9760 
Kubelet supports retrieving stats for pods/containers with and without UID.
This does not always work for the static pods because users may get the UIDs of
the mirror pods from the API server, and use them to query Kubelet. In this
case, Kubelet would fail to locate the containers due to mismatched UIDs.

This change adds a intenral mirror to static pod UID mapping and teaches all
public-facing functions to perform UID lookup before proceeding. This allows
users to use either mirror or static pod's UID to retrieve stats.
@erictune
Copy link
Member

What is this for?

On Fri, Mar 20, 2015 at 3:08 PM, Victor Marmol notifications@github.com
wrote:

LGTM, squash commits and we'll merge.

—
Reply to this email directly or view it on GitHub
#5730 (comment)
.

@vmarmol
Copy link
Contributor

vmarmol commented Mar 20, 2015

@erictune fixing #5688. Mirror pods fail getting stats today because they have a different UID than their static equivalent. This does the conversion in Kubelet API requests.

@erictune
Copy link
Member

Who would get these stats? If you say "master", then: isn't the new
direction to have kubelet post pod status to its master rather than having
the master poll for status?

On Fri, Mar 20, 2015 at 3:49 PM, Victor Marmol notifications@github.com
wrote:

@erictune https://github.com/erictune fixing #5688
#5688. Mirror
pods fail getting stats today because they have a different UID than their
static equivalent. This does the conversion in Kubelet API requests.

—
Reply to this email directly or view it on GitHub
#5730 (comment)
.

@vmarmol
Copy link
Contributor

vmarmol commented Mar 20, 2015

Heapster uses this stats API

@yujuhong
Copy link
Contributor Author

Squashed and rebased.

Shippable's complaints seem irrelevant though.

pkg/kubelet/container/fake_runtime.go:26:2: cannot find package "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume" in any of:
    /home/shippable/.gvm/gos/go1.4/src/github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume (from $GOROOT)
    /home/shippable/workspace/src/github.com/GoogleCloudPlatform/kubernetes/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume (from $GOPATH)
    /home/shippable/workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume

@vmarmol
Copy link
Contributor

vmarmol commented Mar 20, 2015

@yujuhong yeah that seemed odd, I kicked it again and we'll see what it thinks.

@yujuhong
Copy link
Contributor Author

@erictune, heapster queries kubelet directly for container info.

@vishh
Copy link
Contributor

vishh commented Mar 20, 2015

Thanks for the quick fix @yujuhong!

@yujuhong
Copy link
Contributor Author

The head is broken :\ Other PRs are suffering too. This seems to be relevant to #5642.

@vmarmol
Copy link
Contributor

vmarmol commented Mar 20, 2015

:( @vishh do you know if anyone is working on a fix?

@yujuhong
Copy link
Contributor Author

I uploaded one: #5743

@vmarmol
Copy link
Contributor

vmarmol commented Mar 21, 2015

Build is green! Merging.

vmarmol added a commit that referenced this pull request Mar 21, 2015
@vmarmol
Merge pull request #5730 from yujuhong/static_stats
4d2e798 
Kubelet: support retrieving stats using UID of mirror pods
@vmarmol vmarmol merged commit 4d2e798 into kubernetes:master Mar 21, 2015
@yujuhong yujuhong deleted the static_stats branch March 26, 2015 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vmarmol vmarmol

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Kubelet fails to provide stats for static pods
5 participants
@yujuhong @vmarmol @erictune @vishh @googlebot