-
Notifications
You must be signed in to change notification settings - Fork 236
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
Added cgroups v2 CPU implementation. #107
Added cgroups v2 CPU implementation. #107
Conversation
@AkihiroSuda Can you add some acceptance criteria or checkpoints what should be done, for easier testing and validation for this and next PR's. |
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
=======================================
Coverage 45.42% 45.42%
=======================================
Files 23 23
Lines 1638 1638
=======================================
Hits 744 744
Misses 768 768
Partials 126 126 Continue to review full report at Codecov.
|
Please add to Line 32 in 6fb48e1
|
wrt tests I think we can add metrics code to cmd/cgroups-playground and call it a day. In future we should have proper Go tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code seems for v1
See https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/fs/cpu_v2.go
@AkihiroSuda The problem is that https://github.com/opencontainers/runtime-spec/blob/master/specs-go/config.go#L297 this config still not updated to v2. |
No, you can just convert |
de97206
to
a6dfc80
Compare
@AkihiroSuda PTAL |
thanks, just a couple of nits |
a6dfc80
to
cc3ea3c
Compare
@AkihiroSuda Thanks for your reviews. Done. |
cc3ea3c
to
1be6fb4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
1be6fb4
to
8f1875b
Compare
Signed-off-by: bpopovschi <zyqsempai@mail.ru>
One thing before we go farther on this. Should v2 be simpler where we don't have separate files/controllers, cpu.go/memory.go but a single manager.go with all the settings? |
Good idea, could you open PR? (maybe after merging this one) |
Ya, I think we should merge then one and then i can work on a unified manager :) |
LGTM @AkihiroSuda after this last edit, does it still LGTM for you? |
Partially fixes #104
Added cgroups v2 CPU implementation.
Signed-off-by: bpopovschi zyqsempai@mail.ru