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

[dockerutil] cpu subsys will be mistaken for cpuacct, this fixes that. #2829

Merged
merged 1 commit into from Sep 9, 2016

Conversation

truthbk
Copy link
Member

@truthbk truthbk commented Sep 8, 2016

What does this PR do?

If we attempt to grab metrics from the cpu subsystem as is the case with the throttled metric introduced here #2724, we will currently fail to identify the correct cpu.stat file because the cpu subsystem will be mistaken for the cpuacct subsystem or similar. This PR attempts to address the issue for this and future metrics.

Also fixes badly indented function.

Motivation

Found the bug while testing #2724.

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Additional Notes

Anything else we should know when reviewing?

@truthbk truthbk added this to the 5.9.0 milestone Sep 8, 2016
@@ -279,7 +279,7 @@ def find_cgroup_from_proc(cls, mountpoints, pid, subsys, docker_root='/'):
if subsys in subsystems:
for mountpoint in mountpoints.itervalues():
stat_file_path = os.path.join(mountpoint, subsystems[subsys])
if subsys in mountpoint and os.path.exists(stat_file_path):
if subsys == mountpoint.split('/')[-1] and os.path.exists(stat_file_path):
Copy link
Member

Choose a reason for hiding this comment

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

good catch, you saved us hours of wondering why the metrics don't match! 🍪

@hkaj
Copy link
Member

hkaj commented Sep 9, 2016

LGTM

@truthbk truthbk merged commit 4f52969 into master Sep 9, 2016
@truthbk truthbk deleted the jaime/docker_throttle_fix branch August 2, 2017 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants