systemd cgroup driver supports slice management #1084
Conversation
Signed-off-by: derekwaynecarr <[email protected]>
|
/cc @euank |
| @@ -159,8 +160,36 @@ func UseSystemd() bool { | |||
| } | |||
| } | |||
|
|
|||
| // Assume we have the ability to start a transient unit as a slice | |||
| // This was broken until systemd v229, but has been back-ported on RHEL environments >= 219 | |||
| // For details, see: https://bugzilla.redhat.com/show_bug.cgi?id=1370299 | |||
There was a problem hiding this comment.
Any details on other distros?
There was a problem hiding this comment.
I would strongly suspect that other distros will not work with systemd <= 229, we can verify the other distros when we consume this in kubernetes. i doubt others have patched their systemd.
There was a problem hiding this comment.
Can I just clarify here what you mean by "broken"? Do you mean that the interface didn't exist / always returned an error? Or do you mean that there was some issues with how systemd treated such slices (in which case, is the code you've added sufficient to protect against that)? I'm just asking because we've had some horrible issues with systemd's DBus interface and I generally don't trust that project to handle things sanely anymore.
There was a problem hiding this comment.
@cyphar - by broken, I mean you would literally get an error back from systemd when you called start transient unit where the unit was a .slice.
The error response had the text: Unit type slice does not support transient units.
This was fixed in systemd/systemd@17f62e9
As part of systemd/systemd#1886
The code addresses this by:
- attempting to start a slice, and if successful, it sets hasStartTransientUnitSlice to true.
- if it detects you cannot, and you later attempt to start a transient unit of type slice, it will error early with the message that your systemd level did not support the feature yet.
Does that help?
There was a problem hiding this comment.
Alright, in that case I'm okay with this.
| // For details, see: https://bugzilla.redhat.com/show_bug.cgi?id=1370299 | ||
| hasStartTransientSliceUnit = true | ||
|
|
||
| // To ensure simple clean-up, we create a slice off the root with no hierarchy |
There was a problem hiding this comment.
What if runc were to be running underneath a container? Is that a supported use-case @crosbymichael ?
There was a problem hiding this comment.
this follows the same convention as the scope check, so i think it should be fine...
There was a problem hiding this comment.
runC inside a container is a supported config, however in general you aren't running systemd inside that container (so this is a moot point). Though, some people work on running systemd inside containers and might have issues with this. But since it would've been broken before I'm not worried about breaking it twice.
|
poke @mrunalp |
|
LGTM |
|
I'm holding off LGTM-ing this until I can better understand what you mean by "broken". I've also kicked off the CI (it looks like GitHub missed the hook). |
|
LGTM. Merging. |
This patch adds support for the systemd cgroup driver to manage slices.
The ability to manage slices via the dbus api was not available until systemd v229, but some distros have back-ported it to earlier levels. The driver verifies the ability to start a slice via a transient unit is supported prior to using the feature.
If the supplied cgroup name ends with a .slice suffix, we assume the user wanted a slice created instead of a scope. Creation of a slice does not require a PID, so support from the cgroupfs driver for pid=-1 is carried over into systemd driver.
Finally,
ExpandSliceis made public for use by downstream consumers. In addition, it fixes a logic error associated with-.sliceneeding to be/when expanded.Signed-off-by: derekwaynecarr [email protected]