Re: Review Request 52735: Removed TODO message for docker killing.

[Yubo Li]

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

(Updated 十月 21, 2016, 6:30 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


Repository: mesos


Description
---

Removed TODO message for docker killing.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---


Thanks,

Yubo Li

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Yubo Li]

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

(Updated 十月 21, 2016, 6:30 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
---

This added a testing case for end-to-end GPU support for docker
containerizer.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 

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


Testing
---

GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
check


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

(Updated 十月 21, 2016, 6:30 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Yubo Li]

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

(Updated 十月 21, 2016, 6:29 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
---

Added a new flag '--devices' to mesos-docker-executor, and gave its
feature to control devices exposition, isolation, and access permission.
Also, passed GPUs assignment to mesos-docker-executor.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50599: Assigned Nvidia GPU devices to docker container.

[Yubo Li]

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

(Updated 十月 21, 2016, 6:29 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li

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

[Yubo Li]

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

(Updated 十月 21, 2016, 6:28 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 functions to 'Docker::Device' to handle data
parsing and serializing between 'Docker::Device' structure and
string.


Diffs (updated)
-

  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 50841: Added GPU scheduling logic to docker containerizer process.

[Yubo Li]

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

(Updated 十月 21, 2016, 6:28 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Added control logic to allocate/deallocate GPUs to GPU-related task
when the task is started/terminated.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50123: Added GPU scheduler for docker containerizer process.

[Yubo Li]

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

(Updated 十月 21, 2016, 6: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
---

This added 'NvidiaComponents' to docker containerizer process so that
docker containerizer process is ready to use it to allocate GPUs to task
with 'gpus' resource.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
  src/tests/mock_docker.hpp 1bf09c8dba020b421526b650523c879fb87380f8 
  src/tests/mock_docker.cpp 6a0e613bde6889864a37ffd7ec0b454e5fe4df1c 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 53041: Added `setxattr()` and `getxattr()` in stout.

[Qian Zhang]

> On Oct. 20, 2016, 12:03 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/posix/xattr.hpp, line 24
> > 
> >
> > Since the flags differ between platform, it might be a good idea to 
> > define stout-specific versions.

Can you elaborate on stout-specific versions? Did you mean we define two 
versions of setxattr() here, one for Linux and another for APPLE?


> On Oct. 20, 2016, 12:03 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/posix/xattr.hpp, line 51
> > 
> >
> > Add a flags argument? Darwin's ``XATTR_NOFOLLOW`` can be mapped to 
> > ``lgetxattr`` on Linux.

Did you mean if caller passes `XATTR_NOFOLLOW` as flag to this method on Linux, 
internally we call `lgetxattr()`? But I think it may confuse the caller since 
on Linux `getxattr()` does not support flags at all, so I would suggest not to 
mix `getxattr()` and `lgetxattr()` in one method.


> On Oct. 20, 2016, 12:03 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/posix/xattr.hpp, line 64
> > 
> >
> > You could use ``std::vector`` here to avoid manual memory 
> > management.

Can you please let me know the advantage of std::vector? To me, new + 
memset can satisfy my requirement here.


> On Oct. 20, 2016, 12:03 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/posix/xattr.hpp, line 80
> > 
> >
> > Should you add ``removexattr`` for completeness?

Agree!


> On Oct. 20, 2016, 12:03 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/os/xattr.hpp, line 21
> > 
> >
> > I don't think this API makes sense for Windows. NTFS has streams, which 
> > have different semantics that extended attributes. I doubt that you would 
> > want this API on Windows.

Yes, I agree, let me mark those functions as delete on Windows.


- Qian


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


On Oct. 20, 2016, 11:20 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53041/
> ---
> 
> (Updated Oct. 20, 2016, 11:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `setxattr()` and `getxattr()` in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 1eb9c146d3eebacaea32e0f724d145f0c1dd676f 
>   3rdparty/stout/include/stout/os.hpp 
> 96e8621b198a3ec4cce78e0a6ff5f271eda05ff1 
>   3rdparty/stout/include/stout/os/posix/xattr.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/xattr.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/xattr.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53041/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 53041: Added `setxattr()` and `getxattr()` in stout.

[Qian Zhang]

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




3rdparty/stout/include/stout/os/posix/xattr.hpp (line 24)


Can you elaborate on `stout-specific versions`? Did you mean we define two 
versions of `setxattr()` here, one for Linux and another for APPLE?



3rdparty/stout/include/stout/os/posix/xattr.hpp (line 51)


Did you mean if caller passes `XATTR_NOFOLLOW` as flag to this method on 
Linux, internally we call `lgetxattr()`? But I think it may confuse the caller 
since on Linux `getxattr()` does not support flags at all, so I would suggest 
not to mix `getxattr()` and `lgetxattr()` in one method.



3rdparty/stout/include/stout/os/posix/xattr.hpp (line 64)


Can you please let me know the advantage of `std::vector`? To me, 
`new` + `memset` can satisfy my requirement here.



3rdparty/stout/include/stout/os/posix/xattr.hpp (line 80)


Agree!



3rdparty/stout/include/stout/os/xattr.hpp (line 21)


Yes, I agree, let me mark those functions as delete on Windows.


- Qian Zhang


On Oct. 20, 2016, 11:20 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53041/
> ---
> 
> (Updated Oct. 20, 2016, 11:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `setxattr()` and `getxattr()` in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 1eb9c146d3eebacaea32e0f724d145f0c1dd676f 
>   3rdparty/stout/include/stout/os.hpp 
> 96e8621b198a3ec4cce78e0a6ff5f271eda05ff1 
>   3rdparty/stout/include/stout/os/posix/xattr.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/xattr.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/xattr.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53041/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52840: Silenced a GMock warning in ROOT_DOCKER_DockerInspectDiscard.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52840]

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 Oct. 20, 2016, 9:31 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52840/
> ---
> 
> (Updated Oct. 20, 2016, 9:31 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Silenced a GMock warning in ROOT_DOCKER_DockerInspectDiscard.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/52840/diff/
> 
> 
> Testing
> ---
> 
> Without this patch, `"sudo ./src/mesos-tests 
> --gtest_filter="DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard"` 
> consistently results in:
> 
> ```
> [ RUN  ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard
> 
> GMOCK WARNING:
> Uninteresting mock function call - returning directly.
> Function call: executorLost(0x7ffe1aad15e0, @0x7f2018000b00 e1, 
> @0x7f2018000c30 0aec7dbf-3ce3-4d3a-a086-a95bd1e8dada-S0, -1)
> Stack trace:
> [   OK ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard (624 ms)
> ```
> 
> With this patch, running the test 100 times in a loop does not produce a 
> warning.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52735: Removed TODO message for docker killing.

[Yubo Li]

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

(Updated 十月 21, 2016, 2:57 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


Repository: mesos


Description
---

Removed TODO message for docker killing.


Diffs
-

  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---


Thanks,

Yubo Li

Re: Review Request 50841: Added GPU scheduling logic to docker containerizer process.

[Yubo Li]

> On 十月 20, 2016, 1:32 p.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, lines 2154-2158
> > 
> >
> > Do you have a patch for removing this comment?

Yes, removed it in `https://reviews.apache.org/r/52735/`, sorry for foggetting 
publish it...


- Yubo


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


On 十月 20, 2016, 10 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50841/
> ---
> 
> (Updated 十月 20, 2016, 10 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added control logic to allocate/deallocate GPUs to GPU-related task
> when the task is started/terminated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
> 
> Diff: https://reviews.apache.org/r/50841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Review Request 52735: Removed TODO message for docker killing.

[Yubo Li]

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

Review request for mesos.


Repository: mesos


Description
---

Removed TODO message for docker killing.


Diffs
-

  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---


Thanks,

Yubo Li

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Guangya Liu]

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




src/slave/containerizer/mesos/isolators/gpu/allocator.cpp (lines 170 - 174)


Here should also check if using `mesos` containerizer, otherwise, the 
`docker` containerizer will always return here and there may be GPU overcommit 
if we specify more GPU resources than the actual GPU resources.

```
// Pass the GPU resources through if we're not going to do any
// isolation for mesos conainerizer or we cannot validate the
// resources using NVML.
if ((strings::contains(flags.containerizers, "mesos") &&
 isolators.count("gpu/nvidia") == 0) ||
!nvml::isAvailable()) {
  return resources;
}
```


- Guangya Liu


On 十月 20, 2016, 10:01 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50947/
> ---
> 
> (Updated 十月 20, 2016, 10:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
> docker and external containerizers do not need it anymore. This removed
> the 'gpu/nvidia' isolator check for docker and external containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> 2e722691475c84afae14009014ea70cc0fdd0e65 
> 
> Diff: https://reviews.apache.org/r/50947/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 52818: Ensured agent is registered in SlaveEndpointTests.

[Anand Mazumdar]

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



Thanks for the cleanup.

Can you also update the summary/description to mention that we are waiting for 
the agent recovery to be complete and not registration?


src/tests/slave_authorization_tests.cpp (lines 398 - 401)


hmm, let's be consistent and do something like: 
https://github.com/apache/mesos/blob/master/src/tests/slave_tests.cpp#L1727

https://github.com/apache/mesos/blob/master/src/tests/executor_http_api_tests.cpp#L120
https://github.com/apache/mesos/blob/master/src/tests/api_tests.cpp#L2241 
etc.

This works here since we do not have any frameworks and the future would be 
fulfilled without waiting for the executors to re-register i.e., no 
`process::delay(...)` is invoked: 
https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L5265

Not yours but we should fix the other instance in this file too since it 
seems to be an outlier.


- Anand Mazumdar


On Oct. 13, 2016, 9:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52818/
> ---
> 
> (Updated Oct. 13, 2016, 9:08 a.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensured agent is registered in SlaveEndpointTests.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp 
> 6bd2aa96cb05aa087c0807cc3dd1830451efe106 
> 
> Diff: https://reviews.apache.org/r/52818/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OS X, clang-3.9 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53077: Redefined helper macros with existing 'create' functions.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53076, 53077]

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 Oct. 20, 2016, 8:39 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53077/
> ---
> 
> (Updated Oct. 20, 2016, 8:39 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 08bfedb5c3d5a7cc86ab20316c7135f65f8612fb 
> 
> Diff: https://reviews.apache.org/r/53077/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[haosdent huang]

> On Oct. 20, 2016, 8:50 p.m., Jie Yu wrote:
> > src/tests/containerizer/cgroups_tests.cpp, line 412
> > 
> >
> > Who remove the cgroup created above?
> 
> Jiang Yan Xu wrote:
> Hmm, the intention is to have them destroyed in teardown but I guess 
> these cgroups don't start with `TEST_CGROUPS_ROOT`. I'll add back these 
> removal lines and we can think about how to do auto-cleanup better.

Because

```
const static std::string TEST_CGROUPS_ROOT = "mesos_test";
```

these cgroups would be clear during the teardown as well. Let me to make this 
more clear.


- haosdent


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


On Oct. 21, 2016, 1:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Oct. 21, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In most cases, we just want to get the children cgroups instead of
> retrieve descendant cgroups recursively. We added an argument to
> `cgroups::get` to indicate whether to retrieve cgroups recursively and
> made non-recursive retrieve the default behaviour. This patch fixed
> some incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
>   src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
>   src/slave/containerizer/fetcher.cpp 
> ea8eaa6371fc322c240e17b4bb9ba417c45ceaca 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2f6723c64261fb3295626d6479fe844fb23b0650 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 0305d14c1f791c93edcd3b32786b483b15f40a2d 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 4df537747d84daa68c29e2d05b22fa386a4a16db 
>   src/tests/mesos.cpp 2aae160fb941ab3672a5665ae27f517ff40600e2 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[haosdent huang]

> On Oct. 20, 2016, 8:33 p.m., Jie Yu wrote:
> > src/linux/cgroups.hpp, line 227
> > 
> >
> > I would suggest we set default to true to match the previous semantics.

Since `cgroups::create` is

```
Try create(
const std::string& hierarchy,
const std::string& cgroup,
bool recursive = false);
```

Is it better to keep consistent on `recursive` parameter?


- haosdent


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


On Oct. 21, 2016, 1:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Oct. 21, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In most cases, we just want to get the children cgroups instead of
> retrieve descendant cgroups recursively. We added an argument to
> `cgroups::get` to indicate whether to retrieve cgroups recursively and
> made non-recursive retrieve the default behaviour. This patch fixed
> some incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
>   src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
>   src/slave/containerizer/fetcher.cpp 
> ea8eaa6371fc322c240e17b4bb9ba417c45ceaca 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2f6723c64261fb3295626d6479fe844fb23b0650 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 0305d14c1f791c93edcd3b32786b483b15f40a2d 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 4df537747d84daa68c29e2d05b22fa386a4a16db 
>   src/tests/mesos.cpp 2aae160fb941ab3672a5665ae27f517ff40600e2 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51185: Removed the expired TODO about non-recursive version `cgroups::get`.

[haosdent huang]

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

(Updated Oct. 21, 2016, 1:16 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed the expired TODO about non-recursive version `cgroups::get`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2f6723c64261fb3295626d6479fe844fb23b0650 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[haosdent huang]

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

(Updated Oct. 21, 2016, 1:15 a.m.)


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


Changes
---

Fix test cases failure for `NestedContainers`.


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


Repository: mesos


Description (updated)
---

In most cases, we just want to get the children cgroups instead of
retrieve descendant cgroups recursively. We added an argument to
`cgroups::get` to indicate whether to retrieve cgroups recursively and
made non-recursive retrieve the default behaviour. This patch fixed
some incorrect `TEST_CGROUPS_ROOT` checks as well.


Diffs (updated)
-

  src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
  src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
  src/slave/containerizer/fetcher.cpp ea8eaa6371fc322c240e17b4bb9ba417c45ceaca 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2f6723c64261fb3295626d6479fe844fb23b0650 
  src/slave/containerizer/mesos/linux_launcher.cpp 
0305d14c1f791c93edcd3b32786b483b15f40a2d 
  src/tests/containerizer/cgroups_tests.cpp 
0afaec6ae948cabf1472bf01103210d8f9809cb1 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
4df537747d84daa68c29e2d05b22fa386a4a16db 
  src/tests/mesos.cpp 2aae160fb941ab3672a5665ae27f517ff40600e2 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 52543: Added configure/make options to build the new CLI and run unit tests.

[Benjamin Bannier]

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




configure.ac (line 203)


Please do not add options here, instead just use

AC_ARG_ENABLE([new-cli],
  AS_HELP_STRING([--enable-new-cli],
 [build new CLI instead of old CLI default: 
no]))
 
The pattern you copied here leads to fun issues like 
https://issues.apache.org/jira/browse/MESOS-2537.

Also see https://autotools.io/autoconf/arguments.html.



configure.ac (line 2178)


Please `s/python/Python/g` here and below, especially in user-facing 
strings.



configure.ac (lines 2179 - 2180)


Use `AS_IF` here instead of shell `if` (there's already a `TODO` above to 
clean up existing ones, let's not add new ones).



configure.ac (lines 2181 - 2205)


This could be simplified if you used the action-less version with aborts if 
no Python with at least the given version can be found, i.e.,

AM_PATH_PYTHON([2.6])



configure.ac (line 2194)


You could just use the first argument of `AM_PATH_PYTHON` here.



configure.ac (line 2219)


`AS_IF`



src/Makefile.am (line 1410)


Any reason you didn't want to explicitly list the files here? I believe 
this would make it clearer what is hidden :/



src/Makefile.am (line 1416)


Could you explicitly list the dependencies? This not only would make `make` 
run faster in NOP builds, but also would make the build more reliable (less 
magic effects if e.g., junk sits in that directory).

Also out of curiosity, why is `EXEEXT` needed here?



src/Makefile.am (line 1417)


I would remove this `echo` and instead just let below invocation echo, at 
least in verbose builds.



src/Makefile.am (line 1418)


Make all commands here a single shell invocation.

If you then prefix this command with `$(AM_V_GEN)` it would produce 
silenced output in silent mode (`--enable-silent-rules` ...).



src/Makefile.am (line 1424)


Not sure if this already happens earlier in the chain, but a comment 
explaining why we need to bundle the CLI would be helpful. Feel free to drop if 
this obvious.



src/Makefile.am (line 1432)


Could you also add a `dist_` target?



src/Makefile.am (line 2382)


No pun intended, but this explicit `echo` does not seem to add a lot (but a 
newline). I believe this isn't needed and should be removed.



src/Makefile.am (line 2384)


Does mesos-cli-tests introduce any new dependencies? Right now this target 
just depends on `tests`.


- Benjamin Bannier


On Oct. 20, 2016, 8:13 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52543/
> ---
> 
> (Updated Oct. 20, 2016, 8:13 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6008 and MESOS-6032
> https://issues.apache.org/jira/browse/MESOS-6008
> https://issues.apache.org/jira/browse/MESOS-6032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added configure/make options to build the new CLI and run unit tests.
> 
> 
> Diffs
> -
> 
>   configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 
>   docs/getting-started.md de3fba4a821ca7fa9b9e2fd692b33b40edd35b68 
>   src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
> 
> Diff: https://reviews.apache.org/r/52543/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-new-cli
> make -C src mesos
> src/mesos
> 
> GTEST_FILTER="" make -j check
> sudo GTEST_FILTER="" make -j check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52543: Added configure/make options to build the new CLI and run unit tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53074, 52939, 52941, 52942, 52943, 52944, 52945, 52946, 
52947, 51008, 52948, 52950, 52951, 52952, 52953, 52543]

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 Oct. 20, 2016, 6:13 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52543/
> ---
> 
> (Updated Oct. 20, 2016, 6:13 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6008 and MESOS-6032
> https://issues.apache.org/jira/browse/MESOS-6008
> https://issues.apache.org/jira/browse/MESOS-6032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added configure/make options to build the new CLI and run unit tests.
> 
> 
> Diffs
> -
> 
>   configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 
>   docs/getting-started.md de3fba4a821ca7fa9b9e2fd692b33b40edd35b68 
>   src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
> 
> Diff: https://reviews.apache.org/r/52543/diff/
> 
> 
> Testing
> ---
> 
> ../configure --enable-new-cli
> make -C src mesos
> src/mesos
> 
> GTEST_FILTER="" make -j check
> sudo GTEST_FILTER="" make -j check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 53062: Add rlimit support to Mesos containerizer.

[Benjamin Bannier]

> On Oct. 20, 2016, 9:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp, lines 592-603
> > 
> >
> > This won't work because data services explicitly want to get limit 
> > above what's configured.

Wouldn't we poke a giant hole into the system if we allowed unpriviledged tasks 
to set arbitrary rlimits from potentially `root` the agent might be running as 
without any checks on the agent side? AFAICT above code allows non-priviledged 
tasks to only lower limits, while priviledged tasks can still set any limits, 
which should be safe and enables rlimits for a large class of frameworks.

Note that we set rlimits before we potentially drop capabilities like 
`CAP_SYS_RESOURCE`. I now mention this fact explicitly in the comment.

Once we implement agent functionality to check against limiting rlimits we 
might be able to open up above restriction.


> On Oct. 20, 2016, 9:24 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp, line 105
> > 
> >
> > That makes me think if we should make 'unlimited' be more explicit. 
> > With the current scheme, unlimited of RLIMIT_FSIZE will be represented as:
> > ```
> > '{"rlimits":[{"type":"RLMT_FSIZE"}]}'
> > ```
> > 
> > I am wondering if we should introduce an optional `unlimited` field:
> > ```
> > message RLimit {
> >   optional Type type = 1;
> >   optional uint64_t soft = 2;
> >   optional uint64_t hard = 3;
> >   optional bool unlimited = 4;
> > }
> > 
> > '{"rlimits":[{"type":"RLMT_FSIZE","unlimited":true}]}'
> > ```

I agree that the serialized form of this message reads better with `unlimited`.

At the same time this format leads to a potential explosion of complexity, 
e.g., combinations without `unlimited` would be

* both `soft` and `hard` are set (1 case, valid)
* neither `soft` and `hard` are set (1 case, valid)
* one of `{soft, hard}` is set, the other one unset (2 cases, invalid)

while with `unlimited` we could have

* both `soft` and `hard` are set, `unlimited` unset (1 case, valid)
* neither `soft` and `hard` are set, `unlimited` set to `true` (1 case, valid)
* one of `{soft, hard}` is set, the other one unset, any state for `unlimited` 
(4 cases, invalid)
* `unlimited` is `false`, any states for `soft` or `hard` (5 cases, invalid)

It seems it might be more straight-forward to document above string 
serialization when `RLimit`s are used on e.g., CLI interfaces, than to expose 
users to the added complexity with `unlimited`.


- Benjamin


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


On Oct. 21, 2016, 12:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53062/
> ---
> 
> (Updated Oct. 21, 2016, 12:38 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6426
> https://issues.apache.org/jira/browse/MESOS-6426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a new launch flag `--rlimits` which can be used to
> specify POSIX resource limits for the container. The resource limits
> are set as the user, so to increase resource limits beyond configured
> system limits additional priviledges might be needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> f8bac0650965a49562b9910bf6140ded8dbb69ac 
>   src/slave/containerizer/mesos/launch.cpp 
> 4a41aaf103f5a9bc6f7a798f63f491fc7cf11f7e 
> 
> Diff: https://reviews.apache.org/r/53062/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53078/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53061: Added abstractions for working with POSIX resource limits.

[Benjamin Bannier]

> On Oct. 20, 2016, 9:09 p.m., Jie Yu wrote:
> > src/posix/rlimit.hpp, line 25
> > 
> >
> > Windows does not use autotool. So you don't have to do this. We just 
> > need to make sure all callsites are wrapped with ifndef WINDOWS.

Not sure how this is related to autotools, but I removed it nevertheless.


> On Oct. 20, 2016, 9:09 p.m., Jie Yu wrote:
> > src/posix/rlimit.cpp, lines 130-134
> > 
> >
> > Why do you need to call `getrlimit` here?

Removed.


> On Oct. 20, 2016, 9:09 p.m., Jie Yu wrote:
> > src/common/parse.hpp, line 179
> > 
> >
> > please add to `src/v1/parse.hpp` as well

Also added an `operator<<` in v1.


- Benjamin


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


On Oct. 21, 2016, 12:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53061/
> ---
> 
> (Updated Oct. 21, 2016, 12:38 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6426
> https://issues.apache.org/jira/browse/MESOS-6426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds high-level functions to set POSIX resource limits and
> associated protobuf definitions.
> 
> This is intended to be used by the containerizer in later changes.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
>   include/mesos/type_utils.hpp 88c7bcf7e8c3053e6e8bb78c67bf93b8276ac053 
>   include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
>   src/CMakeLists.txt e60d34ca4b714bb237d5864b4682280f11b147a6 
>   src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
>   src/common/parse.hpp ee26717ed54445f57fa3a11c74ece8fe0861bd40 
>   src/common/type_utils.cpp e249fae458d0fc1d32ea4e8b4a26d1a2d75826f1 
>   src/posix/rlimit.hpp PRE-CREATION 
>   src/posix/rlimit.cpp PRE-CREATION 
>   src/v1/mesos.cpp 4c8ee9121db08e5c462b9b611c692b751cfbf40b 
>   src/v1/parse.hpp 8882b32e0ee833cc1e261c7dde0bea4e844b7416 
> 
> Diff: https://reviews.apache.org/r/53061/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53078/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 53078: Added tests for rlimit isolator.

[Benjamin Bannier]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added tests for rlimit isolator.


Diffs
-

  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
  src/tests/containerizer/posix_rlimit_isolator_tests.cpp PRE-CREATION 

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


Testing
---

* make check (OS X, clang-trunk w/o optimizations)
* tested on a number of Linux setups in internal CI


Thanks,

Benjamin Bannier

Re: Review Request 53061: Added abstractions for working with POSIX resource limits.

[Benjamin Bannier]

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

(Updated Oct. 21, 2016, 12:38 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed comments from Jie.


Summary (updated)
-

Added abstractions for working with POSIX resource limits.


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


Repository: mesos


Description
---

This commit adds high-level functions to set POSIX resource limits and
associated protobuf definitions.

This is intended to be used by the containerizer in later changes.


Diffs (updated)
-

  include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
  include/mesos/type_utils.hpp 88c7bcf7e8c3053e6e8bb78c67bf93b8276ac053 
  include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
  src/CMakeLists.txt e60d34ca4b714bb237d5864b4682280f11b147a6 
  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
  src/common/parse.hpp ee26717ed54445f57fa3a11c74ece8fe0861bd40 
  src/common/type_utils.cpp e249fae458d0fc1d32ea4e8b4a26d1a2d75826f1 
  src/posix/rlimit.hpp PRE-CREATION 
  src/posix/rlimit.cpp PRE-CREATION 
  src/v1/mesos.cpp 4c8ee9121db08e5c462b9b611c692b751cfbf40b 
  src/v1/parse.hpp 8882b32e0ee833cc1e261c7dde0bea4e844b7416 

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


Testing (updated)
---

Tested as part of https://reviews.apache.org/r/53078/.


Thanks,

Benjamin Bannier

Re: Review Request 53063: Added rlimit isolator.

[Benjamin Bannier]

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

(Updated Oct. 21, 2016, 12:38 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Broke tests out into separate pr for easier backported (request by Jie); 
rebased.


Summary (updated)
-

Added rlimit isolator.


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


Repository: mesos


Description
---

This commit adds an isolator for POSIX resource limits. Since the
containerizer sets limits as the user of the container setting
permissions beyond the configured system limits likely is only
supported for containers running as root.


Diffs (updated)
-

  include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
  include/mesos/slave/containerizer.proto 
94e8cb4c37aa2a06c59726773834812c5eb660d8 
  include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
  src/CMakeLists.txt e60d34ca4b714bb237d5864b4682280f11b147a6 
  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
  src/slave/containerizer/mesos/containerizer.cpp 
eac70d955e08142a2d054039d610a3d516b1b57e 
  src/slave/containerizer/mesos/isolators/posix/rlimit.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/posix/rlimit.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
4a41aaf103f5a9bc6f7a798f63f491fc7cf11f7e 

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


Testing (updated)
---

Tested as part of https://reviews.apache.org/r/53078/.


Thanks,

Benjamin Bannier

Re: Review Request 53062: Add rlimit support to Mesos containerizer.

[Benjamin Bannier]

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

(Updated Oct. 21, 2016, 12:38 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed comments from Jie.


Summary (updated)
-

Add rlimit support to Mesos containerizer.


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


Repository: mesos


Description
---

This commit adds a new launch flag `--rlimits` which can be used to
specify POSIX resource limits for the container. The resource limits
are set as the user, so to increase resource limits beyond configured
system limits additional priviledges might be needed.


Diffs (updated)
-

  src/slave/containerizer/mesos/launch.hpp 
f8bac0650965a49562b9910bf6140ded8dbb69ac 
  src/slave/containerizer/mesos/launch.cpp 
4a41aaf103f5a9bc6f7a798f63f491fc7cf11f7e 

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


Testing (updated)
---

Tested as part of https://reviews.apache.org/r/53078/.


Thanks,

Benjamin Bannier

Review Request 52840: Silenced a GMock warning in ROOT_DOCKER_DockerInspectDiscard.

[Neil Conway]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Silenced a GMock warning in ROOT_DOCKER_DockerInspectDiscard.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 

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


Testing
---

Without this patch, `"sudo ./src/mesos-tests 
--gtest_filter="DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard"` 
consistently results in:

```
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard

GMOCK WARNING:
Uninteresting mock function call - returning directly.
Function call: executorLost(0x7ffe1aad15e0, @0x7f2018000b00 e1, 
@0x7f2018000c30 0aec7dbf-3ce3-4d3a-a086-a95bd1e8dada-S0, -1)
Stack trace:
[   OK ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard (624 ms)
```

With this patch, running the test 100 times in a loop does not produce a 
warning.


Thanks,

Neil Conway

Re: Review Request 53067: Added log message when terminating agent ID.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53033, 53035, 53067]

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 Oct. 20, 2016, 4:53 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53067/
> ---
> 
> (Updated Oct. 20, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When an agent ID is terminated (e.g., due to "--recover=cleanup"), it
> seems important to notify the operator that the agent will be assigned a
> new agent ID when it is next started.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
> 
> Diff: https://reviews.apache.org/r/53067/diff/
> 
> 
> Testing
> ---
> 
> Visual inspection of log output.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[Jiang Yan Xu]

> On Oct. 20, 2016, 1:50 p.m., Jie Yu wrote:
> > src/tests/containerizer/cgroups_tests.cpp, line 412
> > 
> >
> > Who remove the cgroup created above?

Hmm, the intention is to have them destroyed in teardown but I guess these 
cgroups don't start with `TEST_CGROUPS_ROOT`. I'll add back these removal lines 
and we can think about how to do auto-cleanup better.


- Jiang Yan


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


On Oct. 20, 2016, 10:38 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Oct. 20, 2016, 10:38 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In most cases, we just want to get the children cgroups instead of
> retrieve descendant cgroups recursively. We added an argument to
> `cgroups::get` to indicate whether retrieve cgroups recursively and
> make non-recursive retrieve become the default behaviour. This patch
> fixed some incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
>   src/linux/cgroups.cpp 1475c8eb848ece2a093e8243b51e9ce08981dd7d 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
>   src/tests/mesos.cpp 2aae160fb941ab3672a5665ae27f517ff40600e2 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[Jie Yu]

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




src/tests/containerizer/cgroups_tests.cpp (line 412)


Who remove the cgroup created above?


- Jie Yu


On Oct. 20, 2016, 5:38 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Oct. 20, 2016, 5:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In most cases, we just want to get the children cgroups instead of
> retrieve descendant cgroups recursively. We added an argument to
> `cgroups::get` to indicate whether retrieve cgroups recursively and
> make non-recursive retrieve become the default behaviour. This patch
> fixed some incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
>   src/linux/cgroups.cpp 1475c8eb848ece2a093e8243b51e9ce08981dd7d 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
>   src/tests/mesos.cpp 2aae160fb941ab3672a5665ae27f517ff40600e2 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 53077: Redefined helper macros with existing 'create' functions.

[Joris Van Remoortere]

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

Review request for mesos, Anand Mazumdar and Jie Yu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/mesos.hpp 08bfedb5c3d5a7cc86ab20316c7135f65f8612fb 

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


Testing
---


Thanks,

Joris Van Remoortere

Review Request 53076: Alphabetized 'evolve' and 'devolve' files.

[Joris Van Remoortere]

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

Review request for mesos, Anand Mazumdar and Jie Yu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/internal/devolve.hpp 90681ebe454e12e39b214248f7457931e32295dc 
  src/internal/devolve.cpp efcc5d6050c5cb2fb79a4d676e4427f02e03eae9 
  src/internal/evolve.hpp 128cff3bc6709609dac948bf99a5bc3334a4e961 
  src/internal/evolve.cpp caa193868ec36b258390d2cb6d4572ee07df705a 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 53018: Fixed a comment in `stat.hpp`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 19, 2016, 8:23 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53018/
> ---
> 
> (Updated Oct. 19, 2016, 8:23 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a comment in `stat.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 
> 32fa426a2d108a1d661ef7b48e11b7aa8413fb1a 
> 
> Diff: https://reviews.apache.org/r/53018/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[Jie Yu]

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




src/linux/cgroups.hpp (line 227)


I would suggest we set default to true to match the previous semantics.


- Jie Yu


On Oct. 20, 2016, 5:38 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Oct. 20, 2016, 5:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In most cases, we just want to get the children cgroups instead of
> retrieve descendant cgroups recursively. We added an argument to
> `cgroups::get` to indicate whether retrieve cgroups recursively and
> make non-recursive retrieve become the default behaviour. This patch
> fixed some incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
>   src/linux/cgroups.cpp 1475c8eb848ece2a093e8243b51e9ce08981dd7d 
>   src/tests/containerizer/cgroups_tests.cpp 
> 0afaec6ae948cabf1472bf01103210d8f9809cb1 
>   src/tests/mesos.cpp 2aae160fb941ab3672a5665ae27f517ff40600e2 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Joris Van Remoortere]

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

(Updated Oct. 20, 2016, 8 p.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


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


Repository: mesos


Description
---

Split mesos test helpers into 'internal' and 'v1' namespaces.


Diffs (updated)
-

  src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
  src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
  src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
  src/tests/default_executor_tests.cpp 92e6b9f5fb80811c94632de3bb20c8e6d2e895ff 
  src/tests/executor_http_api_tests.cpp 
a9f1a7b0498acd541c6f58ad1388da49c9951e22 
  src/tests/fault_tolerance_tests.cpp 95ac98c5f6a8a5e458e27792369b7a9d53306741 
  src/tests/http_fault_tolerance_tests.cpp 
57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
  src/tests/master_contender_detector_tests.cpp 
2a7d713f74c907235f82d83eaf46630046645faf 
  src/tests/master_maintenance_tests.cpp 
6917272f2de7a09bf4de7e932994655f4e54d3da 
  src/tests/master_tests.cpp b31502fb3a785b6d46bb71487a08786aae22357d 
  src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
  src/tests/scheduler_http_api_tests.cpp 
6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
  src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
  src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 
  src/tests/slave_tests.cpp e7e3e33f640e35be688e3890f2248a788726c1f1 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 53014: Removed unused tests helper macro 'DEFAULT_CONTAINER_ID'.

[Joris Van Remoortere]

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

(Updated Oct. 20, 2016, 7:53 p.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Repository: mesos


Description (updated)
---

Removed unused tests helper macro 'DEFAULT_CONTAINER_ID'.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
a4a7e5b9a0c1f09cd03cc5b50e628e74c610c9b6 
  src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 53013: Removed extra 'evolve' implementation from 'api_tests.cpp'.

[Joris Van Remoortere]

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

(Updated Oct. 20, 2016, 7:53 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Jie Yu.


Repository: mesos


Description (updated)
---

Removed extra 'evolve' implementation from 'api_tests.cpp'.


Diffs (updated)
-

  src/internal/evolve.hpp a38842a726f0e2634ae74b91f83b15ab1e656a47 
  src/internal/evolve.cpp b4a95771974ef11fda896d04a700c3d3d4fa024c 
  src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
  src/tests/resources_tests.cpp 6a12783c26f359dda835b4866b299a8fcfb3f972 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 53063: WIP: Added rlimit isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53061, 53062, 53063]

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 Oct. 20, 2016, 4:33 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53063/
> ---
> 
> (Updated Oct. 20, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6426
> https://issues.apache.org/jira/browse/MESOS-6426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds an isolator for POSIX resource limits. Since the
> containerizer sets limits as the user of the container setting
> permissions beyond the configured system limits likely is only
> supported for containers running as root.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
>   include/mesos/slave/containerizer.proto 
> 94e8cb4c37aa2a06c59726773834812c5eb660d8 
>   include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
>   src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> eac70d955e08142a2d054039d610a3d516b1b57e 
>   src/slave/containerizer/mesos/isolators/posix/rlimit.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/posix/rlimit.cpp PRE-CREATION 
>   src/tests/containerizer/posix_rlimit_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53063/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53061/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53062: WIP: Added rlimit support to Mesos containerizer.

[Jie Yu]

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




src/slave/containerizer/mesos/launch.cpp (line 45)


Let's wrap this with ifndef WINDOWS



src/slave/containerizer/mesos/launch.cpp (line 105)


That makes me think if we should make 'unlimited' be more explicit. With 
the current scheme, unlimited of RLIMIT_FSIZE will be represented as:
```
'{"rlimits":[{"type":"RLMT_FSIZE"}]}'
```

I am wondering if we should introduce an optional `unlimited` field:
```
message RLimit {
  optional Type type = 1;
  optional uint64_t soft = 2;
  optional uint64_t hard = 3;
  optional bool unlimited = 4;
}

'{"rlimits":[{"type":"RLMT_FSIZE","unlimited":true}]}'
```



src/slave/containerizer/mesos/launch.cpp (lines 591 - 602)


This won't work because data services explicitly want to get limit above 
what's configured.


- Jie Yu


On Oct. 20, 2016, 4:33 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53062/
> ---
> 
> (Updated Oct. 20, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6426
> https://issues.apache.org/jira/browse/MESOS-6426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a new launch flag `--rlimits` which can be used to
> specify POSIX resource limits for the container. The resource limits
> are set as the user, so to increase resource limits beyond configured
> system limits additional priviledges might be needed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> f8bac0650965a49562b9910bf6140ded8dbb69ac 
>   src/slave/containerizer/mesos/launch.cpp 
> 4a41aaf103f5a9bc6f7a798f63f491fc7cf11f7e 
> 
> Diff: https://reviews.apache.org/r/53062/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53061/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53061: WIP: Added abstractions for working with POSIX resource limits.

[Jie Yu]

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




include/mesos/mesos.proto (line 2099)


s/no limit/unlimited/



include/mesos/v1/mesos.proto (line 2098)


Ditto.



src/common/parse.hpp (line 179)


please add to `src/v1/parse.hpp` as well



src/common/type_utils.cpp (line 513)


2 lines apart



src/posix/rlimit.hpp (line 25)


Windows does not use autotool. So you don't have to do this. We just need 
to make sure all callsites are wrapped with ifndef WINDOWS.



src/posix/rlimit.hpp (line 28)


Let's put under mesos::internal::rlimits namespace



src/posix/rlimit.cpp (line 40)


not supported



src/posix/rlimit.cpp (lines 43 - 45)


I'll put this to the end.



src/posix/rlimit.cpp (line 67)


Looks like STACK is in posix:
http://pubs.opengroup.org/onlinepubs/009695399/functions/getrlimit.html



src/posix/rlimit.cpp (lines 130 - 134)


Why do you need to call `getrlimit` here?



src/posix/rlimit.cpp (lines 135 - 136)


We need to use RLIM_INFINITY if both soft and hard are not et.


- Jie Yu


On Oct. 20, 2016, 5:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53061/
> ---
> 
> (Updated Oct. 20, 2016, 5:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6426
> https://issues.apache.org/jira/browse/MESOS-6426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds high-level functions to set POSIX resource limits and
> associated protobuf definitions.
> 
> This is intended to be used by the containerizer in later changes.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
>   include/mesos/type_utils.hpp 88c7bcf7e8c3053e6e8bb78c67bf93b8276ac053 
>   include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
>   src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
>   src/common/parse.hpp ee26717ed54445f57fa3a11c74ece8fe0861bd40 
>   src/common/type_utils.cpp e249fae458d0fc1d32ea4e8b4a26d1a2d75826f1 
>   src/posix/rlimit.hpp PRE-CREATION 
>   src/posix/rlimit.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53061/diff/
> 
> 
> Testing
> ---
> 
> * make check (OS X, clang-trunk w/o optimizations)
> * tested on a number of Linux setups in internal CI
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52543: Added configure/make options to build the new CLI and run unit tests.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:13 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


Bugs: MESOS-6008 and MESOS-6032
https://issues.apache.org/jira/browse/MESOS-6008
https://issues.apache.org/jira/browse/MESOS-6032


Repository: mesos


Description
---

Added configure/make options to build the new CLI and run unit tests.


Diffs (updated)
-

  configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 
  docs/getting-started.md de3fba4a821ca7fa9b9e2fd692b33b40edd35b68 
  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 

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


Testing
---

../configure --enable-new-cli
make -C src mesos
src/mesos

GTEST_FILTER="" make -j check
sudo GTEST_FILTER="" make -j check


Thanks,

Kevin Klues

Re: Review Request 52953: Added container 'exec' command to the CLI.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:13 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added container 'exec' command to the CLI.


Diffs (updated)
-

  src/cli_new/lib/mesos/plugins/container/main.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/container/tests.py PRE-CREATION 
  src/cli_new/lib/mesos/tests/base.py PRE-CREATION 
  src/cli_new/lib/mesos/util.py 87d2a65e78f04209566c1434b489b941d570ee01 
  src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d4318f2d0e439a6edf 

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


Testing
---

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ sudo bin/mesos-cli-tests


Thanks,

Kevin Klues

Re: Review Request 52952: Added a 'container' plugin to the CLI with a 'list()' command.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:12 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added a 'container' plugin to the CLI with a 'list()' command.


Diffs (updated)
-

  src/cli_new/bin/config.py 274f8c63b0c642637f17aa2e3c8c4a8a5a059e37 
  src/cli_new/bin/tests.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/container/__init__.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/container/main.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/container/tests.py PRE-CREATION 

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


Testing
---

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Kevin Klues

Re: Review Request 52948: Added a Table abstraction and some utility functions to the new CLI.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:12 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

These will be used by future plugins.


Diffs (updated)
-

  src/cli_new/lib/mesos/http.py PRE-CREATION 
  src/cli_new/lib/mesos/util.py 87d2a65e78f04209566c1434b489b941d570ee01 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52950: Extended the basic CLI unit test infrastructure.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:12 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

This infrastrcture includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster.


Diffs (updated)
-

  src/cli_new/bin/tests.py PRE-CREATION 
  src/cli_new/lib/mesos/http.py PRE-CREATION 
  src/cli_new/lib/mesos/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/mesos/tests/base.py PRE-CREATION 
  src/cli_new/lib/mesos/tests/tests.py PRE-CREATION 

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


Testing
---

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Kevin Klues

Re: Review Request 51008: Added infrastructure for unit tests in the new python-based CLI.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:12 p.m.)


Review request for mesos, Haris Choudhary and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added infrastructure for unit tests in the new python-based CLI.


Diffs (updated)
-

  src/cli_new/bin/mesos-cli-tests PRE-CREATION 
  src/cli_new/bin/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d4318f2d0e439a6edf 

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


Testing
---

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Kevin Klues

Re: Review Request 52951: Introduced the notion of an AGENT_IP in the new CLI.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:12 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Introduced the notion of an AGENT_IP in the new CLI.


Diffs (updated)
-

  src/cli_new/bin/config.py 274f8c63b0c642637f17aa2e3c8c4a8a5a059e37 
  src/cli_new/lib/mesos/plugins/base.py 
61b15428b029cec9ec7c9d89ab949959c3dc5b88 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52947: Updated parsing for CLI config to be more dynamic.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:12 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

This includes the ability to inject the values from these config
parameters into plugin help strings.


Diffs (updated)
-

  src/cli_new/bin/config.py 274f8c63b0c642637f17aa2e3c8c4a8a5a059e37 
  src/cli_new/lib/mesos/plugins/base.py 
61b15428b029cec9ec7c9d89ab949959c3dc5b88 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52946: Fixed some bugs in the CLI help formatting.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:11 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Fixed some bugs in the CLI help formatting.


Diffs (updated)
-

  src/cli_new/lib/mesos/plugins/base.py 
61b15428b029cec9ec7c9d89ab949959c3dc5b88 
  src/cli_new/lib/mesos/util.py 87d2a65e78f04209566c1434b489b941d570ee01 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52944: Updated CLI pylint configuration to disable 'fixme' warnings.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:11 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Updated CLI pylint configuration to disable 'fixme' warnings.


Diffs (updated)
-

  src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52942: Updated CLI pylint configuration to ignore the 'netifaces' module.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:11 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Updated CLI pylint configuration to ignore the 'netifaces' module.


Diffs (updated)
-

  src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52945: Added the ability for a CLI plugin command to have an alias.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:11 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added the ability for a CLI plugin command to have an alias.


Diffs (updated)
-

  src/cli_new/lib/mesos/plugins/base.py 
61b15428b029cec9ec7c9d89ab949959c3dc5b88 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52943: Updated CLI pylint configuration to disable no-self-use warnings.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:11 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Updated CLI pylint configuration to disable no-self-use warnings.


Diffs (updated)
-

  src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52941: Updated CLI pylint configuration to allow 0 public methods.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:11 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Updated CLI pylint configuration to allow 0 public methods.


Diffs (updated)
-

  src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52939: Updated CLI pylint configuration to add 'ip' to the list of good-names.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 6:11 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased


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


Repository: mesos


Description
---

Updated CLI pylint configuration to add 'ip' to the list of good-names.


Diffs (updated)
-

  src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 

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


Testing
---


Thanks,

Kevin Klues

Review Request 53074: Updated pylint to rebuild 'virtualenv' if pip-requirements.txt modified.

[Kevin Klues]

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

Review request for mesos and Joseph Wu.


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


Repository: mesos


Description
---

Updated pylint to rebuild 'virtualenv' if pip-requirements.txt modified.


Diffs
-

  support/mesos-style.py 3a5bec81950ccf8083cbd73577c49b2fb3b910f8 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52939: Updated CLI pylint configuration to add 'ip' to the list of good-names.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 5:22 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Updated based on Joseph's comments to only add 'ip' to the list of good names.


Summary (updated)
-

Updated CLI pylint configuration to add 'ip' to the list of good-names.


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


Repository: mesos


Description (updated)
---

Updated CLI pylint configuration to add 'ip' to the list of good-names.


Diffs (updated)
-

  src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52941: Updated CLI pylint configuration to allow 0 public methods.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 5:23 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Updated CLI pylint configuration to allow 0 public methods.


Diffs (updated)
-

  src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 52953: Added container 'exec' command to the CLI.

[Kevin Klues]

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

(Updated Oct. 20, 2016, 5:26 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Updated to rename 'io' variable to something more descriptive.


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


Repository: mesos


Description
---

Added container 'exec' command to the CLI.


Diffs (updated)
-

  src/cli_new/lib/mesos/plugins/container/main.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/container/tests.py PRE-CREATION 
  src/cli_new/lib/mesos/tests/base.py PRE-CREATION 
  src/cli_new/lib/mesos/util.py 87d2a65e78f04209566c1434b489b941d570ee01 
  src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d4318f2d0e439a6edf 

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


Testing
---

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ sudo bin/mesos-cli-tests


Thanks,

Kevin Klues

Re: Review Request 53055: Fixed incorrect check in dynamic_cast.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53055]

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 Oct. 20, 2016, 11:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53055/
> ---
> 
> (Updated Oct. 20, 2016, 11:52 a.m.)
> 
> 
> Review request for mesos, Michael Park and Till Toenshoff.
> 
> 
> Bugs: MESOS-6424
> https://issues.apache.org/jira/browse/MESOS-6424
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We checked here whether the passed pointer is not null, while we in
> fact wanted to confirm that the performed dynamic_cast succeeded
> (like is already done e.g., in the definitions of flags.stringify or
> flags.validate).
> 
> This change makes us check the result of the dynamic_cast like
> originally intended.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 15446de795a620e9759faf3a552c234dc739fa3f 
> 
> Diff: https://reviews.apache.org/r/53055/diff/
> 
> 
> Testing
> ---
> 
> * make check (OS X, clang-trunk w/o optimizations)
> * tested on a number of Linux setups in internal CI
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53053: Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.

[Avinash sridharan]

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




src/slave/containerizer/mesos/provisioner/docker/store.cpp (lines 361 - 382)


If I understand this correctly, for directories AUFS relies on the 
`WHITEOUT_OPAQUE_PREFIX` file to remove directories between layers and regular 
whiteout files for removing files, whereas overlayfs relies on setting 
attributes (trusted.overlay.opaque) on the directory to remove them between 
layers and requires a character device file of ID 0 for removing reqular files. 

Would be great to capture this logic in a comment here with citations to 
the overlayfs documentation 
https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt

and the AUFS documentation.


- Avinash sridharan


On Oct. 20, 2016, 1:53 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53053/
> ---
> 
> (Updated Oct. 20, 2016, 1:53 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> e192f86a1848b373f3aa73d29688a96375cac313 
> 
> Diff: https://reviews.apache.org/r/53053/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 53061: Added abstractions for working with POSIX resource limits.

[Benjamin Bannier]

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

(Updated Oct. 20, 2016, 7:01 p.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

Added abstractions for working with POSIX resource limits.


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


Repository: mesos


Description
---

This commit adds high-level functions to set POSIX resource limits and
associated protobuf definitions.

This is intended to be used by the containerizer in later changes.


Diffs (updated)
-

  include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
  include/mesos/type_utils.hpp 88c7bcf7e8c3053e6e8bb78c67bf93b8276ac053 
  include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
  src/common/parse.hpp ee26717ed54445f57fa3a11c74ece8fe0861bd40 
  src/common/type_utils.cpp e249fae458d0fc1d32ea4e8b4a26d1a2d75826f1 
  src/posix/rlimit.hpp PRE-CREATION 
  src/posix/rlimit.cpp PRE-CREATION 

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


Testing
---

* make check (OS X, clang-trunk w/o optimizations)
* tested on a number of Linux setups in internal CI


Thanks,

Benjamin Bannier

Re: Review Request 53061: WIP: Added abstractions for working with POSIX resource limits.

[Benjamin Bannier]

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

(Updated Oct. 20, 2016, 7:01 p.m.)


Review request for mesos and Jie Yu.


Summary (updated)
-

WIP: Added abstractions for working with POSIX resource limits.


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


Repository: mesos


Description
---

This commit adds high-level functions to set POSIX resource limits and
associated protobuf definitions.

This is intended to be used by the containerizer in later changes.


Diffs
-

  include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
  include/mesos/type_utils.hpp 88c7bcf7e8c3053e6e8bb78c67bf93b8276ac053 
  include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
  src/common/parse.hpp ee26717ed54445f57fa3a11c74ece8fe0861bd40 
  src/common/type_utils.cpp e249fae458d0fc1d32ea4e8b4a26d1a2d75826f1 
  src/posix/rlimit.hpp PRE-CREATION 
  src/posix/rlimit.cpp PRE-CREATION 

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


Testing
---

* make check (OS X, clang-trunk w/o optimizations)
* tested on a number of Linux setups in internal CI


Thanks,

Benjamin Bannier

Review Request 53067: Added log message when terminating agent ID.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

When an agent ID is terminated (e.g., due to "--recover=cleanup"), it
seems important to notify the operator that the agent will be assigned a
new agent ID when it is next started.


Diffs
-

  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 

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


Testing
---

Visual inspection of log output.


Thanks,

Neil Conway

Review Request 53062: WIP: Added rlimit support to Mesos containerizer.

[Benjamin Bannier]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This commit adds a new launch flag `--rlimits` which can be used to
specify POSIX resource limits for the container. The resource limits
are set as the user, so to increase resource limits beyond configured
system limits additional priviledges might be needed.


Diffs
-

  src/slave/containerizer/mesos/launch.hpp 
f8bac0650965a49562b9910bf6140ded8dbb69ac 
  src/slave/containerizer/mesos/launch.cpp 
4a41aaf103f5a9bc6f7a798f63f491fc7cf11f7e 

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


Testing
---

Tested as part of https://reviews.apache.org/r/53061/.


Thanks,

Benjamin Bannier

Review Request 53061: WIP: Added abstractions for working with POSIX resource limits.

[Benjamin Bannier]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This commit adds high-level functions to set POSIX resource limits and
associated protobuf definitions.

This is intended to be used by the containerizer in later changes.


Diffs
-

  include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
  include/mesos/type_utils.hpp 88c7bcf7e8c3053e6e8bb78c67bf93b8276ac053 
  include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
  src/common/parse.hpp ee26717ed54445f57fa3a11c74ece8fe0861bd40 
  src/common/type_utils.cpp e249fae458d0fc1d32ea4e8b4a26d1a2d75826f1 
  src/posix/rlimit.hpp PRE-CREATION 
  src/posix/rlimit.cpp PRE-CREATION 

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


Testing
---

* make check (OS X, clang-trunk w/o optimizations)
* tested on a number of Linux setups in internal CI


Thanks,

Benjamin Bannier

Review Request 53063: WIP: Added rlimit isolator.

[Benjamin Bannier]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This commit adds an isolator for POSIX resource limits. Since the
containerizer sets limits as the user of the container setting
permissions beyond the configured system limits likely is only
supported for containers running as root.


Diffs
-

  include/mesos/mesos.proto 9a268dd0aa2f5ee245ad26bf40ab9b12fe861d2f 
  include/mesos/slave/containerizer.proto 
94e8cb4c37aa2a06c59726773834812c5eb660d8 
  include/mesos/v1/mesos.proto 855377f56e1693e11b3c0c0cb61a2c3013a66f54 
  src/Makefile.am 05fe8fa3e3409b35ba66d7e7238e988fb0fab07d 
  src/slave/containerizer/mesos/containerizer.cpp 
eac70d955e08142a2d054039d610a3d516b1b57e 
  src/slave/containerizer/mesos/isolators/posix/rlimit.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/posix/rlimit.cpp PRE-CREATION 
  src/tests/containerizer/posix_rlimit_isolator_tests.cpp PRE-CREATION 

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


Testing
---

Tested as part of https://reviews.apache.org/r/53061/.


Thanks,

Benjamin Bannier

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Guangya Liu]

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




src/tests/containerizer/docker_containerizer_tests.cpp (line 3827)


How about

```
// This test verifies that the docker container can be
// launched with some GPU resources allocated when
// enabling Nvidia GPU allocator.
```



src/tests/containerizer/docker_containerizer_tests.cpp (line 3854)


s/in current/currently



src/tests/containerizer/docker_containerizer_tests.cpp (line 3858)


kill this


- Guangya Liu


On 十月 20, 2016, 10:01 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50127/
> ---
> 
> (Updated 十月 20, 2016, 10:01 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
> ---
> 
> This added a testing case for end-to-end GPU support for docker
> containerizer.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/50127/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
> check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 53049: Close socket after setting flags on the interface.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 20, 2016, 8:51 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53049/
> ---
> 
> (Updated Oct. 20, 2016, 8:51 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6420
> https://issues.apache.org/jira/browse/MESOS-6420
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Close socket after setting flags on the interface.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/link/internal.hpp 
> 551ffda22f69b8670f72b11260ec32551b6299f8 
> 
> Diff: https://reviews.apache.org/r/53049/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 52103: Implement quota update through `PUT` method.

[Alexander Rukletsov]

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



What about updating "quota.md"? How a quota update request will look like for 
an operator? I assume the same as quota set, but we should call it out in the 
docs.


src/master/allocator/mesos/hierarchical.cpp (lines 1140 - 1141)


Even if we leave one function in the allocator for setting *and* updating 
quota (I'm not convinced yet) we should, don't you think it is valuable to 
leave most of this comment so people understand why there are two separate 
paths?



src/master/allocator/mesos/hierarchical.cpp (lines 1205 - 1207)


Ups.



src/master/http.cpp (line 2349)


s/for the first time/without a quota/



src/master/master.hpp (lines 1028 - 1032)


We definitely need an override taking a master call, hence we should update 
the operator API as well. Feel free to do it either here or as a separate 
commit. You can look into https://reviews.apache.org/r/49247 for inspiration.



src/master/quota_handler.cpp (line 81)


This should  go away, either altogether or replaced by another comment.



src/master/quota_handler.cpp (lines 82 - 87)


Instead, you can do a check just for the delta (new - old), and only if the 
delta is positive.



src/master/quota_handler.cpp (line 363)


This should go away if you decide to keep both `set()` and `update()`.



src/master/quota_handler.cpp (lines 559 - 562)


This function is almost identical to `_set()`. Have you explored the 
possibility to calculate the delta (new - old) and re-use the set path if the 
delta > 0 and remove path otherwise?

If we can't express update internally in terms of set and remove, we should 
better maintain a separate update path (including allocator) for simplicity.



src/master/quota_handler.cpp (lines 580 - 585)


Do we need this? Isn't it subsumed by the next check?



src/master/quota_handler.cpp (line 603)


In this case you may rescind way more resources than necessary, right? 
Actaully, if new < old, we can skip capasity heuristic and rescinding 
altogether.


- Alexander Rukletsov


On Oct. 19, 2016, 11:28 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52103/
> ---
> 
> (Updated Oct. 19, 2016, 11:28 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-4941
> https://issues.apache.org/jira/browse/MESOS-4941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement quota update through `PUT` method.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 2d56bd011f2c87c67a02d0ae467a4a537d36867e 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
>   src/master/master.hpp 35db198748b8652eb53e17f592f6b40d1e6a3ed9 
>   src/master/quota_handler.cpp bf6a613a7bb3c62fd77d1ffde3170749d6c21fa2 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/52103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53053: Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.

[Avinash sridharan]

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




src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 342)


Is it possible to move this code to `stout`? Something on the lines of of 
os::overlayfs::wh?

The code is pretty cumbersome and breaks the usual idioms followed in 
Mesos. More importantly, as far as I understand, we will need the same code for 
OCI images as well since it follows the AUFS whiteout format as well?



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 343)


Shouldn't we be putting this code under `#ifdef __linux__`, given that it 
won't compile for windows? Or more importantly add a `stout` version for posix 
? FTS is not available for solaris as well I think?

Unless `store.cpp` is intended to compile specifically for linux since 
`overlayfs` itself is very specific to linux? In which case this comment is 
very confusing.



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 359)


There is an assumption that docker images follow the aufs whiteout 
standard. Probably add a comment and give a citation to the whiteout format 
followed by docker. In case docker changes tommorow developers would atleast 
understand the reasoning for this code.



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 368)


s/\"trusted.overlay.opaque\"/'trusted.overlay.opaque'



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 380)


break this up?
Failure(
"Failed to 


- Avinash sridharan


On Oct. 20, 2016, 1:53 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53053/
> ---
> 
> (Updated Oct. 20, 2016, 1:53 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6360
> https://issues.apache.org/jira/browse/MESOS-6360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> e192f86a1848b373f3aa73d29688a96375cac313 
> 
> Diff: https://reviews.apache.org/r/53053/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 53065: Removed unused function `paths::getArchiveDir`.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Removed unused function `paths::getArchiveDir`.


Diffs
-

  src/slave/paths.hpp ebff3a9fab3f4a8d38502fe02bd5a34cf95c1830 
  src/slave/paths.cpp eb414caae5847949854743d4e3c603217a897898 
  src/tests/paths_tests.cpp 0671ee25b484cacf08c9a20ee6eba88e6f14fa97 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53047: MESOS-6212: name format validation for mesos managed docker containers.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53047]

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 Oct. 20, 2016, 11:46 a.m., Manuwela Kanade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53047/
> ---
> 
> (Updated Oct. 20, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-6212
> https://issues.apache.org/jira/browse/MESOS-6212
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-6212: name format validation for mesos managed docker containers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/uuid_tests.cpp 
> fe9894af93df9cb9b12390d8d7e7885525db384d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/53047/diff/
> 
> 
> Testing
> ---
> 
> 1. Ran the "make check"
> 
> 2. Manual Testing done as below:
> Testing without changes: Created and ran an example container which is not 
> run using mesos : named "mesos-rsyslog" which got killed once mesos-agent 
> started with --containerizer=docker,mesos. 
> 
> Testing with changes: Created and ran an example container (not run using 
> mesos): named "mesos-rsyslog" which keeps running and mesos-agent does not 
> kill it thus avoiding the false positive
> 
> 
> Thanks,
> 
> Manuwela Kanade
> 
>

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Guangya Liu]

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




src/docker/executor.cpp (line 790)


kill this



src/slave/containerizer/docker.cpp (line 241)


s/Exposed/Expose



src/slave/containerizer/docker.cpp (lines 344 - 346)


Can you please update the comments as well, now we have two `None()` fields 
here, both the `taskEnvironment` and `devices`.

We do not set the optional `devices` here as this field is set when 
allocating gpu resources in `DockerContainerizerProcess::launchExecutorProcess`.

So what about update the comments as:

```
// NOTE: We do not set the optional `taskEnvironment` and `devices`
// here. The `taskEnvironment` field is currently used to propagate
// environment variables from a hook, this hook is called after
// `Container::create`. The `devices` field is currently used to
// expose Nvidia devices to the docker container, this is set in
// `DockerContainerizerProcess::launchExecutorProcess` which is also
// after `Container::create`. 
```


- Guangya Liu


On 十月 20, 2016, 10:01 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50125/
> ---
> 
> (Updated 十月 20, 2016, 10:01 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
> ---
> 
> Added a new flag '--devices' to mesos-docker-executor, and gave its
> feature to control devices exposition, isolation, and access permission.
> Also, passed GPUs assignment to mesos-docker-executor.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
> 
> Diff: https://reviews.apache.org/r/50125/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 52721: Fixed typo in log message.

[Neil Conway]

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

(Updated Oct. 20, 2016, 3:06 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Fixed typo in log message.


Diffs (updated)
-

  src/master/master.cpp 3f3ce93155069dd32731783ac4877ba6ee2519c0 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 53035: Tweaked an agent log message.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

The previous wording implied the agent was going to "attempt to
register"; in fact the agent might never attempt to "register" (only
re-register) depending on its local state.


Diffs
-

  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 

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


Testing
---

Visual inspection of log output.


Thanks,

Neil Conway

Review Request 53053: Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.

[Qian Zhang]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
e192f86a1848b373f3aa73d29688a96375cac313 

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


Testing
---

make check


Thanks,

Qian Zhang

Review Request 53058: Added tests for whole protobuf message based authorization.

[Alexander Rojas]

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

Review request for mesos, Adam B, Kapil Arya, and Till Toenshoff.


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


Repository: mesos


Description
---

Adds tests for the authorization of the authorization request
which makes use of whole protobuf messages.


Diffs
-

  src/tests/authorization_tests.cpp 5d7e17b67821357b8cb538798acc883945c8f8fd 

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


Testing
---

make check


Thanks,

Alexander Rojas

Review Request 53057: Updates calls to the authorizer to use whole protobuf messages.

[Alexander Rojas]

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

Review request for mesos, Adam B, Kapil Arya, and Till Toenshoff.


Repository: mesos


Description
---

Makes changes in the calls to the authorizer, so that it uses the
new protobuf message based authorization calls when available.
It still sets the traditional `Object.value` field for compatibility
with old authorizers.


Diffs
-

  src/master/http.cpp bb9c87327dfe2161a6f1fd4cded72aa9a5ffaf66 
  src/master/master.hpp 6d2db9de52d35f3288c618d05138413ce709818b 
  src/master/master.cpp 3f3ce93155069dd32731783ac4877ba6ee2519c0 
  src/master/quota_handler.cpp d87d6c6266ba596c4e6bd36124f5a360428fe171 
  src/master/weights_handler.cpp c240fb2980a7e957136d11ddd3f43ee4761a07dc 

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


Testing
---

Tests in the last patch of the series.


Thanks,

Alexander Rojas

Re: Review Request 53057: Updates calls to the authorizer to use whole protobuf messages.

[Alexander Rojas]

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

(Updated Oct. 20, 2016, 3:47 p.m.)


Review request for mesos, Adam B, Kapil Arya, and Till Toenshoff.


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


Repository: mesos


Description
---

Makes changes in the calls to the authorizer, so that it uses the
new protobuf message based authorization calls when available.
It still sets the traditional `Object.value` field for compatibility
with old authorizers.


Diffs
-

  src/master/http.cpp bb9c87327dfe2161a6f1fd4cded72aa9a5ffaf66 
  src/master/master.hpp 6d2db9de52d35f3288c618d05138413ce709818b 
  src/master/master.cpp 3f3ce93155069dd32731783ac4877ba6ee2519c0 
  src/master/quota_handler.cpp d87d6c6266ba596c4e6bd36124f5a360428fe171 
  src/master/weights_handler.cpp c240fb2980a7e957136d11ddd3f43ee4761a07dc 

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


Testing
---

Tests in the last patch of the series.


Thanks,

Alexander Rojas

Re: Review Request 53049: Close socket after setting flags on the interface.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53049]

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 Oct. 20, 2016, 8:51 a.m., Santhosh Kumar Shanmugham wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53049/
> ---
> 
> (Updated Oct. 20, 2016, 8:51 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6420
> https://issues.apache.org/jira/browse/MESOS-6420
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Close socket after setting flags on the interface.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/link/internal.hpp 
> 551ffda22f69b8670f72b11260ec32551b6299f8 
> 
> Diff: https://reviews.apache.org/r/53049/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Re: Review Request 52600: Enable multiple field based authorization in the authorizer interface.

[Alexander Rojas]

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

(Updated Oct. 20, 2016, 3:45 p.m.)


Review request for mesos, Adam B, Kapil Arya, and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Updates the authorizer interfaces and well as the local authorizer,
such that all actions which were limited to use a _role_ or a
_principal_ as an object, are able to use whole protobuf messages
as objects. This change enables more sofisticated authorization
mechanisms.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
cb365c7d8d088f2810bde11b72dc20843a18fa51 
  include/mesos/authorizer/authorizer.proto 
b6a9f142eecbdfd59210872a92e3126f04de334c 
  src/authorizer/local/authorizer.cpp f1dff65d973fc84f4171f68fd0391a2343a96965 

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


Testing (updated)
---

tests in the last patch of the chain.


Thanks,

Alexander Rojas

Re: Review Request 53005: Updated release guide.

[Till Toenshoff]

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


Fix it, then Ship it!





docs/release-guide.md (line 94)


s/If this/If this is/



docs/release-guide.md (lines 211 - 212)


"Also, make sure to add the names of the release managers in "Description" 
section?"

What are we trying to achieve here?


- Till Toenshoff


On Oct. 18, 2016, 10:47 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53005/
> ---
> 
> (Updated Oct. 18, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added information about release branches among other things.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md c5a12c5a9f36e07334b59edf0788359b42a3125f 
> 
> Diff: https://reviews.apache.org/r/53005/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 50841: Added GPU scheduling logic to docker containerizer process.

[Guangya Liu]

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




src/slave/containerizer/docker.hpp (lines 259 - 260)


This helper will not be called on non linux platform.

```
// Allocate GPU resources for a specified container on Linux.
```



src/slave/containerizer/docker.hpp (lines 269 - 270)


This helper will not be called on non linux platform.

```
// Deallocate GPU resources for a specified container on Linux.
```



src/slave/containerizer/docker.hpp (line 506)


s/GPU/GPU resources



src/slave/containerizer/docker.cpp (lines 661 - 662)


kill this, we already have comments for this helper in the header file. 
Also this will not be called on non-linux platform.



src/slave/containerizer/docker.cpp (line 665)


s/NvidiaGpuAllocator/NvidiaComponents



src/slave/containerizer/docker.cpp (lines 685 - 686)


kill this, we already have comments for this helper in the header file. 
Also this will not be called on non-linux platform.



src/slave/containerizer/docker.cpp (lines 704 - 705)


kill this, we already have comments for this helper in the header file. 
Also this will not be called on non-linux platform.



src/slave/containerizer/docker.cpp (lines 2151 - 2155)


Do you have a patch for removing this comment?



src/slave/containerizer/docker.cpp (lines 2156 - 2166)


What about the following:

```
string failure = "Failed to kill the Docker container: " +
 (kill.isFailed() ? kill.failure() : "discarded 
future");

#ifdef __linux__
if (!container->gpus.empty()) {
  failure += ", " + stringify(container->gpus.size()) + " GPUs leaked";
}
#endif // __linux__

container->termination.fail(failure);
```



src/slave/containerizer/docker.cpp (line 2210)


s/GPUs/GPU resources

It is better to make the concept consistent by using `GPU resources` for 
all of the comments.


- Guangya Liu


On 十月 20, 2016, 10 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50841/
> ---
> 
> (Updated 十月 20, 2016, 10 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and 
> Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added control logic to allocate/deallocate GPUs to GPU-related task
> when the task is started/terminated.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
> 
> Diff: https://reviews.apache.org/r/50841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 53055: Fixed incorrect check in dynamic_cast.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On Oct. 20, 2016, 11:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53055/
> ---
> 
> (Updated Oct. 20, 2016, 11:52 a.m.)
> 
> 
> Review request for mesos, Michael Park and Till Toenshoff.
> 
> 
> Bugs: MESOS-6424
> https://issues.apache.org/jira/browse/MESOS-6424
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We checked here whether the passed pointer is not null, while we in
> fact wanted to confirm that the performed dynamic_cast succeeded
> (like is already done e.g., in the definitions of flags.stringify or
> flags.validate).
> 
> This change makes us check the result of the dynamic_cast like
> originally intended.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 15446de795a620e9759faf3a552c234dc739fa3f 
> 
> Diff: https://reviews.apache.org/r/53055/diff/
> 
> 
> Testing
> ---
> 
> * make check (OS X, clang-trunk w/o optimizations)
> * tested on a number of Linux setups in internal CI
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 53055: Fixed incorrect check in dynamic_cast.

[Benjamin Bannier]

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

Review request for mesos, Michael Park and Till Toenshoff.


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


Repository: mesos


Description
---

We checked here whether the passed pointer is not null, while we in
fact wanted to confirm that the performed dynamic_cast succeeded
(like is already done e.g., in the definitions of flags.stringify or
flags.validate).

This change makes us check the result of the dynamic_cast like
originally intended.


Diffs
-

  3rdparty/stout/include/stout/flags/flags.hpp 
15446de795a620e9759faf3a552c234dc739fa3f 

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


Testing
---

* make check (OS X, clang-trunk w/o optimizations)
* tested on a number of Linux setups in internal CI


Thanks,

Benjamin Bannier

Re: Review Request 53047: MESOS-6212: name format validation for mesos managed docker containers.

[Manuwela Kanade]

> On Oct. 20, 2016, 9:55 a.m., haosdent huang wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, line 1501
> > 
> >
> > This test case would not go through `parse()` method. Since we only use 
> > state to construct the `Container` then put it in `containers_` directly 
> > which would bypass `parse`.
> > 
> > 
> > ```
> > // Create and store a container.
> > Container* container = new Container(containerId);
> > containers_[containerId] = container;
> > container->slaveId = state->id;
> > container->state = Container::RUNNING;
> > container->launchesExecutorContainer =
> >   executorContainers.contains(containerId);
> > ```

I will work on changing the testcase such that it goes through the parse() 
method. Will update soon


- Manuwela


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


On Oct. 20, 2016, 11:46 a.m., Manuwela Kanade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53047/
> ---
> 
> (Updated Oct. 20, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-6212
> https://issues.apache.org/jira/browse/MESOS-6212
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-6212: name format validation for mesos managed docker containers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/uuid_tests.cpp 
> fe9894af93df9cb9b12390d8d7e7885525db384d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/53047/diff/
> 
> 
> Testing
> ---
> 
> 1. Ran the "make check"
> 
> 2. Manual Testing done as below:
> Testing without changes: Created and ran an example container which is not 
> run using mesos : named "mesos-rsyslog" which got killed once mesos-agent 
> started with --containerizer=docker,mesos. 
> 
> Testing with changes: Created and ran an example container (not run using 
> mesos): named "mesos-rsyslog" which keeps running and mesos-agent does not 
> kill it thus avoiding the false positive
> 
> 
> Thanks,
> 
> Manuwela Kanade
> 
>

Re: Review Request 53047: MESOS-6212: name format validation for mesos managed docker containers.

[Manuwela Kanade]

> On Oct. 20, 2016, 8:58 a.m., haosdent huang wrote:
> > src/slave/containerizer/docker.cpp, lines 124-130
> > 
> >
> > I think need to put this in an `else` block, otherwise it would be 
> > perform although we get `containerId` above.

Added the else block


- Manuwela


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


On Oct. 20, 2016, 11:46 a.m., Manuwela Kanade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53047/
> ---
> 
> (Updated Oct. 20, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-6212
> https://issues.apache.org/jira/browse/MESOS-6212
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-6212: name format validation for mesos managed docker containers.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/uuid_tests.cpp 
> fe9894af93df9cb9b12390d8d7e7885525db384d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/53047/diff/
> 
> 
> Testing
> ---
> 
> 1. Ran the "make check"
> 
> 2. Manual Testing done as below:
> Testing without changes: Created and ran an example container which is not 
> run using mesos : named "mesos-rsyslog" which got killed once mesos-agent 
> started with --containerizer=docker,mesos. 
> 
> Testing with changes: Created and ran an example container (not run using 
> mesos): named "mesos-rsyslog" which keeps running and mesos-agent does not 
> kill it thus avoiding the false positive
> 
> 
> Thanks,
> 
> Manuwela Kanade
> 
>

Re: Review Request 53047: MESOS-6212: name format validation for mesos managed docker containers.

[Manuwela Kanade]

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

(Updated Oct. 20, 2016, 11:46 a.m.)


Review request for mesos, haosdent huang, Timothy Chen, and Vinod Kone.


Summary (updated)
-

MESOS-6212: name format validation for mesos managed docker containers.


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


Repository: mesos


Description (updated)
---

MESOS-6212: name format validation for mesos managed docker containers.


Diffs (updated)
-

  3rdparty/stout/tests/uuid_tests.cpp fe9894af93df9cb9b12390d8d7e7885525db384d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
  src/tests/containerizer/docker_containerizer_tests.cpp 
6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 

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


Testing
---

1. Ran the "make check"

2. Manual Testing done as below:
Testing without changes: Created and ran an example container which is not run 
using mesos : named "mesos-rsyslog" which got killed once mesos-agent started 
with --containerizer=docker,mesos. 

Testing with changes: Created and ran an example container (not run using 
mesos): named "mesos-rsyslog" which keeps running and mesos-agent does not kill 
it thus avoiding the false positive


Thanks,

Manuwela Kanade

Re: Review Request 53031: Failure in cleanup of non-existing cgroups is treated as success.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53031]

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 Oct. 20, 2016, 6:20 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53031/
> ---
> 
> (Updated Oct. 20, 2016, 6:20 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6414
> https://issues.apache.org/jira/browse/MESOS-6414
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Failure in cleanup of non-existing cgroups is treated as success.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 1475c8eb848ece2a093e8243b51e9ce08981dd7d 
> 
> Diff: https://reviews.apache.org/r/53031/diff/
> 
> 
> Testing
> ---
> 
> All existing tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 50127: Added DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch.

[Yubo Li]

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

(Updated 十月 20, 2016, 10:01 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
---

This added a testing case for end-to-end GPU support for docker
containerizer.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 

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


Testing
---

GTEST_FILTER="DockerContainerizerTest.ROOT_NVIDIA_GPU_DOCKER_Launch" make -j 
check


Thanks,

Yubo Li

Re: Review Request 53047: MESOS-6212: name format validation for mesos managed docker containers

[haosdent huang]

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




src/tests/containerizer/docker_containerizer_tests.cpp (line 1501)


This test case would not go through `parse()` method. Since we only use 
state to construct the `Container` then put it in `containers_` directly which 
would bypass `parse`.

```
// Create and store a container.
Container* container = new Container(containerId);
containers_[containerId] = container;
container->slaveId = state->id;
container->state = Container::RUNNING;
container->launchesExecutorContainer =
  executorContainers.contains(containerId);
```


- haosdent huang


On Oct. 20, 2016, 7:24 a.m., Manuwela Kanade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53047/
> ---
> 
> (Updated Oct. 20, 2016, 7:24 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, and Vinod Kone.
> 
> 
> Bugs: MESOS-6212
> https://issues.apache.org/jira/browse/MESOS-6212
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-6212: name format validation for mesos managed docker containers
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/uuid_tests.cpp 
> fe9894af93df9cb9b12390d8d7e7885525db384d 
>   src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6d26797abf6d2b5e42b9e7743789e1edc62c9c1a 
> 
> Diff: https://reviews.apache.org/r/53047/diff/
> 
> 
> Testing
> ---
> 
> 1. Ran the "make check"
> 
> 2. Manual Testing done as below:
> Testing without changes: Created and ran an example container which is not 
> run using mesos : named "mesos-rsyslog" which got killed once mesos-agent 
> started with --containerizer=docker,mesos. 
> 
> Testing with changes: Created and ran an example container (not run using 
> mesos): named "mesos-rsyslog" which keeps running and mesos-agent does not 
> kill it thus avoiding the false positive
> 
> 
> Thanks,
> 
> Manuwela Kanade
> 
>

Re: Review Request 50947: Removed isolator flag 'gpu/nvidia' for docker containerizer using GPU.

[Yubo Li]

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

(Updated 十月 20, 2016, 10:01 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Mesos containerizer uses isolator 'gpu/nvidia' for GPU isolation while
docker and external containerizers do not need it anymore. This removed
the 'gpu/nvidia' isolator check for docker and external containerizers.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
2e722691475c84afae14009014ea70cc0fdd0e65 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50125: Added mesos-docker-executor support for devices control.

[Yubo Li]

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

(Updated 十月 20, 2016, 10:01 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
---

Added a new flag '--devices' to mesos-docker-executor, and gave its
feature to control devices exposition, isolation, and access permission.
Also, passed GPUs assignment to mesos-docker-executor.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
  src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50841: Added GPU scheduling logic to docker containerizer process.

[Yubo Li]

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

(Updated 十月 20, 2016, 10 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Added control logic to allocate/deallocate GPUs to GPU-related task
when the task is started/terminated.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li

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

[Yubo Li]

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

(Updated 十月 20, 2016, 10 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 functions to 'Docker::Device' to handle data
parsing and serializing between 'Docker::Device' structure and
string.


Diffs (updated)
-

  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 50599: Assigned Nvidia GPU devices to docker container.

[Yubo Li]

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

(Updated 十月 20, 2016, 10:01 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, Kevin Klues, and Rajat 
Phull.


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


Repository: mesos


Description
---

Assigned Nvidia GPU devices to docker container based on
GPUs allocated by Nvidia GPU allocator.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 8da63101f951892e673612134770fc155d86112d 
  src/slave/containerizer/docker.cpp 8ec4c0a25335fb1b36cb2ab82577f6d3e2f7f008 

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


Testing
---

make check


Thanks,

Yubo Li