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.

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

Add container's cgroup readonly mount documentation. #65

Closed
wants to merge 1 commit into from

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Jul 8, 2015

Signed-off-by: Mrunal Patel mrunalp@gmail.com

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
Copy link
Contributor Author

mrunalp commented Jul 8, 2015

See #62

@wking
Copy link
Contributor

wking commented Jul 8, 2015 via email

@philips
Copy link
Contributor

philips commented Jul 8, 2015

I don't think I agree with this change. My understanding, from talking to many people, is that the only correct way to make this work is to bind mount the host cgroup hierarchy here and then optionally bind mount the container tree read-write.

The problem is that cgroups do not use relative paths in a number of places.

Also I am certain that we will want a read-write subtree to support use cases like pods. cc @vmarmol @rjnagal

@crosbymichael
Copy link
Member

you can have it RW as well and properly nest in the correct cgroup because it's a subtree for the container.

@mrunalp
Copy link
Contributor Author

mrunalp commented Jul 8, 2015

I am not opposed to making it RW.

@philips
Copy link
Contributor

philips commented Jul 9, 2015

For context here is how we use the cgroup hierarchy in rkt today: http://www.spinics.net/lists/linux-containers/msg31037.html

This explanation provides concrete examples. So, could either of you (@mrunalp or @crosbymichael) contrast that with what you are proposing here?

cc @iaguis @alban

@mrunalp
Copy link
Contributor Author

mrunalp commented Jul 9, 2015

@philips
Here is what we are proposing (@crosbymichael can correct me).
We mount container's own cgroups at this path.

/ # ls -l /sys/fs/cgroup/
total 0
drwxr-xr-x    2 root     root             0 Jul  9 16:52 blkio
drwxr-xr-x    2 root     root             0 Jul  9 16:52 cpu,cpuacct
drwxr-xr-x    2 root     root             0 Jul  9 16:52 cpuset
drwxr-xr-x    2 root     root             0 Jul  9 16:52 devices
drwxr-xr-x    2 root     root             0 Jul  9 16:52 freezer
drwxr-xr-x    2 root     root             0 Jul  9 16:52 hugetlb
drwxr-xr-x    2 root     root             0 Jul  9 16:52 memory
drwxr-xr-x    2 root     root             0 Jul  9 16:41 name=systemd
drwxr-xr-x    2 root     root             0 Jul  9 16:52 net_cls,net_prio
drwxr-xr-x    2 root     root             0 Jul  9 16:52 perf_event

@alban
Copy link
Contributor

alban commented Jul 9, 2015

What do you mean with the container's own cgroups? If you mean that you mount a sub-tree of the cgroup, this will not work because /proc/$PID/cgroup lists full paths and the full paths would not match what is in /sys/fs/cgroup/. /proc/$PID/cgroup is unfortunately not namespace-aware (although there was some unfinished effort to make it more namespace-aware in the kernel).

Systemd requires the paths in /sys/fs/cgroup/ and /proc/$PID/cgroup to match. This is documented in the Systemd container interface (section "What You Shouldn't Do").

@crosbymichael
Copy link
Member

@mrunalp i would vote we don't have this as a default yet since there are so many ways of doing it.

@alban the paths for /proc/$PID/cgroup and /sys/fs/cgroup is a good argument for not doing this but I don't think much in that linked document apply because their recommendations are very insecure just make systemd work they way they want it to work.

@mrunalp
Copy link
Contributor Author

mrunalp commented Jul 9, 2015

@crosbymichael I agree.

@philips
Copy link
Contributor

philips commented Jul 9, 2015

@crosbymichael It seems like we should finish this discussion about sub-tree vs. non-subtree mapping and if we need to define an environment variable. I am fine saying the cgroup tree is RO, for now, but I would like to figure this tree stuff out here for @mrunalp's use case of a process wanting to know its own cgroups.

@philips
Copy link
Contributor

philips commented Jul 9, 2015

@mrunalp Maybe you could open an issue for discussion?

@mrunalp
Copy link
Contributor Author

mrunalp commented Jul 9, 2015

@philips I have opened #66 to continue the discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants