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

[Megha Sharma]

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

(Updated Aug. 9, 2017, 4:55 a.m.)


Review request for mesos 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 is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs (updated)
-

  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 


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

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


Testing
---

make check


Thanks,

Megha Sharma

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

[Megha Sharma]

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

(Updated Aug. 9, 2017, 4:48 a.m.)


Review request for mesos 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 is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs (updated)
-

  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 


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

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


Testing
---

make check


Thanks,

Megha Sharma

Review Request 61517: Refactored OpenSSL library checks in libprocess.

[Chun-Hung Hsiao]

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

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/1/


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.

[Chun-Hung Hsiao]

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

(Updated Aug. 9, 2017, 3 a.m.)


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


Changes
---

Made OpenSSL optional.


Summary (updated)
-

Refactored OpenSSL library checks in Mesos.


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


Repository: mesos


Description (updated)
---

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 5a6e42c61e6752c3742532502dbc753071b31323 


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

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


Testing (updated)
---

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


Thanks,

Chun-Hung Hsiao

Review Request 61516: Fixed `check` and `tests` targets.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Target names are being removed from variables, as they introduce
unnecessary noise.


Diffs
-

  CMakeLists.txt 75468abd3641171470a0b8759179dc86496820b5 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 61515: Updated `CompilationConfigure.cmake` for imported libraries.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

Windows and Linux have converged on several libraries (ZooKeeper,
Protobuf, and libevent) such that the same bundled version is used, so
the warning message was updated.

Third-party specific compilation flags were moved to the import of each
library, so this configuration no longer needs to include it.

The minimum CMake version was updated and usupports `string(TIMESTAMP)`,
deprecating the `date` work-around.


Diffs
-

  cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

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. 9, 2017, 1:01 a.m.)


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


Changes
---

Fixed an error.


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/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 
  src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 
  src/tests/status_update_manager_tests.cpp 
710723c2a6ff7b72fce5d1d9ac69bf351e37a2ff 
  src/tests/upgrade_tests.cpp 7c900b17e5c895ea3a842f0f5b05b3a2876e61dd 


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

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


Testing
---

`sudo bin/mesos-tests.sh`


Thanks,

Gastón Kleiman

Review Request 61514: Removed moved/deleted CMake modules from libprocess.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

`External` and `PatchCommand` were consolidated to `3rdparty`, and
`VsBuildCommand` has already been deprecated and is unused.


Diffs
-

  3rdparty/libprocess/cmake/macros/External.cmake 
5bc51bd3822677040ab03edd576c876b74bd8afc 
  3rdparty/libprocess/cmake/macros/PatchCommand.cmake 
12ee3f177a490c9b2319ed865aa3bb73019dad0d 
  3rdparty/libprocess/cmake/macros/VsBuildCommand.bat 
8d8f7c6a368d794438129965063e1d4a4fef063c 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 61513: Removed `Mesos3rdpartyConfigure` from `MesosConfigure`.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

The 3rdparty CMake code configures itself now.


Diffs
-

  CMakeLists.txt 75468abd3641171470a0b8759179dc86496820b5 
  cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 61512: Fixed up 3rdparty CMake modules.

[Andrew Schwartzmeyer]

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

Review request for mesos and Joseph Wu.


Repository: mesos


Description
---

This removes `Mesos3rdpartyConfigure.cmake` entirely, as the only
necessary code in it was moved to `PatchCommand.cmake`, which was
consolidated from `libprocess/cmake/macros` to `cmake`.

`External.cmake` was similarly moved.

The `patch_cmd` function was fixed to work on Windows, allowing
`3rdparty/CMakeLists.txt` to be cleaned up further by consolidating the
setting of different patch commands.


Diffs
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
  3rdparty/cmake/External.cmake PRE-CREATION 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
fa1202a5a6a7d37d94b52e2142528febd1b23a10 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61306: Enabled building Java artifacts with CMake.

[Andrew Schwartzmeyer]

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

(Updated Aug. 8, 2017, 5:56 p.m.)


Review request for mesos.


Changes
---

Removed some duplicate code.


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


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 
  cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 
  src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
  src/tests/CMakeLists.txt 6dd2716de942adf6cefa5a464ef664f3c3ebb7a3 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61347: Linked `stout-tests` to `googletest`.

[Andrew Schwartzmeyer]

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

(Updated Aug. 8, 2017, 5:55 p.m.)


Review request for mesos.


Changes
---

Removed dependency on `LINUX` variable set by Mesos project.


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

  3rdparty/stout/cmake/StoutTestsConfigure.cmake 
43baaca784063773fdd700365cadb3532e5c6b3f 
  3rdparty/stout/tests/CMakeLists.txt 953be64f1fb675fd1166fa77d0b3a56a5763d243 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61291: Imported `glog` library.

[Andrew Schwartzmeyer]

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

(Updated Aug. 8, 2017, 5:54 p.m.)


Review request for mesos.


Changes
---

Moved glog compile definitions to here.


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61303: Imported `curl` library.

[Andrew Schwartzmeyer]

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

(Updated Aug. 8, 2017, 5:53 p.m.)


Review request for mesos.


Changes
---

Moved CURL_STATICLIB definition to here.


Repository: mesos


Description (updated)
---

Uses `ExternalProject_Add` on Windows and `FindCURL` elsewhere.

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


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61327: Imported `zlib` library.

[Andrew Schwartzmeyer]

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

(Updated Aug. 8, 2017, 5:51 p.m.)


Review request for mesos.


Changes
---

Fixed comment.


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61309: Imported ZooKeeper library.

[Andrew Schwartzmeyer]

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

(Updated Aug. 8, 2017, 5:52 p.m.)


Review request for mesos.


Changes
---

Move USE_STATIC_LIB definition to here.


Repository: mesos


Description (updated)
---

This commit imports the ZooKeeper C Client as a library, and removes the
discrepancy between Linux and Windows. Both configurations now use
ZooKeeper 3.4.8, with an updated patch to both build with CMake.

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


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
fa1202a5a6a7d37d94b52e2142528febd1b23a10 
  3rdparty/cmake/Versions.cmake 9586296420d76bd4493034fd710209008ee03595 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

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

[Gastón Kleiman]

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




docs/health-checks.md
Lines 54-55 (original), 59-60 (patched)


I'd revert part of the changes: 

`s/if those are failing health checks/if the health checks fail/`



docs/health-checks.md
Lines 87 (patched)


s/except docker executor/except the Docker executor/



docs/health-checks.md
Lines 112 (patched)


s/unhealthy task/an unhealthy task/



docs/health-checks.md
Lines 114 (patched)


s/1+2+3: run the check/1+2+3: they run the check/



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



docs/health-checks.md
Lines 160 (patched)


s/3rdparty/third party/



docs/health-checks.md
Lines 168-169 (patched)


s/check has not run yet, check has timed out/a has not run yet, or a check 
has timed out/



docs/health-checks.md
Lines 180 (patched)


s/`CheckInfo.Command` message/the `CheckInfo.Command` message/



docs/health-checks.md
Lines 194 (patched)


I'd add here (and in the following snippets):

```
TaskInfo task = [...];

```



docs/health-checks.md
Lines 266 (patched)


Don't the executors send an empty check status in this case? See 
`CommandExecutorCheckTest.CommandCheckTimeout`.



docs/health-checks.md
Line 273 (original), 501 (patched)


s/docker/Docker/


- Gastón Kleiman


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 61406: Introduced `--disallow_sharing_agent_pid_namespace` agent flag.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 6, 2017, 7:52 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61406/
> ---
> 
> (Updated Aug. 6, 2017, 7:52 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced `--disallow_sharing_agent_pid_namespace` agent flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 041c3dfb9c0c1718770f74dfb33a9f5d6fbe9b61 
>   src/slave/flags.hpp 032880dfa68cd29420e559d34e592e57827cfc07 
>   src/slave/flags.cpp 4171604090ffebe79fa35579458cabff7270c0de 
> 
> 
> Diff: https://reviews.apache.org/r/61406/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

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/
---

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 
  src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 
  src/tests/status_update_manager_tests.cpp 
710723c2a6ff7b72fce5d1d9ac69bf351e37a2ff 
  src/tests/upgrade_tests.cpp 7c900b17e5c895ea3a842f0f5b05b3a2876e61dd 


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


Testing
---

`sudo bin/mesos-tests.sh`


Thanks,

Gastón Kleiman

Re: Review Request 61272: Added a MockResourceProvider.

[Jie Yu]

> On Aug. 1, 2017, 10:37 p.m., Jie Yu wrote:
> > src/tests/mesos.hpp
> > Lines 2287-2291 (patched)
> > 
> >
> > This is a bit counter intuitive. I was expecting that 
> > MockResourceProvider will take a real Driver:
> > ```
> > class MockResourceProvider
> > {
> > public:
> >   MockResourceProvider(
> >   const URL& endpoint,
> >   ContentType contentType)
> > : driver(
> >   Owned(new 
> > ConstantEndpointDetector(endpoint)),
> >   contentType,
> >   [this]() { connected(); },
> >   [this]() { disconnected(); },
> >   [this](std::queue events) {
> > while (!events.empty()) {
> >   Event event = std::move(events.front());
> >   events.pop();
> >   received(event);
> > }
> >   }) {}
> >   
> >   MOCK_METHOD0_T(connected, void());
> >   MOCK_METHOD0_T(disconnected, void());
> >   ...
> > 
> > private:
> >   Driver driver;
> > };
> > ```
> 
> Jan Schlicht wrote:
> The driver needs to be accessible in the test case to allow things like
> ```
>   v1::MockResourceProvider resourceProvider;
>   v1::resource_provider::TestDriver driver;
>   ...
> 
>   Future subscribed;
>   EXPECT_CALL(*resourceProvider, subscribed(_))
> .WillOnce(FutureArg<0>());
> 
>   mesos::v1::ResourceProviderInfo resourceProviderInfo;
>   resourceProviderInfo.set_type("org.apache.mesos.rp.test");
>   resourceProviderInfo.set_name("test");
> 
>   // Creates a 'SUBSCRIBE' message and sends it using the driver.
>   subscribe(, resourceProviderInfo);
> 
>   AWAIT_READY(subscribed);
> ```
> 
> Jie Yu wrote:
> hum, why can't we use a real RP manager to send the message?
> 
> Jan Schlicht wrote:
> Sorry, I don't understand. The driver sends `Call`s to the RP manager and 
> receives `Event`s that it relays to `MockResourceProvider`. We need to 
> separate driver and `MockResourceProvider` to be able to set expectations on 
> our mock, before the driver starts. E.g. we'd expect 
> `MockResourceProvider::connected` to be called when a connection to the RP 
> manager has been established. We need to set this expectation (by creating a 
> `Future` and `EXPECT_CALL` on the `MockResourceProvider` instance) before a 
> driver instanciated, hence have to keep them separated.

I mean, `MockResourceProvider` should take `TestDriver` if you want to inject 
dependency for testing, instead of the reverse.


- Jie


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


On Aug. 7, 2017, 6:15 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61272/
> ---
> 
> (Updated Aug. 7, 2017, 6:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7469
> https://issues.apache.org/jira/browse/MESOS-7469
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a MockResourceProvider.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45 
> 
> 
> Diff: https://reviews.apache.org/r/61272/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 61483: Added a test using CMD health checks + DefaultExecutor w/ Docker image.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 7, 2017, 10:47 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61483/
> ---
> 
> (Updated Aug. 7, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test using CMD health checks + DefaultExecutor w/ Docker image.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp e824d1b2c2b17423394da0b089ae8940450378b7 
> 
> 
> Diff: https://reviews.apache.org/r/61483/diff/1/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.DefaultExecutorWithDockerImageCommandHealthCheck"`
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61438: Improved `NvidiaGpuTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyDeviceAccess`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 5, 2017, 12:06 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61438/
> ---
> 
> (Updated Aug. 5, 2017, 12:06 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change the test so that the agent offesr as many GPUs as available on
> the box instead of restricting it to 1. This way the test will fail if
> there's a bug that makes the isolator give a task access to more GPUs
> than what it was allocated.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 9a78ae65c1cd414b5093b54ff51724e31e31c9d3 
> 
> 
> Diff: https://reviews.apache.org/r/61438/diff/1/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 sudo bin/mesos-tests.sh --gtest_filter="*NvidiaGpuTest.*Verify*" 
> --verbose` passed on a machine with 4 Nvidia GPUs.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 61282: Added a test verifying that DefaultExecutor tasks can use nvidia GPUs.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Aug. 5, 2017, 12:05 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61282/
> ---
> 
> (Updated Aug. 5, 2017, 12:05 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test verifying that DefaultExecutor tasks can use nvidia GPUs.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 9a78ae65c1cd414b5093b54ff51724e31e31c9d3 
> 
> 
> Diff: https://reviews.apache.org/r/61282/diff/3/
> 
> 
> Testing
> ---
> 
> `GLOG_v=1 sudo bin/mesos-tests.sh --gtest_filter="*NvidiaGpuTest.*Default*" 
> --verbose` passed on a machine with an Nvidia GPU.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

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

[Benjamin Bannier]

> On Aug. 8, 2017, 4:35 p.m., Benno Evers wrote:
> > src/examples/balloon_framework.cpp
> > Line 520 (original), 524 (patched)
> > 
> >
> > No real issue, but I find it curious that our style guide basically 
> > forces us to make redundant string temporaries here, by insisting on 
> > `constexpr char[]` over `const string`-literals. Seems like a bug in the 
> > style guide?

One main issue when using static `std::string`s would be that `std::string` has 
a non-trivial destructor which makes it hard to reason about behavior on 
library unloading or shutdown in general. The Google style guide (on which the 
Mesos style guide is based) gives some more detail,

  https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables
  
While we probably do not do enough of a job of preventing calling likely 
expensive (copy) constructors, I believe that the performance impact in this 
particular case is small (code is called once in `main`). I also suspect that 
we already incur a couple orders of magnitude more string (copy) constructors 
through other temporary objects created.


- Benjamin


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


On Aug. 7, 2017, 2: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, 2: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 61287: Removed `GroupSource` from `stout`.

[Cloudbase]

> On Aug. 8, 2017, 3:36 p.m., Cloudbase wrote:
> > Patch looks great!
> > 
> > Reviews applied: [61287]
> > 
> > Logs available here: http://test.local

Sorry about this comment! It was just a test.


- Cloudbase


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


On Aug. 2, 2017, 12:53 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61287/
> ---
> 
> (Updated Aug. 2, 2017, 12:53 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6721
> https://issues.apache.org/jira/browse/MESOS-6721
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `GroupSource` from `stout`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/cmake/StoutConfigure.cmake 
> 69474a830e3cbd2e3e2718e4a7569137fc6df6b8 
>   3rdparty/stout/tests/CMakeLists.txt 
> 953be64f1fb675fd1166fa77d0b3a56a5763d243 
> 
> 
> Diff: https://reviews.apache.org/r/61287/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 61287: Removed `GroupSource` from `stout`.

[Cloudbase]

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



Patch looks great!

Reviews applied: [61287]

Logs available here: http://test.local

- Cloudbase


On Aug. 2, 2017, 12:53 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61287/
> ---
> 
> (Updated Aug. 2, 2017, 12:53 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6721
> https://issues.apache.org/jira/browse/MESOS-6721
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `GroupSource` from `stout`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/cmake/StoutConfigure.cmake 
> 69474a830e3cbd2e3e2718e4a7569137fc6df6b8 
>   3rdparty/stout/tests/CMakeLists.txt 
> 953be64f1fb675fd1166fa77d0b3a56a5763d243 
> 
> 
> Diff: https://reviews.apache.org/r/61287/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

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

[Benno Evers]

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


Ship it!




- Benno Evers


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 61111: Extracted strings into constants in example frameworks.

[Benno Evers]

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


Ship it!





src/examples/balloon_framework.cpp
Line 520 (original), 524 (patched)


No real issue, but I find it curious that our style guide basically forces 
us to make redundant string temporaries here, by insisting on `constexpr 
char[]` over `const string`-literals. Seems like a bug in the style guide?


- Benno Evers


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 61182: Sent a resource provider message when providers subscribe.

[Jan Schlicht]

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



Looks great, only nitpicking here. Please rebase, `resource_provider::Driver` 
has been changed to allow reconnections using an `EndpointDetector`.


src/tests/resource_provider_manager_tests.cpp
Lines 383 (patched)


Nit: Use `v1::ResourceProviderInfo` instead of 
`mesos::v1::ResourceProviderInfo` to be consistent with the use of the `v1` 
namespace in this test.



src/tests/resource_provider_manager_tests.cpp
Lines 394 (patched)


See nit above.


- Jan Schlicht


On Aug. 1, 2017, 7:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61182/
> ---
> 
> (Updated Aug. 1, 2017, 7:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7837
> https://issues.apache.org/jira/browse/MESOS-7837
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to inform users of resource provider managers that the
> managed resources have changed, we add a new 'ResourceProviderMessage'
> type for communicating changes to the managed total resources.
> 
> We add code to trigger sending of that message when a resource
> provider subscribes with the manager.
> 
> In the future this message could also be used to communicate changes
> to an already subscribed resource provider's total resources.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp 44e1576d4462295d404429d51682134306047462 
>   src/resource_provider/message.hpp 916926bf278de9ed919384a82a452d3ced06bd04 
>   src/tests/resource_provider_manager_tests.cpp 
> 85906ea5e1bb3516ef264de22913ce0a3c9c58c5 
> 
> 
> Diff: https://reviews.apache.org/r/61182/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61180: Added a resource providers total resources to the subscribe call.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Aug. 1, 2017, 7:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61180/
> ---
> 
> (Updated Aug. 1, 2017, 7:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7837
> https://issues.apache.org/jira/browse/MESOS-7837
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since resource provider resources are dynamic (as opposed to how e.g.,
> agent total resources are implemented), they are not part of the
> 'ResourceProviderInfo'. Instead they are communicated explicitly.
> 
> This commit adds total resources the resource provider 'SUBSCRIBE'
> call which can be used to communicate the total capacity in both
> subscription and resubscription scenarios.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> a8a27ed09cde7a9f137e76900a3a1c1a547752ed 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> 34bce7511bc74e157277371a7f46111c9537bcf2 
> 
> 
> Diff: https://reviews.apache.org/r/61180/diff/3/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/61182/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

[Alexander Rojas]

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



There is a weakness in the implementation here. If the ACLs of a user change 
after subscribing, a subscriber could end with an inconsistent view of the 
state.

For example, when a user subscribes to the event stream at T0, he has no right 
to see framework _foo_, so it is filtered out of the response. Then at T1 he is 
granted access to _foo_ but he still cannot see it since we don’t react to 
changes to ACLs, later at T2 _foo_ launches a task which the subscriber can 
see, then he receives the event of a new task from a framework he doesn’t know.

What we discussed would be the best solution is to cache the set of approves a 
subscriber used when it did so and use the same ACLs for the life of the 
connection.

- Alexander Rojas


On July 27, 2017, 8: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, 8: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 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Qian Zhang]

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

Review request for mesos.


Repository: mesos


Description
---

Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.


Diffs
-

  src/tests/default_executor_tests.cpp b9776314a8781963b92ba9ac297654f61a443bc8 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 61493: Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.

[Qian Zhang]

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

(Updated Aug. 8, 2017, 5:44 p.m.)


Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
and Vinod Kone.


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


Repository: mesos


Description
---

Added a test `DefaultExecutorTest.MultiTaskgroupSharePidNamespace`.


Diffs
-

  src/tests/default_executor_tests.cpp b9776314a8781963b92ba9ac297654f61a443bc8 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 61463: Fixed a bug in the test `NamespacesIsolatorTest`.

[Qian Zhang]

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

(Updated Aug. 8, 2017, 5:43 p.m.)


Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
and Vinod Kone.


Changes
---

Addressed comments.


Summary (updated)
-

Fixed a bug in the test `NamespacesIsolatorTest`.


Repository: mesos


Description (updated)
---

In this test, we used the command `stat -c %i` to get the current
process's pid/ipc namespace inode, that is not correct since
`/proc/self/ns/pid` and `/proc/self/ns/ipc` are symbol links, we
should get the inode that the link references rather than the link
itself. So we need to add a `-L` option in the `stat` command.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
f3c541c92f8ecc5c12356363bbbf3561d3b75230 


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

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

[Qian Zhang]

> On Aug. 5, 2017, 8:33 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
> > Lines 130 (patched)
> > 
> >
> > Could we reverse two logics above? so that we can avoid the size check 
> > here. E.g.,
> > ```
> > if (sharePidNamespace) {
> >   return launchInfo;
> > }
> > ```
> > 
> > similar to the short circuit logic for DEBUG container.
> 
> Qian Zhang wrote:
> Could you elaborate a bit more? Which two logics are you talking about?
> 
> Gilbert Song wrote:
> Do you think this logic looks clearer (please help verify its correctness 
> first)?
> ```
>   ContainerLaunchInfo launchInfo;
> 
>   bool sharePidNamespace =
> containerConfig.container_info().linux_info().share_pid_namespace();
> 
>   if (containerId.has_parent()) {
> launchInfo.add_enter_namespaces(CLONE_NEWPID);
> 
> if (containerConfig.has_container_class() &&
> containerConfig.container_class() == ContainerClass::DEBUG) {
>   return launchInfo;
> }
>   } else {
> if (flags.disallow_sharing_agent_pid_namespace && sharePidNamespace) {
>   return Failure(
>   "Sharing agent pid namespace with "
>   "top-level container is not allowed");
> }
>   }
> 
>   if (sharePidNamespace) {
> return launchInfo;
>   }
> 
>   launchInfo.add_clone_namespaces(CLONE_NEWPID);
>   launchInfo.add_pre_exec_commands()->set_value(
>   "mount -n -t proc proc /proc -o nosuid,noexec,nodev");
> 
>   return launchInfo;
> ```

Yeah, it's correct and clearer, thanks Gilbert!


- Qian


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


On Aug. 8, 2017, 5:40 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61428/
> ---
> 
> (Updated Aug. 8, 2017, 5:40 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added pid ns sharing based on agent flag and protobuf message field.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> 2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> f1dfc9f7398ffc029d7180d7f014a515338cb3f4 
> 
> 
> Diff: https://reviews.apache.org/r/61428/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

[Qian Zhang]

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

(Updated Aug. 8, 2017, 5:40 p.m.)


Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added pid ns sharing based on agent flag and protobuf message field.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
f1dfc9f7398ffc029d7180d7f014a515338cb3f4 


Diff: https://reviews.apache.org/r/61428/diff/5/

Changes: https://reviews.apache.org/r/61428/diff/4-5/


Testing
---


Thanks,

Qian Zhang

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

[Alexander Rojas]

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




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


Why not making the principal part of the `HttpConnection`? any reason in 
particular



src/master/master.hpp
Line 1831 (original), 1834-1835 (patched)


Why passing the `master` and the `authorizer` if one can extract the latter 
from the former.

In fact `master.cpp` line 9550 you do `subscriber->master->authorizer` but 
all the other ones use `subcriber->authorizer`.

Let's get consistent there or add a comment.



src/master/master.cpp
Lines 9547-9569 (patched)


I don't think this part should be done as it is. Consider the case when you 
have an `Acceptor` which uses a security module which connects to LDAP for ACLs.

There is a delay for each request you make. It would be wasteful to create 
an Acceptor which you will later not use.



src/master/master.cpp
Lines 9547-9569 (patched)


In fact, why not having one acceptor for each event, something like:

```c++
// This code doesn't necesarily compile.

class EventTaskAddedAcceptor
{
public:
  static Owned create(Principal, Authorizer);
  
  bool accept(Task, FrameworkInfo);

private:
  EventTaskAddedAcceptor(
Owned authorizeTask,
Owned authorizeFramework);
 
  Owned authorizeTask_;
  Owned authorizeFramework_;
};

Owned EventTaskAddedAcceptor::create(
Principal principal, 
Authorizer authorizer)
{
  Future authorizeRole =
AuthorizationAcceptor::create(
subscriber->principal,
subscriber->master->authorizer,
authorization::VIEW_ROLE);

  Future authorizeFramework =
AuthorizationAcceptor::create(
subscriber->principal,
subscriber->authorizer,
authorization::VIEW_FRAMEWORK);

  return collect(authorizeRole, authorizeFramework)
.then([](tuple acceptors) {
  return new EventTaskAddedAcceptor(acceptors[0], acceptors[1]);
});
}

EventTaskAddedAcceptor::EventTaskAddedAcceptor(
Owned authorizeTask,
Owned authorizeFramework)
  : authorizeTask_(authorizeTask),
authorizeFramework_(authorizeFramework)
{
}

bool EventTaskAddedAcceptor::accept(Task task, FrameworkInfo info)
{
  return authorizeTask->accept(task, info) &&
 authorizeFramework->accept(info);
}
```

This way you only need one acceptor per event and you hid the details on 
how the authorization is made inside the acceptor API and remove that code from 
the caller.


- Alexander Rojas


On July 27, 2017, 8: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, 8: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
> 
>

Re: Review Request 58021: Added storage-related offer operations.

[Jan Schlicht]

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

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


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Added storage-related offer operations.


Diffs (updated)
-

  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/common/protobuf_utils.cpp 3ae68e93a985a4cfe23be9c9bd8f92e418102a39 
  src/common/resources.cpp 8d4388935ada6be60448e6f6a88db0e5fc4ad4a1 
  src/common/resources_utils.cpp 821bd0967d55f7fbcb57a4efae5fc390af53ca79 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/v1/resources.cpp 508f3f85c8388a41f57f964b0f6df3b4708d3442 


Diff: https://reviews.apache.org/r/58021/diff/5/

Changes: https://reviews.apache.org/r/58021/diff/4-5/


Testing
---

make check


Thanks,

Jan Schlicht