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 八月 15, 2016, 7:24 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 'NvidiaGpuAllocator' 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 f2a06065cf99fed934c2c1ffc47461ec8a97f50d 
  src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 
  src/tests/mesos.hpp ad31276aeb2cb7ed5ba3e091a9085f35addf17c4 
  src/tests/mesos.cpp 62e8fcc6fa7bd856aab6148ca6e6cad66b436f04 

Diff: https://reviews.apache.org/r/50123/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 八月 15, 2016, 7:26 a.m.)


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


Changes
---

The implementation is a little different from yours, please have a look.


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 f2a06065cf99fed934c2c1ffc47461ec8a97f50d 
  src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 

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 八月 15, 2016, 7:26 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 'serialize()' and 'parse()' to 'Docker::Device'
to handle data tranformation between 'Docker::Device' structure and
string.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 

Diff: https://reviews.apache.org/r/50128/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 八月 15, 2016, 7: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.


Diffs (updated)
-

  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/docker/executor.cpp 8d679cd33b6ddf3a5c11bb8c458a97b8809473ac 
  src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 

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


Testing
---

make check


Thanks,

Yubo Li

Re: Review Request 50599: Passed allocated GPUs to 'devices' entry of 'docker::Flags'.

[Yubo Li]

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

(Updated 八月 15, 2016, 7:29 a.m.)


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


Changes
---

Added a new member varible `devices` to `container` and wrapped nvidia GPU 
related logic to `DockerContainerizerProcess::allocateNvidiaGpu()`


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


Repository: mesos


Description
---

Added a new string entry 'devices' to 'docker::Flags' to
indicate host devices exposed into docker container, and
passed allocated GPUs to it.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp f2a06065cf99fed934c2c1ffc47461ec8a97f50d 
  src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 

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


Testing
---

make 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 八月 15, 2016, 7:29 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 
c1a87e9e5c07529bc1d077f68477108a40506806 

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


Testing
---

make check


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 八月 15, 2016, 7: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 
42d4364b6fcbc94c7852721511001c103cb5a90d 

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 50123: Added GPU scheduler for docker containerizer process.

[Guangya Liu]

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




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


kill this


- Guangya Liu


On 八月 15, 2016, 7:24 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50123/
> ---
> 
> (Updated 八月 15, 2016, 7:24 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 'NvidiaGpuAllocator' to docker containerizer process so that
> docker containerizer process is ready to use it to allocate GPUs to task
> with 'gpus' resource.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp f2a06065cf99fed934c2c1ffc47461ec8a97f50d 
>   src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 
>   src/tests/mesos.hpp ad31276aeb2cb7ed5ba3e091a9085f35addf17c4 
>   src/tests/mesos.cpp 62e8fcc6fa7bd856aab6148ca6e6cad66b436f04 
> 
> Diff: https://reviews.apache.org/r/50123/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Review Request 51091: Disallowed unknown cgroups isolator.

[haosdent huang]

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

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


Repository: mesos


Description
---

Disallowed unknown cgroups isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 

Diff: https://reviews.apache.org/r/51091/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 Aug. 15, 2016, 1:08 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added non-recursive version of `cgroups::get`.


Diffs (updated)
-

  src/linux/cgroups.hpp 047d3ea596660e704447c0f0c7b914a43c4a2187 
  src/linux/cgroups.cpp f3232c009d04bb82701c0407b12abf999ab60a73 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49828: Added default methods implementations for `Subsystem` base class.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:08 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added default methods implementations for `Subsystem` base class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49850: Implemented `CpuacctSubsystem`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:08 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuacctSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49854: Implemented `DevicesSubsystem`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:08 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `DevicesSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:09 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
9b2d33ec3b023058d00c6671464cc9cd092f653b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49852: Implemented `NetClsSubsystem`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:09 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `NetClsSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b3fd8c85476bf46368bd79f052b7923ad9d32199 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49853: Implemented `PerfEventSubsystem`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:09 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `PerfEventSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50758: Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:10 p.m.)


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


Changes
---

Rebase.


Repository: mesos


Description
---

Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
f917db78157f799efc00b0c7af0230ddb6f26b0b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:09 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50758: Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.

[haosdent huang]

> On Aug. 14, 2016, 2:42 a.m., Qian Zhang wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 1609
> > 
> >
> > s/perf_events/perf_event/

A follow up patch here to disallow unknown isolators. 
https://reviews.apache.org/r/51091/


- haosdent


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


On Aug. 15, 2016, 1:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50758/
> ---
> 
> (Updated Aug. 15, 2016, 1:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f917db78157f799efc00b0c7af0230ddb6f26b0b 
> 
> Diff: https://reviews.apache.org/r/50758/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50733: Removed CgroupsCpushareIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:11 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsCpushareIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
74982a610b6c0a74734165a0c6aa8c9f72f54deb 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
221e814a448c4b5df9dab98de597451a24e2b89c 
  src/tests/containerizer/isolator_tests.cpp 
f917db78157f799efc00b0c7af0230ddb6f26b0b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50758: Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.

[haosdent huang]

> On Aug. 14, 2016, 2:42 a.m., Qian Zhang wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 1609
> > 
> >
> > s/perf_events/perf_event/
> 
> haosdent huang wrote:
> A follow up patch here to disallow unknown isolators. 
> https://reviews.apache.org/r/51091/

Thanks a lot for your nice catch again!


- haosdent


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


On Aug. 15, 2016, 1:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50758/
> ---
> 
> (Updated Aug. 15, 2016, 1:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f917db78157f799efc00b0c7af0230ddb6f26b0b 
> 
> Diff: https://reviews.apache.org/r/50758/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50749: Removed CgroupsDevicesIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:12 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsDevicesIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 
  src/slave/containerizer/mesos/isolators/cgroups/devices.hpp 
7ad96440b21a380c5d9af27b0168e9abf47769af 
  src/slave/containerizer/mesos/isolators/cgroups/devices.cpp 
f1b5e75d23780c0e1d53487852422b02c88de9e8 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50748: Removed CgroupsMemIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:12 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsMemIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 
  src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
b3ce6ed2505312bdd2d800164c2f57cd7625c9fa 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
0a4f38ded5cffd438e2a3b1e7d066c3077557f0d 
  src/tests/containerizer/isolator_tests.cpp 
f917db78157f799efc00b0c7af0230ddb6f26b0b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50751: Removed CgroupsPerfEventIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:12 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsPerfEventIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
4abde12af68a26b94b3706cdb38bf9890d811039 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
31f35385691681ef5da14be747edfb5f57c5d05a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
f917db78157f799efc00b0c7af0230ddb6f26b0b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50750: Removed CgroupsNetClsIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 15, 2016, 1:12 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsNetClsIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b3fd8c85476bf46368bd79f052b7923ad9d32199 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
  src/tests/containerizer/isolator_tests.cpp 
f917db78157f799efc00b0c7af0230ddb6f26b0b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49817: Implemented `CgroupsIsolatorProcess::recover`.

[haosdent huang]

> On Aug. 9, 2016, 9 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 195-197
> > 
> >
> > what you need here is essentially the top level cgroups, right?
> > 
> > We should improve cgroups::get to take an additional `recursive` flag.
> 
> Jie Yu wrote:
> Is this addressed? I don't see it being addressed.
> 
> haosdent huang wrote:
> Sorry, I misunderstand this before. Suppose the hierarchy is 
> `/sys/fs/cgroup/memory` and the `flags.cgroups_root` is `mesos`, we only need 
> the children under `/sys/fs/cgroup/memory/mesos/`, right? Why we need to get 
> the children recursively here?

Post a follow up patch for this at here https://reviews.apache.org/r/51031/.


- haosdent


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


On Aug. 11, 2016, 2:18 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49817/
> ---
> 
> (Updated Aug. 11, 2016, 2:18 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CgroupsIsolatorProcess::recover`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> b191b2a52a9645fc902a35ed52909b2142f0b4c0 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2659252d8cffcefc233bc85fb4707c8147272737 
> 
> Diff: https://reviews.apache.org/r/49817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50751: Removed CgroupsPerfEventIsolatorProcess.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [50751, 50750, 50748, 50749, 50733, 50758, 49855, 49853, 
49852, 49851, 49854, 49850, 49849, 49828, 51031, 51091]

Failed command: ./support/apply-review.sh -n -r 49849

Error:
2016-08-15 13:59:17 URL:https://reviews.apache.org/r/49849/diff/raw/ 
[13585/13585] -> "49849.patch" [1]
error: patch failed: 
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp:18
error: src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp: patch 
does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14809/console

- Mesos ReviewBot


On Aug. 15, 2016, 1:12 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50751/
> ---
> 
> (Updated Aug. 15, 2016, 1:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsPerfEventIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 5ed894d18258f6516866a2acd343c03281039c3c 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 4abde12af68a26b94b3706cdb38bf9890d811039 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 31f35385691681ef5da14be747edfb5f57c5d05a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f917db78157f799efc00b0c7af0230ddb6f26b0b 
> 
> Diff: https://reviews.apache.org/r/50751/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49849: Implemented `CpuSubsystem`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 2:09 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51091: Disallowed unknown cgroups isolator.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 87 - 91)


Do we really need this? I think we have already guaranteed this in 
`MesosContainerizer::create()`, so if there is anything wrong in `--isolation` 
(e.g., `cgroups/xyz`), it must have been detected in 
`MesosContainerizer::create()`, right?

If it is really needed, maybe we should do it as 
`CHECK(isolatorMap.contains(isolator))`?


- Qian Zhang


On Aug. 15, 2016, 9:07 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51091/
> ---
> 
> (Updated Aug. 15, 2016, 9:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallowed unknown cgroups isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 7e205a3eea4ad89faccf564bdb2ad0041fdee249 
> 
> Diff: https://reviews.apache.org/r/51091/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51091: Disallowed unknown cgroups isolator.

[haosdent huang]

> On Aug. 15, 2016, 2:41 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 87-91
> > 
> >
> > Do we really need this? I think we have already guaranteed this in 
> > `MesosContainerizer::create()`, so if there is anything wrong in 
> > `--isolation` (e.g., `cgroups/xyz`), it must have been detected in 
> > `MesosContainerizer::create()`, right?
> > 
> > If it is really needed, maybe we should do it as 
> > `CHECK(isolatorMap.contains(isolator))`?

We need this because we skip the check in `MesosContainerizer::create()`

```
if (strings::startsWith(isolation, "cgroups/")) {
  if (usedCgroups) {
// Skip when `CgroupsIsolatorProcess` have been created.
continue;
  } else {
usedCgroups = true;
  }
}
```

Or we put this check at there?


- haosdent


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


On Aug. 15, 2016, 1:07 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51091/
> ---
> 
> (Updated Aug. 15, 2016, 1:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallowed unknown cgroups isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 7e205a3eea4ad89faccf564bdb2ad0041fdee249 
> 
> Diff: https://reviews.apache.org/r/51091/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49556: Removed the binary way of HealthCheck in src.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:41 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

The binary way of health check is unused and not necessary anymore. We
should remove it.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/health-check/CMakeLists.txt 69e9566a0571220e21c82156b814f61e17802352 
  src/health-check/main.cpp 5346e30d35d4c5c2c4d181eee739ac8ce690a8e1 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50657: Removed the binary way of health check in libprocess.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:41 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

The binary way of health check is unused and not necessary anymore. We
should remove it.


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
497f6107567ef47c16a0c906238bc7dfdcf84701 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49555: Updated mesos-docker-executor to use health check via library way.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:41 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

We change the mesos-executor to use health check via library way in
https://reviews.apache.org/r/49389/. To keep consistent, we replace
health check in mesos-docker-executor from binary way to library way.


Diffs (updated)
-

  src/docker/executor.cpp 8d679cd33b6ddf3a5c11bb8c458a97b8809473ac 

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


Testing
---

Test with Marathon locally.

# 1. Success case.

I start `sleep 200` with the Docker container and use `ls /mnt/mesos/sandbox` 
as the health check command.

According the qurey result of `http://localhost:8080/v2/apps/test-sleep`, could 
see Marathon receive healthy status.

```
# truncate unnecessary messages
{
  "app": {
"id": "/test-health",
"cmd": "sleep 200",
"container": {
  "type": "DOCKER",
  "volumes": [],
  "docker": {
"image": "ubuntu",
"network": "HOST"
  }
},
"healthChecks": [
  {
"protocol": "COMMAND",
"command": {
  "value": "ls /mnt/mesos/sandbox"
},
"gracePeriodSeconds": 300,
"intervalSeconds": 60,
"timeoutSeconds": 20,
"maxConsecutiveFailures": 3
  }
],
"tasksStaged": 0,
"tasksRunning": 1,
"tasksHealthy": 1,
"tasksUnhealthy": 0,
"tasks": [
  {
"id": "test-health.8d4ba884-585c-11e6-9fb5-0242e663b1f5",
"slaveId": "ec5359a2-9c63-4c2e-86da-3ed8ef94800b-S0",
"host": "127.0.0.1",
"startedAt": "2016-08-02T02:55:23.433Z",
"stagedAt": "2016-08-02T02:55:21.217Z",
"version": "2016-08-02T02:48:31.054Z",
"ipAddresses": [
  {
"ipAddress": "127.0.0.1",
"protocol": "IPv4"
  }
],
"appId": "/test-health",
"healthCheckResults": [
  {
"alive": true,
"consecutiveFailures": 0,
"firstSuccess": "2016-08-02T02:55:23.652Z",
"lastFailure": null,
"lastSuccess": "2016-08-02T02:55:23.652Z",
"lastFailureCause": null,
"taskId": "test-health.8d4ba884-585c-11e6-9fb5-0242e663b1f5"
  }
]
  }
]
  }
}
```

# 2. Failed case.

Then I start a similar application which use `false` as health check command, 
could see Marathon receive unhealthy status.

```
# truncate unnecessary messages
{
  "app": {
"id": "/test-unhealth",
"cmd": "sleep 200",
"container": {
  "type": "DOCKER",
  "volumes": [],
  "docker": {
"image": "ubuntu",
"network": "HOST"
  }
},
"healthChecks": [
  {
"protocol": "COMMAND",
"command": {
  "value": "false"
},
"gracePeriodSeconds": 5,
"intervalSeconds": 3,
"timeoutSeconds": 20,
"maxConsecutiveFailures": 3
  }
],
"tasksStaged": 0,
"tasksRunning": 1,
"tasksHealthy": 0,
"tasksUnhealthy": 1,
"tasks": [
  {
"id": "test-unhealth.48a6318a-585e-11e6-9fb5-0242e663b1f5",
"slaveId": "ec5359a2-9c63-4c2e-86da-3ed8ef94800b-S0",
"host": "127.0.0.1",
"startedAt": "2016-08-02T03:07:47.310Z",
"stagedAt": "2016-08-02T03:07:45.041Z",
"ipAddresses": [
  {
"ipAddress": "127.0.0.1",
"protocol": "IPv4"
  }
],
"appId": "/test-unhealth",
"healthCheckResults": [
  {
"alive": false,
"consecutiveFailures": 1,
"firstSuccess": null,
"lastFailure": "2016-08-02T03:07:53.800Z",
"lastSuccess": null,
"lastFailureCause": "",
"taskId": "test-unhealth.48a6318a-585e-11e6-9fb5-0242e663b1f5"
  }
]
  }
]
  }
}
```


Thanks,

haosdent huang

Re: Review Request 51083: Passed the pid of the container into `HealthChecker`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:42 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

To ensure our health check share same namespaces with the container,
we need to pass the pid of the container into `HealthChecker` for
further inspecting and setting.


Diffs (updated)
-

  src/docker/executor.cpp 8d679cd33b6ddf3a5c11bb8c458a97b8809473ac 
  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 
  src/launcher/executor.cpp 2d04edbd58f881f59fca00b95803474724a49154 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51084: Allowed a process with multiple threads call `net::setns`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:42 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

In some cases, we need to use different namespaces in different
threads. For example, we need to make sure `HealthChecker` could
communicate with the executor by the host network namespace while our
health check running in the container network namespace.


Diffs (updated)
-

  src/linux/ns.hpp 2d6c359ed24d6e964882f34df60d8182491a27c9 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:42 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51069: Refactored `_commandHealthCheck` in health check.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:41 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


Repository: mesos


Description (updated)
---

\* Removed blocking \`Future::await\` call.
\* Read stdout and stderr of the health check command.
\* Adjust the level of some logs.
\* Adjust some minor styles.
\* Change the interfaces of different health check handlers to
  \`Future\` to make errors handling more easier.


Diffs (updated)
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49360: Supported TCP in health check.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:43 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Supported TCP in health check.


Diffs (updated)
-

  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51085: Added `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaHTTP`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:43 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Test if a docker task in bridge network mode could pass HTTP health
check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp e69c4f5cc4756f4926e67ce5e9bbd7ab17e41913 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51086: Added `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaTCP`.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:43 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Test if a docker task in bridge network mode could pass TCP health
check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp e69c4f5cc4756f4926e67ce5e9bbd7ab17e41913 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51069: Refactored `_commandHealthCheck` in health check.

[haosdent huang]

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

(Updated Aug. 15, 2016, 3:44 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón Kleiman, 
Gilbert Song, Jie Yu, and Timothy Chen.


Changes
---

Addrees @alexr's comments.


Repository: mesos


Description (updated)
---

* Removed blocking `Future::await` call.
* Read stdout and stderr of the health check command.
* Adjust the level of some logs.
* Adjust some minor styles.
* Change the interfaces of different health check handlers to
  `Future` to make errors handling more easier.


Diffs
-

  src/health-check/health_checker.hpp b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
  src/health-check/health_checker.cpp 45a5fe00a95a6e88b1990c1396e03082feb202bc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50271: Created an isolator for Linux capabilities.

[Benjamin Bannier]

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

(Updated Aug. 15, 2016, 5:57 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Cleanup.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
f917db78157f799efc00b0c7af0230ddb6f26b0b 

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


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50270: Introduced linux capabilities support for mesos containerizer.

[Benjamin Bannier]

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

(Updated Aug. 15, 2016, 5:57 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Cleanup.


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


Repository: mesos


Description
---

This change introduces linux capability based security for unified
containerizer. A new agent flag \`allowed_capabilities\` has been
introduced to override the default capabilities of the user or the
capabilities requested by the user.

This feature is only available on linux.

This patch is based on https://reviews.apache.org/r/46798/.


Diffs (updated)
-

  src/launcher/executor.cpp 2d04edbd58f881f59fca00b95803474724a49154 
  src/slave/flags.hpp ef2394cae5cb72ae627aaef443ac06a50bcfb16f 
  src/slave/flags.cpp c07d6e5e062b75c9dd867e3dced9e2cc8b69872d 

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


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50912: Added the infrastructure for a new python-based CLI.

[Kevin Klues]

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

(Updated Aug. 15, 2016, 4:06 p.m.)


Review request for mesos, Haris Choudhary and Joseph Wu.


Changes
---

Updated the mode of main.py


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


Repository: mesos


Description
---

Added the infrastructure for a new python-based CLI.


Diffs (updated)
-

  configure.ac f82c6314d65a8ea3eb118bab8e110063cae46121 
  docs/getting-started.md ebe52705b8c8757d4a507ce3ae75f56d535a39d1 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  src/cli_new/README.md PRE-CREATION 
  src/cli_new/activate PRE-CREATION 
  src/cli_new/bin/config.py PRE-CREATION 
  src/cli_new/bin/main.py PRE-CREATION 
  src/cli_new/bin/mesos PRE-CREATION 
  src/cli_new/bootstrap PRE-CREATION 
  src/cli_new/deactivate PRE-CREATION 
  src/cli_new/lib/mesos/__init__.py PRE-CREATION 
  src/cli_new/lib/mesos/docopt.py PRE-CREATION 
  src/cli_new/lib/mesos/exceptions.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/__init__.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/base.py PRE-CREATION 
  src/cli_new/lib/mesos/util.py PRE-CREATION 
  src/cli_new/mesos.bash_completion PRE-CREATION 
  src/cli_new/pip-requirements.txt PRE-CREATION 
  src/cli_new/pylint.config PRE-CREATION 

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


Testing
---

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


Thanks,

Kevin Klues

Re: Review Request 51075: Consolidated two executorEnvironment() calls in docker containerizer.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 13, 2016, 5:44 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51075/
> ---
> 
> (Updated Aug. 13, 2016, 5:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-6020
> https://issues.apache.org/jira/browse/MESOS-6020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Consolidated two executorEnvironment() calls in docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
> 
> Diff: https://reviews.apache.org/r/51075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

[haosdent huang]

> On Aug. 11, 2016, 3:19 a.m., Alexander Rukletsov wrote:
> > src/health-check/health_checker.hpp, line 330
> > 
> >
> > Why do you need to discard original future here?
> 
> haosdent huang wrote:
> I saw we discard original future in every `after` callback. So I discard 
> it here as well.
> 
> Alexander Rukletsov wrote:
> This is may be true. Do you know why we do this?

I think it is because the `.then` callback may continue to executed when the 
`future` is ready if we don't call `future.discard()`.


- haosdent


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


On Aug. 15, 2016, 3:42 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated Aug. 15, 2016, 3:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> b4548f385e6bdf12f6bbc402a5d59ba8e165b8a5 
>   src/health-check/health_checker.cpp 
> 45a5fe00a95a6e88b1990c1396e03082feb202bc 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51076: Removed slavePid from the containerizer launch API.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/composing.cpp (line 68)


s/env/environment/

Here and everywhere else.



src/slave/slave.hpp (line 990)


move this to the top


- Jie Yu


On Aug. 13, 2016, 5:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51076/
> ---
> 
> (Updated Aug. 13, 2016, 5:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-6020
> https://issues.apache.org/jira/browse/MESOS-6020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed slavePid from the containerizer launch API.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> f3eebd19bc9e6b3b8a969a2ad967b3e2909e0ee4 
>   src/slave/containerizer/composing.cpp 
> 36d891792380abb01180ccc63cf2cd05b16b9620 
>   src/slave/containerizer/containerizer.hpp 
> b108d68aa05d80febcb4f6cc2ba3cc3183acda4f 
>   src/slave/containerizer/containerizer.cpp 
> 2d59caf6555cb5866ef172d543c868e7f21c2f96 
>   src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6585356543efe8794ff711e0ee251a2e13a6adc2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/slave.hpp ffe4220c9289419ab1b1a2c1f499b6eac3c01e4b 
>   src/slave/slave.cpp 29ca07a40d4e1d75979259cbe4f113baab37035b 
>   src/tests/containerizer.hpp 76feedfdd381d92a1a19caca44e00630d7ecd281 
>   src/tests/containerizer.cpp 6f3950d0f4476d0872903b56dc2c7f95ae7e7ac5 
>   src/tests/containerizer/composing_containerizer_tests.cpp 
> e7619c15709e204d30fcc5b489873b5a8b717fab 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 4d852ad6b14934ae7ec691cc124c77bda9bb2c8d 
>   src/tests/containerizer/isolator_tests.cpp 
> 4f047ae6b2e85e177e8b73d60b9dfca913c832a5 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> e3c8daab4fc688150a4f222e05f9b1bd9aee1912 
>   src/tests/hook_tests.cpp a1c8685d7ae984a728f5225cae89e3c235b46266 
>   src/tests/mesos.hpp 59688dc9d1b16f20facf272085e65b3835f9a56e 
>   src/tests/slave_tests.cpp 787ec5b26fbf588a1148d2c54e03b93e7836de05 
> 
> Diff: https://reviews.apache.org/r/51076/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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 Aug. 15, 2016, 4:30 p.m.)


Review request for mesos, Haris Choudhary and Joseph Wu.


Changes
---

Moved the main test executable into bin.


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/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  src/cli_new/bin/mesos-tests PRE-CREATION 
  src/cli_new/bin/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt PRE-CREATION 

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


Testing
---

GTEST_FILTER="" make -j check


Thanks,

Kevin Klues

Re: Review Request 51061: Added LAUNCH_GROUP offer operation.

[Jie Yu]

> On Aug. 14, 2016, 2:38 p.m., Qian Zhang wrote:
> > include/mesos/mesos.proto, lines 1381-1383
> > 
> >
> > Do we allow a custom executor to launch multiple task groups and each 
> > task group is in its own network? If so, I think we may need to add 
> > `NetworkInfo` in this message.
> 
> James DeFelice wrote:
> That's an interesting idea. We haven't really discussed separate nework 
> namespaces for different task groups all owned by a single, custom executor. 
> What's the use case here? You want multiple task groups that may share access 
> to the same storage but each group has a distinct netns?
> 
> Qian Zhang wrote:
> This idea is actually from K8s/Mesos integration. In K8s/Mesos 
> integration, each node will have a single kubelet (it is actually also a 
> Mesos custom executor) running on it, and the kubelet can launch mulitple 
> pods in the host, and in each pod, there can be multiple containers which 
> share the same network. I think this is a good and natural design which we 
> can follow since we are trying to handle the similar use case: launch a set 
> of containers co-located and co-managed on an agent that share some resources 
> (e.g., network namespace, volumes).

Two thoughts:
1) to achieve similar k8s pod model, one can use multiple executor containers, 
each of which has a separate network namespace. Thus, each executor container 
will be a Pod.
2) for k8s/mesos integration. Long term, we might want to support nested 
network namespace for a group of sub-containers so that kublet can be the top 
level executor and each pod will be a nested sub-container having its own 
network namespace. The sub-container has to have another init inside which will 
be responsible for telling agent to create sub-sub-containers.

The LAUNCH executor call already allows users to specify `ContainerInfo`. 
Specifying a `NetworkInfo` inside will instruct the agent to create a nested 
network namespace. From API level, i think (2) is definitely possible in the 
future. But we won't target that in MVP.


- Jie


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


On Aug. 12, 2016, 11:12 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51061/
> ---
> 
> (Updated Aug. 12, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
> James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A scheduler can use the LAUNCH_GROUP offer operation to send a
> `TaskGroup` to an executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 
> 
> Diff: https://reviews.apache.org/r/51061/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51060: Made `Offer::Operation::Type` optional with default `UNKNOWN`.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 12, 2016, 11:10 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51060/
> ---
> 
> (Updated Aug. 12, 2016, 11:10 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Jie Yu.
> 
> 
> Bugs: MESOS-6036 and MESOS-6037
> https://issues.apache.org/jira/browse/MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is necessary to introduce new offer operations in a backwards
> compatible manner. See MESOS-4997 for context.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 
> 
> Diff: https://reviews.apache.org/r/51060/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51059: Made `ExecutorInfo.command` optional.

[Vinod Kone]

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




include/mesos/mesos.proto (line 505)


s/mesos/Mesos/

here and below.

also update any documentation in docs/ regarding how to use default 
executor.


- Vinod Kone


On Aug. 12, 2016, 11:08 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51059/
> ---
> 
> (Updated Aug. 12, 2016, 11:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is done to allow schedulers to use `ExecutorInfo` with just
> `executor_id` set. This is a requirement for the upcoming support
> for `TaskGroup` semantics.
> 
> Note that in this patch, TASK_ERROR is returned when
> `ExecutorInfo.command` is unset to keep the existing semantics for
> `LAUNCH` offer operation. This might change when we add `LAUNCH_GROUP`
> operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/master/validation.cpp af18e5aef3be59830b0a7b0235bbc739dba1029c 
>   src/tests/master_validation_tests.cpp 
> 9eb82a569f7c48caa96d4d54a93b199ccac74385 
> 
> Diff: https://reviews.apache.org/r/51059/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51061: Added LAUNCH_GROUP offer operation.

[Vinod Kone]

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




include/mesos/mesos.proto (line 1240)


s/mesos/Mesos/

here and below.



include/mesos/mesos.proto (line 1241)


s/mesos/Mesos/



include/mesos/mesos.proto (line 1381)


s/TaskGroup/TaskGroupInfo/


- Vinod Kone


On Aug. 12, 2016, 11:12 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51061/
> ---
> 
> (Updated Aug. 12, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
> James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A scheduler can use the LAUNCH_GROUP offer operation to send a
> `TaskGroup` to an executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 
> 
> Diff: https://reviews.apache.org/r/51061/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51059: Made `ExecutorInfo.command` optional.

[Avinash sridharan]

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




src/master/validation.cpp (line 586)


Maybe add a JIRA, if we have one, for `TaskGroup` validation over here.


- Avinash sridharan


On Aug. 12, 2016, 11:08 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51059/
> ---
> 
> (Updated Aug. 12, 2016, 11:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is done to allow schedulers to use `ExecutorInfo` with just
> `executor_id` set. This is a requirement for the upcoming support
> for `TaskGroup` semantics.
> 
> Note that in this patch, TASK_ERROR is returned when
> `ExecutorInfo.command` is unset to keep the existing semantics for
> `LAUNCH` offer operation. This might change when we add `LAUNCH_GROUP`
> operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/master/validation.cpp af18e5aef3be59830b0a7b0235bbc739dba1029c 
>   src/tests/master_validation_tests.cpp 
> 9eb82a569f7c48caa96d4d54a93b199ccac74385 
> 
> Diff: https://reviews.apache.org/r/51059/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 50992: Make MESOS_{MAJOR|MINOR|PATCH}_VERSION macros integer in header.

[Jiang Yan Xu]

> On Aug. 11, 2016, 10:12 a.m., James Peach wrote:
> > As mentioned on chat, I think this will break some external module builds. 
> > I'm fine with that, just pointing it out for posterity.
> > 
> > I noticed that the Java bindings use these in the 
> > MesosNativeLibrary.Version class. The Java side of the bindings says this 
> > is a ``jlong``, but prior to this change it would have been a string 
> > AFAICT. So I guess this fixes that too?
> 
> Zhitao Li wrote:
> I added as many people who might know what existing module already exists 
> as I can think of, but feel free to suggest more.
> 
> If we want to provide a upgrade cycle, we can keep current macros 
> unchanged and define new ones like
> 
> ```
> // TODO: Remove after migration cycle.
> #define MESOS_MAJOR_VERSION "@MESOS_MAJOR_VERSION@"
> #define MESOS_MAJOR_VERSION_INT @MESOS_MAJOR_VERSION@
> ```
> 
> Just decided to roll forward faster until someone told me this could 
> actually break something.
> 
> James Peach wrote:
> Yup, good plan. +1
> 
> haosdent huang wrote:
> +1 for this changes.
> 
> And I think we need to sending a seprate email about
> 
> ```
> I think this will break some external module builds
> ```
> 
> to modu...@mesos.apache.org and d...@mesos.apache.org

+1 on introducing `_INT` in the interim and checking with the mailing lists. cc 
Kapil Arya as modules maintainer.


- Jiang Yan


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


On Aug. 11, 2016, 10 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50992/
> ---
> 
> (Updated Aug. 11, 2016, 10 a.m.)
> 
> 
> Review request for mesos, Xiaojian Huang, haosdent huang, James Peach, Joris 
> Van Remoortere, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4638
> https://issues.apache.org/jira/browse/MESOS-4638
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This makes version based conditional compiling much easier for
> module writers.
> 
> 
> Diffs
> -
> 
>   include/mesos/version.hpp.in c8fbc822d91abb61df15715c0c6af98b0bfe2cb9 
> 
> Diff: https://reviews.apache.org/r/50992/diff/
> 
> 
> Testing
> ---
> 
> make on Mac OS and Linux.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 51095: Adding protobuf to represent errors returned by CNI plugins.

[Avinash sridharan]

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

Review request for mesos, Jie Yu and Qian Zhang.


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


Repository: mesos


Description
---

Adding protobuf to represent errors returned by CNI plugins.

This will be used by CNI plugins such as `PortMapper` to report errors to the 
`network/cni` isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/spec.proto 
8419bc11d14aa29110377248a92ebb1aa4baf486 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 51086: Added `HealthCheckTest.ROOT_DOCKER_DockerHealthyTaskViaTCP`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49555, 50657, 49556, 51069, 51083, 51084, 36816, 49360, 
51085, 51086]

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 Aug. 15, 2016, 3:43 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51086/
> ---
> 
> (Updated Aug. 15, 2016, 3:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gastón 
> Kleiman, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-3567
> https://issues.apache.org/jira/browse/MESOS-3567
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test if a docker task in bridge network mode could pass TCP health
> check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp e69c4f5cc4756f4926e67ce5e9bbd7ab17e41913 
> 
> Diff: https://reviews.apache.org/r/51086/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50992: Introduce MESOS_{MAJOR|MINOR|PATCH}_VERSION_INT macros.

[Zhitao Li]

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

(Updated Aug. 15, 2016, 5:31 p.m.)


Review request for mesos, Xiaojian Huang, haosdent huang, James Peach, Joris 
Van Remoortere, Vinod Kone, and Jiang Yan Xu.


Changes
---

Introduce int typed version and keep string typed macros around.


Summary (updated)
-

Introduce MESOS_{MAJOR|MINOR|PATCH}_VERSION_INT macros.


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


Repository: mesos


Description
---

This makes version based conditional compiling much easier for
module writers.


Diffs (updated)
-

  include/mesos/version.hpp.in c8fbc822d91abb61df15715c0c6af98b0bfe2cb9 

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


Testing
---

make on Mac OS and Linux.


Thanks,

Zhitao Li

Re: Review Request 51076: Removed slavePid from the containerizer launch API.

[Gilbert Song]

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

(Updated Aug. 15, 2016, 10:48 a.m.)


Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and Kevin 
Klues.


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


Repository: mesos


Description
---

Removed slavePid from the containerizer launch API.


Diffs (updated)
-

  src/slave/containerizer/composing.hpp 
f3eebd19bc9e6b3b8a969a2ad967b3e2909e0ee4 
  src/slave/containerizer/composing.cpp 
36d891792380abb01180ccc63cf2cd05b16b9620 
  src/slave/containerizer/containerizer.hpp 
b108d68aa05d80febcb4f6cc2ba3cc3183acda4f 
  src/slave/containerizer/containerizer.cpp 
2d59caf6555cb5866ef172d543c868e7f21c2f96 
  src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
  src/slave/containerizer/mesos/containerizer.hpp 
6585356543efe8794ff711e0ee251a2e13a6adc2 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/slave.hpp ffe4220c9289419ab1b1a2c1f499b6eac3c01e4b 
  src/slave/slave.cpp 29ca07a40d4e1d75979259cbe4f113baab37035b 
  src/tests/containerizer.hpp 76feedfdd381d92a1a19caca44e00630d7ecd281 
  src/tests/containerizer.cpp 6f3950d0f4476d0872903b56dc2c7f95ae7e7ac5 
  src/tests/containerizer/composing_containerizer_tests.cpp 
e7619c15709e204d30fcc5b489873b5a8b717fab 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
4d852ad6b14934ae7ec691cc124c77bda9bb2c8d 
  src/tests/containerizer/isolator_tests.cpp 
4f047ae6b2e85e177e8b73d60b9dfca913c832a5 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
e3c8daab4fc688150a4f222e05f9b1bd9aee1912 
  src/tests/hook_tests.cpp a1c8685d7ae984a728f5225cae89e3c235b46266 
  src/tests/mesos.hpp 59688dc9d1b16f20facf272085e65b3835f9a56e 
  src/tests/slave_tests.cpp 787ec5b26fbf588a1148d2c54e03b93e7836de05 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 51100: Added a containerizer launch interface to launch sub-containers.

[Jie Yu]

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

Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
Vinod Kone.


Repository: mesos


Description
---

Added a containerizer launch interface to launch sub-containers.


Diffs
-

  src/slave/containerizer/containerizer.hpp 
d00537590d00aba76cda3d74ed4785c9e1592f9d 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 51099: Fixed scalar resources output to print full significant digits.

[Jiang Yan Xu]

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

Review request for mesos, Joris Van Remoortere and Neil Conway.


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


Repository: mesos


Description
---

Also clarified in the comments that when we say "We discard any additional 
precision" we mean the fractional part.


Diffs
-

  src/common/values.cpp cc87d9436f1a75e71cbeb8b709ec323e24f4f519 
  src/tests/resources_tests.cpp e61bdeffd5114ab481903c1178d9ecb3452ec85b 

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


Testing
---

make check. 

Added a test block which would fail without the change.


Thanks,

Jiang Yan Xu

Re: Review Request 51087: Updated elfio to version 3.2.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Aug. 15, 2016, 5:02 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51087/
> ---
> 
> (Updated Aug. 15, 2016, 5:02 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6039
> https://issues.apache.org/jira/browse/MESOS-6039
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated elfio to version 3.2.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 662146f 
>   3rdparty/cmake/Versions.cmake e5cf7d0 
>   3rdparty/elfio-3.1.patch 3d85394 
>   3rdparty/elfio-3.1.tar.gz c0e60f33a413b7832034fe81275c8edf8819fff9 
>   3rdparty/elfio-3.2.tar.gz PRE-CREATION 
>   3rdparty/versions.am 08e47b6 
> 
> Diff: https://reviews.apache.org/r/51087/diff/
> 
> 
> Testing
> ---
> 
> ```
> cd build
> ../configure
> 
> GTEST_FILTER="" make -j check
> sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> ALL TESTS PASSED
> 
> cmake ..
> make -j
> 
> BUILDS SUCCESSFULLY
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 51059: Made `ExecutorInfo.command` optional.

[Benjamin Mahler]

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


Ship it!





include/mesos/mesos.proto (lines 502 - 510)


This seems to suggest that we'll allow LAUNCH to also specify a 
command-less executor to get the default? That sounds good, but not sure if 
that was your intention. If it was, perhaps a TODO to have LAUNCH also allow a 
"default" command-less executor? (Ditto in v1)


- Benjamin Mahler


On Aug. 12, 2016, 11:08 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51059/
> ---
> 
> (Updated Aug. 12, 2016, 11:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is done to allow schedulers to use `ExecutorInfo` with just
> `executor_id` set. This is a requirement for the upcoming support
> for `TaskGroup` semantics.
> 
> Note that in this patch, TASK_ERROR is returned when
> `ExecutorInfo.command` is unset to keep the existing semantics for
> `LAUNCH` offer operation. This might change when we add `LAUNCH_GROUP`
> operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/master/validation.cpp af18e5aef3be59830b0a7b0235bbc739dba1029c 
>   src/tests/master_validation_tests.cpp 
> 9eb82a569f7c48caa96d4d54a93b199ccac74385 
> 
> Diff: https://reviews.apache.org/r/51059/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51060: Made `Offer::Operation::Type` optional with default `UNKNOWN`.

[Benjamin Mahler]

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


Ship it!





src/master/master.cpp (line 3432)


Hm.. seems like an ERROR if we receive an operation we cannot support?



src/master/master.cpp (line 3871)


Hm.. seems like an ERROR if we receive an operation we cannot support?


- Benjamin Mahler


On Aug. 12, 2016, 11:10 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51060/
> ---
> 
> (Updated Aug. 12, 2016, 11:10 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Jie Yu.
> 
> 
> Bugs: MESOS-6036 and MESOS-6037
> https://issues.apache.org/jira/browse/MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is necessary to introduce new offer operations in a backwards
> compatible manner. See MESOS-4997 for context.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 
> 
> Diff: https://reviews.apache.org/r/51060/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51061: Added LAUNCH_GROUP offer operation.

[Vinod Kone]

> On Aug. 13, 2016, 10:18 p.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, line 1379
> > 
> >
> > As we want to allow containers share a network namespace and volumes, 
> > so I think that we shoud also highlight `Volume` here.
> 
> Guangya Liu wrote:
> Another point is I think here you mean 
> ```
> 2) `NetworkInfo` must not be set inside task's `ContainerInfo`. 
> ```
> As the `TaskInfo` has a `resources` field, so using `ContainerInfo` or 
> `container` for above may be more clear?
> 
> For the above, what about add a new item for `Volume` as following:
> ```
> 
> 3) `Volume` can be set in both executor level and task level so as to 
> share a volume between executor and task.
> ```

I'll add Volume information on LaunchGroup message because talking about 
executor here is a bit weird.


- Vinod


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


On Aug. 12, 2016, 11:12 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51061/
> ---
> 
> (Updated Aug. 12, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
> James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A scheduler can use the LAUNCH_GROUP offer operation to send a
> `TaskGroup` to an executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 
> 
> Diff: https://reviews.apache.org/r/51061/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51061: Added LAUNCH_GROUP offer operation.

[Vinod Kone]

> On Aug. 13, 2016, 10:29 p.m., James DeFelice wrote:
> > include/mesos/mesos.proto, line 1379
> > 
> >
> > There was some talk about allowing tasks to specify volumes if they 
> > wanted to use **unshared** volumes.

This is already possible. So not sure if it needs to be explicitly called out?


- Vinod


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


On Aug. 12, 2016, 11:12 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51061/
> ---
> 
> (Updated Aug. 12, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
> James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A scheduler can use the LAUNCH_GROUP offer operation to send a
> `TaskGroup` to an executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 
> 
> Diff: https://reviews.apache.org/r/51061/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 51100: Added a containerizer launch interface to launch sub-containers.

[Jie Yu]

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

(Updated Aug. 15, 2016, 6:43 p.m.)


Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
Vinod Kone.


Changes
---

Updated to use the nested ContainerID.


Repository: mesos


Description
---

Added a containerizer launch interface to launch sub-containers.


Diffs (updated)
-

  src/slave/containerizer/containerizer.hpp 
14f298ead7e91beab1a45cf7b0f9bd2f60f5241a 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 51102: Supported nested ContainerID.

[Jie Yu]

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

Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.


Repository: mesos


Description
---

Introduced an optional 'parent' field in ContainerID so it can encode
nesting relationship.


Diffs
-

  include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 51099: Fixed scalar resources output to print full significant digits.

[Neil Conway]

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




src/common/values.cpp (line 72)


You should also update `v1/values.cpp`.



src/common/values.cpp (line 74)


`stream.precision(int)` returns the previous value, so you only need to 
call it twice in total, not three times.



src/tests/resources_tests.cpp (line 882)


Wouldn't hurt to have another test case that is even larger, IMO.


- Neil Conway


On Aug. 15, 2016, 5:54 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51099/
> ---
> 
> (Updated Aug. 15, 2016, 5:54 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Neil Conway.
> 
> 
> Bugs: MESOS-5875
> https://issues.apache.org/jira/browse/MESOS-5875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also clarified in the comments that when we say "We discard any additional 
> precision" we mean the fractional part.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp cc87d9436f1a75e71cbeb8b709ec323e24f4f519 
>   src/tests/resources_tests.cpp e61bdeffd5114ab481903c1178d9ecb3452ec85b 
> 
> Diff: https://reviews.apache.org/r/51099/diff/
> 
> 
> Testing
> ---
> 
> make check. 
> 
> Added a test block which would fail without the change.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Review Request 51103: Added default stub for launch in Mesos containerizer.

[Jie Yu]

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

Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
Vinod Kone.


Repository: mesos


Description
---

Added default stub for launch in Mesos containerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
c3e3066f92cc20dcf4bfeb328dfa70a252bb19a2 
  src/slave/containerizer/mesos/containerizer.cpp 
8b525cec11ceb8340132930feae197c9e105dd3e 

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


Testing
---


Thanks,

Jie Yu

Review Request 51096: Added the `mesos-port-mapper` binary.

[Avinash sridharan]

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

Review request for mesos, Jie Yu and Qian Zhang.


Repository: mesos


Description
---

The `mesos-port-mapper` is a CNI plugin that can be used to install
port-forwarding rules for a container. The `mesos-port-mapper` is a
wrapper CNI plugin that should always be used in conjunction with
other CNI plugins.


Diffs
-

  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
 PRE-CREATION 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 51102: Supported nested ContainerID.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On Aug. 15, 2016, 6:42 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51102/
> ---
> 
> (Updated Aug. 15, 2016, 6:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced an optional 'parent' field in ContainerID so it can encode
> nesting relationship.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
> 
> Diff: https://reviews.apache.org/r/51102/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51096: Added the `mesos-port-mapper` binary.

[Avinash sridharan]

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

(Updated Aug. 15, 2016, 6:48 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Added corresponding JIRA ticket.


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


Repository: mesos


Description
---

The `mesos-port-mapper` is a CNI plugin that can be used to install
port-forwarding rules for a container. The `mesos-port-mapper` is a
wrapper CNI plugin that should always be used in conjunction with
other CNI plugins.


Diffs
-

  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
 PRE-CREATION 

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


Testing
---

make


Thanks,

Avinash sridharan

Re: Review Request 51100: Added a containerizer launch interface to launch sub-containers.

[Benjamin Hindman]

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


Fix it, then Ship it!





src/slave/containerizer/containerizer.hpp (line 93)


s/sub/nested/

We should be consistent throughout the code base calling these nested 
containers. Sound good?


- Benjamin Hindman


On Aug. 15, 2016, 6:43 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51100/
> ---
> 
> (Updated Aug. 15, 2016, 6:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a containerizer launch interface to launch sub-containers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 14f298ead7e91beab1a45cf7b0f9bd2f60f5241a 
> 
> Diff: https://reviews.apache.org/r/51100/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51103: Added default stub for launch in Mesos containerizer.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On Aug. 15, 2016, 6:43 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51103/
> ---
> 
> (Updated Aug. 15, 2016, 6:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added default stub for launch in Mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> c3e3066f92cc20dcf4bfeb328dfa70a252bb19a2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8b525cec11ceb8340132930feae197c9e105dd3e 
> 
> Diff: https://reviews.apache.org/r/51103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51100: Added a containerizer launch interface to launch sub-containers.

[Jie Yu]

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

(Updated Aug. 15, 2016, 6:53 p.m.)


Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
Vinod Kone.


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


Repository: mesos


Description
---

Added a containerizer launch interface to launch sub-containers.


Diffs
-

  src/slave/containerizer/containerizer.hpp 
14f298ead7e91beab1a45cf7b0f9bd2f60f5241a 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 51103: Added default stub for launch in Mesos containerizer.

[Jie Yu]

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

(Updated Aug. 15, 2016, 6:54 p.m.)


Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
Vinod Kone.


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


Repository: mesos


Description
---

Added default stub for launch in Mesos containerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.hpp 
c3e3066f92cc20dcf4bfeb328dfa70a252bb19a2 
  src/slave/containerizer/mesos/containerizer.cpp 
8b525cec11ceb8340132930feae197c9e105dd3e 

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


Testing
---


Thanks,

Jie Yu

Re: Review Request 51102: Supported nested ContainerID.

[Jie Yu]

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

(Updated Aug. 15, 2016, 6:54 p.m.)


Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.


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


Repository: mesos


Description
---

Introduced an optional 'parent' field in ContainerID so it can encode
nesting relationship.


Diffs
-

  include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 51102: Supported nested ContainerID.

[Gilbert Song]

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


Fix it, then Ship it!





include/mesos/mesos.proto (line 92)


Please add it to v1/mesos.proto as well.


- Gilbert Song


On Aug. 15, 2016, 11:42 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51102/
> ---
> 
> (Updated Aug. 15, 2016, 11:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced an optional 'parent' field in ContainerID so it can encode
> nesting relationship.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
> 
> Diff: https://reviews.apache.org/r/51102/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51100: Added a containerizer launch interface to launch sub-containers.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 15, 2016, 11:53 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51100/
> ---
> 
> (Updated Aug. 15, 2016, 11:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6043
> https://issues.apache.org/jira/browse/MESOS-6043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a containerizer launch interface to launch sub-containers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.hpp 
> 14f298ead7e91beab1a45cf7b0f9bd2f60f5241a 
> 
> Diff: https://reviews.apache.org/r/51100/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51103: Added default stub for launch in Mesos containerizer.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 15, 2016, 11:54 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51103/
> ---
> 
> (Updated Aug. 15, 2016, 11:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6043
> https://issues.apache.org/jira/browse/MESOS-6043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added default stub for launch in Mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> c3e3066f92cc20dcf4bfeb328dfa70a252bb19a2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8b525cec11ceb8340132930feae197c9e105dd3e 
> 
> Diff: https://reviews.apache.org/r/51103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51102: Supported nested ContainerID.

[Jie Yu]

> On Aug. 15, 2016, 6:54 p.m., Gilbert Song wrote:
> > include/mesos/mesos.proto, line 92
> > 
> >
> > Please add it to v1/mesos.proto as well.

Yup, doing that right now.


- Jie


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


On Aug. 15, 2016, 6:54 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51102/
> ---
> 
> (Updated Aug. 15, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.
> 
> 
> Bugs: MESOS-6043
> https://issues.apache.org/jira/browse/MESOS-6043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced an optional 'parent' field in ContainerID so it can encode
> nesting relationship.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
> 
> Diff: https://reviews.apache.org/r/51102/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51061: Added LAUNCH_GROUP offer operation.

[Benjamin Mahler]

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


Ship it!





include/mesos/mesos.proto (lines 1237 - 1245)


Per our conversation, maybe a TODO to consider deprecating Launch in favor 
of LaunchGroup?



include/mesos/mesos.proto (lines 1239 - 1241)


Maybe also mention here that if the scheduler omits, the scheduler should 
make sure that the offer contains enough to also include the default executor 
resources?



include/mesos/mesos.proto (lines 1239 - 1240)


Sounds like you can omit the sentence about not setting 
ExecutorInfo.command if you add the 'type' field that we talked about in 
ExecutorInfo?



include/mesos/mesos.proto (lines 1374 - 1375)


Maybe:

The executor will receive the group in a single message to allow the group 
to be launched "atomically".


- Benjamin Mahler


On Aug. 12, 2016, 11:12 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51061/
> ---
> 
> (Updated Aug. 12, 2016, 11:12 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
> James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A scheduler can use the LAUNCH_GROUP offer operation to send a
> `TaskGroup` to an executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
>   include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
>   src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
>   src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
>   src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 
> 
> Diff: https://reviews.apache.org/r/51061/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50266, 50889, 51042, 51043, 50269, 50270, 50271]

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 Aug. 15, 2016, 3:57 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Aug. 15, 2016, 3:57 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 5ed894d18258f6516866a2acd343c03281039c3c 
>   src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f917db78157f799efc00b0c7af0230ddb6f26b0b 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.

[Greg Mann]

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

(Updated Aug. 15, 2016, 8:18 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

This patch parametrizes the HTTP tests in libprocess so
that they run with SSL both enabled and disabled when
the library was compiled with SSL support.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
24b266df5f17e28e0c95326f5d1ea451952500e8 

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


Testing
---

Tested on OSX with `GTEST_REPEAT=100 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="*HTTPTest*" 3rdparty/libprocess/libprocess-tests`


Thanks,

Greg Mann

Re: Review Request 50857: Modified a scheduler test to run with SSL enabled.

[Greg Mann]

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

(Updated Aug. 15, 2016, 8:18 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


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


Repository: mesos


Description
---

This patch modifies the test `SchedulerTest.Teardown` to
be parametrized by both `ContentType` and SSL configuration,
and renames it to `SchedulerSSLTest.RunTaskAndTeardown`.
This allows the test to verify the scheduler's behavior with
SSL both enabled and disabled.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp ccd0f2f602a7a1a9a44b68fd0f59bdc8e0fa58b1 

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


Testing
---

This test is currently flaky in the SSL-enabled configurations and I've seen it 
produce a segfault on Ubuntu 14.04 after 38 iterations. To reproduce, try:

`GTEST_REPEAT=-1 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/2" 
bin/mesos-tests.sh`


Thanks,

Greg Mann

Review Request 51105: Updated hash and equality functions for ContainerID.

[Jie Yu]

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

Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
Vinod Kone.


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


Repository: mesos


Description
---

Updated hash and equality functions for ContainerID.


Diffs
-

  include/mesos/type_utils.hpp d0d23566f6262d8c4073c7870cb5546d39017e68 
  src/common/type_utils.cpp 7110d87ba8078a5cc00669c82f8cd36f103c14b3 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
b47e569221b90e35aa4cfc25618eb96a067cf2f4 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 51106: Added non-nesting CHECKs to containerizers.

[Jie Yu]

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

Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
Vinod Kone.


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


Repository: mesos


Description
---

Added non-nesting CHECKs to containerizers.


Diffs
-

  src/slave/containerizer/docker.cpp 03668d2b337c3b6c1331b6a807b4f3c3565f89f8 
  src/slave/containerizer/mesos/containerizer.cpp 
9b0cca2feabc3c9fcc2b84f3eb6dffac1253a649 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 51097: Added a `PortMapper` class.

[Avinash sridharan]

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

Review request for mesos, Jie Yu and Qian Zhang.


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


Repository: mesos


Description
---

This class will embody the logic for implementing the CNI port-mapper
plugin.


Diffs
-

  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/main.cpp
 PRE-CREATION 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.hpp
 PRE-CREATION 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 PRE-CREATION 

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


Testing
---

Tested the port-mapper with the following CNI config:
{
"name": "mynet",
"type": "port-mapper",
"chain": "MESOS",
"delegate": {
  "type" : "bridge",
  "bridge": "cni0",
  "isGateway": true,
  "ipMasq": true,
  "ipam": {
  "type": "host-local",
  "subnet": "10.22.0.0/16",
  "routes": [
{ "dst": "0.0.0.0/0" }
  ]
  }
},
"args" : {
  "org.apache.mesos" : {
"network_info" : {
  "port_mappings": {
"host_port" : 80,
"container_port" : 80
  }
}
  }
}
}

If we remove fields from the above CNI config, the creation of the `PortMapper` 
correctly fails. However, if config is passed as is it will create the 
`PortMapper` correctly.


Thanks,

Avinash sridharan

Re: Review Request 51062: Added LAUNCH_GROUP event to executor API.

[Benjamin Mahler]

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


Ship it!





src/examples/long_lived_executor.cpp (line 115)


Do we actually want to implement this? Maybe add a little bit about what it 
would mean.



src/examples/test_http_executor.cpp (lines 161 - 165)


Do we actually want to implement this? Maybe add a little bit about what it 
would mean.



src/launcher/executor.cpp (lines 200 - 201)


I suppose a scheduler (in the future) could target the task to an executor 
id that corresponds to a command executor that it previously launched.

Maybe just say that this should not be possible since we choose the 
group-capable default executor if there is a launch group message? It's not 
clear to me that the master would always be the one to prevent this from 
happening.


- Benjamin Mahler


On Aug. 12, 2016, 11:13 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51062/
> ---
> 
> (Updated Aug. 12, 2016, 11:13 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
> James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-6036
> https://issues.apache.org/jira/browse/MESOS-6036
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This event is used to deliver a task group atomically to an executor.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 9aa046e0e2421e8bb7678e53366970589d41e6d7 
>   include/mesos/v1/executor/executor.proto 
> 5e76e4e630d75673bd5647704a699134b2038338 
>   src/examples/long_lived_executor.cpp 
> 5c1fde195f1e6b599b025d771edd7e9841e51c61 
>   src/examples/test_http_executor.cpp 
> 19d0f684ece0e7a57c803ad541535824ee45131f 
>   src/launcher/executor.cpp 7967684e42c97326770bd83a41c2d4b1338a511f 
>   src/tests/mesos.hpp 59688dc9d1b16f20facf272085e65b3835f9a56e 
> 
> Diff: https://reviews.apache.org/r/51062/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> 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/#review145747
---




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


What about
```
Allocate GPU resources for a specified container.
```



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


s/size_t/const size_t



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


What about comments as this:

```
Deallocate GPU resources for a specified container.
```



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


s/size_t/const size_t



src/slave/containerizer/docker.cpp (lines 694 - 696)


What about move this right after the check so as to fail fast?

```
if (containers_[containerId]->gpuAllocated.empty()) {
  return Nothing();
}
```



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


I think this should be put into onReady section as 
https://github.com/apache/mesos/blob/master/src/log/log.cpp#L116-L119



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


s/size_t/const size_t



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


Keep this align with `.then(` in 1376


- Guangya Liu


On 八月 15, 2016, 7:26 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50841/
> ---
> 
> (Updated 八月 15, 2016, 7:26 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 f2a06065cf99fed934c2c1ffc47461ec8a97f50d 
>   src/slave/containerizer/docker.cpp 5c1ee8e467d1c54c60b67dc5275ef71e1bb90723 
> 
> Diff: https://reviews.apache.org/r/50841/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs: WIP.

[Zhitao Li]

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

(Updated Aug. 15, 2016, 9:21 p.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Changes
---

Pass enableCfsQuota to `Docker` class at creation time because `Docker::run` 
has too many arguments.


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


Repository: mesos


Description (updated)
---

This fixes cpu quota for command executor (which runs outside
of the docker container) by ensuing --cpu-quota flag to docker
run.

Note: we have to add the boolean variable to `Docker` class
because `Docker::run()` has reached the maximum argument length
GMOCK can support.


Diffs (updated)
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/docker/executor.cpp 4c08e5d589a64be21fd2396cf575694b126a15c2 
  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
  src/tests/containerizer/docker_containerizer_tests.cpp 
66adeec1411c71b269ff1d96c379968dab2a407a 
  src/tests/containerizer/docker_tests.cpp 
e9a214a8973b3dfac69d59e90ce08473baa7eba4 
  src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 
  src/tests/hook_tests.cpp a1c8685d7ae984a728f5225cae89e3c235b46266 
  src/tests/mesos.cpp 62e8fcc6fa7bd856aab6148ca6e6cad66b436f04 

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


Testing (updated)
---

I am now able to make docker containers launched through mesos-execute have a 
cpu quota.

Also making sure `make check` still works on mac os for the linux only flag.


Thanks,

Zhitao Li

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs: WIP.

[Zhitao Li]

> On Aug. 13, 2016, 1:02 a.m., Zhitao Li wrote:
> > src/docker/docker.hpp, line 152
> > 
> >
> > Note that this does not apply cleanly anymore, because gmock only 
> > generate mocks for functions with up to 10 arguments, and this is the 11th.
> > 
> > Working in a next branch to see whther I refactor this `Docker::run()` 
> > function to reduce number of arguments passed in. Any suggestion is 
> > welcomed.

Took an idea from @haosent to specify the argument at `Docker::create` instead.


- Zhitao


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


On Aug. 15, 2016, 9:21 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51052/
> ---
> 
> (Updated Aug. 15, 2016, 9:21 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-2154
> https://issues.apache.org/jira/browse/MESOS-2154
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes cpu quota for command executor (which runs outside
> of the docker container) by ensuing --cpu-quota flag to docker
> run.
> 
> Note: we have to add the boolean variable to `Docker` class
> because `Docker::run()` has reached the maximum argument length
> GMOCK can support.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
>   src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
>   src/docker/executor.cpp 4c08e5d589a64be21fd2396cf575694b126a15c2 
>   src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 66adeec1411c71b269ff1d96c379968dab2a407a 
>   src/tests/containerizer/docker_tests.cpp 
> e9a214a8973b3dfac69d59e90ce08473baa7eba4 
>   src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 
>   src/tests/hook_tests.cpp a1c8685d7ae984a728f5225cae89e3c235b46266 
>   src/tests/mesos.cpp 62e8fcc6fa7bd856aab6148ca6e6cad66b436f04 
> 
> Diff: https://reviews.apache.org/r/51052/diff/
> 
> 
> Testing
> ---
> 
> I am now able to make docker containers launched through mesos-execute have a 
> cpu quota.
> 
> Also making sure `make check` still works on mac os for the linux only flag.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 51105: Updated hash and equality functions for ContainerID.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 15, 2016, 1:37 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51105/
> ---
> 
> (Updated Aug. 15, 2016, 1:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6043
> https://issues.apache.org/jira/browse/MESOS-6043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated hash and equality functions for ContainerID.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp d0d23566f6262d8c4073c7870cb5546d39017e68 
>   src/common/type_utils.cpp 7110d87ba8078a5cc00669c82f8cd36f103c14b3 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> b47e569221b90e35aa4cfc25618eb96a067cf2f4 
> 
> Diff: https://reviews.apache.org/r/51105/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51106: Added non-nesting CHECKs to containerizers.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 15, 2016, 1:38 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51106/
> ---
> 
> (Updated Aug. 15, 2016, 1:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6043
> https://issues.apache.org/jira/browse/MESOS-6043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added non-nesting CHECKs to containerizers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 03668d2b337c3b6c1331b6a807b4f3c3565f89f8 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 9b0cca2feabc3c9fcc2b84f3eb6dffac1253a649 
> 
> Diff: https://reviews.apache.org/r/51106/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

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

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50907, 50910, 50912, 51008]

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 Aug. 15, 2016, 4:30 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51008/
> ---
> 
> (Updated Aug. 15, 2016, 4:30 p.m.)
> 
> 
> Review request for mesos, Haris Choudhary and Joseph Wu.
> 
> 
> 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
> -
> 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   src/cli_new/bin/mesos-tests PRE-CREATION 
>   src/cli_new/bin/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51008/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 51108: Added additional parameters to the CLI config.

[Haris Choudhary]

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

Review request for mesos, Joseph Wu and Kevin Klues.


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


Repository: mesos


Description
---

These parameters will be used by future plugins in the CLI. We can either
edit them directly from the config file or discover them through environment
variables. The environment variables override any edits made to the config file.

Also added the ability to format command flags in the base plugin class. This 
allows
us to not import the config file within individual plugins to do so.
The Key-Value pairs for formatting will be introduced with with future plugins.


Diffs
-

  src/cli_new/bin/config.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/base.py PRE-CREATION 

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


Testing
---


Thanks,

Haris Choudhary

Re: Review Request 51059: Made `ExecutorInfo.command` optional.

[Vinod Kone]

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

(Updated Aug. 15, 2016, 9:55 p.m.)


Review request for mesos, Benjamin Mahler, James DeFelice, and Jie Yu.


Changes
---

Added `type` field to ExecutorInfo to make the distinction between custom and 
default executors more explicit.


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


Repository: mesos


Description
---

This is done to allow schedulers to use `ExecutorInfo` with just
`executor_id` set. This is a requirement for the upcoming support
for `TaskGroup` semantics.

Note that in this patch, TASK_ERROR is returned when
`ExecutorInfo.command` is unset to keep the existing semantics for
`LAUNCH` offer operation. This might change when we add `LAUNCH_GROUP`
operation.


Diffs (updated)
-

  include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
  include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
  src/master/validation.cpp af18e5aef3be59830b0a7b0235bbc739dba1029c 
  src/tests/master_validation_tests.cpp 
9eb82a569f7c48caa96d4d54a93b199ccac74385 

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


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 51060: Made `Offer::Operation::Type` optional with default `UNKNOWN`.

[Vinod Kone]

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

(Updated Aug. 15, 2016, 9:56 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Jie Yu.


Changes
---

comments. NNFR.


Bugs: MESOS-6036 and MESOS-6037
https://issues.apache.org/jira/browse/MESOS-6036
https://issues.apache.org/jira/browse/MESOS-6037


Repository: mesos


Description
---

This is necessary to introduce new offer operations in a backwards
compatible manner. See MESOS-4997 for context.


Diffs (updated)
-

  include/mesos/mesos.proto af29dab2af4c28e257ad518c68d3f1af7cad0d06 
  include/mesos/v1/mesos.proto a836f6e792944726ec6f8f66f77f403894377d05 
  src/common/resources.cpp 6b7af9179121efbdc5c29484eb042778bcea8288 
  src/master/master.cpp 0bd1a3490a86fede86a3f5f62ce4745b65aae258 
  src/v1/resources.cpp 03ee0cb0bb5abe7fc1ae4cb47c5b6dbcd8d11998 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 51109: Added a Table abstraction and some functions to the CLI util file. These will be used by future plugins.

[Haris Choudhary]

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

Review request for mesos, Joseph Wu and Kevin Klues.


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


Repository: mesos


Description
---

Added a Table abstraction and some functions to the CLI util file. These will 
be used by future plugins.


Diffs
-

  src/cli_new/lib/mesos/util.py PRE-CREATION 

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


Testing
---


Thanks,

Haris Choudhary

Review Request 51110: Introduced the PluginTestBase class into the CLI.

[Haris Choudhary]

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

Review request for mesos.


Repository: mesos


Description
---

This class will be inherited by future plugin test classes and deals
with setting up and bringing down a cluster for plugins to use during
testing.


Diffs
-

  src/cli_new/lib/mesos/PluginTestBase.py PRE-CREATION 

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


Testing
---


Thanks,

Haris Choudhary