-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53641/
-----------------------------------------------------------

Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Jiang Yan Xu.


Repository: mesos


Description
-------

Refactored `CgroupsAnyHierarchyTest` test cases.


Diffs
-----

  src/tests/containerizer/cgroups_tests.cpp 
0afaec6ae948cabf1472bf01103210d8f9809cb1 

Diff: https://reviews.apache.org/r/53641/diff/


Testing
-------

This patch attempts to refactor `CgroupsAnyHierarchy` test cases to use a 
different hierarchy structure.
If `cpu` have been mounted at `/sys/fs/cgroup/cpu`, it would try to mount all 
rest subsystems at `/tmp/mesos_test_cgroup`.

I have several concerns about this proposal. 

1. Mount `cpu` at `/sys/fs/cgroup/cpu` while mount `memory` or any other 
subsystems at `/tmp/mesos_test_cgroup` would fail if cgroups is managed by 
systemd.

I test this in CentOS 7.

```
## cpu,cpuacct have been mounted
# cat /proc/self/mountinfo |grep cgroup
25 18 0:20 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:8 - tmpfs tmpfs 
ro,mode=755
26 25 0:21 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:9 - 
cgroup cgroup 
rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd
28 25 0:23 / /sys/fs/cgroup/cpu,cpuacct rw,relatime shared:10 - cgroup cgroup 
rw,cpuacct,cpu

## Mount rest subsystems would failed.
# mount -t cgroup -o rw,memory,devices cgroup /tmp/mesos_test_cgroup
mount: cgroup is already mounted or /tmp/mesos_test_cgroup busy
       cgroup is already mounted on /sys/fs/cgroup/systemd
       cgroup is already mounted on /sys/fs/cgroup/cpu,cpuacct
```

2. I think in current code base, we suppose all the hierarchies under the same 
directory, instead of supposing that some hierarchies are mounted at 
`/sys/fs/cgroup/xxx` while some hierarchies are mounted at 
`/tmp/mesos_test_cgroup`. This is what we suppose in 
`ContainerizerTest<slave::MesosContainerizer>` as well.

3. As we see in this patch, change to put hierarchies under different folders 
looks messy, compare to use a single folder for all cgroup subsystems. 
cgroups-v2 unified all subsystems under a same hierarchy, which don't allow us 
mount `cpu` at `/sys/fs/cgroup/cpu` while mount `memory` at 
`/tmp/mesos_test_cgroup` as well.


Thanks,

haosdent huang

Reply via email to