do not create init-dir if not needed #29846
Conversation
daemon/daemon_unix.go
Outdated
There was a problem hiding this comment.
This recursive call confused me; not sure if that was needed, but perhaps I misunderstood 😊
There was a problem hiding this comment.
I think the recursion is needed because all of the parents have to have a value <= than the targeted one in the path for this to work if I'm not mistaken
There was a problem hiding this comment.
Oh, right, so a cgroup has to be created at every level if it doesn't exist? (I don't think this function actually checks it's values, only creates)
There was a problem hiding this comment.
Yes, from what I could gather from the original code.
Also, reading the original PR, the following is in the description:
You will also need to ensure that the system and all parent cgroups have values set for the period and runtime as this limits what the children can be set to.
There was a problem hiding this comment.
I added back the recursive call, but moved it after the new checks
|
I can confirm that this fixes #29833 |
|
Thanks @lrusak! Now we need to check if it still does everything that it needs to do 😊 not too familiar with this part, so I'm awaiting some reviews 😄 |
commit 56f77d5 added support for cpu-rt-period and cpu-rt-runtime, but always initialized the cgroup path, even if not used. As a result, containers failed to start on a read-only filesystem. This patch only creates the cgroup path if one of these options is set. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
a4554d4 to
f285d5b
Compare
mlaventure
left a comment
There was a problem hiding this comment.
LGTM
TestSwarmContainerAutoStart seems to still be flaky unfortunately.
|
Can this go in for 1.13 as a bug fix? |
|
We've frozen 1.13.0 rc's to only add critical issues, but if there's a patch release for 1.13, i'll bring this up for consideration |
fixes #29833
Commit 56f77d5 (#23430) added support for cpu-rt-period and cpu-rt-runtime, but always initialized the cgroup path, even if not used.
As a result, containers failed to start on a read-only filesystem.
This patch only creates the cgroup path if one of these options is set.
ping @erikstmartin @mlaventure PTAL. I think this should still do all that's required, but please double check.