cgroup2: map io.stats to v1 blkio.stats correctly #2968
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/cc @iyashu Does this seem reasonable? |
|
@iyashu can you please test it locally and give us a thumbs up here? |
|
@dims @cyphar lgtm. I've also tested this patch by recompiling the cadvisor with updated runc dependency. All |
iyashu
previously approved these changes
May 28, 2021
AkihiroSuda
previously approved these changes
May 30, 2021
|
Added a unit test. |
Kubelet and cAdvisor depend on the metrics having the same values as in cgroupv1, but we didn't correctly map the number of read and write IOs to the correct cgroupv1 stats table (blkio.io_serviced). In addition, don't leak any extra stats in our output -- if users need that information we can always add a new field for it. Reported-by: Yashpal Choudhary <[email protected]> Signed-off-by: Aleksa Sarai <[email protected]>
strconv.ParseUint(..., 0) is not really safe, because on 32-bit architectures it will trigger runtime errors when trying to parse large numbers (which in the case of the cgroupv2 io controller, is almost certainly going to happen). Fixes: 1932917 ("libcontainer: add initial support for cgroups v2") Signed-off-by: Aleksa Sarai <[email protected]>
|
Turns out there was a bug in our handling of 64-bit values on 32-bit architectures which the new unit test picked up. It's fixed now. |
Signed-off-by: Aleksa Sarai <[email protected]>
|
/ping @opencontainers/runc-maintainers This is ready for review now, and is the last PR for 1.0.0. |
AkihiroSuda
approved these changes
Jun 1, 2021
dqminh
approved these changes
Jun 1, 2021
|
Coolio. I'll send out the vote for 1.0.0 now. 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Kubelet and cAdvisor depend on the metrics having the same values as in
cgroupv1, but we didn't correctly map the number of read and write IOs
to the correct cgroupv1 stats table (blkio.io_serviced).
In addition, don't leak any extra stats in our output -- if users need
that information we can always add a new field for it.
Change-log entry:
Fixes #2967
Reported-by: Yashpal Choudhary [email protected]
Signed-off-by: Aleksa Sarai [email protected]