Review Request 52623: Replaced POSIX `int` with `FileDesc` abstraction in `src` folder. On POSIX this should have no effect.

[Daniel Pravat]

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

Review request for mesos, Joseph Wu and Michael Park.


Repository: mesos


Description
---

Replaced POSIX `int` with `FileDesc` abstraction in `src` folder. On POSIX this 
should have no effect.


Diffs
-

  cmake/CompilationConfigure.cmake 11a8507eb773391073a7b945e2aac503262f86b7 
  src/files/files.cpp 9c634ac928887b3f1a111f67ebb3fc5229c6fa16 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 
  src/slave/containerizer/fetcher.cpp 11104d66e6dd05d8eb1d37a2e3250aca19278110 
  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/launch.cpp 
7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
  src/slave/state.cpp a94bb8d7029295abef70d6595ebc732ac1ab87a8 
  src/slave/status_update_manager.cpp 056a684b52756d5c6309e7e2167a1532c4e60957 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52545: Replaced POSIX `int` with `FileDesc` abstraction.

[Daniel Pravat]

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

(Updated Oct. 7, 2016, 5:45 a.m.)


Review request for mesos, Joseph Wu and Michael Park.


Repository: mesos


Description
---

On POSIX this should have no effect.


Diffs (updated)
-

  3rdparty/libprocess/include/process/io.hpp 
eec5efd7e6b71a783f2bb40826054d0488cee71f 
  3rdparty/libprocess/include/process/network.hpp 
52110667185370a4c92e2fa524819ab1f34bdec9 
  3rdparty/libprocess/include/process/socket.hpp 
67551a904ebc4c2f97d65ad7ab5d4ab8c07f16db 
  3rdparty/libprocess/include/process/subprocess_base.hpp 
1d02454d5541f96cb4928bf027fcae3764989d67 
  3rdparty/libprocess/include/process/windows/subprocess.hpp 
f452f6743d01f0b99010fa5e5bcbaae1358c8241 
  3rdparty/libprocess/src/io.cpp d930ceebc90468e082b984e41385549f906dc6ae 
  3rdparty/libprocess/src/poll_socket.hpp 
d04f3f2d1bcf70464ac659b29f96574bbd233414 
  3rdparty/libprocess/src/poll_socket.cpp 
d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
  3rdparty/libprocess/src/process.cpp f1d746c52cfe659f5cd7da4b7a6424ff585619a3 
  3rdparty/libprocess/src/socket.cpp 6089248639793603226210421a2c2193d14ea049 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
c8350cf8e512dca23933725e6edb3e3d94380211 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 52624: Replaced POSIX `int` with `FileDesc` abstraction in `src` folder.

[Daniel Pravat]

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

Review request for mesos, Joseph Wu and Michael Park.


Repository: mesos


Description
---

On POSIX this should have no effect.


Diffs
-

  cmake/CompilationConfigure.cmake 11a8507eb773391073a7b945e2aac503262f86b7 
  src/files/files.cpp 9c634ac928887b3f1a111f67ebb3fc5229c6fa16 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 
  src/slave/containerizer/fetcher.cpp 11104d66e6dd05d8eb1d37a2e3250aca19278110 
  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/launch.cpp 
7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
  src/slave/state.cpp a94bb8d7029295abef70d6595ebc732ac1ab87a8 
  src/slave/status_update_manager.cpp 056a684b52756d5c6309e7e2167a1532c4e60957 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52347: Send last `TaskStatus` in `TaskUpdated`.

[Zhitao Li]

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

(Updated Oct. 7, 2016, 5:19 a.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description (updated)
---

Send last `TaskStatus` in `TaskUpdated`.


Diffs (updated)
-

  include/mesos/master/master.proto d2312dce31f56dd65ac4d0e554b680749da61335 
  include/mesos/v1/master/master.proto 0dc6cca367590ea8e9f22b22b85baa4c11e69b13 
  src/common/protobuf_utils.hpp 96df060d48d0206cfd745e17dcd568d0fbd032f4 
  src/common/protobuf_utils.cpp f09b0692ad5b47ac3f4c755a3921d15c242778b4 
  src/master/master.cpp c7e74df71aa31edb490f4f3bd95f2d5aa94b4324 
  src/tests/api_tests.cpp 39f3f6641cfb4b2e05cbaa1e8156e8a2f480f87a 

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


Testing
---

unit test.


Thanks,

Zhitao Li

Re: Review Request 51774: Added `AGENT_ADDED` and `AGENT_REMOVED` master events.

[Anand Mazumdar]

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


Fix it, then Ship it!




I would fix these while committing. Also, would update the description to be 
more informative.


include/mesos/v1/master/master.proto (line 486)


s/NOTE that/NOTE: It's


- Anand Mazumdar


On Oct. 6, 2016, 8:17 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51774/
> ---
> 
> (Updated Oct. 6, 2016, 8:17 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `AGENT_ADDED` and `AGENT_REMOVED` master events.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 1ea79fd7ecd9c2b7e94718fc8c7779d1051583db 
>   include/mesos/v1/master/master.proto 
> 58dc65006658f47bb752ee1c0c1520ee3cc03bd7 
> 
> Diff: https://reviews.apache.org/r/51774/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52615: Store terminated containers in the test containerizer.

[Anand Mazumdar]

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


Ship it!




LGTM

- Anand Mazumdar


On Oct. 6, 2016, 7:38 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52615/
> ---
> 
> (Updated Oct. 6, 2016, 7:38 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6319
> https://issues.apache.org/jira/browse/MESOS-6319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows callers to "reap" containers that are already terminated,
> mimicing the behavior of the mesos containerizer.
> 
> This also fixes MESOS-6319, where the `wait()` could occur after a
> `destroy()` had already completed.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer.hpp 0ffba7397e844f863652fc6c6e03b8d2b47e717d 
>   src/tests/containerizer.cpp 3affa0ff8a348df91f7a56c318df2786ee5dea0b 
> 
> Diff: https://reviews.apache.org/r/52615/diff/
> 
> 
> Testing
> ---
> 
> $ ./bin/mesos-tests.sh --gtest_filter="*AgentAPITest.NestedContainer*" 
> --gtest_repeat=-1 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 52574: Parameterized the image volume tests for nested containers.

[Gilbert Song]

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


Ship it!




Smart changes, but just a little harder for people who is new to this part to 
read.

- Gilbert Song


On Oct. 5, 2016, 2:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52574/
> ---
> 
> (Updated Oct. 5, 2016, 2:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Parameterized the image volume tests for nested containers.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/volume_image_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e 
> 
> Diff: https://reviews.apache.org/r/52574/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52250, 52251, 52560, 52561, 52563]

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

- Mesos ReviewBot


On Oct. 6, 2016, 5:26 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52563/
> ---
> 
> (Updated Oct. 6, 2016, 5:26 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Replace `Testing` to `Tests` in comments.
> * Remove redundant `.Times(1)`.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52563/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
> --gtest_also_run_disabled_tests
> [--] Global test environment tear-down
> [==] 19 tests from 1 test case ran. (35264 ms total)
> [  PASSED  ] 19 tests.
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 52620: Updated InverseOffers* maintenance tests to use the new scheduler mock.

[Ilya Pronin]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

Updated InverseOffers* maintenance tests to use the new scheduler mock.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
77eb405ab7314da906bed9ec1d0018c24928d8d8 

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


Testing
---

`make check` on OS X 10.11 and Ubuntu 14.04.


Thanks,

Ilya Pronin

Re: Review Request 52539: Refactored and simplified LinuxFilesystemIsolatorTests.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 4, 2016, 6:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52539/
> ---
> 
> (Updated Oct. 4, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored and simplified LinuxFilesystemIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
>   src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e 
> 
> Diff: https://reviews.apache.org/r/52539/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52347: Send last `TaskStatus` in `TaskUpdated`.

[Zhitao Li]

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

(Updated Oct. 6, 2016, 9:52 p.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Vinod Kone.


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


Repository: mesos


Description
---

Review: https://reviews.apache.org/r/52347


Diffs (updated)
-

  include/mesos/master/master.proto 1ea79fd7ecd9c2b7e94718fc8c7779d1051583db 
  include/mesos/v1/master/master.proto 58dc65006658f47bb752ee1c0c1520ee3cc03bd7 
  src/common/protobuf_utils.hpp c5e5a9ad10134bcf87b59460bec7152e4916c75d 
  src/common/protobuf_utils.cpp 3ff37f498d6c830eb2f4b3d9395f9416cb01f5d3 
  src/master/master.cpp 66a672f6d16233e96b29e330a9e6c474546fa851 
  src/tests/api_tests.cpp e857b17cfe5f05d59859263c025564d33700a26c 

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


Testing
---

unit test.


Thanks,

Zhitao Li

Review Request 52617: Improved symmetry of code in related utility functions.

[Neil Conway]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Improved symmetry of code in related utility functions.


Diffs
-

  src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 52618: Added comment describing a common gotcha.

[Neil Conway]

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

Review request for mesos and Joseph Wu.


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


Repository: mesos


Description
---

Added comment describing a common gotcha.


Diffs
-

  src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 52616: Fixed whitespace infelicities.

[Neil Conway]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Fixed whitespace infelicities.


Diffs
-

  src/tests/utils.cpp cc5259a720bb6451714a100b0451b473395e3650 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52608: Reordered the list of executor env variables in code and documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52556, 52608]

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

- Mesos ReviewBot


On Oct. 6, 2016, 4:29 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52608/
> ---
> 
> (Updated Oct. 6, 2016, 4:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-6288
> https://issues.apache.org/jira/browse/MESOS-6288
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reordered the list of executor env variables in code and documentation.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
>   src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 
> 
> Diff: https://reviews.apache.org/r/52608/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 52539: Refactored and simplified LinuxFilesystemIsolatorTests.

[Jie Yu]

> On Oct. 6, 2016, 7:45 p.m., Gilbert Song wrote:
> > src/tests/containerizer/filesystem_isolator_tests.cpp, line 230
> > 
> >
> > Use `const` if sounds good to you.

I just make things consistent. If we want to use const, why not for others like 
ContainerID, FrameworkID, etc.?


- Jie


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


On Oct. 5, 2016, 1:15 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52539/
> ---
> 
> (Updated Oct. 5, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored and simplified LinuxFilesystemIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
>   src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e 
> 
> Diff: https://reviews.apache.org/r/52539/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52539: Refactored and simplified LinuxFilesystemIsolatorTests.

[Jie Yu]

> On Oct. 6, 2016, 7:45 p.m., Gilbert Song wrote:
> > src/tests/containerizer/filesystem_isolator_tests.cpp, line 115
> > 
> >
> > This is my main concern on this patch. We bundle the `filesystem/linux` 
> > isolator with the `docker/runtime` isolator for all unit tests in this file 
> > because of getting rid of the helper `createContainerizer()`. 
> > 
> > Even though we know that is doable, I still consider whether or not we 
> > should do that, because:
> > 
> > 1. In Mesos, the `docker/runtime` isolator depends on the 
> > `filesystem/linux`. Then, should we have the `filesystem/linux` tests 
> > relying on `docker/runtime`? isn't a stand alone test environment better?
> > 
> > 2. We already know `filesystem/linux` tests slow. And now, except 
> > copying some files from the host rootfs, we also need to tar and untar it. 
> > Have the provisioner, docker store and local puller get involved. And go 
> > through the docker runtime isolator logic. Seems a little expansive to me.

What's the difference? createContainerizer will use LinuxRootfs as well, right?

I am less worried about performance (also, the performance is decent). I prefer 
code cleaness than overly optimize.


- Jie


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


On Oct. 5, 2016, 1:15 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52539/
> ---
> 
> (Updated Oct. 5, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored and simplified LinuxFilesystemIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
>   src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e 
> 
> Diff: https://reviews.apache.org/r/52539/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52573: Removed some redundant cleanup code in linux filesystem isolator.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 5, 2016, 2:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52573/
> ---
> 
> (Updated Oct. 5, 2016, 2:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed some redundant cleanup code in linux filesystem isolator.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
> 
> Diff: https://reviews.apache.org/r/52573/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52572: Used unmountAll to replace the original shell script.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 5, 2016, 2:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52572/
> ---
> 
> (Updated Oct. 5, 2016, 2:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used unmountAll to replace the original shell script.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp fba94f6e3c810a7907ff180e19a770c36d569374 
> 
> Diff: https://reviews.apache.org/r/52572/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52546: Moved a few test helpers to the common header.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 4, 2016, 11:07 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52546/
> ---
> 
> (Updated Oct. 4, 2016, 11:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved a few test helpers to the common header.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
>   src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e 
> 
> Diff: https://reviews.apache.org/r/52546/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51774: Added `AGENT_ADDED` and `AGENT_REMOVED` master events.

[Zhitao Li]

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

(Updated Oct. 6, 2016, 8:17 p.m.)


Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
Vinod Kone.


Summary (updated)
-

Added `AGENT_ADDED` and `AGENT_REMOVED` master events.


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


Repository: mesos


Description (updated)
---

Added `AGENT_ADDED` and `AGENT_REMOVED` master events.


Diffs (updated)
-

  include/mesos/master/master.proto 1ea79fd7ecd9c2b7e94718fc8c7779d1051583db 
  include/mesos/v1/master/master.proto 58dc65006658f47bb752ee1c0c1520ee3cc03bd7 

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


Testing
---

New unit test.


Thanks,

Zhitao Li

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

[Anindya Sinha]

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

(Updated Oct. 6, 2016, 8:16 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

When a framework issues a CREATE for a shared volume, we allow that
operation only if the framework has opted in for the capability of
SHARED_RESOURCES. However, for HTTP operator (/create-volumes), we
do not check as the operator API is not related to a specific
framework.


Diffs (updated)
-

  src/master/master.cpp 02a2fb29bdd8484fc90e5cb033ac29b49a141860 
  src/master/validation.hpp 035f721c610ae566c89a1cf0e65ff0df11679f15 
  src/master/validation.cpp e5da3c9bf0a1042b42522f1ab74ce798fbb1738d 
  src/tests/master_validation_tests.cpp 
99e350e0587e73e9ee25ef20dd369cd146bd446a 
  src/tests/persistent_volume_tests.cpp 
e10a79e9662530e143ccaa2aa2506a4d25158364 

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 52516: Added test for `AGENT_ADDED` and `AGENT_REMOVED` events.

[Zhitao Li]

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

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


Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
Vinod Kone.


Summary (updated)
-

Added test for `AGENT_ADDED` and `AGENT_REMOVED` events.


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


Repository: mesos


Description (updated)
---

Added test for `AGENT_ADDED` and `AGENT_REMOVED` events.


Diffs (updated)
-

  src/tests/api_tests.cpp 7b0ad3c18a38b5ec859a79d09f5707f6958960fb 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52515: Implemented `AGENT_ADDED` and `AGENT_REMOVED` events.

[Zhitao Li]

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

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


Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
Vinod Kone.


Summary (updated)
-

Implemented `AGENT_ADDED` and `AGENT_REMOVED` events.


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


Repository: mesos


Description (updated)
---

Implemented `AGENT_ADDED` and `AGENT_REMOVED` events.


Diffs (updated)
-

  src/common/protobuf_utils.hpp 0c6405f6cfc5a69fa835428fa330d8d63778f469 
  src/common/protobuf_utils.cpp 1f088684c566f89c85dd72eaece295c5e3d50b23 
  src/master/master.cpp 756ab546851952bc1de24b1f469d232237b1c01c 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52584: Created helper function `createAgentResponse`.

[Zhitao Li]

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

(Updated Oct. 6, 2016, 8:10 p.m.)


Review request for mesos, Anand Mazumdar and Xiaojian Huang.


Summary (updated)
-

Created helper function `createAgentResponse`.


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


Repository: mesos


Description (updated)
---

Created helper function `createAgentResponse`.


Diffs (updated)
-

  src/common/protobuf_utils.hpp 0c6405f6cfc5a69fa835428fa330d8d63778f469 
  src/common/protobuf_utils.cpp 1f088684c566f89c85dd72eaece295c5e3d50b23 
  src/master/http.cpp e9f9d16de803175d4f07e0486674e6150c632b02 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 52539: Refactored and simplified LinuxFilesystemIsolatorTests.

[Gilbert Song]

> On Oct. 6, 2016, 12:45 p.m., Gilbert Song wrote:
> > src/tests/containerizer/filesystem_isolator_tests.cpp, lines 199-219
> > 
> >
> > Consider a followup patch to move this to tests/mesos.hpp?

Just see it in your followup patch.


- Gilbert


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


On Oct. 4, 2016, 6:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52539/
> ---
> 
> (Updated Oct. 4, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored and simplified LinuxFilesystemIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
>   src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e 
> 
> Diff: https://reviews.apache.org/r/52539/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52539: Refactored and simplified LinuxFilesystemIsolatorTests.

[Gilbert Song]

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




src/tests/containerizer/filesystem_isolator_tests.cpp 


This is my main concern on this patch. We bundle the `filesystem/linux` 
isolator with the `docker/runtime` isolator for all unit tests in this file 
because of getting rid of the helper `createContainerizer()`. 

Even though we know that is doable, I still consider whether or not we 
should do that, because:

1. In Mesos, the `docker/runtime` isolator depends on the 
`filesystem/linux`. Then, should we have the `filesystem/linux` tests relying 
on `docker/runtime`? isn't a stand alone test environment better?

2. We already know `filesystem/linux` tests slow. And now, except copying 
some files from the host rootfs, we also need to tar and untar it. Have the 
provisioner, docker store and local puller get involved. And go through the 
docker runtime isolator logic. Seems a little expansive to me.



src/tests/containerizer/filesystem_isolator_tests.cpp (lines 89 - 109)


Consider a followup patch to move this to tests/mesos.hpp?



src/tests/containerizer/filesystem_isolator_tests.cpp (line 119)


Use `const` if sounds good to you.


Maybe we can chat a little bit to find the balance?

- Gilbert Song


On Oct. 4, 2016, 6:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52539/
> ---
> 
> (Updated Oct. 4, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored and simplified LinuxFilesystemIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
>   src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e 
> 
> Diff: https://reviews.apache.org/r/52539/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 52615: Store terminated containers in the test containerizer.

[Benjamin Mahler]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

This allows callers to "reap" containers that are already terminated,
mimicing the behavior of the mesos containerizer.

This also fixes MESOS-6319, where the `wait()` could occur after a
`destroy()` had already completed.


Diffs
-

  src/tests/containerizer.hpp 0ffba7397e844f863652fc6c6e03b8d2b47e717d 
  src/tests/containerizer.cpp 3affa0ff8a348df91f7a56c318df2786ee5dea0b 

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


Testing
---

$ ./bin/mesos-tests.sh --gtest_filter="*AgentAPITest.NestedContainer*" 
--gtest_repeat=-1 --gtest_break_on_failure


Thanks,

Benjamin Mahler

Re: Review Request 52347: Send last `TaskStatus` in `TaskUpdated`.

[Anand Mazumdar]

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




include/mesos/master/master.proto (line 465)


s/task_status/status

We have been just using "status" historically everywhere for `TaskStatus`.



include/mesos/master/master.proto (line 467)


Move this comment before L465 since `status` is the task status 
corresponding to the last status update acked by the scheduler. Also consider 
rewording this to:

```cpp
// This is the status of the task corresponding to the last status update 
acknowledged by the scheduler.
```



include/mesos/master/master.proto (line 468)


Since we would be moving the comment in L467 above L465, can we add a 
comment for this field too?



src/common/protobuf_utils.hpp (line 170)


s/last acknowledged state from scheduler/latest state



src/common/protobuf_utils.hpp (line 171)


s/recently updated status/last status update acknowledged by the scheduler



src/master/master.cpp (line 7483)


How about: 
```cpp
// Indicated whether we should send a notification to all subscribers if 
the task transitioned to a new state.
```

and killing the comment on L7533



src/master/master.cpp (line 7539)


hmm, a subscriber might be interested in the `data` field too which gets 
removed in L7531?


- Anand Mazumdar


On Oct. 6, 2016, 5:13 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52347/
> ---
> 
> (Updated Oct. 6, 2016, 5:13 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5936
> https://issues.apache.org/jira/browse/MESOS-5936
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/52347
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 1ea79fd7ecd9c2b7e94718fc8c7779d1051583db 
>   include/mesos/v1/master/master.proto 
> 58dc65006658f47bb752ee1c0c1520ee3cc03bd7 
>   src/common/protobuf_utils.hpp c5e5a9ad10134bcf87b59460bec7152e4916c75d 
>   src/common/protobuf_utils.cpp 3ff37f498d6c830eb2f4b3d9395f9416cb01f5d3 
>   src/master/master.cpp 66a672f6d16233e96b29e330a9e6c474546fa851 
>   src/tests/api_tests.cpp e857b17cfe5f05d59859263c025564d33700a26c 
> 
> Diff: https://reviews.apache.org/r/52347/diff/
> 
> 
> Testing
> ---
> 
> unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52538: Reordered filesystem isolator tests.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 4, 2016, 6:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52538/
> ---
> 
> (Updated Oct. 4, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a pure code movement. Moved all the end to end tests to the
> bottom of the file.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe 
> 
> Diff: https://reviews.apache.org/r/52538/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 51774: Add `AGENT_ADDED` and `AGENT_REMOVED` master events.

[Anand Mazumdar]

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




include/mesos/master/master.proto (line 482)


s/after/within
s/since a master re-election/upon a master failover



include/mesos/master/master.proto (lines 483 - 484)


hmm, this is not true anymore after the recent agent partition aware 
changes. We move the agent to a list of unreachable agents but do not invoke 
`removeSlave()` since it can come back at some point in the future. Hence, you 
won't get the `_REMOVED` event. We might want to add an `_UNCREACHABLE` event 
if a subscriber is interested in them sometime later.

Can we rephrase the comment after "or," to say that this can also happen 
when the agent is scheduled for maintenance instead?



include/mesos/master/master.proto (line 485)


s/Note/NOTE:
Also, new line before this line.


- Anand Mazumdar


On Oct. 5, 2016, 11:45 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51774/
> ---
> 
> (Updated Oct. 5, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add `AGENT_ADDED` and `AGENT_REMOVED` master events.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 1ea79fd7ecd9c2b7e94718fc8c7779d1051583db 
>   include/mesos/v1/master/master.proto 
> 58dc65006658f47bb752ee1c0c1520ee3cc03bd7 
> 
> Diff: https://reviews.apache.org/r/51774/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 52612: Added nested container tests for docker runtime isolator.

[Gilbert Song]

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

Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, and 
Timothy Chen.


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


Repository: mesos


Description
---

Added nested container tests for docker runtime isolator.


Diffs
-

  src/tests/containerizer/runtime_isolator_tests.cpp 
c0c11607024b6a80d5bf5a486b91f7905a9083d7 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 52058: Fixed fetcher to not call chown on sandbox again.

[Megha Sharma]

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

(Updated Oct. 6, 2016, 6:28 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Fetcher code changes the ownership of entire sandbox directory
recursively to the taskuser and as a resut also changes the
ownership of files laid out by other entities in the sandbox.


Diffs (updated)
-

  src/slave/containerizer/fetcher.cpp 11104d66e6dd05d8eb1d37a2e3250aca19278110 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
ad2070246b7fdb90aa6ed02326b5a7eb95365997 

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


Testing
---

make check on linux and macos


Thanks,

Megha Sharma

Re: Review Request 52288: Recover resources when offer is rescinded on DESTROY of shared volume.

[Jiang Yan Xu]

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




src/tests/persistent_volume_tests.cpp (line 1005)


Add the condition "when the shared persistent volume is destroyed" to 
summary?

e.g., 

```
// This test verifies that pending offers with shared persistent volumes 
are rescinded when the volumes are destroyed.
```



src/tests/persistent_volume_tests.cpp (line 1021)


This is the beginning of the "body" of the test. This test is long and 
relatively complicated. We should summarize the steps either here as a numbered 
list or by separating the test body by "headings" (with a step number to 
distinguish from the lower-level comments). The latter looks better to me in 
this case.

Somthing like:

```
1. Framework1 creates the volume and launches a task that uses a portion of 
the offer.
2. Framework2 connects and receives the offer with the remaining resources 
and the shared volume.
3. Framework1 kills the task and receives another offer with the shared 
volume. (Now both frameworks have the volume).
4. Framework2 destroys the volume and the offer for framework1 is recinded.
```

(This slightly differs from your version but it reduces the number of steps 
I think)



src/tests/persistent_volume_tests.cpp (lines 1049 - 1050)


The following more direct?

```
We use a filter of 0 secs so the resources will be available in the next 
allocation cycle.
```



src/tests/persistent_volume_tests.cpp (lines 1062 - 1066)


It's important that the task uses only a portion of cpus/mem so the rest 
can be offered to framework2. Perhaps emphasize this in the comment as well?

The benefit of not using the the shared volume in the task is actually not 
obvious to me: we kill the task before destroying the volume anyways. If it's 
for simplicity that we don't use the volume, we should comment on the reason 
being so.



src/tests/persistent_volume_tests.cpp (lines 1068 - 1071)


Don't reuse the variable `offers` (even though it's reset to pending, this 
detail is very implicit).



src/tests/persistent_volume_tests.cpp (lines 1073 - 1078)


Are these necessary?

We just need a framework2 to be connected to the master and when it 
receives an offer we are golden right?

The task is just a mechanism we use to split offers so we don't care how 
it's doing.



src/tests/persistent_volume_tests.cpp (line 1084)


Move `filters` to the left by one space.



src/tests/persistent_volume_tests.cpp (lines 1093 - 1100)


Connect framework2 before advancing the clock so framework2 can get the 
offer immediately? (Even before the clock advancement).

This reduces the overall number of allocation cycles we have to advance 
(and the length of the code) right?



src/tests/persistent_volume_tests.cpp (line 1094)


No need to resume the clock here and `Clock::advance()` below doesn't work 
without pausing again.



src/tests/persistent_volume_tests.cpp (lines 1102 - 1103)


Since `task` doesn't use all resources, we should expect `famework2` to 
receive the offer immediately right?



src/tests/persistent_volume_tests.cpp (lines 1130 - 1131)


Why wait on status? It should be sufficient that we just wait on offers as 
they are the result of the kill.



src/tests/persistent_volume_tests.cpp (line 1149)


s/has/have/



src/tests/persistent_volume_tests.cpp (lines 1152 - 1153)


Instead of `Times(1)`, we can just 

```
Future rescinded;
EXPECT_CALL(sched1, offerRescinded(, _))
  .WillOnce(FutureSatisfy());
```

And wait on it:

```
AWAIT_READY(rescinded);
```



src/tests/persistent_volume_tests.cpp (lines 1155 - 1157)


No need for this. See below.



src/tests/persistent_volume_tests.cpp (lines 1164 - 1174)


This test is for offer rescind so we can just stop at the expectation for 
that right?


- Jiang Yan Xu


On Sept. 26, 2016, 4:58 p.m., Anindya Sinha wrote:
> 
> 

Re: Review Request 52584: Create helper function `createAgentResponse`.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Oct. 5, 2016, 11:46 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52584/
> ---
> 
> (Updated Oct. 5, 2016, 11:46 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Xiaojian Huang.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create helper function `createAgentResponse`.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0c6405f6cfc5a69fa835428fa330d8d63778f469 
>   src/common/protobuf_utils.cpp 1f088684c566f89c85dd72eaece295c5e3d50b23 
>   src/master/http.cpp e9f9d16de803175d4f07e0486674e6150c632b02 
> 
> Diff: https://reviews.apache.org/r/52584/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52515: Implement `AGENT_ADDED` and `AGENT_REMOVED` events.

[Anand Mazumdar]

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


Fix it, then Ship it!





src/master/http.cpp (line 2303)


Can we move this to the earlier review?


- Anand Mazumdar


On Oct. 5, 2016, 11:47 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52515/
> ---
> 
> (Updated Oct. 5, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement `AGENT_ADDED` and `AGENT_REMOVED` events.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0c6405f6cfc5a69fa835428fa330d8d63778f469 
>   src/common/protobuf_utils.cpp 1f088684c566f89c85dd72eaece295c5e3d50b23 
>   src/master/http.cpp e9f9d16de803175d4f07e0486674e6150c632b02 
>   src/master/master.cpp 756ab546851952bc1de24b1f469d232237b1c01c 
> 
> Diff: https://reviews.apache.org/r/52515/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[haosdent huang]

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

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


Review request for mesos, Alexander Rukletsov and Gastón Kleiman.


Changes
---

Address @gaston's comments.


Repository: mesos


Description
---

* Replace `Testing` to `Tests` in comments.
* Remove redundant `.Times(1)`.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 

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


Testing
---

```
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
--gtest_also_run_disabled_tests
[--] Global test environment tear-down
[==] 19 tests from 1 test case ran. (35264 ms total)
[  PASSED  ] 19 tests.
```


Thanks,

haosdent huang

Re: Review Request 52516: Add test for `AGENT_ADDED` and `AGENT_REMOVED` events.

[Anand Mazumdar]

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


Fix it, then Ship it!





src/tests/api_tests.cpp (lines 1416 - 1417)


hmm, this comment is slightly misleading i.e., we are not creating the 
event stream after seeing the first offer?

It's fine to kill this comment. It's pretty self explanatory that upon 
starting a master we expect a clean state.



src/tests/api_tests.cpp (line 1496)


Nit: Kill this newline?


- Anand Mazumdar


On Oct. 5, 2016, 11:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52516/
> ---
> 
> (Updated Oct. 5, 2016, 11:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add test for `AGENT_ADDED` and `AGENT_REMOVED` events.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 7b0ad3c18a38b5ec859a79d09f5707f6958960fb 
> 
> Diff: https://reviews.apache.org/r/52516/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 52608: Reordered the list of executor env variables in code and documentation.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Reordered the list of executor env variables in code and documentation.


Diffs
-

  docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
  src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 

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


Testing
---

make check


Thanks,

Gastón Kleiman

Re: Review Request 52556: Added the MESOS_RUNTIME_DIRECTORY executor env variable.

[Gastón Kleiman]

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

(Updated Oct. 6, 2016, 4:28 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Jiang Yan Xu.


Changes
---

Addressed AlexR's comments.


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


Repository: mesos


Description (updated)
---

This environment variable points to the directory containing the
binaries used by the Mesos Agent. It will eventually replace the
`--launcher-dir` executor flag.


Diffs (updated)
-

  docs/executor-http-api.md 50b4cb4efff48bcc56330b81bd7c4c217b8a22b5 
  src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
  src/slave/slave.cpp d30001bc0d1798311ba3966f67aadc6be2c92306 

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


Testing
---

`make check`


Thanks,

Gastón Kleiman

Re: Review Request 51700: Add `FrameworkAdded` event to master event stream.

[haosdent huang]

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




src/master/http.cpp (line 1401)


`protobuf::master::event::model(*framework)` should be enough?



src/master/http.cpp (line 1411)


`protobuf::master::event::model(*framework)` should be enough?


- haosdent huang


On Oct. 4, 2016, 2:44 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51700/
> ---
> 
> (Updated Oct. 4, 2016, 2:44 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes included:
> - Moving `model()` helper function to common/protobuf_utils;
> - Definition of event and response;
> - Sending event from master;
> - new test.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 1ea79fd7ecd9c2b7e94718fc8c7779d1051583db 
>   include/mesos/v1/master/master.proto 
> 58dc65006658f47bb752ee1c0c1520ee3cc03bd7 
>   src/common/protobuf_utils.hpp 3324838f0a54491024b3e26ddf38afbfad31b8e4 
>   src/common/protobuf_utils.cpp 8c4a7264080385789157703b05b42716fb0563b3 
>   src/master/http.cpp 52dd80b856cf2317c0b73ba54bf501696786088d 
>   src/master/master.cpp d94a8510c4cee9c010706f79caf27ef4a10b41a8 
>   src/tests/api_tests.cpp e440d1b44fb0b40dd1f68209c71e5ca0cd19f4fb 
> 
> Diff: https://reviews.apache.org/r/51700/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52560: Avoided temporary `MockDocker` pointers in health check test cases.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 5, 2016, 6:11 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52560/
> ---
> 
> (Updated Oct. 5, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided temporary `MockDocker` pointers in health check test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52560/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52604: Removed flags::Flags helper template.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Oct. 6, 2016, 2:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52604/
> ---
> 
> (Updated Oct. 6, 2016, 2:58 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The template `flags::Flags` allowed to compose flags classes on the
> fly, e.g.,
> 
> flags::Flags flags;
> 
> which would create a class inheriting virtually from both `MyFlags1`
> and `MyFlags2`.
> 
> This class was not used in the code.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags.hpp 
> 21257b7ee6ed7a9a48a0ca1ad4edd8890ca2fa8f 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> eab8a001ed19755de58386a55ed4972f58026b29 
>   3rdparty/stout/tests/flags_tests.cpp 
> 94ba915c40836e476cf6097274a85c55acd4d73b 
> 
> Diff: https://reviews.apache.org/r/52604/diff/
> 
> 
> Testing
> ---
> 
> * `git grep 'Flags<'`
> * tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52390: Fully qualified addresses of Flag members in add calls in stout.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 5:06 p.m.)


Review request for mesos.


Changes
---

Rebased, and performed cleanup of stout test changes.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  3rdparty/stout/README.md 172150c586cd4020edc4dbdabffd1306c9177694 
  3rdparty/stout/include/stout/flags.hpp 
21257b7ee6ed7a9a48a0ca1ad4edd8890ca2fa8f 
  3rdparty/stout/include/stout/flags/flags.hpp 
eab8a001ed19755de58386a55ed4972f58026b29 
  3rdparty/stout/tests/flags_tests.cpp 94ba915c40836e476cf6097274a85c55acd4d73b 
  3rdparty/stout/tests/subcommand_tests.cpp 
9213d6b9faec30b5be320ab37ca29c2406c964ac 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 52516: Add test for `AGENT_ADDED` and `AGENT_REMOVED` events.

[haosdent huang]

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




src/tests/api_tests.cpp (line 1486)


Append 
```
slave->reset();
```
above?


- haosdent huang


On Oct. 5, 2016, 11:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52516/
> ---
> 
> (Updated Oct. 5, 2016, 11:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add test for `AGENT_ADDED` and `AGENT_REMOVED` events.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 7b0ad3c18a38b5ec859a79d09f5707f6958960fb 
> 
> Diff: https://reviews.apache.org/r/52516/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 46825: Fully-typed all FlagsBase::add overloads.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 4:59 p.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Fully-typed all FlagsBase::add overloads.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flags.hpp 
eab8a001ed19755de58386a55ed4972f58026b29 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 4:59 p.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  src/cli/execute.cpp fcf627b0dcbedd01700cc8c9acadf7ba0dae4faa 
  src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e 
  src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f 
  src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
  src/examples/disk_full_framework.cpp 1221f83d495f7c1ee1bcbcfe067e303db0a921eb 
  src/examples/dynamic_reservation_framework.cpp 
c9a68637ea2dfbb0a9367f14358b5e5de46f3331 
  src/examples/load_generator_framework.cpp 
5402e31b89b7ead1dc8fdc9065980b5b1c0d380c 
  src/examples/long_lived_framework.cpp 
7c57eb5e4a34208504475013690ae8e3cae74155 
  src/examples/no_executor_framework.cpp 
57425726aa5ca27c9579b0d8ecc0bb9eb9b26852 
  src/examples/persistent_volume_framework.cpp 
3cc7cf0c4c97d90f2e70800d7a8d4ca64c2150d1 
  src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d 
  src/examples/test_http_framework.cpp d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 
  src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
  src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
  src/master/main.cpp 4a1a8e70ab0535aa131681b2b09a99e51c20158e 
  src/slave/container_loggers/lib_logrotate.hpp 
a8653d716a898f03cce83f7b88b666dc46c0ea90 
  src/slave/container_loggers/logrotate.hpp 
f906a167f8897385af5f54e1e77cb790121a 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
d530a364567ae1fbfdcde921eb109acda6667c39 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
20fb6ab35ffa85917c6705d79f22f0d54a416353 
  src/slave/containerizer/mesos/launch.cpp 
7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
  src/slave/containerizer/mesos/mount.cpp 
4b90666d9c75dadd70e6b690ca17fc5699825442 
  src/slave/main.cpp 949a738ab3e44d8efc878e59c482a750ab41d1e4 
  src/tests/active_user_test_helper.cpp 
80bd0acff47689be95a42bad3cb867e0faa68554 
  src/tests/containerizer/capabilities_test_helper.cpp 
2d7be6ddaf4a5e687e928e4651df68ea9b234202 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 46823: Fully qualified addresses of Flag members in add calls in libprocess.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 4:59 p.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
c8350cf8e512dca23933725e6edb3e3d94380211 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 49829: Consistently used virtual inheritance for Flags classes.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 4:58 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

In order for different `Flags` classes to be composable classes should
always use virtual inheritance.


Diffs (updated)
-

  src/cli/execute.cpp fcf627b0dcbedd01700cc8c9acadf7ba0dae4faa 
  src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f 
  src/examples/balloon_framework.cpp 5613ff0c61e2d2f84684a206debc97dcb8b2c0d3 
  src/examples/disk_full_framework.cpp 1221f83d495f7c1ee1bcbcfe067e303db0a921eb 
  src/examples/load_generator_framework.cpp 
5402e31b89b7ead1dc8fdc9065980b5b1c0d380c 
  src/examples/long_lived_framework.cpp 
7c57eb5e4a34208504475013690ae8e3cae74155 
  src/examples/no_executor_framework.cpp 
57425726aa5ca27c9579b0d8ecc0bb9eb9b26852 
  src/examples/persistent_volume_framework.cpp 
3cc7cf0c4c97d90f2e70800d7a8d4ca64c2150d1 
  src/launcher/executor.cpp 8a1051b886a0c95d19ff370e5c77d9c4033c8b61 
  src/local/flags.hpp c77eff13e7a63630ff6c9428e57f3f6707e1f30f 
  src/log/tool/benchmark.hpp e905e85301df089e3a260bff8e51c6aae574bd46 
  src/log/tool/initialize.hpp 33d2c5c4e54cbdad30cb3102f36340b1cac3bc67 
  src/log/tool/read.hpp 7ec82759efc90385ea3d1bc36bdc8ddb95dfa489 
  src/log/tool/replica.hpp 9ca92ae1736a15b88a386cdf9f08c35253065fe5 
  src/master/flags.hpp 708a629dab39e8cf161b0dd43420fcd761044102 
  src/sched/flags.hpp 8782c7bef833cbf418a60a0d296352f1cbe63d7b 
  src/scheduler/flags.hpp 6d0e95ed375726f8ab203e5fb40ef5bd299004e5 
  src/slave/container_loggers/lib_logrotate.hpp 
a8653d716a898f03cce83f7b88b666dc46c0ea90 
  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
70f30831819d7a0e6233fcb13a703dc6981324b6 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
e852c46a5027c7c911bb7f0c9364ff830f4df086 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/mount.hpp 
fcfe8601886e4f93386f71931a514ea7f44f19d3 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/tests/active_user_test_helper.hpp 
b99d1e40acc65821b85c24cc4ac6f34c97678f9a 
  src/tests/containerizer/capabilities_test_helper.hpp 
256ee3b5a986a5c59da7f479a64c63e02884adcb 
  src/tests/flags.hpp 3066399152bf0e152f6876e001af65d4e945fb8f 
  src/uri/fetcher.hpp 949363ecdb15f5b225db7f3ed67eda52c351bfa1 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 52387: Consistently used virtual inheritance for Flags classes in libprocess.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 4:58 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

In order for different `Flags` classes to be composable classes should
always use virtual inheritance.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
c8350cf8e512dca23933725e6edb3e3d94380211 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 49833: Consistently used virtual inheritance for Flags classes in stout.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 4:58 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

In order for different `Flags` classes to be composable classes should
always use virtual inheritance.


Diffs (updated)
-

  3rdparty/stout/tests/flags_tests.cpp 94ba915c40836e476cf6097274a85c55acd4d73b 
  3rdparty/stout/tests/subcommand_tests.cpp 
9213d6b9faec30b5be320ab37ca29c2406c964ac 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 52515: Implement `AGENT_ADDED` and `AGENT_REMOVED` events.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Oct. 5, 2016, 11:47 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52515/
> ---
> 
> (Updated Oct. 5, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-6102
> https://issues.apache.org/jira/browse/MESOS-6102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implement `AGENT_ADDED` and `AGENT_REMOVED` events.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0c6405f6cfc5a69fa835428fa330d8d63778f469 
>   src/common/protobuf_utils.cpp 1f088684c566f89c85dd72eaece295c5e3d50b23 
>   src/master/http.cpp e9f9d16de803175d4f07e0486674e6150c632b02 
>   src/master/master.cpp 756ab546851952bc1de24b1f469d232237b1c01c 
> 
> Diff: https://reviews.apache.org/r/52515/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 50380: Added new benchmark test for port resources.

[Guangya Liu]

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

(Updated 十月 6, 2016, 2:56 p.m.)


Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.


Summary (updated)
-

Added new benchmark test for port resources.


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


Repository: mesos


Description (updated)
---

When run benchmark test for `ports` resources, the `-=` and `-` only
consumed about 10ms, this cannot reflect the real time of operating
1000 `ports` with `-=` and `-`.

The root cause is that the current calculation is always using same
port range, with port, the formula for `+` is `a+a+a+a+...+a==a`;
for `-`, it will be `a-a=0` and `0-a=0`.

With `0-a=0`, the code here
https://github.com/apache/mesos/blob/master/src/common/values.cpp#L544
will cause there is no validation as the `left` is empty after the
`ports` was subtracted to 0.

The fix is adding a new benchmark test for ports resources, using
a initial and a different delta resource to make sure the ports
resources will never be subtracted to 0.


Diffs (updated)
-

  src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 

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


Testing (updated)
---

make
make check


```
./bin/mesos-tests.sh --benchmark 
--gtest_filter="*Resources_Scalar_BENCHMARK_Test.Arithmetic/*"
[==] Running 3 tests from 1 test case.
[--] Global test environment set-up.
[--] 3 tests from 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test
[ RUN  ] 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test.Arithmetic/0
Took 56358us to perform 5 'total += r' operations on cpus(*):1; gpus(*):1; 
mem(*):128; disk(*):256
Took 61269us to perform 5 'total -= r' operations on cpus(*):1; gpus(*):1; 
mem(*):128; disk(*):256
Took 244070us to perform 5 'total = total + r' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
Took 269567us to perform 5 'total = total - r' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
[   OK ] 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test.Arithmetic/0 (632 ms)
[ RUN  ] 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test.Arithmetic/1
Took 4.557034secs to perform 10 'total += r' operations on cpus(0, principal_0, 
{key_0: value_0}):1; gpus(...
Took 4.883114secs to perform 10 'total -= r' operations on cpus(0, principal_0, 
{key_0: value_0}):1; gpus(...
Took 4.721756secs to perform 10 'total = total + r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 4.961495secs to perform 10 'total = total - r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
[   OK ] 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test.Arithmetic/1 (19174 ms)
[ RUN  ] 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test.Arithmetic/2
Took 45381us to perform 5 'total += r' operations on cpus(*):1; mem(*):128; 
disk(test)[persistentId:...
Took 44780us to perform 5 'total -= r' operations on cpus(*):1; mem(*):128; 
disk(test)[persistentId:...
Took 275496us to perform 5 'total = total + r' operations on cpus(*):1; 
mem(*):128; disk(test)[persistentId:...
Took 286105us to perform 5 'total = total - r' operations on cpus(*):1; 
mem(*):128; disk(test)[persistentId:...
[   OK ] 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test.Arithmetic/2 (652 ms)
[--] 3 tests from 
ResourcesScalarOperators/Resources_Scalar_BENCHMARK_Test (20458 ms total)

[--] Global test environment tear-down
[==] 3 tests from 1 test case ran. (20467 ms total)
[  PASSED  ] 3 tests.
```

```
 ./bin/mesos-tests.sh --benchmark 
--gtest_filter="Resources_Ports_BENCHMARK_Test.Arithmetic"
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from Resources_Ports_BENCHMARK_Test
[ RUN  ] Resources_Ports_BENCHMARK_Test.Arithmetic
Took 2.295047secs to perform 5000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1... with initial resources ports(*):[1001-3000]
Took 4.873829secs to perform 5000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1... with initial resources ports(*):[1001-3000]
Took 2.80647secs to perform 5000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... with initial resources 
ports(*):[1001-3000]
Took 5.119215secs to perform 5000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... with initial resources 
ports(*):[1001-3000]
[   OK ] Resources_Ports_BENCHMARK_Test.Arithmetic (15102 ms)
[--] 1 test from Resources_Ports_BENCHMARK_Test (15102 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. 

Re: Review Request 52081: Reorganized includes in containerizer.

[Benjamin Bannier]

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

(Updated Oct. 6, 2016, 4:28 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Repository: mesos


Description
---

Reorganized includes in containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 

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


Testing
---

make check


Thanks,

Benjamin Bannier

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 Oct. 6, 2016, 4:28 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Rebased.


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/containerizer.proto 
86d1859854b44f237ac2ca1ef74982b543c08d25 
  src/CMakeLists.txt ba49d81335fd47a8ced334e282eadb00657bf2c2 
  src/Makefile.am 184afb59fc42de00db85d95fff3fbc4992be7f3b 
  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/launch.cpp 
7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
  src/tests/containerizer/capabilities_isolator_tests.cpp PRE-CREATION 

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


Testing
---

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


Thanks,

Benjamin Bannier

Re: Review Request 52561: Renamed `flags` to `agentFlags` in health check test cases.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 5, 2016, 6:11 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52561/
> ---
> 
> (Updated Oct. 5, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `flags` to `agentFlags` in health check test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52561/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52560: Avoided temporary `MockDocker` pointers in health check test cases.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 5, 2016, 6:11 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52560/
> ---
> 
> (Updated Oct. 5, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided temporary `MockDocker` pointers in health check test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52560/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52563: Minor clean ups in `health_check_tests.cpp` for consistency.

[Gastón Kleiman]

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


Fix it, then Ship it!




Thanks for the cleanup! I suggested an alternative wording for most of the 
tests descriptions.


src/tests/health_check_tests.cpp (line 230)


```
// This test creates a healthy task and verifies that the healthy status is
// reflected in the status updates sent as reconcilliation answers, and in 
the
// state endpoint of both the master and the agents.
```



src/tests/health_check_tests.cpp (line 354)


```
// This test creates a healthy task and verifies that the healthy status is
// reflected in the status updates sent as reconcilliation answers, and in 
the
// state endpoint of both the master and the agents.
```



src/tests/health_check_tests.cpp (line 470)


```
// This test creates a healthy task using the Docker executor and verifies 
that
// the healthy status is reflected in the status updates sent as
// reconcilliation answers, and in the state endpoint of both the master and
// the agents.
```



src/tests/health_check_tests.cpp (line 640)


```
// This test creates a task whose health flaps, and verifies that the health
// status updates are sent to the framework and reflected in the state 
endpoint
// of both the master and the slaves.
```



src/tests/health_check_tests.cpp (line 833)


```
// This test creates a task that uses the Docker executor and whose health
// flaps. It then verifies that the health status updates are sent to the
// framework and reflected in the state endpoint of both the master and the
// slaves.
```



src/tests/health_check_tests.cpp (line 979)


```
// This test ensures that a task is killed if the number of maximum health
// check failures is reached.
```



src/tests/health_check_tests.cpp (line 1055)


```
// Tests that the task's env variables are copied to the env used to execute
// COMMAND health checks.
```



src/tests/health_check_tests.cpp (line 1163)


```
// This test creates a task with a health check command that will time out, 
and
// verifies that the health check is retried after the timeout.
```


- Gastón Kleiman


On Oct. 5, 2016, 6:11 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52563/
> ---
> 
> (Updated Oct. 5, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> * Replace `Testing` to `Tests` in comments.
> * Remove redundant `.Times(1)`.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1d1676d7259bf52cfb1e499954fa815fe7e37522 
> 
> Diff: https://reviews.apache.org/r/52563/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*HealthCheckTest.*" 
> --gtest_also_run_disabled_tests
> [--] Global test environment tear-down
> [==] 19 tests from 1 test case ran. (35264 ms total)
> [  PASSED  ] 19 tests.
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 52599: Fixed a typo for resources benchmark test.

[Guangya Liu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Fixed a typo for resources benchmark test.


Diffs
-

  src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 50551: Added benchmark test for `Resources::contains`.

[Guangya Liu]

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

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


Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added benchmark test for `Resources::contains`.


Diffs (updated)
-

  src/tests/resources_tests.cpp 3e493007d6d1d8194d07035aaa1cde28dedf2b5a 

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


Testing (updated)
---

make
make check

```
./bin/mesos-tests.sh  --benchmark 
--gtest_filter="*Resources_Contains_BENCHMARK_Test.Contains/*"
[==] Running 3 tests from 1 test case.
[--] Global test environment set-up.
[--] 3 tests from ResourcesContains/Resources_Contains_BENCHMARK_Test
[ RUN  ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/0
Took 8629us to perform 5000 'subset.contains(superset)' operations on subset 
resources cpus(*):1; mem(*):128 contains superset resources cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
Took 20378us to perform 5000 'superset.contains(subset)' operations on superset 
resources cpus(*):1; gpus(*):1; mem(*):128; disk(*):256 contains subset 
resources cpus(*):1; mem(*):128
Took 9526us to perform 5000 'subset.contains(subset)' operations on subset 
resources cpus(*):1; mem(*):128 contains subset resources cpus(*):1; mem(*):128
[   OK ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/0 (39 
ms)
[ RUN  ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/1
Took 1.165801secs to perform 5000 'subset.contains(superset)' operations on 
subset resources ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... contains 
superset resources ports(*):[1-1000]
Took 895058us to perform 5000 'superset.contains(subset)' operations on 
superset resources ports(*):[1-1000] contains subset resources ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 19.768667secs to perform 5000 'subset.contains(subset)' operations on 
subset resources ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1... contains 
subset resources ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/1 
(21831 ms)
[ RUN  ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/2
Took 326768us to perform 5000 'subset.contains(superset)' operations on subset 
resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,... contains superset 
resources cpus(*):1; gpus(*):1; mem(*):128; disk(*):256; ...
Took 1.003977secs to perform 5000 'superset.contains(subset)' operations on 
superset resources cpus(*):1; gpus(*):1; mem(*):128; disk(*):256; ... contains 
subset resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,...
Took 19.641502secs to perform 5000 'subset.contains(subset)' operations on 
subset resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,... contains 
subset resources cpus(*):1; mem(*):128; ports(*):[1-2, 4-5, 7-8,...
[   OK ] ResourcesContains/Resources_Contains_BENCHMARK_Test.Contains/2 
(20974 ms)
[--] 3 tests from ResourcesContains/Resources_Contains_BENCHMARK_Test 
(42844 ms total)

[--] Global test environment tear-down
[==] 3 tests from 1 test case ran. (42853 ms total)
[  PASSED  ] 3 tests.
```


Thanks,

Guangya Liu

Review Request 52596: Added special case when sorting hierarchically in MountInfoTable::read.

[Kevin Klues]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

It is legal to have entries in a `MountInfoTable` whose `entry.id` is
the same as `entry.parent`. This can happen (for example), if a system
boots from the network and then keeps the original `/` in RAM.
However, to avoid cycles when walking the mount hierarchy, we should
not treat these entries as children of their parent so we skip them.

This commit adds functionality to handle this case.


Diffs
-

  src/linux/fs.cpp 4b10141a49dfb3c6defdb07e295eb14cfcdd36ce 

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


Testing
---

GTEST_FILTER="" make -j40 check
GTEST_FILTER="FsTest.MountInfoTableReadSorted" src/mesos-tests


Thanks,

Kevin Klues