Skip to content

daemon: use OwnCgroupPath in withCgroups#48730

Merged
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:own-cgroup-path
Oct 23, 2024
Merged

daemon: use OwnCgroupPath in withCgroups#48730
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:own-cgroup-path

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Oct 23, 2024

- What I did

cgroups.InitCgroupPath is removed from runc (see 1), and it is suggested that users use OwnCgroupPath instead, because using init's is problematic when in host PID namespace (see 2) and is generally not the right thing to do (see 3).

Note: the code in question comes from commit 56f77d5 (part of PR #23430).

(This is included in PR #48160, and is needed to bump runc/libcontainer dependency to v1.2.0)

- How I did it

- How to verify it

Fingers crossed there are some tests in this repository.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Note: this usage comes from commit 56f77d5 (part of PR 23430).

cgroups.InitCgroupPath is removed from runc (see [1]), and it is
suggested that users use OwnCgroupPath instead, because using init's is
problematic when in host PID namespace (see [2]) and is generally not
the right thing to do (see [3]).

[1]: opencontainers/runc@fd5debf3
[2]: opencontainers/runc@2b28b3c2
[3]: opencontainers/runc@54e20217

Signed-off-by: Kir Kolyshkin <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Oct 23, 2024

Oh! Forgot about this one, yes I ran into this one when trying updated runc vendor, but wasn't sure if removing the fallback would be OK; thanks! #47668 (comment)

Copy link
Copy Markdown
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

Comment thread daemon/oci_linux.go
p := cgroupsPath
if useSystemd {
initPath, err := cgroups.GetInitCgroup("cpu")
path, err := cgroups.GetOwnCgroup("cpu")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like we don't currently import the "path" package, so we're good (I tend to avoid using path for variable names nowadays as it's usually waiting for a "I shadowing an import" warnings to happen)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants