Re: Review Request 55973: Update the tests to handle MULTI_ROLE support.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55973/#review163936 --- Ship it! src/tests/hook_tests.cpp (line 1124)

Re: Review Request 56195: Updated containerizer->launch path to close IOSwitchboard FDs on error.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56195/ --- (Updated Feb. 2, 2017, 5:29 a.m.) Review request for mesos, Alexander

Re: Review Request 54537: Support 'Basic' auth docker registry on Unified Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54537/#review163933 --- src/uri/fetchers/docker.cpp (line 636)

Re: Review Request 54537: Support 'Basic' auth docker registry on Unified Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54537/#review163920 --- src/uri/fetchers/docker.cpp (line 620)

Re: Review Request 56144: Added unit test for http::Headers abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56144/#review163931 --- Patch looks great! Reviews applied: [56116, 56117, 54537, 56144]

Re: Review Request 56174: Added skipping already stored layers to local Docker puller.

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

Re: Review Request 54537: Support 'Basic' auth docker registry on Unified Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54537/ --- (Updated Feb. 1, 2017, 5:48 p.m.) Review request for mesos, Avinash sridharan,

Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.

> On Feb. 1, 2017, 3:51 p.m., James Peach wrote: > > src/tests/containerizer/rootfs.cpp, line 90 > > > > > > What are you asking for here? This can fail for a number of reasons and > > `errno` describes them.

Re: Review Request 56195: Updated containerizer->launch path to close IOSwitchboard FDs on error.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56195/ --- (Updated Feb. 2, 2017, 1:29 a.m.) Review request for mesos, Alexander

Re: Review Request 56116: Implemented new http::Headers abstraction for WWW-Authenticate.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56116/ --- (Updated Feb. 1, 2017, 4:59 p.m.) Review request for mesos, Avinash sridharan,

Re: Review Request 56144: Added unit test for http::Headers abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56144/ --- (Updated Feb. 1, 2017, 4:59 p.m.) Review request for mesos, Avinash sridharan,

Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.

> On Feb. 1, 2017, 3:51 p.m., James Peach wrote: > > Could you directly reply to the comment in the future? It's pretty hard to follow with this format... > On Feb. 1, 2017, 3:51 p.m., James Peach wrote: > > src/tests/containerizer/rootfs.hpp, line 50 > >

Re: Review Request 56144: Added unit test for http::Headers abstraction.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56144/ --- (Updated Feb. 1, 2017, 4:44 p.m.) Review request for mesos, Avinash sridharan,

Re: Review Request 54537: Support 'Basic' auth docker registry on Unified Containerizer.

> On Feb. 1, 2017, 9:51 a.m., Jie Yu wrote: > > src/uri/fetchers/docker.cpp, lines 470-514 > > > > > > In fact, a second thought on this. Should we rename `getAuthToken` to > > `getAuthHeader` and put all the logic

Re: Review Request 54537: Support 'Basic' auth docker registry on Unified Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54537/ --- (Updated Feb. 1, 2017, 4:42 p.m.) Review request for mesos, Avinash sridharan,

Re: Review Request 56116: Implemented new http::Headers abstraction for WWW-Authenticate.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56116/ --- (Updated Feb. 1, 2017, 4:40 p.m.) Review request for mesos, Avinash sridharan,

Re: Review Request 56116: Implemented new http::Headers abstraction for WWW-Authenticate.

> On Jan. 31, 2017, 4:15 p.m., Jie Yu wrote: > > 3rdparty/libprocess/src/http.cpp, line 624 > > > > > > RFC says that realm is defined for all auth schemes, but didn't say > > that each auth challenge needs to have

Re: Review Request 56193: Use Docker cpus instead of cpu_share if possible.

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

Re: Review Request 54712: Use the stout ELF parser to implement ldd.

> On Feb. 2, 2017, 12:04 a.m., Jiang Yan Xu wrote: > > src/linux/ldd.hpp, line 30 > > > > > > Sorry it only occured to me after the review but can we remove the > > given `path` from the result set to match `ldd`

Re: Review Request 54712: Use the stout ELF parser to implement ldd.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54712/ --- (Updated Feb. 2, 2017, 12:15 a.m.) Review request for mesos, Jie Yu, Kevin

Re: Review Request 54878: Add some simple ldd() tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54878/#review163906 --- Ship it! src/tests/ldd_tests.cpp (line 47)

Re: Review Request 56195: Updated containerizer->launch path to close IOSwitchboard FDs on error.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56195/ --- (Updated Feb. 2, 2017, 12:06 a.m.) Review request for mesos, Alexander

Review Request 56195: Updated containerizer->launch path to close IOSwitchboard FDs on error.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56195/ --- Review request for mesos, Alexander Rukletsov, Gastón Kleiman, Gilbert Song,

Re: Review Request 54712: Use the stout ELF parser to implement ldd.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54712/#review163904 --- src/linux/ldd.hpp (line 30)

Re: Review Request 55868: Cleanups to the allocator tests.

> On Feb. 1, 2017, 1:27 a.m., Jiang Yan Xu wrote: > > src/tests/hierarchical_allocator_tests.cpp, line 4357 > > > > > > re Allocation vs. OfferedResources: > > > > Guangya and I chatted a bunch on

Re: Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55972/#review163902 --- Fix it, then Ship it! src/master/master.cpp (line 5617)

Re: Review Request 54878: Add some simple ldd() tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54878/ --- (Updated Feb. 1, 2017, 11:52 p.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53791/#review163834 --- src/tests/containerizer/rootfs.hpp (line 50)

Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53791/ --- (Updated Feb. 1, 2017, 11:51 p.m.) Review request for mesos, Gilbert Song, Jie

Re: Review Request 56004: Fixed MULTI_ROLE related bugs when updating framework info.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56004/#review163900 --- Ship it! Thanks for catching these issues!

Re: Review Request 55828: Updated Resources::apply to handle Resource.AllocationInfo.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55828/#review163898 --- Filed https://issues.apache.org/jira/browse/MESOS-7048 to move

Re: Review Request 55995: Restored quota correctly during allocator recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55995/#review163897 --- Ship it! Ship It! - Alexander Rukletsov On Jan. 26, 2017,

Re: Review Request 55359: Consolidate update of allocations in `updateAllocation()`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55359/ --- (Updated Feb. 1, 2017, 10:45 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 45962: Updated a persistent volume test framework to include shared volumes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45962/ --- (Updated Feb. 1, 2017, 10:45 p.m.) Review request for mesos, Greg Mann, Jie

Re: Review Request 49571: Added a benchmark test for allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49571/ --- (Updated Feb. 1, 2017, 10:45 p.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 53096: Fix handling in shared count in total resources in the sorter.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53096/ --- (Updated Feb. 1, 2017, 10:45 p.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 56188: Removed all instances of '.Times(1)' from libprocess tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56188/#review163896 --- Patch looks great! Reviews applied: [55954, 55955, 56055, 56052,

Re: Review Request 55828: Updated Resources::apply to handle Resource.AllocationInfo.

> On Jan. 28, 2017, 3:35 a.m., Guangya Liu wrote: > > src/common/resources.cpp, lines 1274-1276 > > > > > > This indicates that if the `Resource` do not have `AllocationInfo`, > > then we will loop all of the

Re: Review Request 55828: Updated Resources::apply to handle Resource.AllocationInfo.

> On Jan. 29, 2017, 8:02 a.m., Michael Park wrote: > > src/common/resources.cpp, lines 1257-1261 > > > > > > We talked about how this condition probably isn't necessary since we > > set the `AllocationInfo` in > >

Re: Review Request 55828: Updated Resources::apply to handle Resource.AllocationInfo.

> On Jan. 30, 2017, 4:40 p.m., Benjamin Bannier wrote: > > src/common/resources.cpp, line 1288 > > > > > > This looks like an expensive computation. Should we precompute it at > > function scope and capture it

Review Request 56193: Use Docker cpus instead of cpu_share if possible.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56193/ --- Review request for mesos, Jie Yu and Kevin Klues. Bugs: MESOS-7006

Re: Review Request 55828: Updated Resources::apply to handle Resource.AllocationInfo.

> On Jan. 30, 2017, 4:40 p.m., Benjamin Bannier wrote: > > src/common/resources.cpp, lines 1263-1266 > > > > > > Could you clarify under what conditions this can happen? > > > > It seems surprising to

Re: Review Request 56188: Removed all instances of '.Times(1)' from libprocess tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56188/#review163892 --- Ship it! Ship It! - Vinod Kone On Feb. 1, 2017, 9:26 p.m.,

Re: Review Request 56187: Removed all instances of '.Times(1)' from Mesos tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56187/#review163891 --- Ship it! Thanks for the cleanup! - Vinod Kone On Feb. 1,

Re: Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

> On Jan. 31, 2017, 8:31 p.m., Vinod Kone wrote: > > include/mesos/v1/mesos.proto, line 1892 > > > > > > ditto. Sorry I'm not sure precisely what this issue is referring to? I added text to this comment regarding

Re: Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56053/ --- (Updated Feb. 1, 2017, 10:08 p.m.) Review request for mesos, Jan Schlicht and

Re: Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55972/#review163883 --- src/master/master.cpp (lines 5663 - 5667)

Re: Review Request 56055: Added validation for 'LAUNCH_NESTED_CONTAINER_SESSION'.

> On Jan. 30, 2017, 9:24 a.m., Jan Schlicht wrote: > > src/tests/slave_validation_tests.cpp, line 282 > > > > > > Indent with 4 spaces. > > Greg Mann wrote: > Unfortunately, I think our style guide is ambiguous

Review Request 56188: Removed all instances of '.Times(1)' from libprocess tests.

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

Review Request 56187: Removed all instances of '.Times(1)' from Mesos tests.

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

Re: Review Request 56006: Added CHECK logging to the allocator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56006/#review163878 --- Ship it! src/master/allocator/mesos/hierarchical.cpp (line

Re: Review Request 55971: Updated the agent to be MULTI_ROLE capable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55971/#review163877 --- src/slave/slave.cpp (line 5235)

Re: Review Request 55971: Updated the agent to be MULTI_ROLE capable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55971/#review163876 --- Ship it! src/slave/slave.cpp (lines 5227 - 5229)

Re: Review Request 55901: Added support for command health checks to the default executor.

> On Jan. 28, 2017, 1:17 a.m., Vinod Kone wrote: > > src/checks/health_checker.cpp, lines 426-429 > > > > > > why a `.repair()` here? > > Gastón Kleiman wrote: > To fail the future/check with a nice descriptive

Re: Review Request 55732: Added `--3way` option to apply-reviews.py.

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

Re: Review Request 56086: Simplified AppC provisioner cache keys comparison.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56086/#review163867 --- Ship it! Ship It! - Benjamin Mahler On Feb. 1, 2017, 6:49

Re: Review Request 56087: Simplified AppC provisioner cache keys hashing.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56087/#review163866 --- Ship it! Ship It! - Benjamin Mahler On Feb. 1, 2017, 6:49

Re: Review Request 55852: Fixed MasterAllocatorTest/1.RebalancedForUpdatedWeights.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55852/ --- (Updated Feb. 1, 2017, 11:19 a.m.) Review request for mesos, Guangya Liu and

Re: Review Request 56174: Added skipping already stored layers to local Docker puller.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56174/#review163854 --- Ship it! Ship It! - Gilbert Song On Feb. 1, 2017, 7:17

Re: Review Request 56087: Simplified AppC provisioner cache keys hashing.

> On Feb. 1, 2017, 6:45 p.m., Gilbert Song wrote: > > LGTM! > > > > Thanks for the patches, Ilya! Could you add `MESOS-7046` to `BUG` in your > > patches? > > > > And just let you know that you can post your patches as a chain using > > `support/post-reviews.py`. Done. Thanks! - Ilya

Re: Review Request 56087: Simplified AppC provisioner cache keys hashing.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56087/ --- (Updated Feb. 1, 2017, 6:49 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 56086: Simplified AppC provisioner cache keys comparison.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56086/ --- (Updated Feb. 1, 2017, 6:49 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 56087: Simplified AppC provisioner cache keys hashing.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56087/#review163852 --- Ship it! LGTM! Thanks for the patches, Ilya! Could you add

Re: Review Request 55732: Added `--3way` option to apply-reviews.py.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55732/ --- (Updated Feb. 1, 2017, 6:45 p.m.) Review request for mesos, Jason Lai, Joseph

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

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

Re: Review Request 56086: Simplified AppC provisioner cache keys comparison.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56086/#review163849 --- Ship it! LGTM! - Gilbert Song On Jan. 30, 2017, 9:27 a.m.,

Re: Review Request 51028: Fix tests with rapidly triggered allocations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51028/ --- (Updated Feb. 1, 2017, 10:36 a.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 55876: Avoided shadowing in `Slave::run()`.

> On Jan. 27, 2017, 5:17 p.m., Neil Conway wrote: > > src/slave/slave.cpp, line 1580 > > > > > > If we're going to change this, there are a few other places that should > > also be changed: e.g., lines 1179, 1226,

Re: Review Request 54537: Support 'Basic' auth docker registry on Unified Containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54537/#review163759 --- src/uri/fetchers/docker.cpp (line 468)

Re: Review Request 56177: Updated reconciliation comment in "scheduler.proto".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56177/#review163836 --- Fix it, then Ship it! include/mesos/scheduler/scheduler.proto

Re: Review Request 55321: Introduced process::after.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55321/#review163833 --- 3rdparty/libprocess/include/process/after.hpp (line 25)

Re: Review Request 56174: Added skipping already stored layers to local Docker puller.

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

Review Request 56177: Updated reconciliation comment in "scheduler.proto".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56177/ --- Review request for mesos, Neil Conway and Vinod Kone. Repository: mesos

Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

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

Re: Review Request 55321: Introduced process::after.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55321/#review163825 --- 3rdparty/libprocess/include/process/after.hpp (lines 50 - 55)

Review Request 56174: Added skipping already stored layers to local Docker puller.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56174/ --- Review request for mesos, Jie Yu and Timothy Chen. Bugs: MESOS-7045

Re: Review Request 55901: Added support for command health checks to the default executor.

> On Jan. 28, 2017, 1:17 a.m., Vinod Kone wrote: > > src/checks/health_checker.cpp, line 480 > > > > > > launch nested container session returns a streaming response, how come > > you are calling `post()` helper

Re: Review Request 55453: Updated comments in `HealthCheck` protobuf for clarity.

> On Jan. 30, 2017, 6:01 p.m., Jiang Yan Xu wrote: > > include/mesos/mesos.proto, lines 424-426 > > > > > > The most direct interpretation for the delay is actually the time since > > the task was launched right?