[1.2] runc delete: fix for rootless cgroup + ro cgroupfs #4531
+48
−8
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
An issue with runc 1.2.0 was reported to buildkit, in which runc delete returns with an error, with the log saying: > unable to destroy container: unable to remove container's cgroup: open /sys/fs/cgroup/snschvixiy3s74w74fjantrdg: no such file or directory Apparently, what happens is runc is running with no cgroup access (because /sys/fs/cgroup is mounted read-only). In this case error to create a cgroup path (in runc create/run) is ignored, but cgroup removal (in runc delete) is not. This is caused by commit d3d7f7d, which changes the cgroup removal logic in RemovePath. In the current code, if the initial rmdir has failed (in this case with EROFS), but the subsequent os.ReadDir returns ENOENT, it is returned (instead of being ignored -- as the path does not exist and so there is nothing to remove). Here is the minimal fix for the issue. Signed-off-by: Kir Kolyshkin <[email protected]> (cherry picked from commit db59489) Signed-off-by: lfbzhm <[email protected]>
If the sub-cgroup RemovePath has failed for any reason, return the error right away. This way, we don't have to check for err != nil before retrying rmdir. This is a cosmetic change and should not change any functionality. Signed-off-by: Kir Kolyshkin <[email protected]> (cherry picked from commit 12e06a7) Signed-off-by: lfbzhm <[email protected]>
Let's explain in greater details what's happening here and why. Signed-off-by: Kir Kolyshkin <[email protected]> (cherry picked from commit ba3d026) Signed-off-by: lfbzhm <[email protected]>
Draft
Signed-off-by: lfbzhm <[email protected]> (cherry picked from commit 11a33c5) Signed-off-by: lfbzhm <[email protected]>
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.
Backport: #4523
This is an alternative to #4520.
An issue with runc 1.2.0 was reported to buildkit and then here (#4518),
in which
runc deletereturns with an error, with the log saying:Apparently, what happens is runc is running with no cgroup access
(because /sys/fs/cgroup is mounted read-only). In this case error to
create a cgroup path (in runc create/run) is ignored, but cgroup removal
(in runc delete) is not.
This is caused by commit d3d7f7d, which changes the cgroup removal
logic in
RemovePath, and contains a bug. In the current code, if theinitial rmdir has failed (in this case with
EROFS), but the subsequentos.ReadDirreturnsENOENT, that error it is ultimately returned(instead of being ignored -- since the path does not exist, there is
nothing to remove).
This PR fixes the issue in 3 commits:
Only the first commit is changing the logic and fixing the issue -- the rest is mostly
cosmetic and "nice to have".
Fixes: #4518.
TODO: add a test case.