Review Request 58410: Removed a duplicate unit test 'ROOT_INTERNET_CURL_Normalize'.

2017-04-12 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58410/ --- Review request for mesos, Avinash sridharan, Chun-Hung Hsiao, Artem

Re: Review Request 48917: Fixed docker fetcher 3xx redirect errors by header attached.

2017-04-12 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48917/ --- (Updated April 12, 2017, 10:33 p.m.) Review request for mesos, Avinash

Re: Review Request 58329: Parameterized the existing alpine based test with more registries.

2017-04-12 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58329/ --- (Updated April 12, 2017, 10:33 p.m.) Review request for mesos, Avinash

Re: Review Request 48917: Fixed docker fetcher 3xx redirect errors by header attached.

2017-04-12 Thread Gilbert Song
> On April 12, 2017, 10:51 a.m., Chun-Hung Hsiao wrote: > > Have you also tested on Dockerhub and perhaps a local private registry? > > Chun-Hung Hsiao wrote: > Oh Dockerhub is tested https://reviews.apache.org/r/58329/. How about a > private Docker registry with common settings? Yes,

Re: Review Request 58408: Overwriting Directories with Files in Copy Provisioner.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58408/#review171840 --- Patch looks great! Reviews applied: [58408] Passed command:

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

2017-04-12 Thread Jie Yu
> 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? > > Alexander Rukletsov

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58394/#review171835 --- Patch looks great! Reviews applied: [58394] Passed command:

Review Request 58408: Overwriting Directories with Files in Copy Provisioner.

2017-04-12 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58408/ --- Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone. Bugs:

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

2017-04-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58195/#review171834 --- Ship it! Ship It! - Vinod Kone On April 4, 2017, 10:24

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

2017-04-12 Thread Vinod Kone
> 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 58194: Hardened HTTP check tests.

2017-04-12 Thread Vinod Kone
> 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 57951: Moved new CLI settings into a user-defined TOML file.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57951/#review171831 --- Patch looks great! Reviews applied: [57896, 57951] Passed

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

2017-04-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58263/#review171830 --- src/slave/containerizer/mesos/containerizer.hpp Lines 325

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

2017-04-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58262/#review171829 --- src/slave/containerizer/mesos/containerizer.cpp Line 1377

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

2017-04-12 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57817/#review171630 --- include/mesos/allocator/allocator.hpp Lines 135 (patched)

Re: Review Request 58258: Added a new agent authorization test which runs a task group.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58258/#review171814 --- Patch looks great! Reviews applied: [58251, 58252, 58253, 58254,

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

2017-04-12 Thread Eric Chung
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58394/ --- (Updated April 12, 2017, 9:44 p.m.) Review request for mesos, Jason Lai,

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

2017-04-12 Thread Eric Chung
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58394/ --- (Updated April 12, 2017, 9:43 p.m.) Review request for mesos, Jason Lai,

Re: Review Request 58329: Parameterized the existing alpine based test with more registries.

2017-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58329/#review171797 --- Fix it, then Ship it!

Re: Review Request 48917: Fixed docker fetcher 3xx redirect errors by header attached.

2017-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48917/#review171789 --- Fix it, then Ship it! src/uri/fetchers/docker.cpp Lines 280

Re: Review Request 58320: Added an upgrade test for executor authentication.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58320/#review171795 --- Patch looks great! Reviews applied: [58321, 58320] Passed

Re: Review Request 57951: Moved new CLI settings into a user-defined TOML file.

2017-04-12 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57951/ --- (Updated April 12, 2017, 7:59 p.m.) Review request for mesos, Joseph Wu and

Re: Review Request 58258: Added a new agent authorization test which runs a task group.

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58258/ --- (Updated April 12, 2017, 6:46 p.m.) Review request for mesos, Adam B,

Re: Review Request 58287: Print corresponding address when socket shutdown.

2017-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58287/#review171772 --- 3rdparty/libprocess/src/process.cpp Lines 2362 (patched)

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

2017-04-12 Thread Jiang Yan Xu
> On April 11, 2017, 2:11 a.m., Adam B wrote: > > src/tests/script.cpp > > Lines 161 (patched) > > > > > > We don't even enable agent authentication in most(/any?) tests, so I'd > > expect the agents to try to

Re: Review Request 57889: Fixed flags logging in Docker executor.

2017-04-12 Thread Till Toenshoff
> On April 12, 2017, 6:12 p.m., Jie Yu wrote: > > Do we have a ticket for this. Looks like this patch is backported to > > multiple branches, but there is not ticket for this. How can we let the > > world know what we did for the patch release? > > > > @till and @adam, can you create a ticket

Re: Review Request 58373: Updated 'Checker' to authenticate with agent operator API.

2017-04-12 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58373/#review171768 --- Ship it! Ship It! - Alexander Rukletsov On April 12, 2017,

Re: Review Request 57889: Fixed flags logging in Docker executor.

2017-04-12 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57889/#review171767 --- Do we have a ticket for this. Looks like this patch is backported

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

2017-04-12 Thread Eric Chung
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58394/ --- (Updated April 12, 2017, 6:10 p.m.) Review request for mesos, Joseph Wu and

Review Request 58394: Setup new directory for python http client lib in src/python.

2017-04-12 Thread Eric Chung
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58394/ --- Review request for mesos, Joseph Wu and Kevin Klues. Bugs: MESOS-7310

Re: Review Request 58258: Added a new agent authorization test which runs a task group.

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58258/ --- (Updated April 12, 2017, 6:07 p.m.) Review request for mesos, Adam B,

Re: Review Request 58137: Added 'mesos config show' command to display the config file.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58137/#review171765 --- Bad patch! Reviews applied: [58137, 57952, 58381] Failed

Re: Review Request 58258: Added a new agent authorization test which runs a task group.

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58258/ --- (Updated April 12, 2017, 5:57 p.m.) Review request for mesos, Adam B,

Re: Review Request 48917: Fixed docker fetcher 3xx redirect errors by header attached.

2017-04-12 Thread Chun-Hung Hsiao
> On April 12, 2017, 5:51 p.m., Chun-Hung Hsiao wrote: > > Have you also tested on Dockerhub and perhaps a local private registry? Oh Dockerhub is tested https://reviews.apache.org/r/58329/. How about a private Docker registry with common settings? - Chun-Hung

Re: Review Request 48917: Fixed docker fetcher 3xx redirect errors by header attached.

2017-04-12 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48917/#review171762 --- Have you also tested on Dockerhub and perhaps a local private

Re: Review Request 58320: Added an upgrade test for executor authentication.

2017-04-12 Thread Greg Mann
> On April 11, 2017, 7:18 p.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp > > Lines 6041-6042 (patched) > > > > > > why is one a ref and not the other? Whoops! - Greg

Re: Review Request 58320: Added an upgrade test for executor authentication.

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58320/ --- (Updated April 12, 2017, 5:45 p.m.) Review request for mesos, Anand Mazumdar

Re: Review Request 58258: Added a new agent authorization test which runs a task group.

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58258/ --- (Updated April 12, 2017, 5:10 p.m.) Review request for mesos, Adam B,

Re: Review Request 57951: Moved new CLI settings into a user-defined TOML file.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57951/#review171756 --- Bad patch! Reviews applied: [57951, 57896] Failed command:

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

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58252/ --- (Updated April 12, 2017, 4:40 p.m.) Review request for mesos, Adam B,

Re: Review Request 58373: Updated 'Checker' to authenticate with agent operator API.

2017-04-12 Thread Greg Mann
> On April 12, 2017, 9:53 a.m., Alexander Rukletsov wrote: > > src/checks/checker.cpp > > Lines 207 (patched) > > > > > > Is pass-by-value intended? Yea I did intend to store a copy of the authorization header here

Re: Review Request 58368: Updated 'HealthChecker' to authenticate with the agent.

2017-04-12 Thread Greg Mann
> On April 12, 2017, 9:53 a.m., Alexander Rukletsov wrote: > > src/checks/health_checker.hpp > > Lines 234 (patched) > > > > > > Is pass-by-value intended here? Yea I did intend to store a copy of the authorization

Re: Review Request 58358: Upgrade vendored 3rdparty protobuf to 3.2.0.

2017-04-12 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58358/ --- (Updated April 12, 2017, 4:29 p.m.) Review request for mesos, Anand Mazumdar

Re: Review Request 58379: Fixed a typo in persistent volume doc.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58379/#review171743 --- Patch looks great! Reviews applied: [58379] Passed command:

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

2017-04-12 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58255/#review171739 --- My concern about this patch is, that unlike all the other

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

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58099/#review171736 --- Patch looks great! Reviews applied: [58095, 58096, 58097, 58099]

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

2017-04-12 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58254/#review171722 --- Ship it! Ship It! - Alexander Rojas On April 12, 2017, 9:31

Re: Review Request 58337: Add allowed devices whitelist for cgroups/devices isolator.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58337/#review171721 --- Patch looks great! Reviews applied: [58337] Passed command:

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

2017-04-12 Thread Alexander Rojas
> On April 8, 2017, 1:30 a.m., Greg Mann wrote: > > For some reason I'm having trouble replying to your previous comment, so > > I'll post a new one :) > > > > I think that it makes sense to have claims in the `authorization::Subject`, > > since this maps directly onto the `Principal`

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

2017-04-12 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58252/#review171707 --- Ship it! Ship It! - Alexander Rojas On April 8, 2017, 1:15

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

2017-04-12 Thread Alexander Rojas
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58251/#review171706 --- Ship it! Ship It! - Alexander Rojas On April 7, 2017, 5:26

Re: Review Request 58287: Print corresponding address when socket shutdown.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58287/#review171703 --- Patch looks great! Reviews applied: [58287] Passed command:

Re: Review Request 58370: Updated a health checker test to enable executor authentication.

2017-04-12 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58370/#review171702 --- Ship it! Assuming you've tested this with SSL enabled. -

Re: Review Request 58369: Updated default executor to pass authorization header to checkers.

2017-04-12 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58369/#review171700 --- src/launcher/default_executor.cpp Line 138 (original), 138-140

Re: Review Request 58373: Updated 'Checker' to authenticate with agent operator API.

2017-04-12 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58373/#review171697 --- LGTM. However, I'm not authz expert. Assuming adding a single

Re: Review Request 58374: Updated check tests to authenticate with agent operator API.

2017-04-12 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58374/#review171701 --- Ship it! Assuming you run tests with SSL configured. -

Re: Review Request 58368: Updated 'HealthChecker' to authenticate with the agent.

2017-04-12 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58368/#review171696 --- Fix it, then Ship it! LGTM. However, I'm not authz expert.

Re: Review Request 58137: Added 'mesos config show' command to display the config file.

2017-04-12 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58137/ --- (Updated April 12, 2017, 9:21 a.m.) Review request for mesos, Joseph Wu and

Re: Review Request 57952: Added 'config' plugin to the new CLI.

2017-04-12 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57952/ --- (Updated April 12, 2017, 9:19 a.m.) Review request for mesos, Joseph Wu and

Review Request 58381: Added a Table abstraction to the new CLI.

2017-04-12 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58381/ --- Review request for mesos and Joseph Wu. Bugs: MESOS-7282

Re: Review Request 57951: Moved new CLI settings into a user-defined TOML file.

2017-04-12 Thread Armand Grillet
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57951/ --- (Updated April 12, 2017, 9:07 a.m.) Review request for mesos, Joseph Wu and

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

2017-04-12 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57474/#review171690 --- Test logic looks good, although I question "archbishop" as a

Re: Review Request 58374: Updated check tests to authenticate with agent operator API.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58374/#review171692 --- Patch looks great! Reviews applied: [58368, 58373, 58369, 58370,

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

2017-04-12 Thread Adam B
> On March 20, 2017, 10:28 a.m., Benjamin Bannier wrote: > > LGTM but for one non-functional issue. > > > > I believe it might make sense to combine this into one change set with the > > previous patch since we e.g., likely wouldn't be interested in backporting. > > Maybe @adam-mesos has some

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

2017-04-12 Thread Greg Mann
> On April 11, 2017, 9:28 a.m., Adam B wrote: > > include/mesos/authorizer/authorizer.proto > > Lines 57 (patched) > > > > > > Good question. I wonder if there's a more generic data structure (not > >

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

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58254/ --- (Updated April 12, 2017, 7:31 a.m.) Review request for mesos, Adam B,

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

2017-04-12 Thread Greg Mann
> On April 11, 2017, 6:33 p.m., Vinod Kone wrote: > > src/slave/http.cpp > > Lines 732 (patched) > > > > > > a `principalContains` helper looks weird. can you just inline the > > helpers? > > > > you can

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

2017-04-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58255/ --- (Updated April 12, 2017, 7:28 a.m.) Review request for mesos, Adam B,

Re: Review Request 58361: Updated LICENSE information for protobuf 3.2.0.

2017-04-12 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58361/#review171681 --- Bad patch! Reviews applied: [58361, 58360, 58359, 58358, 58357]