Re: Review Request 69547: Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxSharedPersistentVolume`.

2018-12-10 Thread Mesos Reviewbot Windows

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69342', '69541', '69542', '69543', '69478', '69479', 
'69544', '67997', '68162', '68163', '69547']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2694/mesos-review-69547

Relevant logs:

- 
[mesos-tests-cmake.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2694/mesos-review-69547/logs/mesos-tests-cmake.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3426):
 warning C4996: 'strerror': This function or variable may be unsafe. Consider 
using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 

Review Request 69547: Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxSharedPersistentVolume`.

2018-12-10 Thread Qian Zhang

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

Review request for mesos.


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


Repository: mesos


Description
---

Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxSharedPersistentVolume`.


Diffs
-

  src/tests/default_executor_tests.cpp 86c3a98ec9e3c5d9d8f2a88218dec1e56d0ebc4c 


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


Testing
---

sudo make check


Thanks,

Qian Zhang



Re: Review Request 68163: Added a test `UNPRIVILEGED_USER_SharedPersistentVolume`.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 2:51 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Added a test `UNPRIVILEGED_USER_SharedPersistentVolume`.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
6afc56cecbf6f06615dbc8632578332e6f7fb6ae 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang



Re: Review Request 68040: Added a new method `getDefaultBackend()` to the provisioner.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 2:49 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


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


Repository: mesos


Description
---

Added a new method `getDefaultBackend()` to the provisioner.


Diffs
-

  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7f84aa499b21140eb29ef7f81e2608743b12eb75 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
ac402fbead81e4b356cb35cea08a00049002e870 


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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 68162: Added a test `ROOT_UNPRIVILEGED_USER_SharedPersistentVolume`.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 2:49 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Added a test `ROOT_UNPRIVILEGED_USER_SharedPersistentVolume`.


Diffs (updated)
-

  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
84b342cdd4b8ef4803725ecfa9f922687ccdadd8 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang



Re: Review Request 67997: Added a test `ROOT_UNPRIVILEGED_USER_ParentTypeDifferentUser`.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 2:45 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Added a test `ROOT_UNPRIVILEGED_USER_ParentTypeDifferentUser`.


Diffs (updated)
-

  src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
cbe677880d6a7dc44697fa5a425b8164dbb4f6e2 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang



Re: Review Request 69479: Added the flag `--task_supplementary_groups` to command executor.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 12:27 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


Changes
---

Changed the definition of `uid_t` and `gid_t` to `UNIT` on Windows.


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


Repository: mesos


Description
---

Added the flag `--task_supplementary_groups` to command executor.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows.hpp 
075ad54ac694a11cb1e981a499e99c02cb734bc6 
  src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421 


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

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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 69474: Added tests for agent/executor heartbeating.

2018-12-10 Thread Mesos Reviewbot Windows

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69463', '69472', '69473', '69474']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2693/mesos-review-69474

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2693/mesos-review-69474/logs/mesos-tests.log):

```
I1211 02:37:15.163028  7732 executor.cpp:922] Sending SIGTERM to process tree 
at pid 96602:37:15.163028  1584 master.cpp:11065] Removing task 
f673b950-c557-427d-8dce-9b9a97035501 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 6c9fcc20-eac8-4e6c-90e1-b79a795de9a7- on 
agent 6c9fcc20-eac8-4e6c-90e1-b79a795de9a7-S0 at slave(463)@192.10.1.4:64631 
(windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I1211 02:37:15.166033  1584 master.cpp:1277] Agent 
6c9fcc20-eac8-4e6c-90e1-b79a795de9a7-S0 at slave(463)@192.10.1.4:64631 
(windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I1211 02:37:15.166033  1584 master.cpp:3280] Disconnecting agent 
6c9fcc20-eac8-4e6c-90e1-b79a795de9a7-S0 at slave(463)@192.10.1.4:64631 
(windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I1211 02:37:15.166033  1584 master.cpp:3299] Deactivating agent 
6c9fcc20-eac8-4e6c-90e1-b79a795de9a7-S0 at slave(463)@192.10.1.4:64631 
(windows-01.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I1211 02:37:15.167024 13936 hierarchical.cpp:357] Removed framework 
6c9fcc20-eac8-4e6c-90e1-b79a795de9a7-
I1211 02:37:15.167024 13936 hierarchical.cpp:801] Agent 
6c9fcc20-eac8-4e6c-90e1-b79a795de9a7-S0 deactivated
I1211 02:37:15.167024  6600 containerizer.cpp:2463] Destroying container 
7346239a-02a5-4f0a-bc21-bcb693639e96 in RUNNING state
I1211 02:37:15.168032  6600 containerizer.cpp:3130] Transitioning the state of 
container 7346239a-02a5-4f0a-bc21-bcb693639e96 from RUNNING to DESTROYING
I1211 02:37:15.168032  6600 launcher.cpp:161] Asked to destroy container 
7346239a-02a5-4f0a-bc21-bcb693639e96
W1211 02:37:15.170037 13532 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=8552 to peer '192.10.1.4:50078': IO failed with error 
code: The specified network name is no longer available.

W1211 02:37:15.170037 13532 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=9144 to peer '192.10.1.4:50079': IO failed with error 
code: The specified network name is no longer available.

I1211 02:37:15.[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 
(682 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (705 ms total)

[--] Global test environment tear-down
[==] 1060 tests from 104 test cases ran. (561735 ms total)
[  PASSED  ] 1059 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

194044  1584 containerizer.cpp:2969] Container 
7346239a-02a5-4f0a-bc21-bcb693639e96 has exited
I1211 02:37:15.224057 14100 master.cpp:1117] Master terminating
I1211 02:37:15.225041 13176 hierarchical.cpp:643] Removed agent 
6c9fcc20-eac8-4e6c-90e1-b79a795de9a7-S0
I1211 02:37:15.529048 13532 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Dec. 10, 2018, 5:20 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69474/
> ---
> 
> (Updated Dec. 10, 2018, 5:20 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds two separate tests which check if the Agent sends heartbeats
> to HTTP executors, and if the HTTP executor driver sends heartbeats
> to the agent.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> b87a8069892b3886329581e86ed15072dd785b3d 
>   src/examples/test_http_executor.cpp 
> e5f7cbb3b4e35663717f2a74550f8378bfafb0f9 
>   src/tests/executor_http_api_tests.cpp 
> a635e1a3c228b52f47ba81a67753ea49ce092143 
>   src/tests/mesos.hpp b57bbd40051fed0829453c56b90f5660354f4334 
> 
> 
> Diff: https://reviews.apache.org/r/69474/diff/6/
> 
> 
> Testing
> ---
> 
> ```
> cmake --build . --target check
> src/mesos-tests --gtest_filter="*Heartbeat*" --verbose --gtest_repeat=-1 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69544: Made non-root containers can access shared persistent volume.

2018-12-10 Thread Mesos Reviewbot Windows

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69342', '69541', '69542', '69543', '69478', '69479', 
'69544']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2692/mesos-review-69544

Relevant logs:

- 
[mesos-tests-cmake.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2692/mesos-review-69544/logs/mesos-tests-cmake.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512):
 

Re: Review Request 69544: Made non-root containers can access shared persistent volume.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 9:17 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


Changes
---

Fixed the compilation issue on Windows.


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


Repository: mesos


Description
---

Made non-root containers can access shared persistent volume.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
8f76944cd9058c0ba809443e32a3d8c8a26ac4a6 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
2a9ea448d7f963f86e8b2909d83e82b498e4104c 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
deacc909a2d323671667cb646c019664bdb660e7 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
f91a2eeb835bb65a855eeb314d4c69e3b58fecae 
  src/slave/containerizer/mesos/isolators/filesystem/windows.hpp 
2bf011d3e7b014a17f759851d755b161c897b131 
  src/slave/containerizer/mesos/isolators/filesystem/windows.cpp 
f169c380f803a2111b1612cee60250ee9a30ef2e 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
PRE-CREATION 


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

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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 69474: Added tests for agent/executor heartbeating.

2018-12-10 Thread Joseph Wu

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

(Updated Dec. 10, 2018, 5:20 p.m.)


Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.


Changes
---

Used an `extern` constant defined in `executor/executor.cpp` instead of 
hardcoding `Minutes(30)`.


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


Repository: mesos


Description
---

This adds two separate tests which check if the Agent sends heartbeats
to HTTP executors, and if the HTTP executor driver sends heartbeats
to the agent.


Diffs (updated)
-

  src/examples/long_lived_executor.cpp b87a8069892b3886329581e86ed15072dd785b3d 
  src/examples/test_http_executor.cpp e5f7cbb3b4e35663717f2a74550f8378bfafb0f9 
  src/tests/executor_http_api_tests.cpp 
a635e1a3c228b52f47ba81a67753ea49ce092143 
  src/tests/mesos.hpp b57bbd40051fed0829453c56b90f5660354f4334 


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

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


Testing
---

```
cmake --build . --target check
src/mesos-tests --gtest_filter="*Heartbeat*" --verbose --gtest_repeat=-1 
--gtest_break_on_failure
```


Thanks,

Joseph Wu



Re: Review Request 69473: Added heartbeaters for agent and HTTP executors.

2018-12-10 Thread Joseph Wu

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

(Updated Dec. 10, 2018, 5:20 p.m.)


Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.


Changes
---

Moved the 30 Minute executor heartbeat interval into a constant.  This is an 
`extern` so that the test can define and use it too, without a header.


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


Repository: mesos


Description
---

This implements two separate heartbeaters for Executor Events (agent
to executor) and Executor Calls (executor to agent).  Both are set to
non-configurable intervals of 30 minutes, which should be sufficient
to keep the connections alive while not flooding logs with warnings
if the executor/agent does not have this patch.


Diffs (updated)
-

  src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea 
  src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b 
  src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37 
  src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71 
  src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421 
  src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1 
  src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c 
  src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
  src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 


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

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


Testing
---

make


Thanks,

Joseph Wu



Re: Review Request 69505: Added an operation status update manager to the agent.

2018-12-10 Thread Mesos Reviewbot Windows

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69505']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2691/mesos-review-69505

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2691/mesos-review-69505/logs/mesos-tests.log):

```
W1211 01:12:40.491051 20332 slave.cpp:3933] Ignoring shutdown framework 
45c94a58-1600-4fb7-87c4-d18d704106d2- because it is terminating
I1211 01:12:40.493053 18688 master.cpp:1275] Agent 
45c94a58-1600-4fb7-87c4-d18d704106d2-S0 at slave(462)@192.10.1.6:52101 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I1211 01:12:40.493053 18688 master.cpp:3278] Disconnecting agent 
45c94a58-1600-4fb7-87c4-d18d704106d2-S0 at slave(462)@192.10.1.6:52101 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I1211 01:12:40.493053 18688 master.cpp:3297] Deactivating agent 
45c94a58-1600-4fb7-87c4-d18d704106d2-S0 at slave(462)@192.10.1.6:52101 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I1211 01:12:40.494048 20708 hierarchical.cpp:357] Removed framework 
45c94a58-1600-4fb7-87c4-d18d704106d2-
I1211 01:12:40.494048 20708 hierarchical.cpp:801] Agent 
45c94a58-1600-4fb7-87c4-d18d704106d2-S0 deactivated
I1211 01:12:40.495060 20708 containerizer.cpp:2463] Destroying container 
94ca8fda-3f8e-4a66-8728-cd3a3337cb82 in RUNNING state
I1211 01:12:40.495060 20708 containerizer.cpp:3130] Transitioning the state of 
container 94ca8fda-3f8e-4a66-8728-cd3a3337cb82 from RUNNING to DESTROYING
I1211 01:12:40.496067 20708 launcher.cpp:161] Asked to destroy container 
94ca8fda-3f8e-4a66-8728-cd3a3337cb82
W1211 01:12:40.497061 14416 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=2616 to peer '192.10.1.6:53879': IO failed with error 
code: The specified network name is no longer available.

W1211 01:12:40.498051 14416[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (683 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (705 ms total)

[--] Global test environment tear-down
[==] 1058 tests from 104 test cases ran. (531377 ms total)
[  PASSED  ] 1057 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

 process.cpp:838] Failed to recv on socket WindowsFD::Type::SOCKET=2184 to peer 
'192.10.1.6:53880': IO failed with error code: The specified network name is no 
longer available.

I1211 01:12:40.542299 19384 containerizer.cpp:2969] Container 
94ca8fda-3f8e-4a66-8728-cd3a3337cb82 has exited
I1211 01:12:40.572299 20332 master.cpp:1117] Master terminating
I1211 01:12:40.573297  4976 hierarchical.cpp:643] Removed agent 
45c94a58-1600-4fb7-87c4-d18d704106d2-S0
I1211 01:12:40.941330 14416 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Dec. 10, 2018, 11:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69505/
> ---
> 
> (Updated Dec. 10, 2018, 11:34 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9278
> https://issues.apache.org/jira/browse/MESOS-9278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an operation status update manager to the agent
> in order to handle updates for operations on agent default
> resources. A new test is also added which verifies that such
> updates are retried.
> 
> 
> Later patches will integrate this status update manager with
> the agent's checkpointing/recovery code.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
>   src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
>   src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
>   src/tests/slave_tests.cpp 1e918712d73d0a03c4c15b41babff1f649ba9c07 
> 
> 
> Diff: https://reviews.apache.org/r/69505/diff/3/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh --gtest_filter="*UpdateOperationStatusRetry*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 69473: Added heartbeaters for agent and HTTP executors.

2018-12-10 Thread Joseph Wu


> On Dec. 10, 2018, 8:22 a.m., Benno Evers wrote:
> > src/executor/executor.cpp
> > Lines 798 (patched)
> > 
> >
> > I think you're adding a constant in `src/slave/constants.hpp` in this 
> > review, so we could use it here. (or if you want to keep the intervals in 
> > both directions separate, let's at least define this as a constant within 
> > this file, instead of using a naked value here)

After some consideration, a good middle ground is to pull this value into a 
constant inside this file.  We'll put off creating a new header, as there might 
be more things we could clean up with a header.

The test will forward-define the constant and link in the value of the 
constant.  Luckily, since this is the executor driver (rather than the 
default/command executor) the code will be compiled into libmesos and won't 
require any special build changes.


- Joseph


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


On Dec. 7, 2018, 2:17 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> ---
> 
> (Updated Dec. 7, 2018, 2:17 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent).  Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
> 
> 
> Diffs
> -
> 
>   src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea 
>   src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b 
>   src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37 
>   src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71 
>   src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421 
>   src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1 
>   src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c 
>   src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
>   src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
> 
> 
> Diff: https://reviews.apache.org/r/69473/diff/4/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69540: Added `ROOT_PseudoDevicesWithRootFilesystem` test.

2018-12-10 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Dec. 10, 2018, 8:41 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69540/
> ---
> 
> (Updated Dec. 10, 2018, 8:41 a.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-9462
> https://issues.apache.org/jira/browse/MESOS-9462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that pseudo devices like /dev/random are properly
> mounted in the container's root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
>   src/tests/containerizer/rootfs.cpp 83662fc2c2b9ea902b444bab9c2911df01ee11d5 
> 
> 
> Diff: https://reviews.apache.org/r/69540/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 69545: Refactored `LinuxFilesystemIsolator{Test, MesosTest}` tests.

2018-12-10 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Dec. 10, 2018, 10:37 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69545/
> ---
> 
> (Updated Dec. 10, 2018, 10:37 a.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-9462
> https://issues.apache.org/jira/browse/MESOS-9462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch factors out boilerplate code related to initialization of
> the agent flags.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
> 
> 
> Diff: https://reviews.apache.org/r/69545/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ./src/mesos-tests --gtest_filter=LinuxFilesystemIsolator*
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 69514: Added gauge metric for operator event stream subscribers.

2018-12-10 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 69514 was successfully built and tested.

Reviews applied: `['69307', '69514']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2690/mesos-review-69514

- Mesos Reviewbot Windows


On Dec. 10, 2018, 10:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69514/
> ---
> 
> (Updated Dec. 10, 2018, 10:24 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Gastón Kleiman, and 
> Greg Mann.
> 
> 
> Bugs: MESOS-9258
> https://issues.apache.org/jira/browse/MESOS-9258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This metric "master/operator_event_stream_subscribers" returns
> the total number of subscribers to the master's operator event stream.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
>   docs/operator-http-api.md 6edf361047ee5915514da892c6d885b74a510a72 
>   src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
>   src/master/metrics.hpp f6bb89da9394bfe469bc690ff2de66824e994098 
>   src/master/metrics.cpp f69ed52c1c89912e7a5d26cb9409f5959b09111a 
>   src/tests/api_tests.cpp edde48a4b4b46c9f47bc44de389c97b77322b8e8 
>   src/tests/master_tests.cpp 3bff7a13bb8013ef91b72c14ace23ee94bc190e9 
> 
> 
> Diff: https://reviews.apache.org/r/69514/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> make check
> 
> src/mesos-tests --gtest_filter="*MaxEventStreamSubscribers*" 
> --gtest_repeat=-1 --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69505: Added an operation status update manager to the agent.

2018-12-10 Thread Greg Mann

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

(Updated Dec. 10, 2018, 11:34 p.m.)


Review request for mesos, Benno Evers and Gastón Kleiman.


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


Repository: mesos


Description (updated)
---

This patch adds an operation status update manager to the agent
in order to handle updates for operations on agent default
resources. A new test is also added which verifies that such
updates are retried.


Later patches will integrate this status update manager with
the agent's checkpointing/recovery code.


Diffs (updated)
-

  src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
  src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
  src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
  src/tests/slave_tests.cpp 1e918712d73d0a03c4c15b41babff1f649ba9c07 


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

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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*UpdateOperationStatusRetry*" 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann



Re: Review Request 69505: Added an operation status update manager to the agent.

2018-12-10 Thread Greg Mann

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

(Updated Dec. 10, 2018, 11:03 p.m.)


Review request for mesos, Benno Evers and Gastón Kleiman.


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


Repository: mesos


Description (updated)
---

This patch adds an operation status update manager to the agent
in order to handle updates for operations on agent default
resources. A new test is also added which verifies that such
updates are retried.

Later patches will integrate this status update manager with
the agent's checkpointing/recovery code.


Diffs
-

  src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
  src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
  src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
  src/tests/slave_tests.cpp 1e918712d73d0a03c4c15b41babff1f649ba9c07 


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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*UpdateOperationStatusRetry*" 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann



Re: Review Request 69505: Added an operation status update manager to the agent.

2018-12-10 Thread Greg Mann


> On Dec. 6, 2018, 12:32 a.m., Gastón Kleiman wrote:
> > src/master/master.cpp
> > Lines 8539-8541 (original)
> > 
> >
> > Should we replace this with something like the following?
> > 
> > ```
> > CHECK(!resourceProviderId.isError())
> >   << "Could not determine resource provider of operation " << 
> > *operation
> >   << ": " << resourceProviderId.error();
> > ```

Good call, thanks!


- Greg


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


On Dec. 10, 2018, 11:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69505/
> ---
> 
> (Updated Dec. 10, 2018, 11:01 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9278
> https://issues.apache.org/jira/browse/MESOS-9278
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an operation status update manager to the
> agent in order to provide reliable status updates to
> schedulers for operations on agent default resources.
> 
> Later patches will integrate this status update manager
> with the agent's checkpointing/recovery code.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
>   src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
>   src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
>   src/tests/slave_tests.cpp 1e918712d73d0a03c4c15b41babff1f649ba9c07 
> 
> 
> Diff: https://reviews.apache.org/r/69505/diff/2/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh --gtest_filter="*UpdateOperationStatusRetry*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 69505: Added an operation status update manager to the agent.

2018-12-10 Thread Greg Mann

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

(Updated Dec. 10, 2018, 11:01 p.m.)


Review request for mesos, Benno Evers and Gastón Kleiman.


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


Repository: mesos


Description (updated)
---

This patch adds an operation status update manager to the
agent in order to provide reliable status updates to
schedulers for operations on agent default resources.

Later patches will integrate this status update manager
with the agent's checkpointing/recovery code.


Diffs (updated)
-

  src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
  src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
  src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
  src/tests/slave_tests.cpp 1e918712d73d0a03c4c15b41babff1f649ba9c07 


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

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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*UpdateOperationStatusRetry*" 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann



Re: Review Request 69514: Added gauge metric for operator event stream subscribers.

2018-12-10 Thread Joseph Wu

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

(Updated Dec. 10, 2018, 2:24 p.m.)


Review request for mesos, Benno Evers, Benjamin Mahler, Gastón Kleiman, and 
Greg Mann.


Changes
---

Rename!!!  No more `active_` prefix, to avoid having to answer the question of 
what an `inactive_` subscriber could be.


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


Repository: mesos


Description (updated)
---

This metric "master/operator_event_stream_subscribers" returns
the total number of subscribers to the master's operator event stream.


Diffs (updated)
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  docs/operator-http-api.md 6edf361047ee5915514da892c6d885b74a510a72 
  src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
  src/master/metrics.hpp f6bb89da9394bfe469bc690ff2de66824e994098 
  src/master/metrics.cpp f69ed52c1c89912e7a5d26cb9409f5959b09111a 
  src/tests/api_tests.cpp edde48a4b4b46c9f47bc44de389c97b77322b8e8 
  src/tests/master_tests.cpp 3bff7a13bb8013ef91b72c14ace23ee94bc190e9 


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

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


Testing
---

```
make check

src/mesos-tests --gtest_filter="*MaxEventStreamSubscribers*" --gtest_repeat=-1 
--gtest_break_on_failure
```


Thanks,

Joseph Wu



Re: Review Request 69472: Refactored master and agent streaming connections.

2018-12-10 Thread Joseph Wu

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

(Updated Dec. 10, 2018, 2 p.m.)


Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.


Changes
---

Address comments and add heartbeater header into Makefile, so distcheck works.


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


Repository: mesos


Description
---

This moves the very similar `HttpConnection` classes inside the
master and agent into a common header.  The refactored
`StreamingHttpConnection` is more explicitly named to avoid
potentially clashing with the libprocess HTTP helpers.

This also moves the master's heartbeater helper into a new header
and transforms it into an RAII libprocess actor wrapper.  The 
heartbeater depends on this `StreamingHttpConnection` and is currently
used by the master for heartbeating the operator event stream
and HTTP framework connection.  A later patch will use this heartbeater
for agent->executor heartbeats.


Diffs (updated)
-

  src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
  src/common/heartbeater.hpp PRE-CREATION 
  src/common/http.hpp ac9ed5e8b1c6e8e100eba401136eb95c9dd621d4 
  src/master/framework.cpp 36eda9f287bef28608024e0036db1f955a1b393c 
  src/master/http.cpp 68ee2a6dcffbc772afec6e797b1af8da48f61937 
  src/master/master.hpp c7becfa615964674dcf1ebd9424aa5818a0fdb85 
  src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
  src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c 
  src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
  src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 


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

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


Testing
---

make

The original plan was to use the same helper for the executor too, but of the 
Master, Framework, and Agent heartbeats, only the executor lacks a streaming 
connection.


Thanks,

Joseph Wu



Re: Review Request 69514: Added gauge metric for operator event stream subscribers.

2018-12-10 Thread Greg Mann

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


Fix it, then Ship it!





docs/monitoring.md
Lines 984 (patched)


I agree with Benno, let's remove the `active_` prefix if you're OK with 
that.


- Greg Mann


On Dec. 6, 2018, 2:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69514/
> ---
> 
> (Updated Dec. 6, 2018, 2:21 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Gastón Kleiman, and 
> Greg Mann.
> 
> 
> Bugs: MESOS-9258
> https://issues.apache.org/jira/browse/MESOS-9258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This metric "master/active_operator_event_stream_subscribers" returns
> the total number of subscribers to the master's operator event stream.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
>   docs/operator-http-api.md 6edf361047ee5915514da892c6d885b74a510a72 
>   src/master/master.cpp 3b3824a67f46866cd64e32d7f9f92484b5891aa2 
>   src/master/metrics.hpp f6bb89da9394bfe469bc690ff2de66824e994098 
>   src/master/metrics.cpp f69ed52c1c89912e7a5d26cb9409f5959b09111a 
>   src/tests/api_tests.cpp edde48a4b4b46c9f47bc44de389c97b77322b8e8 
>   src/tests/master_tests.cpp ef2c00101fc3d30c564a9ca34884dece2cdd2651 
> 
> 
> Diff: https://reviews.apache.org/r/69514/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> make check
> 
> src/mesos-tests --gtest_filter="*MaxEventStreamSubscribers*" 
> --gtest_repeat=-1 --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69545: Refactored `LinuxFilesystemIsolator{Test, MesosTest}` tests.

2018-12-10 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 69545 was successfully built and tested.

Reviews applied: `['69540', '69545']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2689/mesos-review-69545

- Mesos Reviewbot Windows


On Dec. 10, 2018, 6:37 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69545/
> ---
> 
> (Updated Dec. 10, 2018, 6:37 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-9462
> https://issues.apache.org/jira/browse/MESOS-9462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch factors out boilerplate code related to initialization of
> the agent flags.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
> 
> 
> Diff: https://reviews.apache.org/r/69545/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ./src/mesos-tests --gtest_filter=LinuxFilesystemIsolator*
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 69474: Added tests for agent/executor heartbeating.

2018-12-10 Thread Joseph Wu


> On Dec. 10, 2018, 8:33 a.m., Benno Evers wrote:
> > src/tests/executor_http_api_tests.cpp
> > Lines 1232 (patched)
> > 
> >
> > Let's please fix this `TODO` before committing.

I left this TODO because none of the executors have headers, outside of the 
public header defining the executor driver interfaces.  There is a good deal of 
cleanup we could do by defining headers for the executor driver, v0_v1 shim, 
command executor, and default executor.  There are constants to pull out, 
environment variable checks we could translate into `Flags`, classes which 
could be put in headers, etc.

But it wouldn't necessarily make sense to make these cleanups in this chain (or 
one at a time).


- Joseph


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


On Dec. 7, 2018, 2:18 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69474/
> ---
> 
> (Updated Dec. 7, 2018, 2:18 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds two separate tests which check if the Agent sends heartbeats
> to HTTP executors, and if the HTTP executor driver sends heartbeats
> to the agent.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> b87a8069892b3886329581e86ed15072dd785b3d 
>   src/examples/test_http_executor.cpp 
> e5f7cbb3b4e35663717f2a74550f8378bfafb0f9 
>   src/tests/executor_http_api_tests.cpp 
> a635e1a3c228b52f47ba81a67753ea49ce092143 
>   src/tests/mesos.hpp b57bbd40051fed0829453c56b90f5660354f4334 
> 
> 
> Diff: https://reviews.apache.org/r/69474/diff/5/
> 
> 
> Testing
> ---
> 
> ```
> cmake --build . --target check
> src/mesos-tests --gtest_filter="*Heartbeat*" --verbose --gtest_repeat=-1 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69544: Made non-root containers can access shared persistent volume.

2018-12-10 Thread Mesos Reviewbot Windows

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['69342', '69541', '69542', '69543', '69478', '69479', 
'69544']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2688/mesos-review-69544

Relevant logs:

- 
[mesos-tests-cmake.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2688/mesos-review-69544/logs/mesos-tests-cmake.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170):
 warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496):
 warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 

Re: Review Request 69540: Added `ROOT_PseudoDevicesWithRootFilesystem` test.

2018-12-10 Thread Andrei Budnik


> On Dec. 10, 2018, 5:06 p.m., James Peach wrote:
> > src/tests/containerizer/linux_filesystem_isolator_tests.cpp
> > Lines 137 (patched)
> > 
> >
> > As a separate commit, can we factor out the flags set up from all these 
> > test case?

The following patch in the patch chain factors out the flags.


- Andrei


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


On Dec. 10, 2018, 4:41 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69540/
> ---
> 
> (Updated Dec. 10, 2018, 4:41 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-9462
> https://issues.apache.org/jira/browse/MESOS-9462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that pseudo devices like /dev/random are properly
> mounted in the container's root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
>   src/tests/containerizer/rootfs.cpp 83662fc2c2b9ea902b444bab9c2911df01ee11d5 
> 
> 
> Diff: https://reviews.apache.org/r/69540/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 69473: Added heartbeaters for agent and HTTP executors.

2018-12-10 Thread Joseph Wu


> On Dec. 10, 2018, 8:22 a.m., Benno Evers wrote:
> > src/slave/http.cpp
> > Lines 862 (patched)
> > 
> >
> > I wonder: This `switch` statement doesn't have a `default` case, so if 
> > a new executor sends a heartbeat to an old agent, wouldn't it skip past the 
> > `switch` into the `UNREACHABLE` and kill the agent?

When dealing with protobuf enums in general, the `0` value is special in that 
any un-mapped types will be parsed as `0`.

So suppose a server knows about an enum like this:
```
enum Type {
  UNKNOWN = 0;
  ONE = 1;
  TWO = 2;
}
```
While the client's enum has an additional row:
```
enum Type {
  UNKNOWN = 0;
  ONE = 1;
  TWO = 2;
  THREE = 3;
}
```
When passed over the wire, the client will send a `Type = 3`.  The server will 
parse this as `UNKNOWN`, because this is the default value of an enum.
The documentation does not explicitly spell out this behavior, but it is 
considered best-practice for making extendable enums via protobuf: 
https://developers.google.com/protocol-buffers/docs/proto3#enum


- Joseph


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


On Dec. 7, 2018, 2:17 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> ---
> 
> (Updated Dec. 7, 2018, 2:17 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent).  Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
> 
> 
> Diffs
> -
> 
>   src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea 
>   src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b 
>   src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37 
>   src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71 
>   src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421 
>   src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1 
>   src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c 
>   src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
>   src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
> 
> 
> Diff: https://reviews.apache.org/r/69473/diff/4/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Review Request 69545: Refactored `LinuxFilesystemIsolator{Test, MesosTest}` tests.

2018-12-10 Thread Andrei Budnik

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

Review request for mesos, Jie Yu and James Peach.


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


Repository: mesos


Description
---

This patch factors out boilerplate code related to initialization of
the agent flags.


Diffs
-

  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
84b342cdd4b8ef4803725ecfa9f922687ccdadd8 


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


Testing
---

sudo ./src/mesos-tests --gtest_filter=LinuxFilesystemIsolator*


Thanks,

Andrei Budnik



Re: Review Request 69540: Added `ROOT_PseudoDevicesWithRootFilesystem` test.

2018-12-10 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 69540 was successfully built and tested.

Reviews applied: `['69540']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2687/mesos-review-69540

- Mesos Reviewbot Windows


On Dec. 10, 2018, 4:41 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69540/
> ---
> 
> (Updated Dec. 10, 2018, 4:41 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-9462
> https://issues.apache.org/jira/browse/MESOS-9462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that pseudo devices like /dev/random are properly
> mounted in the container's root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
>   src/tests/containerizer/rootfs.cpp 83662fc2c2b9ea902b444bab9c2911df01ee11d5 
> 
> 
> Diff: https://reviews.apache.org/r/69540/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 69538: Added UCR bridge network for Mesos Mini.

2018-12-10 Thread Deepak Goel

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


Ship it!




Ship It!

- Deepak Goel


On Dec. 10, 2018, 6:11 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69538/
> ---
> 
> (Updated Dec. 10, 2018, 6:11 a.m.)
> 
> 
> Review request for mesos, Deepak Goel and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds the UCR bridge network support for Mesos Mini using CNI
> bridge plugin and Mesos port mapper CNI plugin.
> 
> 
> Diffs
> -
> 
>   support/mesos-mini/Dockerfile f8af422d8b8b0be4c462d40bce2d1337ab8b6e95 
>   support/mesos-mini/mesos/agent_environment 
> 48767281987c2e70b3a64f28c5eac139435c17f6 
>   support/mesos-mini/mesos/master_environment 
> 0382558744cb693c862246856b8c16ff42a95d19 
>   support/mesos-mini/mesos/ucr-default-bridge.json PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69538/diff/1/
> 
> 
> Testing
> ---
> 
> Tested using this Marathon app
> ```json
> {
>   "id": "/test",
>   "instances": 1,
>   "container": {
> "type": "MESOS",
> "volumes": [],
> "portMappings": [],
> "docker": {
>   "image": "alpine"
> }
>   },
>   "cpus": 0.1,
>   "mem": 128,
>   "requirePorts": false,
>   "networks": [
> {
>   "mode": "container/bridge"
> }
>   ],
>   "healthChecks": [],
>   "fetch": [],
>   "constraints": [],
>   "cmd": "sleep 10"
> }
> ```
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 69538: Added UCR bridge network for Mesos Mini.

2018-12-10 Thread James Peach

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


Ship it!




Ship It!

- James Peach


On Dec. 10, 2018, 6:11 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69538/
> ---
> 
> (Updated Dec. 10, 2018, 6:11 a.m.)
> 
> 
> Review request for mesos, Deepak Goel and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds the UCR bridge network support for Mesos Mini using CNI
> bridge plugin and Mesos port mapper CNI plugin.
> 
> 
> Diffs
> -
> 
>   support/mesos-mini/Dockerfile f8af422d8b8b0be4c462d40bce2d1337ab8b6e95 
>   support/mesos-mini/mesos/agent_environment 
> 48767281987c2e70b3a64f28c5eac139435c17f6 
>   support/mesos-mini/mesos/master_environment 
> 0382558744cb693c862246856b8c16ff42a95d19 
>   support/mesos-mini/mesos/ucr-default-bridge.json PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69538/diff/1/
> 
> 
> Testing
> ---
> 
> Tested using this Marathon app
> ```json
> {
>   "id": "/test",
>   "instances": 1,
>   "container": {
> "type": "MESOS",
> "volumes": [],
> "portMappings": [],
> "docker": {
>   "image": "alpine"
> }
>   },
>   "cpus": 0.1,
>   "mem": 128,
>   "requirePorts": false,
>   "networks": [
> {
>   "mode": "container/bridge"
> }
>   ],
>   "healthChecks": [],
>   "fetch": [],
>   "constraints": [],
>   "cmd": "sleep 10"
> }
> ```
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 69538: Added UCR bridge network for Mesos Mini.

2018-12-10 Thread Sergey Urbanovich

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


Ship it!




- Sergey Urbanovich


On Dec. 10, 2018, 6:11 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69538/
> ---
> 
> (Updated Dec. 10, 2018, 6:11 a.m.)
> 
> 
> Review request for mesos, Deepak Goel and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds the UCR bridge network support for Mesos Mini using CNI
> bridge plugin and Mesos port mapper CNI plugin.
> 
> 
> Diffs
> -
> 
>   support/mesos-mini/Dockerfile f8af422d8b8b0be4c462d40bce2d1337ab8b6e95 
>   support/mesos-mini/mesos/agent_environment 
> 48767281987c2e70b3a64f28c5eac139435c17f6 
>   support/mesos-mini/mesos/master_environment 
> 0382558744cb693c862246856b8c16ff42a95d19 
>   support/mesos-mini/mesos/ucr-default-bridge.json PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69538/diff/1/
> 
> 
> Testing
> ---
> 
> Tested using this Marathon app
> ```json
> {
>   "id": "/test",
>   "instances": 1,
>   "container": {
> "type": "MESOS",
> "volumes": [],
> "portMappings": [],
> "docker": {
>   "image": "alpine"
> }
>   },
>   "cpus": 0.1,
>   "mem": 128,
>   "requirePorts": false,
>   "networks": [
> {
>   "mode": "container/bridge"
> }
>   ],
>   "healthChecks": [],
>   "fetch": [],
>   "constraints": [],
>   "cmd": "sleep 10"
> }
> ```
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 69480: Made non-root containers can access shared persistent volume.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 1:15 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Made non-root containers can access shared persistent volume.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
8f76944cd9058c0ba809443e32a3d8c8a26ac4a6 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
c7d753ac2e5575a8d687600bfb9e0617fa72c990 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
deacc909a2d323671667cb646c019664bdb660e7 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
f91a2eeb835bb65a855eeb314d4c69e3b58fecae 
  src/slave/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION 


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


Testing
---


Thanks,

Qian Zhang



Review Request 69544: Made non-root containers can access shared persistent volume.

2018-12-10 Thread Qian Zhang

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

Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Made non-root containers can access shared persistent volume.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
8f76944cd9058c0ba809443e32a3d8c8a26ac4a6 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
2a9ea448d7f963f86e8b2909d83e82b498e4104c 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
deacc909a2d323671667cb646c019664bdb660e7 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
f91a2eeb835bb65a855eeb314d4c69e3b58fecae 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
PRE-CREATION 


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


Testing
---


Thanks,

Qian Zhang



Review Request 69542: Made non-root containers can access SANDBOX_PATH volume of PARENT type.

2018-12-10 Thread Qian Zhang

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

Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

If a nested container running as a non-root user tries to use a
SANDBOX_PATH volume of PARENT type, we will make sure the volume owned
by a unique gid allocated by the volume gid manager and the container
process launched with that gid as its supplementary group.


Diffs
-

  include/mesos/slave/containerizer.proto 
5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 
  src/slave/containerizer/mesos/containerizer.hpp 
3102b8755c1fa3b205081d0198c6021c02d15ec6 
  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp 
1631160236379f84c6e1ed1be1370b5f2f2fd563 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
ecd467c5a33c2f41396bc72ddd7cb806bb8adc52 
  src/slave/containerizer/mesos/launch.cpp 
2f1c9e7a8748c9d7eab25bc8567ca68308e680f9 


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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 69540: Added `ROOT_PseudoDevicesWithRootFilesystem` test.

2018-12-10 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On Dec. 10, 2018, 4:41 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69540/
> ---
> 
> (Updated Dec. 10, 2018, 4:41 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-9462
> https://issues.apache.org/jira/browse/MESOS-9462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that pseudo devices like /dev/random are properly
> mounted in the container's root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
>   src/tests/containerizer/rootfs.cpp 83662fc2c2b9ea902b444bab9c2911df01ee11d5 
> 
> 
> Diff: https://reviews.apache.org/r/69540/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 69478: Added `task_supplementary_groups` into `ContainerLaunchInfo`.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 1:09 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Added `task_supplementary_groups` into `ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 


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

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


Testing
---


Thanks,

Qian Zhang



Review Request 69543: Implemented recovery for volume gid manager.

2018-12-10 Thread Qian Zhang

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

Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Implemented recovery for volume gid manager.


Diffs
-

  src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
  src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
  src/slave/containerizer/mesos/containerizer.cpp 
a5cf2da55c046c5c45e0c2ca3400f64de12de62b 
  src/slave/containerizer/mesos/volume_gid_manager/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/state.proto PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
PRE-CREATION 


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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 69540: Added `ROOT_PseudoDevicesWithRootFilesystem` test.

2018-12-10 Thread James Peach

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


Fix it, then Ship it!





src/tests/containerizer/linux_filesystem_isolator_tests.cpp
Lines 137 (patched)


As a separate commit, can we factor out the flags set up from all these 
test case?



src/tests/containerizer/linux_filesystem_isolator_tests.cpp
Lines 156 (patched)


Writing to /dev/full should fail with ENOSPC, but I don't see a 
straightforward way to verify that with dd.


- James Peach


On Dec. 10, 2018, 4:41 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69540/
> ---
> 
> (Updated Dec. 10, 2018, 4:41 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-9462
> https://issues.apache.org/jira/browse/MESOS-9462
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that pseudo devices like /dev/random are properly
> mounted in the container's root filesystem.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
>   src/tests/containerizer/rootfs.cpp 83662fc2c2b9ea902b444bab9c2911df01ee11d5 
> 
> 
> Diff: https://reviews.apache.org/r/69540/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 69344: Added volume gid manager to Mesos agent.

2018-12-10 Thread Qian Zhang

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

(Updated Dec. 11, 2018, 1 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Added volume gid manager to Mesos agent.


Diffs
-

  src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
  src/Makefile.am 8da1a05b618f17542fec9b5057484a9bae57658a 
  src/slave/volume_gid_manager/volume_gid_manager.hpp PRE-CREATION 
  src/slave/volume_gid_manager/volume_gid_manager.cpp PRE-CREATION 


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


Testing
---


Thanks,

Qian Zhang



Review Request 69541: Added volume gid manager.

2018-12-10 Thread Qian Zhang

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

Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin, 
and Jie Yu.


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


Repository: mesos


Description
---

Added volume gid manager.


Diffs
-

  src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
  src/Makefile.am 35500516e18ff251357b9e8d6bba894c44a9253b 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp 
PRE-CREATION 


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


Testing
---


Thanks,

Qian Zhang



Review Request 69540: Added `ROOT_PseudoDevicesWithRootFilesystem` test.

2018-12-10 Thread Andrei Budnik

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

Review request for mesos, Jie Yu and James Peach.


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


Repository: mesos


Description
---

This test verifies that pseudo devices like /dev/random are properly
mounted in the container's root filesystem.


Diffs
-

  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
84b342cdd4b8ef4803725ecfa9f922687ccdadd8 
  src/tests/containerizer/rootfs.cpp 83662fc2c2b9ea902b444bab9c2911df01ee11d5 


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


Testing
---

internal CI


Thanks,

Andrei Budnik



Re: Review Request 69474: Added tests for agent/executor heartbeating.

2018-12-10 Thread Benno Evers

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


Fix it, then Ship it!





src/tests/executor_http_api_tests.cpp
Lines 1001 (patched)


That's a pretty cool technique!



src/tests/executor_http_api_tests.cpp
Lines 1232 (patched)


Let's please fix this `TODO` before committing.


- Benno Evers


On Dec. 7, 2018, 10:18 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69474/
> ---
> 
> (Updated Dec. 7, 2018, 10:18 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds two separate tests which check if the Agent sends heartbeats
> to HTTP executors, and if the HTTP executor driver sends heartbeats
> to the agent.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> b87a8069892b3886329581e86ed15072dd785b3d 
>   src/examples/test_http_executor.cpp 
> e5f7cbb3b4e35663717f2a74550f8378bfafb0f9 
>   src/tests/executor_http_api_tests.cpp 
> a635e1a3c228b52f47ba81a67753ea49ce092143 
>   src/tests/mesos.hpp b57bbd40051fed0829453c56b90f5660354f4334 
> 
> 
> Diff: https://reviews.apache.org/r/69474/diff/5/
> 
> 
> Testing
> ---
> 
> ```
> cmake --build . --target check
> src/mesos-tests --gtest_filter="*Heartbeat*" --verbose --gtest_repeat=-1 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69473: Added heartbeaters for agent and HTTP executors.

2018-12-10 Thread Benno Evers

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




src/executor/executor.cpp
Lines 798 (patched)


I think you're adding a constant in `src/slave/constants.hpp` in this 
review, so we could use it here. (or if you want to keep the intervals in both 
directions separate, let's at least define this as a constant within this file, 
instead of using a naked value here)



src/slave/http.cpp
Lines 862 (patched)


I wonder: This `switch` statement doesn't have a `default` case, so if a 
new executor sends a heartbeat to an old agent, wouldn't it skip past the 
`switch` into the `UNREACHABLE` and kill the agent?


- Benno Evers


On Dec. 7, 2018, 10:17 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69473/
> ---
> 
> (Updated Dec. 7, 2018, 10:17 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements two separate heartbeaters for Executor Events (agent
> to executor) and Executor Calls (executor to agent).  Both are set to
> non-configurable intervals of 30 minutes, which should be sufficient
> to keep the connections alive while not flooding logs with warnings
> if the executor/agent does not have this patch.
> 
> 
> Diffs
> -
> 
>   src/common/validation.cpp 5f8f2889f79f9486f8bb731b39f706908dcd84ea 
>   src/executor/executor.cpp d00ea32f02c1d92d64525b8b5ce5ca18ac840c1b 
>   src/executor/v0_v1executor.cpp aebdbe705dac604da44aba484dc4d007e83b8e37 
>   src/launcher/default_executor.cpp b89b0363d64b8dd8936a62b57803f0eff50bfc71 
>   src/launcher/executor.cpp 6b1204dc5a67d4fc3e8ceae378262d53f3d28421 
>   src/slave/constants.hpp fdc21a35bcc0028dc0a2d7ebd06171fad2a20ba1 
>   src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c 
>   src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
>   src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
> 
> 
> Diff: https://reviews.apache.org/r/69473/diff/4/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69514: Added gauge metric for operator event stream subscribers.

2018-12-10 Thread Benno Evers

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


Ship it!




The `active_`-prefix could probably be dropped without information loss, but 
this is purely subjective so feel free to keep the current name if you prefer.

- Benno Evers


On Dec. 6, 2018, 2:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69514/
> ---
> 
> (Updated Dec. 6, 2018, 2:21 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Gastón Kleiman, and 
> Greg Mann.
> 
> 
> Bugs: MESOS-9258
> https://issues.apache.org/jira/browse/MESOS-9258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This metric "master/active_operator_event_stream_subscribers" returns
> the total number of subscribers to the master's operator event stream.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
>   docs/operator-http-api.md 6edf361047ee5915514da892c6d885b74a510a72 
>   src/master/master.cpp 3b3824a67f46866cd64e32d7f9f92484b5891aa2 
>   src/master/metrics.hpp f6bb89da9394bfe469bc690ff2de66824e994098 
>   src/master/metrics.cpp f69ed52c1c89912e7a5d26cb9409f5959b09111a 
>   src/tests/api_tests.cpp edde48a4b4b46c9f47bc44de389c97b77322b8e8 
>   src/tests/master_tests.cpp ef2c00101fc3d30c564a9ca34884dece2cdd2651 
> 
> 
> Diff: https://reviews.apache.org/r/69514/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> make check
> 
> src/mesos-tests --gtest_filter="*MaxEventStreamSubscribers*" 
> --gtest_repeat=-1 --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69463: Added HEARTBEAT events and calls for the executor HTTP API.

2018-12-10 Thread Benno Evers

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


Ship it!




Ship It!

- Benno Evers


On Dec. 6, 2018, 12:15 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69463/
> ---
> 
> (Updated Dec. 6, 2018, 12:15 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These new messages are meant to be backwards compatible, in that
> they won't cause crashes when new executors send heartbeats to old
> agents, or new agents send heartbeats to old executors.  All recipients
> of these heartbeats are currently expected to ignore them, as their
> only purpose is to keep certain connections from being marked "stale"
> by network intermediaries.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 1b5fa5dab6944a8649fb98447eeec7105495b879 
>   include/mesos/v1/executor/executor.proto 
> b2ef325cf6a72137854355d541889c7c6ae7c230 
> 
> 
> Diff: https://reviews.apache.org/r/69463/diff/2/
> 
> 
> Testing
> ---
> 
> cmake --build . --target mesos-protobufs
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69472: Refactored master and agent streaming connections.

2018-12-10 Thread Benno Evers

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


Fix it, then Ship it!





src/common/heartbeater.hpp
Lines 17 (patched)


Nit: I guess the include guard should be `__COMMON_HEARTBEATER_HPP__`.



src/common/http.hpp
Lines 161 (patched)


Nit: Add backticks around `Event`.



src/master/framework.cpp
Line 684 (original), 679 (patched)


Not related to this review, but I noticed while reading this that we're 
relying here on the thread-safety of `metrics`, but the fact concurrent 
modifications to `Metrics` are safe is only asserted in a comment in 
`master.hpp` but nowhere in libprocess itself.


- Benno Evers


On Dec. 6, 2018, 1:23 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69472/
> ---
> 
> (Updated Dec. 6, 2018, 1:23 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-7564
> https://issues.apache.org/jira/browse/MESOS-7564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This moves the very similar `HttpConnection` classes inside the
> master and agent into a common header.  The refactored
> `StreamingHttpConnection` is more explicitly named to avoid
> potentially clashing with the libprocess HTTP helpers.
> 
> This also moves the master's heartbeater helper into a new header
> and transforms it into an RAII libprocess actor wrapper.  The 
> heartbeater depends on this `StreamingHttpConnection` and is currently
> used by the master for heartbeating the operator event stream
> and HTTP framework connection.  A later patch will use this heartbeater
> for agent->executor heartbeats.
> 
> 
> Diffs
> -
> 
>   src/common/heartbeater.hpp PRE-CREATION 
>   src/common/http.hpp ac9ed5e8b1c6e8e100eba401136eb95c9dd621d4 
>   src/master/framework.cpp 36eda9f287bef28608024e0036db1f955a1b393c 
>   src/master/http.cpp 68ee2a6dcffbc772afec6e797b1af8da48f61937 
>   src/master/master.hpp c7becfa615964674dcf1ebd9424aa5818a0fdb85 
>   src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa 
>   src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c 
>   src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 
>   src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 
> 
> 
> Diff: https://reviews.apache.org/r/69472/diff/4/
> 
> 
> Testing
> ---
> 
> make
> 
> The original plan was to use the same helper for the executor too, but of the 
> Master, Framework, and Agent heartbeats, only the executor lacks a streaming 
> connection.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 69307: Changed master to hold subscribers in a circular buffer.

2018-12-10 Thread Benno Evers

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


Ship it!




Ship It!

- Benno Evers


On Dec. 5, 2018, 10:01 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69307/
> ---
> 
> (Updated Dec. 5, 2018, 10:01 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Gastón Kleiman, and 
> Greg Mann.
> 
> 
> Bugs: MESOS-9258
> https://issues.apache.org/jira/browse/MESOS-9258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a flag (--max_operator_event_stream_subscribers) to the
> master which controls how many active subscribers on the Master's
> event stream will be allowed at any time.
> 
> The default is 1000 subscribers, which is purposefully higher
> than we expect is needed.  Operators aware that their  network
> has clients/proxies whom do not close connections have the
> option of lowering this flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md 575476728079d884fe86b1a3a56614647d1b572e 
>   src/master/constants.hpp 76ad0c3b1494cb97888af5545acc998b8fb15bf6 
>   src/master/flags.hpp ed2d76af2831d1b9718724936fd3e4850e99b332 
>   src/master/flags.cpp 2677738d19caa7d600e591aeb4fb37f0c2318d45 
>   src/master/master.hpp c7becfa615964674dcf1ebd9424aa5818a0fdb85 
>   src/master/master.cpp 3b3824a67f46866cd64e32d7f9f92484b5891aa2 
>   src/tests/api_tests.cpp edde48a4b4b46c9f47bc44de389c97b77322b8e8 
> 
> 
> Diff: https://reviews.apache.org/r/69307/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> make check
> 
> src/mesos-tests --gtest_filter="*MaxEventStreamSubscribers*" 
> --gtest_repeat=-1 --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>