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.

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

kubelet/fake_docker_client: Use self's PID instead of 42 in testing. #6653

Merged
merged 1 commit into from Apr 9, 2015

Conversation

yifan-gu
Copy link
Contributor

@yifan-gu yifan-gu commented Apr 9, 2015

This is safer to use self's PID than some arbitrary PID (say 42),
since the kubelet will set the oom_score_adj for real in kubelet.createPodInfraContainer.
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/kubelet/kubelet.go#L929

We can get rid of this in the future once we make the docker runtime to create pod infra container (thus we can add a stub in the fake docker)

Thanks!
@vmarmol @yujuhong

This is safer to use self's PID than some arbitrary PID (say 42),
since the kubelet will set the oom_score_adj for real.
@dchen1107
Copy link
Member

What's the problem you are trying to solve here? validate the oom_score_adj value?

@yifan-gu
Copy link
Contributor Author

yifan-gu commented Apr 9, 2015

@dchen1107 Just found this on my way of fixing tests for #6608

@lavalamp
Copy link
Member

lavalamp commented Apr 9, 2015

Why is os.Getpid() better than 42?

@lavalamp lavalamp self-assigned this Apr 9, 2015
@yifan-gu
Copy link
Contributor Author

yifan-gu commented Apr 9, 2015

@lavalamp Hmm, PID 42 is some other process on my machine. Getpid() is just the test process.
It does not make much difference since we cannot modify the /proc/42/oom_score_adj since we are not root now, but still makes me uncomfortable.

@lavalamp
Copy link
Member

lavalamp commented Apr 9, 2015

OK, I guess it doesn't make a difference to me. LGTM

lavalamp added a commit that referenced this pull request Apr 9, 2015
kubelet/fake_docker_client: Use self's PID instead of 42 in testing.
@lavalamp lavalamp merged commit f16abee into kubernetes:master Apr 9, 2015
@yifan-gu yifan-gu deleted the fix_pid_test branch May 7, 2015 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants