Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix systemd full path #221

Merged
merged 1 commit into from
Feb 21, 2022
Merged

Fix systemd full path #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions v2/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,12 +701,39 @@ func setDevices(path string, devices []specs.LinuxDeviceCgroup) error {
return nil
}

// getSystemdFullPath returns the full systemd path when creating a systemd slice group.
// the reason this is necessary is because the "-" character has a special meaning in
// systemd slice. For example, when creating a slice called "my-group-112233.slice",
// systemd will create a hierarchy like this:
// /sys/fs/cgroup/my.slice/my-group.slice/my-group-112233.slice
func getSystemdFullPath(slice, group string) string {
Comment on lines +704 to +709
Copy link
Member

Choose a reason for hiding this comment

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

Do you know the corresponding code in systemd? Is it documented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to find documentation from systemd itself and look thru the code. What I have so far is just observation that this is what the resulting cgroup path created by NewSystemd is, and this doc from redhat, which states:

Service, scope, and slice units directly map to objects in the cgroup tree. When these units are activated, they map directly to cgroup paths built from the unit names. For example, the ex.service residing in the test-waldo.slice is mapped to the cgroup test.slice/test-waldo.slice/ex.service/.

Copy link
Member

Choose a reason for hiding this comment

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

Might be https://github.com/systemd/systemd/blob/166e8e36eba661ba5d30974b71241ab0e3f49f37/src/basic/unit-name.c#L622? But well, reading systemd is harder than I thought...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, the docker run command also appears to understand this "-" notation when specified by the --cgroup-parent flag, though I suppose similar to here it's because the parsing is passed off to systemd itself.

% docker run -d --rm --name test2 --cgroup-parent "my-foo-group.slice" debian:stable-slim sleep 1000
ae96786b7f24e7d469be36c44cdb48bf913b33dc10ed3095c6249b380b1a5699

% find /sys/fs/cgroup | grep my-foo-group | grep ae96786b7f24e7
/sys/fs/cgroup/my.slice/my-foo.slice/my-foo-group.slice/docker-ae96786b7f24e7d469be36c44cdb48bf913b33dc10ed3095c6249b380b1a5699.scope
/sys/fs/cgroup/my.slice/my-foo.slice/my-foo-group.slice/docker-ae96786b7f24e7d469be36c44cdb48bf913b33dc10ed3095c6249b380b1a5699.scope/cgroup.events
/sys/fs/cgroup/my.slice/my-foo.slice/my-foo-group.slice/docker-ae96786b7f24e7d469be36c44cdb48bf913b33dc10ed3095c6249b380b1a5699.scope/memory.events

...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the systemd.slice man page does have some (albeit sparse) information about this: https://www.freedesktop.org/software/systemd/man/systemd.slice.html

return filepath.Join(defaultCgroup2Path, dashesToPath(slice), dashesToPath(group))
}

// dashesToPath converts a slice name with dashes to it's corresponding systemd filesystem path.
func dashesToPath(in string) string {
path := ""
if strings.HasSuffix(in, ".slice") && strings.Contains(in, "-") {
parts := strings.Split(in, "-")
for i := range parts {
s := strings.Join(parts[0:i+1], "-")
if !strings.HasSuffix(s, ".slice") {
s += ".slice"
}
path = filepath.Join(path, s)
}
} else {
path = filepath.Join(path, in)
}
return path
}

func NewSystemd(slice, group string, pid int, resources *Resources) (*Manager, error) {
if slice == "" {
slice = defaultSlice
}
ctx := context.TODO()
path := filepath.Join(defaultCgroup2Path, slice, group)
path := getSystemdFullPath(slice, group)
conn, err := systemdDbus.NewWithContext(ctx)
if err != nil {
return &Manager{}, err
Expand Down Expand Up @@ -796,9 +823,9 @@ func LoadSystemd(slice, group string) (*Manager, error) {
if slice == "" {
slice = defaultSlice
}
group = filepath.Join(defaultCgroup2Path, slice, group)
path := getSystemdFullPath(slice, group)
return &Manager{
path: group,
path: path,
}, nil
}

Expand Down
65 changes: 65 additions & 0 deletions v2/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"go.uber.org/goleak"
)

Expand Down Expand Up @@ -74,3 +75,67 @@ func TestEventChanCleanupOnCgroupRemoval(t *testing.T) {
}
goleak.VerifyNone(t)
}

func TestSystemdFullPath(t *testing.T) {
tests := []struct {
inputSlice string
inputGroup string
expectedOut string
}{
{
inputSlice: "user.slice",
inputGroup: "myGroup.slice",
expectedOut: "/sys/fs/cgroup/user.slice/myGroup.slice",
},
{
inputSlice: "/",
inputGroup: "myGroup.slice",
expectedOut: "/sys/fs/cgroup/myGroup.slice",
},
{
inputSlice: "system.slice",
inputGroup: "myGroup.slice",
expectedOut: "/sys/fs/cgroup/system.slice/myGroup.slice",
},
{
inputSlice: "user.slice",
inputGroup: "my-group.slice",
expectedOut: "/sys/fs/cgroup/user.slice/my.slice/my-group.slice",
},
{
inputSlice: "user.slice",
inputGroup: "my-group-more-dashes.slice",
expectedOut: "/sys/fs/cgroup/user.slice/my.slice/my-group.slice/my-group-more.slice/my-group-more-dashes.slice",
},
{
inputSlice: "user.slice",
inputGroup: "my-group-dashes.slice",
expectedOut: "/sys/fs/cgroup/user.slice/my.slice/my-group.slice/my-group-dashes.slice",
},
{
inputSlice: "user.slice",
inputGroup: "myGroup.scope",
expectedOut: "/sys/fs/cgroup/user.slice/myGroup.scope",
},
{
inputSlice: "user.slice",
inputGroup: "my-group-dashes.scope",
expectedOut: "/sys/fs/cgroup/user.slice/my-group-dashes.scope",
},
{
inputSlice: "test-waldo.slice",
inputGroup: "my-group.slice",
expectedOut: "/sys/fs/cgroup/test.slice/test-waldo.slice/my.slice/my-group.slice",
},
{
inputSlice: "test-waldo.slice",
inputGroup: "my.service",
expectedOut: "/sys/fs/cgroup/test.slice/test-waldo.slice/my.service",
},
}

for _, test := range tests {
actual := getSystemdFullPath(test.inputSlice, test.inputGroup)
assert.Equal(t, test.expectedOut, actual)
}
}