Allow cgroup creation without attaching a pid #956
Conversation
| return err | ||
| // Dont attach any pid to the cgroup if -1 is specified as a pid | ||
| if pid != -1 { | ||
| if err := ioutil.WriteFile(filepath.Join(path, "cgroup.procs"), |
There was a problem hiding this comment.
While replacing this, maybe we should be using writeFile here instead of ioutil.
There was a problem hiding this comment.
Re above, Now that I looked into it more carefully this was done to avoid import cycles.
I would suggest that we move the WriteFile method to cgroups.utils.go.
Or we can also not make any such changes and continue to use ioutil in the utils.go file.
|
Just to be clear, this is kubernetes using |
|
Yes this is needed for Kubernete's internal implementation. |
|
LGTM |
| if err := writeFile(path, CgroupProcesses, strconv.Itoa(raw.pid)); err != nil { | ||
| return "", err | ||
| // Dont attach any pid to the cgroup if -1 is specified as a pid | ||
| if raw.pid != -1 { |
There was a problem hiding this comment.
i worry the -1 is kind of undocumented unless we have it that all writeFile calls to cgroup.procs share a common function call. a lot of this code is not unit tested, so would prefer that we at least consolidate the -1 checking into a common function like writeCgroupProc(pid int) and just keep the -1 check local to there..
Signed-off-by: Buddha Prakash <[email protected]>
|
LGTM, thanks for the update. |
|
@cyphar write file is not complicated, we can fix it or remove it |
|
Should we consider moving a bunch of "utility like" functions in apply_raw to utils.go ?? |
|
We should probably move |
|
I will send out a PR for the same. |
|
@cyphar Can you LGTM this PR as it's blocking other PR's. I will move the writeFile to libcontainer/utils in a different PR. |
|
LGTM, sure. But you still need one more maintainer to LGTM this version of the PR. |
|
Oh, right! cc @mrunalp |
|
LGTM |
I am working on introducing pod level cgroups in Kubernetes (kubernetes/kubernetes#27204). As a part I would like to use libcontainer for creating and managing cgroups in the system. I would like to just create a cgroup with no pid attached and if need be apply a pid to the cgroup later on. But currently libcontainer doesn't support cgroup creation without attaching a pid.
This would allow us to skip attaching a pid to a cgroup during creation.
cc @mrunalp @vishh @derekwaynecarr