Re: Review Request 67414: Added default message bodies to libprocess HTTP error responses.

2018-06-05 Thread Alexander Rukletsov 

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




3rdparty/libprocess/include/process/http.hpp
Line 701 (original), 701-702 (patched)


Why not using `process::http::Status::string()`  
https://github.com/apache/mesos/blob/2198b961d24b788564d36490cf52f78d7ec07655/3rdparty/libprocess/src/http.cpp#L176-L180
 instead?

Maybe then you can even put a single line into `Response` c-tor instead of 
modifying individual classes.


- Alexander Rukletsov


On June 1, 2018, 3:14 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67414/
> ---
> 
> (Updated June 1, 2018, 3:14 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By default on error libprocess would only return a response
> with the correct status code and no response body.
> 
> However, most browsers do not visually indicate the response
> status code, making it hard for the user to figure out what
> exactly the problem was.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 055447e13117c0a3ba79d0fc326ece657e8f064f 
> 
> 
> Diff: https://reviews.apache.org/r/67414/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 67454: Fixed failing ArchiverTest due to long filename.

2018-06-05 Thread John Kordich via Review Board 

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, 
Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.


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


Repository: mesos


Description
---

Replaced most of the ArchiverTest.ExtractZipFileWithLongDestinationDir
test such that it constructs a long path similar to how it is done in
filesystem_tests.cpp. This resolves a failure in the jenkins BuildBot
job that runs all of the tests inside a docker container, which uses a
filesystem called AUFS, which has an individual file name limit of 242
characters.


Diffs
-

  3rdparty/stout/tests/archiver_tests.cpp 
fc8db56897ba7e5881a8af27da11c1d21c835b94 


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


Testing
---

I ran the cmake build on both Linux and Windows and verified the tests pass. I 
was not able to run the tests inside a docker container, but considering the 
changes made, I would be very surprised if it failed in the same way because we 
are no longer creating a filename with greater than 242 characters.


Thanks,

John Kordich

Re: Review Request 67454: Fixed failing ArchiverTest due to long filename.

2018-06-05 Thread Joseph Wu 

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


Ship it!




Ship It!

- Joseph Wu


On June 5, 2018, 12:09 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67454/
> ---
> 
> (Updated June 5, 2018, 12:09 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, 
> Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8977
> https://issues.apache.org/jira/browse/MESOS-8977
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced most of the ArchiverTest.ExtractZipFileWithLongDestinationDir
> test such that it constructs a long path similar to how it is done in
> filesystem_tests.cpp. This resolves a failure in the jenkins BuildBot
> job that runs all of the tests inside a docker container, which uses a
> filesystem called AUFS, which has an individual file name limit of 242
> characters.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/archiver_tests.cpp 
> fc8db56897ba7e5881a8af27da11c1d21c835b94 
> 
> 
> Diff: https://reviews.apache.org/r/67454/diff/1/
> 
> 
> Testing
> ---
> 
> I ran the cmake build on both Linux and Windows and verified the tests pass. 
> I was not able to run the tests inside a docker container, but considering 
> the changes made, I would be very surprised if it failed in the same way 
> because we are no longer creating a filename with greater than 242 characters.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 67385: Windows: Added IOCP `HANDLE` to `WindowsFD`.

2018-06-05 Thread Joseph Wu 

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




3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 181 (patched)


Suggestion: It's easy to get `iocp_handle` and `iocp_handle_` mixed up 
while reading this function.  Consider renaming the parameter `iocp_handle` to 
`target`.


- Joseph Wu


On May 30, 2018, 11:58 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67385/
> ---
> 
> (Updated May 30, 2018, 11:58 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5371 and MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-5371
> https://issues.apache.org/jira/browse/MESOS-8668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos/Libprocess uses `os::nonblock()` in a way that assumes that it is
> idempotent. Unfortunately, `CreateIoCompletionPort` is not idempotent,
> so we need to wrap the function around our own code to make it so. We
> need to keep it inside the `WindowsFD` class, because there isn't a way
> to determine if a HANDLE is associated with an IOCP through the Win32
> API.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 5bda095e676b038cdaea04f7be23ba2a1aca9015 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 5dbdff2680370d123579c5e3fdd9b0e0adaf512e 
> 
> 
> Diff: https://reviews.apache.org/r/67385/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Review Request 67456: Windows: Wrapped docker executor code in job object.

2018-06-05 Thread Akash Gupta 

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

One of the docker tests revealed that the docker executor code could
leak some of the docker processes if it gets killed. To avoid this, we
run the docker executor and all processes launched by it in a job
object, so that `os::killtree` will properly kill the process tree.


Diffs
-

  src/docker/docker.cpp 1332d4b8cc8dc48abd5f07689b18a5b39ca946cf 
  src/docker/executor.cpp 572594a110aa4b51a698b176287b6ff823b2dd07 
  src/slave/containerizer/docker.cpp 391700f9698d0658b9273d79857bfa30bf3549be 


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


Testing
---


Thanks,

Akash Gupta

Review Request 67455: Windows: Fixed incorrect return code for `os:kill`.

2018-06-05 Thread Akash Gupta 

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

`os::kill` was returning the opposite error code than expected. It
was returning `KILL_FAIL` on success and `KILL_PASS` on error.


Diffs
-

  3rdparty/stout/include/stout/os/windows/kill.hpp 
bdb83510b7f0529d41f8e895451a941dc22d21bb 


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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 67454: Fixed failing ArchiverTest due to long filename.

2018-06-05 Thread Mesos Reviewbot Windows 

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



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

Reviews applied: `['67454']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[ RUN  ] SocketTests.InitSocket
[   OK ] SocketTests.InitSocket (2 ms)
[ RUN  ] SocketTests.IntFD
[   OK ] SocketTests.IntFD (1 ms)
[--] 2 tests from SocketTests (5 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)

[--] 2 tests from OsSendfileTest
[ RUN  ] OsSendfileTest.Sendfile
[   OK ] OsSendfileTest.Sendfile (1219 ms)
[ RUN  ] OsSendfileTest.SendfileAsync
[   OK ] OsSendfileTest.SendfileAsync (18 ms)
[--] 2 tests from OsSendfileTest (1240 ms total)

[--] Global test environment tear-down
[==] 321 tests from 49 test cases ran. (5907 ms total)
[  PASSED  ] 318 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] ArchiverTest.ExtractTarXZFile
[  FAILED  ] ArchiverTest.ExtractTarBZ2File
[  FAILED  ] ArchiverTest.ExtractTarBz2GzFile

 3 FAILED TESTS
```

- 
[stout-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67454/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 Reviewbot Windows


On June 5, 2018, 7:09 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67454/
> ---
> 
> (Updated June 5, 2018, 7:09 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, 
> Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8977
> https://issues.apache.org/jira/browse/MESOS-8977
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced most of the ArchiverTest.ExtractZipFileWithLongDestinationDir
> test such that it constructs a long path similar to how it is done in
> filesystem_tests.cpp. This resolves a failure in the jenkins BuildBot
> job that runs all of the tests inside a docker container, which uses a
> filesystem called AUFS, which has an individual file name limit of 242
> characters.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/archiver_tests.cpp 
> fc8db56897ba7e5881a8af27da11c1d21c835b94 
> 
> 
> Diff: https://reviews.apache.org/r/67454/diff/1/
> 
> 
> Testing
> ---
> 
> I ran the cmake build on both Linux and Windows and verified the tests pass. 
> I was not able to run the tests inside a docker container, but considering 
> the changes made, I would be very surprised if it failed in the same way 
> because we are no longer creating a filename with greater than 242 characters.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Review Request 67457: Windows: Ported docker_containerizer_tests.cpp.

2018-06-05 Thread Akash Gupta 

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

With some Docker bug fixes and the IOCP backend, the remaining docker
have been ported. The only remaining Docker tests that aren't ported
are either due to limitations on Windows Containers or unimplemented
features in Mesos (Persistent volume and hooks).


Diffs
-

  src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
  src/tests/containerizer/docker_containerizer_tests.cpp 
194308bf9d09a3562e683c49e0da4c9a6463d66e 


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


Testing
---


Thanks,

Akash Gupta

Re: Review Request 67385: Windows: Added IOCP `HANDLE` to `WindowsFD`.

2018-06-05 Thread Andrew Schwartzmeyer 

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




3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 181 (patched)


Maybe just `HANDLE handle` ;)



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 181-182 (patched)


What is the `key` used for? (Maybe we need a small excerpt from the MSDN 
docs for `CreateIoCompletionPort`.)



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 183 (patched)


I'm not saying you should use it, or if you even want to use it, but there 
does exist some synchronization helpers in stout, under `synchronized.hpp`.



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 183-184 (patched)


Is there ever a case where `iocp_handle_` hasn't been allocated? Right now, 
this is implicitly relying on the fact that we chose to make the default 
constructor construct off the `HANDLE` ctor with `INVALID_HANDLE_VALUE`, and 
that ctor uses `make_shared`. Just wondering if we should refactor to make the 
allocation more explicit and certain (like maybe to `make_shared` in the 
default ctor, and ensure all ctors call it too, I don't know yet).



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 184 (patched)


`const`



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 188 (patched)


Oi, I hate that our cast operators are implicit, `*this` threw me for a 
minute.



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 202-215 (patched)


Old comment, but leaving for posterity:

I don't like this `WindowsFD::dup` method; it opens up a can of worms, such 
as: should we move the `os::dup` logic into this method (should we then start 
moving other `os::foo(int_fd)` logic into their own methods)? It also prompted 
me to think about this maybe being done in a copy ctor, but that doesn't make 
sense either.

At a minimum, let's make this `private` and make `os::dup` a `friend` so 
that `WindowsFD::dup` is clearly demarcated as an internal helper.

And once I wrote that, I almost want to suggest: make `os::dup` a friend, 
and just make a new `fd` and manually assign the `overlapped_` and 
`iocp_handle_` fields (that is, get rid of the helper abstraction). Not sure if 
that would be cleaner.



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 231-235 (patched)


This is more like `struct IOCPData`, since it's both the handle and the 
mutex.



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 237 (patched)


This could just be named `iocp_`



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 239-257 (patched)


As discussed in-person, we can simplify all this logic (read: delete these 
ctors and the `dup` method) by making `os::dup` a friend, using the default 
copy ctor in `os::dup`, and then overwriting the `handle_` or `socket_` field 
with the duplicated object (which is then doable as a friend function).


- Andrew Schwartzmeyer


On May 30, 2018, 11:58 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67385/
> ---
> 
> (Updated May 30, 2018, 11:58 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5371 and MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-5371
> https://issues.apache.org/jira/browse/MESOS-8668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos/Libprocess uses `os::nonblock()` in a way that assumes that it is
> idempotent. Unfortunately, `CreateIoCompletionPort` is not idempotent,
> so we need to wrap the function around our own code to make it so. We
> need to keep it inside the `WindowsFD` class, because there isn't a way
> to determine if a HANDLE is associated with an IOCP through the Win32
> API.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 5bda095e676b038cdaea04f7be23ba2a1aca9015 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 5dbdff2680370d123579c5e3fdd9b0e0adaf512e 
> 
> 
> Diff: https://reviews.apache.org/r/67385/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Review Request 67463: Fixed type error when using `check_output` in Python 3 scripts.

2018-06-05 Thread Armand Grillet 

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

Review request for mesos and Andrew Schwartzmeyer.


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


Repository: mesos


Description
---

Fixed type error when using `check_output` in Python 3 scripts.


Diffs
-

  support/python3/generate-endpoint-help.py 
711d3d325acbc1fe9e96d65caaf089727f03904f 
  support/python3/mesos-gtest-runner.py 
153ded2994daad6ffb610866d499603af5a53b0a 
  support/python3/push-commits.py 82a700418341731833f165a24054f0879648ea92 


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


Testing
---

Removed part of the content of `push-commits.py` and run it using Python 3.


Thanks,

Armand Grillet

Review Request 67464: Fixed `python3/mesos-style.py`.

2018-06-05 Thread Andrew Schwartzmeyer 

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

Review request for mesos and Armand Grillet.


Repository: mesos


Description
---

The `cpplint.py` file was made to be single-source compatible with
both Python 2 and Python 3, but `mesos-style.py` was erroneously
changed to look for a copy of it under `support/python3`.


Diffs
-

  support/python3/mesos-style.py 6f9b9d4611c6501f8a2dbda4bfe08cffaadc6d86 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 67465: Windows: Log a fatal error if `WindowsFD(int)` is incorrectly used.

2018-06-05 Thread Andrew Schwartzmeyer 

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

Review request for mesos, Akash Gupta and Joseph Wu.


Repository: mesos


Description
---

This constructor _should_ be `explicit` as the other constructors are;
however, for historical reasons it is implicit in order to support the
semantics of assigning from `-1, 0, 1, 2`. Unfortunately, its
implicitness leads to subtle bugs. For instance, the code `int_fd s =
::socket(...)` on Windows compiles, but behaves incorrectly because
the RHS `SOCKET` type is implicitly converted to an `int` to satisfy
the only available implicit constructor, since the `WindowsFD(SOCKET)`
constructor is marked `explicit`. As we should never construct off any
of these values, but cannot constrain it at compile-time, we should
add a fatal runtime error instead.


Diffs
-

  3rdparty/stout/include/stout/os/windows/fd.hpp 
5dbdff2680370d123579c5e3fdd9b0e0adaf512e 


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


Testing
---

Still building, but this is what I propose. This is better than subtle sad 
panda bugs.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67389: Windows: Implemented Windows IOCP async backend.

2018-06-05 Thread Joseph Wu 

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




3rdparty/libprocess/src/libwinio_impl.cpp
Lines 62-74 (patched)


You can probably move this function's body into the `KEY_TIMER` switch 
case, since we wouldn't want to call this function anywhere else.



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 118 (patched)


Shouldn't this buffer be twice as large to account for both local and 
remote addresses (+16 bytes for each, as required by `::AcceptEx`)?

`unsigned char buf[2 * sizeof(SOCKADDR_STORAGE) + 32];`



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 237-240 (patched)


Let's make this into a CHECK or fatal error.  We would definitely want to 
notice if this branch is possible.



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 295-305 (patched)


Isn't this quick poll almost the same as letting the outer `while` loop 
repeat?

Or is it possible that every GQCS call can be interrupted by APCs, if there 
are enough incoming APCs?



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 312-315 (patched)


It would be ok to `LOG(FATAL)` in these cases and exit immediately.


- Joseph Wu


On May 30, 2018, 11:54 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67389/
> ---
> 
> (Updated May 30, 2018, 11:54 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668, MESOS-8671 and MESOS-8672
> https://issues.apache.org/jira/browse/MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8671
> https://issues.apache.org/jira/browse/MESOS-8672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows IOCP async backend, called libwinio, provides a single
> threaded event loop implementation that allows for async IO and timer
> events. libwinio wraps the native Win32 async functions using
> libprocess's primitives, which makes it easier to use and more type
> safe.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/libwinio_impl.hpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_impl.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67389/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67421: Rewrote the `ROOT_BusyMountPoint` test to reflect updated behavior.

2018-06-05 Thread Mesos Reviewbot Windows 

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



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

Reviews applied: `['67264', '67423', '67421']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (118 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1074 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 (37 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (77 ms 
total)

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

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

[--] Global test environment tear-down
[==] 988 tests from 97 test cases ran. (514230 ms total)
[  PASSED  ] 987 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] 
ROOT_DOCKER_DockerAndMesosContainerizers/DefaultExecutorTest.SigkillExecutor/0, 
where GetParam() = "docker,mesos"

 1 FAILED TEST
  YOU HAVE 220 DISABLED TESTS

```

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

```
I0606 02:31:03.230854 12092 master.cpp:10863] Updating the state of task 
8a364875-ed01-4861-bb2a-f76b4ebdab49 of framework 
a144ace1-b9d0-40be-91ef-86e63e22b19f- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0606 02:31:03.230854 15436 slave.cpp:3939] Shutting down framework 
a144ace1-b9d0-40be-91ef-86e63e22b19f-
I0606 02:31:03.230854 15436 slave.cpp:6660] Shutting down executor 
'8a364875-ed01-4861-bb2a-f76b4ebdab49' of framework 
a144ace1-b9d0-40be-91ef-86e63e22b19f- at executor(1)@192.10.1.5:54146
I0606 02:31:03.232861 15436 slave.cpp:931] Agent terminating
W0606 02:31:03.232861 15436 slave.cpp:3935] Ignoring shutdown framework 
a144ace1-b9d0-40be-91ef-86e63e22b19f- because it is terminating
I0606 02:31:03.233860 12092 master.cpp:10962] Removing task 
8a364875-ed01-4861-bb2a-f76b4ebdab49 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of frameworI0606 02:31:02.730846  2992 exec.cpp:162] Version: 
1.7.0
I0606 02:31:02.757848 18392 exec.cpp:236] Executor registered on agent 
a144ace1-b9d0-40be-91ef-86e63e22b19f-S0
I0606 02:31:02.762850 18164 executor.cpp:178] Received SUBSCRIBED event
I0606 02:31:02.767850 18164 executor.cpp:182] Subscribed executor on 
windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net
I0606 02:31:02.770851 18164 executor.cpp:178] Received LAUNCH event
I0606 02:31:02.776850 18164 executor.cpp:665] Starting task 
8a364875-ed01-4861-bb2a-f76b4ebdab49
I0606 02:31:02.869858 18164 executor.cpp:485] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0606 02:31:03.196879 18164 executor.cpp:678] Forked command at 2232
I0606 02:31:03.232861 19992 exec.cpp:445] Executor asked to shutdown
I0606 02:31:03.233860 18776 executor.cpp:178] Received SHUTDOWN event
I0606 02:31:03.233860 18776 executor.cpp:781] Shutting down
I0606 02:31:03.233860 18776 executor.cpp:894] Sending SIGTERM to process tree 
at pid 223k a144ace1-b9d0-40be-91ef-86e63e22b19f- on agent 
a144ace1-b9d0-40be-91ef-86e63e22b19f-S0 at slave(449)@192.10.1.5:54125 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0606 02:31:03.237862 12092 master.cpp:1293] Agent 
a144ace1-b9d0-40be-91ef-86e63e22b19f-S0 at slave(449)@192.10.1.5:54125 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net) disconnected
I0606 02:31:03.238848 12092 master.cpp:3303] Disconnecting agent 
a144ace1-b9d0-40be-91ef-86e63e22b19f-S0 at slave(449)@192.10.1.5:54125 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0606 02:31:03.238848 12092 master.cpp:3322] Deactivating agent 
a144ace1-b9d0-40be-91ef-86e63e22b19f-S0 at slave(449)@192.10.1.5:54125

Re: Review Request 67463: Fixed type error when using `check_output` in Python 3 scripts.

2018-06-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 67463 was successfully built and tested.

Reviews applied: `['67463']`

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

- Mesos Reviewbot Windows


On June 5, 2018, 3:22 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67463/
> ---
> 
> (Updated June 5, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8979
> https://issues.apache.org/jira/browse/MESOS-8979
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed type error when using `check_output` in Python 3 scripts.
> 
> 
> Diffs
> -
> 
>   support/python3/generate-endpoint-help.py 
> 711d3d325acbc1fe9e96d65caaf089727f03904f 
>   support/python3/mesos-gtest-runner.py 
> 153ded2994daad6ffb610866d499603af5a53b0a 
>   support/python3/push-commits.py 82a700418341731833f165a24054f0879648ea92 
> 
> 
> Diff: https://reviews.apache.org/r/67463/diff/1/
> 
> 
> Testing
> ---
> 
> Removed part of the content of `push-commits.py` and run it using Python 3.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67457: Windows: Ported docker_containerizer_tests.cpp.

2018-06-05 Thread Mesos Reviewbot Windows 

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



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

Reviews applied: `['67384', '67385', '67386', '67387', '67388', '67389', 
'67390', '67391', '67392', '67393', '67455', '67456', '67457']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_DestroyWhilePulling
[   OK ] DockerContainerizerTest.ROOT_DOCKER_DestroyWhilePulling (963 ms)
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_DestroyUnknownContainer
[   OK ] DockerContainerizerTest.ROOT_DOCKER_DestroyUnknownContainer (746 
ms)
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_ExecutorCleanupWhenLaunchFailed
[   OK ] 
DockerContainerizerTest.ROOT_DOCKER_ExecutorCleanupWhenLaunchFailed (1541 ms)
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_FetchFailure
[   OK ] DockerContainerizerTest.ROOT_DOCKER_FetchFailure (969 ms)
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_DockerPullFailure
[   OK ] DockerContainerizerTest.ROOT_DOCKER_DockerPullFailure (973 ms)
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard
[   OK ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard (1422 ms)
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_WaitUnknownContainer
[   OK ] DockerContainerizerTest.ROOT_DOCKER_WaitUnknownContainer (734 ms)
[ RUN  ] 
DockerContainerizerTest.ROOT_DOCKER_NoTransitionFromKillingToRunning
[   OK ] 
DockerContainerizerTest.ROOT_DOCKER_NoTransitionFromKillingToRunning (5601 ms)
[ RUN  ] DockerContainerizerTest.ROOT_DOCKER_DefaultDNS
[   OK ] DockerContainerizerTest.ROOT_DOCKER_DefaultDNS (5811 ms)
[--] 24 tests from DockerContainerizerTest (105814 ms total)

[--] 1 test from HungDockerTest
[ RUN  ] HungDockerTest.ROOT_DOCKER_InspectHungDuringPull

d:\dcos\mesos\mesos\src\tests\mock_docker.hpp(155): ERROR: this mock object 
(used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be 
deleted but never is. Its address is @00FED5AFBF60.
d:\dcos\mesos\mesos\src\tests\containerizer\docker_containerizer_tests.cpp(5189):
 ERROR: this mock object (used in test 
HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never 
is. Its address is @00FED5AFC1C0.
d:\dcos\mesos\mesos\src\tests\mock_docker.cpp(48): ERROR: this mock object 
(used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be 
deleted but never is. Its address is @02EB98C0B8D0.
d:\dcos\mesos\mesos\3rdparty\libprocess\include\process\gmock.hpp(235): ERROR: 
this mock object (used in test 
HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never 
is. Its address is @02EB991A0AD8.
d:\dcos\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object 
(used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be 
deleted but never is. Its address is @02EB99D13190.
ERROR: 5 leaked mock objects found at program exit.
```

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

```
I0605 22:01:23.478771 24908 authenticatee.cpp:299] Authentication success
I0605 22:01:23.479773 25040 master.cpp:9633] Successfully authenticated 
principal 'test-principal' at 
scheduler-d03482ef-f31f-4804-9c7b-5332fe1fd85e@192.10.1.6:54921
I0605 22:01:23.479773 25432 sched.cpp:501] Successfully authenticated with 
master master@192.10.1.6:54921
I0605 22:01:23.480772  6620 master.cpp:2890] Received SUBSCRIBE call for 
framework 'default' at 
scheduler-d03482ef-f31f-4804-9c7b-5332fe1fd85e@192.10.1.6:54921
I0605 22:01:23.480772  6620 master.cpp:2197] Authorizing framework principal 
'test-principal' to receive offers for roles '{ * }'
I0605 22:01:23.481794 23824 master.cpp:2971] Subscribing framework default with 
checkpointing disabled and capabilities [ MULTI_ROLE, RESERVATION_REFINEMENT ]
I0605 22:01:23.482769 23824 master.cpp:9824] Adding framework 
7eefd355-ca3a-46bc-9d8f-f6bf38e8e1b9- (default) at 
scheduler-d03482ef-f31f-4804-9c7b-5332fe1fd85e@192.10.1.6:54921 with roles {  } 
suppressed
I0605 22:01:23.484766  4972 sched.cpp:749] Framework registered with 
7eefd355-ca3a-46bc-9d8f-f6bf38e8e1b9-
I0605 22:01:23.484766 29664 hierarchical.cpp:297] Added framework 
7eefd355-ca3a-46bc-9d8f-f6bf38e8e1b9-
E0605 22:01:23.570777 25584 slave.cpp:7289] EXIT with status 1: Failed to 
perform recovery: Collect failed: Failed to run 'C:\Program Files 
(x86)\Microsoft Visual

Re: Review Request 67264: Unmounted any dangling persistent volume in gc paths.

2018-06-05 Thread Jie Yu 

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




src/slave/gc.cpp
Lines 225 (patched)


`isPersistentVolumePath` won't work for MOUNT type persistent volumes.

Currently, the only mounts in the host mount namespace under the sandbox 
directories (i.e., `/var/lib/mesos/slaves/...` are persistent volume mounts. 
I'd suggest we compare `entry.target` with that to determine if we need to 
unmount.



src/tests/gc_tests.cpp
Line 99 (original), 99 (patched)


remove one `;`?



src/tests/mesos.cpp
Line 694 (original), 694 (patched)


Please move `:` to the next line to follow our style guide


- Jie Yu


On May 31, 2018, 8:38 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67264/
> ---
> 
> (Updated May 31, 2018, 8:38 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-8830
> https://issues.apache.org/jira/browse/MESOS-8830
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In various corner cases, agent may not get chance to properly unmount
> persistent volumes mounted inside an executor's sandbox. When GC later
> gets to these sandbox directories, permanent data loss can happen (see
> MESOS-8830).
> 
> This patch added some protection to unmount possible persistent
> volumes inside a path to gc, and skipped the path if unmount failed.
> 
> NOTE: this means agent will not garbage collect any path if it cannot
> read its own `mountinfo` table.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp afff54653e8e659d947ddbee6dc38ba2715f2a78 
>   src/slave/gc.hpp df40165bb8a23f065156bf6c5f354b143d88c088 
>   src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106 
>   src/slave/gc_process.hpp 20374ad91820341282fdf18ecade60a020e26cea 
>   src/slave/main.cpp 646125344d590b28256d8ee684d7e51a90e82f23 
>   src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48 
>   src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 
>   src/tests/cluster.cpp b56212f6529a4d307e65797ad9bb34f2104fc832 
>   src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031 
>   src/tests/mesos.hpp 733344a2f07ebd9d841a55fb9bbfda2e3c1a1eb2 
>   src/tests/mesos.cpp d3c87c295429481c59d5a49398e289a4b84e4496 
>   src/tests/slave_tests.cpp 65d860594572b58a50a89358e31e97fd2a10bf08 
> 
> 
> Diff: https://reviews.apache.org/r/67264/diff/4/
> 
> 
> Testing
> ---
> 
> Added a unit test in following patch.
> 
> Tested with following procedures:
> 1. Start a test master and agent;
> 2. Created a persistent volume on agent through operator API;
> 3. Use `mesos-execute` to run a task;
> 4. Stop the agent;
> 5. Manually bind mount persistent volume path into a `volume` directory 
> inside the executor sandbox (to simulate a dangling mount in MESOS-8830);
> 6. Restart agent with `--gc_disk_headroom=1.0 --gc_delay=1secs` to force it 
> gc the path immediately.
> 
> With this fix, we observed that the dangling mount is automatically cleaned 
> up, and agent produces log line:
> ```
> W0523 06:00:04.001075 82745 gc.cpp:229] Unmounting dangling mount point 
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0/frameworks/b3eb3aff-d19d-45ff-8113-f0316462d3fa-/executors/test_id/runs/1cd3bd06-2632-4541-a708-80c7cd51c74b/volume'
>  of persistent volume '/home/zhitao/mesos-workdir/volumes/roles/role/id1' 
> inside garbage collected path 
> '/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0'
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 67457: Windows: Ported docker_containerizer_tests.cpp.

2018-06-05 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [67384, 67385, 67386, 67387, 67388, 67389, 67390, 67391, 
67392, 67393, 67455, 67456, 67457]

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 June 5, 2018, 1:40 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67457/
> ---
> 
> (Updated June 5, 2018, 1:40 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-7342
> https://issues.apache.org/jira/browse/MESOS-7342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With some Docker bug fixes and the IOCP backend, the remaining docker
> have been ported. The only remaining Docker tests that aren't ported
> are either due to limitations on Windows Containers or unimplemented
> features in Mesos (Persistent volume and hooks).
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 194308bf9d09a3562e683c49e0da4c9a6463d66e 
> 
> 
> Diff: https://reviews.apache.org/r/67457/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67460: Enable stouttest EnvTest.EraseEnv.

2018-06-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 67460 was successfully built and tested.

Reviews applied: `['67460']`

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

- Mesos Reviewbot Windows


On June 5, 2018, 2:17 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67460/
> ---
> 
> (Updated June 5, 2018, 2:17 p.m.)
> 
> 
> Review request for Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-5813
> https://issues.apache.org/jira/browse/MESOS-5813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable stouttest EnvTest.EraseEnv.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/env_tests.cpp 
> b5b124dc6316e661af6dd90335ade5283c26d9f2 
> 
> 
> Diff: https://reviews.apache.org/r/67460/diff/1/
> 
> 
> Testing
> ---
> 
> Here is one test I could enable for windows. However, one thing I notice is 
> that the original test specifically wanted to call ::getenv which I had to 
> change to os::getenv. So the test is a little different now. I can abandon 
> this review if you think that makes more sense.
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67264: Unmounted any mount points in gc paths.

2018-06-05 Thread Zhitao Li 

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

(Updated June 5, 2018, 5:07 p.m.)


Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.


Changes
---

Jie's comments.


Summary (updated)
-

Unmounted any mount points in gc paths.


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


Repository: mesos


Description (updated)
---

In various corner cases, agent may not get chance to properly unmount
persistent volumes mounted inside an executor's sandbox. When GC later
gets to these sandbox directories, permanent data loss can happen (see
MESOS-8830).

Currently, the only mounts in the host mount namespace under the sandbox
directories are persistent volumes, so this diff added protection to
unmount any dangling mount points before calling `rmdir` on the
directory.

NOTE: this means agent will not garbage collect any path if it cannot
read its own `mountinfo` table.


Diffs (updated)
-

  src/local/local.cpp afff54653e8e659d947ddbee6dc38ba2715f2a78 
  src/slave/gc.hpp df40165bb8a23f065156bf6c5f354b143d88c088 
  src/slave/gc.cpp 390b35e6d17d6614a73c9548decbf10739560106 
  src/slave/gc_process.hpp 20374ad91820341282fdf18ecade60a020e26cea 
  src/slave/main.cpp 646125344d590b28256d8ee684d7e51a90e82f23 
  src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48 
  src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 
  src/tests/cluster.cpp 01eb0950e687227dac81b1cdb9eaba3379cf5dbb 
  src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031 
  src/tests/mesos.hpp 733344a2f07ebd9d841a55fb9bbfda2e3c1a1eb2 
  src/tests/mesos.cpp d3c87c295429481c59d5a49398e289a4b84e4496 
  src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed 


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

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


Testing
---

Added a unit test in following patch.

Tested with following procedures:
1. Start a test master and agent;
2. Created a persistent volume on agent through operator API;
3. Use `mesos-execute` to run a task;
4. Stop the agent;
5. Manually bind mount persistent volume path into a `volume` directory inside 
the executor sandbox (to simulate a dangling mount in MESOS-8830);
6. Restart agent with `--gc_disk_headroom=1.0 --gc_delay=1secs` to force it gc 
the path immediately.

With this fix, we observed that the dangling mount is automatically cleaned up, 
and agent produces log line:
```
W0523 06:00:04.001075 82745 gc.cpp:229] Unmounting dangling mount point 
'/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0/frameworks/b3eb3aff-d19d-45ff-8113-f0316462d3fa-/executors/test_id/runs/1cd3bd06-2632-4541-a708-80c7cd51c74b/volume'
 of persistent volume '/home/zhitao/mesos-workdir/volumes/roles/role/id1' 
inside garbage collected path 
'/home/zhitao/mesos-workdir/slaves/b3eb3aff-d19d-45ff-8113-f0316462d3fa-S0'
```


Thanks,

Zhitao Li

Re: Review Request 67421: Rewrote the `ROOT_BusyMountPoint` test to reflect updated behavior.

2018-06-05 Thread Zhitao Li 

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

(Updated June 5, 2018, 5:08 p.m.)


Review request for mesos, Chun-Hung Hsiao, Jason Lai, and Jie Yu.


Changes
---

Updated test cases.


Summary (updated)
-

Rewrote the `ROOT_BusyMountPoint` test to reflect updated behavior.


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


Repository: mesos


Description (updated)
---

The current `ROOT_BusyMountPoint` test would fail because we added
support for unmounting dangling mount points in directory to gc. This
patch rewrote this test to reflect that after unmounting, the gc
succeeded, directory was gone and metrics were correctly reported.


Diffs (updated)
-

  src/tests/gc_tests.cpp 619ed22edd9b3909ea24cdcbf62c354420a8d031 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 67465: Windows: Log a fatal error if `WindowsFD(int)` is incorrectly used.

2018-06-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 67465 was successfully built and tested.

Reviews applied: `['67465']`

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

- Mesos Reviewbot Windows


On June 5, 2018, 4:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67465/
> ---
> 
> (Updated June 5, 2018, 4:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This constructor _should_ be `explicit` as the other constructors are;
> however, for historical reasons it is implicit in order to support the
> semantics of assigning from `-1, 0, 1, 2`. Unfortunately, its
> implicitness leads to subtle bugs. For instance, the code `int_fd s =
> ::socket(...)` on Windows compiles, but behaves incorrectly because
> the RHS `SOCKET` type is implicitly converted to an `int` to satisfy
> the only available implicit constructor, since the `WindowsFD(SOCKET)`
> constructor is marked `explicit`. As we should never construct off any
> of these values, but cannot constrain it at compile-time, we should
> add a fatal runtime error instead.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 5dbdff2680370d123579c5e3fdd9b0e0adaf512e 
> 
> 
> Diff: https://reviews.apache.org/r/67465/diff/1/
> 
> 
> Testing
> ---
> 
> Still building, but this is what I propose. This is better than subtle sad 
> panda bugs.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67464: Fixed `python3/mesos-style.py`.

2018-06-05 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 67464 was successfully built and tested.

Reviews applied: `['67464']`

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

- Mesos Reviewbot Windows


On June 5, 2018, 4:15 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67464/
> ---
> 
> (Updated June 5, 2018, 4:15 p.m.)
> 
> 
> Review request for mesos and Armand Grillet.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `cpplint.py` file was made to be single-source compatible with
> both Python 2 and Python 3, but `mesos-style.py` was erroneously
> changed to look for a copy of it under `support/python3`.
> 
> 
> Diffs
> -
> 
>   support/python3/mesos-style.py 6f9b9d4611c6501f8a2dbda4bfe08cffaadc6d86 
> 
> 
> Diff: https://reviews.apache.org/r/67464/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67465: Windows: Log a fatal error if `WindowsFD(int)` is incorrectly used.

2018-06-05 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [67465]

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 June 5, 2018, 11:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67465/
> ---
> 
> (Updated June 5, 2018, 11:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This constructor _should_ be `explicit` as the other constructors are;
> however, for historical reasons it is implicit in order to support the
> semantics of assigning from `-1, 0, 1, 2`. Unfortunately, its
> implicitness leads to subtle bugs. For instance, the code `int_fd s =
> ::socket(...)` on Windows compiles, but behaves incorrectly because
> the RHS `SOCKET` type is implicitly converted to an `int` to satisfy
> the only available implicit constructor, since the `WindowsFD(SOCKET)`
> constructor is marked `explicit`. As we should never construct off any
> of these values, but cannot constrain it at compile-time, we should
> add a fatal runtime error instead.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 5dbdff2680370d123579c5e3fdd9b0e0adaf512e 
> 
> 
> Diff: https://reviews.apache.org/r/67465/diff/1/
> 
> 
> Testing
> ---
> 
> Still building, but this is what I propose. This is better than subtle sad 
> panda bugs.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>