Re: Review Request 47942: Windows: Added pipe support functions.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 5:56 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Added pipe support functions.


Diffs
-

  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 

Diff: https://reviews.apache.org/r/47942/diff/


Testing
---

OSX: make check
Windows: build/run


Thanks,

Alex Clemmer



Re: Review Request 47943: Stout: Implemented `shell.hpp` on Windows.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 5:40 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Stout: Implemented `shell.hpp` on Windows.


Diffs
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
ea6ae5ab981f422993085e63543d184a8e41524d 

Diff: https://reviews.apache.org/r/47943/diff/


Testing
---

Windows: buil/run


Thanks,

Alex Clemmer



Re: Review Request 47470: Stout: Added `os::temp`.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 5:32 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Stout: Added `os::temp`.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 

Diff: https://reviews.apache.org/r/47470/diff/


Testing
---

osx: make check


Thanks,

Alex Clemmer



Re: Review Request 47576: Agent: Add Windows support to the containerizer.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 5:31 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
MESOS-3681, MESOS-3682, and MESOS-3684
https://issues.apache.org/jira/browse/MESOS-3617
https://issues.apache.org/jira/browse/MESOS-3618
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3622
https://issues.apache.org/jira/browse/MESOS-3623
https://issues.apache.org/jira/browse/MESOS-3624
https://issues.apache.org/jira/browse/MESOS-3681
https://issues.apache.org/jira/browse/MESOS-3682
https://issues.apache.org/jira/browse/MESOS-3684


Repository: mesos


Description
---

Agent: Add Windows support to the containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
  src/slave/containerizer/external_containerizer.cpp 
cf4384cce44172a028c890f52f71ceb8ae109383 
  src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 
  src/slave/containerizer/mesos/containerizer.cpp 
b154587628a5bf4b1366dbd7a281177e6aa6eb57 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 

Diff: https://reviews.apache.org/r/47576/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47991: Passed `Request` object by ptr instead of copying it.

2016-05-28 Thread Vinod Kone

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



don't think i follow this change. can you explain why wrapping it in owned 
performs better?

- Vinod Kone


On May 27, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47991/
> ---
> 
> (Updated May 27, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change passes the `Request` object by wrapping it in
> `Owned` instead of copying it when forwarding it to `defer`.
> Ideally, this should have been a `unique_ptr`, but our
> `defer` callbacks don't currently support moves.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 988fb3b81259e7b0a6a002f5799d667892874538 
> 
> Diff: https://reviews.apache.org/r/47991/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 47943: Stout: Implemented `shell.hpp` on Windows.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 4:47 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Stout: Implemented `shell.hpp` on Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
ea6ae5ab981f422993085e63543d184a8e41524d 

Diff: https://reviews.apache.org/r/47943/diff/


Testing
---

Windows: buil/run


Thanks,

Alex Clemmer



Re: Review Request 47842: Point slave flags at programmatic temp path.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 4:47 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Point slave flags at programmatic temp path.


Diffs (updated)
-

  src/slave/flags.cpp d30f39c216860c23e24d2d4064470c147c2824d2 

Diff: https://reviews.apache.org/r/47842/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47990: Added move semantics to `Pipe::write()`.

2016-05-28 Thread Vinod Kone

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



what's the reason for not having a rvalue ref overload?

- Vinod Kone


On May 27, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47990/
> ---
> 
> (Updated May 27, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of having separate rvalue overload for `write`,
> this change takes the sink argument by value
> and then does a move. This has the additional small
> overhead of performing an extra move.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 10f6fb90e436300bc47f4d5d2ad4beabd19026ba 
>   3rdparty/libprocess/src/http.cpp b7839b1402d0660a2461085f5a1db191296e3308 
> 
> Diff: https://reviews.apache.org/r/47990/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 47576: Agent: Add Windows support to the containerizer.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 3:35 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
MESOS-3681, MESOS-3682, and MESOS-3684
https://issues.apache.org/jira/browse/MESOS-3617
https://issues.apache.org/jira/browse/MESOS-3618
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3622
https://issues.apache.org/jira/browse/MESOS-3623
https://issues.apache.org/jira/browse/MESOS-3624
https://issues.apache.org/jira/browse/MESOS-3681
https://issues.apache.org/jira/browse/MESOS-3682
https://issues.apache.org/jira/browse/MESOS-3684


Repository: mesos


Description
---

Agent: Add Windows support to the containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
  src/slave/containerizer/external_containerizer.cpp 
cf4384cce44172a028c890f52f71ceb8ae109383 
  src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 
  src/slave/containerizer/mesos/containerizer.cpp 
b154587628a5bf4b1366dbd7a281177e6aa6eb57 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 

Diff: https://reviews.apache.org/r/47576/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47989: Added move semantics to `Future::set`.

2016-05-28 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 27, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47989/
> ---
> 
> (Updated May 27, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 455b750430ae04d16f610c24c0ea0feb8a033708 
> 
> Diff: https://reviews.apache.org/r/47989/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 47987: Constrained constructible types constructor for `Result`.

2016-05-28 Thread Vinod Kone

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




3rdparty/stout/include/stout/result.hpp (lines 65 - 67)


what does this do? can you add some more info to the description for 
posterity?


- Vinod Kone


On May 27, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47987/
> ---
> 
> (Updated May 27, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Constrained constructible types constructor for `Result`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/result.hpp 
> 5d93ee03248249117f8ce259ebb1b126803e1a5c 
> 
> Diff: https://reviews.apache.org/r/47987/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 47988: Added move constructor/assignment to `Try`.

2016-05-28 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 27, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47988/
> ---
> 
> (Updated May 27, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added move constructor/assignment to `Try`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/try.hpp 
> 89dedec0c818263f17f220a2e71ed470bf75a42f 
> 
> Diff: https://reviews.apache.org/r/47988/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 47986: Added move constructor/assignment operator to `Result`.

2016-05-28 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 27, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47986/
> ---
> 
> (Updated May 27, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added move constructor/assignment operator to `Result`.
> Note that `Some` still makes a copy and would be fixed in
> a separate patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/result.hpp 
> 5d93ee03248249117f8ce259ebb1b126803e1a5c 
> 
> Diff: https://reviews.apache.org/r/47986/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 47576: Agent: Add Windows support to the containerizer.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:30 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
MESOS-3681, MESOS-3682, and MESOS-3684
https://issues.apache.org/jira/browse/MESOS-3617
https://issues.apache.org/jira/browse/MESOS-3618
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3622
https://issues.apache.org/jira/browse/MESOS-3623
https://issues.apache.org/jira/browse/MESOS-3624
https://issues.apache.org/jira/browse/MESOS-3681
https://issues.apache.org/jira/browse/MESOS-3682
https://issues.apache.org/jira/browse/MESOS-3684


Repository: mesos


Description
---

Agent: Add Windows support to the containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
  src/slave/containerizer/external_containerizer.cpp 
cf4384cce44172a028c890f52f71ceb8ae109383 
  src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 
  src/slave/containerizer/mesos/containerizer.cpp 
b154587628a5bf4b1366dbd7a281177e6aa6eb57 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 

Diff: https://reviews.apache.org/r/47576/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47660: Fixed slave switch user logic in 'getExecutorInfo'.

2016-05-28 Thread Guangya Liu

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




src/slave/slave.cpp (line 3942)


Just FYI, I filed a JIRA https://issues.apache.org/jira/browse/MESOS-5476 
to trace the `switch_user` documentation issue.

If `switch_user` is set as true, it will first try to see if the `task 
command` also has a user, this user value will takes precedence than user 
defined in `FrameworkInfo`.


- Guangya Liu


On 五月 26, 2016, 8:55 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47660/
> ---
> 
> (Updated 五月 26, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, James Peach, Kevin 
> Klues, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed slave switch user logic in 'getExecutorInfo'.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 470b5c82ea6ff01d799b06245609725853300ef1 
> 
> Diff: https://reviews.apache.org/r/47660/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>



Re: Review Request 47469: Agent: Added `launch.cpp` to Windows build.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:16 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


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


Repository: mesos


Description
---

Agent: Added `launch.cpp` to Windows build.


Diffs (updated)
-

  src/CMakeLists.txt 9eabd781763adba39b60526c1b4a7d99b3f1 
  src/slave/containerizer/mesos/launch.cpp 
e22106b014c871e2184a15c2ab154a0674874e47 

Diff: https://reviews.apache.org/r/47469/diff/


Testing
---

OSX: make check
Windows: Build/run


Thanks,

Alex Clemmer



Re: Review Request 47603: Agent:[2/2] Added Windows support for folder `launcher/`.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:15 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Agent:[2/2] Added Windows support for folder `launcher/`.


Diffs (updated)
-

  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
  src/exec/exec.cpp 69a1fb256fe3005e814833ecac5a5a642b585510 
  src/launcher/executor.hpp PRE-CREATION 
  src/launcher/executor.cpp e4c3b75b647b54ffecedfdfa34fb3925f686c0c7 
  src/launcher/posix/executor.hpp PRE-CREATION 
  src/launcher/windows/executor.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/47603/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 48000: Windows MVP.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:14 a.m.)


Review request for mesos.


Repository: mesos


Description
---

Windows MVP.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake d3a5daee3c4c90712e920136bfe76fc9ab59ec45 
  src/CMakeLists.txt 9eabd781763adba39b60526c1b4a7d99b3f1 
  src/health-check/CMakeLists.txt PRE-CREATION 
  src/launcher/CMakeLists.txt PRE-CREATION 
  src/slave/CMakeLists.txt PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake 187b5cb9d281ea6f3f9c7f81ff2ea9280213b146 
  src/tests/cmake/ContainerizerTestsConfigure.cmake PRE-CREATION 
  src/tests/containerizer/CMakeLists.txt PRE-CREATION 

Diff: https://reviews.apache.org/r/48000/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47576: Agent: Add Windows support to the containerizer.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:12 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
MESOS-3681, MESOS-3682, and MESOS-3684
https://issues.apache.org/jira/browse/MESOS-3617
https://issues.apache.org/jira/browse/MESOS-3618
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3622
https://issues.apache.org/jira/browse/MESOS-3623
https://issues.apache.org/jira/browse/MESOS-3624
https://issues.apache.org/jira/browse/MESOS-3681
https://issues.apache.org/jira/browse/MESOS-3682
https://issues.apache.org/jira/browse/MESOS-3684


Repository: mesos


Description
---

Agent: Add Windows support to the containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
  src/slave/containerizer/external_containerizer.cpp 
cf4384cce44172a028c890f52f71ceb8ae109383 
  src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 
  src/slave/containerizer/mesos/containerizer.cpp 
b154587628a5bf4b1366dbd7a281177e6aa6eb57 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 

Diff: https://reviews.apache.org/r/47576/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47942: Windows: Added pipe support functions.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:11 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Added pipe support functions.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 

Diff: https://reviews.apache.org/r/47942/diff/


Testing
---

OSX: make check
Windows: build/run


Thanks,

Alex Clemmer



Re: Review Request 47602: Stout:[1/2] Added Windows support for folder `launcher/`.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:07 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Stout:[1/2] Added Windows support for folder `launcher/`.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows.hpp 
a71e2f4965c982331c2e8fe4be70027b373a6516 

Diff: https://reviews.apache.org/r/47602/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47942: Windows: Added pipe support functions.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 2:06 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Added pipe support functions.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 

Diff: https://reviews.apache.org/r/47942/diff/


Testing
---

OSX: make check
Windows: build/run


Thanks,

Alex Clemmer



Re: Review Request 48015: Implemented v1::agent::Call::GET_FLAGS.

2016-05-28 Thread Vinod Kone

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

(Updated May 29, 2016, 1:39 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


Changes
---

added reviewers.


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


Repository: mesos


Description
---

The logic is same as what we did for the master.


Diffs
-

  src/internal/evolve.hpp 4d1cb7f2a180f4f4a44491393393449d36ed616b 
  src/internal/evolve.cpp e7afcdb629a509e8bb30ba29542f360066eb8ad4 
  src/slave/http.cpp 64a99d539a0957b92d42c25c8dda298199383018 
  src/slave/slave.hpp 78a6ecfe29881f1723d7d603a99962afdd5a81a8 
  src/slave/slave.cpp 97d15da2a20c74d3a502abac68cc7a695e06a189 
  src/tests/api_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/48015/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

2016-05-28 Thread Vinod Kone


> On May 29, 2016, 1:36 a.m., Vinod Kone wrote:
> >

Can you rebased this?


- Vinod


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


On May 18, 2016, 4:13 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46373/
> ---
> 
> (Updated May 18, 2016, 4:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: mesos-5060
> https://issues.apache.org/jira/browse/mesos-5060
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5060]
> The patch did the following changes:
> 1. Fix the length logic in files.cpp.
> 2. Add some tests to test the /files/read.json endponit with
> negative length.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp e4b0ada00aabba6553810391f4015a896f8a69a5 
>   src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 
> 
> Diff: https://reviews.apache.org/r/46373/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> request 'files/read.json' endpoint with negative offset or length argument
> 
> 
> Thanks,
> 
> zhou xing
> 
>



Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

2016-05-28 Thread Vinod Kone

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


Ship it!





src/files/files.cpp (line 374)


Can we fix the pailer to not send -1 length in a follow up patch?


- Vinod Kone


On May 18, 2016, 4:13 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46373/
> ---
> 
> (Updated May 18, 2016, 4:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: mesos-5060
> https://issues.apache.org/jira/browse/mesos-5060
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5060]
> The patch did the following changes:
> 1. Fix the length logic in files.cpp.
> 2. Add some tests to test the /files/read.json endponit with
> negative length.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp e4b0ada00aabba6553810391f4015a896f8a69a5 
>   src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 
> 
> Diff: https://reviews.apache.org/r/46373/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> request 'files/read.json' endpoint with negative offset or length argument
> 
> 
> Thanks,
> 
> zhou xing
> 
>



Re: Review Request 48015: Implemented v1::agent::Call::GET_FLAGS.

2016-05-28 Thread Mesos ReviewBot

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On May 29, 2016, 1:23 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48015/
> ---
> 
> (Updated May 29, 2016, 1:23 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4791
> https://issues.apache.org/jira/browse/MESOS-4791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The logic is same as what we did for the master.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 4d1cb7f2a180f4f4a44491393393449d36ed616b 
>   src/internal/evolve.cpp e7afcdb629a509e8bb30ba29542f360066eb8ad4 
>   src/slave/http.cpp 64a99d539a0957b92d42c25c8dda298199383018 
>   src/slave/slave.hpp 78a6ecfe29881f1723d7d603a99962afdd5a81a8 
>   src/slave/slave.cpp 97d15da2a20c74d3a502abac68cc7a695e06a189 
>   src/tests/api_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Review Request 48015: Implemented v1::agent::Call::GET_FLAGS.

2016-05-28 Thread Vinod Kone

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

Review request for mesos.


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


Repository: mesos


Description
---

The logic is same as what we did for the master.


Diffs
-

  src/internal/evolve.hpp 4d1cb7f2a180f4f4a44491393393449d36ed616b 
  src/internal/evolve.cpp e7afcdb629a509e8bb30ba29542f360066eb8ad4 
  src/slave/http.cpp 64a99d539a0957b92d42c25c8dda298199383018 
  src/slave/slave.hpp 78a6ecfe29881f1723d7d603a99962afdd5a81a8 
  src/slave/slave.cpp 97d15da2a20c74d3a502abac68cc7a695e06a189 
  src/tests/api_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/48015/diff/


Testing
---

make check


Thanks,

Vinod Kone



Review Request 48014: Added the route handler for v1 agent API.

2016-05-28 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


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


Repository: mesos


Description
---

This is just the basic structure for handling the calls. Actual calls
will be implemented later.


Diffs
-

  src/slave/http.cpp 64a99d539a0957b92d42c25c8dda298199383018 
  src/slave/slave.hpp 78a6ecfe29881f1723d7d603a99962afdd5a81a8 
  src/slave/slave.cpp 97d15da2a20c74d3a502abac68cc7a695e06a189 
  src/slave/validation.hpp 070a77115f0d6f22615fc5bd42537e6d1b86f0bf 
  src/slave/validation.cpp dccc788d7ec782b08abe4b58d8b92031939fa2d7 

Diff: https://reviews.apache.org/r/48014/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 48012: Implemented v1::master::Call::GET_FLAGS.

2016-05-28 Thread Vinod Kone

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

(Updated May 29, 2016, 1:19 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


Changes
---

changed variable name.


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


Repository: mesos


Description
---

The idea is to extract JSON::Object representation of flags into a
function that can be used by both the old JSON REST API handler and the
new v1 API handler.


Diffs (updated)
-

  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
  src/internal/evolve.hpp 4d1cb7f2a180f4f4a44491393393449d36ed616b 
  src/internal/evolve.cpp e7afcdb629a509e8bb30ba29542f360066eb8ad4 
  src/master/http.cpp bbbf0a00e486b96e036f59a3107789d0322bc6cd 
  src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
  src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
  src/master/validation.hpp 580f420a72f33290bcbe718bad839084d4209b8c 
  src/master/validation.cpp 982b61da5fe34dbe46190477e75ee7a4e4cdce9c 
  src/tests/api_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/48012/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 48008: Added v1 protos for master and agent APIs.

2016-05-28 Thread Vinod Kone

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

(Updated May 29, 2016, 1:17 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


Changes
---

Deleted agent::Event because it doesn't exist.


Bugs: MESOS-2257 and MESOS-2720
https://issues.apache.org/jira/browse/MESOS-2257
https://issues.apache.org/jira/browse/MESOS-2720


Repository: mesos


Description
---

Also added common protos to v1/mesos.proto.


Diffs (updated)
-

  include/mesos/master/allocator.proto 555f51b621b21b87b8db00b9ea62cd911d229504 
  include/mesos/v1/agent.hpp PRE-CREATION 
  include/mesos/v1/agent.proto PRE-CREATION 
  include/mesos/v1/master.hpp PRE-CREATION 
  include/mesos/v1/master.proto PRE-CREATION 
  include/mesos/v1/mesos.proto af95b5233158c68db356a4c178d9811cf7bf665d 

Diff: https://reviews.apache.org/r/48008/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 47942: Windows: Added pipe support functions.

2016-05-28 Thread Alex Clemmer

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

(Updated May 29, 2016, 12:07 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Added pipe support functions.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 

Diff: https://reviews.apache.org/r/47942/diff/


Testing
---

OSX: make check
Windows: build/run


Thanks,

Alex Clemmer



Re: Review Request 47822: Removed inconsistancy from routing endpoints in agent code.

2016-05-28 Thread Vinod Kone

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


Ship it!




LGTM modulo the comments I made. I'll fix them when committing. Thanks.

- Vinod Kone


On May 25, 2016, 4:27 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47822/
> ---
> 
> (Updated May 25, 2016, 4:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Benjamin Bannier, Greg Mann, Jan Schlicht, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-5293
> https://issues.apache.org/jira/browse/MESOS-5293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master and agent code for routing endpoints are made
> consistent. Now they both capture 'this' variable in the
> lambda calls in routing function arguments. Continuations
> are no more need to be static.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp dd1f509e9464af7d02db0182b0eebd63993dd029 
>   src/slave/slave.hpp 0de6a570e8b4699771048295ec3fcedf84593495 
>   src/slave/slave.cpp 2941cf1b6ea1e4deabfcbbe3f4897c06a28531a5 
> 
> Diff: https://reviews.apache.org/r/47822/diff/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> On ubuntu 16.04:
> sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check
> 
> Manual testing.
> 
> 1. Ran master with:
> 
> 
>sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 
> 
> 2. ACL File:
> 
> 
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "NONE" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   } 
> 
> 
> 3. Ran slave with:
> 
> 
>sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
> --acls=file:///home/abhishek/testAcl
> 
> 
> 4. Ran toy-framework with:
> 
> 
>sudo ./no-executor-framework --master=master@127.0.0.1:5050 
> --command="echo hello"
> 
> 
> 5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
> error 403: Forbidden
> 
> 6. Changed ACL to:
> 
> 
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "ANY" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   }
> 
> 
> 7. Ran slave and framework again.
> 
> 8. Output:
> 
> 
> 
> [{"container_id":"2059c6a5-d168-4db7-912d-1c70b61fdb5d","executor_id":"699","executor_name":"Command
>  Executor (Task: 699) (Command: sh -c 'echo 
> hello')","framework_id":"48e2fe51-afba-44c5-958e-0ad3cee6454b-","source":"699","statistics":{"cpus_limit":0.2,"cpus_system_time_secs":0.01,"cpus_user_time_secs":0.04,"mem_limit_bytes":67108864,"mem_rss_bytes":88477696,"timestamp":1464168040.50791},"status":{}},{"container_id":"a0be0340-ca6b-4fc4-9862-4af143a7961e",...
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 47822: Removed inconsistancy from routing endpoints in agent code.

2016-05-28 Thread Vinod Kone


> On May 25, 2016, 9:45 a.m., Jan Schlicht wrote:
> > src/slave/http.cpp, line 384
> > 
> >
> > s/slaveFlags/slave->flags/, see comment above.

don't need to pass the flags at all since this is now a member function. i'll 
fix this while committing.


> On May 25, 2016, 9:45 a.m., Jan Schlicht wrote:
> > src/slave/http.cpp, line 783
> > 
> >
> > s/localSlave/slave/

no need to pass `slave` at all because this is now a non-static member 
function. i'll fix this while committing.


- Vinod


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


On May 25, 2016, 4:27 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47822/
> ---
> 
> (Updated May 25, 2016, 4:27 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Benjamin Bannier, Greg Mann, Jan Schlicht, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-5293
> https://issues.apache.org/jira/browse/MESOS-5293
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master and agent code for routing endpoints are made
> consistent. Now they both capture 'this' variable in the
> lambda calls in routing function arguments. Continuations
> are no more need to be static.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp dd1f509e9464af7d02db0182b0eebd63993dd029 
>   src/slave/slave.hpp 0de6a570e8b4699771048295ec3fcedf84593495 
>   src/slave/slave.cpp 2941cf1b6ea1e4deabfcbbe3f4897c06a28531a5 
> 
> Diff: https://reviews.apache.org/r/47822/diff/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> On ubuntu 16.04:
> sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check
> 
> Manual testing.
> 
> 1. Ran master with:
> 
> 
>sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 
> 
> 2. ACL File:
> 
> 
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "NONE" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   } 
> 
> 
> 3. Ran slave with:
> 
> 
>sudo ./bin/mesos-slave.sh --master=127.0.0.1:5050 --ip=0.0.0.0 
> --acls=file:///home/abhishek/testAcl
> 
> 
> 4. Ran toy-framework with:
> 
> 
>sudo ./no-executor-framework --master=master@127.0.0.1:5050 
> --command="echo hello"
> 
> 
> 5. Output while hitting "http://127.0.0.1:5051/slave(1)/containers" - HTTP 
> error 403: Forbidden
> 
> 6. Changed ACL to:
> 
> 
>   {
> "get_endpoints": [
>   {
> "principals": { "type": "ANY" },
> "paths": { "values": ["/flags", "/monitor/statistics", "/containers"] 
> }
>   }
> ]
>   }
> 
> 
> 7. Ran slave and framework again.
> 
> 8. Output:
> 
> 
> 
> [{"container_id":"2059c6a5-d168-4db7-912d-1c70b61fdb5d","executor_id":"699","executor_name":"Command
>  Executor (Task: 699) (Command: sh -c 'echo 
> hello')","framework_id":"48e2fe51-afba-44c5-958e-0ad3cee6454b-","source":"699","statistics":{"cpus_limit":0.2,"cpus_system_time_secs":0.01,"cpus_user_time_secs":0.04,"mem_limit_bytes":67108864,"mem_rss_bytes":88477696,"timestamp":1464168040.50791},"status":{}},{"container_id":"a0be0340-ca6b-4fc4-9862-4af143a7961e",...
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>



Re: Review Request 48008: Added v1 protos for master and agent APIs.

2016-05-28 Thread Vinod Kone


> On May 28, 2016, 10:58 p.m., Kevin Klues wrote:
> > include/mesos/v1/agent.hpp, lines 35-36
> > 
> >
> > Why is this wrapped and the one aboove not? They are the same line 
> > length

it's not of the same length! this has Response::Type (longer string) as 
parameter whereas the above one has Call::Type (shorter string) as parameter.


> On May 28, 2016, 10:58 p.m., Kevin Klues wrote:
> > include/mesos/v1/master.hpp, lines 35-36
> > 
> >
> > Same comment as above.

see above.


- Vinod


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


On May 28, 2016, 8:29 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48008/
> ---
> 
> (Updated May 28, 2016, 8:29 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2257 and MESOS-2720
> https://issues.apache.org/jira/browse/MESOS-2257
> https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added common protos to v1/mesos.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.proto 
> 555f51b621b21b87b8db00b9ea62cd911d229504 
>   include/mesos/v1/agent.hpp PRE-CREATION 
>   include/mesos/v1/agent.proto PRE-CREATION 
>   include/mesos/v1/master.hpp PRE-CREATION 
>   include/mesos/v1/master.proto PRE-CREATION 
>   include/mesos/v1/mesos.proto af95b5233158c68db356a4c178d9811cf7bf665d 
> 
> Diff: https://reviews.apache.org/r/48008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 48012: Implemented v1::master::Call::GET_FLAGS.

2016-05-28 Thread Kevin Klues

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



There's alot of code here, but it looks good overall. I'll defer to Anand / 
BenH to give it a more thorough review.

- Kevin Klues


On May 28, 2016, 8:30 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48012/
> ---
> 
> (Updated May 28, 2016, 8:30 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2720
> https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The idea is to extract JSON::Object representation of flags into a
> function that can be used by both the old JSON REST API handler and the
> new v1 API handler.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
>   src/internal/evolve.hpp 4d1cb7f2a180f4f4a44491393393449d36ed616b 
>   src/internal/evolve.cpp e7afcdb629a509e8bb30ba29542f360066eb8ad4 
>   src/master/http.cpp bbbf0a00e486b96e036f59a3107789d0322bc6cd 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/master/validation.hpp 580f420a72f33290bcbe718bad839084d4209b8c 
>   src/master/validation.cpp 982b61da5fe34dbe46190477e75ee7a4e4cdce9c 
>   src/tests/api_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48012/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 48011: Added overload for `http::OK()` that takes ContentType.

2016-05-28 Thread Kevin Klues

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




3rdparty/libprocess/include/process/http.hpp (lines 524 - 526)


I would probably move this up above the one before it, to group 
constructors that have a parameter list from those that don't.


- Kevin Klues


On May 28, 2016, 8:26 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48011/
> ---
> 
> (Updated May 28, 2016, 8:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2720
> https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Needed this for subsequent review.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 10f6fb90e436300bc47f4d5d2ad4beabd19026ba 
> 
> Diff: https://reviews.apache.org/r/48011/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 48010: Updated Makefile.am to properly build the v1 API protos.

2016-05-28 Thread Kevin Klues

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




src/Makefile.am (lines 223 - 226)


Not sure if we were trying to stay alphabetical, but if so, these should be 
rearranged.


- Kevin Klues


On May 28, 2016, 8:25 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48010/
> ---
> 
> (Updated May 28, 2016, 8:25 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2720
> https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> And also header files.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
> 
> Diff: https://reviews.apache.org/r/48010/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 48009: Added v1 protos for maintenance, allocator and quota.

2016-05-28 Thread Kevin Klues

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




include/mesos/v1/master/allocator.proto (line 28)


This TODO should be removed.


- Kevin Klues


On May 28, 2016, 8:24 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48009/
> ---
> 
> (Updated May 28, 2016, 8:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2257 and MESOS-2720
> https://issues.apache.org/jira/browse/MESOS-2257
> https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note that these files are copies of their non-versioned counterparts.
> The only difference is in their package name.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/v1/master/allocator.proto PRE-CREATION 
>   include/mesos/v1/quota/quota.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48009/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 48008: Added v1 protos for master and agent APIs.

2016-05-28 Thread Kevin Klues

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




include/mesos/v1/agent.hpp (lines 35 - 36)


Why is this wrapped and the one aboove not? They are the same line length



include/mesos/v1/master.hpp (lines 35 - 36)


Same comment as above.


- Kevin Klues


On May 28, 2016, 8:29 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48008/
> ---
> 
> (Updated May 28, 2016, 8:29 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2257 and MESOS-2720
> https://issues.apache.org/jira/browse/MESOS-2257
> https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added common protos to v1/mesos.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.proto 
> 555f51b621b21b87b8db00b9ea62cd911d229504 
>   include/mesos/v1/agent.hpp PRE-CREATION 
>   include/mesos/v1/agent.proto PRE-CREATION 
>   include/mesos/v1/master.hpp PRE-CREATION 
>   include/mesos/v1/master.proto PRE-CREATION 
>   include/mesos/v1/mesos.proto af95b5233158c68db356a4c178d9811cf7bf665d 
> 
> Diff: https://reviews.apache.org/r/48008/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 47943: Stout: Implemented `shell.hpp` on Windows.

2016-05-28 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [47943, 47942, 47411, 47410, 47409, 47404, 47403, 47391, 
47390, 47389, 47388, 47387, 47386, 47169, 47168, 41632, 47054, 47221, 47053, 
47052]

Failed command: ./support/apply-review.sh -n -r 47943

Error:
2016-05-28 22:56:48 URL:https://reviews.apache.org/r/47943/diff/raw/ 
[4248/4248] -> "47943.patch" [1]
error: patch failed: 3rdparty/stout/include/stout/os/windows/shell.hpp:94
error: 3rdparty/stout/include/stout/os/windows/shell.hpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13396/console

- Mesos ReviewBot


On May 28, 2016, 10:06 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47943/
> ---
> 
> (Updated May 28, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `shell.hpp` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> ea6ae5ab981f422993085e63543d184a8e41524d 
> 
> Diff: https://reviews.apache.org/r/47943/diff/
> 
> 
> Testing
> ---
> 
> Windows: buil/run
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 47602: Stout:[1/2] Added Windows support for folder `launcher/`.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 10:43 p.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Stout:[1/2] Added Windows support for folder `launcher/`.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows.hpp 
a71e2f4965c982331c2e8fe4be70027b373a6516 

Diff: https://reviews.apache.org/r/47602/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47576: Agent: Add Windows support to the containerizer.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 10:28 p.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
MESOS-3681, MESOS-3682, and MESOS-3684
https://issues.apache.org/jira/browse/MESOS-3617
https://issues.apache.org/jira/browse/MESOS-3618
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3622
https://issues.apache.org/jira/browse/MESOS-3623
https://issues.apache.org/jira/browse/MESOS-3624
https://issues.apache.org/jira/browse/MESOS-3681
https://issues.apache.org/jira/browse/MESOS-3682
https://issues.apache.org/jira/browse/MESOS-3684


Repository: mesos


Description
---

Agent: Add Windows support to the containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
  src/slave/containerizer/external_containerizer.cpp 
cf4384cce44172a028c890f52f71ceb8ae109383 
  src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 
  src/slave/containerizer/mesos/containerizer.cpp 
b154587628a5bf4b1366dbd7a281177e6aa6eb57 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 

Diff: https://reviews.apache.org/r/47576/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47576: Agent: Add Windows support to the containerizer.

2016-05-28 Thread Alex Clemmer


> On May 24, 2016, 11:37 p.m., Joris Van Remoortere wrote:
> > src/slave/containerizer/mesos/launcher.hpp, lines 118-119
> > 
> >
> > Is this the only difference?
> > It would be great if we could clearly identify the differences, and 
> > reference a JIRA here to keep track of them.
> 
> Alex Clemmer wrote:
> I'm not sure how to capture this. There are too many differences to name.

I will drop this comment. Feel free to re-open if is not right.


- Alex


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


On May 28, 2016, 3:17 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47576/
> ---
> 
> (Updated May 28, 2016, 3:17 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
> MESOS-3681, MESOS-3682, and MESOS-3684
> https://issues.apache.org/jira/browse/MESOS-3617
> https://issues.apache.org/jira/browse/MESOS-3618
> https://issues.apache.org/jira/browse/MESOS-3619
> https://issues.apache.org/jira/browse/MESOS-3622
> https://issues.apache.org/jira/browse/MESOS-3623
> https://issues.apache.org/jira/browse/MESOS-3624
> https://issues.apache.org/jira/browse/MESOS-3681
> https://issues.apache.org/jira/browse/MESOS-3682
> https://issues.apache.org/jira/browse/MESOS-3684
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent: Add Windows support to the containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
>   src/slave/containerizer/external_containerizer.cpp 
> cf4384cce44172a028c890f52f71ceb8ae109383 
>   src/slave/containerizer/fetcher.cpp 
> 176d8863d1becd8864218a0012ab45c614f0ad77 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b154587628a5bf4b1366dbd7a281177e6aa6eb57 
>   src/slave/containerizer/mesos/launcher.hpp 
> 5977c30c0aacc569019f7b34bb0c6577823ec887 
>   src/slave/containerizer/mesos/launcher.cpp 
> a5c8c31b72773d0bd10b9d02675a01f1d641d41c 
> 
> Diff: https://reviews.apache.org/r/47576/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 47943: Stout: Implemented `shell.hpp` on Windows.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 10:06 p.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Stout: Implemented `shell.hpp` on Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
ea6ae5ab981f422993085e63543d184a8e41524d 

Diff: https://reviews.apache.org/r/47943/diff/


Testing
---

Windows: buil/run


Thanks,

Alex Clemmer



Re: Review Request 48012: Implemented v1::master::Call::GET_FLAGS.

2016-05-28 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [48008, 48009, 48010, 48011, 48012]

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

- Mesos ReviewBot


On May 28, 2016, 8:30 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48012/
> ---
> 
> (Updated May 28, 2016, 8:30 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.
> 
> 
> Bugs: MESOS-2720
> https://issues.apache.org/jira/browse/MESOS-2720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The idea is to extract JSON::Object representation of flags into a
> function that can be used by both the old JSON REST API handler and the
> new v1 API handler.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
>   src/internal/evolve.hpp 4d1cb7f2a180f4f4a44491393393449d36ed616b 
>   src/internal/evolve.cpp e7afcdb629a509e8bb30ba29542f360066eb8ad4 
>   src/master/http.cpp bbbf0a00e486b96e036f59a3107789d0322bc6cd 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/master/validation.hpp 580f420a72f33290bcbe718bad839084d4209b8c 
>   src/master/validation.cpp 982b61da5fe34dbe46190477e75ee7a4e4cdce9c 
>   src/tests/api_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/48012/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 47942: Windows: Added pipe support functions.

2016-05-28 Thread Alex Clemmer

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




src/slave/containerizer/mesos/posix_pipe.hpp (line 13)


These are used later for interprocess communication. The comments should be 
self explanatory. On Posix most operatrions are noop.


- Alex Clemmer


On May 28, 2016, 9:39 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47942/
> ---
> 
> (Updated May 28, 2016, 9:39 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added pipe support functions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/pipe.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/posix_pipe.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/windows_pipe.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47942/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build/run
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 47942: Windows: Added pipe support functions.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 9:39 p.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

Remased, added comments.


Repository: mesos


Description
---

Windows: Added pipe support functions.


Diffs (updated)
-

  src/slave/containerizer/mesos/pipe.hpp PRE-CREATION 
  src/slave/containerizer/mesos/posix_pipe.hpp PRE-CREATION 
  src/slave/containerizer/mesos/windows_pipe.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/47942/diff/


Testing
---

OSX: make check
Windows: build/run


Thanks,

Alex Clemmer



Re: Review Request 47736: Used TaskObjectAllower to filter /tasks endpoint.

2016-05-28 Thread Joerg Schad

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

(Updated May 28, 2016, 9:32 p.m.)


Review request for mesos, Adam B and Michael Park.


Repository: mesos


Description
---

Used TaskObjectAllower to filter /tasks endpoint.


Diffs (updated)
-

  src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 

Diff: https://reviews.apache.org/r/47736/diff/


Testing
---

make check (OSX)


Thanks,

Joerg Schad



Review Request 48012: Implemented v1::master::Call::GET_FLAGS.

2016-05-28 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


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


Repository: mesos


Description
---

The idea is to extract JSON::Object representation of flags into a
function that can be used by both the old JSON REST API handler and the
new v1 API handler.


Diffs
-

  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
  src/internal/evolve.hpp 4d1cb7f2a180f4f4a44491393393449d36ed616b 
  src/internal/evolve.cpp e7afcdb629a509e8bb30ba29542f360066eb8ad4 
  src/master/http.cpp bbbf0a00e486b96e036f59a3107789d0322bc6cd 
  src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
  src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
  src/master/validation.hpp 580f420a72f33290bcbe718bad839084d4209b8c 
  src/master/validation.cpp 982b61da5fe34dbe46190477e75ee7a4e4cdce9c 
  src/tests/api_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/48012/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 48008: Added v1 protos for master and agent APIs.

2016-05-28 Thread Vinod Kone

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

(Updated May 28, 2016, 8:29 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


Changes
---

fixed missing braces.


Bugs: MESOS-2257 and MESOS-2720
https://issues.apache.org/jira/browse/MESOS-2257
https://issues.apache.org/jira/browse/MESOS-2720


Repository: mesos


Description
---

Also added common protos to v1/mesos.proto.


Diffs (updated)
-

  include/mesos/master/allocator.proto 555f51b621b21b87b8db00b9ea62cd911d229504 
  include/mesos/v1/agent.hpp PRE-CREATION 
  include/mesos/v1/agent.proto PRE-CREATION 
  include/mesos/v1/master.hpp PRE-CREATION 
  include/mesos/v1/master.proto PRE-CREATION 
  include/mesos/v1/mesos.proto af95b5233158c68db356a4c178d9811cf7bf665d 

Diff: https://reviews.apache.org/r/48008/diff/


Testing
---

make check


Thanks,

Vinod Kone



Review Request 48011: Added overload for `http::OK()` that takes ContentType.

2016-05-28 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


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


Repository: mesos


Description
---

Needed this for subsequent review.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
10f6fb90e436300bc47f4d5d2ad4beabd19026ba 

Diff: https://reviews.apache.org/r/48011/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 48002: Windows: Undefined macros.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 8:24 p.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Summary (updated)
-

Windows: Undefined macros.


Repository: mesos


Description (updated)
---

Windows: Undefined macros.


Diffs (updated)
-

  src/slave/slave.hpp f48e4b6bbb87d1b8b03176f68fe8d5ea6c109652 

Diff: https://reviews.apache.org/r/48002/diff/


Testing
---


Thanks,

Alex Clemmer



Review Request 48010: Updated Makefile.am to properly build the v1 API protos.

2016-05-28 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


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


Repository: mesos


Description
---

And also header files.


Diffs
-

  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 

Diff: https://reviews.apache.org/r/48010/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 48000: Windows MVP.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 8:24 p.m.)


Review request for mesos.


Repository: mesos


Description
---

Windows MVP.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake d3a5daee3c4c90712e920136bfe76fc9ab59ec45 
  src/CMakeLists.txt 9eabd781763adba39b60526c1b4a7d99b3f1 
  src/health-check/CMakeLists.txt PRE-CREATION 
  src/launcher/CMakeLists.txt PRE-CREATION 
  src/slave/CMakeLists.txt PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake 187b5cb9d281ea6f3f9c7f81ff2ea9280213b146 
  src/tests/cmake/ContainerizerTestsConfigure.cmake PRE-CREATION 
  src/tests/containerizer/CMakeLists.txt PRE-CREATION 

Diff: https://reviews.apache.org/r/48000/diff/


Testing
---


Thanks,

Alex Clemmer



Review Request 48009: Added v1 protos for maintenance, allocator and quota.

2016-05-28 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


Bugs: MESOS-2257 and MESOS-2720
https://issues.apache.org/jira/browse/MESOS-2257
https://issues.apache.org/jira/browse/MESOS-2720


Repository: mesos


Description
---

Note that these files are copies of their non-versioned counterparts.
The only difference is in their package name.


Diffs
-

  include/mesos/v1/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/v1/master/allocator.proto PRE-CREATION 
  include/mesos/v1/quota/quota.proto PRE-CREATION 

Diff: https://reviews.apache.org/r/48009/diff/


Testing
---

make check


Thanks,

Vinod Kone



Review Request 48008: Added v1 protos for master and agent APIs.

2016-05-28 Thread Vinod Kone

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Kevin Klues.


Bugs: MESOS-2257 and MESOS-2720
https://issues.apache.org/jira/browse/MESOS-2257
https://issues.apache.org/jira/browse/MESOS-2720


Repository: mesos


Description
---

Also added common protos to v1/mesos.proto.


Diffs
-

  include/mesos/master/allocator.proto 555f51b621b21b87b8db00b9ea62cd911d229504 
  include/mesos/v1/agent.hpp PRE-CREATION 
  include/mesos/v1/agent.proto PRE-CREATION 
  include/mesos/v1/master.hpp PRE-CREATION 
  include/mesos/v1/master.proto PRE-CREATION 
  include/mesos/v1/mesos.proto af95b5233158c68db356a4c178d9811cf7bf665d 

Diff: https://reviews.apache.org/r/48008/diff/


Testing
---

make check


Thanks,

Vinod Kone



Re: Review Request 47921: Enabled authorization for Mesos log access.

2016-05-28 Thread Alexander Rojas


> On May 28, 2016, 8:21 a.m., Adam B wrote:
> > src/authorizer/local/authorizer.cpp, lines 519-523
> > 
> >
> > I don't like this. Let's just remove AccessMesosLog.logs() as a field, 
> > and forego "consistency" in favor of a sensible interface. We can always 
> > add a different optional object later, perhaps for log-level or some 
> > unforeseen metadata.
> > Building and testing this change now. Let me know if you object.
> 
> Alexander Rojas wrote:
> The `AccessMesosLog.log` field cannot be removed since it removes the 
> ability to define rules with `ANY` and `NONE`. For example if only _foo_ cann 
> access the logs and nobody else, one would write:
> 
> ```json
> {
>   "permissive": false,
>   "access_mesos_logs" : [
> {
>   "principals" : { "values" : ["foo"] },
>   "logs" : { "type" : "ANY" }
> },
> {
>   "principals" : { "type" : "ANY" },
>   "logs" : { "type" : "NONE" }
> },
>   ]
> }
> 
> ```
> 
> which cannot be expressed with the removal of the field.

Sorry, the example is wrong, permissive in this case is supposed to be true.


- Alexander


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


On May 27, 2016, 8:08 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47921/
> ---
> 
> (Updated May 27, 2016, 8:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-5153
> https://issues.apache.org/jira/browse/MESOS-5153
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Uses the authorization primitives in `mesos::internal::Files` to add
> protection of the Mesos logs on both master and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b05586ae587edbf9330f1d916340447d157ba80e 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/slave/slave.hpp f48e4b6bbb87d1b8b03176f68fe8d5ea6c109652 
>   src/slave/slave.cpp 9fcf334a69ae96ff8180df50aab571fac99b6fad 
> 
> Diff: https://reviews.apache.org/r/47921/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> and the script
> 
> ```bash
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "access_mesos_log" : [
> {
>   "principals" : { "values" : ["foo"] },
>   "logs" : { "type" : "ANY" }
> }
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>   --authenticate_http \
>   --credentials=file:///tmp/credentials.txt \
>   --acls=file:///tmp/acls.json \
>   --log_dir=/tmp/mesos/logs/master &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/agent \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json \
>  --log_dir=/tmp/mesos/logs/agent &
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a foo:bar
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a foo:bar
> 
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a baz:bar
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 47921: Enabled authorization for Mesos log access.

2016-05-28 Thread Alexander Rojas


> On May 28, 2016, 8:21 a.m., Adam B wrote:
> > src/authorizer/local/authorizer.cpp, lines 519-523
> > 
> >
> > I don't like this. Let's just remove AccessMesosLog.logs() as a field, 
> > and forego "consistency" in favor of a sensible interface. We can always 
> > add a different optional object later, perhaps for log-level or some 
> > unforeseen metadata.
> > Building and testing this change now. Let me know if you object.

The `AccessMesosLog.log` field cannot be removed since it removes the ability 
to define rules with `ANY` and `NONE`. For example if only _foo_ cann access 
the logs and nobody else, one would write:

```json
{
  "permissive": false,
  "access_mesos_logs" : [
{
  "principals" : { "values" : ["foo"] },
  "logs" : { "type" : "ANY" }
},
{
  "principals" : { "type" : "ANY" },
  "logs" : { "type" : "NONE" }
},
  ]
}

```

which cannot be expressed with the removal of the field.


- Alexander


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


On May 27, 2016, 8:08 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47921/
> ---
> 
> (Updated May 27, 2016, 8:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-5153
> https://issues.apache.org/jira/browse/MESOS-5153
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Uses the authorization primitives in `mesos::internal::Files` to add
> protection of the Mesos logs on both master and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b05586ae587edbf9330f1d916340447d157ba80e 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/slave/slave.hpp f48e4b6bbb87d1b8b03176f68fe8d5ea6c109652 
>   src/slave/slave.cpp 9fcf334a69ae96ff8180df50aab571fac99b6fad 
> 
> Diff: https://reviews.apache.org/r/47921/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> and the script
> 
> ```bash
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "access_mesos_log" : [
> {
>   "principals" : { "values" : ["foo"] },
>   "logs" : { "type" : "ANY" }
> }
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>   --authenticate_http \
>   --credentials=file:///tmp/credentials.txt \
>   --acls=file:///tmp/acls.json \
>   --log_dir=/tmp/mesos/logs/master &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/agent \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json \
>  --log_dir=/tmp/mesos/logs/agent &
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a foo:bar
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a foo:bar
> 
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a baz:bar
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 47509: Fixed authorization::Request initializings.

2016-05-28 Thread Adam B

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



I think we should allow each action to define whether or not it requires an 
object, since some like ACCESS_MESOS_LOG might actually want to check that 
there is never an object. And then, if the action allows/requires an object, we 
can define an object with no fields set (including the new optional 
FrameworkInfo/ExecutorInfo/TaskInfo fields) as an ANY object.


src/authorizer/local/authorizer.cpp (line 423)


Why must a request have an object field? The new ACCESS_MESOS_LOG action 
doesn't have an object, since there is only one log, and a noop object seems 
worse than not having one.
https://reviews.apache.org/r/47921/



src/tests/authorization_tests.cpp (line 598)


Nice catch


- Adam B


On May 17, 2016, 8:10 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47509/
> ---
> 
> (Updated May 17, 2016, 8:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes a number of a authorization::Request initializings that were
> missing required fields. Also adds a CHECK to the LocalAuthorizer
> helping us to catch and prevent these problems in the future.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/common/http.cpp ad6a4b44af3ec847a3a6c0839a88fba18cda5011 
>   src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
>   src/master/master.cpp 35b428b0f7dee5954514d8860cfc498271ccf267 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
>   src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
> 
> Diff: https://reviews.apache.org/r/47509/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 47505: Updated authorizer.proto Subject, Object and Request.

2016-05-28 Thread Adam B

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



Not sure if I'm supposed to be reviewing this or 
https://reviews.apache.org/r/47509

Also, if you're changing some fields to optional, won't you have to edit the 
code to check for has_subject, has_object?
>From the JIRA, "This will also require updating local authorizer, which should 
>properly handle the situation when these fields are absent. We may also want 
>to notify authors of external authorizers to update their code accordingly."


include/mesos/authorizer/authorizer.proto (line 28)


I'd rather avoid too many required fields.
What if we later want to represent subject as an integer, or a more complex 
message/metadata?



include/mesos/authorizer/authorizer.proto (line 38)


Cannot be required due to the latest update, where we now optionally set 
other fields in Object.


- Adam B


On May 17, 2016, 5:35 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47505/
> ---
> 
> (Updated May 17, 2016, 5:35 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Makes `value` required for clearity, to make sure that we do not
> introduce two ways of expressing `ANY`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
> 
> Diff: https://reviews.apache.org/r/47505/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>



Re: Review Request 47536: Agent: Added Windows isolators.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 7:38 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3619, MESOS-3620 and MESOS-3683
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3620
https://issues.apache.org/jira/browse/MESOS-3683


Repository: mesos


Description
---

Agent: Added Windows isolators.


Diffs (updated)
-

  src/CMakeLists.txt 9eabd781763adba39b60526c1b4a7d99b3f1 
  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
01ab1793d3d5469e6590617326b5e77e56bc7c50 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
cb42d83a779c63c9ae25bbe72292aba3fe4cb8c9 
  src/slave/containerizer/mesos/isolators/filesystem/windows.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/filesystem/windows.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/posix.hpp 
227505a70a440f15e68ac001878bcf25610db45f 
  src/slave/containerizer/mesos/isolators/windows.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
9e803f6ae55f09f7e871c60531acce16ebd5e8fe 
  src/usage/main.cpp ab3bb233caf6ba02af1b306dcf31d2b9ba6e322a 
  src/usage/usage.hpp 2e1996a78c617c2559ef882ebede5c8aab6f899f 
  src/usage/usage.cpp 3b19682e67372b81484eacddbab78c2e5eda3c5b 

Diff: https://reviews.apache.org/r/47536/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47602: Stout:[1/2] Added Windows support for folder `launcher/`.

2016-05-28 Thread Alex Clemmer

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

(Updated May 28, 2016, 7:01 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Stout:[1/2] Added Windows support for folder `launcher/`.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows.hpp 
a71e2f4965c982331c2e8fe4be70027b373a6516 

Diff: https://reviews.apache.org/r/47602/diff/


Testing
---


Thanks,

Alex Clemmer



Re: Review Request 47891: Added RUN_TASK authorization action.

2016-05-28 Thread Adam B

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




include/mesos/authorizer/authorizer.proto (line 49)


This is a dangerous setting and should be removed as soon as we no longer 
need it.
Please add a TODO comment to remove it when we remove the alias.
Please add a top-level comment that actions in this enum should be kept in 
numerical order, to prevent accidental aliasing.



include/mesos/authorizer/authorizer.proto (lines 91 - 92)


"// set. For backwards compatibility with the deprecated alias 
`RUN_TASK_WITH_USER`, the value will also be set to the operating system user."



include/mesos/authorizer/authorizer.proto (line 93)


Put the deprecation TODO immediately above the deprecated field.



include/mesos/authorizer/authorizer.proto (line 94)


Since this is numbered `2`, please put it between 1 and 3.



src/master/master.cpp 


Interesting that the previous logic favored the TaskInfo.command.user over 
the ExecutorInfo.command.user.
I wonder if we should reverse our evaluation ordering in the local 
authorizer to maintain behavior, but I can't imagine a scenario where setting 
both would make a difference.



src/master/master.cpp (line 3036)


FrameworkInfo.user is the wrong user to pass in. It should be the user 
calculated by the code you removed above.


- Adam B


On May 27, 2016, 2:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47891/
> ---
> 
> (Updated May 27, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-5459
> https://issues.apache.org/jira/browse/MESOS-5459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorization requests for RUN_TASK actions can pass `SOME`
> authorization object either in a `FrameworkInfo` holding a user, or a
> `TaskInfo` with optionally a `CommandInfo` which can optionally hold a
> user. If either of these fields is set it will be used as the object;
> otherwise an `ANY` type authorization object will be created.
> 
> `RUN_TASK` aliases `RUN_TASK_WITH_USER` which becomes deprecated with
> 0.29.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47891/diff/
> 
> 
> Testing
> ---
> 
> Tested on a range of Linux configurations on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 47921: Enabled authorization for Mesos log access.

2016-05-28 Thread Adam B

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



The more I see the unused AccessMesosLog.logs(), the less I like it.


src/authorizer/local/authorizer.cpp (lines 518 - 522)


I don't like this. Let's just remove AccessMesosLog.logs() as a field, and 
forego "consistency" in favor of a sensible interface. We can always add a 
different optional object later, perhaps for log-level or some unforeseen 
metadata.
Building and testing this change now. Let me know if you object.


- Adam B


On May 27, 2016, 11:08 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47921/
> ---
> 
> (Updated May 27, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-5153
> https://issues.apache.org/jira/browse/MESOS-5153
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Uses the authorization primitives in `mesos::internal::Files` to add
> protection of the Mesos logs on both master and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b05586ae587edbf9330f1d916340447d157ba80e 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/slave/slave.hpp f48e4b6bbb87d1b8b03176f68fe8d5ea6c109652 
>   src/slave/slave.cpp 9fcf334a69ae96ff8180df50aab571fac99b6fad 
> 
> Diff: https://reviews.apache.org/r/47921/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> and the script
> 
> ```bash
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "access_mesos_log" : [
> {
>   "principals" : { "values" : ["foo"] },
>   "logs" : { "type" : "ANY" }
> }
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>   --authenticate_http \
>   --credentials=file:///tmp/credentials.txt \
>   --acls=file:///tmp/acls.json \
>   --log_dir=/tmp/mesos/logs/master &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/agent \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json \
>  --log_dir=/tmp/mesos/logs/agent &
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a foo:bar
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a foo:bar
> 
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a baz:bar
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 47891: Added RUN_TASK authorization action.

2016-05-28 Thread Adam B


> On May 27, 2016, 1:52 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, line 86
> > 
> >
> > I wonder if we should just alias RUN_TASK to the same enum value as 
> > RUN_TASK_WITH_USER.. There shouldn't be any backwards compatibility issues 
> > since these are only used in-memory, and modules have to recompile anyway.
> 
> Benjamin Bannier wrote:
> Good idea. I changed the proto message definitions to that effect. As a 
> note, it seems we usually don't use aliasing fields as this will be the only 
> enum using proto's `allow_alias` setting.

Saw that. Looks like it was added in protobuf 2.5.0, which we've supported for 
a while. Now our minimum is 2.6.1, so it should compile fine. It's just 
dangerous, especially for out of order enums.


> On May 27, 2016, 1:52 a.m., Adam B wrote:
> > src/tests/authorization_tests.cpp, line 203
> > 
> >
> > But principal "foo" could run as any other user, e.g. "bar", right? 
> > That'd be worth testing.
> 
> Benjamin Bannier wrote:
> Let's not mix this into this patch.

Fair enough.


> On May 27, 2016, 1:52 a.m., Adam B wrote:
> > src/tests/authorization_tests.cpp, line 529
> > 
> >
> > Would be better to test that "bar" cannot run a "user1", since we've 
> > shown previously that somebody else ("foo") can.
> 
> Benjamin Bannier wrote:
> Let's not mix this into this patch.

Fair enough.


- Adam


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


On May 27, 2016, 2:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47891/
> ---
> 
> (Updated May 27, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-5459
> https://issues.apache.org/jira/browse/MESOS-5459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorization requests for RUN_TASK actions can pass `SOME`
> authorization object either in a `FrameworkInfo` holding a user, or a
> `TaskInfo` with optionally a `CommandInfo` which can optionally hold a
> user. If either of these fields is set it will be used as the object;
> otherwise an `ANY` type authorization object will be created.
> 
> `RUN_TASK` aliases `RUN_TASK_WITH_USER` which becomes deprecated with
> 0.29.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47891/diff/
> 
> 
> Testing
> ---
> 
> Tested on a range of Linux configurations on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>