Skip to content

Conversation

kolyshkin
Copy link
Contributor

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: #3387
Fixes: 35d20c4

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
Copy link
Contributor Author

@frasertweedale PTAL

@frasertweedale
Copy link
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]>
@kolyshkin
Copy link
Contributor Author

@opencontainers/runc-maintainers PTAL 🙏🏻

}
} else {
filesToChown = append(filesToChown, "cgroup.procs", "cgroup.subtree_control", "cgroup.threads")
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we logrus.Debugf the err?

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thaJeztah thaJeztah merged commit 1515d93 into opencontainers:main Mar 4, 2022
@kolyshkin kolyshkin added backport/1.1-done A PR in main branch which has been backported to release-1.1 and removed backport/1.1-todo A PR in main branch which needs to be backported to release-1.1 labels Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cgroupv2 area/systemd backport/1.1-done A PR in main branch which has been backported to release-1.1 kind/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v2 cgroup delegation fails: memory.oom.group: no such file or directory

4 participants