Review Request 61583: Bundled gRPC v1.4.2 into 3rdparty libraries.

[Chun-Hung Hsiao]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The bundled package is generated with the following command:

git clone -b v1.4.2 https://github.com/grpc/grpc.git grpc-1.4.2
(cd grpc-1.4.2 && git submodule update --init third_party/cares)
tar zcvf grpc-1.4.2.tar.gz --exclude .git grpc-1.4.2

We download gRPC from GitHub instead of using the official tarball
because we need to use `git submodule` to download dependent third-party
packages for gRPC, and the necessary git metadata does not exist in the
official tarball.

gRPC also depends on openssl and protobuf 3. Since we have already
bundled the latter, the only system dependency is openssl.


Diffs
-

  3rdparty/grpc-1.4.2.tar.gz PRE-CREATION 


Diff: https://reviews.apache.org/r/61583/diff/1/


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 61584: Fixed a typo in a test.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Aug. 10, 2017, 9:52 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61584/
> ---
> 
> (Updated Aug. 10, 2017, 9:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in a test.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp 6d212cd460322ad6e97f4cf6ef537323275b7da0 
> 
> 
> Diff: https://reviews.apache.org/r/61584/diff/1/
> 
> 
> Testing
> ---
> 
> None - not a functional change.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

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

(Updated Aug. 11, 2017, 12:12 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.


Changes
---

Addressed some comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 


Diff: https://reviews.apache.org/r/61434/diff/2/

Changes: https://reviews.apache.org/r/61434/diff/1-2/


Testing
---

https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded

Additionally rendered in MacDown.


Thanks,

Alexander Rukletsov

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Line 29 (original), 29 (patched)
> > 
> >
> > s/defines/which defines?

It was intended as a simple enumeration.


> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Lines 208 (patched)
> > 
> >
> > how is the `host` resolved? It's not necessary that it will resolve to 
> > `127.0.0.1`?

Yes, we do not resolve anything. I'll reword to "set".


> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Line 267 (original), 496 (patched)
> > 
> >
> > A bit confused here? I thought only `Health checks` are supported for 
> > `docker executor` and not `checks`?

Right, but health checks are using checks underneath.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 9, 2017, 12:47 a.m., Gastón Kleiman wrote:
> > docs/health-checks.md
> > Lines 136 (patched)
> > 
> >
> > Actually... only one status update is sent after a success, but 
> > failures are NOT deduplicated, see:
> > 
> > 
> > https://github.com/apache/mesos/blob/86e635b9f11e441bce3901a941c2d52b20518dbb/src/tests/health_check_tests.cpp#L990-L1076

Good catch!


> On Aug. 9, 2017, 12:47 a.m., Gastón Kleiman wrote:
> > docs/health-checks.md
> > Line 273 (original), 501 (patched)
> > 
> >
> > s/docker/Docker/

We agreed not to capitalize docker for docker containerizer and docker executor.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 4, 2017, 7:18 p.m., Vinod Kone wrote:
> > docs/health-checks.md
> > Lines 209 (patched)
> > 
> >
> > Is it 127.0.0.1 even in the CNI network case? cc @avinash

Yes, however, there is no resolution in this case happening.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59146: Added a 'UNKNOWN' field to the Update.State enumeration.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 10, 2017, 3:09 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59146/
> ---
> 
> (Updated May 10, 2017, 3:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 11d01df114eafb729514dab64596c5f1ecf09f26 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> 97a85ce34eeecf86d14c097712867cd24624a6ec 
> 
> 
> Diff: https://reviews.apache.org/r/59146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 61588: Added a `--skip-style-check` option when applying reviews.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Kevin Klues.


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


Repository: mesos


Description
---

Added a `--skip-style-check` option when applying reviews.


Diffs
-

  support/apply-reviews.py aa2eb8d23bed52a4f33510c53ace2c63bd66ec49 


Diff: https://reviews.apache.org/r/61588/diff/1/


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 60088: CLI: Added 'masters' key as an acceptable key in config.toml.

[Armand Grillet]

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

(Updated Aug. 10, 2017, 11:19 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

masters instead of master.


Summary (updated)
-

CLI: Added 'masters' key as an acceptable key in config.toml.


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


Repository: mesos


Description (updated)
---

This key is a dictionnary of masters, each
master includes two keys: 'ip' and 'port'.
It will be used by future plugins.


Diffs (updated)
-

  src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
  src/python/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5 


Diff: https://reviews.apache.org/r/60088/diff/4/

Changes: https://reviews.apache.org/r/60088/diff/3-4/


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Re: Review Request 61222: Added V1 teardown Call.

[Greg Mann]

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




src/master/http.cpp
Lines 3766-3777 (original), 3776-3785 (patched)


Let's replace this code with

```
return __teardown(id);
```

and rename the original `_teardown()` continuation to `__teardown()`. This 
will let us avoid duplicating the `master->removeFramework` call.


- Greg Mann


On Aug. 3, 2017, 5:56 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61222/
> ---
> 
> (Updated Aug. 3, 2017, 5:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-6846
> https://issues.apache.org/jira/browse/MESOS-6846
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added V1 call support for teardown operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 0e4c30e3df704a929a3bd2e1787a76421d14a983 
>   include/mesos/v1/master/master.proto 
> c04fd1602396a58086331f5fa56518c5dee9af89 
>   src/master/http.cpp 9df086c417a9392f62d600c7a6486be0a1cf7e70 
>   src/master/master.hpp 84465af782d4024f22463d981ef9d0ef7827d043 
>   src/master/validation.cpp 4885339b2112b6dbdc930875c20e1f5872b1edbf 
> 
> 
> Diff: https://reviews.apache.org/r/61222/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61517: Refactored OpenSSL library checks in libprocess.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 9, 2017, 11:48 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61517/
> ---
> 
> (Updated Aug. 9, 2017, 11:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
> https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled libprocess build with gRPC support. Also fixed an error
> that a bad linker might link a configure test with libssl unnecessarily,
> which would cause a runtime failure if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac 29b69b97428a8d5fead09e507ae0e98a46761464 
> 
> 
> Diff: https://reviews.apache.org/r/61517/diff/4/
> 
> 
> Testing
> ---
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 10, 2017, 9:58 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61433/
> ---
> 
> (Updated Aug. 10, 2017, 9:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
> https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
> a bad linker might link the configure tests with libssl unnecessarily,
> which would cause runtime failures if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -
> 
>   configure.ac 307f0aea7f19932befba37c5467851718d317c92 
> 
> 
> Diff: https://reviews.apache.org/r/61433/diff/6/
> 
> 
> Testing
> ---
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61408: Added test cases for V1 teardown Call.

[Quinn Leng]

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

(Updated Aug. 10, 2017, 10:47 p.m.)


Review request for mesos, Anand Mazumdar and Greg Mann.


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


Repository: mesos


Description
---

Added test cases for teardown operation in V1 operator API.


Diffs (updated)
-

  src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 


Diff: https://reviews.apache.org/r/61408/diff/2/

Changes: https://reviews.apache.org/r/61408/diff/1-2/


Testing
---

make check


Thanks,

Quinn Leng

Re: Review Request 61576: Added a patch for building/installing the bundled gRPC library.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 10, 2017, 8:52 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61576/
> ---
> 
> (Updated Aug. 10, 2017, 8:52 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7749
> https://issues.apache.org/jira/browse/MESOS-7749
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is cherry-picked from the following 2 commits in gRPC's
> master branch and 2 PR's with at least one approval:
> 
> https://github.com/grpc/grpc/commit/807693b
> https://github.com/grpc/grpc/commit/43f7acf
> https://github.com/grpc/grpc/pull/11558
> https://github.com/grpc/grpc/pull/11565
> 
> 
> Diffs
> -
> 
>   3rdparty/grpc-1.4.2.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61576/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61578: Updated the CPP linter to ignore patch files for gRPC.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Aug. 10, 2017, 9:54 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61578/
> ---
> 
> (Updated Aug. 10, 2017, 9:54 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: mesos-7749
> https://issues.apache.org/jira/browse/mesos-7749
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the CPP linter to ignore patch files for gRPC.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py cf37d9f4da4ab90b92f0136a1dcd5dd8bbae5785 
> 
> 
> Diff: https://reviews.apache.org/r/61578/diff/2/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61260: Added agent garbage collection metrics.

[James Peach]

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

(Updated Aug. 10, 2017, 10 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added some basic sandbox garbage collection metrics to track the number
of paths removals, the number of failed path removal and the number of
paths that are still scheduled for removal.


Diffs (updated)
-

  src/slave/gc.cpp f270fa47b22904e926f4bd4b0a7693a41b2c8271 
  src/slave/gc_process.hpp PRE-CREATION 
  src/tests/gc_tests.cpp d4daad3993bb9a92db2c7add6e74f12aeb71d535 


Diff: https://reviews.apache.org/r/61260/diff/2/

Changes: https://reviews.apache.org/r/61260/diff/1-2/


Testing
---

make check (Fedora 26).


Thanks,

James Peach

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

[Chun-Hung Hsiao]

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

(Updated Aug. 10, 2017, 9:58 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
a bad linker might link the configure tests with libssl unnecessarily,
which would cause runtime failures if libssl is not in the runtime
library search path.


Diffs (updated)
-

  configure.ac 307f0aea7f19932befba37c5467851718d317c92 


Diff: https://reviews.apache.org/r/61433/diff/6/

Changes: https://reviews.apache.org/r/61433/diff/5-6/


Testing
---

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61531: Fixed the device number proto 'major' and 'minor' to avoid MACROs.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Aug. 9, 2017, 10:24 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> ---
> 
> (Updated Aug. 9, 2017, 10:24 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568 
>   src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> 96014b560377b5c716e30781de18d34c2ea8e17b 
> 
> 
> Diff: https://reviews.apache.org/r/61531/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61578: Updated the CPP linter to ignore patch files for gRPC.

[Chun-Hung Hsiao]

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

(Updated Aug. 10, 2017, 9:54 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Updated the CPP linter to ignore patch files for gRPC.


Diffs (updated)
-

  support/mesos-style.py cf37d9f4da4ab90b92f0136a1dcd5dd8bbae5785 


Diff: https://reviews.apache.org/r/61578/diff/2/

Changes: https://reviews.apache.org/r/61578/diff/1-2/


Testing
---

N/A


Thanks,

Chun-Hung Hsiao

Review Request 61584: Fixed a typo in a test.

[Gastón Kleiman]

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

Review request for mesos, Alexander Rukletsov and Vinod Kone.


Repository: mesos


Description
---

Fixed a typo in a test.


Diffs
-

  src/tests/fetcher_cache_tests.cpp 6d212cd460322ad6e97f4cf6ef537323275b7da0 


Diff: https://reviews.apache.org/r/61584/diff/1/


Testing
---

None - not a functional change.


Thanks,

Gastón Kleiman

Re: Review Request 61408: Added test cases for V1 teardown Call.

[Quinn Leng]

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




src/tests/api_tests.cpp
Lines 2739-2758 (patched)


check in the unversioned teardown call test case, if it doesn't contain 
authorization test, add it here.


- Quinn Leng


On Aug. 3, 2017, 5:57 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61408/
> ---
> 
> (Updated Aug. 3, 2017, 5:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-6846
> https://issues.apache.org/jira/browse/MESOS-6846
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for teardown operation in V1 operator API.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 1d5b080c809248bdf4c76ddad382d714692c804b 
> 
> 
> Diff: https://reviews.apache.org/r/61408/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61511: Improved the readability of some assertions/expectations.

[Gastón Kleiman]

> On Aug. 9, 2017, 3:21 p.m., Benjamin Bannier wrote:
> > src/tests/api_tests.cpp
> > Line 1 (original), 1 (patched)
> > 
> >
> > Searching with
> > 
> > $ git grep -E '(EXPECT|ASSERT)_(EQ|NE).*(0u,\s.*size())'
> > 
> > I still get ~50 hits. Since you are at it, would you mind adjusting 
> > also?
> 
> Gastón Kleiman wrote:
> I had done a similar search, and as far as I can see, the other matches 
> are either false possitives or in stout/libprocess:
> 
> ```
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().completed_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_agents().agents_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> v1Response->get_agents().agents_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> v1Response->get_agents().recovered_agents_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_agents().agents_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().launched_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_frameworks().completed_frameworks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().completed_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().completed_executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().launched_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().executors_size());
> ```
> 
> I don't think we can use `empty()` here.
> 
> And in the following match `manifest` is not a real container:
> 
> ```
> src/tests/containerizer/docker_spec_tests.cpp:  EXPECT_EQ(0u, 
> manifest->size());
> ```
> 
> I'll drop this issue and add two more patches for stout and libprocess to 
> the chain.
> 
> Benjamin Bannier wrote:
> In principle we are still able to explicitly check emptiness here, e.g., 
> we can replace 
> 
> ASSERT_EQ(0u, getState.get_tasks().tasks_size());
> 
> with 
> 
> ASSERT_TRUE(getState.get_tasks().tasks().empty());
> 
> There is some trade-off between local and global consistency in most of 
> the cases in e.g., `api_tests.cpp`. When checking proto fields there, we 
> often mix checking emptiness and checks for a particular size. I would go 
> with a checks capturing our intention as much as possible, i.e., prefer 
> checking for emptiness over local typographic consistency.

I fixed this, I also made the `u` in `0u` optional running: `git grep -E 
'(EXPECT|ASSERT)_(EQ|NE).*(0u,\s.*size())'`

This returned a lot of matches that I had initially missed. I fixed 'em all =).


- Gastón


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


On Aug. 10, 2017, 9:10 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61511/
> ---
> 
> (Updated Aug. 10, 2017, 9:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 

Re: Review Request 61511: Improved the readability of some assertions/expectations.

[Gastón Kleiman]

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

(Updated Aug. 10, 2017, 9:10 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.


Changes
---

Used `git grep -E '(EXPECT|ASSERT)_(EQ|NE).*(0u?,\s.*size())'` to find even 
mor occurences.


Repository: mesos


Description
---

Prefer checking whether a container is empty instead of checking its
size.


Diffs (updated)
-

  src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
  src/tests/check_tests.cpp 0810851cf1811f2b4f511f5ca49fe372fd0bac82 
  src/tests/container_logger_tests.cpp 2ebff809636fbf68c9e3fb87b98f168338f80d6d 
  src/tests/containerizer/appc_spec_tests.cpp 
9bc82531dbb5f10b3d17f26609867c909d86816d 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
67242119f2d10f6f3010c374ea58138e40e4a33e 
  src/tests/containerizer/cgroups_tests.cpp 
506fc7f00dc1ac48476334de930b720b85a691dc 
  src/tests/containerizer/cni_isolator_tests.cpp 
60c85adab11af06be474661544957ca20b7de8c3 
  src/tests/containerizer/cpu_isolator_tests.cpp 
0e86318bb7e261ac00be19f6405557f29a2e92af 
  src/tests/containerizer/docker_containerizer_tests.cpp 
fa91bcf8d4fdcda44fb5607d99b86a6323096244 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
866af61f8669163ff3ddd10ed7ecb655568f8250 
  src/tests/containerizer/environment_secret_isolator_tests.cpp 
b034ceec7d1bf92db8a1d344835ad48ea2d24952 
  src/tests/containerizer/io_switchboard_tests.cpp 
742143a0f093e415fe98235bbd25342fd65e0483 
  src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
6d95d607b81358953a4afcec03b60e87e7192edd 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
f9cab2fd123e48a5b4387fb2b609d70c7bf535cd 
  src/tests/containerizer/memory_isolator_tests.cpp 
b7b7acd37f69832e9ba79c9fe7ed0bc473b53e02 
  src/tests/containerizer/memory_pressure_tests.cpp 
8a43c4fada8bdcedc6e1e24833d0d6c94cf927d2 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
1fc56c4a8ca95bb47af968ad72dfef69455e7d46 
  src/tests/containerizer/port_mapping_tests.cpp 
84b39b15880c7b5d9b7967f1e686baa59f52a015 
  src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
bf25049515e8b8d0c085be24c4de22b3ade261b2 
  src/tests/containerizer/routing_tests.cpp 
d05b3b11dc5fcd54d956990f252509865168e6b6 
  src/tests/containerizer/runtime_isolator_tests.cpp 
fbca31a4da1c83574cce7414fe5e03b1f86591cb 
  src/tests/containerizer/xfs_quota_tests.cpp 
c220a6ba5b274892bee195b26a5069d0750b4cb2 
  src/tests/default_executor_tests.cpp b9776314a8781963b92ba9ac297654f61a443bc8 
  src/tests/dynamic_weights_tests.cpp 3c86325a0749112606683bffdf305661170493e5 
  src/tests/exception_tests.cpp b5b657c2d1b31b59ab1554bf854ef1a69b1517ed 
  src/tests/fault_tolerance_tests.cpp 5b8213531f5688e94063937af19e674508f0dd8c 
  src/tests/fetcher_cache_tests.cpp 6d212cd460322ad6e97f4cf6ef537323275b7da0 
  src/tests/group_tests.cpp f2e37cc7184dd328466b554a16da3173cfbca4a7 
  src/tests/health_check_tests.cpp e824d1b2c2b17423394da0b089ae8940450378b7 
  src/tests/hook_tests.cpp 04edbf809b7289a7d5336e4f00f23dfb55d3dbf8 
  src/tests/http_fault_tolerance_tests.cpp 
8fcd56d86dcbdd181864756187beb4ff2ac1ff2a 
  src/tests/master_authorization_tests.cpp 
8d54472d8a31956e6e8bb5fe7ffbf47dc793c0bc 
  src/tests/master_maintenance_tests.cpp 
e7a80ff4199927df8bf0fb54458d357ae444260d 
  src/tests/master_quota_tests.cpp 9d52f767c112abd55ab3f49d172eb6e3caea511b 
  src/tests/master_slave_reconciliation_tests.cpp 
9c31eeae1a1b67af142a01e6c548b509ba06740c 
  src/tests/master_tests.cpp e6ed02e07a9860ca3a56bba9c502a09444c5b26e 
  src/tests/master_validation_tests.cpp 
813fb25007973f3499b94dcd0e9d2184ba08634c 
  src/tests/oversubscription_tests.cpp 54535a35e33dde0db3b547b9e31b4545d2900b67 
  src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
  src/tests/persistent_volume_tests.cpp 
7b0f436aa270f64cde93adc58833eef97c73dc26 
  src/tests/reconciliation_tests.cpp f1635fd627d9a7944cb9d4b0117fcded7b6693e1 
  src/tests/registrar_tests.cpp e2c38d32bc5882b95bbcf1da2e849e4d5ec6a9af 
  src/tests/registrar_zookeeper_tests.cpp 
cc082d320b4b150717728ec48edd0b26169cecd7 
  src/tests/resource_offers_tests.cpp e1fcab4b8fbb625876cf246505db2d05ac5d5710 
  src/tests/scheduler_driver_tests.cpp 47ad1dc9a4e5e0c5444fcc3a90d4645c3b6013ca 
  src/tests/scheduler_tests.cpp 5c42dc00f177a61d6ee595c26255492aa07aaad9 
  src/tests/slave_authorization_tests.cpp 
4e55148af77e4aae5829a056b0599954277758bb 
  src/tests/slave_recovery_tests.cpp 9ba6f6005c2edce67e1f63005518ab71ac981423 
  src/tests/slave_tests.cpp 896591527f20ae3697fcc98f66a3cbb97f2ec33c 
  src/tests/state_tests.cpp 7434473795d5122061eca9f7e62e87ae84af3133 
  src/tests/status_update_manager_tests.cpp 
710723c2a6ff7b72fce5d1d9ac69bf351e37a2ff 
  src/tests/upgrade_tests.cpp 7c900b17e5c895ea3a842f0f5b05b3a2876e61dd 


Diff: 

Re: Review Request 61262: Added 'heartbeat' event for the operator API.

[Quinn Leng]

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




src/master/master.hpp
Lines 1826 (patched)


Move the templated Heartbeater before these usages in subscriber and 
framework.



src/master/master.hpp
Line 1830 (original), 1874 (patched)


don't have to pass in the uuid, also not necessary to keep it as a class 
memeber.



src/tests/api_tests.cpp
Lines 2189 (patched)


put int into the for bracket.


- Quinn Leng


On July 31, 2017, 5:30 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61262/
> ---
> 
> (Updated July 31, 2017, 5:30 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-7695
> https://issues.apache.org/jira/browse/MESOS-7695
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the 'HEARTBEAT' event for the operator API, modified other
> related test cases to accept heartbeats.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 0e4c30e3df704a929a3bd2e1787a76421d14a983 
>   include/mesos/v1/master/master.proto 
> c04fd1602396a58086331f5fa56518c5dee9af89 
>   src/master/http.cpp 9df086c417a9392f62d600c7a6486be0a1cf7e70 
>   src/master/master.hpp 84465af782d4024f22463d981ef9d0ef7827d043 
>   src/master/master.cpp 7bb3adf48431cd97b3c404b8a1eba4ac6a01efcc 
>   src/tests/api_tests.cpp 1d5b080c809248bdf4c76ddad382d714692c804b 
> 
> 
> Diff: https://reviews.apache.org/r/61262/diff/3/
> 
> 
> Testing
> ---
> 
> make check -j48
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Jiang Yan Xu]

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



Some of the comments below were made before I started to feel that we are 
probably doing too many conversions to justify storing these tasks in 
TASK_UNREACHABLE. Perhaps we can just store them in 
`Framework.unreachableTasks` but in TASK_LOST state?

It's possible that we can add another map `BoundedHashMap unreachableNonPartitionAwareTasks;` for these tasks but 
it's clunky in the sense that you have to clarify that `unreachableTasks` is 
only for partition aware tasks but in fact all of these tasks belong to the 
same framework which is either parition aware or not, however with the 
possibility of changing capability... so it's probably easier to describe 
things if we just put all of them in `unreachableTasks` and simply say that "if 
the framework is not partition-aware, the tasks stored in `unreachableTasks` 
may be in `TASK_LOST`".

If we do that, then some of the comments below don't apply any more but I am 
keep them just for posterity (some styling issues etc).


src/master/http.cpp
Lines 342 (patched)


One empty line above.



src/master/http.cpp
Lines 3203-3206 (original), 3215-3227 (patched)


Should we only loop through the once? And in order to try not to duplicate 
similar lines, consider the following?

```
foreachvalue (const Owned& _task, framework->unreachableTasks) {
  Owned task = _task;

  if (framework->capabilities.partitionAware) {
task = ...
  }
  
  frameworkTaskSummaries[frameworkId].count(*task.get());
  slaveTaskSummaries[task->slave_id()].count(*task.get());
}
```



src/master/http.cpp
Lines 4017-4027 (original), 4038-4063 (patched)


Similar to above, don't duplicate lines.



src/master/http.cpp
Lines 4167-4175 (original), 4203-4221 (patched)


Ditto.



src/master/master.hpp
Lines 2403-2408 (patched)


Can we hold off creating a helper for this? IMO this helper is doing too 
little and not so much of an "abstraction", i.e., what the method does it not 
perfectly captured by the name of the mehod. Inlining 1) making a copy & 2) 
setting the state is not too much redudancy than calling this method.



src/master/master.hpp
Lines 2769-2771 (original), 2773-2775 (patched)


Fix the comment.



src/master/master.cpp
Lines 6398-6414 (original), 6389-6405 (patched)


Fix the comments.



src/master/master.cpp
Lines 6422 (patched)


`erase` can handle the `!contains` case.



src/master/master.cpp
Lines 6477-6480 (original), 6464-6467 (patched)


Fix the comment.



src/master/master.cpp
Line 7137 (original), 7112 (patched)


If our code stops making such distinction, I don't think the comment should 
continue to make such distinction.



src/master/master.cpp
Lines 7132-7137 (patched)


Do this only when we actually send an update i.e., `framework->connected()`?



src/master/master.cpp
Lines 8928-8931 (original), 8907-8910 (patched)


So for this update event we are going to send TASK_UNREACHABLE instead 
TASK_LOST, which is unintended?


- Jiang Yan Xu


On Aug. 10, 2017, 9:07 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 10, 2017, 9:07 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework 

Review Request 61576: Added a patch for building/installing the bundled gRPC library.

[Chun-Hung Hsiao]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This patch is cherry-picked from the following 2 commits in gRPC's
master branch and 2 PR's with at least one approval:

https://github.com/grpc/grpc/commit/807693b
https://github.com/grpc/grpc/commit/43f7acf
https://github.com/grpc/grpc/pull/11558
https://github.com/grpc/grpc/pull/11565


Diffs
-

  3rdparty/grpc-1.4.2.patch PRE-CREATION 


Diff: https://reviews.apache.org/r/61576/diff/1/


Testing
---

N/A


Thanks,

Chun-Hung Hsiao

Review Request 61578: Updated the CPP linter to ignore patch files for gRPC.

[Chun-Hung Hsiao]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Updated the CPP linter to ignore patch files for gRPC.


Diffs
-

  support/mesos-style.py 77a4e855a5595b3933fd271919a56a869378b742 


Diff: https://reviews.apache.org/r/61578/diff/1/


Testing
---

N/A


Thanks,

Chun-Hung Hsiao

Re: Review Request 61530: Enabled retries for `killTasks` in docker executor.

[Alexander Rukletsov]

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


Ship it!




Great job, great testing. Thanks a lot!

- Alexander Rukletsov


On Aug. 10, 2017, 4:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61530/
> ---
> 
> (Updated Aug. 10, 2017, 4:14 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-6743
> https://issues.apache.org/jira/browse/MESOS-6743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, after `docker stop` command failure, docker executor
> neither allowed a scheduler to retry `killTask` command, nor retried to
> kill a task when task killing was triggered by a failed health check.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 
> 
> 
> Diff: https://reviews.apache.org/r/61530/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Manual testing:
> 
> Emulating `docker stop` errors:
> ===
> 1. Add `return fmt.Errorf("Emulating error!")` to 
> https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
> 2. build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 3. stop docker service and launch dockerd binary, like: `sudo 
> ./bundles/17.06.0-dev/binary-daemon/dockerd`
> 
> Emulating docker daemon hang:
> =
> 1. `ps aux|grep dockerd` - 2 processes will be found
> 2. `sudo kill -STOP  ` - send SIGSTOP to docker daemon processes 
> just before sending `docker stop`
> 
> Emulating health check failure in docker executor:
> ==
> 1. Add
> ```c++
>   static int fake = 0;
>   if (++fake > 10) {
> failure();
> return;
>   }
> ```
> to `HealthChecker::processCheckResult()` in `src/checks/health_checker.cpp`
> 2. Add
> ```c++
>HealthCheck healthCheck;
>healthCheck.set_type(HealthCheck::COMMAND);
>healthCheck.mutable_command()->set_value("exit 0");
>healthCheck.set_delay_seconds(0);
>healthCheck.set_interval_seconds(0);
>healthCheck.set_grace_period_seconds(1);
>_task.mutable_health_check()->CopyFrom(healthCheck);
> ```
> to `CommandScheduler::offers()` in `src/cli/execute.cpp`
> 3. compile mesos
> 4. run mesos agent: `sudo GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/some_user/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 5. launch docker executor: `./src/mesos-execute --master="127.0.1.1:5050" 
> --name="a" --containerizer=docker --docker_image="ubuntu:xenial" 
> --command="while true; do : ; done"`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61580: Extracted JNI code into a protected function for clarity.

[Alexander Rukletsov]

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

(Updated Aug. 10, 2017, 8:41 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
1f58fbff4e8414e4d2ae4c8f69b637ee3315e411 


Diff: https://reviews.apache.org/r/61580/diff/1/


Testing
---

make check


Thanks,

Alexander Rukletsov

Review Request 61580: Extracted JNI code into a protected function for clarity.

[Alexander Rukletsov]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Extracted JNI code into a protected function for clarity.


Diffs
-

  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
1f58fbff4e8414e4d2ae4c8f69b637ee3315e411 


Diff: https://reviews.apache.org/r/61580/diff/1/


Testing
---

make check


Thanks,

Alexander Rukletsov

Review Request 61579: Ensured JAVA HTTP adapter propagates a subscription error.

[Alexander Rukletsov]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Prior to this patch, if an error occurred during subscription /
registration to the master, it was not propagated back to the
scheduler if the HTTP adapter was used. This happened because
the HTTP adapter does not call `scheduler.connected` until after
successful registration and hence the scheduler does not try to
send the `SUBSCRIBE` call, without which the adapter does not
send any events to the scheduler.

A fix is to call `scheduler.connected` if an error occurred
before the scheduler had subscribed.


Diffs
-

  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
1f58fbff4e8414e4d2ae4c8f69b637ee3315e411 


Diff: https://reviews.apache.org/r/61579/diff/1/


Testing
---

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


Thanks,

Alexander Rukletsov

Review Request 61570: Added the `kill()` function to the containerizer interface.

[Anand Mazumdar]

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

Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

This would be used by now on for killing a container by sending
a signal to it similar to the linux equivalent `kill()` system call.


Diffs
-

  src/slave/containerizer/containerizer.hpp 
0954ed6175e4c8f963bf371e54e0f9ffe7bc9c1c 
  src/slave/containerizer/mesos/containerizer.hpp 
fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
  src/slave/containerizer/mesos/containerizer.cpp 
ff192bb085f3554ad1b1f20fb73bf827bf04ef13 


Diff: https://reviews.apache.org/r/61570/diff/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 61571: Added `kill()` call to the composing containerizer.

[Anand Mazumdar]

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

Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

Added `kill()` call to the composing containerizer.


Diffs
-

  src/slave/containerizer/composing.hpp 
bef6d880ca1d815fc21d9d017f6de2d26ad5218f 
  src/slave/containerizer/composing.cpp 
a003e1b80dc9b4dec5b3fbbadb2daecf855c90c7 


Diff: https://reviews.apache.org/r/61571/diff/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 61572: Made `killNestedContainer()` use `kill()` on the containerizer.

[Anand Mazumdar]

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

Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

Instead of invoking `destroy()` directly, `killNestedContainer()`
would invoke `kill()` to terminate the nested container.


Diffs
-

  src/slave/http.cpp 2d33f0b498c8c819d1aaa6b39ae38b1009fda3e4 


Diff: https://reviews.apache.org/r/61572/diff/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 61573: Made the default executor support signal escalation.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This modifies the default executor to perform signal escalation
via the 'KILL_NESTED_CONTAINER' call i.e., send a SIGTERM followed
by a SIGKILL after some grace period. Note that support for kill
policies still needs to be done instead of using a constant grace
period.


Diffs
-

  src/launcher/default_executor.cpp c25cc941eefc0cca998a99d76497bfdd05babe92 


Diff: https://reviews.apache.org/r/61573/diff/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 61569: Added the field `signal` to the `KillNestedContainer` call.

[Anand Mazumdar]

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

Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

This would be used later for sending signals (SIGTERM, SIGKILL etc.)
to the running container. Previously, SIGKILL was used by default.


Diffs
-

  include/mesos/agent/agent.proto 9bac9541acd24e1123ca5dd5925e2a1381d13b4a 
  include/mesos/v1/agent/agent.proto ea9282cf12fbe1c2ddeaa37223e4811685263734 


Diff: https://reviews.apache.org/r/61569/diff/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 61574: Added `kill()` support to the test containerizer interface.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is needed to make some tests pass that rely on the test
containerizer.


Diffs
-

  src/tests/containerizer.hpp 4bd40c32625bc1f7998d523c6ee81bf78ac74538 
  src/tests/containerizer.cpp 1d2b6391cf7a7545fa44206c59d05764f3e8cdfb 


Diff: https://reviews.apache.org/r/61574/diff/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 61575: Added a test for verifying signal escalation on the default executor.

[Anand Mazumdar]

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

Review request for mesos, Jie Yu and Vinod Kone.


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


Repository: mesos


Description
---

This test uses the kill policy helper and blocks the SIGTERM signal.


Diffs
-

  src/tests/default_executor_tests.cpp b9776314a8781963b92ba9ac297654f61a443bc8 


Diff: https://reviews.apache.org/r/61575/diff/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 61189: Added authorization for V1 events.

[Greg Mann]

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




src/master/master.hpp
Lines 1835 (patched)


Does `Subscriber` need to store the authorizer? Can it just do 
`master->authorizer`?



src/master/master.cpp
Lines 9586 (patched)


Perhaps we could make this a member function of `Subscriber` and eliminate 
the `subscriber` parameter? Then you could do `subscriber->send(...)` here.


- Greg Mann


On July 27, 2017, 6:25 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61189/
> ---
> 
> (Updated July 27, 2017, 6:25 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Alexander Rojas, and Greg Mann.
> 
> 
> Bugs: MESOS-7785
> https://issues.apache.org/jira/browse/MESOS-7785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization filtering for V1 streaming events, the
> subscriber should only receive events that are authorized
> based on their principal and ACLs.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9df086c417a9392f62d600c7a6486be0a1cf7e70 
>   src/master/master.hpp 84465af782d4024f22463d981ef9d0ef7827d043 
>   src/master/master.cpp e12c997dad04f8a4ddb47a993a84b2b05c9e2f32 
>   src/tests/api_tests.cpp f22ca28c819712d8797e0c0c69dc1ebf1fe5ac1f 
> 
> 
> Diff: https://reviews.apache.org/r/61189/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> GLOG_v=2 ./bin/mesos-tests.sh 
> --gtest_filter="ContentType/MasterAPITest.EventsAuthorizationFiltering*" 
> --verbose --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>

Review Request 61565: Stout: Improved the readability of some assertions/expectations.

[Gastón Kleiman]

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

Review request for mesos.


Repository: mesos


Description
---

Prefer checking whether a container is empty instead of checking its
size.


Diffs
-

  3rdparty/stout/tests/flags_tests.cpp d78ab55b983ff6918733617ae8db5be29f817c69 
  3rdparty/stout/tests/multimap_tests.cpp 
36860e43efab04cf8a2276523e6937c1918d5a90 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
841f655a19e87657de77cb57c30b8310bb5898a4 
  3rdparty/stout/tests/os/process_tests.cpp 
4b5c25782dd67624bf139ad826142183a6d6f1a2 
  3rdparty/stout/tests/os_tests.cpp e8ecece1d5361e8d0e1ad42682db51b48ab6be1f 
  3rdparty/stout/tests/proc_tests.cpp c6d1d442df3e38d1d89716dc036b4a57ab0941b1 
  3rdparty/stout/tests/strings_tests.cpp 
a51144df652c5d456d8dab49ca8b2cbec69ea4b6 


Diff: https://reviews.apache.org/r/61565/diff/1/


Testing
---

The stout tests still pass.


Thanks,

Gastón Kleiman

Review Request 61564: Libprocess: Improved the readability of some assertions/expectations.

[Gastón Kleiman]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.


Repository: mesos


Description
---

Prefer checking whether a container is empty instead of checking its
size.


Diffs
-

  3rdparty/libprocess/src/tests/decoder_tests.cpp 
5742c83c632a2f03b4935738c3e78f39edc33e6d 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
d71fa4b9619a5fb5b8b8cae3310c36aaefc878ae 


Diff: https://reviews.apache.org/r/61564/diff/1/


Testing
---

The libprocess tests still pass.


Thanks,

Gastón Kleiman

Re: Review Request 60088: CLI: Added 'master' key as an acceptable key in config.toml.

[Kevin Klues]

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




src/python/cli_new/README.md
Lines 81 (patched)


Can we make this a list of masters instead of a single master?


- Kevin Klues


On July 28, 2017, 12:54 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> ---
> 
> (Updated July 28, 2017, 12:54 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This key is a dictionnary that includes two keys: 'ip' and 'port'.
> It will be used by future plugins.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
>   src/python/cli_new/lib/cli/config.py 
> 36a32f94bb12a033705b20f3c91d8bba972ba6c5 
> 
> 
> Diff: https://reviews.apache.org/r/60088/diff/3/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 61204: CLI: Added 'mesos task list' command.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This command displays all the tasks running in a Mesos cluster.


Testing
---

```
$ ./bootstrap
$ source activate
$ mesos-cli-tests
```

I also checked that the Python linter was still working.


Thanks,

Armand Grillet

Re: Review Request 60235: Linted support/test-upgrade.py.

[Joseph Wu]

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


Ship it!




Only some slight changes in error messages if/when the test framework fails.  
Not a big deal though, as we can glean the version of the agent/master from the 
other printed messages.

- Joseph Wu


On July 29, 2017, 2:03 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60235/
> ---
> 
> (Updated July 29, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-6390
> https://issues.apache.org/jira/browse/MESOS-6390
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will allow us to use PyLint on the
> entire support directory in the future.
> 
> 
> Diffs
> -
> 
>   support/test-upgrade.py 84df21dd6e63653e1a18e700ef904aa3a04b2b45 
> 
> 
> Diff: https://reviews.apache.org/r/60235/diff/3/
> 
> 
> Testing
> ---
> 
> Added `support` to `source_dirs` in the PyLinter defined
> in `mesos-style.py`. Linted until only acceptable errors
> where displayed (e.g. due to the name of the file containing
> a dash instead of an underscore).
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61111: Extracted strings into constants in example frameworks.

[Alexander Rukletsov]

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


Fix it, then Ship it!




I will fix outstanding issues and commit it for you.


src/examples/disk_full_framework.cpp
Line 59 (original), 60-61 (patched)


Let's keep the original double blank line space.



src/examples/load_generator_framework.cpp
Line 44 (original), 44-45 (patched)


Extra blank line?



src/examples/long_lived_framework.cpp
Lines 100 (patched)


`ExecutorInfo.source` is deprecated since Mesos 1.0. Let's remove it 
altogether. As a separate commit.



src/examples/long_lived_framework.cpp
Line 98 (original), 102-103 (patched)


Let's keep the original double blank line



src/examples/long_lived_framework.cpp
Line 584 (original), 592 (patched)


This field is deprecated since Mesos 1.0. Let's remove it altogether.



src/examples/no_executor_framework.cpp
Lines 46 (patched)


Missing `;` at the end.



src/examples/persistent_volume_framework.cpp
Line 55 (original), 55-56 (patched)


Ditto



src/examples/test_framework.cpp
Lines 56 (patched)


Ditto



src/examples/test_framework.cpp
Lines 58 (patched)


Missing []?



src/examples/test_framework.cpp
Lines 225-227 (original), 231-233 (patched)


```
uri = 
  path::join(os::realpath(Path(argv[0]).dirname()).get(), 
EXECUTOR_BINARY);
``` 
?



src/examples/test_framework.cpp
Line 255 (original), 261 (patched)


Ditto



src/examples/test_http_framework.cpp
Lines 70 (patched)


Ditto



src/examples/test_http_framework.cpp
Lines 411-413 (original), 416-418 (patched)


Can it fit two lines now?



src/examples/test_http_framework.cpp
Line 459 (original), 464 (patched)


Ditto


- Alexander Rukletsov


On Aug. 7, 2017, 12:50 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6/
> ---
> 
> (Updated Aug. 7, 2017, 12:50 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-7814
> https://issues.apache.org/jira/browse/MESOS-7814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp dfd049b860345adc33c3e774dcdff0320da107f6 
>   src/examples/disk_full_framework.cpp 
> 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
>   src/examples/docker_no_executor_framework.cpp 
> 4a58f11fc8892f23ade1d8c872ab9b4fc580d478 
>   src/examples/dynamic_reservation_framework.cpp 
> f3b1c8c4d2684e827fc10776fef4f2287e315b85 
>   src/examples/load_generator_framework.cpp 
> abb70f42a3a755afaa1d56b1491058b90958f030 
>   src/examples/long_lived_framework.cpp 
> 2a79dfd3f81e5254922895af45c2b72d80ce5f49 
>   src/examples/no_executor_framework.cpp 
> 7d841c6f364e1b671ec829aa9bff3b1f8ecf55ef 
>   src/examples/persistent_volume_framework.cpp 
> 17140294a1eaeeeb92e9e14fc8638182b3a0682e 
>   src/examples/test_framework.cpp 9dbc18b039ee13cc1ec9454bd41cb3cfe30d63b4 
>   src/examples/test_http_framework.cpp 
> a0cd84ab6dfdd8d6ed3403c6e06fabae6d5b46c9 
> 
> 
> Diff: https://reviews.apache.org/r/6/diff/6/
> 
> 
> Testing
> ---
> 
> $ make check
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61137: Cleaned up style in example frameworks.

[Alexander Rukletsov]

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


Fix it, then Ship it!




Thanks for the cleanup! I'll fix the issues and commit it for you.


src/examples/balloon_framework.cpp
Lines 261-266 (original), 261-265 (patched)


I'd say the previous version is more readable. Let's keep it.



src/examples/balloon_framework.cpp
Lines 551-552 (original), 541-542 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.

Even though I guess clang-format agrees with your suggestion.



src/examples/docker_no_executor_framework.cpp
Lines 95-96 (original), 95-96 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/dynamic_reservation_framework.cpp
Lines 229-231 (original), 229-230 (patched)


Let's keep this as is



src/examples/load_generator_framework.cpp
Line 69 (original), 69 (patched)


Good catch!



src/examples/load_generator_framework.cpp
Lines 72-74 (original), 72-74 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/long_lived_framework.cpp
Lines 305-308 (original), 305-308 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/long_lived_framework.cpp
Lines 351-353 (original), 351-353 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/long_lived_framework.cpp
Lines 636-637 (original), 636-637 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/no_executor_framework.cpp
Lines 152-153 (original), 147 (patched)


Let's keep it as is for readability.



src/examples/no_executor_framework.cpp
Lines 177-180 (original), 171-174 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/no_executor_framework.cpp
Lines 210-211 (original), 202-203 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/persistent_volume_framework.cpp
Lines 418-419 (original), 413-414 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/persistent_volume_framework.cpp
Lines 463-464 (original), 455 (patched)


Missing blank line?



src/examples/test_framework.cpp
Lines 107-108 (original), 107-108 (patched)


There is not much value in this change (same amount of lines, no explicit 
violation of our style), hence let leave it as is.



src/examples/test_framework.cpp
Line 140 (original), 140-141 (patched)


Let's put them on the same line.



src/examples/test_framework.cpp
Lines 152-154 (original), 153-154 (patched)


Let's keep them as is for readability.



src/examples/test_framework.cpp
Lines 155-160 (original), 155-158 (patched)


I would argue that the previous variant is more readable. Let's keep it.



src/examples/test_framework.cpp
Lines 176-177 (patched)


Same line?



src/examples/test_framework.cpp
Lines 178-179 (original), 179-180 (patched)


Blanke line?



src/examples/test_framework.cpp
Lines 185-186 (patched)


Put them onto the same line?



src/examples/test_http_framework.cpp
Lines 40-42 (original), 40-41 (patched)

Re: Review Request 61110: Added name flag to balloon and disk full frameworks.

[Alexander Rukletsov]

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


Ship it!




- Alexander Rukletsov


On Aug. 1, 2017, 10:20 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61110/
> ---
> 
> (Updated Aug. 1, 2017, 10:20 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-7814
> https://issues.apache.org/jira/browse/MESOS-7814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allows to distinguish the framework between different instances.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp dfd049b860345adc33c3e774dcdff0320da107f6 
>   src/examples/disk_full_framework.cpp 
> 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
> 
> 
> Diff: https://reviews.apache.org/r/61110/diff/5/
> 
> 
> Testing
> ---
> 
> $ make check
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61530: Enabled retries for `killTasks` in docker executor.

[Andrei Budnik]

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

(Updated Aug. 10, 2017, 4:14 p.m.)


Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Previously, after `docker stop` command failure, docker executor
neither allowed a scheduler to retry `killTask` command, nor retried to
kill a task when task killing was triggered by a failed health check.


Diffs (updated)
-

  src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 


Diff: https://reviews.apache.org/r/61530/diff/2/

Changes: https://reviews.apache.org/r/61530/diff/1-2/


Testing
---

sudo make check

Manual testing:

Emulating `docker stop` errors:
===
1. Add `return fmt.Errorf("Emulating error!")` to 
https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
2. build docker from sources: 
http://oyvindsk.com/writing/docker-build-from-source
3. stop docker service and launch dockerd binary, like: `sudo 
./bundles/17.06.0-dev/binary-daemon/dockerd`

Emulating docker daemon hang:
=
1. `ps aux|grep dockerd` - 2 processes will be found
2. `sudo kill -STOP  ` - send SIGSTOP to docker daemon processes 
just before sending `docker stop`

Emulating health check failure in docker executor:
==
1. Add
```c++
  static int fake = 0;
  if (++fake > 10) {
failure();
return;
  }
```
to `HealthChecker::processCheckResult()` in `src/checks/health_checker.cpp`
2. Add
```c++
   HealthCheck healthCheck;
   healthCheck.set_type(HealthCheck::COMMAND);
   healthCheck.mutable_command()->set_value("exit 0");
   healthCheck.set_delay_seconds(0);
   healthCheck.set_interval_seconds(0);
   healthCheck.set_grace_period_seconds(1);
   _task.mutable_health_check()->CopyFrom(healthCheck);
```
to `CommandScheduler::offers()` in `src/cli/execute.cpp`
3. compile mesos
4. run mesos agent: `sudo GLOG_v=1 ./bin/mesos-agent.sh 
--resources="cpus:1;mem:100" 
--work_dir='/home/some_user/mesos/build/var/agent-1' 
--containerizers="docker,mesos" --master="127.0.1.1:5050"`
5. launch docker executor: `./src/mesos-execute --master="127.0.1.1:5050" 
--name="a" --containerizer=docker --docker_image="ubuntu:xenial" 
--command="while true; do : ; done"`


Thanks,

Andrei Budnik

Re: Review Request 61435: Added logging in docker executor on `docker stop` failure.

[Andrei Budnik]

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

(Updated Aug. 10, 2017, 4:12 p.m.)


Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Added logging in docker executor on `docker stop` failure.


Diffs (updated)
-

  src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 


Diff: https://reviews.apache.org/r/61435/diff/2/

Changes: https://reviews.apache.org/r/61435/diff/1-2/


Testing
---

Manual testing steps:
1. force docker daemon to always return error: 
https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
2. build docker from sources and run it
3. run master, agent
4. launch a task using docker executor via mesos-execute in terminal
5. send SIGINT to the launched mesos-execute
6. check stderr logs of an executor in its sandbox


Thanks,

Andrei Budnik

Review Request 61558: Move duplicate comment closer to implementation.

[Benno Evers]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Move duplicate comment closer to implementation.


Diffs (updated)
-

  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 


Diff: https://reviews.apache.org/r/61558/diff/1/


Testing
---


Thanks,

Benno Evers

Re: Review Request 61495: Add documentation for possible task reasons.

[Benno Evers]

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

(Updated Aug. 10, 2017, 2:01 p.m.)


Review request for mesos, Alexander Rukletsov, James Peach, and Till Toenshoff.


Changes
---

Refactor moving of comment into a separate review.


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


Repository: mesos


Description
---

Add documentation for possible task reasons.


Diffs (updated)
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/task-state-reasons.md PRE-CREATION 
  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 


Diff: https://reviews.apache.org/r/61495/diff/4/

Changes: https://reviews.apache.org/r/61495/diff/3-4/


Testing
---

Built website with site/build.sh and verified it renders ok.

HTML preview: 
http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html


Thanks,

Benno Evers

Re: Review Request 61435: Added logging in docker executor on `docker stop` failure.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/docker/executor.cpp
Lines 415 (patched)


s/string &/ string&


- Alexander Rukletsov


On Aug. 4, 2017, 6:34 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61435/
> ---
> 
> (Updated Aug. 4, 2017, 6:34 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-6743
> https://issues.apache.org/jira/browse/MESOS-6743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logging in docker executor on `docker stop` failure.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 
> 
> 
> Diff: https://reviews.apache.org/r/61435/diff/1/
> 
> 
> Testing
> ---
> 
> Manual testing steps:
> 1. force docker daemon to always return error: 
> https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
> 2. build docker from sources and run it
> 3. run master, agent
> 4. launch a task using docker executor via mesos-execute in terminal
> 5. send SIGINT to the launched mesos-execute
> 6. check stderr logs of an executor in its sandbox
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61530: Enabled retries for `killTasks` in docker executor.

[Alexander Rukletsov]

> On Aug. 10, 2017, 12:07 p.m., Alexander Rukletsov wrote:
> > src/docker/executor.cpp
> > Lines 410-415 (original), 416-421 (patched)
> > 
> >
> > Let's add a comment explaining why are we doing retry / unblock on 
> > failure and why not timeout. Something like:
> > ```
> > // Invoking `docker stop` might be unsuccessful, in which case the 
> > container most probably does not receive the signal. In this case we should 
> > allow schedulers to retry the kill operation or, if the kill was initiated 
> > by a failing health check, retry ourselves. We do not bail out nor stop 
> > retrying to avoid sending a terminal status update while the container 
> > might still be running.
> > //
> > // NOTE: `docker stop` might also hang. We do not address this for now, 
> > because there is no evidence that in this case docker daemon might funciton 
> > properly, i.e., it is only the docker cli command that hangs, and hence 
> > there is not so much we can do.
> > ```

We should also refer to https://issues.apache.org/jira/browse/MESOS-6743 in the 
comment so that folks can get more context.


- Alexander


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


On Aug. 9, 2017, 4:55 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61530/
> ---
> 
> (Updated Aug. 9, 2017, 4:55 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-6743
> https://issues.apache.org/jira/browse/MESOS-6743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, after `docker stop` command failure, docker executor
> neither allowed a scheduler to retry `killTask` command, nor retried to
> kill a task when task killing was triggered by a failed health check.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 
> 
> 
> Diff: https://reviews.apache.org/r/61530/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Manual testing:
> 
> Emulating `docker stop` errors:
> ===
> 1. Add `return fmt.Errorf("Emulating error!")` to 
> https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
> 2. build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 3. stop docker service and launch dockerd binary, like: `sudo 
> ./bundles/17.06.0-dev/binary-daemon/dockerd`
> 
> Emulating docker daemon hang:
> =
> 1. `ps aux|grep dockerd` - 2 processes will be found
> 2. `sudo kill -STOP  ` - send SIGSTOP to docker daemon processes 
> just before sending `docker stop`
> 
> Emulating health check failure in docker executor:
> ==
> 1. Add
> ```c++
>   static int fake = 0;
>   if (++fake > 10) {
> failure();
> return;
>   }
> ```
> to `HealthChecker::processCheckResult()` in `src/checks/health_checker.cpp`
> 2. Add
> ```c++
>HealthCheck healthCheck;
>healthCheck.set_type(HealthCheck::COMMAND);
>healthCheck.mutable_command()->set_value("exit 0");
>healthCheck.set_delay_seconds(0);
>healthCheck.set_interval_seconds(0);
>healthCheck.set_grace_period_seconds(1);
>_task.mutable_health_check()->CopyFrom(healthCheck);
> ```
> to `CommandScheduler::offers()` in `src/cli/execute.cpp`
> 3. compile mesos
> 4. run mesos agent: `sudo GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/some_user/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 5. launch docker executor: `./src/mesos-execute --master="127.0.1.1:5050" 
> --name="a" --containerizer=docker --docker_image="ubuntu:xenial" 
> --command="while true; do : ; done"`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61530: Enabled retries for `killTasks` in docker executor.

[Alexander Rukletsov]

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




src/docker/executor.cpp
Lines 31 (patched)


We sort includes alphabetically.



src/docker/executor.cpp
Lines 410-415 (original), 416-421 (patched)


Let's add a comment explaining why are we doing retry / unblock on failure 
and why not timeout. Something like:
```
// Invoking `docker stop` might be unsuccessful, in which case the 
container most probably does not receive the signal. In this case we should 
allow schedulers to retry the kill operation or, if the kill was initiated by a 
failing health check, retry ourselves. We do not bail out nor stop retrying to 
avoid sending a terminal status update while the container might still be 
running.
//
// NOTE: `docker stop` might also hang. We do not address this for now, 
because there is no evidence that in this case docker daemon might funciton 
properly, i.e., it is only the docker cli command that hangs, and hence there 
is not so much we can do.
```



src/docker/executor.cpp
Lines 427-428 (patched)


Please no "magic constants" in the code. Add it to the existing section 
above. `5s` sounds reasonable to me.



src/docker/executor.cpp
Lines 604-606 (original), 616-618 (patched)


We should augment the comment here explaining why we need 
`killAttemptInProgress` or whatever we end up having.



src/docker/executor.cpp
Lines 606-608 (original), 618-621 (patched)


Instead of `killed` flag, which can be rolled back, and `firstKillAttempt`, 
that kinda indicates that a kill has been issued before, how about having 
`killed` and `killAttemptInProgress` with the following semantics:
- After task transitions to `killed`, there is no way back. This 
corresponds to our intent. This also fixes the race with the health update that 
might come in after we set `killed = false` to unblock schedulers.
- `killAttemptInProgress` will guard kill requests from schedulers and will 
be adjusted accordingly.


- Alexander Rukletsov


On Aug. 9, 2017, 4:55 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61530/
> ---
> 
> (Updated Aug. 9, 2017, 4:55 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-6743
> https://issues.apache.org/jira/browse/MESOS-6743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, after `docker stop` command failure, docker executor
> neither allowed a scheduler to retry `killTask` command, nor retried to
> kill a task when task killing was triggered by a failed health check.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 
> 
> 
> Diff: https://reviews.apache.org/r/61530/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Manual testing:
> 
> Emulating `docker stop` errors:
> ===
> 1. Add `return fmt.Errorf("Emulating error!")` to 
> https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
> 2. build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 3. stop docker service and launch dockerd binary, like: `sudo 
> ./bundles/17.06.0-dev/binary-daemon/dockerd`
> 
> Emulating docker daemon hang:
> =
> 1. `ps aux|grep dockerd` - 2 processes will be found
> 2. `sudo kill -STOP  ` - send SIGSTOP to docker daemon processes 
> just before sending `docker stop`
> 
> Emulating health check failure in docker executor:
> ==
> 1. Add
> ```c++
>   static int fake = 0;
>   if (++fake > 10) {
> failure();
> return;
>   }
> ```
> to `HealthChecker::processCheckResult()` in `src/checks/health_checker.cpp`
> 2. Add
> ```c++
>HealthCheck healthCheck;
>healthCheck.set_type(HealthCheck::COMMAND);
>healthCheck.mutable_command()->set_value("exit 0");
>healthCheck.set_delay_seconds(0);
>healthCheck.set_interval_seconds(0);
>healthCheck.set_grace_period_seconds(1);
>_task.mutable_health_check()->CopyFrom(healthCheck);
> ```
> to `CommandScheduler::offers()` in `src/cli/execute.cpp`
> 3. compile mesos
> 4. run mesos agent: `sudo GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/some_user/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 5. launch 

Re: Review Request 61495: Add documentation for possible task reasons.

[Benno Evers]

> On Aug. 10, 2017, 1:56 a.m., Till Toenshoff wrote:
> > docs/task-reasons.md
> > Lines 49 (patched)
> > 
> >
> > Can we avoid HTML code here? We typically used HTML for getting tables 
> > properly formatted as the apache site rendering otherwise caused issues in 
> > the resulting HTML code.
> > 
> > If not done already, I would suggest you to play with the site renderer 
> > a bit to see if the results are actually what you are hoping for.
> 
> Till Toenshoff wrote:
> Just noticed you actually had that in the testing done section - sorry 
> for not seeing that earlier.

I had a look through the RDiscount docs (the markdown rendering engine we use) 
but I didn't find any way to force a line break.
I switched to vertical bars here, but the same issue remains in the 
`**Note:**`-lines in some of the descriptions, which are rendered as a single 
paragraph in HTML, but I don't know how to fix this.


- Benno


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


On Aug. 10, 2017, 10:43 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> ---
> 
> (Updated Aug. 10, 2017, 10:43 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-5078
> https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for possible task reasons.
> 
> 
> Diffs
> -
> 
>   docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
>   docs/task-state-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/3/
> 
> 
> Testing
> ---
> 
> Built website with site/build.sh and verified it renders ok.
> 
> HTML preview: 
> http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-reasons/index.html
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 61495: Add documentation for possible task reasons.

[Benno Evers]

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

(Updated Aug. 10, 2017, 10:43 a.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


Changes
---

Change -tags to vertical bars


Summary (updated)
-

Add documentation for possible task reasons.


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


Repository: mesos


Description (updated)
---

Add documentation for possible task reasons.


Diffs (updated)
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/task-state-reasons.md PRE-CREATION 
  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 


Diff: https://reviews.apache.org/r/61495/diff/3/

Changes: https://reviews.apache.org/r/61495/diff/2-3/


Testing
---

Built website with site/build.sh and verified it renders ok.

HTML preview: 
http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-reasons/index.html


Thanks,

Benno Evers

Re: Review Request 61511: Improved the readability of some assertions/expectations.

[Benjamin Bannier]

> On Aug. 9, 2017, 5:21 p.m., Benjamin Bannier wrote:
> > Thanks for the cleanup Gaston!
> > 
> > Until looking at your patch I didn't realize how widespread the mistake of 
> > possibly using junk elements from possibly empty containers was, e.g.,
> > 
> >  EXPECT_NE(0u, offers.size()); // or equivalent: 
> > EXPECT_FALSE(offers.empty())
> >  
> >  // Do something with offers[0] which for e.g., 
> >  // `vector` doesn't do bounds checks.
> > 
> > The correct pattern here would be to do a hard assert for a non-empty 
> > collection so we don't run into undefined behavior should we against our 
> > expectations receive an empty collection. Could you please file a ticket 
> > for that?

I filed https://issues.apache.org/jira/browse/MESOS-7877.


- Benjamin


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


On Aug. 9, 2017, 3:01 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61511/
> ---
> 
> (Updated Aug. 9, 2017, 3:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
>   src/tests/container_logger_tests.cpp 
> 2ebff809636fbf68c9e3fb87b98f168338f80d6d 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 67242119f2d10f6f3010c374ea58138e40e4a33e 
>   src/tests/containerizer/cgroups_tests.cpp 
> 506fc7f00dc1ac48476334de930b720b85a691dc 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 0e86318bb7e261ac00be19f6405557f29a2e92af 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c7984592aec2d4e7d1eb7ce077e742c4edc03bb9 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> 866af61f8669163ff3ddd10ed7ecb655568f8250 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> b034ceec7d1bf92db8a1d344835ad48ea2d24952 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> 742143a0f093e415fe98235bbd25342fd65e0483 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 6d95d607b81358953a4afcec03b60e87e7192edd 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> f9cab2fd123e48a5b4387fb2b609d70c7bf535cd 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> b7b7acd37f69832e9ba79c9fe7ed0bc473b53e02 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 8a43c4fada8bdcedc6e1e24833d0d6c94cf927d2 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 1fc56c4a8ca95bb47af968ad72dfef69455e7d46 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 84b39b15880c7b5d9b7967f1e686baa59f52a015 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> bf25049515e8b8d0c085be24c4de22b3ade261b2 
>   src/tests/containerizer/routing_tests.cpp 
> d05b3b11dc5fcd54d956990f252509865168e6b6 
>   src/tests/dynamic_weights_tests.cpp 
> 3c86325a0749112606683bffdf305661170493e5 
>   src/tests/exception_tests.cpp b5b657c2d1b31b59ab1554bf854ef1a69b1517ed 
>   src/tests/fault_tolerance_tests.cpp 
> 5b8213531f5688e94063937af19e674508f0dd8c 
>   src/tests/fetcher_cache_tests.cpp 6d212cd460322ad6e97f4cf6ef537323275b7da0 
>   src/tests/group_tests.cpp f2e37cc7184dd328466b554a16da3173cfbca4a7 
>   src/tests/health_check_tests.cpp e824d1b2c2b17423394da0b089ae8940450378b7 
>   src/tests/hook_tests.cpp 04edbf809b7289a7d5336e4f00f23dfb55d3dbf8 
>   src/tests/master_authorization_tests.cpp 
> 8d54472d8a31956e6e8bb5fe7ffbf47dc793c0bc 
>   src/tests/master_maintenance_tests.cpp 
> e7a80ff4199927df8bf0fb54458d357ae444260d 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 9c31eeae1a1b67af142a01e6c548b509ba06740c 
>   src/tests/master_tests.cpp e6ed02e07a9860ca3a56bba9c502a09444c5b26e 
>   src/tests/master_validation_tests.cpp 
> 813fb25007973f3499b94dcd0e9d2184ba08634c 
>   src/tests/oversubscription_tests.cpp 
> 54535a35e33dde0db3b547b9e31b4545d2900b67 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
>   src/tests/persistent_volume_tests.cpp 
> 7b0f436aa270f64cde93adc58833eef97c73dc26 
>   src/tests/reconciliation_tests.cpp f1635fd627d9a7944cb9d4b0117fcded7b6693e1 
>   src/tests/registrar_zookeeper_tests.cpp 
> cc082d320b4b150717728ec48edd0b26169cecd7 
>   src/tests/resource_offers_tests.cpp 
> e1fcab4b8fbb625876cf246505db2d05ac5d5710 
>   src/tests/scheduler_driver_tests.cpp 
> 47ad1dc9a4e5e0c5444fcc3a90d4645c3b6013ca 
>   src/tests/slave_authorization_tests.cpp 
> 4e55148af77e4aae5829a056b0599954277758bb 
>   src/tests/slave_recovery_tests.cpp 9ba6f6005c2edce67e1f63005518ab71ac981423 
>   

Review Request 61546: Created staging dir only when needed.

[Qian Zhang]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Created staging dir only when needed.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
8058dcb31b9de59fa8d3638b553632235542 


Diff: https://reviews.apache.org/r/61546/diff/1/


Testing
---


Thanks,

Qian Zhang

Re: Review Request 61511: Improved the readability of some assertions/expectations.

[Benjamin Bannier]

> On Aug. 9, 2017, 5:21 p.m., Benjamin Bannier wrote:
> > src/tests/api_tests.cpp
> > Line 1 (original), 1 (patched)
> > 
> >
> > Searching with
> > 
> > $ git grep -E '(EXPECT|ASSERT)_(EQ|NE).*(0u,\s.*size())'
> > 
> > I still get ~50 hits. Since you are at it, would you mind adjusting 
> > also?
> 
> Gastón Kleiman wrote:
> I had done a similar search, and as far as I can see, the other matches 
> are either false possitives or in stout/libprocess:
> 
> ```
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().completed_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_agents().agents_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> v1Response->get_agents().agents_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> v1Response->get_agents().recovered_agents_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_agents().agents_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_tasks().tasks_size());
> src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().launched_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_frameworks().completed_frameworks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().completed_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().completed_executors_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_frameworks().frameworks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_tasks().launched_tasks_size());
> src/tests/api_tests.cpp:ASSERT_EQ(0u, 
> getState.get_executors().executors_size());
> ```
> 
> I don't think we can use `empty()` here.
> 
> And in the following match `manifest` is not a real container:
> 
> ```
> src/tests/containerizer/docker_spec_tests.cpp:  EXPECT_EQ(0u, 
> manifest->size());
> ```
> 
> I'll drop this issue and add two more patches for stout and libprocess to 
> the chain.

In principle we are still able to explicitly check emptiness here, e.g., we can 
replace 

ASSERT_EQ(0u, getState.get_tasks().tasks_size());

with 

ASSERT_TRUE(getState.get_tasks().tasks().empty());

There is some trade-off between local and global consistency in most of the 
cases in e.g., `api_tests.cpp`. When checking proto fields there, we often mix 
checking emptiness and checks for a particular size. I would go with a checks 
capturing our intention as much as possible, i.e., prefer checking for 
emptiness over local typographic consistency.


- Benjamin


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


On Aug. 9, 2017, 3:01 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61511/
> ---
> 
> (Updated Aug. 9, 2017, 3:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
>   src/tests/container_logger_tests.cpp 
> 2ebff809636fbf68c9e3fb87b98f168338f80d6d 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 67242119f2d10f6f3010c374ea58138e40e4a33e 
>   src/tests/containerizer/cgroups_tests.cpp 
> 506fc7f00dc1ac48476334de930b720b85a691dc 
>   

Re: Review Request 61495: Removed table from markdown and added cross-links.

[Benno Evers]

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

(Updated Aug. 10, 2017, 8:55 a.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


Changes
---

Added preview link


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


Repository: mesos


Description
---

Removed table from markdown and added cross-links.


Diffs
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/task-reasons.md PRE-CREATION 
  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/master/master.cpp 43cb6977ca58dce1808e4bdb2d109d549622beb9 


Diff: https://reviews.apache.org/r/61495/diff/2/


Testing (updated)
---

Built website with site/build.sh and verified it renders ok.

HTML preview: 
http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-reasons/index.html


Thanks,

Benno Evers