Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55242/#review161636 --- Bad patch! Reviews applied: [55238, 55239, 55240, 55241, 55242]

Re: Review Request 55521: Handle perf versions with more than 3 components.

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

Re: Review Request 55489: Used CMake to generate the compilation database instead.

> On Jan. 13, 2017, 4 a.m., Benjamin Bannier wrote: > > support/mesos-tidy.sh, line 19 > > > > > > Should we also set `pipefail`? I didn't know about that. Sounds great. > On Jan. 13, 2017, 4 a.m., Benjamin

Re: Review Request 55488: Slimmed down the size of the docker image from 2GB to roughly 650MB.

> On Jan. 13, 2017, 3:53 a.m., Benjamin Bannier wrote: > > support/mesos-tidy/Dockerfile, lines 69-77 > > > > > > These packages are from the Mesos getting started guide, and I am not > > sure we would like to

Re: Review Request 55343: Fixed SSL socket 'shutdown()'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55343/#review161626 --- Ship it! I can tweak this before committing.

Re: Review Request 53803: Added new libprocess socket tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53803/#review161625 --- Ship it! I can tweak the below comments before committing.

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

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

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55509/#review161617 --- Fix it, then Ship it! Sorry missed a few minor things.

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

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

Re: Review Request 55241: Stop using os::system to validate perf event names.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55241/#review161616 --- Ship it! Ship It! - Jiang Yan Xu On Jan. 13, 2017, 3:11

Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

> On Jan. 13, 2017, 8:27 a.m., Jiang Yan Xu wrote: > > 3rdparty/stout/tests/os_tests.cpp, line 751 > > > > > > s/tree/subtree/? This commend looks correct to me. - James

Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

> On Jan. 13, 2017, 8:27 a.m., Jiang Yan Xu wrote: > > 3rdparty/stout/tests/os_tests.cpp, line 751 > > > > > > s/tree/subtree/? This commend looks correct to me. - James

Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55242/ --- (Updated Jan. 13, 2017, 11:12 p.m.) Review request for mesos, Benjamin Mahler

Re: Review Request 55240: Stop using os::system to copy local files.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55240/ --- (Updated Jan. 13, 2017, 11:11 p.m.) Review request for mesos, Jie Yu and Jiang

Re: Review Request 55239: Stop using os::system to extract fetcher archives.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55239/ --- (Updated Jan. 13, 2017, 11:11 p.m.) Review request for mesos, Jie Yu and Jiang

Re: Review Request 55241: Stop using os::system to validate perf event names.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55241/ --- (Updated Jan. 13, 2017, 11:11 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55238: Use os::spawn in the CNI isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55238/ --- (Updated Jan. 13, 2017, 11:11 p.m.) Review request for mesos, Avinash

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55509/#review161611 --- Fix it, then Ship it! Looks like there are a few more cases of

Re: Review Request 55021: Added a framework capabilities struct.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55021/#review161610 --- src/tests/protobuf_utils_tests.cpp (line 65)

Re: Review Request 55068: Added Capabilities to Framework struct of Hierarchical allocator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55068/#review161607 --- Ship it! Ship It! - Benjamin Mahler On Jan. 5, 2017, 12:19

Re: Review Request 55066: Added Capabilities to Framework struct of Master and Agent.

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

Re: Review Request 55021: Added a framework capabilities struct.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55021/#review161604 --- Fix it, then Ship it! Looks good, just a few minor suggestions

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

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

Re: Review Request 55521: Handle perf versions with more than 3 components.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55521/ --- (Updated Jan. 13, 2017, 8:24 p.m.) Review request for mesos, Ian Downes, Kapil

Review Request 55521: Handle perf versions with more than 3 components.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55521/ --- Review request for mesos, Ian Downes, Kapil Arya, and Jiang Yan Xu. Bugs:

Re: Review Request 55447: Added sanity checks on IDs and roles before creating directories.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55447/#review161582 --- Ship it! Ship It! - James Peach On Jan. 12, 2017, 9:48

Re: Review Request 55446: Added common validation for IDs.

> On Jan. 12, 2017, 1:43 p.m., James Peach wrote: > > src/common/validation.hpp, line 34 > > > > > > Since this is never used externally, consider making it a local static > > helper/ This is used externally. >

Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.

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

Re: Review Request 55448: Implemented TODOs to use common ID validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55448/#review161574 --- Ship it! Ship It! - James Peach On Jan. 12, 2017, 9:48

Re: Review Request 55343: Fixed SSL socket 'shutdown()'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55343/ --- (Updated Jan. 13, 2017, 7:19 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 55238: Use os::spawn in the CNI isolator.

> On Jan. 11, 2017, 10:11 a.m., Jiang Yan Xu wrote: > > I feel if we keep `os::system()` in the codebase at all, this is one of the > > few places it could actually be used... we could eliminate it so we can say > > there's no references to `os::systems()` left today but it's a bit harsh to >

Re: Review Request 55446: Added common validation for IDs.

> On Jan. 13, 2017, 10:50 a.m., Anindya Sinha wrote: > > src/common/validation.cpp, line 36 > > > > > > Should we consider disallowing spaces as well? `iscntrl` includes tabs > > but do we want spaces to be included

Re: Review Request 55238: Use os::spawn in the CNI isolator.

> On Jan. 11, 2017, 2:11 a.m., Jiang Yan Xu wrote: > > I feel if we keep `os::system()` in the codebase at all, this is one of the > > few places it could actually be used... we could eliminate it so we can say > > there's no references to `os::systems()` left today but it's a bit harsh to >

Re: Review Request 55487: Renamed executable files from '_' to '-' in the `support` directory.

> On Jan. 13, 2017, 10:13 a.m., Vinod Kone wrote: > > Make sure you disable the ASF CI jobs before you land this. Re-enable them > > after updating the Jenkins job configurations. Yep! - Michael --- This is an automatically generated

Re: Review Request 55446: Added common validation for IDs.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55446/#review161560 --- src/common/validation.cpp (line 36)

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

> On Jan. 13, 2017, 6:39 p.m., Neil Conway wrote: > > Seems like you need to rebase? Oops, thanks :) - James --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55509/#review161555

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55509/ --- (Updated Jan. 13, 2017, 6:41 p.m.) Review request for mesos and Jiang Yan Xu.

Re: Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55509/#review161555 --- Seems like you need to rebase? - Neil Conway On Jan. 13, 2017,

Re: Review Request 55487: Renamed executable files from '_' to '-' in the `support` directory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55487/#review161553 --- Ship it! Make sure you disable the ASF CI jobs before you land

Review Request 55509: Validate the StatusUpdate UUID in Master::statusUpdate.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55509/ --- Review request for mesos and Jiang Yan Xu. Bugs: MESOS-6920

Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 13, 2017, 4:46 p.m.) Review request for mesos and Anand

Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 13, 2017, 3:56 p.m.) Review request for mesos and Anand

Re: Review Request 55480: Fix segfault when the executor sets a UUID that is not a valid v4 UUID.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55480/ --- (Updated Jan. 13, 2017, 3:52 p.m.) Review request for mesos and Anand

Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55496/ --- (Updated Jan. 13, 2017, 4:51 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 55461: Made resource reservation validation multi-role aware.

> On Jan. 13, 2017, 9:17 a.m., Jay Guo wrote: > > When a resource with `*` is offered to a multi-role framework, how does the > > framework decide which role to reserve the resource for? Frameworks with default role cannot reserve resources; this is the first check in the validation function

Re: Review Request 55461: Made resource reservation validation multi-role aware.

> On Jan. 13, 2017, 9:15 a.m., Jay Guo wrote: > > src/master/master.cpp, lines 3944-3950 > > > > > > I think we explicitly disallow empyt role field? > >

Re: Review Request 55461: Made resource reservation validation multi-role aware.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55461/ --- (Updated Jan. 13, 2017, 4:50 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55496/#review161537 --- 3rdparty/libprocess/src/decoder.hpp (line 416)

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55464/ --- (Updated Jan. 13, 2017, 1:52 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55496/ --- (Updated Jan. 13, 2017, 2:48 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55496/#review161516 --- Don't merge yet, this seems to break chunked transfers. - Jan

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

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

Re: Review Request 55456: Fixed include order in "launcher/executor.cpp".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55456/#review161512 --- src/launcher/executor.cpp (line 79)

Re: Review Request 55454: Ensured zero health check timeout means infinite timeout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55454/#review161509 --- Ship it! Ship It! - Gastón Kleiman On Jan. 12, 2017, 1:19

Re: Review Request 55491: Added a `jenkins/tidybot.sh`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55491/#review161505 --- Is the intention here to add more stuff later, or move

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55453/#review161504 --- Ship it! Ship It! - Gastón Kleiman On Jan. 12, 2017, 1:18

Re: Review Request 55489: Used CMake to generate the compilation database instead.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55489/#review161500 --- support/mesos-tidy.sh (line 19)

Re: Review Request 55488: Slimmed down the size of the docker image from 2GB to roughly 650MB.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55488/#review161503 --- Fix it, then Ship it! support/mesos-tidy/Dockerfile

Review Request 55496: Fixed parsing of proxy CONNECT responses.

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

Re: Review Request 55491: Added a `jenkins/tidybot.sh`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55491/#review161499 --- Bad patch! Reviews applied: [55487, 55488, 55489, 55490, 55491]

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

> On Jan. 12, 2017, 3:50 p.m., Alexander Rojas wrote: > > src/tests/api_tests.cpp, lines 3619-3659 > > > > > > Why not replacing this test with one that shows the opposite? I added a test that launches parent and

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55464/ --- (Updated Jan. 13, 2017, 10:08 a.m.) Review request for mesos, Adam B,

Review Request 55491: Added a `jenkins/tidybot.sh`.

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

Re: Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55490/ --- (Updated Jan. 13, 2017, 2 a.m.) Review request for mesos and Benjamin Bannier.

Re: Review Request 55489: Used CMake to generate the compilation database instead.

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

Review Request 55489: Used CMake to generate the compilation database instead.

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

Review Request 55487: Renamed executable files from '_' to '-' in the `support` directory.

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

Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

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

Re: Review Request 55449: Validate executor IDs in master validation.

> On Jan. 12, 2017, 4:31 p.m., Joseph Wu wrote: > > src/tests/master_validation_tests.cpp, line 1751 > > > > > > I had the impression that spaces in IDs were also bad... > > > > I'm hoping we don't use the

Re: Review Request 55242: Stop using os::system to chown a directory hierarchy.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55242/#review161413 --- 3rdparty/stout/include/stout/os/posix/chown.hpp (lines 36 - 42)

Re: Review Request 55461: Made resource reservation validation multi-role aware.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55461/#review161489 --- When a resource with `*` is offered to a multi-role framework,

Re: Review Request 55461: Made resource reservation validation multi-role aware.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55461/#review161488 --- src/master/master.cpp (lines 3944 - 3950)