Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.

[Akash Gupta]

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




src/tests/containerizer/launcher.hpp
Line 60 (original), 60 (patched)


Just curious but why is `forkImpl` needed instead of the old way?


- Akash Gupta


On May 31, 2018, 10:50 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67394/
> ---
> 
> (Updated May 31, 2018, 10:50 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
>   src/slave/containerizer/mesos/launcher.hpp 
> f69d934d2e1a129e10df8c7f5c78723e832adc7d 
>   src/slave/containerizer/mesos/launcher.cpp 
> 2fe47d368cb82a46328e1f636baa836272db244c 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 80e444501e429c1e1ae354abcd51f86430316ada 
>   src/tests/containerizer/launcher.hpp 
> a8e436f164b67d937ebcff35e084d3ca755c003c 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 01f2b38cfa67b144298c361e92170322864ac201 
> 
> 
> Diff: https://reviews.apache.org/r/67394/diff/1/
> 
> 
> Testing
> ---
> 
> All mesos tests on windows
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67287: White list fds that child processes can inherit in libprocess.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On May 24, 2018, 10:47 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67287/
> ---
> 
> (Updated May 24, 2018, 10:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
> Yu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> 6a1262340c333b617402637e648c12769827ffc4 
>   3rdparty/libprocess/src/subprocess.cpp 
> d7a725363251f9c54072cd7551f5598696938308 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 
> 
> 
> Diff: https://reviews.apache.org/r/67287/diff/2/
> 
> 
> Testing
> ---
> 
> All Mesos-tests
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67286: White list fds that child processes can inherit in stout.

[Akash Gupta]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/internal/windows/inherit.hpp
Lines 55 (patched)


Note that this destructor is called regardless of the state of 
`attribute_list`. So, it will call `::DeleteProcThreadAttributeList()` for 
these two cases, where `::InitializeProcThreadAttributeList` hasn't succeeded 
yet.

```
  if (attribute_list == nullptr) {
return WindowsError(ERROR_OUTOFMEMORY);
// Destructor calls `::DeleteProc...(nullptr)`
  }

  result =
::InitializeProcThreadAttributeList(attribute_list.get(), 1, 0, );
  if (result == FALSE) {
return WindowsError();
// Destructor calls `::DeleteProc...()` on uninitialized attribute_list.
  }
```



3rdparty/stout/include/stout/os/windows/shell.hpp
Lines 245 (patched)


I think the mesos empty vector style is `const std::vector& 
whitelist_fds = {}` .


- Akash Gupta


On May 24, 2018, 10:47 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67286/
> ---
> 
> (Updated May 24, 2018, 10:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
> Yu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp 
> 7dbde820e775cbaeb8db4bc4559ab432903e75ea 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
> 
> 
> Diff: https://reviews.apache.org/r/67286/diff/2/
> 
> 
> Testing
> ---
> 
> All Mesos-tests
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67223: Used move constructors for making CSI gRPC calls.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67223 was successfully built and tested.

Reviews applied: `['67157', '67223']`

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

- Mesos Reviewbot Windows


On June 8, 2018, 3:04 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67223/
> ---
> 
> (Updated June 8, 2018, 3:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8924
> https://issues.apache.org/jira/browse/MESOS-8924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch takes the advantages of the moving constructors of the CSI
> request protobuf messages to avoid copying the messages.
> 
> 
> Diffs
> -
> 
>   src/csi/client.hpp 1c57ac52ade6295d5c45ac8a439ea572d16d7ae3 
>   src/csi/client.cpp 923ee6f85dc8070d60bee9834846f859883c73df 
>   src/resource_provider/storage/provider.cpp 
> 36e9109d8cdc7fc1fa29582e2717b84e3e60 
> 
> 
> Diff: https://reviews.apache.org/r/67223/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!





src/tests/containerizer/docker_containerizer_tests.cpp
Line 300 (original), 339 (patched)


What changed here to need another argument?



src/tests/containerizer/docker_containerizer_tests.cpp
Lines 5107-5120 (patched)


I am so sorry.


- Andrew Schwartzmeyer


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 67393: Windows: Ported io_tests.cpp.

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!




:D


3rdparty/libprocess/src/tests/CMakeLists.txt
Line 49 (original), 49 (patched)


Where these not portable?



3rdparty/libprocess/src/tests/io_tests.cpp
Lines 41 (patched)


Leave a NOTE for disabled tests.



3rdparty/libprocess/src/tests/io_tests.cpp
Lines 216-217 (original), 235-240 (patched)


:')


- Andrew Schwartzmeyer


On May 30, 2018, 11:40 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67393/
> ---
> 
> (Updated May 30, 2018, 11:40 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5815
> https://issues.apache.org/jira/browse/MESOS-5815
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the IOCP backened implemented, io_tests.cpp is now ported to
> Windows. Since the tests require async pipe IO, the tests are only
> run if Mesos was compiled with the IOCP backened.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 8c1353b442d5fc7b2e796a4d87f5b41389d4e1c3 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> de7272b3ef0f414f056509448856c6bbd86ef75c 
> 
> 
> Diff: https://reviews.apache.org/r/67393/diff/1/
> 
> 
> Testing
> ---
> 
> The entire chain was tested with `make check` on Linux. The tests were run on 
> Windows as well with `-DENABLE_LIBEVENT` and `-DENABLE_LIBWINIO`.
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67392: Windows: Made PipeLargeOutput test work with IOCP backend.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On May 30, 2018, 11:41 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67392/
> ---
> 
> (Updated May 30, 2018, 11:41 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
> ---
> 
> THe PipeLargeOutput test was calling `io::read`, which was returning the
> wrong error since it operates at a higher level abstraction. The test
> really just checks if a pipe EOF returns `ERROR_BROKEN_PIPE`, so the
> test was changed to use the lower level `os::read`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 568d77b94b3afef8c1ebdde71e14aa0b498da5c8 
> 
> 
> Diff: https://reviews.apache.org/r/67392/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67391: Windows: Added CMake `ENABLE_LIBWINIO` flag for Windows IOCP backend.

[Andrew Schwartzmeyer]

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




3rdparty/libprocess/include/process/io.hpp
Lines 29-39 (original), 29-41 (patched)


Should we re-use the header guard like this, or add a define to CMake?



3rdparty/libprocess/src/CMakeLists.txt
Lines 86-87 (patched)


There shouldn't be headers in CMake source lists... I'm not sure why the 
others are in here either...



3rdparty/libprocess/src/CMakeLists.txt
Lines 118-121 (original), 130-140 (patched)


I think you could just replace `,libev` with 
`,$<$>:libev>` and get rid of the outside 
conditional.



cmake/CompilationConfigure.cmake
Line 87 (original), 87-92 (patched)


I'm sorry... I definitely remember saying I was going to do this...



cmake/CompilationConfigure.cmake
Line 87 (original), 87-92 (patched)


We should sanity check if this got defined on a non-Windows platform.


- Andrew Schwartzmeyer


On May 30, 2018, 11:42 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67391/
> ---
> 
> (Updated May 30, 2018, 11:42 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
> ---
> 
> Added Windows IOCP backend to the build system. Now, there are two async
> backends, which are libevent through `ENABLE_LIBEVENT` and the Windows
> IOCP (libwinio) through `ENABLE_LIBWINIO`.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt ecb6946401d9b81c6610cf9f33dcf2caa9ff0f04 
>   3rdparty/libprocess/include/process/io.hpp 
> cc2caf44e065bed40263f3820e95a4f7c378bb98 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   cmake/CompilationConfigure.cmake 2f92acb3a140faa48e3639c7000be7f43020ad7d 
> 
> 
> Diff: https://reviews.apache.org/r/67391/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67390: Windows: Integrated libwinio with libprocess code.

[Andrew Schwartzmeyer]

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




3rdparty/libprocess/src/libwinio_eventloop.cpp
Lines 52-73 (patched)


I know this code exists elsewhere too... we should refactor into a stout 
function.



3rdparty/libprocess/src/libwinio_io.cpp
Lines 78-101 (patched)


Wait... doesn't this belong in the `prepare_async` from earlier? Did 
something get moved?



3rdparty/libprocess/src/libwinio_socket.cpp
Lines 16-18 (patched)


Huh? Isn't this file called `libWINio_socket.cpp`?



3rdparty/libprocess/src/libwinio_socket.cpp
Lines 100-102 (patched)


I think you want a `WindowsError error; ... + error.message`. Otherwise 
it's just going to print `socket: 3` or whatever.



3rdparty/libprocess/src/libwinio_socket.cpp
Lines 105 (patched)


What is it, and why?



3rdparty/libprocess/src/libwinio_socket.cpp
Lines 147 (patched)


What does that mean?



3rdparty/libprocess/src/libwinio_socket.cpp
Lines 176 (patched)


This code looks very... reused. Nothing necessarily wrong with that, but 
should we reduce it at all?


- Andrew Schwartzmeyer


On May 30, 2018, 11:46 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67390/
> ---
> 
> (Updated May 30, 2018, 11:46 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 libprocess eventloops are used to implement the `EventLoop`,
> SocketImpl` and `io::read/write` interfaces. Thus, by providing those
> implementations using libwinio, libprocess now supports the libwinio
> backend.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libwinio.hpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_eventloop.cpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_io.cpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_socket.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67390/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67223: Used move constructors for making CSI gRPC calls.

[Chun-Hung Hsiao]

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

(Updated June 8, 2018, 10:04 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Zhitao Li.


Changes
---

Made the client call functions accept lvalues, as suggested by Benjamin.


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


Repository: mesos


Description
---

This patch takes the advantages of the moving constructors of the CSI
request protobuf messages to avoid copying the messages.


Diffs (updated)
-

  src/csi/client.hpp 1c57ac52ade6295d5c45ac8a439ea572d16d7ae3 
  src/csi/client.cpp 923ee6f85dc8070d60bee9834846f859883c73df 
  src/resource_provider/storage/provider.cpp 
36e9109d8cdc7fc1fa29582e2717b84e3e60 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

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

[Andrew Schwartzmeyer]

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




3rdparty/libprocess/src/libwinio_impl.cpp
Lines 53-59 (patched)


Should we `s/CALLBACK/__stdcall` so that this is _slightly_ more clear for 
non-Windows readers (and maybe add a brief comment explaining why this is 
necessary for Windows, e.g. that the Win32 APIs use a different calling 
convention than C++).

Also, we definitely should add a comment explaining that this is a 
TimerAPCProc callback function, and is used for SetWaitableTimer (with links to 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686786(v=vs.85).aspx 
and 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686289(v=vs.85).aspx).



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


s/io/IO



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 91-119 (patched)


Would this be cleaner with a templated struct?

```
template 
struct OverlappedIO
{
  Promise* promise;
  OVERLAPPED overlapped;
  HANDLE handle;
};

struct OverlappedReadWrite : OverlappedIO {};

struct OverlappedAccept : OverlappedIO
{
  unsigned char buf[sizeof(SOCKADDR_STORAGE) + 16];
};
```

Since they _all_ hold a `Promise promise`.



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


I'd double check that `stringify(dword)` gets the error message; I think 
you probably want `+ Error(error).message`



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 137-138 (patched)


Wait, how? `OVERLAPPED` is a Windows type, and `IOOverlappedBase` is a type 
we defined... which has an `OVERLAPPED` field. I'm either missing something 
here or we're doing something weird...



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 161-170 (patched)


```
std::unique_ptr> promise(io_read.promise);
```

Default deleter uses `delete` operator, so then you'd have free RAII and no 
`delete promise;` in each of these cases.



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


Since we do nothing after the switch, opt for `return` over `break` to 
lessen cognitive overhead.



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 219-220 (patched)


Could we just `return false`?



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


Could we just `return true`?



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


Could we just `return true`?



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


And then should this be `UNREACHABLE();` or `true`? That is, can 
`entry->lpCompletionKey` be a value not covered in the switch? Current code 
leaves this as an open question...



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


Should we really wait infinitely? Surely there should be a timeout at some 
point.



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


Is there ever a case where, at this point, `loop == false`? I'm wondering 
why it's `loop = loop && continue_loop` instead of `loop = continue_loop`.



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


s/unamed/unnamed



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 349-350 (patched)


Eh, even for Win32 APIs, use `nullptr`.



3rdparty/libprocess/src/libwinio_impl.cpp
Lines 354-356 (patched)


LOL



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


Everywhere else we were using `HANDLE`; and here we switched to `int_fd`; 
I'm guessing this is a more "public" API?

(Scrolls way up...)

Yup. Okay, maybe a comment :D



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


use or capture?



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


I don't think you need to explicitly cast it, but you can leave it if 

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

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On June 7, 2018, 9:59 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67465/
> ---
> 
> (Updated June 7, 2018, 9:59 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/2/
> 
> 
> Testing
> ---
> 
> Still building, but this is what I propose. This is better than subtle sad 
> panda bugs.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67388: Moved: `io::internal::read/write` to separate file.

[Andrew Schwartzmeyer]

> On June 4, 2018, 4: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/ )
> > |-- ...
> > ```

+1


- Andrew


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


On May 30, 2018, 11:55 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67388/
> ---
> 
> (Updated May 30, 2018, 11:55 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
> ---
> 
> The implementation in `io::internal::read/write` uses `io::poll`, which
> is UNIX specific. The Windows IOCP implementation will not use a
> polling function, since there no unified mechanism, so the functions
> have been moved to their own file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am d434001fbc49d337b6e29f6ac8c9c7475922a819 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 
>   3rdparty/libprocess/src/io_internal.hpp PRE-CREATION 
>   3rdparty/libprocess/src/poll_io.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67388/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67387: Updated Mesos code to use `io::prepare_async`.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On May 30, 2018, 11:56 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67387/
> ---
> 
> (Updated May 30, 2018, 11:56 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
> ---
> 
> Due to the previous patch, instances that use `os::nonblock` before
> sending the file descriptor to the async IO functions now need to use
> `io::prepare_async`.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 8d896ed59e88de86149e00ba80806b61aba15c86 
>   src/slave/container_loggers/logrotate.cpp 
> bd41912fdf9faa70957a768ea5541e60824864ba 
> 
> 
> Diff: https://reviews.apache.org/r/67387/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67386: Added `io::prepare_async` and `io::is_async` functions for libprocess.

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/io.hpp
Lines 49 (patched)


s/UNIX/POSIX



3rdparty/libprocess/include/process/io.hpp
Lines 58-60 (patched)


Maybe expand that `asynchronous` on POSIX means non-blocking and on Windows 
means overlapped and associated with an IOCP.



3rdparty/libprocess/include/process/io.hpp
Lines 60-91 (original), 78-111 (patched)


Just to note: this has semantically been true all along, it's not a new 
requirement we're adding to libprocess.



3rdparty/libprocess/include/process/io.hpp
Line 88 (original), 107 (patched)


Missing a space before the first backtick



3rdparty/libprocess/src/io.cpp
Lines 136-147 (patched)


Just to clarify: the reason we're not using the `os::` functions directly 
is that for Windows IOCP this step requires access to information (the handle 
which is the IO Completion Port) only available at the level of libprocess, not 
stout, right? We should probably document this so this makes sense to 
non-Windows people.


- Andrew Schwartzmeyer


On May 30, 2018, 11:56 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67386/
> ---
> 
> (Updated May 30, 2018, 11:56 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
> ---
> 
> `os::nonblock` and `os::isNonBlock` are stubs that only do things for
> Windows sockets. For the IOCP implementation, we need to associate the
> handles with the IOCP hande, which is created by libprocess. Thus, we
> need these new functions to prepare the handle for the async IO
> functions by either setting the hande to non-blocking on UNIX systems
> or by associating it with an IOCP handle on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/io.hpp 
> cc2caf44e065bed40263f3820e95a4f7c378bb98 
>   3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 
>   3rdparty/libprocess/src/socket.cpp 504cb541785650d2d05aabd25f5258b9bad52baa 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 568d77b94b3afef8c1ebdde71e14aa0b498da5c8 
> 
> 
> Diff: https://reviews.apache.org/r/67386/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67384: Windows: Made socket `int_fd` castable to `HANDLE` type.

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/fd.hpp
Line 151 (original), 151 (patched)


This makes me want to mark these conversion ops as explicit even more...



3rdparty/stout/include/stout/os/windows/fd.hpp
Line 153 (original), 153-155 (patched)


We should at least leave a NOTE as to why this is valid and necessary.


- Andrew Schwartzmeyer


On May 30, 2018, 11:39 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67384/
> ---
> 
> (Updated May 30, 2018, 11:39 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
> ---
> 
> Many Win32 functions that take in `HANDLE` also work on `SOCKET`, such
> as `ReadFile` or `CreateIoCompletionPort`. IOCP on Windows uses the
> `HANDLE` type uniformly, so we need to be able to convert socket
> `int_fd` to `HANDLE`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> 5dbdff2680370d123579c5e3fdd9b0e0adaf512e 
> 
> 
> Diff: https://reviews.apache.org/r/67384/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67358: Added a test `CgroupsIsolatorTest.ROOT_CGROUPS_PERF_AutoLoadSubsystems`.

[Gilbert Song]

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




src/tests/containerizer/cgroups_isolator_tests.cpp
Lines 1801 (patched)


I think we dont need the _PERF_ if we do the change in previous patch


- Gilbert Song


On May 30, 2018, 6:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67358/
> ---
> 
> (Updated May 30, 2018, 6:10 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-7691
> https://issues.apache.org/jira/browse/MESOS-7691
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `CgroupsIsolatorTest.ROOT_CGROUPS_PERF_AutoLoadSubsystems`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 231e9588c0d831c05a1d84f35f0f68105900789c 
> 
> 
> Diff: https://reviews.apache.org/r/67358/diff/2/
> 
> 
> Testing
> ---
> 
> Ran this test repeatedly
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67343: Automatically loaded all the local enabled cgroups subsystems.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Lines 132-134 (patched)


Instead of returning an error; as we discussed, let's return an empty 
subsystem


- Gilbert Song


On May 31, 2018, 8:21 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67343/
> ---
> 
> (Updated May 31, 2018, 8:21 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-7691
> https://issues.apache.org/jira/browse/MESOS-7691
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When `cgroups/all` is specified in the agent flag `--isolation`, we
> will automatically load all the local enabled cgroups subsystems in
> the cgroups isolator with one exception: the `perf_event` subsystem,
> we will only automatically load it when the agent flag `--perf_events`
> is specified, otherwise it will be skipped.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 17a1a3762b2012ff875e4da9c9d4622514e48051 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 215e32461e851668247f9fae62aa656f5dd5e245 
> 
> 
> Diff: https://reviews.apache.org/r/67343/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67335: Added `windows/cpu` and `windows/mem` isolators into `agent.md`.

[Andrew Schwartzmeyer]

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



I tried to apply it to ship it and get it out of my inbox ;) but:

```
> ./support/python3/apply-reviews.py -r 67335
2018-06-08 10:25:02 URL:https://reviews.apache.org/r/67335/diff/raw/ 
[1167/1167] -> "67335.patch" [1]
error: patch failed: docs/configuration/agent.md:967
error: docs/configuration/agent.md: patch does not apply
```

Looks like enough was added to the file that it needs a rebase :/

- Andrew Schwartzmeyer


On May 28, 2018, 7:59 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67335/
> ---
> 
> (Updated May 28, 2018, 7:59 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `windows/cpu` and `windows/mem` isolators into `agent.md`.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md af0c0405da54bad748dcf4de1af0d1cad2dd4661 
> 
> 
> Diff: https://reviews.apache.org/r/67335/diff/1/
> 
> 
> Testing
> ---
> 
> Not a code change.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67505: Refactored verify-reviews.py to use commons.py and argparse

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67502, 67503, 67504, 67505]

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 8, 2018, 12:07 p.m., Dragos Schebesch wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67505/
> ---
> 
> (Updated June 8, 2018, 12:07 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored verify-reviews.py to use commons.py and argparse
> 
> 
> Diffs
> -
> 
>   support/python3/verify-reviews.py 2e925908ffb59dbcdfe99691c5bdbc36a3b7d855 
> 
> 
> Diff: https://reviews.apache.org/r/67505/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dragos Schebesch
> 
>

Re: Review Request 67505: Refactored verify-reviews.py to use commons.py and argparse

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67505 was successfully built and tested.

Reviews applied: `['67502', '67503', '67504', '67505']`

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

- Mesos Reviewbot Windows


On June 8, 2018, 12:07 p.m., Dragos Schebesch wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67505/
> ---
> 
> (Updated June 8, 2018, 12:07 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored verify-reviews.py to use commons.py and argparse
> 
> 
> Diffs
> -
> 
>   support/python3/verify-reviews.py 2e925908ffb59dbcdfe99691c5bdbc36a3b7d855 
> 
> 
> Diff: https://reviews.apache.org/r/67505/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dragos Schebesch
> 
>

Re: Review Request 67501: Added authorization for resource provider operations.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67501]

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 8, 2018, 4:20 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> ---
> 
> (Updated June 8, 2018, 4:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-7329
> https://issues.apache.org/jira/browse/MESOS-7329
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Framework operations `CREATE_VOLUME`, `DESTROY_VOLUME`, `CREATE_BLOCK`,
> `DESTROY_BLOCK` are authorized. Respective ACL actions have been added
> to the local authorizer. Currently access can only be given to either
> 'ANY' or 'NONE' resource providers.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
>   docs/csi.md 7c38fc10633aa28d012606150099ab5cc4b60cb6 
>   include/mesos/authorizer/acls.proto 
> e4889939481dabe6c1c2876a54d654f98d00dec8 
>   include/mesos/authorizer/authorizer.proto 
> bb1010d7eb97de17807b0a730ce16a4b28bc2aa3 
>   src/authorizer/local/authorizer.cpp 
> 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
>   src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c 
>   src/master/master.cpp 5db5a8da85f02323a5654c93ac47ec4aa7e711d2 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67501: Added authorization for resource provider operations.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67501 was successfully built and tested.

Reviews applied: `['67501']`

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

- Mesos Reviewbot Windows


On June 8, 2018, 10:50 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67501/
> ---
> 
> (Updated June 8, 2018, 10:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-7329
> https://issues.apache.org/jira/browse/MESOS-7329
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Framework operations `CREATE_VOLUME`, `DESTROY_VOLUME`, `CREATE_BLOCK`,
> `DESTROY_BLOCK` are authorized. Respective ACL actions have been added
> to the local authorizer. Currently access can only be given to either
> 'ANY' or 'NONE' resource providers.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
>   docs/csi.md 7c38fc10633aa28d012606150099ab5cc4b60cb6 
>   include/mesos/authorizer/acls.proto 
> e4889939481dabe6c1c2876a54d654f98d00dec8 
>   include/mesos/authorizer/authorizer.proto 
> bb1010d7eb97de17807b0a730ce16a4b28bc2aa3 
>   src/authorizer/local/authorizer.cpp 
> 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
>   src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c 
>   src/master/master.cpp 5db5a8da85f02323a5654c93ac47ec4aa7e711d2 
>   src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 
> 
> 
> Diff: https://reviews.apache.org/r/67501/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 67504: Added helper for posting the result of a build

[Dragos Schebesch via Review Board]

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
---

Added helper for posting the result of a build


Diffs (updated)
-

  support/python3/post-build-result.py PRE-CREATION 


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


Testing
---


Thanks,

Dragos Schebesch

Review Request 67505: Refactored verify-reviews.py to use commons.py and argparse

[Dragos Schebesch via Review Board]

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
---

Refactored verify-reviews.py to use commons.py and argparse


Diffs
-

  support/python3/verify-reviews.py 2e925908ffb59dbcdfe99691c5bdbc36a3b7d855 


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


Testing
---


Thanks,

Dragos Schebesch

Review Request 67502: Refactored API functionality into separate module

[Dragos Schebesch via Review Board]

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
---

Refactored API functionality into separate module


Diffs (updated)
-

  support/python3/common.py PRE-CREATION 


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


Testing
---


Thanks,

Dragos Schebesch

Review Request 67503: Added helper for fetching review id

[Dragos Schebesch via Review Board]

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
---

Added helper for fetching review id


Diffs (updated)
-

  support/python3/get-review-ids.py PRE-CREATION 


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


Testing
---


Thanks,

Dragos Schebesch

Review Request 67501: Added authorization for resource provider operations.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Framework operations `CREATE_VOLUME`, `DESTROY_VOLUME`, `CREATE_BLOCK`,
`DESTROY_BLOCK` are authorized. Respective ACL actions have been added
to the local authorizer. Currently access can only be given to either
'ANY' or 'NONE' resource providers.


Diffs
-

  docs/authorization.md cd8622b9848b7a020c079cc1901e3933fa6eb0c0 
  docs/csi.md 7c38fc10633aa28d012606150099ab5cc4b60cb6 
  include/mesos/authorizer/acls.proto e4889939481dabe6c1c2876a54d654f98d00dec8 
  include/mesos/authorizer/authorizer.proto 
bb1010d7eb97de17807b0a730ce16a4b28bc2aa3 
  src/authorizer/local/authorizer.cpp 61e9ab5ce9f1ce4eee4a3f8502c9b60140efcb7e 
  src/master/master.hpp 4180341e2c7b16503a4376c501f611bb78ba901c 
  src/master/master.cpp 5db5a8da85f02323a5654c93ac47ec4aa7e711d2 
  src/tests/authorization_tests.cpp f6f77692112d2299f3009fde4468f82bfd934c60 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 67157: Refactored the gRPC client runtime wrapper in libprocess.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On June 8, 2018, 3:21 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67157/
> ---
> 
> (Updated June 8, 2018, 3:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-8924
> https://issues.apache.org/jira/browse/MESOS-8924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The refactoring does the following things:
> 1. Manage the gRPC completion queue and the looper thread in the runtime
>process to get rid of a lock in `Runtime::Data`.
> 2. Move the computation of sending a request into the runtime process.
> 3. Let libprocess manage the runtime process automatically instead of
>managing its lifecycle in `Runtime::Data`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> cf165f0ca1fe3f4abea9117024ac97c25f6f31dc 
>   3rdparty/libprocess/src/grpc.cpp a80bcb614ec96d92d21bc88a281d3208e86141a0 
> 
> 
> Diff: https://reviews.apache.org/r/67157/diff/5/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67223: Used move constructors for making CSI gRPC calls.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67164, 67190, 67191, 67154, 67155, 67156, 67158, 67157, 67223]

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 8, 2018, 1:31 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67223/
> ---
> 
> (Updated June 8, 2018, 1:31 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8924
> https://issues.apache.org/jira/browse/MESOS-8924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch takes the advantages of the moving constructors of the CSI
> request protobuf messages to avoid copying the messages.
> 
> 
> Diffs
> -
> 
>   src/csi/client.hpp 1c57ac52ade6295d5c45ac8a439ea572d16d7ae3 
>   src/csi/client.cpp 923ee6f85dc8070d60bee9834846f859883c73df 
>   src/resource_provider/storage/provider.cpp 
> 36e9109d8cdc7fc1fa29582e2717b84e3e60 
> 
> 
> Diff: https://reviews.apache.org/r/67223/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>