Re: Review Request 64226: Added a `ns::supported` convenience API.

2018-02-23 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [64226]

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

- Mesos Reviewbot


On Dec. 1, 2017, 3:46 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64226/
> ---
> 
> (Updated Dec. 1, 2017, 3:46 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-8284
> https://issues.apache.org/jira/browse/MESOS-8284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `ns::supported` convenience API which directly expresses the
> intent to probe whether a specific Linux namespace is supported on
> the running kernel and takes care of kernel versioning special cases.
> 
> This API subsumes the previous usages of `ns::namespaces`, so that API
> is now used only retained as an internal helper.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 8b6406754340d5bc244047a9d8c9fad5cead8f4d 
>   src/linux/ns.cpp 49f8fca5884da7425f2fed5e8b598ef7cc4dc6e1 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
> 90773d7b237ac83899d161ab251b4eecda02b3b9 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> d765929c2d5133ad5eac9f8a6c4d68ceda922eea 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> d60052e9e1ccdfaaac6c0db6acfdae325cea6a9e 
>   src/tests/containerizer/ns_tests.cpp 
> 61adf8525b1df1a70d7fd9c747d2106630a7627d 
>   src/tests/containerizer/setns_test_helper.cpp 
> 045caf684baf6e724fa5f99b6cc3feeb87817ba3 
> 
> 
> Diff: https://reviews.apache.org/r/64226/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 64226: Added a `ns::supported` convenience API.

2018-02-23 Thread Mesos Reviewbot Windows

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



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

Reviews applied: `['64226']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226

Relevant logs:

- 
[stout-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/stout-tests-stdout.log):

```
[ RUN  ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile
[   OK ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile (4 ms)
[ RUN  ] RmdirTest.RemoveDirectoryButPreserveRoot
[   OK ] RmdirTest.RemoveDirectoryButPreserveRoot (5 ms)
[--] 11 tests from RmdirTest (52 ms total)

[--] 1 test from SocketTests
[ RUN  ] SocketTests.InitSocket
[   OK ] SocketTests.InitSocket (1 ms)
[--] 1 test from SocketTests (2 ms total)

[--] 2 tests from StrerrorTest
[ RUN  ] StrerrorTest.ValidErrno
[   OK ] StrerrorTest.ValidErrno (0 ms)
[ RUN  ] StrerrorTest.InvalidErrno
[   OK ] StrerrorTest.InvalidErrno (0 ms)
[--] 2 tests from StrerrorTest (2 ms total)

[--] Global test environment tear-down
[==] 273 tests from 46 test cases ran. (4259 ms total)
[  PASSED  ] 270 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] OsTest.SYMLINK_Realpath
[  FAILED  ] FsTest.CreateDirectoryAtMaxPath
[  FAILED  ] FsTest.CreateDirectoryLongerThanMaxPath

 3 FAILED TESTS
  YOU HAVE 7 DISABLED TESTS

```

- 
[stout-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/stout-tests-stderr.log):

```
'invalid.command' is not recognized as an internal or external command,
operable program or batch file.
Subcommand 'subcommand' is not available
Usage: command  [OPTIONS]

Available subcommands:
help
subcommand2

Multiple subcommands have name 'subcommand'
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/mesos-tests-stdout.log):

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (98 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (920 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (33 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (37 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (71 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2145 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2167 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2270 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2292 ms total)

[--] Global test environment tear-down
[==] 914 tests from 90 test cases ran. (408828 ms total)
[  PASSED  ] 913 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] FilesTest.DebugTest

 1 FAILED TEST
  YOU HAVE 211 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/mesos-tests-stderr.log):

```
I0224 03:59:35.364686  7040 master.cpp:10258] Updating the state of task 
1059220c-2489-4b2f-9d6e-90dfe9c2a62b of framework 
5495faf4-2635-4db8-8011-7e84191b8829- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0224 03:59:35.364686  1100 slave.cpp:3879] Shutting down framework 
5495faf4-2635-4db8-8011-7e84191b8829-
I0224 03:59:35.365685  1100 slave.cpp:6586] Shutting down executor 
'1059220c-2489-4b2f-9d6e-90dfe9c2a62b' of framework 
5495faf4-2635-4db8-8011-7e84191b8829- at executor(1)@10.3.1.5:59961
I0224 03:59:35.365685  1100 slave.cpp:922] Agent terminating
W0224 03:59:35.365685  1100 slave.cpp:3875] Ignoring shutdown framework 
5495faf4-2635-4db8-I0224 03:59:34.712641  4684 exec.cpp:162] Version: 1.6.0
I0224 03:59:34.736641  8352 exec.cpp:236] Executor registered on agent 
5495faf4-2635-4db8-8011-7e84191b8829-S0
I0224 03:59:34.740639  2900 executor.cpp:174] Received SUBSCRIBED event
I0224 03:59:34.744666  2900 executor.cpp:178] Subscribed executor on 
build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0224 03:59:34.744666  2900 executor.cpp:174] Received LAUNCH event
I0224 03:59:34.749668  2900 executor.cpp:646] Starting task 

Re: Review Request 65775: Added a master API test for agent re-registration after master failover.

2018-02-23 Thread Mesos Reviewbot Windows

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



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

Reviews applied: `['65774', '65775']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65775

Relevant logs:

- 
[stout-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65775/logs/stout-tests-stdout.log):

```
[ RUN  ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile
[   OK ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile (4 ms)
[ RUN  ] RmdirTest.RemoveDirectoryButPreserveRoot
[   OK ] RmdirTest.RemoveDirectoryButPreserveRoot (6 ms)
[--] 11 tests from RmdirTest (51 ms total)

[--] 1 test from SocketTests
[ RUN  ] SocketTests.InitSocket
[   OK ] SocketTests.InitSocket (1 ms)
[--] 1 test from SocketTests (2 ms total)

[--] 2 tests from StrerrorTest
[ RUN  ] StrerrorTest.ValidErrno
[   OK ] StrerrorTest.ValidErrno (0 ms)
[ RUN  ] StrerrorTest.InvalidErrno
[   OK ] StrerrorTest.InvalidErrno (0 ms)
[--] 2 tests from StrerrorTest (2 ms total)

[--] Global test environment tear-down
[==] 273 tests from 46 test cases ran. (4207 ms total)
[  PASSED  ] 270 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] OsTest.SYMLINK_Realpath
[  FAILED  ] FsTest.CreateDirectoryAtMaxPath
[  FAILED  ] FsTest.CreateDirectoryLongerThanMaxPath

 3 FAILED TESTS
  YOU HAVE 7 DISABLED TESTS

```

- 
[stout-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65775/logs/stout-tests-stderr.log):

```
'invalid.command' is not recognized as an internal or external command,
operable program or batch file.
Subcommand 'subcommand' is not available
Usage: command  [OPTIONS]

Available subcommands:
help
subcommand2

Multiple subcommands have name 'subcommand'
```

- 
[libprocess-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65775/logs/libprocess-tests-stdout.log):

```
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (1050 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
D:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp(446): error: 
(await_subprocess(client.get(), 0)).failure(): 
[++] Subprocess output.
[==] Running 1 test from 1 test case.

[--] Global test environment set-up.

[--] 1 test from SSLClientTest

[ RUN  ] SSLClientTest.client

[   OK ] SSLClientTest.client (19 ms)

[--] 1 test from SSLClientTest (19 ms total)



[--] Global test environment tear-down

[==] 1 test from 1 test case ran. (19 ms total)

[  PASSED  ] 1 test.

[++]

[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1, where GetParam() = 
"true" (1001 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (2844 ms total)

[--] Global test environment tear-down
[==] 225 tests from 35 test cases ran. (35128 ms total)
[  PASSED  ] 224 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1, where GetParam() = 
"true"

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

- 
[libprocess-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65775/logs/libprocess-tests-stderr.log):

```
I0224 02:38:18.807351  3920 process.cpp:929] Stopped the socket accept loop
I0224 02:38:18.837352  4664 process.cpp:929] Stopped the socket accept loop
I0224 02:38:19.246352  3920 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0224 02:38:19.246352  3920 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0224 02:38:19.246352  3920 openssl.cpp:561] Using CA file: 
C:\Windows\TEMP\u5LLQX\cert.pem
I0224 02:38:19.246352  3920 openssl.cpp:564] Using CA dir: 
C:\Windows\TEMP\u5LLQX
I0224 02:38:19.627352  3920 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0224 02:38:19.627352  3920 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I0224 02:38:19.627352  3920 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0224 02:38:19.627352  3920 openssl.cpp:561] Using CA file: 
C:\Windows\TEMP\96arb8\cert.pem
I0224 02:38:19.627352  3920 openssl.cpp:564] Using CA dir: 
C:\Windows\TEMP\96arb8
I0224 02:38:20.078384  3920 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with 

Re: Review Request 65774: Fixed a master API bug for agent re-registration after master failover.

2018-02-23 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On Feb. 23, 2018, 8:08 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65774/
> ---
> 
> (Updated Feb. 23, 2018, 8:08 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-8601
> https://issues.apache.org/jira/browse/MESOS-8601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the master fails over and a client subscribes to the master before
> agent re-registration, the master will crash when sending `TASK_ADDED`
> because the framework info might not have been added to the master yet.
> This patch fixes this bug.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514 
>   src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44 
> 
> 
> Diff: https://reviews.apache.org/r/65774/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 65774: Fixed a master API bug for agent re-registration after master failover.

2018-02-23 Thread Greg Mann

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




src/master/master.hpp
Lines 2281-2285 (patched)


Now that I'm looking at the other methods in the Framework struct, I don't 
love that we're sending to subscribers here :-/

The only other use of the `master` member is to send messages to the 
framework itself. This new code entangles the Framework struct a bit more with 
unrelated master state, which perhaps isn't great.

I think this will work as a short-term solution, but maybe we can come up 
with something later on which will move this block back into 'master.cpp'.


- Greg Mann


On Feb. 23, 2018, 8:08 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65774/
> ---
> 
> (Updated Feb. 23, 2018, 8:08 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-8601
> https://issues.apache.org/jira/browse/MESOS-8601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the master fails over and a client subscribes to the master before
> agent re-registration, the master will crash when sending `TASK_ADDED`
> because the framework info might not have been added to the master yet.
> This patch fixes this bug.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514 
>   src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44 
> 
> 
> Diff: https://reviews.apache.org/r/65774/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 65785: Added a test for QuotaRequest validation.

2018-02-23 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [65334, 65514, 65515, 65780, 65781, 65782, 65783, 65784, 65785]

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

- Mesos Reviewbot


On Feb. 23, 2018, 2:43 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65785/
> ---
> 
> (Updated Feb. 23, 2018, 2:43 p.m.)
> 
> 
> Review request for mesos, Michael Park and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 77ec7fb37c9b9c168013d1d81ca862617682af6d 
> 
> 
> Diff: https://reviews.apache.org/r/65785/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 65775: Added a master API test for agent re-registration after master failover.

2018-02-23 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On Feb. 24, 2018, 2:25 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65775/
> ---
> 
> (Updated Feb. 24, 2018, 2:25 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-8601
> https://issues.apache.org/jira/browse/MESOS-8601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that subscribing to the 'api/v1' endpoint between a
> master failover and an agent re-registration won't cause the master to
> crash.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 31906eb4ede52c70fd538e4536c4f416eee5ca67 
> 
> 
> Diff: https://reviews.apache.org/r/65775/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> This test would result in a check failure without r65774.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 64226: Added a `ns::supported` convenience API.

2018-02-23 Thread Mesos Reviewbot Windows

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



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

Reviews applied: `['64226']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226

Relevant logs:

- 
[stout-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/stout-tests-stdout.log):

```
[ RUN  ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile
[   OK ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile (4 ms)
[ RUN  ] RmdirTest.RemoveDirectoryButPreserveRoot
[   OK ] RmdirTest.RemoveDirectoryButPreserveRoot (5 ms)
[--] 11 tests from RmdirTest (49 ms total)

[--] 1 test from SocketTests
[ RUN  ] SocketTests.InitSocket
[   OK ] SocketTests.InitSocket (1 ms)
[--] 1 test from SocketTests (2 ms total)

[--] 2 tests from StrerrorTest
[ RUN  ] StrerrorTest.ValidErrno
[   OK ] StrerrorTest.ValidErrno (0 ms)
[ RUN  ] StrerrorTest.InvalidErrno
[   OK ] StrerrorTest.InvalidErrno (0 ms)
[--] 2 tests from StrerrorTest (2 ms total)

[--] Global test environment tear-down
[==] 273 tests from 46 test cases ran. (4138 ms total)
[  PASSED  ] 270 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] OsTest.SYMLINK_Realpath
[  FAILED  ] FsTest.CreateDirectoryAtMaxPath
[  FAILED  ] FsTest.CreateDirectoryLongerThanMaxPath

 3 FAILED TESTS
  YOU HAVE 7 DISABLED TESTS

```

- 
[stout-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/stout-tests-stderr.log):

```
'invalid.command' is not recognized as an internal or external command,
operable program or batch file.
Subcommand 'subcommand' is not available
Usage: command  [OPTIONS]

Available subcommands:
help
subcommand2

Multiple subcommands have name 'subcommand'
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/mesos-tests-stdout.log):

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (104 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (941 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (39 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (42 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (83 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2152 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2174 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2265 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2286 ms total)

[--] Global test environment tear-down
[==] 912 tests from 90 test cases ran. (401596 ms total)
[  PASSED  ] 911 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] FilesTest.DebugTest

 1 FAILED TEST
  YOU HAVE 211 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/mesos-tests-stderr.log):

```
I0224 02:27:00.366788  6516 slave.cpp:3879] Shutting down framework 
6cf16fe6-43dd-460f-b8be-1c925d723cce-
I0224 02:27:00.366788 10236 master.cpp:10248] Updating the state of task 
5320fa4b-5c3a-4a5a-82b6-77cf18849108 of framework 
6cf16fe6-43dd-460f-b8be-1c925d723cce- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0224 02:27:00.366788  6516 slave.cpp:6586] Shutting down executor 
'5320fa4b-5c3a-4a5a-82b6-77cf18849108' of framework 
6cf16fe6-43dd-460f-b8be-1c925d723cce- at executor(1)@10.3.1.5:55480
I0224 02:27:00.367931  6516 slave.cpp:922] Agent terminating
W0224 02:27:00.369148  6516 slave.cpp:3875] IgnoriI0224 02:26:59.718770  3544 
exec.cpp:162] Version: 1.6.0
I0224 02:26:59.751807  7352 exec.cpp:236] Executor registered on agent 
6cf16fe6-43dd-460f-b8be-1c925d723cce-S0
I0224 02:26:59.755766  5084 executor.cpp:174] Received SUBSCRIBED event
I0224 02:26:59.759766  5084 executor.cpp:178] Subscribed executor on 
build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0224 02:26:59.759766  5084 executor.cpp:174] Received LAUNCH event
I0224 02:26:59.765765  5084 executor.cpp:646] Starting task 
5320fa4b-5c3a-4a5a-82b6-77cf18849108
I0224 02:26:59.845770  5084 

Re: Review Request 65775: Added a master API test for agent re-registration after master failover.

2018-02-23 Thread Chun-Hung Hsiao

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

(Updated Feb. 24, 2018, 2:25 a.m.)


Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

This test verifies that subscribing to the 'api/v1' endpoint between a
master failover and an agent re-registration won't cause the master to
crash.


Diffs (updated)
-

  src/tests/api_tests.cpp 31906eb4ede52c70fd538e4536c4f416eee5ca67 


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

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


Testing
---

sudo make check

This test would result in a check failure without r65774.


Thanks,

Chun-Hung Hsiao



Re: Review Request 65683: Updated discard handling of the Docker 'inspect' call.

2018-02-23 Thread Greg Mann

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




src/docker/docker.cpp
Lines 1287 (patched)


Derp. This will clearly not work because the `s` which we capture into the 
`onDiscard` callback is a different copy than the `s` which we reassign in 
`_inspect()`.

It probably makes sense to use a `shared_ptr` here, and do an 
`atomic_exchange` to reassign. This will maintain thread safety, and keep the 
object alive until the `onDiscard` callback has executed.


- Greg Mann


On Feb. 23, 2018, 11:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 11:07 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling of the Docker 'inspect' call.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65775: Added a master API test for agent re-registration after master failover.

2018-02-23 Thread Greg Mann

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




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


s/See: https://issues.apache.org/jira/browse/MESOS-8601/See MESOS-8601./


- Greg Mann


On Feb. 24, 2018, 12:46 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65775/
> ---
> 
> (Updated Feb. 24, 2018, 12:46 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-8601
> https://issues.apache.org/jira/browse/MESOS-8601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that subscribing to the 'api/v1' endpoint between a
> master failover and an agent re-registration won't cause the master to
> crash.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 31906eb4ede52c70fd538e4536c4f416eee5ca67 
> 
> 
> Diff: https://reviews.apache.org/r/65775/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> This test would result in a check failure without r65774.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 65792: Fixed compilation error in SLRP.

2018-02-23 Thread Benjamin Mahler

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


Ship it!




Ship It!

- Benjamin Mahler


On Feb. 24, 2018, 1:21 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65792/
> ---
> 
> (Updated Feb. 24, 2018, 1:21 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed compilation error in SLRP.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 33abc0e05a804969ae14da9cb9c58698ba1aaea5 
> 
> 
> Diff: https://reviews.apache.org/r/65792/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Review Request 65792: Fixed compilation error in SLRP.

2018-02-23 Thread Chun-Hung Hsiao

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Fixed compilation error in SLRP.


Diffs
-

  src/resource_provider/storage/provider.cpp 
33abc0e05a804969ae14da9cb9c58698ba1aaea5 


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


Testing
---

make


Thanks,

Chun-Hung Hsiao



Re: Review Request 52064: Support for multiple versions of docs.

2018-02-23 Thread Benjamin Mahler

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



Vinod and I went over this, a couple of issues:

* The [endpoint 
documentation](http://mesos.apache.org/documentation/latest/endpoints/) is only 
being generated for latest (see 
https://github.com/apache/mesos/blob/master/support/mesos-website/build.sh), 
this needs to be generated for each version.
* The C++ and Java API documentation is only being generated for latest (not 
sure how easy this is to fix, maybe if you also tackle this do so in a seperate 
patch). Also note that home.md linked directly to "/api/latest/c++" and 
"/api/latest/java" which means that the old versions would send users to latest 
c++ and java docs.
* We had a hard time figuring why some of the code changes were needed or what 
they were doing (e.g. some of the new regexes, or why the version selector 
drop-down has an empty value for the first entry), some comments would be 
helpful for the reader.

We were thinking the following approach would be good alternative:

* In order to ensure that we generate the HTTP help endpoint documentation 
correctly for each version, the 
[build.sh](https://github.com/apache/mesos/blob/master/support/mesos-website/build.sh)
 script would loop over each 1.x.y tag (excluding -rc#'s) and checkout the tag 
into a sub-directory. Within this sub-directory, we would build mesos and 
generate the help endpoints documentation. This sub-directory would probably be 
named directly as the version (e.g. "1.0.0", "1.4.1", etc).
* Note that for efficiency reasons (it's very expensive to build mesos for so 
many tags), the build.sh script would skip a tag if it finds that the publish 
directory already exists for the version in the mesos-site repository.
* The Rakefile would be updated to look at each version checkout subdirectory, 
and would generate the docs using the documentation folder within that specific 
version subdirectory.
* Not sure if the releases YAML still needs the docs tag, it seems so since 
it's used in the javascript. This would at least need to be updated so that 
everything before 1.0.0 is "false".
* Is it possible to put the version selector on the right side of the blue 
banner?

- Benjamin Mahler


On Feb. 13, 2018, 3:44 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated Feb. 13, 2018, 3:44 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the website build process to allow for documentation to be 
> generated for all versions of Mesos, based on the existing Git tags.  
> Additionally, the website is updated with a dropdown list of the available 
> versions of documentation.  If a user changes versions and the old version 
> does not have the current page, a notice will be displayed and the user 
> redirected to the home page for the desired version.
> 
> This is a temporary measure to "catchup" the documentation for the website by 
> generating docs for all previous versions.  Going forward, as each new 
> version of Mesos is released the documentation will be generated for that 
> version only.
> 
> ![Dropdown list of versions.](https://i.imgur.com/xvukEBGl.png)
> Screenshot of the dropdown list of versions.
> 
> ![404 Message](https://i.imgur.com/kqXNsxvl.png)
> Screenshot of the message displayed if a page does not exist for the selected 
> version.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 877fe914a9787c140848fdf9958571fec5fa58ff 
>   site/Gemfile.lock 909f3f3badeaa47c80929e243ce36307766edee4 
>   site/Rakefile 31ef6ffe225ce7ddc573054058af1070b9e96b09 
>   site/config.rb 04bc7aa1e0ac61ce5d89fd53d32f265532996913 
>   site/data/releases.yml 56fd0fc7f5e34873c9b088778d77f9a6718a5933 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 8a07488940f3793d6fdd291dbe896e098f321c96 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/7/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>



Re: Review Request 65775: Added a master API test for agent re-registration after master failover.

2018-02-23 Thread Chun-Hung Hsiao

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

(Updated Feb. 24, 2018, 12:46 a.m.)


Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.


Changes
---

Fixed the flakiness of the test.


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


Repository: mesos


Description
---

This test verifies that subscribing to the 'api/v1' endpoint between a
master failover and an agent re-registration won't cause the master to
crash.


Diffs (updated)
-

  src/tests/api_tests.cpp 31906eb4ede52c70fd538e4536c4f416eee5ca67 


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

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


Testing
---

sudo make check

This test would result in a check failure without r65774.


Thanks,

Chun-Hung Hsiao



Re: Review Request 65791: Removed stale generated HTTP endpoint documentation.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65791`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65791

Relevant logs:

- 
[apply-review-65791-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65791/logs/apply-review-65791-stdout.log):

```

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'SYSTEM@build-srv-04.(none)')
```

- Mesos Reviewbot Windows


On Feb. 24, 2018, 12:29 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65791/
> ---
> 
> (Updated Feb. 24, 2018, 12:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-8442
> https://issues.apache.org/jira/browse/MESOS-8442
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is now generated by the website CI bot, rather than having
> to be checked in and kept up to date within the docs/ folder.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/files/browse.json.md 
> 6b21e50bb0c0098babedcd612a5acfd0e6ae0aab 
>   docs/endpoints/files/browse.md 0fa204a85fc62aa4b43dacc07aec6951b618a015 
>   docs/endpoints/files/debug.json.md 2806a5844f49a3c051e58db0723879c9627e18e1 
>   docs/endpoints/files/debug.md 33380663eab59c704c610bd392676aee99528f77 
>   docs/endpoints/files/download.json.md 
> 31d442a7e1e3d4c6e32986ec85e42f3364f37de9 
>   docs/endpoints/files/download.md 202d015484b22e63c9d24610808a43d091786844 
>   docs/endpoints/files/read.json.md 64f348e76d61d3d0ad364f4712d3c1d90818d6d1 
>   docs/endpoints/files/read.md ef95c80739eeb09f52658470429c6ffb9b41fb8c 
>   docs/endpoints/index.md 871153774c4b21cdca4afec9bc2ce2bd955b17c4 
>   docs/endpoints/logging/toggle.md 014a1969a7efb709e06c0c90ca023f2fa747414e 
>   docs/endpoints/master/api/v1.md 2d778f33c830caccb0254cf15adc46a86b5db48a 
>   docs/endpoints/master/api/v1/scheduler.md 
> f029d8e6e90bc7d974d60ad4a71d63807f0e901a 
>   docs/endpoints/master/create-volumes.md 
> c9ce39ead8041f249ee6f85d1afdd8f90b38ce9b 
>   docs/endpoints/master/destroy-volumes.md 
> ff82e96a6ee1e9f6e1bbc565ab40c98b758c1227 
>   docs/endpoints/master/flags.md 6b3de41d45278cc34a4f4f0e52b76407ab96b790 
>   docs/endpoints/master/frameworks.md 
> 7aa22130dfa8702d8b93690cb75f1323ea3c2223 
>   docs/endpoints/master/health.md 3752a1bb4a9a7c2cc5cbf25b021592280c174a80 
>   docs/endpoints/master/machine/down.md 
> 05abd155e693163eb50f675d2724534a6f87dfba 
>   docs/endpoints/master/machine/up.md 
> 8a3488bb3f96bc7470aaea5114559cb593cbb3a8 
>   docs/endpoints/master/maintenance/schedule.md 
> 86ae30b06118f13593e91b70dbec97945b1d9bd8 
>   docs/endpoints/master/maintenance/status.md 
> f8e1c7eed44e14a8324b582a2db91e523b2fbc15 
>   docs/endpoints/master/quota.md 6ce7811136ac1c4ae00a3011dca17cbde948f14e 
>   docs/endpoints/master/redirect.md 3c568e3eef658f9a489008f4f84745827f8002f0 
>   docs/endpoints/master/reserve.md 8174027f0e66dc4e9ae7bca951ed9ca38033008d 
>   docs/endpoints/master/roles.json.md 
> 9acba2cfed120918b2571d7336651d125cbe 
>   docs/endpoints/master/roles.md de58120172c390ef1f084031077fd9b865b817db 
>   docs/endpoints/master/slaves.md 9365cdb9e5b17cdf26ed38a0ab1fff8afef38f00 
>   docs/endpoints/master/state-summary.md 
> 21b8703bb011daf5cc560550e6c2ce36ba615bba 
>   docs/endpoints/master/state.json.md 
> 62a723051270ab24591b956805aee5455848201b 
>   docs/endpoints/master/state.md 09a478263d4cd015040f47619b0aa94341c3bba2 
>   docs/endpoints/master/tasks.json.md 
> f9553d7bde7de33b02795ba9088a88e68cb2a0cc 
>   docs/endpoints/master/tasks.md 019a14bb6c8d8acd3aab4a1f7ba6893f4bad835b 
>   docs/endpoints/master/teardown.md b017084e3d195363615a59011671fdbd76182a1a 
>   docs/endpoints/master/unreserve.md 324dcded2013aeb6c167a695bbad1da69340cd57 
>   docs/endpoints/master/weights.md 3c9019e74867cbd931dc5c68a0d8de3ea269e5b2 
>   docs/endpoints/metrics/snapshot.md 0912fa939b38fd1d8335e165c7aeddf1f076 
>   docs/endpoints/profiler/start.md a25f0d0468f2ad9f8d1ff41f6c50ea0bbe87d316 
>   docs/endpoints/profiler/stop.md 5028d13f8a3f9aa3d41b60959a9cc3d6b03fb7d9 
>   docs/endpoints/registrar/registry.md 
> e347db269839d60fac6de0777b245e830279661e 
>   docs/endpoints/slave/api/v1.md b394f7fac2c4600b7e11f5c7f4def44edc22e7db 
>   docs/endpoints/slave/api/v1/executor.md 
> 5ef40e3ca5ad072b5bbfd480d4e0b56d9530a17f 
>  

Re: Review Request 65791: Removed stale generated HTTP endpoint documentation.

2018-02-23 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 24, 2018, 12:29 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65791/
> ---
> 
> (Updated Feb. 24, 2018, 12:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Vinod Kone.
> 
> 
> Bugs: MESOS-8442
> https://issues.apache.org/jira/browse/MESOS-8442
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is now generated by the website CI bot, rather than having
> to be checked in and kept up to date within the docs/ folder.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/files/browse.json.md 
> 6b21e50bb0c0098babedcd612a5acfd0e6ae0aab 
>   docs/endpoints/files/browse.md 0fa204a85fc62aa4b43dacc07aec6951b618a015 
>   docs/endpoints/files/debug.json.md 2806a5844f49a3c051e58db0723879c9627e18e1 
>   docs/endpoints/files/debug.md 33380663eab59c704c610bd392676aee99528f77 
>   docs/endpoints/files/download.json.md 
> 31d442a7e1e3d4c6e32986ec85e42f3364f37de9 
>   docs/endpoints/files/download.md 202d015484b22e63c9d24610808a43d091786844 
>   docs/endpoints/files/read.json.md 64f348e76d61d3d0ad364f4712d3c1d90818d6d1 
>   docs/endpoints/files/read.md ef95c80739eeb09f52658470429c6ffb9b41fb8c 
>   docs/endpoints/index.md 871153774c4b21cdca4afec9bc2ce2bd955b17c4 
>   docs/endpoints/logging/toggle.md 014a1969a7efb709e06c0c90ca023f2fa747414e 
>   docs/endpoints/master/api/v1.md 2d778f33c830caccb0254cf15adc46a86b5db48a 
>   docs/endpoints/master/api/v1/scheduler.md 
> f029d8e6e90bc7d974d60ad4a71d63807f0e901a 
>   docs/endpoints/master/create-volumes.md 
> c9ce39ead8041f249ee6f85d1afdd8f90b38ce9b 
>   docs/endpoints/master/destroy-volumes.md 
> ff82e96a6ee1e9f6e1bbc565ab40c98b758c1227 
>   docs/endpoints/master/flags.md 6b3de41d45278cc34a4f4f0e52b76407ab96b790 
>   docs/endpoints/master/frameworks.md 
> 7aa22130dfa8702d8b93690cb75f1323ea3c2223 
>   docs/endpoints/master/health.md 3752a1bb4a9a7c2cc5cbf25b021592280c174a80 
>   docs/endpoints/master/machine/down.md 
> 05abd155e693163eb50f675d2724534a6f87dfba 
>   docs/endpoints/master/machine/up.md 
> 8a3488bb3f96bc7470aaea5114559cb593cbb3a8 
>   docs/endpoints/master/maintenance/schedule.md 
> 86ae30b06118f13593e91b70dbec97945b1d9bd8 
>   docs/endpoints/master/maintenance/status.md 
> f8e1c7eed44e14a8324b582a2db91e523b2fbc15 
>   docs/endpoints/master/quota.md 6ce7811136ac1c4ae00a3011dca17cbde948f14e 
>   docs/endpoints/master/redirect.md 3c568e3eef658f9a489008f4f84745827f8002f0 
>   docs/endpoints/master/reserve.md 8174027f0e66dc4e9ae7bca951ed9ca38033008d 
>   docs/endpoints/master/roles.json.md 
> 9acba2cfed120918b2571d7336651d125cbe 
>   docs/endpoints/master/roles.md de58120172c390ef1f084031077fd9b865b817db 
>   docs/endpoints/master/slaves.md 9365cdb9e5b17cdf26ed38a0ab1fff8afef38f00 
>   docs/endpoints/master/state-summary.md 
> 21b8703bb011daf5cc560550e6c2ce36ba615bba 
>   docs/endpoints/master/state.json.md 
> 62a723051270ab24591b956805aee5455848201b 
>   docs/endpoints/master/state.md 09a478263d4cd015040f47619b0aa94341c3bba2 
>   docs/endpoints/master/tasks.json.md 
> f9553d7bde7de33b02795ba9088a88e68cb2a0cc 
>   docs/endpoints/master/tasks.md 019a14bb6c8d8acd3aab4a1f7ba6893f4bad835b 
>   docs/endpoints/master/teardown.md b017084e3d195363615a59011671fdbd76182a1a 
>   docs/endpoints/master/unreserve.md 324dcded2013aeb6c167a695bbad1da69340cd57 
>   docs/endpoints/master/weights.md 3c9019e74867cbd931dc5c68a0d8de3ea269e5b2 
>   docs/endpoints/metrics/snapshot.md 0912fa939b38fd1d8335e165c7aeddf1f076 
>   docs/endpoints/profiler/start.md a25f0d0468f2ad9f8d1ff41f6c50ea0bbe87d316 
>   docs/endpoints/profiler/stop.md 5028d13f8a3f9aa3d41b60959a9cc3d6b03fb7d9 
>   docs/endpoints/registrar/registry.md 
> e347db269839d60fac6de0777b245e830279661e 
>   docs/endpoints/slave/api/v1.md b394f7fac2c4600b7e11f5c7f4def44edc22e7db 
>   docs/endpoints/slave/api/v1/executor.md 
> 5ef40e3ca5ad072b5bbfd480d4e0b56d9530a17f 
>   docs/endpoints/slave/api/v1/resource_provider.md 
> c721c6d005224807bdb4fbfcf4fd7c30edb82e6c 
>   docs/endpoints/slave/containers.md f8bcb3847a6e29dc98d424cdfeacfce26ad98d67 
>   docs/endpoints/slave/flags.md 4a8a0361124078342887c291acaa73c9f6f4bc4d 
>   docs/endpoints/slave/health.md 988567cdbe885dbfe1270732f84e529b13983546 
>   docs/endpoints/slave/monitor/statistics.json.md 
> 51d7c39ff994aeabf0bb805a087e8d0f8c85c818 
>   docs/endpoints/slave/monitor/statistics.md 
> 342638f028885c7c27a76ccea3213567f64731dd 
>   docs/endpoints/slave/state.json.md 0f82c1926404e79b281b2ea5f4d0ca21323aeded 
>   docs/endpoints/slave/state.md b34459e8624f0b29e927ff79be7fc845ac88080b 
>   

Re: Review Request 65791: Removed stale generated HTTP endpoint documentation.

2018-02-23 Thread Benjamin Mahler

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

(Updated Feb. 24, 2018, 12:29 a.m.)


Review request for mesos, Benjamin Bannier and Vinod Kone.


Changes
---

Added JIRA.


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


Repository: mesos


Description
---

This is now generated by the website CI bot, rather than having
to be checked in and kept up to date within the docs/ folder.


Diffs
-

  docs/endpoints/files/browse.json.md 6b21e50bb0c0098babedcd612a5acfd0e6ae0aab 
  docs/endpoints/files/browse.md 0fa204a85fc62aa4b43dacc07aec6951b618a015 
  docs/endpoints/files/debug.json.md 2806a5844f49a3c051e58db0723879c9627e18e1 
  docs/endpoints/files/debug.md 33380663eab59c704c610bd392676aee99528f77 
  docs/endpoints/files/download.json.md 
31d442a7e1e3d4c6e32986ec85e42f3364f37de9 
  docs/endpoints/files/download.md 202d015484b22e63c9d24610808a43d091786844 
  docs/endpoints/files/read.json.md 64f348e76d61d3d0ad364f4712d3c1d90818d6d1 
  docs/endpoints/files/read.md ef95c80739eeb09f52658470429c6ffb9b41fb8c 
  docs/endpoints/index.md 871153774c4b21cdca4afec9bc2ce2bd955b17c4 
  docs/endpoints/logging/toggle.md 014a1969a7efb709e06c0c90ca023f2fa747414e 
  docs/endpoints/master/api/v1.md 2d778f33c830caccb0254cf15adc46a86b5db48a 
  docs/endpoints/master/api/v1/scheduler.md 
f029d8e6e90bc7d974d60ad4a71d63807f0e901a 
  docs/endpoints/master/create-volumes.md 
c9ce39ead8041f249ee6f85d1afdd8f90b38ce9b 
  docs/endpoints/master/destroy-volumes.md 
ff82e96a6ee1e9f6e1bbc565ab40c98b758c1227 
  docs/endpoints/master/flags.md 6b3de41d45278cc34a4f4f0e52b76407ab96b790 
  docs/endpoints/master/frameworks.md 7aa22130dfa8702d8b93690cb75f1323ea3c2223 
  docs/endpoints/master/health.md 3752a1bb4a9a7c2cc5cbf25b021592280c174a80 
  docs/endpoints/master/machine/down.md 
05abd155e693163eb50f675d2724534a6f87dfba 
  docs/endpoints/master/machine/up.md 8a3488bb3f96bc7470aaea5114559cb593cbb3a8 
  docs/endpoints/master/maintenance/schedule.md 
86ae30b06118f13593e91b70dbec97945b1d9bd8 
  docs/endpoints/master/maintenance/status.md 
f8e1c7eed44e14a8324b582a2db91e523b2fbc15 
  docs/endpoints/master/quota.md 6ce7811136ac1c4ae00a3011dca17cbde948f14e 
  docs/endpoints/master/redirect.md 3c568e3eef658f9a489008f4f84745827f8002f0 
  docs/endpoints/master/reserve.md 8174027f0e66dc4e9ae7bca951ed9ca38033008d 
  docs/endpoints/master/roles.json.md 9acba2cfed120918b2571d7336651d125cbe 
  docs/endpoints/master/roles.md de58120172c390ef1f084031077fd9b865b817db 
  docs/endpoints/master/slaves.md 9365cdb9e5b17cdf26ed38a0ab1fff8afef38f00 
  docs/endpoints/master/state-summary.md 
21b8703bb011daf5cc560550e6c2ce36ba615bba 
  docs/endpoints/master/state.json.md 62a723051270ab24591b956805aee5455848201b 
  docs/endpoints/master/state.md 09a478263d4cd015040f47619b0aa94341c3bba2 
  docs/endpoints/master/tasks.json.md f9553d7bde7de33b02795ba9088a88e68cb2a0cc 
  docs/endpoints/master/tasks.md 019a14bb6c8d8acd3aab4a1f7ba6893f4bad835b 
  docs/endpoints/master/teardown.md b017084e3d195363615a59011671fdbd76182a1a 
  docs/endpoints/master/unreserve.md 324dcded2013aeb6c167a695bbad1da69340cd57 
  docs/endpoints/master/weights.md 3c9019e74867cbd931dc5c68a0d8de3ea269e5b2 
  docs/endpoints/metrics/snapshot.md 0912fa939b38fd1d8335e165c7aeddf1f076 
  docs/endpoints/profiler/start.md a25f0d0468f2ad9f8d1ff41f6c50ea0bbe87d316 
  docs/endpoints/profiler/stop.md 5028d13f8a3f9aa3d41b60959a9cc3d6b03fb7d9 
  docs/endpoints/registrar/registry.md e347db269839d60fac6de0777b245e830279661e 
  docs/endpoints/slave/api/v1.md b394f7fac2c4600b7e11f5c7f4def44edc22e7db 
  docs/endpoints/slave/api/v1/executor.md 
5ef40e3ca5ad072b5bbfd480d4e0b56d9530a17f 
  docs/endpoints/slave/api/v1/resource_provider.md 
c721c6d005224807bdb4fbfcf4fd7c30edb82e6c 
  docs/endpoints/slave/containers.md f8bcb3847a6e29dc98d424cdfeacfce26ad98d67 
  docs/endpoints/slave/flags.md 4a8a0361124078342887c291acaa73c9f6f4bc4d 
  docs/endpoints/slave/health.md 988567cdbe885dbfe1270732f84e529b13983546 
  docs/endpoints/slave/monitor/statistics.json.md 
51d7c39ff994aeabf0bb805a087e8d0f8c85c818 
  docs/endpoints/slave/monitor/statistics.md 
342638f028885c7c27a76ccea3213567f64731dd 
  docs/endpoints/slave/state.json.md 0f82c1926404e79b281b2ea5f4d0ca21323aeded 
  docs/endpoints/slave/state.md b34459e8624f0b29e927ff79be7fc845ac88080b 
  docs/endpoints/system/stats.json.md 87e470aa9d29af4ee0e62d7a6d5561e784ef8818 
  docs/endpoints/version.md 4f5db22baef76a499a657745f1275ecd97bffbcd 


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


Testing
---

None


Thanks,

Benjamin Mahler



Review Request 65791: Removed stale generated HTTP endpoint documentation.

2018-02-23 Thread Benjamin Mahler

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

Review request for mesos, Benjamin Bannier and Vinod Kone.


Repository: mesos


Description
---

This is now generated by the website CI bot, rather than having
to be checked in and kept up to date within the docs/ folder.


Diffs
-

  docs/endpoints/files/browse.json.md 6b21e50bb0c0098babedcd612a5acfd0e6ae0aab 
  docs/endpoints/files/browse.md 0fa204a85fc62aa4b43dacc07aec6951b618a015 
  docs/endpoints/files/debug.json.md 2806a5844f49a3c051e58db0723879c9627e18e1 
  docs/endpoints/files/debug.md 33380663eab59c704c610bd392676aee99528f77 
  docs/endpoints/files/download.json.md 
31d442a7e1e3d4c6e32986ec85e42f3364f37de9 
  docs/endpoints/files/download.md 202d015484b22e63c9d24610808a43d091786844 
  docs/endpoints/files/read.json.md 64f348e76d61d3d0ad364f4712d3c1d90818d6d1 
  docs/endpoints/files/read.md ef95c80739eeb09f52658470429c6ffb9b41fb8c 
  docs/endpoints/index.md 871153774c4b21cdca4afec9bc2ce2bd955b17c4 
  docs/endpoints/logging/toggle.md 014a1969a7efb709e06c0c90ca023f2fa747414e 
  docs/endpoints/master/api/v1.md 2d778f33c830caccb0254cf15adc46a86b5db48a 
  docs/endpoints/master/api/v1/scheduler.md 
f029d8e6e90bc7d974d60ad4a71d63807f0e901a 
  docs/endpoints/master/create-volumes.md 
c9ce39ead8041f249ee6f85d1afdd8f90b38ce9b 
  docs/endpoints/master/destroy-volumes.md 
ff82e96a6ee1e9f6e1bbc565ab40c98b758c1227 
  docs/endpoints/master/flags.md 6b3de41d45278cc34a4f4f0e52b76407ab96b790 
  docs/endpoints/master/frameworks.md 7aa22130dfa8702d8b93690cb75f1323ea3c2223 
  docs/endpoints/master/health.md 3752a1bb4a9a7c2cc5cbf25b021592280c174a80 
  docs/endpoints/master/machine/down.md 
05abd155e693163eb50f675d2724534a6f87dfba 
  docs/endpoints/master/machine/up.md 8a3488bb3f96bc7470aaea5114559cb593cbb3a8 
  docs/endpoints/master/maintenance/schedule.md 
86ae30b06118f13593e91b70dbec97945b1d9bd8 
  docs/endpoints/master/maintenance/status.md 
f8e1c7eed44e14a8324b582a2db91e523b2fbc15 
  docs/endpoints/master/quota.md 6ce7811136ac1c4ae00a3011dca17cbde948f14e 
  docs/endpoints/master/redirect.md 3c568e3eef658f9a489008f4f84745827f8002f0 
  docs/endpoints/master/reserve.md 8174027f0e66dc4e9ae7bca951ed9ca38033008d 
  docs/endpoints/master/roles.json.md 9acba2cfed120918b2571d7336651d125cbe 
  docs/endpoints/master/roles.md de58120172c390ef1f084031077fd9b865b817db 
  docs/endpoints/master/slaves.md 9365cdb9e5b17cdf26ed38a0ab1fff8afef38f00 
  docs/endpoints/master/state-summary.md 
21b8703bb011daf5cc560550e6c2ce36ba615bba 
  docs/endpoints/master/state.json.md 62a723051270ab24591b956805aee5455848201b 
  docs/endpoints/master/state.md 09a478263d4cd015040f47619b0aa94341c3bba2 
  docs/endpoints/master/tasks.json.md f9553d7bde7de33b02795ba9088a88e68cb2a0cc 
  docs/endpoints/master/tasks.md 019a14bb6c8d8acd3aab4a1f7ba6893f4bad835b 
  docs/endpoints/master/teardown.md b017084e3d195363615a59011671fdbd76182a1a 
  docs/endpoints/master/unreserve.md 324dcded2013aeb6c167a695bbad1da69340cd57 
  docs/endpoints/master/weights.md 3c9019e74867cbd931dc5c68a0d8de3ea269e5b2 
  docs/endpoints/metrics/snapshot.md 0912fa939b38fd1d8335e165c7aeddf1f076 
  docs/endpoints/profiler/start.md a25f0d0468f2ad9f8d1ff41f6c50ea0bbe87d316 
  docs/endpoints/profiler/stop.md 5028d13f8a3f9aa3d41b60959a9cc3d6b03fb7d9 
  docs/endpoints/registrar/registry.md e347db269839d60fac6de0777b245e830279661e 
  docs/endpoints/slave/api/v1.md b394f7fac2c4600b7e11f5c7f4def44edc22e7db 
  docs/endpoints/slave/api/v1/executor.md 
5ef40e3ca5ad072b5bbfd480d4e0b56d9530a17f 
  docs/endpoints/slave/api/v1/resource_provider.md 
c721c6d005224807bdb4fbfcf4fd7c30edb82e6c 
  docs/endpoints/slave/containers.md f8bcb3847a6e29dc98d424cdfeacfce26ad98d67 
  docs/endpoints/slave/flags.md 4a8a0361124078342887c291acaa73c9f6f4bc4d 
  docs/endpoints/slave/health.md 988567cdbe885dbfe1270732f84e529b13983546 
  docs/endpoints/slave/monitor/statistics.json.md 
51d7c39ff994aeabf0bb805a087e8d0f8c85c818 
  docs/endpoints/slave/monitor/statistics.md 
342638f028885c7c27a76ccea3213567f64731dd 
  docs/endpoints/slave/state.json.md 0f82c1926404e79b281b2ea5f4d0ca21323aeded 
  docs/endpoints/slave/state.md b34459e8624f0b29e927ff79be7fc845ac88080b 
  docs/endpoints/system/stats.json.md 87e470aa9d29af4ee0e62d7a6d5561e784ef8818 
  docs/endpoints/version.md 4f5db22baef76a499a657745f1275ecd97bffbcd 


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


Testing
---

None


Thanks,

Benjamin Mahler



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [65786, 65787, 65751, 65750]

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

- Mesos Reviewbot


On Feb. 23, 2018, 11:26 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65750/
> ---
> 
> (Updated Feb. 23, 2018, 11:26 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for a hung 'docker inspect' during container pull.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65750/diff/4/
> 
> 
> Testing
> ---
> 
> sudo bin/mesos-tests.sh 
> --gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 64226: Added a `ns::supported` convenience API.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 64226`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226

Relevant logs:

- 
[apply-review-64226-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64226/logs/apply-review-64226-stdout.log):

```

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'SYSTEM@build-srv-04.(none)')
```

- Mesos Reviewbot Windows


On Dec. 1, 2017, 11:46 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64226/
> ---
> 
> (Updated Dec. 1, 2017, 11:46 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-8284
> https://issues.apache.org/jira/browse/MESOS-8284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `ns::supported` convenience API which directly expresses the
> intent to probe whether a specific Linux namespace is supported on
> the running kernel and takes care of kernel versioning special cases.
> 
> This API subsumes the previous usages of `ns::namespaces`, so that API
> is now used only retained as an internal helper.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp e24d79a41eefcade343b2825b5a758d7d30a5f91 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
> 2d89d598d24e3bcf01d652ce3f586c9e3ccfc20b 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> 4f8253b58018581e022eb1832b9b07703cbd318d 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> d60052e9e1ccdfaaac6c0db6acfdae325cea6a9e 
>   src/tests/containerizer/ns_tests.cpp 
> 61adf8525b1df1a70d7fd9c747d2106630a7627d 
>   src/tests/containerizer/setns_test_helper.cpp 
> 045caf684baf6e724fa5f99b6cc3feeb87817ba3 
> 
> 
> Diff: https://reviews.apache.org/r/64226/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

2018-02-23 Thread Jie Yu

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




src/resource_provider/storage/provider.cpp
Line 1304 (original), 1329 (patched)


This flag is a bit confusing. When we will set this flag to `true`? Do we 
need to set it to true when `reconcileStoragePools` is called (currently, it's 
not)?

What if `GetCapacity` reports shrinked capacity? Should we drop 
`CreateVolume` in that case?



src/resource_provider/storage/provider.cpp
Lines 2968-2969 (patched)


Do you need a dispatch here?



src/resource_provider/storage/provider.cpp
Lines 2970-2971 (patched)


This is a bit confusing to me initially, and a bit hard to follow.

I think the goal here is trying to prevent concurrent reconciliation. 
However, currently in the code, we used two ways to enforce that:
1) the loop for detecting profile changes (to prevent two reconcileProfiles 
run concurrently
2) operationSequence to prevent reconcileProfiles and reconcileStoragePool 
run concurrently

Maybe we should use a unified way.



src/resource_provider/storage/provider.cpp
Lines 3087 (patched)


Do you want to add some CHECK here to check resource `has_disk`, 
`has_source` and `has_profile`?


- Jie Yu


On Feb. 13, 2018, 10:18 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated Feb. 13, 2018, 10:18 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and no
> longer assumes that profiles won't be removed by the disk profile
> adaptor. It will reject volume/block creation requests that use
> missing profiles, but existing volumes/blocks will continue to work.
> When a volume/block with a missing profiles is destroyed, it will be
> converted to an empty resource, and SLRP will update the sizes of
> storage pools for currently known profiles.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> 33abc0e05a804969ae14da9cb9c58698ba1aaea5 
>   src/resource_provider/storage/uri_disk_profile.cpp 
> 665798fdb085ea34f93bd287fe6f9ab29a265cbf 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> NOTE: This patch does not change the URI disk profile daptor to allow missing 
> profiles from upstream.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 64226: Added a `ns::supported` convenience API.

2018-02-23 Thread Jie Yu

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


Fix it, then Ship it!




LGTM


src/linux/ns.cpp
Lines 158 (patched)


nits: we don't use `&` for primitive types, `const int n`.


- Jie Yu


On Dec. 1, 2017, 11:46 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64226/
> ---
> 
> (Updated Dec. 1, 2017, 11:46 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-8284
> https://issues.apache.org/jira/browse/MESOS-8284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `ns::supported` convenience API which directly expresses the
> intent to probe whether a specific Linux namespace is supported on
> the running kernel and takes care of kernel versioning special cases.
> 
> This API subsumes the previous usages of `ns::namespaces`, so that API
> is now used only retained as an internal helper.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp e24d79a41eefcade343b2825b5a758d7d30a5f91 
>   src/linux/ns.cpp 5e2df1ed56432c6c1bfa04a31dba7f9f547d6139 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
> 2d89d598d24e3bcf01d652ce3f586c9e3ccfc20b 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> 4f8253b58018581e022eb1832b9b07703cbd318d 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> d60052e9e1ccdfaaac6c0db6acfdae325cea6a9e 
>   src/tests/containerizer/ns_tests.cpp 
> 61adf8525b1df1a70d7fd9c747d2106630a7627d 
>   src/tests/containerizer/setns_test_helper.cpp 
> 045caf684baf6e724fa5f99b6cc3feeb87817ba3 
> 
> 
> Diff: https://reviews.apache.org/r/64226/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 65785: Added a test for QuotaRequest validation.

2018-02-23 Thread Mesos Reviewbot Windows

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



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

Reviews applied: `['65334', '65514', '65515', '65780', '65781', '65782', 
'65783', '65784', '65785']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65785

Relevant logs:

- 
[stout-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65785/logs/stout-tests-stdout.log):

```
[ RUN  ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile
[   OK ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile (5 ms)
[ RUN  ] RmdirTest.RemoveDirectoryButPreserveRoot
[   OK ] RmdirTest.RemoveDirectoryButPreserveRoot (6 ms)
[--] 11 tests from RmdirTest (62 ms total)

[--] 1 test from SocketTests
[ RUN  ] SocketTests.InitSocket
[   OK ] SocketTests.InitSocket (2 ms)
[--] 1 test from SocketTests (2 ms total)

[--] 2 tests from StrerrorTest
[ RUN  ] StrerrorTest.ValidErrno
[   OK ] StrerrorTest.ValidErrno (0 ms)
[ RUN  ] StrerrorTest.InvalidErrno
[   OK ] StrerrorTest.InvalidErrno (0 ms)
[--] 2 tests from StrerrorTest (2 ms total)

[--] Global test environment tear-down
[==] 273 tests from 46 test cases ran. (5037 ms total)
[  PASSED  ] 270 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] OsTest.SYMLINK_Realpath
[  FAILED  ] FsTest.CreateDirectoryAtMaxPath
[  FAILED  ] FsTest.CreateDirectoryLongerThanMaxPath

 3 FAILED TESTS
  YOU HAVE 7 DISABLED TESTS

```

- 
[stout-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65785/logs/stout-tests-stderr.log):

```
'invalid.command' is not recognized as an internal or external command,
operable program or batch file.
Subcommand 'subcommand' is not available
Usage: command  [OPTIONS]

Available subcommands:
help
subcommand2

Multiple subcommands have name 'subcommand'
```

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65785/logs/mesos-tests-stdout.log):

```
[--] 9 tests from Endpoint/SlaveEndpointTest (1151 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (37 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (43 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (84 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2620 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2644 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2585 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2609 ms total)

[--] Global test environment tear-down
[==] 912 tests from 90 test cases ran. (477960 ms total)
[  PASSED  ] 910 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
[  FAILED  ] FilesTest.DebugTest

 2 FAILED TESTS
  YOU HAVE 211 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65785/logs/mesos-tests-stderr.log):

```
I0223 23:54:40.547863 10740 master.cpp:10248] Updating the state of task 
67e4097e-2190-45a6-bd93-f2756be81890 of framework 
ed6b543c-41bd-4cb1-b546-578dd370e8b7- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0223 23:54:40.547863  8928 slave.cpp:3879] Shutting down framework 
ed6b543c-41bd-4cb1-b546-578dd370e8b7-
I0223 23:54:40.548864  8928 slave.cpp:6586] Shutting down executor '67e409I0223 
23:54:39.823874 10912 exec.cpp:162] Version: 1.6.0
I0223 23:54:39.853849  4268 exec.cpp:236] Executor registered on agent 
ed6b543c-41bd-4cb1-b546-578dd370e8b7-S0
I0223 23:54:39.857852  2316 executor.cpp:174] Received SUBSCRIBED event
I0223 23:54:39.862853  2316 executor.cpp:178] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0223 23:54:39.863878  2316 executor.cpp:174] Received LAUNCH event
I0223 23:54:39.867851  2316 executor.cpp:646] Starting task 
67e4097e-2190-45a6-bd93-f2756be81890
I0223 23:54:39.951854  2316 executor.cpp:481] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0223 23:54:40.510931  2316 executor.cpp:659] Forked command at 10280
I0223 

Re: Review Request 65786: Prevented Docker library from terminating incorrect processes.

2018-02-23 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Feb. 23, 2018, 2:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65786/
> ---
> 
> (Updated Feb. 23, 2018, 2:37 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Docker library might call `os::killtree()` on a
> PID after the associated subprocess had already terminated, which
> could lead to an unknown process being incorrectly killed.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65786/diff/1/
> 
> 
> Testing
> ---
> 
> Testing details are found later in this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Greg Mann

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

(Updated Feb. 23, 2018, 11:26 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


Repository: mesos


Description
---

Added test for a hung 'docker inspect' during container pull.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


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

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


Testing
---

sudo bin/mesos-tests.sh 
--gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"


Thanks,

Greg Mann



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Greg Mann

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

(Updated Feb. 23, 2018, 11:24 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description (updated)
---

The new 'HungDockerTest' class allows test authors to force
certain Docker daemon calls to be delayed for a specified
duration.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Gilbert Song

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


Fix it, then Ship it!





src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5259-5267 (patched)


Should we just do:

```
  TaskInfo task = createTask(
  offer.slave_id(),
  offer.resources(),
  SLEEP_COMMAND(1000));
```


- Gilbert Song


On Feb. 23, 2018, 2:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65750/
> ---
> 
> (Updated Feb. 23, 2018, 2:41 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for a hung 'docker inspect' during container pull.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65750/diff/3/
> 
> 
> Testing
> ---
> 
> sudo bin/mesos-tests.sh 
> --gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 23, 2018, 2:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 23, 2018, 2:39 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test fixture for a hung Docker daemon.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/4/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65787: Updated discard handling for Docker 'stop' and 'pull' commands.

2018-02-23 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 23, 2018, 2:38 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65787/
> ---
> 
> (Updated Feb. 23, 2018, 2:38 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling for Docker 'stop' and 'pull' commands.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65787/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65786: Prevented Docker library from terminating incorrect processes.

2018-02-23 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 23, 2018, 2:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65786/
> ---
> 
> (Updated Feb. 23, 2018, 2:37 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the Docker library might call `os::killtree()` on a
> PID after the associated subprocess had already terminated, which
> could lead to an unknown process being incorrectly killed.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65786/diff/1/
> 
> 
> Testing
> ---
> 
> Testing details are found later in this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling of the Docker 'inspect' call.

2018-02-23 Thread Greg Mann


> On Feb. 23, 2018, 5:38 p.m., Gilbert Song wrote:
> > src/docker/docker.cpp
> > Lines 1199 (patched)
> > 
> >
> > Should we use a lambda instead of `lambda::bind`? (I know we want 
> > consistency in this file since we used `lambda::bind` in two other places)
> > 
> > e.g.,
> > ```
> > .onDiscard([=]() {
> >   return commandDiscarded();
> > }
> > ```
> > 
> > We used `lambda::bind` before we have the lambda support in libprocess.

I think I'll just leave it to be consistent with the other callsites if that's 
OK. We could update them all together.


- Greg


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


On Feb. 23, 2018, 11:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 11:07 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling of the Docker 'inspect' call.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling of the Docker 'inspect' call.

2018-02-23 Thread Greg Mann


> On Feb. 23, 2018, 5:38 p.m., Gilbert Song wrote:
> > src/docker/docker.cpp
> > Lines 151 (patched)
> > 
> >
> > Should we do `killtree()` even if the status is not pending (e.g., 
> > ready or failure)?
> > 
> > If the semantic (caller invokes `.discard()` -> no longer care about 
> > the subprocess) is something we want to define for the library, should we 
> > always killtree regardless whatever status?

I think it's incorrect for us to kill the process if `s.status()` is no longer 
pending. In that case, the process has actually terminated, so the PID could be 
some completely unrelated process which was spawned recently.


- Greg


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


On Feb. 23, 2018, 11:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 11:07 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling of the Docker 'inspect' call.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling of the Docker 'inspect' call.

2018-02-23 Thread Greg Mann


> On Feb. 23, 2018, 9:14 a.m., Greg Mann wrote:
> > src/docker/docker.cpp
> > Lines 1287-1293 (patched)
> > 
> >
> > Whoops, this is not threadsafe :facepalm:
> > 
> > For example, we could call `output->discard()` from two different 
> > threads simultaneously (see `__inspect`).
> > 
> > I need to figure out a better way to do this...
> 
> Andrei Budnik wrote:
> One possible solution might be using `defer(self(), ...)` for callbacks.
> 
> Gilbert Song wrote:
> yea, you may need `defer`.
> 
> Greg Mann wrote:
> Unfortunately, the docker library is not a libprocess process :'( So we 
> have nothing to defer to.

I got rid of the `Owned output`, and used a `Shared` for the 
subprocess. I think this works.


- Greg


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


On Feb. 23, 2018, 11:07 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 11:07 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling of the Docker 'inspect' call.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling of the Docker 'inspect' call.

2018-02-23 Thread Greg Mann

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

(Updated Feb. 23, 2018, 11:07 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and Vinod 
Kone.


Summary (updated)
-

Updated discard handling of the Docker 'inspect' call.


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


Repository: mesos


Description (updated)
---

Updated discard handling of the Docker 'inspect' call.


Diffs (updated)
-

  src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
  src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 


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

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


Testing
---


Thanks,

Greg Mann



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 65786.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65786`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65750

Relevant logs:

- 
[apply-review-65786-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65750/logs/apply-review-65786-stdout.log):

```

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'SYSTEM@build-srv-04.(none)')
```

- Mesos Reviewbot Windows


On Feb. 23, 2018, 10:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65750/
> ---
> 
> (Updated Feb. 23, 2018, 10:41 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for a hung 'docker inspect' during container pull.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65750/diff/3/
> 
> 
> Testing
> ---
> 
> sudo bin/mesos-tests.sh 
> --gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65514: Introduced a CHECK_NOTERROR macro.

2018-02-23 Thread Michael Park

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


Fix it, then Ship it!





3rdparty/stout/include/stout/check.hpp
Lines 123 (patched)


Ideally `t.error()` here would always return the `E` and we can simply 
access the message via `t.error().message`, but it returns `string` when `E == 
Error` for backward-compatibility.

One approach here would be to do `string(t.error())` and add a `operator 
const string&()` to `Error`.


- Michael Park


On Feb. 5, 2018, 2:06 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65514/
> ---
> 
> (Updated Feb. 5, 2018, 2:06 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/check.hpp 
> 6a33579b6c7590590fd3157e14cdc72bcc2bc27b 
> 
> 
> Diff: https://reviews.apache.org/r/65514/diff/1/
> 
> 
> Testing
> ---
> 
> make check, used in a subsequent patch
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Review Request 65785: Added a test for QuotaRequest validation.

2018-02-23 Thread Benjamin Mahler

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

Review request for mesos, Michael Park and Meng Zhu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_validation_tests.cpp 
77ec7fb37c9b9c168013d1d81ca862617682af6d 


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


Testing
---

make check


Thanks,

Benjamin Mahler



Review Request 65783: Added a test for validation of UPDATE_QUOTA master::Call.

2018-02-23 Thread Benjamin Mahler

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

Review request for mesos, Michael Park and Meng Zhu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_validation_tests.cpp 
77ec7fb37c9b9c168013d1d81ca862617682af6d 


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


Testing
---

make check


Thanks,

Benjamin Mahler



Review Request 65780: Removed an unnecessary argument from master::Call validation.

2018-02-23 Thread Benjamin Mahler

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

Review request for mesos, Michael Park and Meng Zhu.


Repository: mesos


Description
---

The principal is not needed by the validation function.


Diffs
-

  src/master/http.cpp acf10a0f396234ec9f444c9b41515d657379d6ad 
  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 


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


Testing
---

make check


Thanks,

Benjamin Mahler



Review Request 65784: Added validation of QuotaRequest.

2018-02-23 Thread Benjamin Mahler

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

Review request for mesos, Michael Park and Meng Zhu.


Repository: mesos


Description
---

This obviates the need for the old-style validation of quotaInfo,
but leaves the latter in order to keep the v0 code path untouched.
A TODO has been added to remove the old validation and replace it
with the one in this patch.


Diffs
-

  src/master/quota.hpp 7f43996d9aa3d3f462a6ed750733a55d4ef770c8 
  src/master/quota.cpp 58bab6a678bac9e41a7994ba0b7cc1ed069a8a18 


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


Testing
---

make check (test added in subsequent patch)


Thanks,

Benjamin Mahler



Review Request 65782: Implemented master::Call validation for UPDATE_QUOTA.

2018-02-23 Thread Benjamin Mahler

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

Review request for mesos, Michael Park and Meng Zhu.


Repository: mesos


Description
---

Note that this function does not currently validate the nested
messages, so this adheres to this for consistency.


Diffs
-

  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 


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


Testing
---

make check (test added in subsequent patch)


Thanks,

Benjamin Mahler



Review Request 65781: Added a TODO to clean up master::Call validation.

2018-02-23 Thread Benjamin Mahler

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

Review request for mesos, Michael Park and Meng Zhu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 


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


Testing
---

make check


Thanks,

Benjamin Mahler



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Greg Mann


> On Feb. 23, 2018, 6:12 p.m., Gilbert Song wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5251-5258 (patched)
> > 
> >
> > Instead of following the style in this file, could we do `createTask()`?

I used the `createTask` helper, but since we don't have an overload which 
accepts a ContainerInfo I still had to modify the TaskInfo. We could add a new 
overload in the future perhaps.


> On Feb. 23, 2018, 6:12 p.m., Gilbert Song wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5284-5285 (patched)
> > 
> >
> > Could we also verify that the statusFailed's reason is due to TIMEOUT?

The reason is actually due to container launch failure - I think this makes 
sense, since although the executor registration timeout triggers the status 
update, the container was never launched successfully. I added an expectation 
to check this.


- Greg


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


On Feb. 23, 2018, 10:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65750/
> ---
> 
> (Updated Feb. 23, 2018, 10:41 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for a hung 'docker inspect' during container pull.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65750/diff/3/
> 
> 
> Testing
> ---
> 
> sudo bin/mesos-tests.sh 
> --gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Greg Mann

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

(Updated Feb. 23, 2018, 10:41 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


Repository: mesos


Description (updated)
---

Added test for a hung 'docker inspect' during container pull.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


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

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


Testing
---

sudo bin/mesos-tests.sh 
--gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"


Thanks,

Greg Mann



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Greg Mann

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

(Updated Feb. 23, 2018, 10:39 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description (updated)
---

Added test fixture for a hung Docker daemon.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
d1e657050d623ad0412208b3aa3e3101e3654e99 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann



Review Request 65787: Updated discard handling for Docker 'stop' and 'pull' commands.

2018-02-23 Thread Greg Mann

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

Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description
---

Updated discard handling for Docker 'stop' and 'pull' commands.


Diffs
-

  src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 


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


Testing
---


Thanks,

Greg Mann



Review Request 65786: Prevented Docker library from terminating incorrect processes.

2018-02-23 Thread Greg Mann

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

Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.


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


Repository: mesos


Description
---

Previously, the Docker library might call `os::killtree()` on a
PID after the associated subprocess had already terminated, which
could lead to an unknown process being incorrectly killed.


Diffs
-

  src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 


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


Testing
---

Testing details are found later in this chain.


Thanks,

Greg Mann



Re: Review Request 65334: Added quota limit to the master API protos.

2018-02-23 Thread Benjamin Mahler

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

(Updated Feb. 23, 2018, 10:23 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Meng Zhu.


Changes
---

Increased the protobuf tag numbers to avoid a new collision after rebase.


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


Repository: mesos


Description
---

This introduces a `limit` in `QuotaInfo` and `QuotaRequest`.

A new `UPDATE_QUOTA` master call is added as a replacement of
`SET_QUOTA` and `REMOVE_QUOTA`, which provides the new semantics
of unset limit meaning "no limit" and unset guarantee meaning
"no guarantee".

The new APIs are marked as experimental for now.


Diffs (updated)
-

  include/mesos/master/master.proto f40caa2a6338ff58a325444cfb72cd24f2c99472 
  include/mesos/quota/quota.proto f2ed6f555e1e7b0290fefa3e301c9bdbf550bd11 
  include/mesos/v1/master/master.proto 67c9560eaa7bd6b20b73af07f7666f0d98c4453d 
  include/mesos/v1/quota/quota.proto 5dd772fee46f740b062827ed7c9704c26187cb12 
  src/master/http.cpp acf10a0f396234ec9f444c9b41515d657379d6ad 
  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 


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

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


Testing
---

make check


Thanks,

Benjamin Mahler



Re: Review Request 65515: Added rvalue reference Try::get overloads.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 65334.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65334`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65515

Relevant logs:

- 
[apply-review-65334-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65515/logs/apply-review-65334-stdout.log):

```

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'SYSTEM@build-srv-04.(none)')
```

- Mesos Reviewbot Windows


On Feb. 23, 2018, 10:16 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65515/
> ---
> 
> (Updated Feb. 23, 2018, 10:16 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-2664
> https://issues.apache.org/jira/browse/MESOS-2664
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/try.hpp 
> bcd55b9979b8268c8d34db3402fc5eac5e643e2c 
> 
> 
> Diff: https://reviews.apache.org/r/65515/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 65515: Added rvalue reference Try::get overloads.

2018-02-23 Thread Benjamin Mahler

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

(Updated Feb. 23, 2018, 10:16 p.m.)


Review request for mesos and Michael Park.


Changes
---

Updated per mpark's suggestion.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/stout/include/stout/try.hpp bcd55b9979b8268c8d34db3402fc5eac5e643e2c 


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

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


Testing
---

make check


Thanks,

Benjamin Mahler



Re: Review Request 65775: Added a master API test for agent re-registration after master failover.

2018-02-23 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [65774, 65775]

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

- Mesos Reviewbot


On Feb. 23, 2018, 8:11 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65775/
> ---
> 
> (Updated Feb. 23, 2018, 8:11 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-8601
> https://issues.apache.org/jira/browse/MESOS-8601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that subscribing to the 'api/v1' endpoint between a
> master failover and an agent re-registration won't cause the master to
> crash.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 31906eb4ede52c70fd538e4536c4f416eee5ca67 
> 
> 
> Diff: https://reviews.apache.org/r/65775/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> This test would result in a check failure without r65774.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 65774: Fixed a master API bug for agent re-registration after master failover.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65774`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65774

Relevant logs:

- 
[apply-review-65774-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65774/logs/apply-review-65774-stdout.log):

```

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'SYSTEM@build-srv-04.(none)')
```

- Mesos Reviewbot Windows


On Feb. 23, 2018, 8:08 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65774/
> ---
> 
> (Updated Feb. 23, 2018, 8:08 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan 
> Xu.
> 
> 
> Bugs: MESOS-8601
> https://issues.apache.org/jira/browse/MESOS-8601
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the master fails over and a client subscribes to the master before
> agent re-registration, the master will crash when sending `TASK_ADDED`
> because the framework info might not have been added to the master yet.
> This patch fixes this bug.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514 
>   src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44 
> 
> 
> Diff: https://reviews.apache.org/r/65774/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Review Request 65775: Added a master API test for agent re-registration after master failover.

2018-02-23 Thread Chun-Hung Hsiao

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

Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

This test verifies that subscribing to the 'api/v1' endpoint between a
master failover and an agent re-registration won't cause the master to
crash.


Diffs
-

  src/tests/api_tests.cpp 31906eb4ede52c70fd538e4536c4f416eee5ca67 


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


Testing
---

sudo make check

This test would result in a check failure without r65774.


Thanks,

Chun-Hung Hsiao



Review Request 65774: Fixed a master API bug for agent re-registration after master failover.

2018-02-23 Thread Chun-Hung Hsiao

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

Review request for mesos, Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

When the master fails over and a client subscribes to the master before
agent re-registration, the master will crash when sending `TASK_ADDED`
because the framework info might not have been added to the master yet.
This patch fixes this bug.


Diffs
-

  src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514 
  src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao



Re: Review Request 65674: Introduced helper for creating Mesos UUIDs.

2018-02-23 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [65587, 65588, 65589, 65590, 65591, 65673, 65674]

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

- Mesos Reviewbot


On Feb. 15, 2018, 2:54 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65674/
> ---
> 
> (Updated Feb. 15, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced helper for creating Mesos UUIDs.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266 
>   src/common/protobuf_utils.cpp f516091e9208552488c154f34adde6f20d3413bf 
>   src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377 
>   src/resource_provider/storage/provider.cpp 
> 33abc0e05a804969ae14da9cb9c58698ba1aaea5 
>   src/slave/slave.cpp e2facb3c15a2f907f6497c58a36842ed707f2c70 
>   src/tests/operation_status_update_manager_tests.cpp 
> c4429f40e1d7226be59d8ba4283bd91e16799d5a 
>   src/tests/resource_provider_manager_tests.cpp 
> c8997ec41fe0c3e02b0f6ab205c9009205c992da 
>   src/tests/resource_provider_validation_tests.cpp 
> 9f34c459e24396815f4a785815f83923e276874f 
>   src/tests/slave_tests.cpp e253317a67019302f18afe11e2a314e716cec226 
> 
> 
> Diff: https://reviews.apache.org/r/65674/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Greg Mann


> On Feb. 23, 2018, 6:06 p.m., Gilbert Song wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5148 (patched)
> > 
> >
> > Should we use `double`?

The `sleep` command only accepts integer arguments. I opted for `sleep` due to 
portability, but if there is a highly-portable option which allows sleeping for 
fractions of a second, that would be great.


- Greg


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


On Feb. 22, 2018, 11:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65713: Handled hanging docker `stop`, `inspect` commands in docker executor.

2018-02-23 Thread Andrei Budnik


> On Feb. 23, 2018, 6:24 p.m., Gilbert Song wrote:
> > src/docker/executor.cpp
> > Lines 319-321 (patched)
> > 
> >
> > Should we also call `.discard()` in this case?

We need to discard `inspect` only in `killTask()` and `reaped()` methods. So we 
give `inspect` a chance to complete, before a scheduler sends a `killTask` 
message.
BTW, this code is removed in the following patch in the chain.


- Andrei


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


On Feb. 22, 2018, 11:28 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65713/
> ---
> 
> (Updated Feb. 22, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previosly, if `docker inspect` command hanged, the docker container
> ended up in an unkillable state. This patch adds a timeout for inspect
> command after receiving `killTask` analogically to `reaped` handler.
> In addition we've added a timeout for `docker stop` command. If docker
> `stop` or `inspect` command times out, we discard the related future,
> thus the docker library kills previously spawned docker cli subprocess.
> As a result, a scheduler can retry `killTask` operation to handle
> nasty docker bugs that lead to hanging docker cli.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65713/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manual testing:
> 1. Build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 2. Modify `ContainerInspect` function from `docker/inspect.go`:
> ```
>  func (daemon *Daemon) ContainerInspect(name string, size bool, version 
> string) (interface{}, error) {
> +   time.Sleep(10 * time.Second)
> ```
> 3. Modify `ContainerStop` function from `docker/stop.go`:
> ```
>  func (daemon *Daemon) ContainerStop(name string, seconds *int) error {
> +   rand.Seed(time.Now().UTC().UnixNano())
> +   if rand.Intn(2) == 0 {
> +   time.Sleep(20 * time.Second)
> +   }
> ```
> 4. Rebuild docker: `sudo make build && sudo make binary`
> 5. Stop system docker daemon: `sudo service docker stop`
> 6. Start modified docker daemon: `sudo ./bundles/binary-daemon/dockerd-dev`
> 7. Modify `src/cli/execute.cpp`:
>   a) Add `delay(Seconds(15), self(), ::retryKill, task->task_id(), 
> offer.agent_id());` after 
> https://github.com/apache/mesos/blob/072ea2787ffca6f2a6dcb2d636f68c51823d6665/src/cli/execute.cpp#L606
>   b) Add a new method `retryKill` to `CommandScheduler`:
> ```
>   void retryKill(const TaskID& taskId, const AgentID& agentId)
>   {
> killTask(taskId, agentId);
> delay(Seconds(6), self(), ::retryKill, taskId, agentId);
>   }
> ```
> 8. Rebuild mesos
> 9. Run mesos master: `./bin/mesos-master.sh --work_dir='var/master-1'`
> 10. Run mesos agent: `GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/abudnik/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 11. Submit a task for the docker executor: `./src/mesos-execute 
> --master="127.0.1.1:5050" --name="a" --containerizer=docker 
> --docker_image="ubuntu:xenial" --command="sleep "`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 65683: Updated discard handling in Docker library functions.

2018-02-23 Thread Greg Mann


> On Feb. 23, 2018, 2:18 p.m., Andrei Budnik wrote:
> > src/docker/docker.cpp
> > Lines 1292 (patched)
> > 
> >
> > I think we don't need to discard the output, because the process will 
> > be killed, hence `io::read()` will be terminated after receiving EOF. Also, 
> > as we check `promise->future().hasDiscard()` before checking 
> > `output.isReady()` in `___inspect()`, the promise won't be set to `FAILED` 
> > state.

Ah good point, we can probably rely on the subprocess termination causing the 
read to end.


- Greg


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


On Feb. 23, 2018, 7:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 7:46 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling in Docker library functions.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling in Docker library functions.

2018-02-23 Thread Greg Mann


> On Feb. 23, 2018, 9:14 a.m., Greg Mann wrote:
> > src/docker/docker.cpp
> > Lines 1287-1293 (patched)
> > 
> >
> > Whoops, this is not threadsafe :facepalm:
> > 
> > For example, we could call `output->discard()` from two different 
> > threads simultaneously (see `__inspect`).
> > 
> > I need to figure out a better way to do this...
> 
> Andrei Budnik wrote:
> One possible solution might be using `defer(self(), ...)` for callbacks.
> 
> Gilbert Song wrote:
> yea, you may need `defer`.

Unfortunately, the docker library is not a libprocess process :'( So we have 
nothing to defer to.


- Greg


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


On Feb. 23, 2018, 7:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 7:46 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling in Docker library functions.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65713: Handled hanging docker `stop`, `inspect` commands in docker executor.

2018-02-23 Thread Gilbert Song

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




src/docker/executor.cpp
Lines 319-321 (patched)


Should we also call `.discard()` in this case?


- Gilbert Song


On Feb. 22, 2018, 3:28 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65713/
> ---
> 
> (Updated Feb. 22, 2018, 3:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previosly, if `docker inspect` command hanged, the docker container
> ended up in an unkillable state. This patch adds a timeout for inspect
> command after receiving `killTask` analogically to `reaped` handler.
> In addition we've added a timeout for `docker stop` command. If docker
> `stop` or `inspect` command times out, we discard the related future,
> thus the docker library kills previously spawned docker cli subprocess.
> As a result, a scheduler can retry `killTask` operation to handle
> nasty docker bugs that lead to hanging docker cli.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65713/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manual testing:
> 1. Build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 2. Modify `ContainerInspect` function from `docker/inspect.go`:
> ```
>  func (daemon *Daemon) ContainerInspect(name string, size bool, version 
> string) (interface{}, error) {
> +   time.Sleep(10 * time.Second)
> ```
> 3. Modify `ContainerStop` function from `docker/stop.go`:
> ```
>  func (daemon *Daemon) ContainerStop(name string, seconds *int) error {
> +   rand.Seed(time.Now().UTC().UnixNano())
> +   if rand.Intn(2) == 0 {
> +   time.Sleep(20 * time.Second)
> +   }
> ```
> 4. Rebuild docker: `sudo make build && sudo make binary`
> 5. Stop system docker daemon: `sudo service docker stop`
> 6. Start modified docker daemon: `sudo ./bundles/binary-daemon/dockerd-dev`
> 7. Modify `src/cli/execute.cpp`:
>   a) Add `delay(Seconds(15), self(), ::retryKill, task->task_id(), 
> offer.agent_id());` after 
> https://github.com/apache/mesos/blob/072ea2787ffca6f2a6dcb2d636f68c51823d6665/src/cli/execute.cpp#L606
>   b) Add a new method `retryKill` to `CommandScheduler`:
> ```
>   void retryKill(const TaskID& taskId, const AgentID& agentId)
>   {
> killTask(taskId, agentId);
> delay(Seconds(6), self(), ::retryKill, taskId, agentId);
>   }
> ```
> 8. Rebuild mesos
> 9. Run mesos master: `./bin/mesos-master.sh --work_dir='var/master-1'`
> 10. Run mesos agent: `GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/abudnik/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 11. Submit a task for the docker executor: `./src/mesos-execute 
> --master="127.0.1.1:5050" --name="a" --containerizer=docker 
> --docker_image="ubuntu:xenial" --command="sleep "`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Gilbert Song

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


Fix it, then Ship it!





src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5188 (patched)


s/ROOT_DOCKER_InspectDuringPull/ROOT_DOCKER_InspectHungDuringPull/g?



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5251-5258 (patched)


Instead of following the style in this file, could we do `createTask()`?



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5284-5285 (patched)


Could we also verify that the statusFailed's reason is due to TIMEOUT?


- Gilbert Song


On Feb. 22, 2018, 3:10 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65750/
> ---
> 
> (Updated Feb. 22, 2018, 3:10 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new test 'HungDockerTest.ROOT_DOCKER_InspectDuringPull'
> verifies that the Mesos agent will send a TASK_FAILED
> update for a Docker task which fails because the executor
> registration timeout elapses while the container is stuck
> in the PULLING state due to a hung 'docker inspect' call.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65750/diff/2/
> 
> 
> Testing
> ---
> 
> sudo bin/mesos-tests.sh 
> --gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song


> On Feb. 23, 2018, 8:35 a.m., Andrei Budnik wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp
> > Lines 5133 (patched)
> > 
> >
> > `os::write()` is not atomical, so we should write to a temporary file, 
> > then rename it: `mv new_env.tmp test-docker.env`. Otherwise, if someone 
> > launches docker cli, while the script is being updated - `test-docker.sh` 
> > might read/import half-written env file.
> > 
> > Probably, we don't need to fix it now.

A TODO sounds ok to me for now.


- Gilbert


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


On Feb. 22, 2018, 3:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 3:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 22, 2018, 3:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 3:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Gilbert Song

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




src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5148 (patched)


Should we use `double`?


- Gilbert Song


On Feb. 22, 2018, 3:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 3:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65743: Ensured that Docker containerizer returns a failed Future in one case.

2018-02-23 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 21, 2018, 4:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65743/
> ---
> 
> (Updated Feb. 21, 2018, 4:39 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Because the Docker library did not immediately transition the
> Future returned by `inspect()` to DISCARDED, it was safe for the
> Docker containerizer to discard this Future before failing the
> Promise associated with the Future returned by `launch()`.
> 
> However, the introduction of an `onDiscard` callback in
> `Docker::inspect()` makes this assumption invalid. This patch
> addresses this by failing the Promise before discarding the
> Future.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 7585178f34bdb782f0108d3ab94c23edfacc5564 
> 
> 
> Diff: https://reviews.apache.org/r/65743/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling in Docker library functions.

2018-02-23 Thread Gilbert Song


> On Feb. 23, 2018, 1:14 a.m., Greg Mann wrote:
> > src/docker/docker.cpp
> > Lines 1287-1293 (patched)
> > 
> >
> > Whoops, this is not threadsafe :facepalm:
> > 
> > For example, we could call `output->discard()` from two different 
> > threads simultaneously (see `__inspect`).
> > 
> > I need to figure out a better way to do this...
> 
> Andrei Budnik wrote:
> One possible solution might be using `defer(self(), ...)` for callbacks.

yea, you may need `defer`.


- Gilbert


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


On Feb. 22, 2018, 11:46 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 22, 2018, 11:46 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling in Docker library functions.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling in Docker library functions.

2018-02-23 Thread Gilbert Song

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




src/docker/docker.cpp
Lines 151 (patched)


Should we do `killtree()` even if the status is not pending (e.g., ready or 
failure)?

If the semantic (caller invokes `.discard()` -> no longer care about the 
subprocess) is something we want to define for the library, should we always 
killtree regardless whatever status?



src/docker/docker.cpp
Lines 1199 (patched)


Should we use a lambda instead of `lambda::bind`? (I know we want 
consistency in this file since we used `lambda::bind` in two other places)

e.g.,
```
.onDiscard([=]() {
  return commandDiscarded();
}
```

We used `lambda::bind` before we have the lambda support in libprocess.



src/docker/docker.cpp
Lines 1641 (patched)


ditto.


- Gilbert Song


On Feb. 22, 2018, 11:46 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 22, 2018, 11:46 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling in Docker library functions.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65750: Added test for a hung 'docker inspect' during container pull.

2018-02-23 Thread Andrei Budnik

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




src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5193-5195 (patched)


Can `CreateSlaveFlags` be overridden in `HungDockerTest`?


- Andrei Budnik


On Feb. 22, 2018, 11:10 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65750/
> ---
> 
> (Updated Feb. 22, 2018, 11:10 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new test 'HungDockerTest.ROOT_DOCKER_InspectDuringPull'
> verifies that the Mesos agent will send a TASK_FAILED
> update for a Docker task which fails because the executor
> registration timeout elapses while the container is stuck
> in the PULLING state due to a hung 'docker inspect' call.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65750/diff/2/
> 
> 
> Testing
> ---
> 
> sudo bin/mesos-tests.sh 
> --gtest_filter="HungDockerTest.ROOT_DOCKER_InspectDuringPull"
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65773: Introduced Docker image build and publish scripts.

2018-02-23 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [65772, 65773]

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

- Mesos Reviewbot


On Feb. 23, 2018, 3:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65773/
> ---
> 
> (Updated Feb. 23, 2018, 3:03 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces a build and publish script for Mesos-related
> docker images. The intention is to run these scripts from CI so that
> images on Dockerhub are kept up to date.
> 
> 
> Diffs
> -
> 
>   support/docker/Makefile PRE-CREATION 
>   support/docker/test/Dockerfile PRE-CREATION 
>   support/docker/test/foo.sh PRE-CREATION 
>   support/jenkins/build_docker_images.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65773/diff/1/
> 
> 
> Testing
> ---
> 
> Tested with my own account.
> 
> If image sources are unchanged, the images are only pulled down, but not 
> republished. If images sources changed they are rebuild and published.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 65751: Added test fixture for a hung Docker daemon.

2018-02-23 Thread Andrei Budnik

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




src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5133 (patched)


`os::write()` is not atomical, so we should write to a temporary file, then 
rename it: `mv new_env.tmp test-docker.env`. Otherwise, if someone launches 
docker cli, while the script is being updated - `test-docker.sh` might 
read/import half-written env file.

Probably, we don't need to fix it now.



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5164-5165 (patched)


Why do we need `export TEST_DOCKER_ARGUMENTS=$@`?

Let's get rid of `shift 2` command and `ARGS` variable.


- Andrei Budnik


On Feb. 22, 2018, 11:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65751/
> ---
> 
> (Updated Feb. 22, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8591
> https://issues.apache.org/jira/browse/MESOS-8591
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `HungDockerTest` class allows test authors to force
> certain Docker daemon calls to be delayed for a specified
> duration.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d1e657050d623ad0412208b3aa3e3101e3654e99 
> 
> 
> Diff: https://reviews.apache.org/r/65751/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65674: Introduced helper for creating Mesos UUIDs.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 65587.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65587`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65674

Relevant logs:

- 
[apply-review-65587-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65674/logs/apply-review-65587-stdout.log):

```

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'SYSTEM@build-srv-04.(none)')
```

- Mesos Reviewbot Windows


On Feb. 15, 2018, 2:54 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65674/
> ---
> 
> (Updated Feb. 15, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced helper for creating Mesos UUIDs.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266 
>   src/common/protobuf_utils.cpp f516091e9208552488c154f34adde6f20d3413bf 
>   src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377 
>   src/resource_provider/storage/provider.cpp 
> 33abc0e05a804969ae14da9cb9c58698ba1aaea5 
>   src/slave/slave.cpp e2facb3c15a2f907f6497c58a36842ed707f2c70 
>   src/tests/operation_status_update_manager_tests.cpp 
> c4429f40e1d7226be59d8ba4283bd91e16799d5a 
>   src/tests/resource_provider_manager_tests.cpp 
> c8997ec41fe0c3e02b0f6ab205c9009205c992da 
>   src/tests/resource_provider_validation_tests.cpp 
> 9f34c459e24396815f4a785815f83923e276874f 
>   src/tests/slave_tests.cpp e253317a67019302f18afe11e2a314e716cec226 
> 
> 
> Diff: https://reviews.apache.org/r/65674/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 65743: Ensured that Docker containerizer returns a failed Future in one case.

2018-02-23 Thread Andrei Budnik

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


Ship it!




Ship It!

- Andrei Budnik


On Feb. 22, 2018, 12:39 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65743/
> ---
> 
> (Updated Feb. 22, 2018, 12:39 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, and Michael Park.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Because the Docker library did not immediately transition the
> Future returned by `inspect()` to DISCARDED, it was safe for the
> Docker containerizer to discard this Future before failing the
> Promise associated with the Future returned by `launch()`.
> 
> However, the introduction of an `onDiscard` callback in
> `Docker::inspect()` makes this assumption invalid. This patch
> addresses this by failing the Promise before discarding the
> Future.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 7585178f34bdb782f0108d3ab94c23edfacc5564 
> 
> 
> Diff: https://reviews.apache.org/r/65743/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65591: Explicitly tracked resource providers in master.

2018-02-23 Thread Benjamin Bannier

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

(Updated Feb. 23, 2018, 4:22 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

* fixed `GET_OPERATIONS` handler in master
* fixed lifetime when adding resource providers after master failover


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


Repository: mesos


Description
---

This patch adds explicit tracking of resource providers to the master
process. While we already had explicitly send resource provider
information in e.g., `UpdateSlaveMessage`, we only stored that
information aggregated over the full agent in the master up to now.


Diffs (updated)
-

  src/common/protobuf_utils.cpp f516091e9208552488c154f34adde6f20d3413bf 
  src/master/http.cpp acf10a0f396234ec9f444c9b41515d657379d6ad 
  src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514 
  src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier



Re: Review Request 65588: Used proto UUID instead stout UUID internally for operation IDs.

2018-02-23 Thread Benjamin Bannier

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

(Updated Feb. 23, 2018, 4:22 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

* fixed grpc-enabled code


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


Repository: mesos


Description
---

Used proto UUID instead stout UUID internally for operation IDs.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
e96a40493c351a7242465c8591cae981abc92f24 
  src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266 
  src/common/protobuf_utils.cpp f516091e9208552488c154f34adde6f20d3413bf 
  src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514 
  src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44 
  src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377 
  src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9 
  src/resource_provider/storage/provider.cpp 
33abc0e05a804969ae14da9cb9c58698ba1aaea5 
  src/slave/slave.hpp 42c3ebcf52b608e40e26ca85f5efb054dad2d2a1 
  src/slave/slave.cpp e2facb3c15a2f907f6497c58a36842ed707f2c70 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier



Re: Review Request 65773: Introduced Docker image build and publish scripts.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 65772.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65772`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65773

Relevant logs:

- 
[apply-review-65772-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65773/logs/apply-review-65772-stdout.log):

```

*** Please tell me who you are.

Run

  git config --global user.email "y...@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'SYSTEM@build-srv-04.(none)')
```

- Mesos Reviewbot Windows


On Feb. 23, 2018, 3:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65773/
> ---
> 
> (Updated Feb. 23, 2018, 3:03 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces a build and publish script for Mesos-related
> docker images. The intention is to run these scripts from CI so that
> images on Dockerhub are kept up to date.
> 
> 
> Diffs
> -
> 
>   support/docker/Makefile PRE-CREATION 
>   support/docker/test/Dockerfile PRE-CREATION 
>   support/docker/test/foo.sh PRE-CREATION 
>   support/jenkins/build_docker_images.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65773/diff/1/
> 
> 
> Testing
> ---
> 
> Tested with my own account.
> 
> If image sources are unchanged, the images are only pulled down, but not 
> republished. If images sources changed they are rebuild and published.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Review Request 65772: Moved some docker image setups to dedicated directory.

2018-02-23 Thread Benjamin Bannier

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Moved some docker image setups to dedicated directory.


Diffs
-

  support/mesos-build/centos-7.dockerfile  
  support/mesos-build/enable-devtoolset-4.sh  
  support/mesos-build/entrypoint.sh  
  support/mesos-build/ubuntu-16.04.dockerfile  
  support/mesos-tidy/Dockerfile  
  support/mesos-tidy/README.md  
  support/mesos-tidy/entrypoint.sh  


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


Testing
---


Thanks,

Benjamin Bannier



Review Request 65773: Introduced Docker image build and publish scripts.

2018-02-23 Thread Benjamin Bannier

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

This patch introduces a build and publish script for Mesos-related
docker images. The intention is to run these scripts from CI so that
images on Dockerhub are kept up to date.


Diffs
-

  support/docker/Makefile PRE-CREATION 
  support/docker/test/Dockerfile PRE-CREATION 
  support/docker/test/foo.sh PRE-CREATION 
  support/jenkins/build_docker_images.sh PRE-CREATION 


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


Testing
---

Tested with my own account.

If image sources are unchanged, the images are only pulled down, but not 
republished. If images sources changed they are rebuild and published.


Thanks,

Benjamin Bannier



Re: Review Request 65683: Updated discard handling in Docker library functions.

2018-02-23 Thread Andrei Budnik

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




src/docker/docker.cpp
Lines 1292 (patched)


I think we don't need to discard the output, because the process will be 
killed, hence `io::read()` will be terminated after receiving EOF. Also, as we 
check `promise->future().hasDiscard()` before checking `output.isReady()` in 
`___inspect()`, the promise won't be set to `FAILED` state.


- Andrei Budnik


On Feb. 23, 2018, 7:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 7:46 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling in Docker library functions.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65683: Updated discard handling in Docker library functions.

2018-02-23 Thread Andrei Budnik


> On Feb. 23, 2018, 9:14 a.m., Greg Mann wrote:
> > src/docker/docker.cpp
> > Lines 1287-1293 (patched)
> > 
> >
> > Whoops, this is not threadsafe :facepalm:
> > 
> > For example, we could call `output->discard()` from two different 
> > threads simultaneously (see `__inspect`).
> > 
> > I need to figure out a better way to do this...

One possible solution might be using `defer(self(), ...)` for callbacks.


- Andrei


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


On Feb. 23, 2018, 7:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 7:46 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling in Docker library functions.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65759: Added inspect retries to the Docker executor.

2018-02-23 Thread Mesos Reviewbot

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



Bad patch!

Reviews applied: [65759, 65713, 65743, 65683]

Failed command: python support/apply-reviews.py -n -r 65683

Error:
2018-02-23 12:16:26 URL:https://reviews.apache.org/r/65683/diff/raw/ 
[7076/7076] -> "65683.patch" [1]
error: patch failed: src/docker/docker.cpp:1274
error: src/docker/docker.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/21742/console

- Mesos Reviewbot


On Feb. 22, 2018, 8:32 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65759/
> ---
> 
> (Updated Feb. 22, 2018, 8:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Michael Park.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds retries for `inspect` command to workaround docker
> daemon hangs. We assume that the docker daemon can be temporarily
> unresponsive. If it's unresponsive, then any started docker cli
> command hangs. To address the issue, we retry `inspect` in the loop.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65759/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manually, described in /r/65713
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 65759: Added inspect retries to the Docker executor.

2018-02-23 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 65683.

Failed command: `python.exe .\support\apply-reviews.py -n -r 65683`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65759

Relevant logs:

- 
[apply-review-65683-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65759/logs/apply-review-65683-stdout.log):

```
error: patch failed: src/docker/docker.cpp:1274
error: src/docker/docker.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Feb. 22, 2018, 8:32 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65759/
> ---
> 
> (Updated Feb. 22, 2018, 8:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Michael Park.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds retries for `inspect` command to workaround docker
> daemon hangs. We assume that the docker daemon can be temporarily
> unresponsive. If it's unresponsive, then any started docker cli
> command hangs. To address the issue, we retry `inspect` in the loop.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65759/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manually, described in /r/65713
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 65759: Added inspect retries to the Docker executor.

2018-02-23 Thread Andrei Budnik


> On Feb. 23, 2018, 8:57 a.m., Greg Mann wrote:
> > src/docker/executor.cpp
> > Lines 25 (patched)
> > 
> >
> > Is this necessary?

Without this header, I'm getting:
```
../../src/docker/executor.cpp:230:32: error: ‘await’ was not declared in this 
scope
```


- Andrei


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


On Feb. 22, 2018, 8:32 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65759/
> ---
> 
> (Updated Feb. 22, 2018, 8:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Michael Park.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds retries for `inspect` command to workaround docker
> daemon hangs. We assume that the docker daemon can be temporarily
> unresponsive. If it's unresponsive, then any started docker cli
> command hangs. To address the issue, we retry `inspect` in the loop.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65759/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manually, described in /r/65713
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 65713: Handled hanging docker `stop`, `inspect` commands in docker executor.

2018-02-23 Thread Andrei Budnik


> On Feb. 23, 2018, 8:42 a.m., Greg Mann wrote:
> > src/docker/executor.cpp
> > Lines 535 (patched)
> > 
> >
> > We should probably move this inside the `if (killedByHealthCheck)` 
> > conditional, since the kill is still in progress after this if it was not 
> > initiated by a health check.

If we move `killingInProgress` inside the conditional, all subsequent attempts 
to kill a task by a scheduler will be ignored.


- Andrei


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


On Feb. 22, 2018, 11:28 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65713/
> ---
> 
> (Updated Feb. 22, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previosly, if `docker inspect` command hanged, the docker container
> ended up in an unkillable state. This patch adds a timeout for inspect
> command after receiving `killTask` analogically to `reaped` handler.
> In addition we've added a timeout for `docker stop` command. If docker
> `stop` or `inspect` command times out, we discard the related future,
> thus the docker library kills previously spawned docker cli subprocess.
> As a result, a scheduler can retry `killTask` operation to handle
> nasty docker bugs that lead to hanging docker cli.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65713/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manual testing:
> 1. Build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 2. Modify `ContainerInspect` function from `docker/inspect.go`:
> ```
>  func (daemon *Daemon) ContainerInspect(name string, size bool, version 
> string) (interface{}, error) {
> +   time.Sleep(10 * time.Second)
> ```
> 3. Modify `ContainerStop` function from `docker/stop.go`:
> ```
>  func (daemon *Daemon) ContainerStop(name string, seconds *int) error {
> +   rand.Seed(time.Now().UTC().UnixNano())
> +   if rand.Intn(2) == 0 {
> +   time.Sleep(20 * time.Second)
> +   }
> ```
> 4. Rebuild docker: `sudo make build && sudo make binary`
> 5. Stop system docker daemon: `sudo service docker stop`
> 6. Start modified docker daemon: `sudo ./bundles/binary-daemon/dockerd-dev`
> 7. Modify `src/cli/execute.cpp`:
>   a) Add `delay(Seconds(15), self(), ::retryKill, task->task_id(), 
> offer.agent_id());` after 
> https://github.com/apache/mesos/blob/072ea2787ffca6f2a6dcb2d636f68c51823d6665/src/cli/execute.cpp#L606
>   b) Add a new method `retryKill` to `CommandScheduler`:
> ```
>   void retryKill(const TaskID& taskId, const AgentID& agentId)
>   {
> killTask(taskId, agentId);
> delay(Seconds(6), self(), ::retryKill, taskId, agentId);
>   }
> ```
> 8. Rebuild mesos
> 9. Run mesos master: `./bin/mesos-master.sh --work_dir='var/master-1'`
> 10. Run mesos agent: `GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/abudnik/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 11. Submit a task for the docker executor: `./src/mesos-execute 
> --master="127.0.1.1:5050" --name="a" --containerizer=docker 
> --docker_image="ubuntu:xenial" --command="sleep "`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 65674: Introduced helper for creating Mesos UUIDs.

2018-02-23 Thread Jan Schlicht

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




src/common/protobuf_utils.hpp
Lines 187 (patched)


Nit: We should document that using `None()` here will create a random UUID. 
These helper function currently have the semantic that an `Option` parameter 
indicated an optional field in the underlying protobuf. This assumption is 
broken here. And (not yours) is also broken in `createOperation` were we should 
document it as well.


- Jan Schlicht


On Feb. 15, 2018, 3:54 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65674/
> ---
> 
> (Updated Feb. 15, 2018, 3:54 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced helper for creating Mesos UUIDs.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266 
>   src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f 
>   src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377 
>   src/slave/slave.cpp c5ec62c0e55e7416d9cd2a49c13459b85e315150 
>   src/tests/operation_status_update_manager_tests.cpp 
> c4429f40e1d7226be59d8ba4283bd91e16799d5a 
>   src/tests/resource_provider_manager_tests.cpp 
> c8997ec41fe0c3e02b0f6ab205c9009205c992da 
>   src/tests/resource_provider_validation_tests.cpp 
> 9f34c459e24396815f4a785815f83923e276874f 
>   src/tests/slave_tests.cpp d2c242eae3169bff3b0197a36f171cd668ba 
> 
> 
> Diff: https://reviews.apache.org/r/65674/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-02-23 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [64970]

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

- Mesos Reviewbot


On Feb. 23, 2018, 7:58 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Feb. 23, 2018, 7:58 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use tox for linting and testing code living uder src/python.
> 
> At the moment, all linting is done through the same `pylint`
> installation under support/.virtualenv, which requires ALL dependencies
> (i.e. pip-requirements.txt, requirements.in scattered in various
> directories) to be installed in the same virtualenv, making things
> really messy -- e.g. when I've changed some code under `src/python/lib`,
> but don't have the dev virtualenv activated, linting will fail since
> none of the dependencies under `src/python/lib` have been installed.
> 
> Using tox, we can solve this problem by distributing a "test spec"
> (tox.ini) in each of the python source directories which are aware of
> its local dependencies only. To test or lint the code there would be as
> simple as running `tox -e py27-lint `, and the corresponding
> virtualenv and test dependencies would automatically be setup.
> 
> This patch modifies `support/mesos-style.py` to install `tox` in
> `support/.virtualenv` and delegates linting to a `tox` call when it sees
> python directories that have tox setup for it. Linting for all other
> languages will not be effected.
> 
> Testing Done:
> 1. intentionally create a lint error, such as extra spaces before a
> parens in a python file
> 2. run the pre-commit hook and see tox in action
> 
> Reviewed at https://reviews.apache.org/r/64970/
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/tox.ini PRE-CREATION 
>   src/python/lib/requirements-test.in 
> b2b73aab65377d9310797203ea84c5150ae60805 
>   src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
>   support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/4/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>



Re: Review Request 65683: Updated discard handling in Docker library functions.

2018-02-23 Thread Greg Mann

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




src/docker/docker.cpp
Lines 1287-1293 (patched)


Whoops, this is not threadsafe :facepalm:

For example, we could call `output->discard()` from two different threads 
simultaneously (see `__inspect`).

I need to figure out a better way to do this...


- Greg Mann


On Feb. 23, 2018, 7:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65683/
> ---
> 
> (Updated Feb. 23, 2018, 7:46 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Michael Park, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8575
> https://issues.apache.org/jira/browse/MESOS-8575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated discard handling in Docker library functions.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp d9e71f8841a868082170d28fc4f3d495e2eb1e61 
>   src/docker/docker.cpp 3df370e988fce12d323ff6b441da15dab27bdd28 
> 
> 
> Diff: https://reviews.apache.org/r/65683/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 65759: Added inspect retries to the Docker executor.

2018-02-23 Thread Greg Mann

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




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


Is this necessary?



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


s/docker/Docker/
s/retry/retry the/



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


s/inspect_loop/inspectLoop/



src/docker/executor.cpp
Lines 233-236 (patched)


Suggestion:

"We need to clean up the hanging Docker CLI process. Discarding the 
`inspect` future triggers a callback in the Docker library that kills the 
subprocess and transitions the future."


- Greg Mann


On Feb. 22, 2018, 8:32 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65759/
> ---
> 
> (Updated Feb. 22, 2018, 8:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Michael Park.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds retries for `inspect` command to workaround docker
> daemon hangs. We assume that the docker daemon can be temporarily
> unresponsive. If it's unresponsive, then any started docker cli
> command hangs. To address the issue, we retry `inspect` in the loop.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65759/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manually, described in /r/65713
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 64970: Replace ad hoc venv under support/ with tox.

2018-02-23 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 64970 was successfully built and tested.

Reviews applied: `['64970']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64970

- Mesos Reviewbot Windows


On Feb. 23, 2018, 7:58 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated Feb. 23, 2018, 7:58 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use tox for linting and testing code living uder src/python.
> 
> At the moment, all linting is done through the same `pylint`
> installation under support/.virtualenv, which requires ALL dependencies
> (i.e. pip-requirements.txt, requirements.in scattered in various
> directories) to be installed in the same virtualenv, making things
> really messy -- e.g. when I've changed some code under `src/python/lib`,
> but don't have the dev virtualenv activated, linting will fail since
> none of the dependencies under `src/python/lib` have been installed.
> 
> Using tox, we can solve this problem by distributing a "test spec"
> (tox.ini) in each of the python source directories which are aware of
> its local dependencies only. To test or lint the code there would be as
> simple as running `tox -e py27-lint `, and the corresponding
> virtualenv and test dependencies would automatically be setup.
> 
> This patch modifies `support/mesos-style.py` to install `tox` in
> `support/.virtualenv` and delegates linting to a `tox` call when it sees
> python directories that have tox setup for it. Linting for all other
> languages will not be effected.
> 
> Testing Done:
> 1. intentionally create a lint error, such as extra spaces before a
> parens in a python file
> 2. run the pre-commit hook and see tox in action
> 
> Reviewed at https://reviews.apache.org/r/64970/
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/tox.ini PRE-CREATION 
>   src/python/lib/requirements-test.in 
> b2b73aab65377d9310797203ea84c5150ae60805 
>   src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
>   support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/4/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>



Re: Review Request 65713: Handled hanging docker `stop`, `inspect` commands in docker executor.

2018-02-23 Thread Greg Mann

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




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


We should probably move this inside the `if (killedByHealthCheck)` 
conditional, since the kill is still in progress after this if it was not 
initiated by a health check.


- Greg Mann


On Feb. 22, 2018, 11:28 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65713/
> ---
> 
> (Updated Feb. 22, 2018, 11:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Greg Mann, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-8574
> https://issues.apache.org/jira/browse/MESOS-8574
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previosly, if `docker inspect` command hanged, the docker container
> ended up in an unkillable state. This patch adds a timeout for inspect
> command after receiving `killTask` analogically to `reaped` handler.
> In addition we've added a timeout for `docker stop` command. If docker
> `stop` or `inspect` command times out, we discard the related future,
> thus the docker library kills previously spawned docker cli subprocess.
> As a result, a scheduler can retry `killTask` operation to handle
> nasty docker bugs that lead to hanging docker cli.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 80e2d81169f0d4303ca1ddbcef9fa87fe52601fc 
> 
> 
> Diff: https://reviews.apache.org/r/65713/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> Manual testing:
> 1. Build docker from sources: 
> http://oyvindsk.com/writing/docker-build-from-source
> 2. Modify `ContainerInspect` function from `docker/inspect.go`:
> ```
>  func (daemon *Daemon) ContainerInspect(name string, size bool, version 
> string) (interface{}, error) {
> +   time.Sleep(10 * time.Second)
> ```
> 3. Modify `ContainerStop` function from `docker/stop.go`:
> ```
>  func (daemon *Daemon) ContainerStop(name string, seconds *int) error {
> +   rand.Seed(time.Now().UTC().UnixNano())
> +   if rand.Intn(2) == 0 {
> +   time.Sleep(20 * time.Second)
> +   }
> ```
> 4. Rebuild docker: `sudo make build && sudo make binary`
> 5. Stop system docker daemon: `sudo service docker stop`
> 6. Start modified docker daemon: `sudo ./bundles/binary-daemon/dockerd-dev`
> 7. Modify `src/cli/execute.cpp`:
>   a) Add `delay(Seconds(15), self(), ::retryKill, task->task_id(), 
> offer.agent_id());` after 
> https://github.com/apache/mesos/blob/072ea2787ffca6f2a6dcb2d636f68c51823d6665/src/cli/execute.cpp#L606
>   b) Add a new method `retryKill` to `CommandScheduler`:
> ```
>   void retryKill(const TaskID& taskId, const AgentID& agentId)
>   {
> killTask(taskId, agentId);
> delay(Seconds(6), self(), ::retryKill, taskId, agentId);
>   }
> ```
> 8. Rebuild mesos
> 9. Run mesos master: `./bin/mesos-master.sh --work_dir='var/master-1'`
> 10. Run mesos agent: `GLOG_v=1 ./bin/mesos-agent.sh 
> --resources="cpus:1;mem:100" 
> --work_dir='/home/abudnik/mesos/build/var/agent-1' 
> --containerizers="docker,mesos" --master="127.0.1.1:5050"`
> 11. Submit a task for the docker executor: `./src/mesos-execute 
> --master="127.0.1.1:5050" --name="a" --containerizer=docker 
> --docker_image="ubuntu:xenial" --command="sleep "`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>