Fix systemd full path

[sparrc]

This PR fixes systemd managers to consider the special "-" character when creating a cgroup slice with the systemd cgroup driver.

Before this change, a simple program like below would fail because the manager was not correctly setting up the Manager.path variable.

package main

import (
	"log"

	v2 "github.com/containerd/cgroups/v2"
)

func main() {
	res := v2.Resources{}
	m, err := v2.NewSystemd("/", "my-foo-group.slice", -1, &res)
	if err != nil {
		log.Fatalf("%s", err)
	}

	controllers, err := m.Controllers()
	if err != nil {
		log.Fatalf("%s", err)
	}

	log.Printf("%v", controllers)
}

Before this change, this program would fail with the following error message:

# go run ./cmd/testload/main.go
2022/02/17 18:18:41 open /sys/fs/cgroup/my-foo-group.slice/cgroup.controllers: no such file or directory
exit status 1

The issue here is that m.Controllers() is looking in the wrong path. This slice does actually exist, but at a different path:

# find /sys/fs/cgroup/ -type d | grep "my-foo-group"
/sys/fs/cgroup/my.slice/my-foo.slice/my-foo-group.slice

After this change, NewSystemd (and LoadSystemd) will correctly set the path variable and the m.Controllers() function works correctly:

# go run ./cmd/testload/main.go
2022/02/17 18:22:17 [io memory pids]

[estesp]

Seems like we have a new intermittent issue with either tests hanging or something with CI cleanup on the vagrant-executed v2 suite. It's happening on a few other PRs as well.

[kzys]

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

[sparrc]

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/.

[kzys]

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

[sparrc]

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

...

[sparrc]

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

[estesp]

Can you try rebasing this patch on current HEAD and see if CI will pass? Thanks!

[sparrc]

Can you try rebasing this patch on current HEAD and see if CI will pass? Thanks!

done, looks like it worked 👍

[estesp]

LGTM