Review Request 58200: Fix mesos runs with docker(pid namespace mismatch).

[Deshi Xiao]

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

Review request for mesos, Alexander Rukletsov and haosdent huang.


Bugs: mesos-7210
https://issues.apache.org/jira/browse/mesos-7210


Repository: mesos


Description
---

Becuase MESOS HTTP checks doesn't work when mesos runs with
--docker_mesos_image ( pid namespace mismatch ).So let docker
executor run with container add host pid mapping(--pid=host)


Diffs
-

  src/slave/containerizer/docker.cpp ad9ab847cb3093724ef374d036c896b4e7f18b5e 


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


Testing
---


Thanks,

Deshi Xiao

Re: Review Request 57818: Added unit tests to verify offers are suppressed based on registration.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57815, 57816, 57817, 57818]

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 April 4, 2017, 10:37 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57818/
> ---
> 
> (Updated April 4, 2017, 10:37 p.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit tests to verify offers are suppressed based on registration.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp d1828eb42e0aedc9330c3786bbd9bb63aa42a64e 
>   src/tests/scheduler_tests.cpp 0f5d9ada6eb880379baf5f106fd2d5b12e9738db 
> 
> 
> Diff: https://reviews.apache.org/r/57818/diff/3/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.

[Jay Guo]

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

(Updated April 5, 2017, 10:24 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael 
Park.


Changes
---

add `MESOS-4732`. add mcypark as reviewer.


Bugs: MESOS-4732 and MESOS-7260
https://issues.apache.org/jira/browse/MESOS-4732
https://issues.apache.org/jira/browse/MESOS-7260


Repository: mesos


Description
---

Instead of generating JSON object, `Master::Http::roles()` now
leverages `jsonify` to compute output. Also approver is taken
out from its continuation function. This is for easier and cleaner
implementation of framework authorization in /roles endpoint,
see MESOS-7260.


Diffs
-

  src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff 
  src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab 


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


Testing
---

no functional changes
make check


Thanks,

Jay Guo

Re: Review Request 58196: Implemented TCP check support in command and default executors.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58190, 58191, 58192, 58193, 58194, 58195, 58196]

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 April 4, 2017, 10:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> ---
> 
> (Updated April 4, 2017, 10:25 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
> https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58058: Moved libprocess initialization of worker threads later.

[Greg Mann]

> On April 4, 2017, 9:58 p.m., Greg Mann wrote:
> > IIUC, previously-queued events can execute in a later instance of 
> > libprocess because they persist in the file-scope `functions` queue that we 
> > declare in the event loop implementations? So this patch still allows 
> > previously-queued events to execute, but ensures that any GC-managed 
> > processes they spawn will be GC'd correctly?
> > 
> > If so, perhaps we could clear the `functions` queue during 
> > `EventLoop::stop()` instead, to ensure that no previously-queued events are 
> > executed at all. WDYT?
> 
> Joseph Wu wrote:
> Unfortunately, the `functions` object we keep in the thread's memory does 
> not actually include all the callbacks.  It basically only includes functions 
> we add via `run_in_event_loop`.  Other libevent-APIs will end up putting 
> lambdas in libevent's own storage (such as, for server sockets, 
> `evconnlistener_new`).
> 
> So clearing `functions` doesn't help for the segfaults in this chain :(
> 
> Greg Mann wrote:
> True, but once we call `EventLoop::stop()`, libev/libevent will no longer 
> queue up new callbacks on the event loop, right? So if, during finalization, 
> we clear the `functions` queue and wait for all pending event loop callbacks 
> to execute (which should be the case by the time the event loop thread 
> joins), then I think we could be assured a clean finalization with no pending 
> callbacks that would execute when libprocess is initialized again?

Regarding `evconnlistener_new`, as long as the destructors for all SSL sockets 
have executed, then all of their listeners should have been freed? And with 
your synchronous socket destruction change, we could be assured of that.


- Greg


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


On March 30, 2017, 1:20 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58058/
> ---
> 
> (Updated March 30, 2017, 1:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-6919
> https://issues.apache.org/jira/browse/MESOS-6919
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit moves the creation of all libprocess worker threads
> after the creation of the garbage collector process.
> 
> This deals with a test-only case where:
>   1) Events are queued on the event loop.
>   2) Libprocess is finalized as part of the test,
>  before processing all events.
>   3) Libprocess is reinitialized and the previously queued events
>  are allowed to resume.
> 
> Because the events were queued in a previous incarnation of
> libprocess, they potentially bypass the synchronization variables
> in `process::initialize` (i.e. `initialize_complete`) and can
> spawn garbage-collected processes before the garbage collector
> has been spawned.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> f6ee24e2db43d63d91222549efee85421bbf9bf3 
> 
> 
> Diff: https://reviews.apache.org/r/58058/diff/1/
> 
> 
> Testing
> ---
> 
> See end of chain
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58058: Moved libprocess initialization of worker threads later.

[Greg Mann]

> On April 4, 2017, 9:58 p.m., Greg Mann wrote:
> > IIUC, previously-queued events can execute in a later instance of 
> > libprocess because they persist in the file-scope `functions` queue that we 
> > declare in the event loop implementations? So this patch still allows 
> > previously-queued events to execute, but ensures that any GC-managed 
> > processes they spawn will be GC'd correctly?
> > 
> > If so, perhaps we could clear the `functions` queue during 
> > `EventLoop::stop()` instead, to ensure that no previously-queued events are 
> > executed at all. WDYT?
> 
> Joseph Wu wrote:
> Unfortunately, the `functions` object we keep in the thread's memory does 
> not actually include all the callbacks.  It basically only includes functions 
> we add via `run_in_event_loop`.  Other libevent-APIs will end up putting 
> lambdas in libevent's own storage (such as, for server sockets, 
> `evconnlistener_new`).
> 
> So clearing `functions` doesn't help for the segfaults in this chain :(

True, but once we call `EventLoop::stop()`, libev/libevent will no longer queue 
up new callbacks on the event loop, right? So if, during finalization, we clear 
the `functions` queue and wait for all pending event loop callbacks to execute 
(which should be the case by the time the event loop thread joins), then I 
think we could be assured a clean finalization with no pending callbacks that 
would execute when libprocess is initialized again?


- Greg


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


On March 30, 2017, 1:20 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58058/
> ---
> 
> (Updated March 30, 2017, 1:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-6919
> https://issues.apache.org/jira/browse/MESOS-6919
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit moves the creation of all libprocess worker threads
> after the creation of the garbage collector process.
> 
> This deals with a test-only case where:
>   1) Events are queued on the event loop.
>   2) Libprocess is finalized as part of the test,
>  before processing all events.
>   3) Libprocess is reinitialized and the previously queued events
>  are allowed to resume.
> 
> Because the events were queued in a previous incarnation of
> libprocess, they potentially bypass the synchronization variables
> in `process::initialize` (i.e. `initialize_complete`) and can
> spawn garbage-collected processes before the garbage collector
> has been spawned.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> f6ee24e2db43d63d91222549efee85421bbf9bf3 
> 
> 
> Diff: https://reviews.apache.org/r/58058/diff/1/
> 
> 
> Testing
> ---
> 
> See end of chain
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58059: Changed libprocess SocketManager to refer to HttpProxy by PID.

[Greg Mann]

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



Have you encountered a deadlock at all when running the test in 
https://reviews.apache.org/r/58056/ ? When running with libev, I see a deadlock 
after several hundred repetitions. It looks like `ProcessManager::use()` is 
being called on an `HttpProxy` through the `StremaingResponseDecoder` as part 
of a callback on the event loop, while `ProcessManager::cleanup()` is being 
called on the `HttpProcess` from the test.

One thread is blocked on this line:
https://github.com/apache/mesos/blob/7f04cf886fc2ed59414bf0056a2f351959a2d1f8/3rdparty/libprocess/src/process.cpp#L2800
while the other is stuck here:
https://github.com/apache/mesos/blob/7f04cf886fc2ed59414bf0056a2f351959a2d1f8/3rdparty/libprocess/src/process.cpp#L3231

It's not clear to me why the process references remain >0 for the `HttpProcess`.

- Greg Mann


On March 30, 2017, 1:20 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58059/
> ---
> 
> (Updated March 30, 2017, 1:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-6919
> https://issues.apache.org/jira/browse/MESOS-6919
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> HttpProxy actors are spawned to manage incoming HTTP connections.
> These actors are themselves garbage collected, meaning it is unsafe
> to refer to an HttpProxy by pointer (which is what is currently done).
> 
> During libprocess finalization, it is possible for an incoming
> connection to spawn an HttpProxy, whose pointer is then deleted by
> finalization.  This leads to a potential segfault when cleaning up
> the incoming connection, as the SocketManager will dereference
> any related HttpProxy actors by pointer.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> f6ee24e2db43d63d91222549efee85421bbf9bf3 
> 
> 
> Diff: https://reviews.apache.org/r/58059/diff/1/
> 
> 
> Testing
> ---
> 
> With the additional test here: https://reviews.apache.org/r/58056/
> 
> make check
> 
> 3rdparty/libprocess/src/tests/libprocess-tests 
> --gtest_filter="*RapidReconnect*" --gtest_repeat=1000
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58058: Moved libprocess initialization of worker threads later.

[Joseph Wu]

> On April 4, 2017, 2:58 p.m., Greg Mann wrote:
> > IIUC, previously-queued events can execute in a later instance of 
> > libprocess because they persist in the file-scope `functions` queue that we 
> > declare in the event loop implementations? So this patch still allows 
> > previously-queued events to execute, but ensures that any GC-managed 
> > processes they spawn will be GC'd correctly?
> > 
> > If so, perhaps we could clear the `functions` queue during 
> > `EventLoop::stop()` instead, to ensure that no previously-queued events are 
> > executed at all. WDYT?

Unfortunately, the `functions` object we keep in the thread's memory does not 
actually include all the callbacks.  It basically only includes functions we 
add via `run_in_event_loop`.  Other libevent-APIs will end up putting lambdas 
in libevent's own storage (such as, for server sockets, `evconnlistener_new`).

So clearing `functions` doesn't help for the segfaults in this chain :(


- Joseph


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


On March 29, 2017, 6:20 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58058/
> ---
> 
> (Updated March 29, 2017, 6:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-6919
> https://issues.apache.org/jira/browse/MESOS-6919
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit moves the creation of all libprocess worker threads
> after the creation of the garbage collector process.
> 
> This deals with a test-only case where:
>   1) Events are queued on the event loop.
>   2) Libprocess is finalized as part of the test,
>  before processing all events.
>   3) Libprocess is reinitialized and the previously queued events
>  are allowed to resume.
> 
> Because the events were queued in a previous incarnation of
> libprocess, they potentially bypass the synchronization variables
> in `process::initialize` (i.e. `initialize_complete`) and can
> spawn garbage-collected processes before the garbage collector
> has been spawned.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> f6ee24e2db43d63d91222549efee85421bbf9bf3 
> 
> 
> Diff: https://reviews.apache.org/r/58058/diff/1/
> 
> 
> Testing
> ---
> 
> See end of chain
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58186: Stop logging the HTTP user agent.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58185, 58186]

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 April 4, 2017, 8:15 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58186/
> ---
> 
> (Updated April 4, 2017, 8:15 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-7340
> https://issues.apache.org/jira/browse/MESOS-7340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The HTTP user agent can be quite noisy in log files, but is not
> particularly helpful.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 89133e0175ffb792ae95705315844a32550470a1 
> 
> 
> Diff: https://reviews.apache.org/r/58186/diff/1/
> 
> 
> Testing
> ---
> 
> Make check (Fedora 25). Manual inspection of log output.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58125: Stout: Added stringify for std::wstring.

[Joseph Wu]

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


Ship it!





3rdparty/stout/include/stout/stringify.hpp
Lines 47-48 (patched)


As Benjamin suggested, I'll make this into an override (rather than a 
template specialization) before committing so that we can take a `const &` 
instead of a `const`.


- Joseph Wu


On March 31, 2017, 9:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58125/
> ---
> 
> (Updated March 31, 2017, 9:32 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5418
> https://issues.apache.org/jira/browse/MESOS-5418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Added stringify for std::wstring.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/stringify.hpp 
> e9588d8d940046791794100c53469288656a14f0 
> 
> 
> Diff: https://reviews.apache.org/r/58125/diff/2/
> 
> 
> Testing
> ---
> 
> Testing done later in chain.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58160: Stout: Removed `TRUE_COMMAND`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 3, 2017, 2:21 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58160/
> ---
> 
> (Updated April 3, 2017, 2:21 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This macro was not working because they are semantically different:
> a single binary versus a binary with arguments.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/gtest.hpp 
> 763fd7b06b240c4a2e1eeb6a283eb192e5a88df9 
> 
> 
> Diff: https://reviews.apache.org/r/58160/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58127: Windows: Rewrote subprocess helpers.

[Joseph Wu]

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



- Joseph Wu


On March 31, 2017, 4:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58127/
> ---
> 
> (Updated March 31, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5418
> https://issues.apache.org/jira/browse/MESOS-5418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Explicitly use `CreateProcessW` for proper Unicode support, and fix
> `getSystemEnvironment` and `createProcessEnvironment` to support UTF-16.
> Note that `wstring` has to be used over `u16string` due to an MSVC bug,
> see MESOS-7335.
> 
> Most importantly, fix the following incorrect escaping algorithm:
> 
> std::string command = strings::join(" ", argv);
> 
> By replacing it with the rewritten `stringify_args` from `shell.hpp`.
> This resolves problems using "advanced" arguments with `subprocess`:
> 
> .\mesos-containerizer.exe launch --help=false \
> --launch_info={"command":{"shell":true,"value":"ping 127.0.0.1 -n 1"}}
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/windows/subprocess.hpp 
> 1d93b08b035a5eaf677ead3356d0b4be808c39cc 
> 
> 
> Diff: https://reviews.apache.org/r/58127/diff/1/
> 
> 
> Testing
> ---
> 
> Testing done later in chain.
> 
> Note that this fixes the majority of the test failures originally reported in 
> #57976.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58128: Windows: Updated use of UTF-16 `getSystemEnvironment`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 3, 2017, 10:54 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58128/
> ---
> 
> (Updated April 3, 2017, 10:54 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5418
> https://issues.apache.org/jira/browse/MESOS-5418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The containerizer needs to convert UTF-16 values and keys to UTF-8, but
> this is trivial with the updated `stringify`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp ad9ab847cb3093724ef374d036c896b4e7f18b5e 
>   src/slave/containerizer/mesos/launch.cpp 
> 395394f04982a7df58e32e9aeebb63756e85b89b 
> 
> 
> Diff: https://reviews.apache.org/r/58128/diff/1/
> 
> 
> Testing
> ---
> 
> # Linux
> 
> ```
> [==] 1531 tests from 173 test cases ran. (448806 ms total)
> [  PASSED  ] 1530 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] LdcacheTest.Parse
> ```
> 
> Unrelated failure (fails for me on master too, so just a box issue, not 
> introduced by these changes).
> 
> # Windows
> 
> ## stout-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 230 tests from 39 test cases ran. (4456 ms total)
> [  PASSED  ] 228 tests.
> [  FAILED  ] 2 tests, listed below:
> [  FAILED  ] Base64Test.EncodeURLSafe
> [  FAILED  ] Base64Test.DecodeURLSafe
> ```
> 
> * These are [known 
> failures](https://issues.apache.org/jira/browse/MESOS-7236).
> 
> ## libprocess-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 116 tests from 26 test cases ran. (10889 ms total)
> [  PASSED  ] 116 tests.
> ```
> 
> ## mesos-tests
> 
> ```
> [==] 559 tests from 58 test cases ran. (8886776 ms total)
> [  PASSED  ] 556 tests.
> [  FAILED  ] 3 tests, listed below:
> [  FAILED  ] HealthCheckTest.HealthyTaskNonShell
> [  FAILED  ] CombinedAuthenticatorTest.MultipleAuthenticators
> [  FAILED  ] CopyFetcherPluginTest.FetchExistingFile
> ```
> 
> * The `FetchExistingFile` is a [known 
> failure](https://issues.apache.org/jira/browse/MESOS-7311).
> * The `HealthyTaskNonShell` I'm currently debugging (something seems off 
> about the definition of `TRUE_COMMAND` on Windows and its use in a `NonShell` 
> test).
> * The `MultipleAuthenticators` test fails for me on `master` too (spefically 
> e37442578), so it's probably a machine configuration issue, but was not 
> introduced by these changes.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58161: Fixed HealthyTaskNonShell test on Windows.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 3, 2017, 4:35 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58161/
> ---
> 
> (Updated April 3, 2017, 4:35 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using `TRUE_COMMAND`, we needed to use `add_arguments` for
> each argument to avoid smashing the set of arguments into one string.
> That is, we cannot execute `"cmd /c exit 0"`, we have to execute `"cmd"`
> with arguments `"cmd", "/c", "exit 0"`.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 211f8b8578e811d3f2a229387cc0ce8327ae8cb6 
> 
> 
> Diff: https://reviews.apache.org/r/58161/diff/1/
> 
> 
> Testing
> ---
> 
> ## make check
> 
> `make check` on CentOS passed (excepting `LdcacheTest.Parse` which also fails 
> on `master` so it's machine issue, not caused by these changes).
> 
> ## mesos-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 559 tests from 58 test cases ran. (4051620 ms total)
> [  PASSED  ] 556 tests.
> [  FAILED  ] 3 tests, listed below:
> [  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
> [  FAILED  ] CombinedAuthenticatorTest.MultipleAuthenticators
> [  FAILED  ] CopyFetcherPluginTest.FetchExistingFile
> 
> ...
> 
> [   OK ] CommandExecutorCheckTest.CommandCheckTimeout (15730 ms)
> ```
> 
> Re-ran `CommandCheckTimeout`, failure was a fluke. `MultipleAuthenticators` 
> and `FetchExistingFile` are known issues.
> 
> ## libprocess-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 116 tests from 26 test cases ran. (11408 ms total)
> [  PASSED  ] 116 tests.
> ```
> 
> ## stout-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 230 tests from 39 test cases ran. (4712 ms total)
> [  PASSED  ] 228 tests.
> [  FAILED  ] 2 tests, listed below:
> [  FAILED  ] Base64Test.EncodeURLSafe
> [  FAILED  ] Base64Test.DecodeURLSafe
> ```
> 
> Known failure.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58128: Windows: Updated use of UTF-16 `getSystemEnvironment`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 3, 2017, 10:54 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58128/
> ---
> 
> (Updated April 3, 2017, 10:54 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5418
> https://issues.apache.org/jira/browse/MESOS-5418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The containerizer needs to convert UTF-16 values and keys to UTF-8, but
> this is trivial with the updated `stringify`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp ad9ab847cb3093724ef374d036c896b4e7f18b5e 
>   src/slave/containerizer/mesos/launch.cpp 
> 395394f04982a7df58e32e9aeebb63756e85b89b 
> 
> 
> Diff: https://reviews.apache.org/r/58128/diff/1/
> 
> 
> Testing
> ---
> 
> # Linux
> 
> ```
> [==] 1531 tests from 173 test cases ran. (448806 ms total)
> [  PASSED  ] 1530 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] LdcacheTest.Parse
> ```
> 
> Unrelated failure (fails for me on master too, so just a box issue, not 
> introduced by these changes).
> 
> # Windows
> 
> ## stout-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 230 tests from 39 test cases ran. (4456 ms total)
> [  PASSED  ] 228 tests.
> [  FAILED  ] 2 tests, listed below:
> [  FAILED  ] Base64Test.EncodeURLSafe
> [  FAILED  ] Base64Test.DecodeURLSafe
> ```
> 
> * These are [known 
> failures](https://issues.apache.org/jira/browse/MESOS-7236).
> 
> ## libprocess-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 116 tests from 26 test cases ran. (10889 ms total)
> [  PASSED  ] 116 tests.
> ```
> 
> ## mesos-tests
> 
> ```
> [==] 559 tests from 58 test cases ran. (8886776 ms total)
> [  PASSED  ] 556 tests.
> [  FAILED  ] 3 tests, listed below:
> [  FAILED  ] HealthCheckTest.HealthyTaskNonShell
> [  FAILED  ] CombinedAuthenticatorTest.MultipleAuthenticators
> [  FAILED  ] CopyFetcherPluginTest.FetchExistingFile
> ```
> 
> * The `FetchExistingFile` is a [known 
> failure](https://issues.apache.org/jira/browse/MESOS-7311).
> * The `HealthyTaskNonShell` I'm currently debugging (something seems off 
> about the definition of `TRUE_COMMAND` on Windows and its use in a `NonShell` 
> test).
> * The `MultipleAuthenticators` test fails for me on `master` too (spefically 
> e37442578), so it's probably a machine configuration issue, but was not 
> introduced by these changes.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58160: Stout: Removed `TRUE_COMMAND`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 3, 2017, 2:21 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58160/
> ---
> 
> (Updated April 3, 2017, 2:21 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This macro was not working because they are semantically different:
> a single binary versus a binary with arguments.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/gtest.hpp 
> 763fd7b06b240c4a2e1eeb6a283eb192e5a88df9 
> 
> 
> Diff: https://reviews.apache.org/r/58160/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58161: Fixed HealthyTaskNonShell test on Windows.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 3, 2017, 4:35 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58161/
> ---
> 
> (Updated April 3, 2017, 4:35 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using `TRUE_COMMAND`, we needed to use `add_arguments` for
> each argument to avoid smashing the set of arguments into one string.
> That is, we cannot execute `"cmd /c exit 0"`, we have to execute `"cmd"`
> with arguments `"cmd", "/c", "exit 0"`.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 211f8b8578e811d3f2a229387cc0ce8327ae8cb6 
> 
> 
> Diff: https://reviews.apache.org/r/58161/diff/1/
> 
> 
> Testing
> ---
> 
> ## make check
> 
> `make check` on CentOS passed (excepting `LdcacheTest.Parse` which also fails 
> on `master` so it's machine issue, not caused by these changes).
> 
> ## mesos-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 559 tests from 58 test cases ran. (4051620 ms total)
> [  PASSED  ] 556 tests.
> [  FAILED  ] 3 tests, listed below:
> [  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
> [  FAILED  ] CombinedAuthenticatorTest.MultipleAuthenticators
> [  FAILED  ] CopyFetcherPluginTest.FetchExistingFile
> 
> ...
> 
> [   OK ] CommandExecutorCheckTest.CommandCheckTimeout (15730 ms)
> ```
> 
> Re-ran `CommandCheckTimeout`, failure was a fluke. `MultipleAuthenticators` 
> and `FetchExistingFile` are known issues.
> 
> ## libprocess-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 116 tests from 26 test cases ran. (11408 ms total)
> [  PASSED  ] 116 tests.
> ```
> 
> ## stout-tests
> 
> ```
> [--] Global test environment tear-down
> [==] 230 tests from 39 test cases ran. (4712 ms total)
> [  PASSED  ] 228 tests.
> [  FAILED  ] 2 tests, listed below:
> [  FAILED  ] Base64Test.EncodeURLSafe
> [  FAILED  ] Base64Test.DecodeURLSafe
> ```
> 
> Known failure.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58127: Windows: Rewrote subprocess helpers.

[Joseph Wu]

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


Ship it!




Basically a refactoring.  Looks functionally similar to before.


3rdparty/libprocess/include/process/windows/subprocess.hpp
Lines 173 (patched)


Over 80 characters, needs a newline.



3rdparty/libprocess/include/process/windows/subprocess.hpp
Line 230 (original), 239 (patched)


Slightly over 80 characters



3rdparty/libprocess/include/process/windows/subprocess.hpp
Line 237 (original), 246 (patched)


Slightly over 80 characters.


- Joseph Wu


On March 31, 2017, 4:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58127/
> ---
> 
> (Updated March 31, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, Li Li, and 
> Michael Park.
> 
> 
> Bugs: MESOS-5418
> https://issues.apache.org/jira/browse/MESOS-5418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Explicitly use `CreateProcessW` for proper Unicode support, and fix
> `getSystemEnvironment` and `createProcessEnvironment` to support UTF-16.
> Note that `wstring` has to be used over `u16string` due to an MSVC bug,
> see MESOS-7335.
> 
> Most importantly, fix the following incorrect escaping algorithm:
> 
> std::string command = strings::join(" ", argv);
> 
> By replacing it with the rewritten `stringify_args` from `shell.hpp`.
> This resolves problems using "advanced" arguments with `subprocess`:
> 
> .\mesos-containerizer.exe launch --help=false \
> --launch_info={"command":{"shell":true,"value":"ping 127.0.0.1 -n 1"}}
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/windows/subprocess.hpp 
> 1d93b08b035a5eaf677ead3356d0b4be808c39cc 
> 
> 
> Diff: https://reviews.apache.org/r/58127/diff/1/
> 
> 
> Testing
> ---
> 
> Testing done later in chain.
> 
> Note that this fixes the majority of the test failures originally reported in 
> #57976.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 57818: Added unit tests to verify offers are suppressed based on registration.

[Anindya Sinha]

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

(Updated April 4, 2017, 10:37 p.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added unit tests to verify offers are suppressed based on registration.


Diffs (updated)
-

  src/tests/master_tests.cpp d1828eb42e0aedc9330c3786bbd9bb63aa42a64e 
  src/tests/scheduler_tests.cpp 0f5d9ada6eb880379baf5f106fd2d5b12e9738db 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 57817: Suppress offers for frameworks on registration.

[Anindya Sinha]

> On April 4, 2017, 5:57 p.m., James Peach wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 271 (original), 272 (patched)
> > 
> >
> > Remove double space after `framework`.

There does not seem to be any double space after `framework` since we are 
logging the `frameworkId`.


- Anindya


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


On April 4, 2017, 10:37 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> ---
> 
> (Updated April 4, 2017, 10:37 p.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If requested in SUBSCRIBE api call, offers are suppressed on
> framework registration.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 6eda1b8619269c1501a935045b18b1deaf845b33 
>   src/master/allocator/mesos/allocator.hpp 
> 57b54b86c43c7731e64d422d285c4b8ca7e27a60 
>   src/master/allocator/mesos/hierarchical.hpp 
> f84b0574ce9a392c9528c87b04b01dbb2053cff7 
>   src/master/allocator/mesos/hierarchical.cpp 
> ff9e2340e88f50f02aa8ebfd6b6ce039f347bb5d 
>   src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab 
>   src/master/master.cpp 6a6a570e52d21bfb2443f981e3d7faf8c36f74bc 
>   src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d 
>   src/tests/hierarchical_allocator_tests.cpp 
> e343dc37bd7136f0f6dd5dbc22a25cabe715038d 
>   src/tests/master_allocator_tests.cpp 
> 9f3750215f2b72f6148d0c47cdde6a3f7dfb1b50 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> d5fcdbf7575acf99d5dec5315aee127f14e54e6d 
>   src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
>   src/tests/resource_offers_tests.cpp 
> f0bca1d9e03013ce35215b0ffa6b50b38972dc0c 
>   src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 
> 
> 
> Diff: https://reviews.apache.org/r/57817/diff/3/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 57817: Suppress offers for frameworks on registration.

[Anindya Sinha]

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

(Updated April 4, 2017, 10:37 p.m.)


Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.


Changes
---

Addressed review comment.


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


Repository: mesos


Description
---

If requested in SUBSCRIBE api call, offers are suppressed on
framework registration.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
6eda1b8619269c1501a935045b18b1deaf845b33 
  src/master/allocator/mesos/allocator.hpp 
57b54b86c43c7731e64d422d285c4b8ca7e27a60 
  src/master/allocator/mesos/hierarchical.hpp 
f84b0574ce9a392c9528c87b04b01dbb2053cff7 
  src/master/allocator/mesos/hierarchical.cpp 
ff9e2340e88f50f02aa8ebfd6b6ce039f347bb5d 
  src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab 
  src/master/master.cpp 6a6a570e52d21bfb2443f981e3d7faf8c36f74bc 
  src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d 
  src/tests/hierarchical_allocator_tests.cpp 
e343dc37bd7136f0f6dd5dbc22a25cabe715038d 
  src/tests/master_allocator_tests.cpp 9f3750215f2b72f6148d0c47cdde6a3f7dfb1b50 
  src/tests/persistent_volume_endpoints_tests.cpp 
d5fcdbf7575acf99d5dec5315aee127f14e54e6d 
  src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
  src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c 
  src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 58188: Linked task group doc from home.md.

[Anand Mazumdar]

> On April 4, 2017, 10:09 p.m., Gilbert Song wrote:
> > docs/home.md
> > Lines 39 (patched)
> > 
> >
> > * [Nested Container and Task Group](nested-container-and-task-group.md) 
> > a.k.a. Pods in Mesos.

Renamed it to `Task Group (Pod)` instead of using `aka`. The `in Mesos` suffic 
seemed redundant.

Also, nested containers would be something only a limited subset of the users 
would be interested in and they can get that information from the task group 
doc.


- Anand


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


On April 4, 2017, 10:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58188/
> ---
> 
> (Updated April 4, 2017, 10:06 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Linked task group doc from home.md.
> 
> 
> Diffs
> -
> 
>   docs/home.md 20d136e4b9770db1c873205cb29e90a166a48244 
> 
> 
> Diff: https://reviews.apache.org/r/58188/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that the link opens
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 58196: Implemented TCP check support in command and default executors.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
  src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
  src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
  src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
  src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 


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


Testing
---

make check on Mac OS 10.11.6 and various linux variants.


Thanks,

Alexander Rukletsov

Review Request 58195: Added TCP checks in Mesos API.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

>From now on executors may implement TCP checks for tasks.


Diffs
-

  include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
  include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
  src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
  src/common/type_utils.cpp dc0dd71f52581e2067fed279677bda8c82aa7298 
  src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
  src/launcher/executor.cpp bc69beb884d95d1616b2a3d928cdbf00f70f7c88 
  src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
  src/v1/mesos.cpp 5605ff22da77724a7947637bc17e12143ee34802 


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


Testing
---

See https://reviews.apache.org/r/58196/


Thanks,

Alexander Rukletsov

Review Request 58194: Hardened HTTP check tests.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

Introduce 1s delay to ensure the task (HTTP server) has enough time
to start and start serving request.


Diffs
-

  src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 


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


Testing
---

See https://reviews.apache.org/r/58196/


Thanks,

Alexander Rukletsov

Review Request 58192: Used constexpr char instead of static const string for consistency.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 


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


Testing
---

See https://reviews.apache.org/r/58196/


Thanks,

Alexander Rukletsov

Review Request 58193: Renamed variables in checker library for clarity.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 


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


Testing
---

See https://reviews.apache.org/r/58196/


Thanks,

Alexander Rukletsov

Review Request 58191: Renamed a test helper for clarity.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

`HealthCheckTestHelper` is actually a simple libprocess-based HTTP
server. To make it clear and enable it usage in non health check
related tests, rename it to `HttpServerTestHelper`.


Diffs
-

  src/Makefile.am 071656ad7354a802e8292140a7181cb70b68fe9e 
  src/tests/CMakeLists.txt 8e368a8c8f83293529e00f9902174bff8e798d51 
  src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
  src/tests/health_check_test_helper.hpp 
cdedf09ebaeb976d33090fe708b0ce5c118a6682 
  src/tests/health_check_test_helper.cpp 
88352c1bfe5d484ec2ba602b4f9a8139c868fef7 
  src/tests/health_check_tests.cpp 211f8b8578e811d3f2a229387cc0ce8327ae8cb6 
  src/tests/test_helper_main.cpp 5d99ede44f958b36d33dede02f2ab7a5854a01f3 


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


Testing
---

See https://reviews.apache.org/r/58196/


Thanks,

Alexander Rukletsov

Review Request 58190: Improved termination logging in default executor.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

WSTRINGIFY already includes a preamble in addition to exit status,
no need to repeat it.


Diffs
-

  src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 


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


Testing
---

See https://reviews.apache.org/r/58196/


Thanks,

Alexander Rukletsov

Re: Review Request 58188: Linked task group doc from home.md.

[Gilbert Song]

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




docs/home.md
Lines 39 (patched)


* [Nested Container and Task Group](nested-container-and-task-group.md) 
a.k.a. Pods in Mesos.


- Gilbert Song


On April 4, 2017, 3:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58188/
> ---
> 
> (Updated April 4, 2017, 3:06 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Linked task group doc from home.md.
> 
> 
> Diffs
> -
> 
>   docs/home.md 20d136e4b9770db1c873205cb29e90a166a48244 
> 
> 
> Diff: https://reviews.apache.org/r/58188/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that the link opens
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 58188: Linked task group doc from home.md.

[Anand Mazumdar]

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

Linked task group doc from home.md.


Diffs
-

  docs/home.md 20d136e4b9770db1c873205cb29e90a166a48244 


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


Testing
---

Verified that the link opens


Thanks,

Anand Mazumdar

Re: Review Request 58058: Moved libprocess initialization of worker threads later.

[Greg Mann]

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



IIUC, previously-queued events can execute in a later instance of libprocess 
because they persist in the file-scope `functions` queue that we declare in the 
event loop implementations? So this patch still allows previously-queued events 
to execute, but ensures that any GC-managed processes they spawn will be GC'd 
correctly?

If so, perhaps we could clear the `functions` queue during `EventLoop::stop()` 
instead, to ensure that no previously-queued events are executed at all. WDYT?

- Greg Mann


On March 30, 2017, 1:20 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58058/
> ---
> 
> (Updated March 30, 2017, 1:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: MESOS-6919
> https://issues.apache.org/jira/browse/MESOS-6919
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit moves the creation of all libprocess worker threads
> after the creation of the garbage collector process.
> 
> This deals with a test-only case where:
>   1) Events are queued on the event loop.
>   2) Libprocess is finalized as part of the test,
>  before processing all events.
>   3) Libprocess is reinitialized and the previously queued events
>  are allowed to resume.
> 
> Because the events were queued in a previous incarnation of
> libprocess, they potentially bypass the synchronization variables
> in `process::initialize` (i.e. `initialize_complete`) and can
> spawn garbage-collected processes before the garbage collector
> has been spawned.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> f6ee24e2db43d63d91222549efee85421bbf9bf3 
> 
> 
> Diff: https://reviews.apache.org/r/58058/diff/1/
> 
> 
> Testing
> ---
> 
> See end of chain
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58095, 58096, 58097, 58099]

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 April 4, 2017, 10:47 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58099/
> ---
> 
> (Updated April 4, 2017, 10:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for frameworks in `GetRoles` v1 API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff 
> 
> 
> Diff: https://reviews.apache.org/r/58099/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 57998: Added ResourceProviderID to Resource protos.

[Jie Yu]

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




src/tests/resources_tests.cpp
Lines 1537 (patched)


Instead of just one type of resources, i'd try to use multiple types of 
resources here (e.g., "disk" and "cpu") so that we exercise the path that two 
different types of the resources might have the same resource provider ID.



src/tests/resources_tests.cpp
Lines 1546 (patched)


I would also check r1's size here.



src/tests/resources_tests.cpp
Lines 1547 (patched)


I'd move this to the dedicated 'contains' test



src/tests/resources_tests.cpp
Lines 1548 (patched)


looks like this check is for shared resources. Can you remove this check 
from this test?



src/tests/resources_tests.cpp
Lines 1559-1560 (patched)


Ditto. `count` sounds very shared resource related.



src/tests/resources_tests.cpp
Lines 1572 (patched)


Ditto. I would prefer we include another type of resources with the same 
provider ID here in this test.



src/tests/resources_tests.cpp
Lines 1577-1578 (patched)


This is contains check, let's factor this out into a separate test.



src/tests/resources_tests.cpp
Lines 1594 (patched)


Let's also add some test around equality (i.e., checking if `==` or `!=` 
works properly or not. For instance, the fact that you don't have a `==` 
defined for unversioned API should be captured here in the unit tests.


- Jie Yu


On March 31, 2017, 9:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57998/
> ---
> 
> (Updated March 31, 2017, 9:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an optional resource provider id to resources. In
> future changes we will introduce abstract providers of resources.
> While currently agents are implicit resource providers, later on an
> agent might use multiple resource providers. By having a provider id
> in the resource we can unambigously detect which provider contributed
> which resource.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/57998/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 58186: Stop logging the HTTP user agent.

[James Peach]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

The HTTP user agent can be quite noisy in log files, but is not
particularly helpful.


Diffs
-

  src/common/http.cpp 89133e0175ffb792ae95705315844a32550470a1 


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


Testing
---

Make check (Fedora 25). Manual inspection of log output.


Thanks,

James Peach

Review Request 58185: Log HTTP accesses to the /files endpoint.

[James Peach]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

Consolidate the master and agent HTTP request log helper functions
into common code. Add HTTP access logging to `FilesProcess` so that the
`/files` endpoint logs consistently.


Diffs
-

  src/common/http.hpp b6e61f7f7f8ebcf5b25a37684cae06cb96188478 
  src/common/http.cpp 89133e0175ffb792ae95705315844a32550470a1 
  src/files/files.cpp f066146b7cbff35c452717d179b79039bc603cc8 
  src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff 
  src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab 
  src/master/master.cpp 6a6a570e52d21bfb2443f981e3d7faf8c36f74bc 
  src/slave/http.cpp e253ce9749fc8a03c21dac1ba0e6efe09311414b 
  src/slave/slave.hpp e4f46d42b3c0d0f09cff2d896abf6b84aed6c396 
  src/slave/slave.cpp 65e4a67888fe908e5b2f6ca2ecc9e3a5b9958b2e 


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


Testing
---

Make check (Fedora 25). Manual inspection of log output.


Thanks,

James Peach

Re: Review Request 57663: Updated documentation for multiple HTTP authenticators.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 23, 2017, 10:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57663/
> ---
> 
> (Updated March 23, 2017, 10:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7004
> https://issues.apache.org/jira/browse/MESOS-7004
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation for multiple HTTP authenticators.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 1574db981d5f8ddd7d1f6bef1c2b032823d17297 
> 
> 
> Diff: https://reviews.apache.org/r/57663/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 57667: Added executor authentication to the docs.

[Vinod Kone]

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




docs/authentication.md
Lines 120 (patched)


Maybe also explicitly note that, command executor and custom executors not 
using HTTP API (i.e., driver based) are still allowed to register without 
authentication?



docs/authentication.md
Lines 145 (patched)


s/command/HTTP command/



docs/authentication.md
Lines 146 (patched)


s/custom/custom HTTP/



docs/authentication.md
Lines 148 (patched)


s/executor/HTTP executor/


- Vinod Kone


On April 4, 2017, 6:57 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57667/
> ---
> 
> (Updated April 4, 2017, 6:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7005
> https://issues.apache.org/jira/browse/MESOS-7005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added executor authentication to the docs.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 1574db981d5f8ddd7d1f6bef1c2b032823d17297 
>   docs/executor-http-api.md d666d3c459540df9490d7f67b0e43323957d5025 
> 
> 
> Diff: https://reviews.apache.org/r/57667/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 57925: Added further tests for executor secret generation.

[Vinod Kone]

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




src/tests/slave_tests.cpp
Lines 5904-5905 (patched)


I'm assuming the tasks also fail even if the secret generation succeeds 
because of the shutdown sent by the framework? If yes, should we test that as 
well or instead?


- Vinod Kone


On March 25, 2017, 6:27 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57925/
> ---
> 
> (Updated March 25, 2017, 6:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds two further tests for executor secret
> generation, `SlaveTest.RunTaskGroupReferenceTypeSecret`
> and `SlaveTest.RunTaskGroupFailedSecretAfterShutdown`.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp c31c670b667240c4876d415aa5cf90cb34273e8a 
> 
> 
> Diff: https://reviews.apache.org/r/57925/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> `bin/mesos-tests.sh --gtest_filter="SlaveTest.*Secret*" --gtest_repeat=-1 
> --gtest_break_on_failure` was used to check the new tests.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 57731: Fixed a test in MasterAuthorizationTest.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On March 17, 2017, 3:55 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57731/
> ---
> 
> (Updated March 17, 2017, 3:55 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Vinod Kone.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - `MasterAuthorizationTest.PendingExecutorInfoDiffersOnDifferentSlaves`
>   used to assume the mock authorizer is only called for tasks
>   authorization but with the new `regsiter_agents` ACL this is no
>   longer true.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> 1a0285a3f345ef21a8256d4123d8bb684f538da4 
> 
> 
> Diff: https://reviews.apache.org/r/57731/diff/4/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57730: Fixed example tests which broke due to the new `register_agents` ACL.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On March 17, 2017, 3:53 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57730/
> ---
> 
> (Updated March 17, 2017, 3:53 p.m.)
> 
> 
> Review request for mesos and Anindya Sinha.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - With the new `register_agents` ACL, `permissive == false` would lead
>   to the agent unable to register unless explicitly allowed.
> 
> 
> Diffs
> -
> 
>   src/tests/script.cpp 3b68b845b06fe19acb8b08e1ff3dd0bec9117f05 
> 
> 
> Diff: https://reviews.apache.org/r/57730/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On March 28, 2017, 8:24 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57534/
> ---
> 
> (Updated March 28, 2017, 8:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Greg 
> Mann.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added and implemented RegisterAgent ACL.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 46e66e8ed42240e5258890cdaf5aebe8800efcf0 
>   include/mesos/authorizer/authorizer.proto 
> 736f76d552956f2351ffd40fc51d088dff83f8c8 
>   src/authorizer/local/authorizer.cpp 
> be8037299601427e5d5e79f58f77eea3f89579d0 
>   src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 
> 
> 
> Diff: https://reviews.apache.org/r/57534/diff/6/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57667: Added executor authentication to the docs.

[Greg Mann]

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

(Updated April 4, 2017, 6:57 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Added executor authentication to the docs.


Diffs
-

  docs/authentication.md 1574db981d5f8ddd7d1f6bef1c2b032823d17297 
  docs/executor-http-api.md d666d3c459540df9490d7f67b0e43323957d5025 


Diff: https://reviews.apache.org/r/57667/diff/7/


Testing
---


Thanks,

Greg Mann

Re: Review Request 58175: Fixed an iterator bug in 'CombinedAuthenticator::authenticate()'.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58174, 58175]

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 April 4, 2017, 2:15 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58175/
> ---
> 
> (Updated April 4, 2017, 2:15 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the `loop()` logic within
> `CombinedAuthenticatorProcess::authenticate()` to fix a bug
> in which an iterator is incremented past the end of its vector.
> 
> 
> Diffs
> -
> 
>   src/authentication/http/combined_authenticator.cpp 
> c734e7672e96087fa715501cfc594a0165f5bce7 
> 
> 
> Diff: https://reviews.apache.org/r/58175/diff/1/
> 
> 
> Testing
> ---
> 
> `make && bin/mesos-tests.sh 
> --gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 57535: Applied RegisterAgent ACL to the master.

[Greg Mann]

> On March 17, 2017, 9:32 p.m., Greg Mann wrote:
> > src/master/master.hpp
> > Lines 684-686 (patched)
> > 
> >
> > Could you leave a TODO here to update this function to use `Principal` 
> > when MESOS-7202 is resolved?
> 
> Jiang Yan Xu wrote:
> Will MESOS-7202 cover the master <-> agent protocol and the 
> `authenticated`?

Hmm perhaps not, since the master <-> agent communication will still be 
authenticated using the PID-based authenticator interface.


- Greg


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


On March 28, 2017, 8:40 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57535/
> ---
> 
> (Updated March 28, 2017, 8:40 a.m.)
> 
> 
> Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, Benjamin 
> Mahler, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Applied RegisterAgent ACL to the master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 
>   src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 
>   src/tests/master_authorization_tests.cpp 
> 1a0285a3f345ef21a8256d4123d8bb684f538da4 
> 
> 
> Diff: https://reviews.apache.org/r/57535/diff/6/
> 
> 
> Testing
> ---
> 
> make check.
> 
> The tests added here cover some basic scenarios, I have more tests but will 
> add them when MESOS-7244 is fixed.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57818: Added unit tests to verify offers are suppressed based on registration.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On April 3, 2017, 10:35 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57818/
> ---
> 
> (Updated April 3, 2017, 10:35 p.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit tests to verify offers are suppressed based on registration.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp d1828eb42e0aedc9330c3786bbd9bb63aa42a64e 
>   src/tests/scheduler_tests.cpp 0f5d9ada6eb880379baf5f106fd2d5b12e9738db 
> 
> 
> Diff: https://reviews.apache.org/r/57818/diff/2/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 57817: Suppress offers for frameworks on registration.

[James Peach]

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


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.hpp
Line 304 (original), 305 (patched)


Undo this change?



src/master/allocator/mesos/hierarchical.cpp
Line 271 (original), 272 (patched)


Remove double space after `framework`.


- James Peach


On April 3, 2017, 10:35 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> ---
> 
> (Updated April 3, 2017, 10:35 p.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If requested in SUBSCRIBE api call, offers are suppressed on
> framework registration.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 6eda1b8619269c1501a935045b18b1deaf845b33 
>   src/master/allocator/mesos/allocator.hpp 
> 57b54b86c43c7731e64d422d285c4b8ca7e27a60 
>   src/master/allocator/mesos/hierarchical.hpp 
> f84b0574ce9a392c9528c87b04b01dbb2053cff7 
>   src/master/allocator/mesos/hierarchical.cpp 
> ff9e2340e88f50f02aa8ebfd6b6ce039f347bb5d 
>   src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab 
>   src/master/master.cpp 6a6a570e52d21bfb2443f981e3d7faf8c36f74bc 
>   src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d 
>   src/tests/hierarchical_allocator_tests.cpp 
> e343dc37bd7136f0f6dd5dbc22a25cabe715038d 
>   src/tests/master_allocator_tests.cpp 
> 9f3750215f2b72f6148d0c47cdde6a3f7dfb1b50 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> d5fcdbf7575acf99d5dec5315aee127f14e54e6d 
>   src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
>   src/tests/resource_offers_tests.cpp 
> f0bca1d9e03013ce35215b0ffa6b50b38972dc0c 
>   src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 
> 
> 
> Diff: https://reviews.apache.org/r/57817/diff/2/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Jay Guo]

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

(Updated April 5, 2017, 1:47 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Added authorization for frameworks in `GetRoles` v1 API.


Diffs (updated)
-

  src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff 


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

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.

[Jay Guo]

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

(Updated April 5, 2017, 1:46 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Added a test to check framework filtering in /roles endpoint.


Diffs (updated)
-

  src/tests/master_authorization_tests.cpp 
1a0285a3f345ef21a8256d4123d8bb684f538da4 


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

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


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.

[Jay Guo]

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

(Updated April 5, 2017, 1:46 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

While /roles displays a list of frameworksIds that register with
a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which
impose a security risk. This patch fixed this issue by taking a
frameworksApprover in `Master::Http::roles()` which is used to
filter framework IDs.


Diffs (updated)
-

  src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff 


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

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


Testing
---

see next patch in the chain.


Thanks,

Jay Guo

Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.

[Jay Guo]

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

(Updated April 5, 2017, 1:46 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Instead of generating JSON object, `Master::Http::roles()` now
leverages `jsonify` to compute output. Also approver is taken
out from its continuation function. This is for easier and cleaner
implementation of framework authorization in /roles endpoint,
see MESOS-7260.


Diffs (updated)
-

  src/master/http.cpp 6cf9d350446d1b2d4a6e67d552217daff32657ff 
  src/master/master.hpp 1b077424373d6e195e4ab29e150dedbc3f3f95ab 


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

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


Testing
---

no functional changes
make check


Thanks,

Jay Guo

Re: Review Request 58175: Fixed an iterator bug in 'CombinedAuthenticator::authenticate()'.

[Andrew Schwartzmeyer]

> On April 4, 2017, 5:42 p.m., Andrew Schwartzmeyer wrote:
> > Ship It!

This patch fixes the `CombinedAuthenticatorTest.MultipleAuthenticators` test 
failure that otherwise reproduces on Windows 10 with the VS 2017 tools.


- Andrew


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


On April 4, 2017, 2:15 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58175/
> ---
> 
> (Updated April 4, 2017, 2:15 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the `loop()` logic within
> `CombinedAuthenticatorProcess::authenticate()` to fix a bug
> in which an iterator is incremented past the end of its vector.
> 
> 
> Diffs
> -
> 
>   src/authentication/http/combined_authenticator.cpp 
> c734e7672e96087fa715501cfc594a0165f5bce7 
> 
> 
> Diff: https://reviews.apache.org/r/58175/diff/1/
> 
> 
> Testing
> ---
> 
> `make && bin/mesos-tests.sh 
> --gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58175: Fixed an iterator bug in 'CombinedAuthenticator::authenticate()'.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 4, 2017, 2:15 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58175/
> ---
> 
> (Updated April 4, 2017, 2:15 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the `loop()` logic within
> `CombinedAuthenticatorProcess::authenticate()` to fix a bug
> in which an iterator is incremented past the end of its vector.
> 
> 
> Diffs
> -
> 
>   src/authentication/http/combined_authenticator.cpp 
> c734e7672e96087fa715501cfc594a0165f5bce7 
> 
> 
> Diff: https://reviews.apache.org/r/58175/diff/1/
> 
> 
> Testing
> ---
> 
> `make && bin/mesos-tests.sh 
> --gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 57816: Add a scheduler flag `offers_suppressed_on_register`.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On April 3, 2017, 10:34 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57816/
> ---
> 
> (Updated April 3, 2017, 10:34 p.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the framework sets this flag to indicate that it wants offers
> suppressed on successful registration to master, we pass this
> information to the master in the `SUBSCRIBE` call.
> 
> 
> Diffs
> -
> 
>   src/sched/flags.hpp d19d20bad7dba48c8792783c73115affa1430cbe 
>   src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 
> 
> 
> Diff: https://reviews.apache.org/r/57816/diff/2/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58175: Fixed an iterator bug in 'CombinedAuthenticator::authenticate()'.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On April 4, 2017, 2:15 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58175/
> ---
> 
> (Updated April 4, 2017, 2:15 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the `loop()` logic within
> `CombinedAuthenticatorProcess::authenticate()` to fix a bug
> in which an iterator is incremented past the end of its vector.
> 
> 
> Diffs
> -
> 
>   src/authentication/http/combined_authenticator.cpp 
> c734e7672e96087fa715501cfc594a0165f5bce7 
> 
> 
> Diff: https://reviews.apache.org/r/58175/diff/1/
> 
> 
> Testing
> ---
> 
> `make && bin/mesos-tests.sh 
> --gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58174: Fixed the signatures of some test helpers.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On April 4, 2017, 2:11 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58174/
> ---
> 
> (Updated April 4, 2017, 2:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes some test helpers for
> 'CombinedAuthenticatorTest.MultipleAuthenticators' to
> accept their parameters as const ref.
> 
> 
> Diffs
> -
> 
>   src/tests/http_authentication_tests.cpp 
> 36d2b73fa3d0f65aed5e37fe661c93dd46132f82 
> 
> 
> Diff: https://reviews.apache.org/r/58174/diff/1/
> 
> 
> Testing
> ---
> 
> `make && bin/mesos-tests.sh 
> --gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58173: Fixed 'CombinedAuthenticator' to avoid an extra copy.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On April 4, 2017, 2:11 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58173/
> ---
> 
> (Updated April 4, 2017, 2:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a missing call to `std::move` in the constructor
> for `CombinedAuthenticatorProcess` to avoid an unnecessary copy.
> 
> 
> Diffs
> -
> 
>   src/authentication/http/combined_authenticator.cpp 
> c734e7672e96087fa715501cfc594a0165f5bce7 
> 
> 
> Diff: https://reviews.apache.org/r/58173/diff/1/
> 
> 
> Testing
> ---
> 
> `make && bin/mesos-tests.sh 
> --gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58173: Fixed 'CombinedAuthenticator' to avoid an extra copy.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58173]

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 April 4, 2017, 2:11 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58173/
> ---
> 
> (Updated April 4, 2017, 2:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a missing call to `std::move` in the constructor
> for `CombinedAuthenticatorProcess` to avoid an unnecessary copy.
> 
> 
> Diffs
> -
> 
>   src/authentication/http/combined_authenticator.cpp 
> c734e7672e96087fa715501cfc594a0165f5bce7 
> 
> 
> Diff: https://reviews.apache.org/r/58173/diff/1/
> 
> 
> Testing
> ---
> 
> `make && bin/mesos-tests.sh 
> --gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 57998: Added ResourceProviderID to Resource protos.

[Jan Schlicht]

> On April 4, 2017, 4:40 p.m., Jan Schlicht wrote:
> > Sorry for not noticing this earlier: Because I've just run into this, could 
> > you also add a `std::hash` specialization for `ResourceProviderID` in 
> > `type_utils.hpp`? This will allow us to use `ResourceProviderID` as key in 
> > a hashmap. But I could also create a follow-up review for that.
> 
> Jan Schlicht wrote:
> There's also `operator==` in `type_utils.hpp` which needs to be 
> implemented.

And other operators as well :). They all make sense to be implemented for 
`ResourceProviderID`.


- Jan


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


On March 31, 2017, 11:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57998/
> ---
> 
> (Updated March 31, 2017, 11:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an optional resource provider id to resources. In
> future changes we will introduce abstract providers of resources.
> While currently agents are implicit resource providers, later on an
> agent might use multiple resource providers. By having a provider id
> in the resource we can unambigously detect which provider contributed
> which resource.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/57998/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 57998: Added ResourceProviderID to Resource protos.

[Jan Schlicht]

> On April 4, 2017, 4:40 p.m., Jan Schlicht wrote:
> > Sorry for not noticing this earlier: Because I've just run into this, could 
> > you also add a `std::hash` specialization for `ResourceProviderID` in 
> > `type_utils.hpp`? This will allow us to use `ResourceProviderID` as key in 
> > a hashmap. But I could also create a follow-up review for that.

There's also `operator==` in `type_utils.hpp` which needs to be implemented.


- Jan


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


On March 31, 2017, 11:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57998/
> ---
> 
> (Updated March 31, 2017, 11:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an optional resource provider id to resources. In
> future changes we will introduce abstract providers of resources.
> While currently agents are implicit resource providers, later on an
> agent might use multiple resource providers. By having a provider id
> in the resource we can unambigously detect which provider contributed
> which resource.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/57998/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 57998: Added ResourceProviderID to Resource protos.

[Jan Schlicht]

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



Sorry for not noticing this earlier: Because I've just run into this, could you 
also add a `std::hash` specialization for `ResourceProviderID` in 
`type_utils.hpp`? This will allow us to use `ResourceProviderID` as key in a 
hashmap. But I could also create a follow-up review for that.

- Jan Schlicht


On March 31, 2017, 11:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57998/
> ---
> 
> (Updated March 31, 2017, 11:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an optional resource provider id to resources. In
> future changes we will introduce abstract providers of resources.
> While currently agents are implicit resource providers, later on an
> agent might use multiple resource providers. By having a provider id
> in the resource we can unambigously detect which provider contributed
> which resource.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/57998/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 58175: Fixed an iterator bug in 'CombinedAuthenticator::authenticate()'.

[Greg Mann]

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

Review request for mesos, Anand Mazumdar, Michael Park, and Vinod Kone.


Repository: mesos


Description
---

This patch updates the `loop()` logic within
`CombinedAuthenticatorProcess::authenticate()` to fix a bug
in which an iterator is incremented past the end of its vector.


Diffs
-

  src/authentication/http/combined_authenticator.cpp 
c734e7672e96087fa715501cfc594a0165f5bce7 


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


Testing
---

`make && bin/mesos-tests.sh 
--gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`


Thanks,

Greg Mann

Review Request 58174: Fixed the signatures of some test helpers.

[Greg Mann]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

This patch fixes some test helpers for
'CombinedAuthenticatorTest.MultipleAuthenticators' to
accept their parameters as const ref.


Diffs
-

  src/tests/http_authentication_tests.cpp 
36d2b73fa3d0f65aed5e37fe661c93dd46132f82 


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


Testing
---

`make && bin/mesos-tests.sh 
--gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`


Thanks,

Greg Mann

Review Request 58173: Fixed 'CombinedAuthenticator' to avoid an extra copy.

[Greg Mann]

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

This patch adds a missing call to `std::move` in the constructor
for `CombinedAuthenticatorProcess` to avoid an unnecessary copy.


Diffs
-

  src/authentication/http/combined_authenticator.cpp 
c734e7672e96087fa715501cfc594a0165f5bce7 


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


Testing
---

`make && bin/mesos-tests.sh 
--gtest_filter="CombinedAuthenticatorTest.MultipleAuthenticators"`


Thanks,

Greg Mann

Re: Review Request 58021: Added storage-related offer operations.

[Jan Schlicht]

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

(Updated April 4, 2017, 2:47 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Made operations use a single resource. This simplifies resource handling, 
bookkeeping and possible user errors.


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


Repository: mesos


Description (updated)
---

Added storage-related offer operations.


Diffs (updated)
-

  include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
  include/mesos/v1/mesos.proto 228623155c7f68c0f24d173aacbc6eb734f1382f 
  src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
  src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
  src/master/master.cpp 6a6a570e52d21bfb2443f981e3d7faf8c36f74bc 
  src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 


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

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 57474: Added test for authorization of hierarchical roles.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [57474, 57473, 57166, 56805, 57165, 57164]

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

- Mesos Reviewbot


On April 4, 2017, 9:22 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57474/
> ---
> 
> (Updated April 4, 2017, 9:22 a.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds tests for each of the actions which support hierarchical roles.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d 
> 
> 
> Diff: https://reviews.apache.org/r/57474/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 57474: Added test for authorization of hierarchical roles.

[Alexander Rojas]

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

(Updated April 4, 2017, 11:22 a.m.)


Review request for mesos, Adam B and Benjamin Bannier.


Changes
---

Rebased.


Repository: mesos


Description
---

Adds tests for each of the actions which support hierarchical roles.


Diffs (updated)
-

  src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d 


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

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


Testing
---

`make check`


Thanks,

Alexander Rojas

Re: Review Request 57473: Added support for authorization of Hierachical roles.

[Alexander Rojas]

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

(Updated April 4, 2017, 11:21 a.m.)


Review request for mesos, Adam B and Benjamin Bannier.


Changes
---

Rebased.


Repository: mesos


Description
---

Adds mechanisms to support authorization of hierarchical roles,
that is, it allows operators to write ACLs of the form `role/%`
which will enforce the rule for any nested role, e.g. `role/a`,
`role/b` and such.


Diffs (updated)
-

  src/authorizer/local/authorizer.cpp e241edf4afa48d35d94d72e8e8690f5bedfc 


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

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


Testing
---

`make check`


Thanks,

Alexander Rojas

Re: Review Request 57998: Added ResourceProviderID to Resource protos.

[Benjamin Bannier]

> On March 30, 2017, 1:33 a.m., Jie Yu wrote:
> > src/tests/resources_tests.cpp
> > Lines 1532 (patched)
> > 
> >
> > Let's split this test into smaller tests each of which test one 
> > functionality. Take a look how we do tests for shared resources.
> > 
> > I'd suggest we use a new test case name for all those tests: 
> > `ResourceProviderIDTest`

I split this into two tests, one for addition and one for subtraction.


> On March 30, 2017, 1:33 a.m., Jie Yu wrote:
> > src/tests/resources_utils.hpp
> > Lines 46 (patched)
> > 
> >
> > I would name it: `getResourcesByProviderID`

I removed this helper for now as the rewritten test does not use it anymore.


- Benjamin


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


On March 31, 2017, 11:38 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57998/
> ---
> 
> (Updated March 31, 2017, 11:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an optional resource provider id to resources. In
> future changes we will introduce abstract providers of resources.
> While currently agents are implicit resource providers, later on an
> agent might use multiple resource providers. By having a provider id
> in the resource we can unambigously detect which provider contributed
> which resource.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 82d020e05b303a8248a90bc482b76b54b335146c 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/tests/resources_tests.cpp 343cab2af225a05e32c5a8bd4a5d9ddfbf76536d 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/57998/diff/3/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>