-
Notifications
You must be signed in to change notification settings - Fork 347
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
Fix emulated cgroups v1 subsystem when running docker-in-docker #2532
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2532 +/- ##
==========================================
+ Coverage 65.87% 65.89% +0.01%
==========================================
Files 133 133
Lines 16868 16892 +24
==========================================
+ Hits 11112 11131 +19
- Misses 5756 5761 +5 |
Signed-off-by: Jorge Prendes <[email protected]>
Signed-off-by: utam0k <[email protected]>
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.
It looks good to me. Let's continue to add a test on this PR if you want.
Yes, that's a good plan :-) |
Hey, should we go ahead and merge this fix as it is? We can add the tests in a separate PR if you want 👍 |
I'd like to include this PR in the next patch release, so please send out another PR 🙏 |
Will do. The tests will need the musl builds PR to be merged. I'll work on that first. |
Fixes #2528.
Normally the pid-1 cgroup path is empty, but this is not the case when running inside a container.
This needs to be accounted on for the cgroups subsystem emulation.
This PR fixes the source of the mountpoints for the emulated cgroups subsystem by stripping the cgroup paths of pid-1 from the mountpoint source path.
I also makes the failure to setup tracing to syslog a warning instead of fatal, as that fails with the
docker
image used for docker-in-docker.I would like to add testing for this based on the reproduction steps in #2528, but that would be better done by building
youki
as a static binary usingmusl
(since thedocker
image is based onalpine
). I would like to wait until we merge #2498 to addmusl
support and add a test.