fix init.scope in cgroup paths

[sjenning]

Fixes #931

There was talk in the issue about using dbus to determine which cgroup contains systemd, but I'm not seeing how that is relevant. Seems like we just want to always start at the cgroup root.

@derekwaynecarr @vishh @euank @mrunalp

[euank]

The hardcoded string system.slice is what you'd use a systemd dbus call to replace iiuc.

All the same, LGTM as a definite improvement over the existing state of things. Thanks!

[cyphar]

Rejected.

The reason why we do this is because of the usecase where someone is running a container inside a container (currently we don't have support for the cgroup namespace, and there's no requirement that a user will use that namespace even if we did support it -- see #781). If you start joining cgroups at the root, things start to not go very well (systemd can't track the process anymore and it doesn't like that very much).

Now, for the apply_raw cgroup handler we check against /proc/1/cgroups but if you tell runC to specifically use systemd for cgroup handling that's what we will do instead.

The issue you linked references that we should be using systemd to query the root of the cgroup hierarchy, not to ignore systemd in the systemd cgroup driver. I'd be okay with a change that does that.

/cc @crosbymichael

[sjenning]

@cyphar thanks for explaining that. I have pushed a different possible fix.

The idea is that if "init.scope" is in the initPath, we should remove it, and it will only every appear once at the end of the initPath. If pid 1 is not systemd, nothing changes. If pid 1 is systemd < 226, nothing changes. If pid 1 is systemd >= 226, "init.scope" is removed from the initPath.

Does this work?

[mrunalp]

@sjenning This sounds good to me. IMO, this is the least intrusive fix for now.

[mrunalp]

LGTM

[mrunalp]

@cyphar @crosbymichael @hqhq PTAL

[cyphar]

I'm wondering if we should use strings.TrimSuffix. This will replace the first occurrence of init.scope in the path, but we actually want to remove the last one -- right? Though you'd want to do a filepath.Clean first.

[sjenning]

@cyphar I can do that. There should only ever be one occurrence of init.scope in the path but I can change it just to make it more self-documenting.

[sjenning]

@cyphar updated PTAL. Also I think Jenkins flaked, might need someone retry that.

[mrunalp]

I've kicked off the tests again.

[cyphar]

LGTM.

[mrunalp]

LGTM