Re: Review Request 53296: Added cgroup namespace support for unified container.

[Jie Yu]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.
> 
> Jie Yu wrote:
> If you don't enable cgroups isolator, all container's process will be in 
> root cgroup. IIUC, even the new container is put into a new cgroup namespace, 
> it can still see docker's cgroups, no?
> 
> haosdent huang wrote:
> >all container's process will be in root cgroup
> 
> Yes
> 
> >it can still see docker's cgroups, no
> 
> Could not. Refer to https://reviews.apache.org/r/53517/, we could a 
> cgroup in the host namesapce, but it invisible in the containers.
> 
> haosdent huang wrote:
> systemd would let the containers use user.slice as the default cgroup 
> root in that case.
> 
> Jie Yu wrote:
> Here is the experiment I ran on my box:
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# mkdir /sys/fs/cgroup/memory/test
> root@ubuntu-xenial:~/opt# echo $$
> 29643
> root@ubuntu-xenial:~/opt# echo 29643 > /sys/fs/cgroup/memory/test/tasks 
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/test
> root@ubuntu-xenial:~/opt# /home/ubuntu/opt/util-linux/bin/unshare -Cm 
> /bin/bash
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/
> root@ubuntu-xenial:~/opt# cat /proc/1/cgroup  | grep memory
> 9:memory:/../init.scope
> ```
> 
> Console 2:
> ```
> root@ubuntu-xenial:~# sudo mkdir /sys/fs/cgroup/memory/test/sub-test
> ```
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# ls -al /sys/fs/cgroup/memory | grep sub-test
> drwxr-xr-x  2 root root   0 Nov  6 23:21 sub-test
> ```
> 
> haosdent huang wrote:
> In console 1, need to remount cgroup after 
> `/home/ubuntu/opt/util-linux/bin/unshare -Cm /bin/bash`.
> 
> ```
> $ unshare -Cm bash
> $ awk '{   if ($8 == "cgroup" && $4 ~ /^\/../) {cmd = cmd 
> sprintf("umount %s\n", $5);cmd = cmd sprintf("mount -t cgroup -o %s %s 
> %s\n", $10, $9, $5);  }} END {   system(cmd);}' /proc/self/mountinfo
> ```
> 
> Then `sub-test`
> 
> ```
> $ ls -1 /sys/fs/cgroup/memory/|grep sub-test
> sub-test
> ```
> 
> Jie Yu wrote:
> Sorry, Yeah, I did do a remount of memory subsystem and forgot to paste 
> the command there. The result I showed above is after I do a re-mount of 
> subsystem.
> 
> My point is: even the container uses cgroup namespace, the host processes 
> can still create cgroups in its root cgroup, and that cgroup will show up in 
> container's cgroup.
> 
> That being said, using cgroup namespace along without cgroup isolator 
> sounds weird because all containers share the same cgroup.
> 
> haosdent huang wrote:
> Hmm, I see, let me do it in the cgroups isolator. So we add a new flag 
> like `--enable_cgroup_namespace` in the agent or add a new field to `message 
> ContainerInfo`?

ok, thought about this more. Although I feel like it's not quite useful if 
cgroups isolator is not used, but this namespace isolator along does address 
one scenario where the agent itself is running in a non root cgroup (e.g., it's 
launched by init like systemd). So, let's keep it as a separate isolator.


- Jie


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie

Re: Review Request 45962: Updated a persistent volume test framework to include shared volumes.

[Jiang Yan Xu]

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




src/examples/persistent_volume_framework.cpp 


Keep the TODO?



src/examples/persistent_volume_framework.cpp (line 123)


s/shared/(possibly shared)/



src/examples/persistent_volume_framework.cpp (line 127)


Keep the name?

Shared persistent volume is a kind of persistent volume.



src/examples/persistent_volume_framework.cpp (lines 137 - 143)


Here we can just create regular shards and shared shards based on input and 
one after another. No interleaving necessary. See comments on the flags below.



src/examples/persistent_volume_framework.cpp (lines 186 - 187)


Why is "shared-only" a possible value? A shard has only one persistent 
volume so it's either "shared" or not. How about just a `bool isShared`. (As 
suggested below, we can put it in the volume).



src/examples/persistent_volume_framework.cpp (lines 206 - 207)


Any need for `string task_id`? If so it needs to be named `taskId` but I 
don't see the need? Once it's assigned to the task we can get it out by 
`task.task_id()`.



src/examples/persistent_volume_framework.cpp (lines 213 - 220)


No need to act differently based on the shard type.

We can just have 

- The first task (`launched == 0`) writes to the file: `echo hello > 
volume/persisted`
- Later tasks read from the file: `cat volume/persisted`

For regular persistent volumes the two tasks will be sequential, for shared 
ones they'll be simulatenous. The fact that 1st task could finish before the 
2nd is launched is not important: the example framework mainly serves the 
purpose of demostrating the usage of the feature and we don't want the tasks to 
block the CI for too long.

As a potentially followup we can take a read/consumer command and a 
write/producer command from flags (with default values). So CI would use the 
default values to complete quickly while a human could try out different write 
and reads commands which could represent more interesting/edge cases.



src/examples/persistent_volume_framework.cpp (lines 455 - 461)


I think it's sufficient to have the following states. (We should use a 
minimun number of state, we can always add more when more features are added to 
persistent volumes which demand more state but starting with a large number of 
states makes it difficult to evolve the test).

```
  STAGING = 0, // The shard is awaiting offers to launch more tasks.
  RUNNING, // The shard is fully running (all its tasks are 
launched).
  TERMINATING, // The shard is terminating and needs to clean up its 
persistent volume.
  DONE // The shard is terminated.
```

This translates to:

```
  STAGING = 0, // In resourceOffers: launch tasks
  RUNNING, // In resourceOffers: noop; In statusUpdate: check shard 
TERMINATING condition.
  TERMINATING, // In resourceOffers: DESTROY
  DONE // Test terminal condition.
```



src/examples/persistent_volume_framework.cpp (line 458)






src/examples/persistent_volume_framework.cpp (line 459)


To represent a state, use a `-ing` word?

See the overall comment on the enum.



src/examples/persistent_volume_framework.cpp (lines 460 - 461)


Why both `WAIT_DONE` and `DONE`?

Doesn't look like we need to treat shared pv and regular pv differently.

See the overall comment on the enum.



src/examples/persistent_volume_framework.cpp (lines 465 - 475)


I understand this is "just a test" but it would be cleaner the following 
way.

```
struct Volume
{
  explicit Volume(bool _isShared)
: isShared(_isShared) {}
  
  Option id;
  Option slave;
  bool isShared;
}
```

Here the volume is really the "intention" for the persistent volume so 
`isShared` is known when the shard is created and the optional values are 
filled when known.



src/examples/persistent_volume_framework.cpp (line 495)


s/taskId/taskIds/?

```
// The IDs of the tasks running on th

Re: Review Request 53517: Added test case for cgroup namespace isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53296, 53515, 53516, 53517]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Nov. 6, 2016, 12:48 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53517/
> ---
> 
> (Updated Nov. 6, 2016, 12:48 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case for cgroup namespace isolator.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/namespaces_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53517/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53330: Tracked layers and pull latency in docker store.

[Zhitao Li]

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

(Updated Nov. 7, 2016, 4:41 a.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


Bugs: MESOS-6451
https://issues.apache.org/jira/browse/MESOS-6451


Repository: mesos


Description
---

This patch added several metrics for docker store:
- a counter for number of layers pulled
- a gauge for total number of layers
- a timer for pull latency distribution in the last hour


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
9dccd0673dbc0c61abfd4b88097f86e7d7131c46 

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


Testing
---

Use mesos-execute to pull several different images, and verified following 
counters in agent's metrics/snapshot:
```
curl localhost:5051/metrics/snapshot | jq . | grep 
containerizer/mesos/docker_pull
  "containerizer/mesos/docker_store/layers_pulled": 47,
  "containerizer/mesos/docker_store/pull_ms/p50": 7080.760064,
  "containerizer/mesos/docker_store/pull_ms/p90": 11653.6390912,
  "containerizer/mesos/docker_store/pull_ms/p": 12528.3066648832,
  "containerizer/mesos/docker_store/pull_ms": 4550.814976,
  "containerizer/mesos/docker_store/pull_ms/count": 4,
  "containerizer/mesos/docker_store/pull_ms/max": 12529.182208,
  "containerizer/mesos/docker_store/pull_ms/p999": 12520.426776832,
  "containerizer/mesos/docker_store/pull_ms/min": 3167.337984,
  "containerizer/mesos/docker_store/pull_ms/p95": 12091.4106496,
  "containerizer/mesos/docker_store/pull_ms/p99": 12441.62789632,
  "containerizer/mesos/docker_store/layers": 47,
```


Thanks,

Zhitao Li

Re: Review Request 53105: Added an hourly timer for `slave/docker_containerizer/pull`.

[Zhitao Li]

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

(Updated Nov. 7, 2016, 4:40 a.m.)


Review request for mesos, Xiaojian Huang, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


Bugs: MESOS-6451
https://issues.apache.org/jira/browse/MESOS-6451


Repository: mesos


Description
---

Added an hourly timer for `slave/docker_containerizer/pull`.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

Ran `mesos-execute` with a docker image and docker containerizer, observed the 
following metrics:
```
curl -s localhost:5051/metrics/snapshot | jq . | grep pull
  "containerizer/docker/pull_ms/p999": 26209.36173824,
  "containerizer/docker/pull_ms/p90": 21036.405248,
  "containerizer/docker/pull_ms/p": 26256.388615424,
  "containerizer/docker/pull_ms/p50": 135.570944,
  "containerizer/docker/pull_ms/max": 26261.613824,
  "containerizer/docker/pull_ms/p95": 23649.009536,
  "containerizer/docker/pull_ms/min": 103.215872,
  "containerizer/docker/pull_ms/p99": 25739.0929664,
  "containerizer/docker/pull_ms": 26261.613824,
  "containerizer/docker/pull_ms/count": 3
```


Thanks,

Zhitao Li

Re: Review Request 53515: Fixed a typo in slave_recovery_tests.cpp.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On Nov. 6, 2016, 8:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53515/
> ---
> 
> (Updated Nov. 6, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in slave_recovery_tests.cpp.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp cc50498040986e9aca1031df3622b13e7a44218a 
> 
> Diff: https://reviews.apache.org/r/53515/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53296: Added cgroup namespace support for unified container.

[haosdent huang]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.
> 
> Jie Yu wrote:
> If you don't enable cgroups isolator, all container's process will be in 
> root cgroup. IIUC, even the new container is put into a new cgroup namespace, 
> it can still see docker's cgroups, no?
> 
> haosdent huang wrote:
> >all container's process will be in root cgroup
> 
> Yes
> 
> >it can still see docker's cgroups, no
> 
> Could not. Refer to https://reviews.apache.org/r/53517/, we could a 
> cgroup in the host namesapce, but it invisible in the containers.
> 
> haosdent huang wrote:
> systemd would let the containers use user.slice as the default cgroup 
> root in that case.
> 
> Jie Yu wrote:
> Here is the experiment I ran on my box:
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# mkdir /sys/fs/cgroup/memory/test
> root@ubuntu-xenial:~/opt# echo $$
> 29643
> root@ubuntu-xenial:~/opt# echo 29643 > /sys/fs/cgroup/memory/test/tasks 
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/test
> root@ubuntu-xenial:~/opt# /home/ubuntu/opt/util-linux/bin/unshare -Cm 
> /bin/bash
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/
> root@ubuntu-xenial:~/opt# cat /proc/1/cgroup  | grep memory
> 9:memory:/../init.scope
> ```
> 
> Console 2:
> ```
> root@ubuntu-xenial:~# sudo mkdir /sys/fs/cgroup/memory/test/sub-test
> ```
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# ls -al /sys/fs/cgroup/memory | grep sub-test
> drwxr-xr-x  2 root root   0 Nov  6 23:21 sub-test
> ```
> 
> haosdent huang wrote:
> In console 1, need to remount cgroup after 
> `/home/ubuntu/opt/util-linux/bin/unshare -Cm /bin/bash`.
> 
> ```
> $ unshare -Cm bash
> $ awk '{   if ($8 == "cgroup" && $4 ~ /^\/../) {cmd = cmd 
> sprintf("umount %s\n", $5);cmd = cmd sprintf("mount -t cgroup -o %s %s 
> %s\n", $10, $9, $5);  }} END {   system(cmd);}' /proc/self/mountinfo
> ```
> 
> Then `sub-test`
> 
> ```
> $ ls -1 /sys/fs/cgroup/memory/|grep sub-test
> sub-test
> ```
> 
> Jie Yu wrote:
> Sorry, Yeah, I did do a remount of memory subsystem and forgot to paste 
> the command there. The result I showed above is after I do a re-mount of 
> subsystem.
> 
> My point is: even the container uses cgroup namespace, the host processes 
> can still create cgroups in its root cgroup, and that cgroup will show up in 
> container's cgroup.
> 
> That being said, using cgroup namespace along without cgroup isolator 
> sounds weird because all containers share the same cgroup.

Hmm, I see, let me do it in the cgroups isolator. So we add a new flag like 
`--enable_cgroup_namespace` in the agent or add a new field to `message 
ContainerInfo`?


- haosdent


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5

Re: Review Request 50128: Added helper function to 'Docker::Device'.

[Yubo Li]

> On 十一月 4, 2016, 3:03 p.m., Guangya Liu wrote:
> > What about update the summary and desription as this?
> > 
> > Summary:
> > ```
> > Overload the << operator for 'Docker::Device'.
> > ```
> > 
> > Description: 
> > ```
> > This patch overload the << operator for 'Docker::Device',
> > with such overload, we can just run the global `stringify()`
> > function to turn 'Docker::Device' into a string.
> > ```

commit comment changed


- Yubo


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


On 十一月 2, 2016, 7:27 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50128/
> ---
> 
> (Updated 十一月 2, 2016, 7:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Wrapped helper function to stringify 'Docker::Device' structure.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
> 
> Diff: https://reviews.apache.org/r/50128/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 53296: Added cgroup namespace support for unified container.

[Jie Yu]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.
> 
> Jie Yu wrote:
> If you don't enable cgroups isolator, all container's process will be in 
> root cgroup. IIUC, even the new container is put into a new cgroup namespace, 
> it can still see docker's cgroups, no?
> 
> haosdent huang wrote:
> >all container's process will be in root cgroup
> 
> Yes
> 
> >it can still see docker's cgroups, no
> 
> Could not. Refer to https://reviews.apache.org/r/53517/, we could a 
> cgroup in the host namesapce, but it invisible in the containers.
> 
> haosdent huang wrote:
> systemd would let the containers use user.slice as the default cgroup 
> root in that case.
> 
> Jie Yu wrote:
> Here is the experiment I ran on my box:
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# mkdir /sys/fs/cgroup/memory/test
> root@ubuntu-xenial:~/opt# echo $$
> 29643
> root@ubuntu-xenial:~/opt# echo 29643 > /sys/fs/cgroup/memory/test/tasks 
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/test
> root@ubuntu-xenial:~/opt# /home/ubuntu/opt/util-linux/bin/unshare -Cm 
> /bin/bash
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/
> root@ubuntu-xenial:~/opt# cat /proc/1/cgroup  | grep memory
> 9:memory:/../init.scope
> ```
> 
> Console 2:
> ```
> root@ubuntu-xenial:~# sudo mkdir /sys/fs/cgroup/memory/test/sub-test
> ```
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# ls -al /sys/fs/cgroup/memory | grep sub-test
> drwxr-xr-x  2 root root   0 Nov  6 23:21 sub-test
> ```
> 
> haosdent huang wrote:
> In console 1, need to remount cgroup after 
> `/home/ubuntu/opt/util-linux/bin/unshare -Cm /bin/bash`.
> 
> ```
> $ unshare -Cm bash
> $ awk '{   if ($8 == "cgroup" && $4 ~ /^\/../) {cmd = cmd 
> sprintf("umount %s\n", $5);cmd = cmd sprintf("mount -t cgroup -o %s %s 
> %s\n", $10, $9, $5);  }} END {   system(cmd);}' /proc/self/mountinfo
> ```
> 
> Then `sub-test`
> 
> ```
> $ ls -1 /sys/fs/cgroup/memory/|grep sub-test
> sub-test
> ```

Sorry, Yeah, I did do a remount of memory subsystem and forgot to paste the 
command there. The result I showed above is after I do a re-mount of subsystem.

My point is: even the container uses cgroup namespace, the host processes can 
still create cgroups in its root cgroup, and that cgroup will show up in 
container's cgroup.

That being said, using cgroup namespace along without cgroup isolator sounds 
weird because all containers share the same cgroup.


- Jie


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave

Re: Review Request 53296: Added cgroup namespace support for unified container.

[haosdent huang]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.
> 
> Jie Yu wrote:
> If you don't enable cgroups isolator, all container's process will be in 
> root cgroup. IIUC, even the new container is put into a new cgroup namespace, 
> it can still see docker's cgroups, no?
> 
> haosdent huang wrote:
> >all container's process will be in root cgroup
> 
> Yes
> 
> >it can still see docker's cgroups, no
> 
> Could not. Refer to https://reviews.apache.org/r/53517/, we could a 
> cgroup in the host namesapce, but it invisible in the containers.
> 
> haosdent huang wrote:
> systemd would let the containers use user.slice as the default cgroup 
> root in that case.
> 
> Jie Yu wrote:
> Here is the experiment I ran on my box:
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# mkdir /sys/fs/cgroup/memory/test
> root@ubuntu-xenial:~/opt# echo $$
> 29643
> root@ubuntu-xenial:~/opt# echo 29643 > /sys/fs/cgroup/memory/test/tasks 
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/test
> root@ubuntu-xenial:~/opt# /home/ubuntu/opt/util-linux/bin/unshare -Cm 
> /bin/bash
> root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
> 9:memory:/
> root@ubuntu-xenial:~/opt# cat /proc/1/cgroup  | grep memory
> 9:memory:/../init.scope
> ```
> 
> Console 2:
> ```
> root@ubuntu-xenial:~# sudo mkdir /sys/fs/cgroup/memory/test/sub-test
> ```
> 
> Console 1:
> ```
> root@ubuntu-xenial:~/opt# ls -al /sys/fs/cgroup/memory | grep sub-test
> drwxr-xr-x  2 root root   0 Nov  6 23:21 sub-test
> ```

In console 1, need to remount cgroup after 
`/home/ubuntu/opt/util-linux/bin/unshare -Cm /bin/bash`.

```
$ unshare -Cm bash
$ awk '{   if ($8 == "cgroup" && $4 ~ /^\/../) {cmd = cmd sprintf("umount 
%s\n", $5);cmd = cmd sprintf("mount -t cgroup -o %s %s %s\n", $10, $9, $5); 
 }} END {   system(cmd);}' /proc/self/mountinfo
```

Then `sub-test`

```
$ ls -1 /sys/fs/cgroup/memory/|grep sub-test
sub-test
```


- haosdent


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53296/diff/
> 
> 
> Testing
> ---
> 
> The test case is on the way.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53296: Added cgroup namespace support for unified container.

[Jie Yu]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.
> 
> Jie Yu wrote:
> If you don't enable cgroups isolator, all container's process will be in 
> root cgroup. IIUC, even the new container is put into a new cgroup namespace, 
> it can still see docker's cgroups, no?
> 
> haosdent huang wrote:
> >all container's process will be in root cgroup
> 
> Yes
> 
> >it can still see docker's cgroups, no
> 
> Could not. Refer to https://reviews.apache.org/r/53517/, we could a 
> cgroup in the host namesapce, but it invisible in the containers.
> 
> haosdent huang wrote:
> systemd would let the containers use user.slice as the default cgroup 
> root in that case.

Here is the experiment I ran on my box:

Console 1:
```
root@ubuntu-xenial:~/opt# mkdir /sys/fs/cgroup/memory/test
root@ubuntu-xenial:~/opt# echo $$
29643
root@ubuntu-xenial:~/opt# echo 29643 > /sys/fs/cgroup/memory/test/tasks 
root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
9:memory:/test
root@ubuntu-xenial:~/opt# /home/ubuntu/opt/util-linux/bin/unshare -Cm /bin/bash
root@ubuntu-xenial:~/opt# cat /proc/self/cgroup | grep memory
9:memory:/
root@ubuntu-xenial:~/opt# cat /proc/1/cgroup  | grep memory
9:memory:/../init.scope
```

Console 2:
```
root@ubuntu-xenial:~# sudo mkdir /sys/fs/cgroup/memory/test/sub-test
```

Console 1:
```
root@ubuntu-xenial:~/opt# ls -al /sys/fs/cgroup/memory | grep sub-test
drwxr-xr-x  2 root root   0 Nov  6 23:21 sub-test
```


- Jie


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53296/diff/
> 
> 
> Testing
> ---
> 
> The test case is on the way.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52587: Allow CREATE of shared volumes based on capability of framework.

[Neil Conway]

> On Nov. 4, 2016, 7:30 p.m., Neil Conway wrote:
> > src/master/validation.cpp, line 1532
> > 
> >
> > Can we use consistent tense here? The other volume error messages say 
> > "has been attempted" rather than "being attempted".
> > 
> > Also, the other error messages don't include the volume that failed 
> > validation, so we should probably be consistent.
> 
> Jiang Yan Xu wrote:
> +1 on the tense.
> 
> but re: "the other error messages don't include the volume that failed 
> validation" they don't include the volume but they include other things this 
> error message doesn't include. Whether the additional information is useful 
> depends on the failure IMO. Nevertheless I guess it's true we don't have to 
> say it's a shared volume separately because `stringify(volume)` reveals it.
> 
> 
> So overall the following look good?
> 
> ```
> ...
> 
> "Create volume operation for '" + stringify(volume) +
> "' has been attempted by framework '" +
> 
> ...
> ```

Why do you think this particular error message warrants including 
`stringify(volume)` any more/less than the other places where a volume fails 
validation? 

I'm not opposed to including more context in validation failure messages, but I 
think we should also be consistent.


- Neil


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


On Nov. 1, 2016, 4:54 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52587/
> ---
> 
> (Updated Nov. 1, 2016, 4:54 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6316
> https://issues.apache.org/jira/browse/MESOS-6316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework issues a CREATE for a shared volume, we allow that
> operation only if the framework has opted in for the capability of
> SHARED_RESOURCES. However, for HTTP operator (/create-volumes), we
> do not check as the operator API is not related to a specific
> framework.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 013bb592ba47b785c552e199633e4784e8aa71b1 
>   src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
>   src/master/validation.cpp f690a9eacd278b51a52f5588dbeea377df074435 
>   src/tests/master_validation_tests.cpp 
> a5d8610bd61822cdf55cbc8d7056e5cf8fecfa54 
>   src/tests/persistent_volume_tests.cpp 
> 6289009fe9ed0a57ba5eff46dbbe0633a75d2616 
> 
> Diff: https://reviews.apache.org/r/52587/diff/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 53365: Fixed an issue in the gzip error handling.

[Anand Mazumdar]

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



hmm, I couldn't find any documentation around when `msg` may be not be set for 
error cases except for `deflateEnd()` and `inflateEnd()`. Can you share some? 
AFAICT, we only need to add a fallback for these 2 calls and not all of the 
ones as has been done in this review. Also, I don't think we need to even check 
the return type of `deflateEnd()`/`inflateEnd()` since we are sure that the 
state of the stream is consistent i.e., `Z_STREAM_END`?

- Anand Mazumdar


On Nov. 2, 2016, 3:02 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53365/
> ---
> 
> (Updated Nov. 2, 2016, 3:02 a.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It turns out that zlib does not always set the `msg` field, the
> calling code is expected to handle the case where `msg` is NULL.
> I discovered this while I was playing with the library during
> the implementation of a streaming decompressor.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/gzip.hpp 
> b78a8a31204ee585f8e4a88eaefe7346faa46b8d 
> 
> Diff: https://reviews.apache.org/r/53365/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 53296: Added cgroup namespace support for unified container.

[haosdent huang]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.
> 
> Jie Yu wrote:
> If you don't enable cgroups isolator, all container's process will be in 
> root cgroup. IIUC, even the new container is put into a new cgroup namespace, 
> it can still see docker's cgroups, no?
> 
> haosdent huang wrote:
> >all container's process will be in root cgroup
> 
> Yes
> 
> >it can still see docker's cgroups, no
> 
> Could not. Refer to https://reviews.apache.org/r/53517/, we could a 
> cgroup in the host namesapce, but it invisible in the containers.

systemd would let the containers use user.slice as the default cgroup root in 
that case.


- haosdent


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53296/diff/
> 
> 
> Testing
> ---
> 
> The test case is on the way.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53296: Added cgroup namespace support for unified container.

[haosdent huang]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.
> 
> Jie Yu wrote:
> If you don't enable cgroups isolator, all container's process will be in 
> root cgroup. IIUC, even the new container is put into a new cgroup namespace, 
> it can still see docker's cgroups, no?

>all container's process will be in root cgroup

Yes

>it can still see docker's cgroups, no

Could not. Refer to https://reviews.apache.org/r/53517/, we could a cgroup in 
the host namesapce, but it invisible in the containers.


- haosdent


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53296/diff/
> 
> 
> Testing
> ---
> 
> The test case is on the way.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53296: Added cgroup namespace support for unified container.

[Jie Yu]

> On Nov. 1, 2016, 4:43 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp, line 28
> > 
> >
> > Instead of creating a new namespace/cgroup isolator, I would suggest we 
> > add the support to cgroups isolator. It looks weird to me to have a 
> > namespace/cgroup isolator without using the cgroups isolator.
> 
> haosdent huang wrote:
> I think it still possible to use `namespaces/cgroup` isolator without 
> `cgroups` isolation? If user only want to isolate the host cgroups 
> environment from the container.
> 
> Jie Yu wrote:
> What's the use case for that? I feel that it will be strange to enable 
> cgroup namespace if containers still share the same cgroup. There will be no 
> isolation if two containers try to manipulate the cgroups. That defeats the 
> purpose of using cgroup namespace.
> 
> haosdent huang wrote:
> For example, we launch docker daemon in the host, which would use 
> `/sys/fs/cgroup/xx/subsystem_name` as the hierarchies.
> Then we want hide this in the containers launched by Mesos. In this case, 
> we only need `namespace/cgroup` isolator without cgroups isolation.

If you don't enable cgroups isolator, all container's process will be in root 
cgroup. IIUC, even the new container is put into a new cgroup namespace, it can 
still see docker's cgroups, no?


- Jie


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


On Nov. 6, 2016, 12:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53296/diff/
> 
> 
> Testing
> ---
> 
> The test case is on the way.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 53517: Added test case for cgroup namespace isolator.

[haosdent huang]

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

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


Bugs: MESOS-5410
https://issues.apache.org/jira/browse/MESOS-5410


Repository: mesos


Description
---

Added test case for cgroup namespace isolator.


Diffs
-

  src/tests/containerizer/namespaces_isolator_tests.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Review Request 53516: Moved `namespaces/pid` associated test cases to a separate file.

[haosdent huang]

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

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


Bugs: MESOS-5410
https://issues.apache.org/jira/browse/MESOS-5410


Repository: mesos


Description
---

Moved `namespaces/pid` associated test cases to a separate file.


Diffs
-

  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/tests/containerizer/isolator_tests.cpp 
8fefeef8c83ed2ab01f56a1ec572d3acb307143c 
  src/tests/containerizer/namespaces_isolator_tests.cpp PRE-CREATION 
  src/tests/slave_recovery_tests.cpp cc50498040986e9aca1031df3622b13e7a44218a 

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


Testing
---


Thanks,

haosdent huang

Review Request 53515: Fixed a typo in slave_recovery_tests.cpp.

[haosdent huang]

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

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


Bugs: MESOS-5410
https://issues.apache.org/jira/browse/MESOS-5410


Repository: mesos


Description
---

Fixed a typo in slave_recovery_tests.cpp.


Diffs
-

  src/tests/slave_recovery_tests.cpp cc50498040986e9aca1031df3622b13e7a44218a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 53296: Added cgroup namespace support for unified container.

[haosdent huang]

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

(Updated Nov. 6, 2016, 12:47 p.m.)


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


Changes
---

Rebase.


Bugs: MESOS-5410
https://issues.apache.org/jira/browse/MESOS-5410


Repository: mesos


Description
---

Added cgroup namespace support for unified container.


Diffs (updated)
-

  src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/slave/containerizer/mesos/containerizer.cpp 
67cc595278f124cdf518d2f4fcfb257439f067e2 
  src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 

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


Testing
---

The test case is on the way.


Thanks,

haosdent huang

Re: Review Request 53296: Added cgroup namespace support for unified container.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53296]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Nov. 6, 2016, 7:35 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53296/
> ---
> 
> (Updated Nov. 6, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Qian Zhang, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5410
> https://issues.apache.org/jira/browse/MESOS-5410
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added cgroup namespace support for unified container.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53296/diff/
> 
> 
> Testing
> ---
> 
> The test case is on the way.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53296: Added cgroup namespace support for unified container.

[haosdent huang]

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

(Updated Nov. 6, 2016, 7:35 a.m.)


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


Changes
---

Address @jpeach's comments.


Bugs: MESOS-5410
https://issues.apache.org/jira/browse/MESOS-5410


Repository: mesos


Description
---

Added cgroup namespace support for unified container.


Diffs (updated)
-

  src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/slave/containerizer/mesos/containerizer.cpp 
67cc595278f124cdf518d2f4fcfb257439f067e2 
  src/slave/containerizer/mesos/isolators/namespaces/cgroup.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/namespaces/cgroup.cpp PRE-CREATION 

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


Testing
---

The test case is on the way.


Thanks,

haosdent huang