libct/cg/sd/v2: fix ENOENT on cgroup delegation#3389
Merged
thaJeztah merged 1 commit intoopencontainers:mainfrom Mar 4, 2022
Merged
libct/cg/sd/v2: fix ENOENT on cgroup delegation#3389thaJeztah merged 1 commit intoopencontainers:mainfrom
thaJeztah merged 1 commit intoopencontainers:mainfrom
Conversation
Apparently, not all files listed in /sys/kernel/cgroup/delegate must exist in every cgroup, so we should ignore ENOENT. Dot not ignore ENOENT on the directory itself though. Change cgroupFilesToChown to not return ".", and refactor it to not do any dynamic slice appending in case we're using the default built-in list of files. Fixes: 35d20c4 Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
added
kind/bug
area/cgroupv2
area/systemd
backport/1.1-todo
Contributor
Author
|
@frasertweedale PTAL |
frasertweedale
approved these changes
Feb 22, 2022
Contributor
|
/lgtm Thanks for the fix @kolyshkin . I'll also create a PR against runtime-spec to add a note about the possibility of missing files. |
frasertweedale
added a commit
to frasertweedale/runtime-spec
that referenced
this pull request
Feb 22, 2022
Not all files listed in /sys/kernel/cgroup/delegate necessarily exist in all cgroups. For example, see this issue and PR: - opencontainers/runc#3387 - opencontainers/runc#3389 Expand the cgroup ownership semantics to ensure that runtime authors are aware of this possibility and implementations handle it gracefully. Signed-off-by: Fraser Tweedale <[email protected]>
Contributor
Author
|
@opencontainers/runc-maintainers PTAL 🙏🏻 |
AkihiroSuda
reviewed
Mar 4, 2022
| } | ||
| } else { | ||
| filesToChown = append(filesToChown, "cgroup.procs", "cgroup.subtree_control", "cgroup.threads") | ||
| if err != nil { |
Member
There was a problem hiding this comment.
Can we logrus.Debugf the err?
AkihiroSuda
approved these changes
Mar 4, 2022
kolyshkin
added
backport/1.1-done
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Apparently, not all files listed in
/sys/kernel/cgroup/delegatemustexist in every cgroup, so we should ignore ENOENT.
Dot not ignore ENOENT on the directory itself though.
Change
cgroupFilesToChownto not return., and refactor it to not doany dynamic slice appending in case we're using the default built-in
list of files.
Fixes: #3387
Fixes: 35d20c4