Re: Review Request 67601: Added field container_id in ResourceUsage to agent monitor endpoint.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67601 was successfully built and tested.

Reviews applied: `['67601']`

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

- Mesos Reviewbot Windows


On June 27, 2018, 3:03 a.m., longfei niu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67601/
> ---
> 
> (Updated June 27, 2018, 3:03 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8816
> https://issues.apache.org/jira/browse/MESOS-8816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added field container_id in ResourceUsage to agent monitor endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp a6739e12e55431a84844c747e584ef6420694076 
>   src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed 
> 
> 
> Diff: https://reviews.apache.org/r/67601/diff/3/
> 
> 
> Testing
> ---
> 
> add unit test to src/tests/slave_tests.cpp line 2414 to test whether 
> container_id field added sucessfully or not
> 
> 
> Thanks,
> 
> longfei niu
> 
>

Re: Review Request 67601: Added field container_id in ResourceUsage to agent monitor endpoint.

[longfei niu]

> On 六月 21, 2018, 6:52 p.m., Gilbert Song wrote:
> > src/tests/slave_tests.cpp
> > Lines 2413 (patched)
> > 
> >
> > Instead of to string.find(), I would prefer:
> > 
> > ```
> >   ASSERT_TRUE(value->is());
> > 
> >   JSON::Object object = value->as();
> >   EXPECT_SOME(object.find("container_id"));
> > ```

Got it!Let me fix.


- longfei


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


On 六月 27, 2018, 3:03 a.m., longfei niu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67601/
> ---
> 
> (Updated 六月 27, 2018, 3:03 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8816
> https://issues.apache.org/jira/browse/MESOS-8816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added field container_id in ResourceUsage to agent monitor endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp a6739e12e55431a84844c747e584ef6420694076 
>   src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed 
> 
> 
> Diff: https://reviews.apache.org/r/67601/diff/3/
> 
> 
> Testing
> ---
> 
> add unit test to src/tests/slave_tests.cpp line 2414 to test whether 
> container_id field added sucessfully or not
> 
> 
> Thanks,
> 
> longfei niu
> 
>

Re: Review Request 67601: Added field container_id in ResourceUsage to agent monitor endpoint.

[longfei niu]

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

(Updated 六月 27, 2018, 3:03 a.m.)


Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Added field container_id in ResourceUsage to agent monitor endpoint.


Diffs (updated)
-

  src/slave/http.cpp a6739e12e55431a84844c747e584ef6420694076 
  src/tests/slave_tests.cpp 3d67511de5abd3466eeb5ad1daf318209bd69eed 


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

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


Testing
---

add unit test to src/tests/slave_tests.cpp line 2414 to test whether 
container_id field added sucessfully or not


Thanks,

longfei niu

Re: Review Request 67730: Made Git normalize all line endings automatically.

[Till Toenshoff]

> On June 25, 2018, 11:26 p.m., Till Toenshoff wrote:
> > Given that the problem triggering change on the CI got reverted, can we 
> > cann this review?
> 
> Andrew Schwartzmeyer wrote:
> I think we'll still want this as it will enable us to avoid this problem 
> entirely in the future. Simply put, Mesos on Windows will fail to build (with 
> a cryptic error) if this global Git configuration is set differently. 
> Unfortunately, other Git repos may require a different setting, so we should 
> not depend on a global configuration, but instead set it in our repo itself.

Makes sense - then let's get this to work :)


- Till


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


On June 25, 2018, 8:21 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67730/
> ---
> 
> (Updated June 25, 2018, 8:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of relying on the user to `core.autocrlf = true` before
> cloning, we now set this in `.gitattributes` for all files.
> 
> Only one file needed normalization via `git add --renormalize .`, it
> was `site/doap.rdf`.
> 
> The Windows instructions were updated to remove the note about setting
> autocrlf globally, and the outdated note to use Python 2 was updated
> while we were in the file.
> 
> 
> Diffs
> -
> 
>   .gitattributes PRE-CREATION 
>   docs/windows.md 635c62c678e0a295d55aa958c75c7164c194399f 
>   site/doap.rdf ce23184de617e4f4cd82018d5847561dd84a3865 
> 
> 
> Diff: https://reviews.apache.org/r/67730/diff/2/
> 
> 
> Testing
> ---
> 
> This RR is the test since it'll trigger the currently broken review bot.
> 
> Also see https://git-scm.com/docs/gitattributes
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67751: WIP: Added missing files to CMake build.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67751]

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 26, 2018, 11:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67751/
> ---
> 
> (Updated June 26, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-8994
> https://issues.apache.org/jira/browse/MESOS-8994
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Added missing files to CMake build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am bd94a6488c1c1cc2481b9e9edb25307ced8c0d21 
>   src/cli/CMakeLists.txt 7b2abf2fe14888ec1da11414189f71da972ac427 
>   src/python/executor/CMakeLists.txt PRE-CREATION 
>   src/python/scheduler/CMakeLists.txt PRE-CREATION 
>   src/slave/containerizer/mesos/CMakeLists.txt 
> ba1f92fe7dd59c34c6dee0bc7ecf6f1b5160eee8 
>   src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
> 
> 
> Diff: https://reviews.apache.org/r/67751/diff/1/
> 
> 
> Testing
> ---
> 
> This does not add the `option(FOO)` yet to the configuration, not is there 
> logic (yet) to find the necessary libraries to enable those options. How do 
> we want to proceed with this? I was thinking add each `option(ENABLE_XFS)` 
> etc. followed by a `if (ENABLE_XFS) message(FATAL_ERROR "Please add the 
> necessary logic to CMake to build this and see MESOS-1234."` ... but it may 
> honestly take just as much time to add the `find_library` logic myself...
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67751: WIP: Added missing files to CMake build.

[Mesos Reviewbot Windows]

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



FAIL: Mesos binaries failed to build.

Reviews applied: `['67751']`

Failed command: `cmake.exe --build . --config Release -- /maxcpucount`

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

Relevant logs:

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

```
   "D:\DCOS\mesos\ALL_BUILD.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\master\mesos-master.vcxproj" (default target) (43) ->
 d:\dcos\mesos\mesos\3rdparty\stout\include\stout\exit.hpp(69): warning 
C4722: '__Exit::~__Exit': destructor never returns, potential memory leak 
[D:\DCOS\mesos\src\master\mesos-master.vcxproj]


   "D:\DCOS\mesos\ALL_BUILD.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj" (default target) 
(39) ->
 d:\dcos\mesos\mesos\3rdparty\stout\include\stout\exit.hpp(69): warning 
C4722: '__Exit::~__Exit': destructor never returns, potential memory leak 
[D:\DCOS\mesos\src\launcher\mesos-executor.vcxproj]


   "D:\DCOS\mesos\ALL_BUILD.vcxproj" (default target) (1) ->
   
"D:\DCOS\mesos\src\resource_provider\storage\uri_disk_profile_adaptor.vcxproj" 
(default target) (46) ->
 d:\dcos\mesos\mesos\3rdparty\stout\include\stout\exit.hpp(69): warning 
C4722: '__Exit::~__Exit': destructor never returns, potential memory leak 
[D:\DCOS\mesos\src\resource_provider\storage\uri_disk_profile_adaptor.vcxproj]


   "D:\DCOS\mesos\ALL_BUILD.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\slave\mesos-agent.vcxproj" (default target) (32) ->
 d:\dcos\mesos\mesos\3rdparty\stout\include\stout\exit.hpp(69): warning 
C4722: '__Exit::~__Exit': destructor never returns, potential memory leak 
[D:\DCOS\mesos\src\slave\mesos-agent.vcxproj]


   "D:\DCOS\mesos\ALL_BUILD.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\cli\mesos-cli.vcxproj" (default target) (36) ->
   (ClCompile target) -> 
 d:\dcos\mesos\mesos\src\cli\mesos.cpp(17): fatal error C1083: Cannot 
open include file: 'unistd.h': No such file or directory 
[D:\DCOS\mesos\src\cli\mesos-cli.vcxproj]

506 Warning(s)
1 Error(s)

Time Elapsed 00:26:33.98
```

- Mesos Reviewbot Windows


On June 26, 2018, 11:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67751/
> ---
> 
> (Updated June 26, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-8994
> https://issues.apache.org/jira/browse/MESOS-8994
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Added missing files to CMake build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am bd94a6488c1c1cc2481b9e9edb25307ced8c0d21 
>   src/cli/CMakeLists.txt 7b2abf2fe14888ec1da11414189f71da972ac427 
>   src/python/executor/CMakeLists.txt PRE-CREATION 
>   src/python/scheduler/CMakeLists.txt PRE-CREATION 
>   src/slave/containerizer/mesos/CMakeLists.txt 
> ba1f92fe7dd59c34c6dee0bc7ecf6f1b5160eee8 
>   src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
> 
> 
> Diff: https://reviews.apache.org/r/67751/diff/1/
> 
> 
> Testing
> ---
> 
> This does not add the `option(FOO)` yet to the configuration, not is there 
> logic (yet) to find the necessary libraries to enable those options. How do 
> we want to proceed with this? I was thinking add each `option(ENABLE_XFS)` 
> etc. followed by a `if (ENABLE_XFS) message(FATAL_ERROR "Please add the 
> necessary logic to CMake to build this and see MESOS-1234."` ... but it may 
> honestly take just as much time to add the `find_library` logic myself...
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67388: Moved libprocess POSIX and Windows implementations to separate folders.

[Benjamin Mahler]

> On June 4, 2018, 11:53 p.m., Joseph Wu wrote:
> > General note on code organization:
> > 
> > Since you are splitting the `io::internal::read/write` functions into Posix 
> > and Windows implementations, it would be helpful to organize the code like:
> > ```
> > 3rdparty/libprocess/src/
> > |-- io/
> > |-- io.cpp
> > |
> > |-- posix/
> > |   |-- io.hpp // Instead of io_internal.hpp
> > |   |-- io.cpp // Instead of poll_io.cpp
> > |
> > |-- windows/
> > |   // Instead of libwinio_impl.hpp ( 
> > https://reviews.apache.org/r/67389/ )
> > |   // Possibly combine libwinio.hpp too ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- event_loop.hpp 
> > |
> > |   // Instead of libwinio_impl.cpp ( 
> > https://reviews.apache.org/r/67389/ )
> > |   // Possibly combine libwinio_eventloop.cpp too ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- event_loop.cpp 
> > |
> > |-- io.hpp 
> > |-- io.cpp // Instead of libwinio_io.cpp ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- ...
> > ```
> 
> Andrew Schwartzmeyer wrote:
> +1

Doesn't this type of approach mean that I have look at two different headers 
(POSIX and Windows) whenever I want to write cross-platform code? I would hope 
that I only have to look at one header unless I need to #ifdef in the call site.


- Benjamin


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


On June 21, 2018, 11:47 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67388/
> ---
> 
> (Updated June 21, 2018, 11:47 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In preparation of the new Windows IOCP library, the POSIX and Windows
> specific files in libprocess have been moved to their own directories
> for better code organization.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 8910416ce4313a0d70721cf1bb1d1453aaf691f9 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 619183eff6d6d301a011ab03f007410f50a0aa4f 
>   3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 
>   3rdparty/libprocess/src/io_internal.hpp PRE-CREATION 
>   3rdparty/libprocess/src/libev.hpp  
>   3rdparty/libprocess/src/libev.cpp  
>   3rdparty/libprocess/src/libev_poll.cpp  
>   3rdparty/libprocess/src/libevent.hpp  
>   3rdparty/libprocess/src/libevent.cpp  
>   3rdparty/libprocess/src/libevent_poll.cpp  
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp  
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp  
>   3rdparty/libprocess/src/poll_socket.cpp  
>   3rdparty/libprocess/src/posix/io.cpp PRE-CREATION 
>   3rdparty/libprocess/src/socket.cpp 504cb541785650d2d05aabd25f5258b9bad52baa 
>   3rdparty/libprocess/src/subprocess.cpp 
> 0b2c02a9651563961532fdd5ab0f6d558f69f74e 
>   3rdparty/libprocess/src/subprocess_posix.hpp  
>   3rdparty/libprocess/src/subprocess_posix.cpp  
>   3rdparty/libprocess/src/subprocess_windows.hpp  
>   3rdparty/libprocess/src/subprocess_windows.cpp  
> 
> 
> Diff: https://reviews.apache.org/r/67388/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67751: WIP: Added missing files to CMake build.

[Andrew Schwartzmeyer]

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




src/Makefile.am
Lines 2711-2718 (original), 2711-2718 (patched)


This was backwards from where it was first declared.


- Andrew Schwartzmeyer


On June 26, 2018, 4:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67751/
> ---
> 
> (Updated June 26, 2018, 4:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-8994
> https://issues.apache.org/jira/browse/MESOS-8994
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Added missing files to CMake build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am bd94a6488c1c1cc2481b9e9edb25307ced8c0d21 
>   src/cli/CMakeLists.txt 7b2abf2fe14888ec1da11414189f71da972ac427 
>   src/python/executor/CMakeLists.txt PRE-CREATION 
>   src/python/scheduler/CMakeLists.txt PRE-CREATION 
>   src/slave/containerizer/mesos/CMakeLists.txt 
> ba1f92fe7dd59c34c6dee0bc7ecf6f1b5160eee8 
>   src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
> 
> 
> Diff: https://reviews.apache.org/r/67751/diff/1/
> 
> 
> Testing
> ---
> 
> This does not add the `option(FOO)` yet to the configuration, not is there 
> logic (yet) to find the necessary libraries to enable those options. How do 
> we want to proceed with this? I was thinking add each `option(ENABLE_XFS)` 
> etc. followed by a `if (ENABLE_XFS) message(FATAL_ERROR "Please add the 
> necessary logic to CMake to build this and see MESOS-1234."` ... but it may 
> honestly take just as much time to add the `find_library` logic myself...
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 67751: WIP: Added missing files to CMake build.

[Andrew Schwartzmeyer]

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

Review request for mesos, Benjamin Bannier, James Peach, and Joseph Wu.


Repository: mesos


Description
---

WIP: Added missing files to CMake build.


Diffs
-

  src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
  src/Makefile.am bd94a6488c1c1cc2481b9e9edb25307ced8c0d21 
  src/cli/CMakeLists.txt 7b2abf2fe14888ec1da11414189f71da972ac427 
  src/python/executor/CMakeLists.txt PRE-CREATION 
  src/python/scheduler/CMakeLists.txt PRE-CREATION 
  src/slave/containerizer/mesos/CMakeLists.txt 
ba1f92fe7dd59c34c6dee0bc7ecf6f1b5160eee8 
  src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 


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


Testing
---

This does not add the `option(FOO)` yet to the configuration, not is there 
logic (yet) to find the necessary libraries to enable those options. How do we 
want to proceed with this? I was thinking add each `option(ENABLE_XFS)` etc. 
followed by a `if (ENABLE_XFS) message(FATAL_ERROR "Please add the necessary 
logic to CMake to build this and see MESOS-1234."` ... but it may honestly take 
just as much time to add the `find_library` logic myself...


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67707: Added a support script to check for files missing in the cmake build.

[Andrew Schwartzmeyer]

> On June 25, 2018, 2:15 p.m., Andrew Schwartzmeyer wrote:
> > support/check-cmake-missing-files.sh
> > Lines 23 (patched)
> > 
> >
> > Issue found in testing: this isn't compatible with Ubuntu's "sh" (which 
> > is actually dash).
> 
> Benjamin Bannier wrote:
> I feel while unfortunate this shouldn't be a blocker going forward; in 
> other support scripts we also do not guard against Ubuntu using a non POSIX 
> `sh` implementation by default.
> 
> What do you think Andrew?

Agreed. Not our fault Ubuntu ships with Dash.


- Andrew


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


On June 22, 2018, 2:47 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67707/
> ---
> 
> (Updated June 22, 2018, 2:47 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8994
> https://issues.apache.org/jira/browse/MESOS-8994
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a support script to check for files missing in the cmake build.
> 
> 
> Diffs
> -
> 
>   support/check-cmake-missing-files.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67707/diff/2/
> 
> 
> Testing
> ---
> 
> Ran the added script and verified that the files where indeed missing from 
> the cmake build.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67707: Added a support script to check for files missing in the cmake build.

[Benjamin Bannier]

> On June 25, 2018, 11:15 p.m., Andrew Schwartzmeyer wrote:
> > support/check-cmake-missing-files.sh
> > Lines 23 (patched)
> > 
> >
> > Issue found in testing: this isn't compatible with Ubuntu's "sh" (which 
> > is actually dash).

I feel while unfortunate this shouldn't be a blocker going forward; in other 
support scripts we also do not guard against Ubuntu using a non POSIX `sh` 
implementation by default.

What do you think Andrew?


- Benjamin


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


On June 22, 2018, 11:47 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67707/
> ---
> 
> (Updated June 22, 2018, 11:47 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8994
> https://issues.apache.org/jira/browse/MESOS-8994
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a support script to check for files missing in the cmake build.
> 
> 
> Diffs
> -
> 
>   support/check-cmake-missing-files.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67707/diff/2/
> 
> 
> Testing
> ---
> 
> Ran the added script and verified that the files where indeed missing from 
> the cmake build.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67748: Added monitoring paragraph to high-availability.md.

[Joerg Schad]

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


Fix it, then Ship it!




Thanks Till, great addition. Only one comment regarding the exact defition of 
the ensemble size metric.


docs/high-availability.md
Lines 58 (patched)


Can we check what the exact defintion of this metric is? (connected to each 
other, connected to the leader, how much does it depend on the queried master). 
I unfortunately do not have the correct/best formulation here at hand, but I 
feel that this is a bit ambigious: 

@xujyan should know more here.


- Joerg Schad


On June 26, 2018, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67748/
> ---
> 
> (Updated June 26, 2018, 7:12 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added monitoring paragraph to high-availability.md.
> 
> 
> Diffs
> -
> 
>   docs/high-availability.md f5d25e910763a85ddbb8c43d3dd5019c9a3b00af 
> 
> 
> Diff: https://reviews.apache.org/r/67748/diff/1/
> 
> 
> Testing
> ---
> 
> Rendered by github markdown renderer.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 67748: Added monitoring paragraph to high-availability.md.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67748 was successfully built and tested.

Reviews applied: `['67748']`

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

- Mesos Reviewbot Windows


On June 26, 2018, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67748/
> ---
> 
> (Updated June 26, 2018, 7:12 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added monitoring paragraph to high-availability.md.
> 
> 
> Diffs
> -
> 
>   docs/high-availability.md f5d25e910763a85ddbb8c43d3dd5019c9a3b00af 
> 
> 
> Diff: https://reviews.apache.org/r/67748/diff/1/
> 
> 
> Testing
> ---
> 
> Rendered by github markdown renderer.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 67748: Added monitoring paragraph to high-availability.md.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67748]

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 26, 2018, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67748/
> ---
> 
> (Updated June 26, 2018, 7:12 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added monitoring paragraph to high-availability.md.
> 
> 
> Diffs
> -
> 
>   docs/high-availability.md f5d25e910763a85ddbb8c43d3dd5019c9a3b00af 
> 
> 
> Diff: https://reviews.apache.org/r/67748/diff/1/
> 
> 
> Testing
> ---
> 
> Rendered by github markdown renderer.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 67748: Added monitoring paragraph to high-availability.md.

[Till Toenshoff]

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

Review request for mesos, Gastón Kleiman and Joerg Schad.


Repository: mesos


Description
---

Added monitoring paragraph to high-availability.md.


Diffs
-

  docs/high-availability.md f5d25e910763a85ddbb8c43d3dd5019c9a3b00af 


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


Testing
---

Rendered by github markdown renderer.


Thanks,

Till Toenshoff

Re: Review Request 67742: Allowed SOME principal in ACLs for new destroy operations.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67501, 67723, 67724, 67725, 67742]

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 26, 2018, 7:13 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67742/
> ---
> 
> (Updated June 26, 2018, 7:13 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-7329
> https://issues.apache.org/jira/browse/MESOS-7329
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds support for controlling the principals of disk
> resources a local authorizer subject is allowed to work on with
> `DESTROY_MOUNT` and `DESTROY_BLOCK` operations.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
>   include/mesos/authorizer/acls.proto 
> e4889939481dabe6c1c2876a54d654f98d00dec8 
>   src/authorizer/local/authorizer.cpp 
> 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67742/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67738: Renamed APR and SVN `find_package` implementations.

[Andrew Schwartzmeyer]

> On June 26, 2018, 2:21 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Failed to apply the current review.
> > 
> > Failed command: `python.exe .\support\python3\apply-reviews.py -n -r 67738`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67738
> > 
> > Relevant logs:
> > 
> > - 
> > [apply-review-67738-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67738/logs/apply-review-67738-stdout.log):
> > 
> > ```
> > error: 3rdparty/stout/cmake/FindAPR.cmake: already exists in working 
> > directory
> > error: 3rdparty/stout/cmake/FindSVN.cmake: already exists in working 
> > directory
> > ```

HAHAHA case-insensitive file-systems. Safe to ignore.


- Andrew


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


On June 26, 2018, 12:41 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67738/
> ---
> 
> (Updated June 26, 2018, 12:41 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upstream follows a convention of using ALL UPERCASE letters for
> package names for implementations of `FindPACKAGE.cmake`. This patch
> renames our `find_package` implementations fro APR and SVN.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 1360d188e2eef6743951db005f651aac499c606c 
>   3rdparty/stout/cmake/FindApr.cmake  
>   3rdparty/stout/cmake/FindSvn.cmake  
> 
> 
> Diff: https://reviews.apache.org/r/67738/diff/1/
> 
> 
> Testing
> ---
> 
> * fresh build works
> * rebuild works, `ninja clean && ninja tests`, `ninja tests`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67738: Renamed APR and SVN `find_package` implementations.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On June 26, 2018, 12:41 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67738/
> ---
> 
> (Updated June 26, 2018, 12:41 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upstream follows a convention of using ALL UPERCASE letters for
> package names for implementations of `FindPACKAGE.cmake`. This patch
> renames our `find_package` implementations fro APR and SVN.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 1360d188e2eef6743951db005f651aac499c606c 
>   3rdparty/stout/cmake/FindApr.cmake  
>   3rdparty/stout/cmake/FindSvn.cmake  
> 
> 
> Diff: https://reviews.apache.org/r/67738/diff/1/
> 
> 
> Testing
> ---
> 
> * fresh build works
> * rebuild works, `ninja clean && ninja tests`, `ninja tests`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67730: Made Git normalize all line endings automatically.

[Andrew Schwartzmeyer]

> On June 25, 2018, 1:44 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> > 
> > Reviews applied: `['67730']`
> > 
> > Failed command: `Start-MesosCITesting`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67730
> > 
> > Relevant logs:
> > 
> > - 
> > [stout-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67730/logs/stout-tests-cmake-stdout.log):
> > 
> > ```
> >"D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj" (default target) (10) ->
> >(CustomBuild target) -> 
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj" (default target) 
> > (3) ->
> >"D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj" (default target) (4) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj" (default target) (9) 
> > ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj" (default target) (7) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) 
> > (14) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]
> > 
> > 1 Warning(s)
> > 5 Error(s)
> > 
> > Time Elapsed 00:03:16.35
> > ```
> > 
> > - 
> > [libprocess-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67730/logs/libprocess-tests-cmake-stdout.log):
> > 
> > ```
> >(CustomBuild target) -> 
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj" (default target) 
> > (10) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj" (default target) (4) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\libprocess\src\tests\benchmarks.vcxproj" 
> > (default target) (7) ->
> >"D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) 
> > (19) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\libprocess\src\tests\benchmarks.vcxproj" 
> > (default target) (7) ->
> >

Re: Review Request 67730: Made Git normalize all line endings automatically.

[Andrew Schwartzmeyer]

> On June 25, 2018, 4:26 p.m., Till Toenshoff wrote:
> > Given that the problem triggering change on the CI got reverted, can we 
> > cann this review?

I think we'll still want this as it will enable us to avoid this problem 
entirely in the future. Simply put, Mesos on Windows will fail to build (with a 
cryptic error) if this global Git configuration is set differently. 
Unfortunately, other Git repos may require a different setting, so we should 
not depend on a global configuration, but instead set it in our repo itself.


- Andrew


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


On June 25, 2018, 1:21 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67730/
> ---
> 
> (Updated June 25, 2018, 1:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of relying on the user to `core.autocrlf = true` before
> cloning, we now set this in `.gitattributes` for all files.
> 
> Only one file needed normalization via `git add --renormalize .`, it
> was `site/doap.rdf`.
> 
> The Windows instructions were updated to remove the note about setting
> autocrlf globally, and the outdated note to use Python 2 was updated
> while we were in the file.
> 
> 
> Diffs
> -
> 
>   .gitattributes PRE-CREATION 
>   docs/windows.md 635c62c678e0a295d55aa958c75c7164c194399f 
>   site/doap.rdf ce23184de617e4f4cd82018d5847561dd84a3865 
> 
> 
> Diff: https://reviews.apache.org/r/67730/diff/2/
> 
> 
> Testing
> ---
> 
> This RR is the test since it'll trigger the currently broken review bot.
> 
> Also see https://git-scm.com/docs/gitattributes
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67743: Made `gpu/nvidia` isolator works with `cgroups/all` isolation option.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67743 was successfully built and tested.

Reviews applied: `['67743']`

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

- Mesos Reviewbot Windows


On June 26, 2018, 1:58 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67743/
> ---
> 
> (Updated June 26, 2018, 1:58 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-9027
> https://issues.apache.org/jira/browse/MESOS-9027
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `gpu/nvidia` isolator works with `cgroups/all` isolation option.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> d79c940498fe3cfeeaddad5f5e5e21fcdf4e04da 
> 
> 
> Diff: https://reviews.apache.org/r/67743/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67742: Allowed SOME principal in ACLs for new destroy operations.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67742 was successfully built and tested.

Reviews applied: `['67501', '67723', '67724', '67725', '67742']`

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

- Mesos Reviewbot Windows


On June 26, 2018, 2:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67742/
> ---
> 
> (Updated June 26, 2018, 2:13 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-7329
> https://issues.apache.org/jira/browse/MESOS-7329
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds support for controlling the principals of disk
> resources a local authorizer subject is allowed to work on with
> `DESTROY_MOUNT` and `DESTROY_BLOCK` operations.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
>   include/mesos/authorizer/acls.proto 
> e4889939481dabe6c1c2876a54d654f98d00dec8 
>   src/authorizer/local/authorizer.cpp 
> 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67742/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67737: Updated CNI slave recovery test.

[Jie Yu]

> On June 26, 2018, 7:44 a.m., Qian Zhang wrote:
> > Do we still need to kill the task and wait for `TASK_KILLED`?
> 
> Qian Zhang wrote:
> And is it possible for CNI DEL command gets called after 
> `reregisterExecutorMessage` is received?

It's not possible. recover containerizer should happen first before executors 
are allowed to re-register.


- Jie


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


On June 26, 2018, 5:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67737/
> ---
> 
> (Updated June 26, 2018, 5:36 a.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Bugs: MESOS-9025
> https://issues.apache.org/jira/browse/MESOS-9025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the test so that it can catch regression described in
> MESOS-9025. This test fails without the fix for MESOS-9025 and passes
> after the fix.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> b58a9caca136cfa42689159389bfdcb3f92f05ee 
> 
> 
> Diff: https://reviews.apache.org/r/67737/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 67743: Made `gpu/nvidia` isolator works with `cgroups/all` isolation option.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67743]

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 26, 2018, 1:58 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67743/
> ---
> 
> (Updated June 26, 2018, 1:58 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-9027
> https://issues.apache.org/jira/browse/MESOS-9027
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `gpu/nvidia` isolator works with `cgroups/all` isolation option.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> d79c940498fe3cfeeaddad5f5e5e21fcdf4e04da 
> 
> 
> Diff: https://reviews.apache.org/r/67743/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67722: Fixed unproperly guarded future.

[Greg Mann]

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




src/master/master.cpp
Lines 9667-9672 (patched)


Perhaps we could provide more precise logging here?

If the future is failed, I might expect to see a log like:
"An error was encountered while attempting authentication of : "

If the Option is NONE, I might expect something like:
"Authentication of  was unsuccessful: invalid credentials"

WDYT?


- Greg Mann


On June 25, 2018, 5:56 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67722/
> ---
> 
> (Updated June 25, 2018, 5:56 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes a bug where the code path could cause a crash because
> of calling `Fture::get()` on a future which is failed.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
> 
> 
> Diff: https://reviews.apache.org/r/67722/diff/1/
> 
> 
> Testing
> ---
> 
> ```sh
> make check
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 67722: Fixed unproperly guarded future.

[Gastón Kleiman]

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



A regression test would be nice, I guess that would involve mocking an 
authenticator?


src/master/master.cpp
Lines 9669-9670 (patched)


I think the previous wording was clearer. If there is an error, the new 
error message implies that authorization failed, but what actually failed was 
the authentication.

Nit: `Unauthorized` starts with upper case, but `discarded` with lower 
case, if we decide to keep the new wording, I'd try to be consistent.


- Gastón Kleiman


On June 25, 2018, 10:56 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67722/
> ---
> 
> (Updated June 25, 2018, 10:56 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes a bug where the code path could cause a crash because
> of calling `Fture::get()` on a future which is failed.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 4ade16f044f8a4fdafd5afaba4e6a23232f83a5a 
> 
> 
> Diff: https://reviews.apache.org/r/67722/diff/1/
> 
> 
> Testing
> ---
> 
> ```sh
> make check
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 67737: Updated CNI slave recovery test.

[Qian Zhang]

> On June 26, 2018, 3:44 p.m., Qian Zhang wrote:
> > Do we still need to kill the task and wait for `TASK_KILLED`?

And is it possible for CNI DEL command gets called after 
`reregisterExecutorMessage` is received?


- Qian


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


On June 26, 2018, 1:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67737/
> ---
> 
> (Updated June 26, 2018, 1:36 p.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Bugs: MESOS-9025
> https://issues.apache.org/jira/browse/MESOS-9025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the test so that it can catch regression described in
> MESOS-9025. This test fails without the fix for MESOS-9025 and passes
> after the fix.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> b58a9caca136cfa42689159389bfdcb3f92f05ee 
> 
> 
> Diff: https://reviews.apache.org/r/67737/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 67634: Removed PICOJSON_USE_INT64 from Mesos build system.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67634 was successfully built and tested.

Reviews applied: `['67632', '67633', '67634']`

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

- Mesos Reviewbot Windows


On June 18, 2018, 3:53 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67634/
> ---
> 
> (Updated June 18, 2018, 3:53 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It is no longer needed, since the macro is now directly
> defined in json.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 3c1dfcc7885693cd76331b515ca8e361bd1976d0 
>   mesos.pc.in 61d6d0b2883f1893100c8aa0f309c2d27de4ea14 
>   src/Makefile.am 2f7f7f8b8683fc9ecd5685d28d36bd17f7e4a24a 
> 
> 
> Diff: https://reviews.apache.org/r/67634/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67742: Allowed SOME principal in ACLs for new destroy operations.

[Benjamin Bannier]

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

(Updated June 26, 2018, 4:13 p.m.)


Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds support for controlling the principals of disk
resources a local authorizer subject is allowed to work on with
`DESTROY_MOUNT` and `DESTROY_BLOCK` operations.


Diffs (updated)
-

  docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
  include/mesos/authorizer/acls.proto e4889939481dabe6c1c2876a54d654f98d00dec8 
  src/authorizer/local/authorizer.cpp 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 67743: Made `gpu/nvidia` isolator works with `cgroups/all` isolation option.

[Qian Zhang]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Made `gpu/nvidia` isolator works with `cgroups/all` isolation option.


Diffs
-

  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
d79c940498fe3cfeeaddad5f5e5e21fcdf4e04da 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 67738: Renamed APR and SVN `find_package` implementations.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67738]

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 26, 2018, 7:41 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67738/
> ---
> 
> (Updated June 26, 2018, 7:41 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upstream follows a convention of using ALL UPERCASE letters for
> package names for implementations of `FindPACKAGE.cmake`. This patch
> renames our `find_package` implementations fro APR and SVN.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 1360d188e2eef6743951db005f651aac499c606c 
>   3rdparty/stout/cmake/FindApr.cmake  
>   3rdparty/stout/cmake/FindSvn.cmake  
> 
> 
> Diff: https://reviews.apache.org/r/67738/diff/1/
> 
> 
> Testing
> ---
> 
> * fresh build works
> * rebuild works, `ninja clean && ninja tests`, `ninja tests`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 67742: Allowed SOME principal in ACLs for new destroy operations.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds support for controlling the principals of disk
resources a local authorizer subject is allowed to work on with
`DESTROY_MOUNT` and `DESTROY_BLOCK` operations.


Diffs
-

  src/authorizer/local/authorizer.cpp 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 67634: Removed PICOJSON_USE_INT64 from Mesos build system.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On June 18, 2018, 5:53 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67634/
> ---
> 
> (Updated June 18, 2018, 5:53 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It is no longer needed, since the macro is now directly
> defined in json.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 3c1dfcc7885693cd76331b515ca8e361bd1976d0 
>   mesos.pc.in 61d6d0b2883f1893100c8aa0f309c2d27de4ea14 
>   src/Makefile.am 2f7f7f8b8683fc9ecd5685d28d36bd17f7e4a24a 
> 
> 
> Diff: https://reviews.apache.org/r/67634/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 67632: Always define PICOJSON_USE_INT64.

[Benno Evers]

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

(Updated June 26, 2018, 11:48 a.m.)


Review request for mesos.


Changes
---

Switched around comment and #undef.


Repository: mesos


Description
---

The macro PICOJSON_USE_INT64 must always be defined
when attempting to use stout's JSON facilities, since
they unconditionally depend on these features.

Previously, we relied on the Mesos build system to set
that macro from the command line, but since we don't
generate a separate pkg-config file for stout there is
no way to convey this information to other users of stout.

Even for users trying to use stout as part of a libmesos
installation, it might be surprising that it is required to
read CPPFLAGS if only the header-only stout part is to be
used.


Diffs (updated)
-

  3rdparty/stout/Makefile.am 5b922af6cd58dc03c44aacf88637e89f6e289702 
  3rdparty/stout/include/stout/json.hpp 
5e738cf6f72f32b852beb61a16787e48082dab14 


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

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


Testing
---

Installed stout on my system and compiled an external program using stout's 
JSON processing functions.


Thanks,

Benno Evers

Re: Review Request 67739: Documented the container-specific cgroups mounts feature.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67662, 67526, 67564, 67565, 67673, 67739]

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 26, 2018, 7:40 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67739/
> ---
> 
> (Updated June 26, 2018, 7:40 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8327
> https://issues.apache.org/jira/browse/MESOS-8327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the container-specific cgroups mounts feature.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0e44277389e5a96c221d6c3d501da2bb93d27faa 
>   docs/upgrades.md cc8a251945af27a432dbeb5585b998cd84e35b22 
> 
> 
> Diff: https://reviews.apache.org/r/67739/diff/1/
> 
> 
> Testing
> ---
> 
> Not a code change.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67738: Renamed APR and SVN `find_package` implementations.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: 3rdparty/stout/cmake/FindAPR.cmake: already exists in working directory
error: 3rdparty/stout/cmake/FindSVN.cmake: already exists in working directory
```

- Mesos Reviewbot Windows


On June 26, 2018, 7:41 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67738/
> ---
> 
> (Updated June 26, 2018, 7:41 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upstream follows a convention of using ALL UPERCASE letters for
> package names for implementations of `FindPACKAGE.cmake`. This patch
> renames our `find_package` implementations fro APR and SVN.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 1360d188e2eef6743951db005f651aac499c606c 
>   3rdparty/stout/cmake/FindApr.cmake  
>   3rdparty/stout/cmake/FindSvn.cmake  
> 
> 
> Diff: https://reviews.apache.org/r/67738/diff/1/
> 
> 
> Testing
> ---
> 
> * fresh build works
> * rebuild works, `ninja clean && ninja tests`, `ninja tests`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67739: Documented the container-specific cgroups mounts feature.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67739 was successfully built and tested.

Reviews applied: `['67739']`

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

- Mesos Reviewbot Windows


On June 26, 2018, 7:40 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67739/
> ---
> 
> (Updated June 26, 2018, 7:40 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-8327
> https://issues.apache.org/jira/browse/MESOS-8327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented the container-specific cgroups mounts feature.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0e44277389e5a96c221d6c3d501da2bb93d27faa 
>   docs/upgrades.md cc8a251945af27a432dbeb5585b998cd84e35b22 
> 
> 
> Diff: https://reviews.apache.org/r/67739/diff/1/
> 
> 
> Testing
> ---
> 
> Not a code change.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67722: Fixed unproperly guarded future.

[Alexander Rojas]

> On June 25, 2018, 8:14 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some of the unit tests failed. Please check the relevant logs.
> > 
> > Reviews applied: `['67722']`
> > 
> > Failed command: `Start-MesosCITesting`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67722
> > 
> > Relevant logs:
> > 
> > - 
> > [stout-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67722/logs/stout-tests-cmake-stdout.log):
> > 
> > ```
> >"D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj" (default target) (10) ->
> >(CustomBuild target) -> 
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\libarchive-3.3.2.vcxproj" (default target) 
> > (3) ->
> >"D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj" (default target) (4) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj" (default target) (9) 
> > ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj" (default target) (7) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj]
> > 
> > 
> >"D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
> > target) (1) ->
> >"D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) 
> > (14) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj]
> > 
> > 1 Warning(s)
> > 5 Error(s)
> > 
> > Time Elapsed 00:03:15.61
> > ```
> > 
> > - 
> > [libprocess-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67722/logs/libprocess-tests-cmake-stdout.log):
> > 
> > ```
> >(CustomBuild target) -> 
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\glog-da816ea70.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\libprocess\examples\example.vcxproj" 
> > (default target) (8) ->
> >"D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj" (default target) (20) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\xz-5.2.3.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj" (default target) (5) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj" (default target) (7) ->
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(171,5):
> >  error MSB6006: "cmd.exe" exited with code 3. 
> > [D:\DCOS\mesos\3rdparty\boost-1.65.0.vcxproj]
> > 
> > 
> >
> > "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
> > (default target) (1) ->
> >"D:\DCOS\mesos\3rdparty\libprocess\examples\example.vcxproj" 
> > (default target) (8) ->
> >"D:\DCOS\mesos\3rdparty\protobuf-3.5.0.vcxproj" (default target) 
> > 

Re: Review Request 67737: Updated CNI slave recovery test.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67728, 67737]

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 26, 2018, 5:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67737/
> ---
> 
> (Updated June 26, 2018, 5:36 a.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Bugs: MESOS-9025
> https://issues.apache.org/jira/browse/MESOS-9025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the test so that it can catch regression described in
> MESOS-9025. This test fails without the fix for MESOS-9025 and passes
> after the fix.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> b58a9caca136cfa42689159389bfdcb3f92f05ee 
> 
> 
> Diff: https://reviews.apache.org/r/67737/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 67737: Updated CNI slave recovery test.

[Qian Zhang]

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


Ship it!




Do we still need to kill the task and wait for `TASK_KILLED`?

- Qian Zhang


On June 26, 2018, 1:36 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67737/
> ---
> 
> (Updated June 26, 2018, 1:36 p.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Bugs: MESOS-9025
> https://issues.apache.org/jira/browse/MESOS-9025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the test so that it can catch regression described in
> MESOS-9025. This test fails without the fix for MESOS-9025 and passes
> after the fix.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> b58a9caca136cfa42689159389bfdcb3f92f05ee 
> 
> 
> Diff: https://reviews.apache.org/r/67737/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 67606: Allow for unbundled libevent cmake builds.

[Benjamin Bannier]

> On June 18, 2018, 11:17 a.m., Benjamin Bannier wrote:
> > 3rdparty/cmake/Findlibevent.cmake
> > Lines 1 (patched)
> > 
> >
> > This file should be called `3rdparty/cmake/FindLIBEVENT.cmake` (package 
> > name all UPPERCASE). This is the usual convention, see e.g., cmake upstream.
> 
> Till Toenshoff wrote:
> Interesting - is there a ticket for this techdebt (on the other FindXXX 
> components)?

https://reviews.apache.org/r/67738/


- Benjamin


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


On June 25, 2018, 4:01 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67606/
> ---
> 
> (Updated June 25, 2018, 4:01 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Joseph Wu, 
> and Jan Schlicht.
> 
> 
> Bugs: MESOS-8998
> https://issues.apache.org/jira/browse/MESOS-8998
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow for unbundled libevent cmake builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 3c1dfcc7885693cd76331b515ca8e361bd1976d0 
>   3rdparty/cmake/FindLIBEVENT.cmake PRE-CREATION 
>   cmake/CompilationConfigure.cmake 2f92acb3a140faa48e3639c7000be7f43020ad7d 
>   docs/configuration/cmake.md 74abe65507d251ffb9cbae31a6fa18eb0d76e79b 
> 
> 
> Diff: https://reviews.apache.org/r/67606/diff/5/
> 
> 
> Testing
> ---
> 
> ```
> $ cmake .. -DENABLE_LIBEVENT=TRUE -DENABLE_SSL=TRUE 
> -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl
> $ cmake --build . --target tests -- -j6
> 
> $ otool -L 3rdparty/libprocess/src/libprocess.dylib
> libprocess.dylib:
>   @rpath/libprocess.dylib (compatibility version 0.0.0, current version 
> 0.0.0)
>   /usr/local/opt/openssl/lib/libssl.1.0.0.dylib (compatibility version 
> 1.0.0, current version 1.0.0)
>   /usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib (compatibility version 
> 1.0.0, current version 1.0.0)
>   
> /Users/till/Development/mesos/build/3rdparty/libevent-2.1.5-beta/src/libevent-2.1.5-beta-build/lib/libevent.2.1.5.dylib
>  (compatibility version 2.1.5, current version 0.0.0)
>   /usr/local/opt/apr/libexec/lib/libapr-1.0.dylib (compatibility version 
> 7.0.0, current version 7.3.0)
>   /usr/lib/libcurl.4.dylib (compatibility version 7.0.0, current version 
> 9.0.0)
>   
> /Users/till/Development/mesos/build/3rdparty/glog-0.3.3/src/glog-0.3.3-build/lib/libglog.0.dylib
>  (compatibility version 1.0.0, current version 1.0.0)
>   
> /Users/till/Development/mesos/build/3rdparty/protobuf-3.5.0/src/protobuf-3.5.0-build/libprotobuf.dylib
>  (compatibility version 0.0.0, current version 0.0.0)
>   /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 
> 1.2.11)
>   /usr/local/opt/subversion/lib/libsvn_delta-1.0.dylib (compatibility 
> version 1.0.0, current version 1.0.0)
>   /usr/local/opt/subversion/lib/libsvn_diff-1.0.dylib (compatibility 
> version 1.0.0, current version 1.0.0)
>   /usr/local/opt/subversion/lib/libsvn_subr-1.0.dylib (compatibility 
> version 1.0.0, current version 1.0.0)
>   /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 
> 400.9.3)
>   /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
> version 1252.200.5)
> ```
> 
> ```
> $ cmake .. -DENABLE_LIBEVENT=TRUE -DENABLE_SSL=TRUE 
> -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl -DUNBUNDLED_LIBEVENT=TRUE
> $ cmake --build . --target tests -- -j6
> 
> $ otool -L 3rdparty/libprocess/src/libprocess.dylib
> libprocess.dylib:
>   @rpath/libprocess.dylib (compatibility version 0.0.0, current version 
> 0.0.0)
>   /usr/local/opt/openssl/lib/libssl.1.0.0.dylib (compatibility version 
> 1.0.0, current version 1.0.0)
>   /usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib (compatibility version 
> 1.0.0, current version 1.0.0)
>   /usr/local/opt/libevent/lib/libevent-2.0.5.dylib (compatibility version 
> 7.0.0, current version 7.9.0)
>   /usr/local/opt/apr/libexec/lib/libapr-1.0.dylib (compatibility version 
> 7.0.0, current version 7.3.0)
>   /usr/lib/libcurl.4.dylib (compatibility version 7.0.0, current version 
> 9.0.0)
>   
> /Users/till/Development/mesos/build/3rdparty/glog-0.3.3/src/glog-0.3.3-build/lib/libglog.0.dylib
>  (compatibility version 1.0.0, current version 1.0.0)
>   
> /Users/till/Development/mesos/build/3rdparty/protobuf-3.5.0/src/protobuf-3.5.0-build/libprotobuf.dylib
>  (compatibility version 0.0.0, current version 0.0.0)
>   /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 
> 1.2.11)
>   

Review Request 67738: Renamed APR and SVN `find_package` implementations.

[Benjamin Bannier]

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

Review request for mesos, Andrew Schwartzmeyer and Till Toenshoff.


Repository: mesos


Description
---

Upstream follows a convention of using ALL UPERCASE letters for
package names for implementations of `FindPACKAGE.cmake`. This patch
renames our `find_package` implementations fro APR and SVN.


Diffs
-

  3rdparty/CMakeLists.txt 1360d188e2eef6743951db005f651aac499c606c 
  3rdparty/stout/cmake/FindApr.cmake  
  3rdparty/stout/cmake/FindSvn.cmake  


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


Testing
---

* fresh build works
* rebuild works, `ninja clean && ninja tests`, `ninja tests`


Thanks,

Benjamin Bannier

Review Request 67739: Documented the container-specific cgroups mounts feature.

[Qian Zhang]

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

Review request for mesos, Gilbert Song and Jason Lai.


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


Repository: mesos


Description
---

Documented the container-specific cgroups mounts feature.


Diffs
-

  CHANGELOG 0e44277389e5a96c221d6c3d501da2bb93d27faa 
  docs/upgrades.md cc8a251945af27a432dbeb5585b998cd84e35b22 


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


Testing
---

Not a code change.


Thanks,

Qian Zhang

Re: Review Request 67737: Updated CNI slave recovery test.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67737 was successfully built and tested.

Reviews applied: `['67728', '67737']`

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

- Mesos Reviewbot Windows


On June 26, 2018, 5:36 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67737/
> ---
> 
> (Updated June 26, 2018, 5:36 a.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Bugs: MESOS-9025
> https://issues.apache.org/jira/browse/MESOS-9025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the test so that it can catch regression described in
> MESOS-9025. This test fails without the fix for MESOS-9025 and passes
> after the fix.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> b58a9caca136cfa42689159389bfdcb3f92f05ee 
> 
> 
> Diff: https://reviews.apache.org/r/67737/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 67736: Removed ifdef from library path construction.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67736]

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 26, 2018, 3:57 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67736/
> ---
> 
> (Updated June 26, 2018, 3:57 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Gilbert 
> Song, Jie Yu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We don't need to use ifdefs to construct the platform-specific
> Mesos library name, we can use the stout `os::libraries` API
> instead.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 8edd652f7f410dbadaf6c2ca3736349065e4340a 
> 
> 
> Diff: https://reviews.apache.org/r/67736/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67728: Fixed orphan container cleanup issue in CNI isolator.

[Stéphane Cottin via Review Board]

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


Ship it!




Ship It!

- Stéphane Cottin


On June 25, 2018, 7 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67728/
> ---
> 
> (Updated June 25, 2018, 7 p.m.)
> 
> 
> Review request for mesos, Qian Zhang and Sagar Patwardhan.
> 
> 
> Bugs: MESOS-9025
> https://issues.apache.org/jira/browse/MESOS-9025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The bug was introduced in https://reviews.apache.org/r/65987/ when we
> want to allow nested container to have a separate network namespace than
> its parent (MESOS-8534).
> 
> The original code misses a `continue` statement after recovering regular
> containers.
> 
> I am surprised that the unit tests didn't catch the bug. We'll add a new
> unit test for catching the regression.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> b6ad4fcf9f1596c07ddeb9bbb134f4619d189671 
> 
> 
> Diff: https://reviews.apache.org/r/67728/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>