Re: Review Request 54068: Fixed some nits in hooks interfaces.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54038, 54068]

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

- Mesos ReviewBot


On Nov. 25, 2016, 9:38 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54068/
> ---
> 
> (Updated Nov. 25, 2016, 9:38 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, and Kapil Arya.
> 
> 
> Bugs: MESOS-6396
> https://issues.apache.org/jira/browse/MESOS-6396
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp f0606e3a68fa179cf7ea036f10563ef47c2aefa7 
>   src/examples/test_hook_module.cpp 5e91a71f2450cf3c37eb9039ef28c026095c917e 
>   src/hook/manager.hpp 5ecfcab48da808c84d36f9bcfcb5a8e0ad2167e5 
>   src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 
> 
> Diff: https://reviews.apache.org/r/54068/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing of entire RR chain.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 54088: Removed unused header include from stout's hashmap.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On Nov. 25, 2016, 8:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54088/
> ---
> 
> (Updated Nov. 25, 2016, 8:04 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused header include from stout's hashmap.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashmap.hpp 
> a1bebcb5f992012eb538469e8a3cecb5984bdc5a 
> 
> Diff: https://reviews.apache.org/r/54088/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 54085: Made internal Mesos headers more standalone.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54085]

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

- Mesos ReviewBot


On Nov. 25, 2016, 4:53 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54085/
> ---
> 
> (Updated Nov. 25, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Missing includes were found by attempting to parse the headeer files
> standalone with
> 
> $ clang++ -fsyntax-only -I  HEADER_FILE.hpp
> 
> By fixing failures we can make sure that all required symbols are
> defined.
> 
> Note this change does not address the issue of transitive includes.
> 
> 
> Diffs
> -
> 
>   src/common/build.hpp 80e071fea7e2ca98772ccd85930a538a8dbca0a6 
>   src/credentials/credentials.hpp c790793c7ea5ed384bdb397bfc1592b8fd1ff327 
>   src/master/allocator/mesos/metrics.hpp 
> 753f90acc52ada84883cbbe3350e61d1e1eaff48 
>   src/master/machine.hpp 22b87770f0811bcef25bb79d678e0a3e78d3f7ac 
>   src/slave/container_loggers/logrotate.hpp 
> 96dbd2db9495925cb618bd7fe6271023c74fd9fe 
>   src/slave/containerizer/composing.hpp 
> 292374aefbc70e4b9d8c81740656864dbada8e32 
>   src/slave/containerizer/mesos/isolators/appc/runtime.hpp 
> 8d225878ddbd04e4297b84c9ff782a994986aa4e 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> 642dd53a8028d26b0a8c7429a502d33db26838b7 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> 2cc8e764ff18c95c29598df75cdb370ccf120662 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
> cd79033b24cd54e1b7e1c18beeb1629fc0f18537 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b8fc755a8dd4757d904f7e97a71d3cf7f29d2033 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
> 7678a7c81c3cdb27410c1f066021eb34bd02a83f 
>   src/slave/containerizer/mesos/provisioner/appc/cache.hpp 
> 1e10dc604b29e418beacad6475910dcf7f8899c0 
>   src/slave/posix_signalhandler.hpp 49692b140662280b9e150654c5f336f1b920586f 
>   src/slave/qos_controllers/load.hpp 098a6d0b2dfc54b5b95a261a780eea70a838c12d 
>   src/tests/containerizer/mock_containerizer.hpp 
> 7a30b8307b93b7bf549efb52d72367f652d0d95a 
>   src/usage/usage.hpp 6576b9001f93cb33c8c4ae83b32ee22ea0354fc7 
> 
> Diff: https://reviews.apache.org/r/54085/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang trunk w/o optimizations, SSL)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54084: Made public Mesos header standalone.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54084]

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

- Mesos ReviewBot


On Nov. 25, 2016, 4:53 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54084/
> ---
> 
> (Updated Nov. 25, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Missing includes were found by attempting to parse the header files
> standalone with
> 
> $ clang++ -fsyntax-only -I  HEADER_FILE.hpp
> 
> By fixing failures we can make sure that all required symbols are
> defined.
> 
> Note this change does not address the issue of transitive includes.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.hpp e9430df03b49f49d367e19b18991ead77fe5b830 
>   include/mesos/executor/executor.hpp 
> 598d0106fe6c166507bde2bddfd7bed8d544da5a 
> 
> Diff: https://reviews.apache.org/r/54084/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang trunk w/o optimizations, SSL)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54083: Made headers in stout standalone.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54083]

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

- Mesos ReviewBot


On Nov. 25, 2016, 4:53 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54083/
> ---
> 
> (Updated Nov. 25, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Missing includes were found by attempting to parse the header files
> standalone with
> 
> $ clang++ -fsyntax-only -I  HEADER_FILE.hpp
> 
> By fixing failures we can make sure that all required symbols are
> defined.
> 
> Note this change does not address the issue of transitive includes.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os.hpp 
> bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba 
>   3rdparty/stout/include/stout/os/osx.hpp 
> 770c85d2a2569a4242d07b221f15a87565dd0bb5 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 1fa7614878cb0f58270824c88ab0013a9d9f14e9 
>   3rdparty/stout/include/stout/os/posix/socket.hpp 
> 836e4b3c37435a7f952321a13f3193043c1055d9 
>   3rdparty/stout/include/stout/os/posix/su.hpp 
> c3ce87e35761ab8543051cd3fcc6f0188a61eae1 
>   3rdparty/stout/include/stout/os/posix/xattr.hpp 
> 518940fdffab38ad97cf229078c4494fa944e1d8 
> 
> Diff: https://reviews.apache.org/r/54083/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang trunk w/o optimizations, SSL)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54068: Fixed some nits in hooks interfaces.

[Till Toenshoff]

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

(Updated Nov. 25, 2016, 9:38 p.m.)


Review request for mesos, Adam B, Gastón Kleiman, and Kapil Arya.


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


Repository: mesos


Description
---

see summary


Diffs (updated)
-

  include/mesos/hook.hpp f0606e3a68fa179cf7ea036f10563ef47c2aefa7 
  src/examples/test_hook_module.cpp 5e91a71f2450cf3c37eb9039ef28c026095c917e 
  src/hook/manager.hpp 5ecfcab48da808c84d36f9bcfcb5a8e0ad2167e5 
  src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 

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


Testing
---

make check && functional testing of entire RR chain.


Thanks,

Till Toenshoff

Re: Review Request 54038: Added new hook for covering executor and task environment.

[Till Toenshoff]

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

(Updated Nov. 25, 2016, 9:37 p.m.)


Review request for mesos, Adam B, Gastón Kleiman, Kapil Arya, and Joseph Wu.


Changes
---

Fixed typos - thank for the review!


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


Repository: mesos


Description
---

For being able to supply environment additions for both, a task and
its executor separately we need to introduce a new hook as the
existing ones slavePreLaunchDockerHook (deprecated) as well as
slavePreLaunchDockerEnvironmentDecorator do not allow for this.

This new hook will likely allow for further additions like e.g.
adding volumes without having to adapt the signature but only the
returned proto message TaskExecutorDecoratorInfo.


Diffs (updated)
-

  include/mesos/hook.hpp f0606e3a68fa179cf7ea036f10563ef47c2aefa7 
  include/mesos/module/hook.proto PRE-CREATION 
  src/Makefile.am dd1626d177b38a6613f18f32bb0668abbb5100e0 
  src/hook/manager.hpp 5ecfcab48da808c84d36f9bcfcb5a8e0ad2167e5 
  src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 
  src/slave/containerizer/docker.cpp a8e522fc058f50560e8ec162c31be079e620bf9d 

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


Testing
---

make check

*WIP - functional test pending - unit tests pending - WIP*


Thanks,

Till Toenshoff

Re: Review Request 54081: Added `--pidfile` option to master and agent binaries.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54081]

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

- Mesos ReviewBot


On Nov. 25, 2016, 4:21 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54081/
> ---
> 
> (Updated Nov. 25, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-1648
> https://issues.apache.org/jira/browse/MESOS-1648
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The PID file is created and kept locked while master / agent binary is 
> running to prevent other instances with the same PID file location from 
> starting.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am dd1626d177b38a6613f18f32bb0668abbb5100e0 
>   src/common/pid_file.hpp PRE-CREATION 
>   src/common/pid_file.cpp PRE-CREATION 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
>   src/master/main.cpp fa7ba1310142a3bef71379ba37fded9b8390aae9 
>   src/slave/main.cpp 8010f8e229e2d820649750c9db0456ecd1b854d3 
>   src/tests/common/pid_file_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54081/diff/
> 
> 
> Testing
> ---
> 
> Added test to verify that PID file is created upon `PIDFile` object creation 
> and deleted upon its destruction. Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Review Request 54087: Updated stout's README.

[Neil Conway]

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

Review request for mesos.


Repository: mesos


Description
---

Updated stout's README.


Diffs
-

  3rdparty/stout/README.md 80beb4430c28b571854c76ac3925e8b68f21f5f5 

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


Testing
---

No functional change.


Thanks,

Neil Conway

Review Request 54088: Removed unused header include from stout's hashmap.

[Neil Conway]

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

Review request for mesos.


Repository: mesos


Description
---

Removed unused header include from stout's hashmap.


Diffs
-

  3rdparty/stout/include/stout/hashmap.hpp 
a1bebcb5f992012eb538469e8a3cecb5984bdc5a 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 54077: Made sure parser settings member is properly initialized.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54077]

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

- Mesos ReviewBot


On Nov. 25, 2016, 4:07 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54077/
> ---
> 
> (Updated Nov. 25, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6646
> https://issues.apache.org/jira/browse/MESOS-6646
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made sure parser settings member is properly initialized.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 13db2eeea6f4b564c970c0cddcdebbe789aba65d 
> 
> Diff: https://reviews.apache.org/r/54077/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OS X, clang trunk w/o optimizations, SSL build)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 54084: Made public Mesos header standalone.

[Benjamin Bannier]

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

Review request for mesos, Kevin Klues and Qian Zhang.


Repository: mesos


Description
---

Missing includes were found by attempting to parse the header files
standalone with

$ clang++ -fsyntax-only -I  HEADER_FILE.hpp

By fixing failures we can make sure that all required symbols are
defined.

Note this change does not address the issue of transitive includes.


Diffs
-

  include/mesos/appc/spec.hpp e9430df03b49f49d367e19b18991ead77fe5b830 
  include/mesos/executor/executor.hpp 598d0106fe6c166507bde2bddfd7bed8d544da5a 

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


Testing
---

make check (OS X, clang trunk w/o optimizations, SSL)


Thanks,

Benjamin Bannier

Review Request 54083: Made headers in stout standalone.

[Benjamin Bannier]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Missing includes were found by attempting to parse the header files
standalone with

$ clang++ -fsyntax-only -I  HEADER_FILE.hpp

By fixing failures we can make sure that all required symbols are
defined.

Note this change does not address the issue of transitive includes.


Diffs
-

  3rdparty/stout/include/stout/os.hpp bd085e4e29bbdb2d2baaaeff1d10c0bd95ca65ba 
  3rdparty/stout/include/stout/os/osx.hpp 
770c85d2a2569a4242d07b221f15a87565dd0bb5 
  3rdparty/stout/include/stout/os/permissions.hpp 
1fa7614878cb0f58270824c88ab0013a9d9f14e9 
  3rdparty/stout/include/stout/os/posix/socket.hpp 
836e4b3c37435a7f952321a13f3193043c1055d9 
  3rdparty/stout/include/stout/os/posix/su.hpp 
c3ce87e35761ab8543051cd3fcc6f0188a61eae1 
  3rdparty/stout/include/stout/os/posix/xattr.hpp 
518940fdffab38ad97cf229078c4494fa944e1d8 

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


Testing
---

make check (OS X, clang trunk w/o optimizations, SSL)


Thanks,

Benjamin Bannier

Review Request 54085: Made internal Mesos headers more standalone.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rojas and Jiang Yan Xu.


Repository: mesos


Description
---

Missing includes were found by attempting to parse the headeer files
standalone with

$ clang++ -fsyntax-only -I  HEADER_FILE.hpp

By fixing failures we can make sure that all required symbols are
defined.

Note this change does not address the issue of transitive includes.


Diffs
-

  src/common/build.hpp 80e071fea7e2ca98772ccd85930a538a8dbca0a6 
  src/credentials/credentials.hpp c790793c7ea5ed384bdb397bfc1592b8fd1ff327 
  src/master/allocator/mesos/metrics.hpp 
753f90acc52ada84883cbbe3350e61d1e1eaff48 
  src/master/machine.hpp 22b87770f0811bcef25bb79d678e0a3e78d3f7ac 
  src/slave/container_loggers/logrotate.hpp 
96dbd2db9495925cb618bd7fe6271023c74fd9fe 
  src/slave/containerizer/composing.hpp 
292374aefbc70e4b9d8c81740656864dbada8e32 
  src/slave/containerizer/mesos/isolators/appc/runtime.hpp 
8d225878ddbd04e4297b84c9ff782a994986aa4e 
  src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
642dd53a8028d26b0a8c7429a502d33db26838b7 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
2cc8e764ff18c95c29598df75cdb370ccf120662 
  src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
cd79033b24cd54e1b7e1c18beeb1629fc0f18537 
  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
b8fc755a8dd4757d904f7e97a71d3cf7f29d2033 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
7678a7c81c3cdb27410c1f066021eb34bd02a83f 
  src/slave/containerizer/mesos/provisioner/appc/cache.hpp 
1e10dc604b29e418beacad6475910dcf7f8899c0 
  src/slave/posix_signalhandler.hpp 49692b140662280b9e150654c5f336f1b920586f 
  src/slave/qos_controllers/load.hpp 098a6d0b2dfc54b5b95a261a780eea70a838c12d 
  src/tests/containerizer/mock_containerizer.hpp 
7a30b8307b93b7bf549efb52d72367f652d0d95a 
  src/usage/usage.hpp 6576b9001f93cb33c8c4ae83b32ee22ea0354fc7 

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


Testing
---

make check (OS X, clang trunk w/o optimizations, SSL)


Thanks,

Benjamin Bannier

Review Request 54081: Added `--pidfile` option to master and agent binaries.

[Ilya Pronin]

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

Review request for mesos, Benjamin Mahler and Vinod Kone.


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


Repository: mesos


Description
---

The PID file is created and kept locked while master / agent binary is running 
to prevent other instances with the same PID file location from starting.


Diffs
-

  src/Makefile.am dd1626d177b38a6613f18f32bb0668abbb5100e0 
  src/common/pid_file.hpp PRE-CREATION 
  src/common/pid_file.cpp PRE-CREATION 
  src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
  src/master/main.cpp fa7ba1310142a3bef71379ba37fded9b8390aae9 
  src/slave/main.cpp 8010f8e229e2d820649750c9db0456ecd1b854d3 
  src/tests/common/pid_file_tests.cpp PRE-CREATION 

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


Testing
---

Added test to verify that PID file is created upon `PIDFile` object creation 
and deleted upon its destruction. Ran `make check`.


Thanks,

Ilya Pronin

Re: Review Request 54077: Made sure parser settings member is properly initialized.

[Benjamin Bannier]

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

(Updated Nov. 25, 2016, 5:07 p.m.)


Review request for mesos and Anand Mazumdar.


Changes
---

Initialized remaining `settings` members.


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


Repository: mesos


Description
---

Made sure parser settings member is properly initialized.


Diffs (updated)
-

  3rdparty/libprocess/src/decoder.hpp 13db2eeea6f4b564c970c0cddcdebbe789aba65d 

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


Testing
---

`make check` (OS X, clang trunk w/o optimizations, SSL build)


Thanks,

Benjamin Bannier

Re: Review Request 54077: Made sure parser settings member is properly initialized.

[Anand Mazumdar]

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



Thanks for the cleanup. Can you do a sweep in this file and fix the other 
decoders to correctly initialize the `settings` variable too?

- Anand Mazumdar


On Nov. 25, 2016, 11:41 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54077/
> ---
> 
> (Updated Nov. 25, 2016, 11:41 a.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6646
> https://issues.apache.org/jira/browse/MESOS-6646
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made sure parser settings member is properly initialized.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 13db2eeea6f4b564c970c0cddcdebbe789aba65d 
> 
> Diff: https://reviews.apache.org/r/54077/diff/
> 
> 
> Testing
> ---
> 
> `make check` (OS X, clang trunk w/o optimizations, SSL build)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54062: Added logic to validate FrameworkInfo roles.

[Jay Guo]

> On Nov. 25, 2016, 3:04 p.m., Guangya Liu wrote:
> > src/master/validation.cpp, lines 295-305
> > 
> >
> > How about the following?
> > 
> > ```
> > // TODO(jay_guo) Consider extending this method to validate more
> > // fields of FrameworkInfo in the future.
> > Option validate(const mesos::FrameworkInfo& frameworkInfo)
> > {
> >   vector> validators = {
> > lambda::bind(internal::validateRoles, frameworkInfo)
> >   };
> > 
> >   foreach (const lambda::function& validator, 
> > validators) {
> > Option error = validator();
> > if (error.isSome()) {
> >   return error;
> > }
> >   }
> > 
> >   return None();
> > }
> > ```

Probably let's not overengineer this for now and revisit when we need to extend 
it in the future?


- Jay


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


On Nov. 25, 2016, 11:26 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54062/
> ---
> 
> (Updated Nov. 25, 2016, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, and Qiang Zhang.
> 
> 
> Bugs: MESOS-6629
> https://issues.apache.org/jira/browse/MESOS-6629
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to do necessary validation for the conflicts of role, roles
> and MULTI_ROLE capability. It complies with following matrix:
> 
> -- MULTI_ROLE is NOT set -
> |---|-|
> |Roles  |No Roles |
> |---|---|-|
> |Role   | Error |  None   |
> |---|---|-|
> |No Role| Error |  None   |
> |---|---|-|
> 
> --- MULTI_ROLE is set 
> |---|-|
> |Roles  |No Roles |
> |---|---|-|
> |Role   | Error |  Error  |
> |---|---|-|
> |No Role| None  |  None   |
> |---|---|-|
> 
> One test case is added to ensure validateRoles() catches invalid
> combination of these attributes. Another three test cases are added
> to ensure the master accepts/rejects subscription given valid/invalid
> multiple roles.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
>   src/master/validation.hpp b8389460f34b3531f2b6ff93f18f496c01e1a079 
>   src/master/validation.cpp 42d9b4a8784c2a161b74d7b46619cc22272e14e3 
>   src/tests/master_validation_tests.cpp 
> f893067859425967654401f3226149268b51cf57 
> 
> Diff: https://reviews.apache.org/r/54062/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 54062: Added logic to validate FrameworkInfo roles.

[Jay Guo]

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

(Updated Nov. 25, 2016, 3:40 p.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, and Qiang Zhang.


Changes
---

addressed guangya's comments


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


Repository: mesos


Description
---

We need to do necessary validation for the conflicts of role, roles
and MULTI_ROLE capability. It complies with following matrix:

-- MULTI_ROLE is NOT set -
|---|-|
|Roles  |No Roles |
|---|---|-|
|Role   | Error |  None   |
|---|---|-|
|No Role| Error |  None   |
|---|---|-|

--- MULTI_ROLE is set 
|---|-|
|Roles  |No Roles |
|---|---|-|
|Role   | Error |  Error  |
|---|---|-|
|No Role| None  |  None   |
|---|---|-|

One test case is added to ensure validateRoles() catches invalid
combination of these attributes. Another three test cases are added
to ensure the master accepts/rejects subscription given valid/invalid
multiple roles.


Diffs (updated)
-

  src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
  src/master/validation.hpp b8389460f34b3531f2b6ff93f18f496c01e1a079 
  src/master/validation.cpp 42d9b4a8784c2a161b74d7b46619cc22272e14e3 
  src/tests/master_validation_tests.cpp 
f893067859425967654401f3226149268b51cf57 

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


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[Alexander Rukletsov]

> On Nov. 25, 2016, 3:09 p.m., haosdent huang wrote:
> > src/tests/health_check_tests.cpp, line 842
> > 
> >
> > Change here to make it consistent with
> > 
> > ```
> > // This test creates a task whose health flaps, and verifies that the
> > // health status updates are sent to the framework.
> > 
> > TEST_F(HealthCheckTest, HealthStatusChange)
> > ```

Framework scheduler is probably the best name : )


- Alexander


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


On Nov. 25, 2016, 3:08 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46307/
> ---
> 
> (Updated Nov. 25, 2016, 3:08 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> Neil Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-1802
> https://issues.apache.org/jira/browse/MESOS-1802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In HealthStatusChange test cases, we launch a task that toggles between
> healthy and unhealthy, and will never be killed because no consecutive
> health failures occur. We need to ignore subsequent status updates: it
> is possible to continue to receive status updates before we stop the
> driver.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp a4436bdb70ca988106742dadb0762c99a4ebe369 
> 
> Diff: https://reviews.apache.org/r/46307/diff/
> 
> 
> Testing
> ---
> 
> # I still could not reproduce the problem in old code after repeatedly tests. 
> So seems no way to verify whether my assumption is correct or not.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 25, 2016, 3:08 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46307/
> ---
> 
> (Updated Nov. 25, 2016, 3:08 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> Neil Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-1802
> https://issues.apache.org/jira/browse/MESOS-1802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In HealthStatusChange test cases, we launch a task that toggles between
> healthy and unhealthy, and will never be killed because no consecutive
> health failures occur. We need to ignore subsequent status updates: it
> is possible to continue to receive status updates before we stop the
> driver.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp a4436bdb70ca988106742dadb0762c99a4ebe369 
> 
> Diff: https://reviews.apache.org/r/46307/diff/
> 
> 
> Testing
> ---
> 
> # I still could not reproduce the problem in old code after repeatedly tests. 
> So seems no way to verify whether my assumption is correct or not.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53849: Added parsing of V2 schema 2 Docker image manifests.

[Ilya Pronin]

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

(Updated Nov. 25, 2016, 3:15 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Added missing proto file.


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


Repository: mesos


Description
---

Added parsing of V2 schema 2 Docker image manifests.


Diffs (updated)
-

  include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
  include/mesos/docker/v2.hpp abab12b8f73564a6da1a0265a503d407a2849b3f 
  include/mesos/docker/v2_s2.proto PRE-CREATION 
  src/Makefile.am dd1626d177b38a6613f18f32bb0668abbb5100e0 
  src/docker/spec.cpp 2f2c32e9b7d78debb31dcc1aa91a4d45c1ced192 
  src/tests/containerizer/docker_spec_tests.cpp 
82bddd228df3db95a00eb277ff9c380039b70b1e 

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


Testing
---

Added tests to verify that V2 schema 2 image manifest can be successfully 
parsed and errors like missing required fields or empty layers list are caught. 
Ran `make check`.


Thanks,

Ilya Pronin

Re: Review Request 52865: Refactored HealthChecker to never stop health checking.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 3:10 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

Prior to this patch, HealthChecker would stop performing health
checks after it marks the task for kill. Since tasks' lifecycle
is managed by scheduler-executor, HealthChecker should never stop
health checking on its own.

Allowing health checks to run forever enables the scheduler
make the decision about how to deal with unhealthy tasks.


Diffs
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 
  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 
  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[haosdent huang]

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




src/tests/health_check_tests.cpp (line 842)


Change here to make it consistent with

```
// This test creates a task whose health flaps, and verifies that the
// health status updates are sent to the framework.

TEST_F(HealthCheckTest, HealthStatusChange)
```


- haosdent huang


On Nov. 25, 2016, 3:08 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46307/
> ---
> 
> (Updated Nov. 25, 2016, 3:08 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> Neil Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-1802
> https://issues.apache.org/jira/browse/MESOS-1802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In HealthStatusChange test cases, we launch a task that toggles between
> healthy and unhealthy, and will never be killed because no consecutive
> health failures occur. We need to ignore subsequent status updates: it
> is possible to continue to receive status updates before we stop the
> driver.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp a4436bdb70ca988106742dadb0762c99a4ebe369 
> 
> Diff: https://reviews.apache.org/r/46307/diff/
> 
> 
> Testing
> ---
> 
> # I still could not reproduce the problem in old code after repeatedly tests. 
> So seems no way to verify whether my assumption is correct or not.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52869: Ensured command executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 3:09 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description (updated)
---

We stop health checking both when the task is reaped and
killed since these may be two different execution paths.


Diffs
-

  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52870: Ensured docker executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 3:09 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description (updated)
---

We stop health checking both when the task is reaped and
killed since these may be two different execution paths.


Diffs
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[haosdent huang]

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

(Updated Nov. 25, 2016, 3:08 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, Neil 
Conway, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

In HealthStatusChange test cases, we launch a task that toggles between
healthy and unhealthy, and will never be killed because no consecutive
health failures occur. We need to ignore subsequent status updates: it
is possible to continue to receive status updates before we stop the
driver.


Diffs (updated)
-

  src/tests/health_check_tests.cpp a4436bdb70ca988106742dadb0762c99a4ebe369 

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


Testing
---

# I still could not reproduce the problem in old code after repeatedly tests. 
So seems no way to verify whether my assumption is correct or not.


Thanks,

haosdent huang

Re: Review Request 47088: Dropped http status check in HealthCheckTest.HealthStatusChange.

[haosdent huang]

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

(Updated Nov. 25, 2016, 3:08 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
Artem Harutyunyan, Neil Conway, and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Dropped http status check because so far we could not guarantee the
http queries order match the `statusUpdate` order.


Diffs (updated)
-

  src/tests/health_check_tests.cpp a4436bdb70ca988106742dadb0762c99a4ebe369 

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


Testing
---

Using

```
GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter="HealthCheckTest.HealthStatusChange" --verbose 
--gtest_break_on_failure --gtest_repeat=-1
```

More than 1000 interactions in my slow vm.


Thanks,

haosdent huang

Re: Review Request 52869: Ensured command executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 3:07 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52870: Ensured docker executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 3:07 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 54062: Added logic to validate FrameworkInfo roles.

[Guangya Liu]

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




src/master/validation.hpp (lines 60 - 61)


new line here as `//`



src/master/validation.hpp (line 61)


```
--- MULTI_ROLE is NOT set ---
```



src/master/validation.hpp (line 69)


Add `//` here



src/master/validation.hpp (line 70)


```
--- MULTI_ROLE is set ---
```



src/master/validation.hpp (lines 77 - 78)


new line here as `//`



src/master/validation.cpp (line 249)


s/FrameworkInfo.role/'FrameworkInfo.role' 
s/FrameworkInfo.roles/'FrameworkInfo.roles'

Ditto for other error messages.



src/master/validation.cpp (lines 264 - 265)


How about 

```
return Error("The MULTI_ROLE framework capability must be enabled"
 " when 'FrameworkInfo.roles' is set");
```



src/master/validation.cpp (lines 295 - 305)


How about the following?

```
// TODO(jay_guo) Consider extending this method to validate more
// fields of FrameworkInfo in the future.
Option validate(const mesos::FrameworkInfo& frameworkInfo)
{
  vector> validators = {
lambda::bind(internal::validateRoles, frameworkInfo)
  };

  foreach (const lambda::function& validator, validators) {
Option error = validator();
if (error.isSome()) {
  return error;
}
  }

  return None();
}
```



src/master/validation.cpp (line 301)


I think we do not need to mention `Invalid roles` here as here we are 
checking both `role` and `roles`, using the error returned by 
`internal::validateRoles` is good enough.



src/tests/master_validation_tests.cpp (lines 2333 - 2335)


```
// This tests the validate roles function of FrameworkInfo. For 3
// attributes including role, roles and MULTI_ROLE capability,
// we have 2^3 cases to test in total.
```



src/tests/master_validation_tests.cpp (line 2390)


keep align with `FrameworkValidationTest`



src/tests/master_validation_tests.cpp (line 2415)


```
// This test ensures subscription succeeds for multiple role 
// framework when MULTI_ROLE capability is enabled.
```



src/tests/master_validation_tests.cpp (line 2423)



// Add multiple roles to the FrameworkInfo 
with MULTI_ROLE
// capability is enabled.




src/tests/master_validation_tests.cpp (line 2454)


```
// Add multiple roles to the FrameworkInfo with MULTI_ROLE
// capability is enabled.
```


- Guangya Liu


On 十一月 25, 2016, 11:26 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54062/
> ---
> 
> (Updated 十一月 25, 2016, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, and Qiang Zhang.
> 
> 
> Bugs: MESOS-6629
> https://issues.apache.org/jira/browse/MESOS-6629
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to do necessary validation for the conflicts of role, roles
> and MULTI_ROLE capability. It complies with following matrix:
> 
> -- MULTI_ROLE is NOT set -
> |---|-|
> |Roles  |No Roles |
> |---|---|-|
> |Role   | Error |  None   |
> |---|---|-|
> |No Role| Error |  None   |
> |---|---|-|
> 
> --- MULTI_ROLE is set 
> |---|-|
> |Roles  |No Roles |
> |---|---|-|
> |Role   | Error |  Error  |
> |---|---|-|
> |No Role| None  |  None   |
> |---|---|-|
> 
> One test case is added to ensure validateRoles() catches invalid
> combination of these attributes. Another three test cases are added
> to ensure the master accepts/rejects subscription given valid/invalid
> multiple roles.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
>   src/master/validation.hpp 

Re: Review Request 53607: Added Seccomp imeplementation.

[Jay Guo]

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

(Updated Nov. 25, 2016, 2:47 p.m.)


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


Changes
---

rebase


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


Repository: mesos


Description
---

It leverages libseccomp to initialize a seccomp filter context,
add architectures and rules, and loads it into kernel.


Diffs (updated)
-

  src/Makefile.am dd1626d177b38a6613f18f32bb0668abbb5100e0 
  src/linux/seccomp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
320e42748adbabf09f77cb4f5951e2a7ea58fe64 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 53608: Added --enable-seccomp flag to configure options.

[Jay Guo]

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

(Updated Nov. 25, 2016, 2:47 p.m.)


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


Changes
---

rebase


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


Repository: mesos


Description
---

User would be able to enable seccomp figures via --enable-seccomp.
It requires libseccomp to be pre-installed.


Diffs (updated)
-

  configure.ac 5380cbc6a7951ede2f883f7045952a3f3434479e 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 53606: Added `linux/seccomp` isolator.

[Jay Guo]

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

(Updated Nov. 25, 2016, 2:47 p.m.)


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


Changes
---

rebase


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


Repository: mesos


Description
---

Added `linux/seccomp` isolator.


Diffs (updated)
-

  src/Makefile.am dd1626d177b38a6613f18f32bb0668abbb5100e0 
  src/slave/containerizer/mesos/containerizer.cpp 
9b33495d9babc3ee489a8712fe1977746c41043f 
  src/slave/containerizer/mesos/isolators/linux/seccomp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/seccomp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
155e3c5a27b8c710971ee4b508600d3b5589a2e0 
  src/slave/containerizer/mesos/launch.cpp 
320e42748adbabf09f77cb4f5951e2a7ea58fe64 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 53605: Added --seccomp_profile flag to Mesos agent.

[Jay Guo]

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

(Updated Nov. 25, 2016, 2:46 p.m.)


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


Changes
---

rebase


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


Repository: mesos


Description
---

Operator uses this flag to pass in JSON file that specifies a set of
Seccomp rules that will be enforced on all container processes launched
on this agent.


Diffs (updated)
-

  include/mesos/type_utils.hpp 7824407a6bbfc06edeec69479ed82cabae34a440 
  src/common/parse.hpp 1dca9ba4ed4ff2163193ab6432b5b017d2be787e 
  src/common/type_utils.cpp 8270f808277d84a73823992c639d6536a99353b8 
  src/slave/flags.hpp c6c3197bbf30ec617751f4a1a34914c0f0e29eb5 
  src/slave/flags.cpp 0de15eca7da9bf8fbdbb90c6e96edfe76f4a0f44 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 53604: Added SeccompInfo protobuf message.

[Jay Guo]

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

(Updated Nov. 25, 2016, 2:46 p.m.)


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


Changes
---

rebase


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


Repository: mesos


Description
---

Added SeccompInfo protobuf message.


Diffs (updated)
-

  include/mesos/mesos.proto cbfcd8a666e0b4a486f6dcd9e8356c9d5a1cea05 
  include/mesos/slave/containerizer.proto 
2621522ae59cf9275f607679b4678ac54508993d 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 54068: Fixed some nits in hooks interfaces.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Nov. 25, 2016, 4:30 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54068/
> ---
> 
> (Updated Nov. 25, 2016, 4:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, and Kapil Arya.
> 
> 
> Bugs: MESOS-6396
> https://issues.apache.org/jira/browse/MESOS-6396
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp f0606e3a68fa179cf7ea036f10563ef47c2aefa7 
>   src/examples/test_hook_module.cpp 5e91a71f2450cf3c37eb9039ef28c026095c917e 
>   src/hook/manager.hpp 5ecfcab48da808c84d36f9bcfcb5a8e0ad2167e5 
>   src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 
> 
> Diff: https://reviews.apache.org/r/54068/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing of entire RR chain.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 54038: Added new hook for covering executor and task environment.

[Gastón Kleiman]

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


Fix it, then Ship it!




Just a few typos, but it looks good to me =).


include/mesos/hook.hpp (line 111)


s/superceded/superseded/



include/mesos/hook.hpp (line 149)


s/mappe/mapped/



include/mesos/hook.hpp (line 159)


ditto typo


- Gastón Kleiman


On Nov. 25, 2016, 4:10 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54038/
> ---
> 
> (Updated Nov. 25, 2016, 4:10 a.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-6396
> https://issues.apache.org/jira/browse/MESOS-6396
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For being able to supply environment additions for both, a task and
> its executor separately we need to introduce a new hook as the
> existing ones slavePreLaunchDockerHook (deprecated) as well as
> slavePreLaunchDockerEnvironmentDecorator do not allow for this.
> 
> This new hook will likely allow for further additions like e.g.
> adding volumes without having to adapt the signature but only the
> returned proto message TaskExecutorDecoratorInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp f0606e3a68fa179cf7ea036f10563ef47c2aefa7 
>   include/mesos/module/hook.proto PRE-CREATION 
>   src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 
>   src/hook/manager.hpp 5ecfcab48da808c84d36f9bcfcb5a8e0ad2167e5 
>   src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 
>   src/slave/containerizer/docker.cpp ccabf99f305d7874e1c46bc618ea74341eb281ef 
> 
> Diff: https://reviews.apache.org/r/54038/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> *WIP - functional test pending - unit tests pending - WIP*
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52382: Added stubs for OCI store.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52349, 52379, 52382]

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

- Mesos ReviewBot


On Nov. 25, 2016, 7:48 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52382/
> ---
> 
> (Updated Nov. 25, 2016, 7:48 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5011
> https://issues.apache.org/jira/browse/MESOS-5011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stubs for OCI store.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 
>   src/slave/containerizer/mesos/provisioner/oci/store.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/oci/store.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52382/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 54007: Printed complete health check configuration on task launch.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:56 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 52868: Health checks may be stopped on demand.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Nov. 25, 2016, 11:50 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52868/
> ---
> 
> (Updated Nov. 25, 2016, 11:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.
> 
> 
> Bugs: MESOS-5963
> https://issues.apache.org/jira/browse/MESOS-5963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 837d1358e418d21536da488e4a23cbfa41db6060 
>   src/health-check/health_checker.cpp 
> af5500be249c74a4a5e64bf38dea607173e2f998 
> 
> Diff: https://reviews.apache.org/r/52868/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 54007: Printed complete health check configuration on task launch.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:52 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 52872: Used callback instead of `send()` for health status updates.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:52 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

Since HealthChecker is now used as a library only and does not live
in a separate OS process, there is no need to use libprocess message
sending for health status updates; a callback will do.


Diffs (updated)
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 
  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 
  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52873: Cleaned up private members in HealthChecker class.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:52 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 52868: Health checks may be stopped on demand.

[Alexander Rukletsov]

> On Nov. 24, 2016, 5:25 p.m., Gastón Kleiman wrote:
> > I think that `HealthChecker::stop()` should just terminate the actor, no 
> > need to introduce a boolean.

Good idea. WILCO.


- Alexander


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


On Nov. 25, 2016, 11:50 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52868/
> ---
> 
> (Updated Nov. 25, 2016, 11:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.
> 
> 
> Bugs: MESOS-5963
> https://issues.apache.org/jira/browse/MESOS-5963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 837d1358e418d21536da488e4a23cbfa41db6060 
>   src/health-check/health_checker.cpp 
> af5500be249c74a4a5e64bf38dea607173e2f998 
> 
> Diff: https://reviews.apache.org/r/52868/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52871: Ensured default executor ignores health updates for terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:51 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

After the task has been terminated, its health updates become
irrelevant and should be ignored. Also if the default executor
shuts down, we can safely stop all health checkers.

Technically health checking should be stopped right before
TASK_KILLING update is sent to avoid subsequent TASK_RUNNING
updates, but the default executor currently does not support
TASK_KILLING.


Diffs (updated)
-

  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52869: Ensured command executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:50 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52868: Health checks may be stopped on demand.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:50 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:49 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

To facilitate code reuse, HealthChecker::reschedule() is generalized.
This will become even more valuable when we add pause/resume functions.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52870: Ensured docker executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:50 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52867: Used `Duration::create()` for double -> Duration conversion.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:49 a.m.)


Review request for mesos, Daniel Pravat, Gastón Kleiman, haosdent huang, and 
Joseph Wu.


Repository: mesos


Description
---

Additionally persist health check parameters from the `HealthCheck`
protobuf as class members to avoid code duplication.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 54004: Renamed functions in HealthChecker for clarity.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:49 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

Use descriptive function names instead of underscore
prefixes for functions in HealthChecker.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52865: Refactored HealthChecker to never stop health checking.

[Alexander Rukletsov]

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

(Updated Nov. 25, 2016, 11:49 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

Prior to this patch, HealthChecker would stop performing health
checks after it marks the task for kill. Since tasks' lifecycle
is managed by scheduler-executor, HealthChecker should never stop
health checking on its own.

Allowing health checks to run forever enables the scheduler
make the decision about how to deal with unhealthy tasks.


Diffs (updated)
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 
  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 
  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Review Request 54077: Made sure parser settings member is properly initialized.

[Benjamin Bannier]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

Made sure parser settings member is properly initialized.


Diffs
-

  3rdparty/libprocess/src/decoder.hpp 13db2eeea6f4b564c970c0cddcdebbe789aba65d 

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


Testing
---

`make check` (OS X, clang trunk w/o optimizations, SSL build)


Thanks,

Benjamin Bannier

Re: Review Request 54062: Added logic to validate FrameworkInfo roles.

[Jay Guo]

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

(Updated Nov. 25, 2016, 11:26 a.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, and Qiang Zhang.


Changes
---

Addressed Guangya's comments


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


Repository: mesos


Description (updated)
---

We need to do necessary validation for the conflicts of role, roles
and MULTI_ROLE capability. It complies with following matrix:

-- MULTI_ROLE is NOT set -
|---|-|
|Roles  |No Roles |
|---|---|-|
|Role   | Error |  None   |
|---|---|-|
|No Role| Error |  None   |
|---|---|-|

--- MULTI_ROLE is set 
|---|-|
|Roles  |No Roles |
|---|---|-|
|Role   | Error |  Error  |
|---|---|-|
|No Role| None  |  None   |
|---|---|-|

One test case is added to ensure validateRoles() catches invalid
combination of these attributes. Another three test cases are added
to ensure the master accepts/rejects subscription given valid/invalid
multiple roles.


Diffs (updated)
-

  src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
  src/master/validation.hpp b8389460f34b3531f2b6ff93f18f496c01e1a079 
  src/master/validation.cpp 42d9b4a8784c2a161b74d7b46619cc22272e14e3 
  src/tests/master_validation_tests.cpp 
f893067859425967654401f3226149268b51cf57 

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


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 54068: Fixed some nits in hooks interfaces.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54038, 54068]

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

- Mesos ReviewBot


On Nov. 25, 2016, 4:30 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54068/
> ---
> 
> (Updated Nov. 25, 2016, 4:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, and Kapil Arya.
> 
> 
> Bugs: MESOS-6396
> https://issues.apache.org/jira/browse/MESOS-6396
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp f0606e3a68fa179cf7ea036f10563ef47c2aefa7 
>   src/examples/test_hook_module.cpp 5e91a71f2450cf3c37eb9039ef28c026095c917e 
>   src/hook/manager.hpp 5ecfcab48da808c84d36f9bcfcb5a8e0ad2167e5 
>   src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 
> 
> Diff: https://reviews.apache.org/r/54068/diff/
> 
> 
> Testing
> ---
> 
> make check && functional testing of entire RR chain.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 53949: Added test helper to obtain unused port.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 25, 2016, 9:05 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53949/
> ---
> 
> (Updated Nov. 25, 2016, 9:05 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test helper to obtain unused port.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.hpp 140ebaaae43b03568ec49891635f0660cdfb4c85 
>   src/tests/utils.cpp eb36616f68d81d33d4bd04a7f23295e8c7558fc8 
> 
> Diff: https://reviews.apache.org/r/53949/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53950/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 24, 2016, 5:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 24, 2016, 5:39 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> * `make check` (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> * tested with in-tree parallel test runner: `GTEST_FILTER='HealthCheckTest.*' 
>  ~/src/mesos/support/mesos-gtest-runner.py ./mesos-tests` which would fail 
> about for about 50% before this fix, and not measurably anymore after.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54074: Added isolator nested aware user doc.

[Qian Zhang]

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




docs/mesos-containerizer.md (line 413)


A space was missed here.


- Qian Zhang


On Nov. 25, 2016, 4:36 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54074/
> ---
> 
> (Updated Nov. 25, 2016, 4:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, Qian 
> Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6335
> https://issues.apache.org/jira/browse/MESOS-6335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a table for current semantics of isolators
> in Mesos, as well as the guidance of how the make a custom
> isolator module to be nested aware.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md 2bff35f6361f760a9001a249d2c01bbbc9e72932 
> 
> Diff: https://reviews.apache.org/r/54074/diff/
> 
> 
> Testing
> ---
> 
> Tested by gist view. Here is the link:
> 
> https://github.com/Gilbert88/mesos/blob/doc_pod/docs/mesos-containerizer.md#isolator-nested-aware
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 54013: Added user doc for nested container and task group.

[Qian Zhang]

> On Nov. 24, 2016, 2:41 p.m., Qian Zhang wrote:
> > docs/nested-container-and-task-group.md, line 209
> > 
> >
> > Currently we only support a single level of container nesting, right? 
> > If so, then I think we may need to mention this limitation here, and also 
> > mention we will support multiple level of container netsting in the section 
> > of `Future Work`.
> 
> Gilbert Song wrote:
> We already supported the multi-level nested container. We have a simple 
> unit test `ROOT_CGROUPS_LaunchNestedThreeLevels` for three-level nested. 
> Technically, arbitrary level nested up to 32 should work.

Thanks Gilbert for the clarification! I think the following code confused me:
https://github.com/apache/mesos/blob/1.1.0/src/slave/http.cpp#L1933:L1934

Now I understand we do support multi-level nested container, but the nested 
container can only be created level by level rather than across levels.


- Qian


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


On Nov. 25, 2016, 4:36 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54013/
> ---
> 
> (Updated Nov. 25, 2016, 4:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, Qian 
> Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6335
> https://issues.apache.org/jira/browse/MESOS-6335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added user doc for nested container and task group.
> 
> 
> Diffs
> -
> 
>   docs/nested-container-and-task-group.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54013/diff/
> 
> 
> Testing
> ---
> 
> Tested by gist view. Here is the link:
> 
> https://github.com/Gilbert88/mesos/blob/doc_pod/docs/nested-container-and-task-group.md
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 53949: Added test helper to obtain unused port.

[Benjamin Bannier]

> On Nov. 24, 2016, 6:34 p.m., Alexander Rukletsov wrote:
> > src/tests/utils.hpp, line 80
> > 
> >
> > #include 

Even though this is the correct header for C++ code like we have here, we seem 
to widely prefer getting this definition from the C stdlib. Went with 
`stdint.h`.


- Benjamin


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


On Nov. 25, 2016, 10:05 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53949/
> ---
> 
> (Updated Nov. 25, 2016, 10:05 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test helper to obtain unused port.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.hpp 140ebaaae43b03568ec49891635f0660cdfb4c85 
>   src/tests/utils.cpp eb36616f68d81d33d4bd04a7f23295e8c7558fc8 
> 
> Diff: https://reviews.apache.org/r/53949/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53950/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53949: Added test helper to obtain unused port.

[Benjamin Bannier]

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

(Updated Nov. 25, 2016, 10:05 a.m.)


Review request for mesos, Alexander Rukletsov and haosdent huang.


Changes
---

Addressed alexr's comments.


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


Repository: mesos


Description
---

Added test helper to obtain unused port.


Diffs (updated)
-

  src/tests/utils.hpp 140ebaaae43b03568ec49891635f0660cdfb4c85 
  src/tests/utils.cpp eb36616f68d81d33d4bd04a7f23295e8c7558fc8 

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


Testing
---

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


Thanks,

Benjamin Bannier

Re: Review Request 54062: Added logic to validate FrameworkInfo roles.

[Guangya Liu]

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




src/master/master.cpp (line 2336)


We shoudl also make sure `isWhitelistedRole` can cover the multiple role 
case.



src/master/validation.hpp (line 57)


I think we do not need to put `validate` to `internal` for now, but may 
need to add some new functions in future if needed, such as `validateRoles()` 
etc.



src/master/validation.hpp (line 59)


kill this



src/master/validation.hpp (line 83)


kill this



src/master/validation.hpp (line 84)


I'd prefer we name it as `validate()` here to keep consistent, also we may 
need to validate other fields for `framework` in the future.



src/master/validation.cpp (line 252)


we do not need period for those error message in the end, please kill all 
of the period for those error messages.



src/tests/master_validation_tests.cpp (line 2389)


We usally use a full name, it is difficult to understand what `Cap` means.

```
TEST_F(FrameworkValidationTest,
   RejectFrameworkWithRolesWithoutMultipleRoleCapability)
```


- Guangya Liu


On 十一月 25, 2016, 6:45 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54062/
> ---
> 
> (Updated 十一月 25, 2016, 6:45 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Qiang Zhang.
> 
> 
> Bugs: MESOS-6629
> https://issues.apache.org/jira/browse/MESOS-6629
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to do necessary validation for the conflicts of role, roles
> and MULTI_ROLE capability. It complies with following matrix:
> 
> -- MULTI_ROLE is NOT set -
> |---|-|
> |Roles  |No Roles |
> |---|---|-|
> |Role   | Error |  None   |
> |---|---|-|
> |No Role| Error |  None   |
> |---|---|-|
> 
> --- MULTI_ROLE is set 
> |---|-|
> |Roles  |No Roles |
> |---|---|-|
> |Role   | Error |  Error  |
> |---|---|-|
> |No Role| None  |  None   |
> |---|---|-|
> 
> Two test cases are added, one is for validateRoles method and another
> ensures that the master rejects subscription when provided invalid
> roles.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
>   src/master/validation.hpp b8389460f34b3531f2b6ff93f18f496c01e1a079 
>   src/master/validation.cpp 42d9b4a8784c2a161b74d7b46619cc22272e14e3 
>   src/tests/master_validation_tests.cpp 
> f893067859425967654401f3226149268b51cf57 
> 
> Diff: https://reviews.apache.org/r/54062/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53948, 53986, 53949, 53950]

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

- Mesos ReviewBot


On Nov. 24, 2016, 5:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 24, 2016, 5:39 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> * `make check` (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> * tested with in-tree parallel test runner: `GTEST_FILTER='HealthCheckTest.*' 
>  ~/src/mesos/support/mesos-gtest-runner.py ./mesos-tests` which would fail 
> about for about 50% before this fix, and not measurably anymore after.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 54074: Added isolator nested aware user doc.

[Gilbert Song]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a table for current semantics of isolators
in Mesos, as well as the guidance of how the make a custom
isolator module to be nested aware.


Diffs
-

  docs/mesos-containerizer.md 2bff35f6361f760a9001a249d2c01bbbc9e72932 

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


Testing
---

Tested by gist view. Here is the link:

https://github.com/Gilbert88/mesos/blob/doc_pod/docs/mesos-containerizer.md#isolator-nested-aware


Thanks,

Gilbert Song

Re: Review Request 54013: Added user doc for nested container and task group.

[Gilbert Song]

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

(Updated Nov. 25, 2016, 12:36 a.m.)


Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

Added user doc for nested container and task group.


Diffs (updated)
-

  docs/nested-container-and-task-group.md PRE-CREATION 

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


Testing
---

Tested by gist view. Here is the link:

https://github.com/Gilbert88/mesos/blob/doc_pod/docs/nested-container-and-task-group.md


Thanks,

Gilbert Song

Review Request 54073: Added linux launcher nested container support doc.

[Gilbert Song]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a table showing the current semantics
that the linux launcher supports namespaces for nested
container.


Diffs
-

  docs/containerizer-internals.md 97776a69e5a7df8b65f672a754d456e7b2b90320 

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


Testing
---

Tested by gist view. Here is the link:

https://github.com/Gilbert88/mesos/blob/doc_pod/docs/containerizer-internals.md#linux-launcher


Thanks,

Gilbert Song

Re: Review Request 54013: Added user doc for nested container and task group.

[Gilbert Song]

> On Nov. 23, 2016, 10:41 p.m., Qian Zhang wrote:
> > docs/nested-container-and-task-group.md, line 166
> > 
> >
> > Indent, it seems a space is missed here.

Good catch, Qian.


> On Nov. 23, 2016, 10:41 p.m., Qian Zhang wrote:
> > docs/nested-container-and-task-group.md, line 209
> > 
> >
> > Currently we only support a single level of container nesting, right? 
> > If so, then I think we may need to mention this limitation here, and also 
> > mention we will support multiple level of container netsting in the section 
> > of `Future Work`.

We already supported the multi-level nested container. We have a simple unit 
test `ROOT_CGROUPS_LaunchNestedThreeLevels` for three-level nested. 
Technically, arbitrary level nested up to 32 should work.


> On Nov. 23, 2016, 10:41 p.m., Qian Zhang wrote:
> > docs/nested-container-and-task-group.md, line 297
> > 
> >
> > Does this refer to multiple level of container nesting or something 
> > else?

The task group api is different from the nested container api. We may want to 
support launching multiple task groups using `LaunchGroup` api in the future.


> On Nov. 23, 2016, 10:41 p.m., Qian Zhang wrote:
> > docs/nested-container-and-task-group.md, line 213
> > 
> >
> > I see here you have illustrated `NESTED_CONTAINER_LAUNCH` and 
> > `NESTED_CONTAINER_WAIT`, do you want to illustrate  `NESTED_CONTAINER_KILL` 
> > too?

Thought about it before, but it depends on users whether or not to call 
`NESTED_CONTAINER_KILL ` from the executor. We already document 
`NESTED_CONTAINER_KILL` in previous section. This workflow just simply shows a 
common case of a nested container's life cycle using the new agent api.


- Gilbert


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


On Nov. 22, 2016, 8:23 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54013/
> ---
> 
> (Updated Nov. 22, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6335
> https://issues.apache.org/jira/browse/MESOS-6335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added user doc for nested container and task group.
> 
> 
> Diffs
> -
> 
>   docs/nested-container-and-task-group.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54013/diff/
> 
> 
> Testing
> ---
> 
> Tested by gist view. Here is the link:
> 
> https://github.com/Gilbert88/mesos/blob/doc_pod/docs/nested-container-and-task-group.md
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>