Re: Review Request 58280: Added a test to verify persistent volume mount points removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58280/#review171396 --- Patch looks great! Reviews applied: [58278, 58279, 58280]

Re: Review Request 58279: Lazily unmount persistent volumes in DockerContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58279/#review171394 --- Ship it! Ship It! - Gilbert Song On April 7, 2017, 4:46

Re: Review Request 58278: Lazily unmount persistent volumes in MesosContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58278/#review171393 --- Ship it! Ship It! - Gilbert Song On April 7, 2017, 4:45

Re: Review Request 58280: Added a test to verify persistent volume mount points removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58280/#review171395 --- Ship it! Ship It! - Gilbert Song On April 7, 2017, 5:27

Re: Review Request 57166: Updated role validation for hierarchical roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57166/#review171391 --- Ship it! Ship It! - Adam B On March 7, 2017, 7:38 a.m.,

Re: Review Request 58280: Added a test to verify persistent volume mount points removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58280/ --- (Updated April 8, 2017, 12:27 a.m.) Review request for mesos, Gilbert Song,

Re: Review Request 58280: Added a test to verify persistent volume mount points removal.

> On April 8, 2017, 12:17 a.m., Zhitao Li wrote: > > Can you confirm that this test fails without the fix in r/58278? Updated the "test" section. - Jie --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 58280: Added a test to verify persistent volume mount points removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58280/#review171389 --- Can you confirm that this test fails without the fix in r/58278?

Review Request 58280: Added a test to verify persistent volume mount points removal.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58280/ --- Review request for mesos, Gilbert Song, Jason Lai, and Zhitao Li. Bugs:

Review Request 58279: Lazily unmount persistent volumes in DockerContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58279/ --- Review request for mesos, Gilbert Song, Jason Lai, and Zhitao Li. Bugs:

Review Request 58278: Lazily unmount persistent volumes in MesosContainerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58278/ --- Review request for mesos, Gilbert Song, Jason Lai, and Zhitao Li. Bugs:

Re: Review Request 58253: Added a ContainerID to 'ObjectApprover::Object'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58253/#review171388 --- For some reason I'm having trouble replying to your previous

Re: Review Request 58254: Added implicit executor authorization to the agent operator API.

> On April 7, 2017, 11:40 a.m., Alexander Rojas wrote: > > src/authorizer/local/authorizer.cpp > > Lines 654-657 (original), 683-690 (patched) > > > > > > I'm not so sure returning a `RejectingObjectApprover()` is

Re: Review Request 58254: Added implicit executor authorization to the agent operator API.

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

Re: Review Request 58252: Allowed the local authorizer to accept subjects with no value.

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

Re: Review Request 58251: Changed 'Principal.claims' to a hashmap.

> On April 7, 2017, 7:47 a.m., Alexander Rojas wrote: > > 3rdparty/libprocess/include/process/authenticator.hpp > > Line 69 (original), 69 (patched) > > > > > > My only concern with using either a `std::map` or a

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57473/#review171379 --- src/authorizer/local/authorizer.cpp Lines 401 (patched)

Re: Review Request 57972: Added base stout Environment class to mesos-tests Environment class.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57972/#review171377 --- src/tests/environment.cpp Lines 721-722 (original), 86-90

Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57824/#review171378 --- 3rdparty/stout/include/stout/tests/environment.hpp Lines 205-210

Re: Review Request 58247: Windows: Fixed test CopyFetcherPluginTest.FetchExistingFile.

> On April 6, 2017, 4:10 p.m., Andrew Schwartzmeyer wrote: > > src/uri/fetchers/copy.cpp > > Line 98 (original), 99-104 (patched) > > > > > > I think I would have named it `path` over `baseCommand`, and remove the >

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

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

Re: Review Request 58247: Windows: Fixed test CopyFetcherPluginTest.FetchExistingFile.

> On April 6, 2017, 11:10 p.m., Andrew Schwartzmeyer wrote: > > src/uri/fetchers/copy.cpp > > Line 98 (original), 99-104 (patched) > > > > > > I think I would have named it `path` over `baseCommand`, and remove the

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

> On April 7, 2017, 4:31 p.m., haosdent huang wrote: > > This requires `mesos-docker-executor` share the same namespace with `mesos-agent`. So need `--pid=host`. And `nsenter` requires privileged permissions, so need `--privileged=true`. - haosdent

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58200/ --- (Updated April 7, 2017, 4:40 p.m.) Review request for mesos, Alexander

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58200/#review171361 --- src/slave/containerizer/docker.cpp Lines 361-364 (patched)

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58196/#review171360 --- Patch looks great! Reviews applied: [58190, 58191, 58192, 58193,

Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

> On April 7, 2017, 2:30 p.m., Jie Yu wrote: > > What if agent crashes and restart? Jie, I've added checkpointing. We probably need test this functionality. Do you think we should write a separate test or you have an idea which test we maybe can augment? - Alexander

Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58262/ --- (Updated April 7, 2017, 4:21 p.m.) Review request for mesos, Gastón Kleiman,

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58263/ --- (Updated April 7, 2017, 4:19 p.m.) Review request for mesos, Gastón Kleiman,

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58196/ --- (Updated April 7, 2017, 3:25 p.m.) Review request for mesos, Gastón Kleiman

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58200/#review171353 --- src/slave/containerizer/docker.cpp Lines 361 (patched)

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58196/#review171346 --- src/checks/checker.cpp Lines 1163-1164 (patched)

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

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote: > > src/checks/checker.cpp > > Lines 1081 (patched) > > > > > > Does this work even when executor is running with its own file system? This is a good question. My

Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58262/#review171348 --- What if agent crashes and restart? - Jie Yu On April 7, 2017,

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

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote: > > src/checks/checker.cpp > > Lines 1166-1169 (patched) > > > > > > It's a bit unfortunate that the `errno` returned by `connect` is > > subsumed by the tcp check

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58263/#review171345 --- ``` I0407 11:44:08.444990 22135 linux_launcher.cpp:429] Launching

Re: Review Request 58195: Added TCP checks in Mesos API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58195/#review171343 --- Ship it! Ship It! - Gastón Kleiman On April 4, 2017, 10:24

Re: Review Request 58195: Added TCP checks in Mesos API.

> On April 7, 2017, 1:09 a.m., Vinod Kone wrote: > > src/checks/checker.cpp > > Line 341 (original) > > > > > > why no space? We seem a bit inconsistent regarding blank lines between cases in `switch` statements,

Re: Review Request 58195: Added TCP checks in Mesos API.

> On April 7, 2017, 1:09 a.m., Vinod Kone wrote: > > include/mesos/mesos.proto > > Lines 1773 (patched) > > > > > > `succeeded` seems a bit weird, can we call it `status` or > > `connection_status` to be

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58263/ --- (Updated April 7, 2017, 1:15 p.m.) Review request for mesos, Gastón Kleiman,

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/#review171337 --- src/master/http.cpp Line 3383 (original), 3385 (patched)

Re: Review Request 58194: Hardened HTTP check tests.

> On April 7, 2017, 12:31 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Line 837 (original), 839 (patched) > > > > > > Does this mean the test runs for atleast 1 second? If yes, that's > > unfortunate.

Re: Review Request 58194: Hardened HTTP check tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58194/#review171339 --- Ship it! Ship It! - Gastón Kleiman On April 4, 2017, 10:24

Re: Review Request 58194: Hardened HTTP check tests.

> On April 7, 2017, 12:31 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Line 837 (original), 839 (patched) > > > > > > Does this mean the test runs for atleast 1 second? If yes, that's > > unfortunate.

Re: Review Request 58224: RFC: Add some consistency checks for libprocess UPIDs.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58224/#review171330 --- 3rdparty/libprocess/src/process.cpp Lines 471-477 (patched)

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

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

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58193/#review171329 --- Ship it! Ship It! - Gastón Kleiman On April 4, 2017, 10:23

Re: Review Request 58254: Added implicit executor authorization to the agent operator API.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58254/#review171320 --- src/authorizer/local/authorizer.cpp Line 576 (original), 601-603

Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58262/ --- (Updated April 7, 2017, 11:05 a.m.) Review request for mesos, Gastón Kleiman,

Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58263/ --- Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.

Review Request 58262: Inherited environment from parent when launching a DEBUG container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58262/ --- Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.

Re: Review Request 58253: Added a ContainerID to 'ObjectApprover::Object'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58253/#review171327 --- include/mesos/authorizer/authorizer.proto Lines 57 (patched)

Re: Review Request 56152: Implemented the 'fetchManifest()' method of prefix puller.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56152/ --- (Updated April 7, 2017, 4:10 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 58261: Unified the descriptor type in the OCI protobuf messages.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58261/ --- (Updated April 7, 2017, 4:09 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 55331: Added 'OCI' message into 'Image' message.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55331/ --- (Updated April 7, 2017, 4:09 p.m.) Review request for mesos, Gilbert Song and

Review Request 58261: Unified the descriptor type in the OCI protobuf messages.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58261/ --- Review request for mesos, Gilbert Song and Jie Yu. Bugs: MESOS-6681

Re: Review Request 58220: Used move semantics when returning gzip compressed responses.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58220/#review171318 --- Ship it! Ship It! - Michael Park On April 5, 2017, 1:53

Re: Review Request 58252: Allowed the local authorizer to accept subjects with no value.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58252/#review171317 --- src/authorizer/local/authorizer.cpp Line 1091 (original), 1091

Re: Review Request 58251: Changed 'Principal.claims' to a hashmap.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58251/#review171316 --- 3rdparty/libprocess/include/process/authenticator.hpp Line 69