Re: Review Request 57111: Updated the allocator to handle frameworks that change its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57111/#review167928 --- Ship it! Ship It! - Benjamin Mahler On Feb. 27, 2017, 10:20

Re: Review Request 57111: Updated the allocator to handle frameworks that change its roles.

> On March 3, 2017, 5:58 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 429-432 (original), 394-400 (patched) > > > > > > Ditto from last review, could we simplify with a -

Re: Review Request 57111: Updated the allocator to handle frameworks that change its roles.

> On Feb. 27, 2017, 2:30 p.m., Michael Park wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 1069 (patched) > > > > > > This is way more expensive than it needs to be. There's currently not a > >

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57110/ --- (Updated March 3, 2017, 6:08 p.m.) Review request for mesos and Benjamin

Re: Review Request 57112: Updated existing test cases to allow for frameworks to change its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57112/#review167923 --- Fix it, then Ship it! src/tests/master_validation_tests.cpp

Re: Review Request 57282: Sent `UpdateFrameworkMessage` even to disconnected frameworks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57282/#review167921 --- Ship it! In the summary / description: "to frameworks" -> "to

Re: Review Request 57111: Updated the allocator to handle frameworks that change its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57111/#review167882 --- src/master/allocator/mesos/hierarchical.hpp Lines 426-429

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57110/#review167922 --- src/master/master.hpp Lines 1785-1786 (original), 1793-1797

Re: Review Request 56623: Implemented the 'Principal' type in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56623/ --- (Updated March 4, 2017, 1:37 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 57298: Added a new libprocess test for invalid principals.

> On March 4, 2017, 12:35 a.m., Vinod Kone wrote: > > 3rdparty/libprocess/src/tests/http_tests.cpp > > Lines 1873 (patched) > > > > > > should we `delete` this constructor as a way to enforce? ofcourse one > >

Re: Review Request 57298: Added a new libprocess test for invalid principals.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57298/ --- (Updated March 4, 2017, 1:36 a.m.) Review request for mesos, Alexander Rojas,

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

> On March 4, 2017, 12:31 a.m., Vinod Kone wrote: > > src/master/http.cpp > > Lines 482 (patched) > > > > > > Maybe `BadRequest` is better here than `Forbidden`. For example we send > > `BadRequest` below in

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

> On March 4, 2017, 12:31 a.m., Vinod Kone wrote: > > src/master/http.cpp > > Lines 482 (patched) > > > > > > Maybe `BadRequest` is better here than `Forbidden`. For example we send > > `BadRequest` below in

Re: Review Request 57158: Added default parameter value to master validation function.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57158/#review167912 --- Ship it! Ship It! - Vinod Kone On March 3, 2017, 6:52 p.m.,

Re: Review Request 56621: Updated Mesos tests to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56621/#review167911 --- Ship it! Ship It! - Vinod Kone On March 3, 2017, 6:51 p.m.,

Re: Review Request 57298: Added a new libprocess test for invalid principals.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57298/#review167910 --- Ship it! 3rdparty/libprocess/src/tests/http_tests.cpp Lines

Re: Review Request 56624: Updated libprocess tests to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56624/#review167909 --- Ship it! Ship It! - Vinod Kone On Feb. 28, 2017, 6:59 a.m.,

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/#review167908 --- src/master/http.cpp Lines 482 (patched)

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

> On March 2, 2017, 10:41 p.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 1565-1573 (original), 1569-1577 (patched) > > > > > > CHECK(principal->value.isSome()) > > Greg Mann wrote: > In this case,

Re: Review Request 56812: Updated agent handlers to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56812/#review167902 --- Ship it! Ship It! - Vinod Kone On March 3, 2017, 6:43 p.m.,

Re: Review Request 56619: Updated 'Files' handlers to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56619/#review167901 --- Ship it! Ship It! - Vinod Kone On Feb. 28, 2017, 6:32 p.m.,

Re: Review Request 57153: Removed unnecessary 'using' statement in master HTTP code.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57153/#review167898 --- Ship it! Ship It! - Vinod Kone On Feb. 28, 2017, 6:31 p.m.,

Re: Review Request 56623: Implemented the 'Principal' type in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56623/ --- (Updated March 3, 2017, 11:39 p.m.) Review request for mesos, Adam B,

Re: Review Request 56618: Updated common Mesos code to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56618/ --- (Updated March 3, 2017, 11:38 p.m.) Review request for mesos, Adam B,

Re: Review Request 56901: Updated master validation code to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56901/#review167895 --- Ship it! Ship It! - Vinod Kone On March 3, 2017, 6:41 p.m.,

Re: Review Request 57262: CMake: Added support for SSL-enabled builds.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57262/#review167892 --- Patch looks great! Reviews applied: [57260, 57261, 57262]

Re: Review Request 56618: Updated common Mesos code to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56618/#review167891 --- Ship it! src/common/http.hpp Line 134 (original), 134

Re: Review Request 56617: Updated libprocess handlers to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56617/#review167890 --- Ship it! Ship It! - Vinod Kone On Feb. 28, 2017, 6:21 a.m.,

Re: Review Request 56623: Implemented the 'Principal' type in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56623/#review167889 --- Fix it, then Ship it!

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

> On March 2, 2017, 8:26 p.m., Benjamin Mahler wrote: > > src/master/master.hpp > > Lines 2526 (patched) > > > > > > At this point, you have based it on `info` instead of `source`, right? > > Maybe that's a little

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

> On March 2, 2017, 8:26 p.m., Benjamin Mahler wrote: > > src/master/master.hpp > > Lines 2263 (patched) > > > > > > This last sentence seems a bit redundant? Removed with your suggestion above. > On March 2,

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57110/ --- (Updated March 3, 2017, 3:10 p.m.) Review request for mesos and Benjamin

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57110/#review167880 --- src/master/master.hpp Lines 1785-1786 (original), 1793-1794

Re: Review Request 57261: CMake: Added configuration options for SSL builds.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57261/ --- (Updated March 3, 2017, 1:50 p.m.) Review request for mesos, Andrew

Re: Review Request 57262: CMake: Added support for SSL-enabled builds.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57262/ --- (Updated March 3, 2017, 1:50 p.m.) Review request for mesos, Andrew

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57110/ --- (Updated March 3, 2017, 1:47 p.m.) Review request for mesos and Benjamin

Re: Review Request 57282: Sent `UpdateFrameworkMessage` even to disconnected frameworks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57282/ --- (Updated March 3, 2017, 1:47 p.m.) Review request for mesos and Benjamin

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55888/#review167870 --- Patch looks great! Reviews applied: [55887, 55888] Passed

Re: Review Request 57262: WIP: CMake: Added support for SSL-enabled builds.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57262/ --- (Updated March 3, 2017, 1:05 p.m.) Review request for mesos, Andrew

Re: Review Request 55888: Test to ensure non-authorized users cannot launch tasks on agents.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55888/ --- (Updated March 3, 2017, 8:09 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 55887: Check task user before allowing a task to be launched on the agent.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55887/ --- (Updated March 3, 2017, 8:08 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 55887: Check task user before allowing a task to be launched on the agent.

> On March 2, 2017, 9:26 a.m., Jiang Yan Xu wrote: > > src/slave/slave.hpp > > Lines 152-153 (original), 152-153 (patched) > > > > > > If we are keeping the original semantics of `_run`, do we still need to > >

Re: Review Request 56449: Moved health checker closer to container in default executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56449/#review167861 --- No updates to this diff yet? - Vinod Kone On Feb. 28, 2017,

Re: Review Request 56213: Added check tests for command executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review167860 --- LGTM modulo addressing the existing issues.

Re: Review Request 57158: Added default parameter value to master validation function.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57158/ --- (Updated March 3, 2017, 6:52 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56621: Updated Mesos tests to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56621/ --- (Updated March 3, 2017, 6:51 p.m.) Review request for mesos, Adam B, Alexander

Review Request 57298: Added a new libprocess test for invalid principals.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57298/ --- Review request for mesos, Alexander Rojas, Jan Schlicht, and Vinod Kone. Bugs:

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

> On March 2, 2017, 10:41 p.m., Greg Mann wrote: > > src/master/master.hpp > > Line 746 (original), 746 (patched) > > > > > > See if we can typedef this. Sounds like we can't :( > On March 2, 2017, 10:41 p.m.,

Re: Review Request 56813: Updated master handlers to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56813/ --- (Updated March 3, 2017, 6:45 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 56812: Updated agent handlers to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56812/ --- (Updated March 3, 2017, 6:43 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 56901: Updated master validation code to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56901/ --- (Updated March 3, 2017, 6:41 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 56618: Updated common Mesos code to use the 'Principal' type.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56618/ --- (Updated March 3, 2017, 6:39 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

> On Nov. 1, 2016, 10:08 p.m., Jie Yu wrote: > > The agent subsystems is a hack to me. I think we should consider support > > running systemd (or other init system) to manage agent process and put it > > under proper cgroup using the init system, rather than doing it ourself. > > Anindya

Re: Review Request 56623: Implemented the 'Principal' type in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56623/ --- (Updated March 3, 2017, 6:36 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

> On Nov. 2, 2016, 5:08 a.m., Jie Yu wrote: > > The agent subsystems is a hack to me. I think we should consider support > > running systemd (or other init system) to manage agent process and put it > > under proper cgroup using the init system, rather than doing it ourself. > > Anindya Sinha

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

> On Nov. 1, 2016, 10:08 p.m., Jie Yu wrote: > > The agent subsystems is a hack to me. I think we should consider support > > running systemd (or other init system) to manage agent process and put it > > under proper cgroup using the init system, rather than doing it ourself. > > Anindya

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

> On Nov. 2, 2016, 5:08 a.m., Jie Yu wrote: > > The agent subsystems is a hack to me. I think we should consider support > > running systemd (or other init system) to manage agent process and put it > > under proper cgroup using the init system, rather than doing it ourself. > > Anindya Sinha

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

> On Nov. 1, 2016, 10:08 p.m., Jie Yu wrote: > > The agent subsystems is a hack to me. I think we should consider support > > running systemd (or other init system) to manage agent process and put it > > under proper cgroup using the init system, rather than doing it ourself. > > Anindya

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

> On Nov. 2, 2016, 5:08 a.m., Jie Yu wrote: > > The agent subsystems is a hack to me. I think we should consider support > > running systemd (or other init system) to manage agent process and put it > > under proper cgroup using the init system, rather than doing it ourself. > > Anindya Sinha

Re: Review Request 56667: Added support for JSON Web Tokens.

> On March 1, 2017, 8:24 a.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 94-95 (patched) > > > > > > Join on one line: > > > > `} else if ( ... ) {` > > > > Could also use a

Re: Review Request 56667: Added support for JSON Web Tokens.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated March 3, 2017, 3:03 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56666: Added a HMAC SHA256 generator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5/ --- (Updated March 3, 2017, 3:02 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56665: Added a URL-safe base64 implementation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56665/ --- (Updated March 3, 2017, 3 p.m.) Review request for mesos, Alexander Rojas and

Re: Review Request 56665: Added a URL-safe base64 implementation.

> On Feb. 28, 2017, 11:16 p.m., Greg Mann wrote: > > 3rdparty/stout/tests/base64_tests.cpp > > Lines 48-69 (patched) > > > > > > Could we use strings for these tests that include both of the modified > > alphabet

Re: Review Request 57282: Sent `UpdateFrameworkMessage` even to disconnected frameworks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57282/#review167822 --- Patch looks great! Reviews applied: [57282] Passed command:

Re: Review Request 57269: Added a test to ensure allocation roles are exposed in master API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57269/#review167811 --- Patch looks great! Reviews applied: [57269] Passed command:

Review Request 57282: Sent `UpdateFrameworkMessage` even to disconnected frameworks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57282/ --- Review request for mesos and Benjamin Mahler. Repository: mesos Description

Re: Review Request 57110: Updated the master to handle frameworks that changes its roles.

> On Feb. 27, 2017, 2:25 p.m., Michael Park wrote: > > src/master/master.cpp > > Lines 5992-5993 (patched) > > > > > > Resolve this before committing. Addressed by https://reviews.apache.org/r/57282/ - Michael

Re: Review Request 53369: Agent cgroup assignment should precede agent initialization.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53369/#review167806 --- Patch looks great! Reviews applied: [53369] Passed command:

Re: Review Request 56623: Implemented the 'Principal' type in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56623/#review167805 --- LGTM 3rdparty/libprocess/include/process/http.hpp Line 106

Re: Review Request 57109: Re-checkpointed the `Executor`s and `Task`s during agent recovery.

> On March 2, 2017, 3:11 p.m., Benjamin Mahler wrote: > > src/slave/slave.hpp > > Lines 1062 (patched) > > > > > > maybe `recheckpointExecutor`? Took this recommendation and also updated to `recheckpointTask`. >

Re: Review Request 57109: Re-checkpointed the `Executor`s and `Task`s during agent recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57109/ --- (Updated March 3, 2017, 1:12 a.m.) Review request for mesos and Benjamin

Review Request 57269: Added a test to ensure allocation roles are exposed in master API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57269/ --- Review request for mesos and Benjamin Mahler. Bugs: MESOS-7158