Re: Review Request 47258: Make sure 'activate' is a no-op if the client is already active.

[Guangya Liu]

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




src/master/allocator/sorter/sorter.hpp (line 70)


s/already not/not


- Guangya Liu


On 五月 11, 2016, 9:15 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47258/
> ---
> 
> (Updated 五月 11, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos and Dario Rexin.
> 
> 
> Bugs: MESOS-5279
> https://issues.apache.org/jira/browse/MESOS-5279
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make sure 'activate' is a no-op if the client is already active.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 4306973277b9d32356eed31ceabac09fb2a03e6c 
>   src/master/allocator/sorter/sorter.hpp 
> 9e04adf54f2d80541a95f0a9a49b329dc9e8f5e3 
> 
> Diff: https://reviews.apache.org/r/47258/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 47313: Add authentication documentation to authenticated libprocess endpoints.

[Benjamin Bannier]

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

(Updated May 13, 2016, 9:25 a.m.)


Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  3rdparty/libprocess/src/logging.cpp ad7aa3aabbfc4a496e7e624a691a72e45c72e35d 
  3rdparty/libprocess/src/metrics/metrics.cpp 
65b58433158c51cebace9b591d4121e8d9a59ecc 
  3rdparty/libprocess/src/profiler.cpp 9b2f106cc7db1ff0b94c72ed55188d94799cf7df 

Diff: https://reviews.apache.org/r/47313/diff/


Testing
---

make check (OS X, clang-trunk, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 47313: Add authentication documentation to authenticated libprocess endpoints.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 13, 2016, 9:25 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47313/
> ---
> 
> (Updated May 13, 2016, 9:25 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/logging.cpp 
> ad7aa3aabbfc4a496e7e624a691a72e45c72e35d 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 65b58433158c51cebace9b591d4121e8d9a59ecc 
>   3rdparty/libprocess/src/profiler.cpp 
> 9b2f106cc7db1ff0b94c72ed55188d94799cf7df 
> 
> Diff: https://reviews.apache.org/r/47313/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 37257: Fixed FTS handle leak issue in xfs isolator.

[Guangya Liu]

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

(Updated 五月 13, 2016, 6:57 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Timothy Chen.


Summary (updated)
-

Fixed FTS handle leak issue in xfs isolator.


Repository: mesos


Description (updated)
---

Fixed FTS handle leak issue in xfs isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
80404c396ae98622a69ec290881226c1753d3626 

Diff: https://reviews.apache.org/r/37257/diff/


Testing (updated)
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 47266: Implemented provisioner removing docker whitelist files.

[Guangya Liu]

> On 五月 13, 2016, 6:33 a.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, line 375
> > 
> >
> > You seem to only fts_close on error cases, but in the normal case 
> > skipped to do so.

@tnachen, I also found a similar issue for xfs isolator and posted a patch here 
https://reviews.apache.org/r/37257/ , can you help review? Thanks.


- Guangya


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


On 五月 11, 2016, 10:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47266/
> ---
> 
> (Updated 五月 11, 2016, 10:46 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5277
> https://issues.apache.org/jira/browse/MESOS-5277
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented provisioner removing docker whitelist files.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
> 
> Diff: https://reviews.apache.org/r/47266/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with docker image containing `.wh.` files.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47313: Add authentication documentation to authenticated libprocess endpoints.

[Benjamin Bannier]

> On May 12, 2016, 6:37 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/logging.cpp, line 97
> > 
> >
> > Strictly speaking, libprocess endpoints can be both 
> > authentication-enabled and not. This depends on how libprocess is 
> > initialized.

Like we discussed offline, the issue here is that whether a libprocess endpoint 
is authorized or not depends not only if on whether the endpoint supports 
authorization, but also on the way libprocess itself was initialized. Since we 
currently expose these endpoint through either master or agent processes this 
is not an issue here as int that case libprocess endpoints will behave 
consistently with what was specified by the user when these mesos processes 
where started. If the endpoints are exposed by other processes the situation is 
less clear.

As `AUTHENTICATION` just causes some documentation to be emitted in the 
endpoint help which is currently only accessible via mesos masters/agents let's 
give the user a consistent story there and actually document the behavior for 
that case (right now the documentation is just wrong). Should cases arise where 
the documentation is really misleading we can revisit this and try to make the 
help strings more reusable across libprocess users.


- Benjamin


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


On May 12, 2016, 3:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47313/
> ---
> 
> (Updated May 12, 2016, 3:55 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add authentication documentation to authenticated libprocess endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/logging.cpp 
> ad7aa3aabbfc4a496e7e624a691a72e45c72e35d 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 65b58433158c51cebace9b591d4121e8d9a59ecc 
>   3rdparty/libprocess/src/profiler.cpp 
> 9b2f106cc7db1ff0b94c72ed55188d94799cf7df 
> 
> Diff: https://reviews.apache.org/r/47313/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 47337: Fixed erroneous `CHECK` in the command executor.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 13, 2016, 7:19 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47337/
> ---
> 
> (Updated May 13, 2016, 7:19 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There might be instances when a command might get completed
> and is not required to have an explicit `Kill` request e.g.,
> "echo hello world" etc. Unfortunately, all these cases can
> lead to the `CHECK` failing since `reaped` can be invoked
> directly.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp 
> c62fe3ee6ae06536cbb89ea208b669790efe4b39 
> 
> Diff: https://reviews.apache.org/r/47337/diff/
> 
> 
> Testing
> ---
> 
> make check (This would be tested automatically when we move to the executor 
> shim I am working on i.e. MESOS-5302)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47314: Add authentication documentation to authenticated endpoints.

[Alexander Rukletsov]

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


Fix it, then Ship it!




I'll fix the outstanding issue and commit shortly.


src/files/files.cpp (line 58)


using process::AUTHENTICATION;


- Alexander Rukletsov


On May 12, 2016, 1:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47314/
> ---
> 
> (Updated May 12, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add authentication documentation to authenticated endpoints.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/files/browse.json.md 
> ee2f48fa03f8125565c3c066c9b416ddb5fe8498 
>   docs/endpoints/files/browse.md 3060c5b8ea2814a53b80c2c2641673363d16cb12 
>   docs/endpoints/files/debug.json.md 7e367b342879632331853040971666d0f09b2f68 
>   docs/endpoints/files/debug.md 31d344637c751dd649cd068361e7abb26f457101 
>   docs/endpoints/files/download.json.md 
> 6b73f296d381ced21804bb915e9868afaf655d39 
>   docs/endpoints/files/download.md 17cdb2686ead95924266519924ad3db8073c7f54 
>   docs/endpoints/files/read.json.md c4d59679c6c52ad723204a119f17ec0cdfa8f913 
>   docs/endpoints/files/read.md 2e1ffac7fd7d0008b14047366bebb1a3a8454384 
>   docs/endpoints/logging/toggle.md f76accd1e2e0208cd991e4fb6e54f0ff0bf0d8b0 
>   docs/endpoints/metrics/snapshot.md 7d3a7ef810a6f28f9101b35241737bac59fed965 
>   docs/endpoints/profiler/start.md 36bd0b6d661692db930723e8385a9da593e98c53 
>   docs/endpoints/profiler/stop.md 085b8523e3ccc03cae971d9ce1d8e2ef6a47a411 
>   docs/endpoints/registrar/registry.md 
> b3697a6107d45d5c99c57b6e293ef4cb29dc73ba 
>   src/files/files.cpp 4e916101b378b0e9032a08a3f6c73e195b2a08a1 
>   src/master/registrar.cpp a509f45e094abdfcef71914058611ecaa5d6448e 
> 
> Diff: https://reviews.apache.org/r/47314/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 47061: Authenticated the agent's '/containers' endpoint.

[Abhishek Dasgupta]

> On May 12, 2016, 8:51 p.m., Alexander Rukletsov wrote:
> > Could you please update `Slave::Http::CONTAINERS_HELP()` as well?

Yeah, updated. Added another RR https://reviews.apache.org/r/47340/ for the 
updated doc for /containers endpoint generated by running 
support/generate-endpoint.sh.


- Abhishek


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


On May 13, 2016, 9:35 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47061/
> ---
> 
> (Updated May 13, 2016, 9:35 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5316
> https://issues.apache.org/jira/browse/MESOS-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authenticated the agent's '/containers' endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 6f4548d7817e239035e491c10899ede5679d282d 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
> 
> Diff: https://reviews.apache.org/r/47061/diff/
> 
> 
> Testing
> ---
> 
> Testing is done when testcases are modified in the next patch 
> https://reviews.apache.org/r/47062/ .
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47303: Slave/Agent terminology replacement in documentations.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47303]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 3:29 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47303/
> ---
> 
> (Updated May 13, 2016, 3:29 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3783
> https://issues.apache.org/jira/browse/MESOS-3783
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rename 'slave' in documentations with 'agent'. However, not all of
> them are renamed due to stickiness of class name, json response,
> endpoint, etc.
> 
> 
> Diffs
> -
> 
>   docs/architecture.md 3eff931a9ebb91ae06c0c7bab6c6a17d5fdb8743 
>   docs/attributes-resources.md 7d687ffcba2bc4c9cc6d83172ac852abe4174c88 
>   docs/authentication.md 1084e638c913878de6742fcd459792272689b8e1 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
>   docs/c++-style-guide.md fdd765e8a56974b25849a3509cbf99fa57fc7e27 
>   docs/committers.md 0ae0e920ab9e09ff985fd77eb5b3092b8499a3d7 
>   docs/configuration.md 34271c76d10ad930e6cc586c2b820ce8989a053a 
>   docs/container-image.md a7659de7a08d7badd185dac6bb43fb1fb6fbf5c8 
>   docs/containerizer-internals.md 6d050b9bb0c6f564e2fad83859a177bc8d742474 
>   docs/deploy-scripts.md af83d28e26b133bbf6b4fe821cb468c9d373bd57 
>   docs/docker-containerizer.md 6ec1cecd2e39bc4737688d8c4f6fc05e82c92023 
>   docs/documentation-guide.md 8239dcd908e2509797d7881d080ccbcf56f8fe4e 
>   docs/doxygen-style-guide.md 1817aa0ac7ac7dd15a3d77fcab3451c874d1ef98 
>   docs/executor-http-api.md 2b362f616d02611a8eea37b88c60aabdbca8bfd4 
>   docs/external-containerizer.md 07a795f2352a169488400c948d6bc0501e01dc5e 
>   docs/fetcher-cache-internals.md 7863fd2fc2b7ff8f45ad553b2764677ebec8a67d 
>   docs/fetcher.md b23fdf2e7ae55cc807cd2ddb3b8dc4a7d3a79eb8 
>   docs/getting-started.md 546b04cfa3aced67477cda6da18754fcd76324ba 
>   docs/high-availability-framework-guide.md 
> d5949508dafdf1a6e515663fa1524a81fe4ffba4 
>   docs/high-availability.md a8b4fce5e1bfcaec719ac7379dfc19e0ae43a928 
>   docs/home.md dc41fc4479e6c23650cd8ac78dcc4b9161d00721 
>   docs/mesos-containerizer.md 6f40c576e8402db1f56238e6a9cbacda0e166bac 
>   docs/modules.md 28eb233a30b844b302fd95c03e4ff6647355cdfa 
>   docs/monitoring.md 69d0f9848dcfed7069a8b25039cc3262bf406994 
>   docs/network-monitoring.md 1e503753a0aef127b555f072a4c8fb4768418e67 
>   docs/operational-guide.md 193cf0adf956ab9c32140875ad7434067da871b7 
>   docs/oversubscription.md 51eefb63da87607f991d7b10d6282dd73b3b092a 
>   docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 
>   docs/reconciliation.md 83cf15d1d188fc9c3a2a5b7cd1c8c8ec30c3fc6c 
>   docs/release-guide.md 69b0a0f06574fea5ec6576325698068870306ae8 
>   docs/replicated-log-internals.md a04e774e12ee411beae84ef8c739e083f3469c25 
>   docs/slave-recovery.md ff584f0bd8b25421ce50c6fdaa38f5b98d93ff69 
>   docs/ssl.md d0fcd536a4d7fb86a9f80dfe4629c22df50686b2 
>   docs/testing-patterns.md 360d1c29e3ac0e4e99dcbcabdb2204e04eda7cea 
>   docs/tools.md 11c8122ae61cb5788b2204a548a9a26650a26b97 
>   docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
> 
> Diff: https://reviews.apache.org/r/47303/diff/
> 
> 
> Testing
> ---
> 
> Draft patch. Not going to be committed until flags being renamed.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Review Request 47340: Modified doc file for '/containers'.

[Abhishek Dasgupta]

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

Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
Toenshoff.


Bugs: Mesos-5316
https://issues.apache.org/jira/browse/Mesos-5316


Repository: mesos


Description
---

This patch modifies marked down file for generating doc for endpoint 
'/containers' to include authentication requirement.


Diffs
-

  docs/endpoints/slave/containers.md ae6559b4fa360a012aab2cc5f00e4ba626a59256 

Diff: https://reviews.apache.org/r/47340/diff/


Testing
---

Viewed the marked down file in the mesos documentation website generated 
locally.


Thanks,

Abhishek Dasgupta

Re: Review Request 47313: Add authentication documentation to authenticated libprocess endpoints.

[Benjamin Bannier]

> On May 12, 2016, 6:37 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/logging.cpp, line 97
> > 
> >
> > Strictly speaking, libprocess endpoints can be both 
> > authentication-enabled and not. This depends on how libprocess is 
> > initialized.
> 
> Benjamin Bannier wrote:
> Like we discussed offline, the issue here is that whether a libprocess 
> endpoint is authorized or not depends not only if on whether the endpoint 
> supports authorization, but also on the way libprocess itself was 
> initialized. Since we currently expose these endpoint through either master 
> or agent processes this is not an issue here as int that case libprocess 
> endpoints will behave consistently with what was specified by the user when 
> these mesos processes where started. If the endpoints are exposed by other 
> processes the situation is less clear.
> 
> As `AUTHENTICATION` just causes some documentation to be emitted in the 
> endpoint help which is currently only accessible via mesos masters/agents 
> let's give the user a consistent story there and actually document the 
> behavior for that case (right now the documentation is just wrong). Should 
> cases arise where the documentation is really misleading we can revisit this 
> and try to make the help strings more reusable across libprocess users.

Filed MESOS-5379 so we can track this discrepancy.


- Benjamin


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


On May 12, 2016, 3:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47313/
> ---
> 
> (Updated May 12, 2016, 3:55 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add authentication documentation to authenticated libprocess endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/logging.cpp 
> ad7aa3aabbfc4a496e7e624a691a72e45c72e35d 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 65b58433158c51cebace9b591d4121e8d9a59ecc 
>   3rdparty/libprocess/src/profiler.cpp 
> 9b2f106cc7db1ff0b94c72ed55188d94799cf7df 
> 
> Diff: https://reviews.apache.org/r/47313/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 47339: Refactored URL path endpoint extraction.

[Jan Schlicht]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Extracting the HTTP endpoint from `URL.path` is now done in a separate
function in `Slave::Http`, making `Slave::Http::extractEndpoint`
consistent with the one in `Master::Http`.


Diffs
-

  src/slave/http.cpp db60a243c6f099c3a4cfe2503ef4a4c8d57354ce 
  src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 

Diff: https://reviews.apache.org/r/47339/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 47313: Add authentication documentation to authenticated libprocess endpoints.

[Alexander Rukletsov]

> On May 12, 2016, 4:37 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/logging.cpp, line 97
> > 
> >
> > Strictly speaking, libprocess endpoints can be both 
> > authentication-enabled and not. This depends on how libprocess is 
> > initialized.
> 
> Benjamin Bannier wrote:
> Like we discussed offline, the issue here is that whether a libprocess 
> endpoint is authorized or not depends not only if on whether the endpoint 
> supports authorization, but also on the way libprocess itself was 
> initialized. Since we currently expose these endpoint through either master 
> or agent processes this is not an issue here as int that case libprocess 
> endpoints will behave consistently with what was specified by the user when 
> these mesos processes where started. If the endpoints are exposed by other 
> processes the situation is less clear.
> 
> As `AUTHENTICATION` just causes some documentation to be emitted in the 
> endpoint help which is currently only accessible via mesos masters/agents 
> let's give the user a consistent story there and actually document the 
> behavior for that case (right now the documentation is just wrong). Should 
> cases arise where the documentation is really misleading we can revisit this 
> and try to make the help strings more reusable across libprocess users.
> 
> Benjamin Bannier wrote:
> Filed MESOS-5379 so we can track this discrepancy.

Thanks.


- Alexander


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


On May 12, 2016, 1:55 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47313/
> ---
> 
> (Updated May 12, 2016, 1:55 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add authentication documentation to authenticated libprocess endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/logging.cpp 
> ad7aa3aabbfc4a496e7e624a691a72e45c72e35d 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 65b58433158c51cebace9b591d4121e8d9a59ecc 
>   3rdparty/libprocess/src/profiler.cpp 
> 9b2f106cc7db1ff0b94c72ed55188d94799cf7df 
> 
> Diff: https://reviews.apache.org/r/47313/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 47061: Authenticated the agent's '/containers' endpoint.

[Abhishek Dasgupta]

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

(Updated May 13, 2016, 9:35 a.m.)


Review request for mesos, Adam B, Greg Mann, and Till Toenshoff.


Bugs: MESOS-5316
https://issues.apache.org/jira/browse/MESOS-5316


Repository: mesos


Description (updated)
---

Authenticated the agent's '/containers' endpoint.


Diffs (updated)
-

  src/slave/http.cpp 6f4548d7817e239035e491c10899ede5679d282d 
  src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
  src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 

Diff: https://reviews.apache.org/r/47061/diff/


Testing
---

Testing is done when testcases are modified in the next patch 
https://reviews.apache.org/r/47062/ .


Thanks,

Abhishek Dasgupta

Re: Review Request 44839: Enabled mesos containerizer force_pull_image for appc.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44837, 44838, 44839]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 3:30 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44839/
> ---
> 
> (Updated May 13, 2016, 3:30 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for appc.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 
> 
> Diff: https://reviews.apache.org/r/44839/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Alexander Rukletsov]

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




src/slave/http.cpp (line 838)


Let's do the same in the master.



src/slave/http.cpp (lines 845 - 846)


Fits one line now.



src/slave/slave.hpp (line 475)


`static`?



src/slave/slave.hpp (lines 475 - 483)


Why these functions are in the "continuations" sections? Please move them 
below _statistics() and feel free to prepend them with `// Helper routines.



src/slave/slave.hpp (lines 477 - 480)


The comment is outdated now. You can take the one from 
`Master::Http::authorizeEndpoint()` as an example.


- Alexander Rukletsov


On May 13, 2016, 9:14 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47339/
> ---
> 
> (Updated May 13, 2016, 9:14 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extracting the HTTP endpoint from `URL.path` is now done in a separate
> function in `Slave::Http`, making `Slave::Http::extractEndpoint`
> consistent with the one in `Master::Http`.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp db60a243c6f099c3a4cfe2503ef4a4c8d57354ce 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
> 
> Diff: https://reviews.apache.org/r/47339/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Jan Schlicht]

> On May 13, 2016, 12:47 p.m., Alexander Rukletsov wrote:
> > src/slave/slave.hpp, line 475
> > 
> >
> > `static`?

The function uses `slave->this().id` so it would be a bit inconvenient to make 
it static.


- Jan


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


On May 13, 2016, 11:14 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47339/
> ---
> 
> (Updated May 13, 2016, 11:14 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extracting the HTTP endpoint from `URL.path` is now done in a separate
> function in `Slave::Http`, making `Slave::Http::extractEndpoint`
> consistent with the one in `Master::Http`.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp db60a243c6f099c3a4cfe2503ef4a4c8d57354ce 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
> 
> Diff: https://reviews.apache.org/r/47339/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 38148: Fixed a typo when run 'mesos slave'.

[Guangya Liu]

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

Review request for mesos, Jay Guo and Vinod Kone.


Summary (updated)
-

Fixed a typo when run 'mesos slave'.


Repository: mesos


Description (updated)
---

Fixed a typo when run 'mesos slave'.


Diffs (updated)
-

  src/cli/mesos.cpp d5274c288be3af6c46e6587d8e471b6a35f394a4 

Diff: https://reviews.apache.org/r/38148/diff/


Testing (updated)
---

Before fix:
root@mesos002:~/src/mesos/m1/mesos/build# ./src/.libs/mesos slave
WARNING: subcommand 'slave' is deprecated in flavor of 'agent'.

After fix:
root@mesos002:~/src/mesos/m1/mesos/build# ./src/.libs/mesos slave
WARNING: subcommand 'slave' is deprecated in favor of 'agent'.


Thanks,

Guangya Liu

Re: Review Request 44154: Added appc_simple_discovery_uri_prefix to configuration.md.

[Guangya Liu]

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

(Updated 五月 13, 2016, 6:10 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.


Summary (updated)
-

Added appc_simple_discovery_uri_prefix to configuration.md.


Repository: mesos


Description (updated)
---

Added appc_simple_discovery_uri_prefix to configuration.md.


Diffs (updated)
-

  docs/configuration.md 458b7bd300e29bbb0fc97cbcacfad42a0bdef574 
  src/slave/flags.cpp 6fde51fc61cfcad61d4085c208bd2eca2eae8f14 

Diff: https://reviews.apache.org/r/44154/diff/


Testing (updated)
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 37257: Fixed FTS handle leak issue in xfs isolator.

[Timothy Chen]

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



Can you actually create a new reviewboard? The history doesn't line up at all 
and it's quite confusing.

- Timothy Chen


On May 13, 2016, 6:57 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37257/
> ---
> 
> (Updated May 13, 2016, 6:57 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed FTS handle leak issue in xfs isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 80404c396ae98622a69ec290881226c1753d3626 
> 
> Diff: https://reviews.apache.org/r/37257/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47266: Implemented provisioner removing docker whitelist files.

[Guangya Liu]

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




src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 320)


s/bind/bind and overlay


- Guangya Liu


On 五月 11, 2016, 10:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47266/
> ---
> 
> (Updated 五月 11, 2016, 10:46 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5277
> https://issues.apache.org/jira/browse/MESOS-5277
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented provisioner removing docker whitelist files.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
> 
> Diff: https://reviews.apache.org/r/47266/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with docker image containing `.wh.` files.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 46964: Using the summary attribute of the table element.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46964]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 1:59 a.m., Chen Nan Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46964/
> ---
> 
> (Updated May 13, 2016, 1:59 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and haosdent huang.
> 
> 
> Bugs: MESOS-5201
> https://issues.apache.org/jira/browse/MESOS-5201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Using the summary attribute of the table element.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> 04a4600f0c762a2412ddee078ba2c173d595aa8d 
>   src/webui/master/static/framework.html 
> 041513b0e005e8b54ca9723741b21b136ff61ca6 
>   src/webui/master/static/frameworks.html 
> f172e022e18df5b6aa3d232e610c3c732e20aa09 
>   src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
>   src/webui/master/static/offers.html 
> ec32a649239da48270a1ad1d5bf195326c31ff9d 
>   src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
>   src/webui/master/static/slave_executor.html 
> 99b23ed9e85011a66bad780fb2d3076e946821a6 
>   src/webui/master/static/slave_framework.html 
> 176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
>   src/webui/master/static/slaves.html 
> 063031771cef8b9f45723869198bad3460591936 
> 
> Diff: https://reviews.apache.org/r/46964/diff/
> 
> 
> Testing
> ---
> 
> Have checked with the patch by firebug, each table should have summary 
> attribute.
> 
> For details refer to https://www.w3.org/TR/WCAG20-TECHS/H73.html
> 
> 
> Thanks,
> 
> Chen Nan Li
> 
>

Re: Review Request 37257: Fixed FTS handle leak issue in xfs isolator.

[Guangya Liu]

> On 五月 13, 2016, 7:01 a.m., Timothy Chen wrote:
> > Can you actually create a new reviewboard? The history doesn't line up at 
> > all and it's quite confusing.

Thanks Tim. Done in https://reviews.apache.org/r/47336/ , I was updating this 
because I want to reuse the number of discard patches, it is ok discard this to 
upload a new patch.


- Guangya


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


On 五月 13, 2016, 6:57 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37257/
> ---
> 
> (Updated 五月 13, 2016, 6:57 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed FTS handle leak issue in xfs isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 80404c396ae98622a69ec290881226c1753d3626 
> 
> Diff: https://reviews.apache.org/r/37257/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 47336: Fixed FTS handle leak issue in xfs isolator.

[Guangya Liu]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Fixed FTS handle leak issue in xfs isolator.


Diffs
-

  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
80404c396ae98622a69ec290881226c1753d3626 

Diff: https://reviews.apache.org/r/47336/diff/


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 47324: Update leveldb.

[Zhiwei Chen]

> On May 13, 2016, 12:38 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [47324]
> > 
> > Failed command: ./support/apply-review.sh -n -r 47324
> > 
> > Error:
> > 2016-05-13 04:38:10 URL:https://reviews.apache.org/r/47324/diff/raw/ 
> > [4739/4739] -> "47324.patch" [1]
> > error: missing binary patch data for '3rdparty/leveldb-1.18.tar.gz'
> > error: binary patch does not apply to '3rdparty/leveldb-1.18.tar.gz'
> > error: 3rdparty/leveldb-1.18.tar.gz: patch does not apply
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/13038/console

The rbtools does not support binary patch, please apply this patch to your 
rbtools. https://reviews.reviewboard.org/r/7571/diff/1#index_header

Or use git diff --binary to generate a patch file and upload it through 
reviewboard Web UI.

I will help you test on ppc64le when you fix this.


- Zhiwei


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


On May 13, 2016, 4:46 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47324/
> ---
> 
> (Updated May 13, 2016, 4:46 a.m.)
> 
> 
> Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-970
> https://issues.apache.org/jira/browse/MESOS-970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update leveldb to 1.18.
> Remove patch that is already included in latest leveldb release.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 86c51edb3aa2daf6451459aaf18278f09b91b000 
>   3rdparty/leveldb-1.18.tar.gz PRE-CREATION 
>   3rdparty/leveldb-1.4.patch b899f0141d633b1ffb2321e573395256fc893b16 
>   3rdparty/leveldb-1.4.tar.gz 2ddbc0c2e02054406ff0ea43ddc10d14979de8d8 
>   3rdparty/versions.am 7dcd6bf914de3213755ec9d4e701a190750424e9 
>   LICENSE eb39f6d69a165f59c00e8bb0ba9e15be8c958a5b 
>   src/python/native_common/ext_modules.py.in 
> 2d4a45efa224b32f80ace4542a00062c5ccb06d5 
> 
> Diff: https://reviews.apache.org/r/47324/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu amd_64 need to test on PPC
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47324: Update leveldb.

[haosdent huang]

> On May 13, 2016, 4:38 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [47324]
> > 
> > Failed command: ./support/apply-review.sh -n -r 47324
> > 
> > Error:
> > 2016-05-13 04:38:10 URL:https://reviews.apache.org/r/47324/diff/raw/ 
> > [4739/4739] -> "47324.patch" [1]
> > error: missing binary patch data for '3rdparty/leveldb-1.18.tar.gz'
> > error: binary patch does not apply to '3rdparty/leveldb-1.18.tar.gz'
> > error: 3rdparty/leveldb-1.18.tar.gz: patch does not apply
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/13038/console
> 
> Zhiwei Chen wrote:
> The rbtools does not support binary patch, please apply this patch to 
> your rbtools. https://reviews.reviewboard.org/r/7571/diff/1#index_header
> 
> Or use git diff --binary to generate a patch file and upload it through 
> reviewboard Web UI.
> 
> I will help you test on ppc64le when you fix this.

Thank you so much for your kindly help. @zhiwei. Sorry to forgot remind @janisz 
before. I think we may document this at the `submitting a patch`. :-)


- haosdent


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


On May 12, 2016, 8:46 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47324/
> ---
> 
> (Updated May 12, 2016, 8:46 p.m.)
> 
> 
> Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-970
> https://issues.apache.org/jira/browse/MESOS-970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update leveldb to 1.18.
> Remove patch that is already included in latest leveldb release.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 86c51edb3aa2daf6451459aaf18278f09b91b000 
>   3rdparty/leveldb-1.18.tar.gz PRE-CREATION 
>   3rdparty/leveldb-1.4.patch b899f0141d633b1ffb2321e573395256fc893b16 
>   3rdparty/leveldb-1.4.tar.gz 2ddbc0c2e02054406ff0ea43ddc10d14979de8d8 
>   3rdparty/versions.am 7dcd6bf914de3213755ec9d4e701a190750424e9 
>   LICENSE eb39f6d69a165f59c00e8bb0ba9e15be8c958a5b 
>   src/python/native_common/ext_modules.py.in 
> 2d4a45efa224b32f80ace4542a00062c5ccb06d5 
> 
> Diff: https://reviews.apache.org/r/47324/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu amd_64 need to test on PPC
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47336: Fixed FTS handle leak issue in xfs isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47336]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 7:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47336/
> ---
> 
> (Updated May 13, 2016, 7:09 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed FTS handle leak issue in xfs isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 80404c396ae98622a69ec290881226c1753d3626 
> 
> Diff: https://reviews.apache.org/r/47336/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47062: Modified testcase for authentication on '/containers' endpoint.

[Abhishek Dasgupta]

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

(Updated May 13, 2016, 2:24 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
Toenshoff.


Bugs: MESOS-5316
https://issues.apache.org/jira/browse/MESOS-5316


Repository: mesos


Description (updated)
---

Testcase SlaveTest.HTTPEndpointsBadAuthentication is updated to use
authentication when hitting '/containers' endpoint as well as when
bad credentials are provided.


Diffs
-

  src/tests/slave_tests.cpp 3f653354869987dce3f5fbc4513b6f3466a718cb 

Diff: https://reviews.apache.org/r/47062/diff/


Testing
---

sudo make -j2 check 
On Ubuntu 16.04


Thanks,

Abhishek Dasgupta

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Alexander Rukletsov]

> On May 13, 2016, 10:47 a.m., Alexander Rukletsov wrote:
> > src/slave/slave.hpp, line 475
> > 
> >
> > `static`?
> 
> Jan Schlicht wrote:
> The function uses `slave->this().id` so it would be a bit inconvenient to 
> make it static.

I see.


- Alexander


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


On May 13, 2016, 9:14 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47339/
> ---
> 
> (Updated May 13, 2016, 9:14 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extracting the HTTP endpoint from `URL.path` is now done in a separate
> function in `Slave::Http`, making `Slave::Http::extractEndpoint`
> consistent with the one in `Master::Http`.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp db60a243c6f099c3a4cfe2503ef4a4c8d57354ce 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
> 
> Diff: https://reviews.apache.org/r/47339/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Jan Schlicht]

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

(Updated May 13, 2016, 4:12 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Addressed Alex's issues (added `extractEndpoint` to `Master::Http`)


Repository: mesos


Description
---

Extracting the HTTP endpoint from `URL.path` is now done in a separate
function in `Slave::Http`, making `Slave::Http::extractEndpoint`
consistent with the one in `Master::Http`.


Diffs (updated)
-

  src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
  src/master/master.hpp bd0fa9870199bb8792f6c2b8c24ad93da4f027e0 
  src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
  src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 

Diff: https://reviews.apache.org/r/47339/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Alexander Rukletsov]

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



Looks good. Could you please make this patch self-contained, i.e., remove 
dependency on /r/47222?

Did you manually test that authorization works? I mean setup ACLs, start a 
master, force-set some quotas and check that they are being properly filtered? 
Could you please do it and describe in the "testing done" section?


include/mesos/authorizer/acls.proto (line 206)


Let's move this line up, before `SetQuota`



include/mesos/authorizer/authorizer.proto (line 62)


Ditto.



src/authorizer/local/authorizer.cpp (lines 219 - 229)


Ditto (move up before set quota).



src/master/master.hpp (line 1013)


This can be simply named `authorizeGetQuota` after you remove 
coarse-grained authz.



src/master/master.hpp (line 1024)


Once you remove coarse-grained authz, you can prepare the authz action in 
`status()` and hence you don't need `principal` in `_status()`, right?



src/master/quota_handler.cpp (lines 441 - 447)


Do we really need this copy? Or we can directly use `master->quotas`?



src/master/quota_handler.cpp (lines 449 - 450)


Please blank line between comment sections, e.g.:
```
// The quick, brown fox jumps over a lazy dog. DJs flock by when MTV
// ax quiz prog. Junk MTV quiz graced by fox whelps. Bawds jog, flick
// quartz, vex nymphs.
//
// NOTE: Waltz, bad nymph, for quick jigs vex! Fox nymphs grab quick-jived
// waltz. Brick quiz whangs jumpy veldt fox. Bright vixens jump; fowl quack.
//
// TODO(zhitao): Quick wafting zephyrs vex bold Jim. Quick zephyrs blow,
// vexing daft Jim. Sex-charged fop blew my junk TV quiz. How quickly daft
// jumping zebras vex. Two driven jocks help fax my big quiz.
```



src/master/quota_handler.cpp (line 454)


We indent by 4 spaces if the previous line was a function call ending by '('



src/master/quota_handler.cpp (lines 459 - 470)


Let's make it a continuation. Since you'll be moving this code to 
`status()`, you can reuse `_status()` for continuation.



src/master/quota_handler.cpp (line 463)


How about a comment here:
```
// Create an entry (including role and resources) for each quota,
// except those filtered out based on the authorizer's response.
```



src/master/quota_handler.cpp (line 464)


If we eliminate intermediate vector, we will be looping thorugh hashmap and 
list simultaneously. I can't think of a nice way to do it; we should probably 
use `for` with iterators.



src/master/quota_handler.cpp (line 519)


Blank line



src/tests/master_quota_tests.cpp (line 1301)


"previous authorized quota"? Do you want to say "previously requested 
quota"?


- Alexander Rukletsov


On May 12, 2016, 12:48 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 12, 2016, 12:48 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 46428: Fixed the broken Docker Volume Rootfs Test on Centos7.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/tests/containerizer/docker_volume_isolator_tests.cpp (line 801)


s/volumes/volume/g



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 803)


s/volumes/volume/g



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 939)


s/volumes/volume/g



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 941)


s/volumes/volume/g


- haosdent huang


On May 10, 2016, 11:49 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46428/
> ---
> 
> (Updated May 10, 2016, 11:49 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5351
> https://issues.apache.org/jira/browse/MESOS-5351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the broken Docker Volume Rootfs Test on Centos7.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 
> 
> Diff: https://reviews.apache.org/r/46428/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from DockerVolumeIsolatorTest
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
>  (1732 ms)
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
>  (1708 ms)
> [--] 2 tests from DockerVolumeIsolatorTest (3451 ms total)
>  
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (3460 ms total)
> [  PASSED  ] 2 tests.
> [root@mesos-24 build]# cat /etc/*release
> CentOS Linux release 7.2.1511 (Core) 
> NAME="CentOS Linux"
> VERSION="7 (Core)"
> ID="centos"
> ID_LIKE="rhel fedora"
> VERSION_ID="7"
> PRETTY_NAME="CentOS Linux 7 (Core)"
> ANSI_COLOR="0;31"
> CPE_NAME="cpe:/o:centos:centos:7"
> HOME_URL="https://www.centos.org/;
> BUG_REPORT_URL="https://bugs.centos.org/;
>  
> CENTOS_MANTISBT_PROJECT="CentOS-7"
> CENTOS_MANTISBT_PROJECT_VERSION="7"
> REDHAT_SUPPORT_PRODUCT="centos"
> REDHAT_SUPPORT_PRODUCT_VERSION="7"
>  
> CentOS Linux release 7.2.1511 (Core) 
> CentOS Linux release 7.2.1511 (Core) 
> [root@mesos-24 build]#
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47061: Authenticated the agent's '/containers' endpoint.

[Till Toenshoff]

> On May 13, 2016, 2:59 p.m., Till Toenshoff wrote:
> > Ship It!

Minus the "debugging artefact?" Alex noted.


- Till


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


On May 13, 2016, 9:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47061/
> ---
> 
> (Updated May 13, 2016, 9:50 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5316
> https://issues.apache.org/jira/browse/MESOS-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authenticated the agent's '/containers' endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 6f4548d7817e239035e491c10899ede5679d282d 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
> 
> Diff: https://reviews.apache.org/r/47061/diff/
> 
> 
> Testing
> ---
> 
> Testing is done when testcases are modified in the next patch 
> https://reviews.apache.org/r/47062/ .
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47061: Authenticated the agent's '/containers' endpoint.

[Till Toenshoff]

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


Ship it!




Ship It!

- Till Toenshoff


On May 13, 2016, 9:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47061/
> ---
> 
> (Updated May 13, 2016, 9:50 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5316
> https://issues.apache.org/jira/browse/MESOS-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authenticated the agent's '/containers' endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 6f4548d7817e239035e491c10899ede5679d282d 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
> 
> Diff: https://reviews.apache.org/r/47061/diff/
> 
> 
> Testing
> ---
> 
> Testing is done when testcases are modified in the next patch 
> https://reviews.apache.org/r/47062/ .
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47340: Modified doc file for '/containers'.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47340, 47062, 47061]

Failed command: ./support/apply-review.sh -n -r 47062

Error:
2016-05-13 13:52:46 URL:https://reviews.apache.org/r/47062/diff/raw/ 
[1646/1646] -> "47062.patch" [1]
Total errors found: 0
Checking 1 files
Error: No line in the commit message summary may exceed 72 characters.

Full log: https://builds.apache.org/job/mesos-reviewbot/13046/console

- Mesos ReviewBot


On May 13, 2016, 9:46 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47340/
> ---
> 
> (Updated May 13, 2016, 9:46 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: Mesos-5316
> https://issues.apache.org/jira/browse/Mesos-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies marked down file for generating doc for endpoint 
> '/containers' to include authentication requirement.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md ae6559b4fa360a012aab2cc5f00e4ba626a59256 
> 
> Diff: https://reviews.apache.org/r/47340/diff/
> 
> 
> Testing
> ---
> 
> Viewed the marked down file in the mesos documentation website generated 
> locally.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Alexander Rukletsov]

> On May 13, 2016, 1:16 p.m., Alexander Rukletsov wrote:
> > src/slave/http.cpp, line 855
> > 
> >
> > Do we need the first check given we explicitly requested `tokenize()` 
> > to split endpoint into two parts?
> 
> Jan Schlicht wrote:
> Size could be 0 or 1 if the input string doesn't look as expected -- 
> exactly what we're testing for here.

Maybe then s/!=/<=/ for documenting our intention?


- Alexander


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


On May 13, 2016, 2:12 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47339/
> ---
> 
> (Updated May 13, 2016, 2:12 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extracting the HTTP endpoint from `URL.path` is now done in a separate
> function in `Slave::Http`, making `Slave::Http::extractEndpoint`
> consistent with the one in `Master::Http`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
>   src/master/master.hpp bd0fa9870199bb8792f6c2b8c24ad93da4f027e0 
>   src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
> 
> Diff: https://reviews.apache.org/r/47339/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 47061: Authenticated the agent's '/containers' endpoint.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/slave/slave.cpp (line 766)


Uups, is it intended? If yes, please do it in a separate patch.


- Alexander Rukletsov


On May 13, 2016, 9:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47061/
> ---
> 
> (Updated May 13, 2016, 9:50 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5316
> https://issues.apache.org/jira/browse/MESOS-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authenticated the agent's '/containers' endpoint.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 6f4548d7817e239035e491c10899ede5679d282d 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
> 
> Diff: https://reviews.apache.org/r/47061/diff/
> 
> 
> Testing
> ---
> 
> Testing is done when testcases are modified in the next patch 
> https://reviews.apache.org/r/47062/ .
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47340: Modified doc file for '/containers'.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 13, 2016, 2:23 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47340/
> ---
> 
> (Updated May 13, 2016, 2:23 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: Mesos-5316
> https://issues.apache.org/jira/browse/Mesos-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies marked down file for generating doc for endpoint
> '/containers' to include authentication requirement.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md ae6559b4fa360a012aab2cc5f00e4ba626a59256 
> 
> Diff: https://reviews.apache.org/r/47340/diff/
> 
> 
> Testing
> ---
> 
> Viewed the marked down file in the mesos documentation website generated 
> locally.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47062: Modified testcase for authentication on '/containers' endpoint.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 13, 2016, 2:24 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47062/
> ---
> 
> (Updated May 13, 2016, 2:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5316
> https://issues.apache.org/jira/browse/MESOS-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Testcase SlaveTest.HTTPEndpointsBadAuthentication is updated to use
> authentication when hitting '/containers' endpoint as well as when
> bad credentials are provided.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 3f653354869987dce3f5fbc4513b6f3466a718cb 
> 
> Diff: https://reviews.apache.org/r/47062/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j2 check 
> On Ubuntu 16.04
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Jan Schlicht]

> On May 13, 2016, 3:16 p.m., Alexander Rukletsov wrote:
> > src/slave/http.cpp, line 855
> > 
> >
> > Do we need the first check given we explicitly requested `tokenize()` 
> > to split endpoint into two parts?
> 
> Jan Schlicht wrote:
> Size could be 0 or 1 if the input string doesn't look as expected -- 
> exactly what we're testing for here.
> 
> Alexander Rukletsov wrote:
> Maybe then s/!=/<=/ for documenting our intention?

Yeah, makes sense. The `!=2` could be a leftover from an older version that 
didn't use `tokenize()` and where values >2 were possible.


- Jan


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


On May 13, 2016, 4:12 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47339/
> ---
> 
> (Updated May 13, 2016, 4:12 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extracting the HTTP endpoint from `URL.path` is now done in a separate
> function in `Slave::Http`, making `Slave::Http::extractEndpoint`
> consistent with the one in `Master::Http`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
>   src/master/master.hpp bd0fa9870199bb8792f6c2b8c24ad93da4f027e0 
>   src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
> 
> Diff: https://reviews.apache.org/r/47339/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 47222: Add authorization to /quota endpoint.

[Alexander Rukletsov]

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



Let's punt on the coarse-grained authz for now.

- Alexander Rukletsov


On May 12, 2016, 4:21 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47222/
> ---
> 
> (Updated May 12, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add authorization to /quota endpoint.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a0d67671c89b9794e721dc3ba012acd3bdc447e3 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47222/diff/
> 
> 
> Testing
> ---
> 
> Adding tests in MasterQuotaTest.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47340: Modified doc file for '/containers'.

[Abhishek Dasgupta]

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

(Updated May 13, 2016, 2:23 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
Toenshoff.


Bugs: Mesos-5316
https://issues.apache.org/jira/browse/Mesos-5316


Repository: mesos


Description (updated)
---

This patch modifies marked down file for generating doc for endpoint
'/containers' to include authentication requirement.


Diffs
-

  docs/endpoints/slave/containers.md ae6559b4fa360a012aab2cc5f00e4ba626a59256 

Diff: https://reviews.apache.org/r/47340/diff/


Testing
---

Viewed the marked down file in the mesos documentation website generated 
locally.


Thanks,

Abhishek Dasgupta

Re: Review Request 38148: Fixed a typo when run 'mesos slave'.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [38148]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 10:08 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38148/
> ---
> 
> (Updated May 13, 2016, 10:08 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo when run 'mesos slave'.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp d5274c288be3af6c46e6587d8e471b6a35f394a4 
> 
> Diff: https://reviews.apache.org/r/38148/diff/
> 
> 
> Testing
> ---
> 
> Before fix:
> root@mesos002:~/src/mesos/m1/mesos/build# ./src/.libs/mesos slave
> WARNING: subcommand 'slave' is deprecated in flavor of 'agent'.
> 
> After fix:
> root@mesos002:~/src/mesos/m1/mesos/build# ./src/.libs/mesos slave
> WARNING: subcommand 'slave' is deprecated in favor of 'agent'.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47062: Modified testcase for authentication on '/containers' endpoint.

[Till Toenshoff]

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



I am not convinced that this test adds anything. libprocess handles the 
authentication and hence testing a single endpoint as an integration test seems 
to get us all the information we need -> "have we properly integrated the 
http-endpoint-authentication of libprocess into the slave?". 

Note that a failed authentication will never hit the implementation of the 
(slave) endpoint itself, hence we will not get any endpoint implementation 
specific information out of such test.

Adding more endpoints into this test IMHO does not add any value and I believe 
we should remove the second one and stick with a single one - e.g. /state or 
ANY other randomly chosen one.

- Till Toenshoff


On May 13, 2016, 2:24 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47062/
> ---
> 
> (Updated May 13, 2016, 2:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5316
> https://issues.apache.org/jira/browse/MESOS-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Testcase SlaveTest.HTTPEndpointsBadAuthentication is updated to use
> authentication when hitting '/containers' endpoint as well as when
> bad credentials are provided.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 3f653354869987dce3f5fbc4513b6f3466a718cb 
> 
> Diff: https://reviews.apache.org/r/47062/diff/
> 
> 
> Testing
> ---
> 
> sudo make -j2 check 
> On Ubuntu 16.04
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 41632: Windows: Added Console Ctrl handling in `slave.cpp`.

[Joris Van Remoortere]

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


Fix it, then Ship it!





src/slave/slave.cpp (lines 799 - 805)


i just rewrapped these a little.


- Joris Van Remoortere


On May 12, 2016, 10:31 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41632/
> ---
> 
> (Updated May 12, 2016, 10:31 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5373
> https://issues.apache.org/jira/browse/MESOS-5373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added Console Ctrl handling in `slave.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/posix_signalhandler.hpp PRE-CREATION 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
>   src/slave/windows_ctrlhandler.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41632/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: make
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47055: Make the proper contrast ratio in web page for accessibility.

[haosdent huang]

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




src/webui/master/static/css/mesos.css (line 2)


I think could put this overwritten section at the end of file. And add 
comments to descript it is used to increase contrast ratio.



src/webui/master/static/css/mesos.css (line 6)


use 2 space indent here.



src/webui/master/static/css/mesos.css (line 9)


We need adjust the style to

```
.pagination > li > a,
.pagination > li > span {
  color: #005580;
}

.pagination > .disabled > a,
.pagination > .disabled > a:focus,
.pagination > .disabled > a:hover,
.pagination > .disabled > span,
.pagination > .disabled > span:focus,
.pagination > .disabled > span:hover {
...
```

to make it more readable and avoid the line length limit.



src/webui/master/static/css/mesos.css (line 17)


`span:hover{` should be `span:hover {`, missing a space here.



src/webui/master/static/css/mesos.css (line 21)


I perfer to move `a` on the front of this section, to follow the order in 
bootstrap.



src/webui/master/static/css/mesos.css (line 30)


Could not understand why change from `#ff` to `#555` here. I think you 
may want to change `background-color: #99;` to `#555` here?



src/webui/master/static/css/mesos.css (line 42)


use 2 space indent here.


- haosdent huang


On May 6, 2016, 8:58 a.m., Chen Nan Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47055/
> ---
> 
> (Updated May 6, 2016, 8:58 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and haosdent huang.
> 
> 
> Bugs: MESOS-5204
> https://issues.apache.org/jira/browse/MESOS-5204
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Make the proper contrast ratio in web page for accessibility.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/css/mesos.css 
> 5b1227e9d64757f9fc106e497f7fa3ed72112c10 
> 
> Diff: https://reviews.apache.org/r/47055/diff/
> 
> 
> Testing
> ---
> 
> Ensuring that a contrast ratio of at least 4.5:1 exists between text (and 
> images of text) and background behind the text.
> 
> refer to https://www.w3.org/TR/WCAG20-TECHS/G18.html
> 
> 
> Thanks,
> 
> Chen Nan Li
> 
>

Re: Review Request 41632: Windows: Added Console Ctrl handling in `slave.cpp`.

[Daniel Pravat]

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

(Updated May 13, 2016, 3:40 p.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
Van Remoortere, and Michael Park.


Bugs: MESOS-5374
https://issues.apache.org/jira/browse/MESOS-5374


Repository: mesos


Description
---

Windows: Added Console Ctrl handling in `slave.cpp`.


Diffs
-

  src/slave/posix_signalhandler.hpp PRE-CREATION 
  src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
  src/slave/windows_ctrlhandler.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/41632/diff/


Testing
---

OSX: make check
Windows: make


Thanks,

Daniel Pravat

Re: Review Request 46392: Windows: Added stub implementations of `fcntl.hpp` functions.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/fcntl.hpp (line 26)


can we say `not supported`? here and below.



3rdparty/stout/include/stout/os/windows/fcntl.hpp (line 44)


space between `){`



3rdparty/stout/include/stout/os/windows/fcntl.hpp (line 45)


How about `non_block_mode`?



3rdparty/stout/include/stout/os/windows/fcntl.hpp (lines 57 - 64)


Don't need the else, can just follow through as the previous error checking 
code returns.



3rdparty/stout/include/stout/os/windows/fcntl.hpp (line 62)


don't need the semi-colon?


- Joris Van Remoortere


On May 12, 2016, 6:59 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46392/
> ---
> 
> (Updated May 12, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-5371
> https://issues.apache.org/jira/browse/MESOS-5371
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces temporary versions of 2 important functions:
> `os::nonblock` and `os::cloexec`. We put them here in a placeholder
> commit so that reviewers can make progress on subprocess. In the
> immediate term, the plan is to figure out on a callsite-by-callsite
> basis how to work around the functionality of `os::cloexec`. When we
> collect more data, we will be in a better position to offer a way
> forward here.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os.hpp 
> 25e93200cf8b95f57931771073f3b6e34fff061b 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> 14734317d7fb40053ee808745ac3ba8c706a7669 
> 
> Diff: https://reviews.apache.org/r/46392/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38148: Fixed a typo when run 'mesos slave'.

[Jay Guo]

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


Ship it!




Ship It!

- Jay Guo


On May 13, 2016, 10:08 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38148/
> ---
> 
> (Updated May 13, 2016, 10:08 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo when run 'mesos slave'.
> 
> 
> Diffs
> -
> 
>   src/cli/mesos.cpp d5274c288be3af6c46e6587d8e471b6a35f394a4 
> 
> Diff: https://reviews.apache.org/r/38148/diff/
> 
> 
> Testing
> ---
> 
> Before fix:
> root@mesos002:~/src/mesos/m1/mesos/build# ./src/.libs/mesos slave
> WARNING: subcommand 'slave' is deprecated in flavor of 'agent'.
> 
> After fix:
> root@mesos002:~/src/mesos/m1/mesos/build# ./src/.libs/mesos slave
> WARNING: subcommand 'slave' is deprecated in favor of 'agent'.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47221: Windows: Used `os::random` in libprocess.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/libprocess/src/tests/sequence_tests.cpp (line 258)


missing `` include?


- Joris Van Remoortere


On May 12, 2016, 10:19 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47221/
> ---
> 
> (Updated May 12, 2016, 10:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5372
> https://issues.apache.org/jira/browse/MESOS-5372
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Used `os::random` in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/sequence_tests.cpp 
> 566393fbf3f19644b86843f07194d1de36a2015e 
> 
> Diff: https://reviews.apache.org/r/47221/diff/
> 
> 
> Testing
> ---
> 
> OSX: make ckeck
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47222, 47274]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 12, 2016, 12:48 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 12, 2016, 12:48 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Alexander Rukletsov]

> On May 13, 2016, 2:17 p.m., Alexander Rukletsov wrote:
> > src/master/quota_handler.cpp, line 464
> > 
> >
> > If we eliminate intermediate vector, we will be looping thorugh hashmap 
> > and list simultaneously. I can't think of a nice way to do it; we should 
> > probably use `for` with iterators.

Though we need the intermediate collection, I'd still say having the same type 
(`list`) and avoid using indices can make the code more readable. What'd you 
say?


- Alexander


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


On May 12, 2016, 12:48 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 12, 2016, 12:48 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Jan Schlicht]

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

(Updated May 13, 2016, 4:54 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Addressed Alex's issues.


Repository: mesos


Description
---

Extracting the HTTP endpoint from `URL.path` is now done in a separate
function in `Slave::Http`, making `Slave::Http::extractEndpoint`
consistent with the one in `Master::Http`.


Diffs (updated)
-

  src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
  src/master/master.hpp bd0fa9870199bb8792f6c2b8c24ad93da4f027e0 
  src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
  src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 

Diff: https://reviews.apache.org/r/47339/diff/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Alexander Rukletsov]

> On May 13, 2016, 2:17 p.m., Alexander Rukletsov wrote:
> > src/master/quota_handler.cpp, lines 441-447
> > 
> >
> > Do we really need this copy? Or we can directly use `master->quotas`?

After a second thought, most probably we need a copy here because 
`master->quotas` may change when we process authz response in the continuation.


- Alexander


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


On May 12, 2016, 12:48 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 12, 2016, 12:48 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47336: Fixed FTS handle leak issue in xfs isolator.

[Timothy Chen]

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


Ship it!




Ship It!

- Timothy Chen


On May 13, 2016, 7:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47336/
> ---
> 
> (Updated May 13, 2016, 7:09 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed FTS handle leak issue in xfs isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 80404c396ae98622a69ec290881226c1753d3626 
> 
> Diff: https://reviews.apache.org/r/47336/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46424: Windows: Added libprocess to build.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On May 12, 2016, 6:06 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46424/
> ---
> 
> (Updated May 12, 2016, 6:06 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3103
> https://issues.apache.org/jira/browse/MESOS-3103
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added libprocess to build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/CMakeLists.txt 5633c395bcb3b3ce377193c1ca1d6d9810c97852 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 9558adf1e45043630b637fa6fb21dc92ec036bd4 
> 
> Diff: https://reviews.apache.org/r/46424/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Zhitao Li]

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




src/master/quota_handler.cpp (line 464)


I'd like to keep using vector because we know the expected size in the 
beginning and can reserve the capacity beforehand. It's slightly more memory 
efficient than list (which requires one memory per push_back IIUIC).

I can still avoid index by using vector::const_iterator to loop in it. It 
should be no difference interface wise.


- Zhitao Li


On May 12, 2016, 12:48 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 12, 2016, 12:48 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47168: Windows: Implemented `kill`.

[Joris Van Remoortere]

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




3rdparty/stout/include/stout/os/windows/kill.hpp (lines 47 - 48)


new line between closing function and closing namespace



3rdparty/stout/include/stout/windows.hpp (line 20)


Missing period `.`.


- Joris Van Remoortere


On May 13, 2016, 4:01 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47168/
> ---
> 
> (Updated May 13, 2016, 4:01 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5375
> https://issues.apache.org/jira/browse/MESOS-5375
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `kill`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 9b39ce32c0269479066cf7991afaeed65d8ab547 
>   3rdparty/stout/include/stout/os/kill.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/kill.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/kill.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/windows.hpp 
> a7a59e78575e1456b4e14d18ac97f51dd23d794e 
> 
> Diff: https://reviews.apache.org/r/47168/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/test w/ Marathon
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47162: Libprocess: Made some of the tests work on Windows.

[Joris Van Remoortere]

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




3rdparty/libprocess/src/tests/main.cpp (lines 45 - 63)


Can we add little comments before these conditional compilation blocks?



3rdparty/libprocess/src/tests/subprocess_tests.cpp (line 56)


Can we use snake_case please?



3rdparty/libprocess/src/tests/subprocess_tests.cpp (line 107)


How about `close_fd`?
Also, why can't we use `os::close` here? Do we even need the lambda then?



3rdparty/libprocess/src/tests/subprocess_tests.cpp (lines 115 - 116)


Can you look through your review and remove the `std::`?



3rdparty/libprocess/src/tests/subprocess_tests.cpp (lines 212 - 216)


We don't need the `std::`
Do we need to mention map a second time?
I think you can just initialize after the `=`.



3rdparty/libprocess/src/tests/subprocess_tests.cpp (lines 226 - 231)


indentation.



3rdparty/libprocess/src/tests/subprocess_tests.cpp (line 236)


ASSERT_SOME if you're assuming that state in the next line.
Here and elsewhere.



3rdparty/libprocess/src/tests/subprocess_tests.cpp (lines 730 - 731)


Can we add a comment explaining why we're not running this on windows?


- Joris Van Remoortere


On May 12, 2016, 6:05 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47162/
> ---
> 
> (Updated May 12, 2016, 6:05 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3371
> https://issues.apache.org/jira/browse/MESOS-3371
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Made some of the tests work on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/main.cpp 
> c2ce9bc7830cd7bb0aa473f985106ec745207bf8 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 727e940f12643974de4ff2734fba431b285b5de3 
> 
> Diff: https://reviews.apache.org/r/47162/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47169: Windows: Implemented `killtree` using NT job objects.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/killtree.hpp (line 34)


Do we need to manually convert to `Option`?



3rdparty/stout/include/stout/os/windows/killtree.hpp (lines 47 - 48)


new line


- Joris Van Remoortere


On May 13, 2016, 5:23 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47169/
> ---
> 
> (Updated May 13, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3639
> https://issues.apache.org/jira/browse/MESOS-3639
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented  `killtree` using NT job objects.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 
> b075d625541ed6c10192e3e98bf399b38b69cdc5 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 261768eace6ab09956f4a80e1ec5dba988d831e1 
> 
> Diff: https://reviews.apache.org/r/47169/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/test with Marathon
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47336: Fixed FTS handle leak issue in xfs isolator.

[James Peach]

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


Ship it!




Looks fine. Consider switching to ``unique_ptr``, since we only check the 
``fts_close`` error 1 of 3 times.

- James Peach


On May 13, 2016, 7:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47336/
> ---
> 
> (Updated May 13, 2016, 7:09 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed FTS handle leak issue in xfs isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 80404c396ae98622a69ec290881226c1753d3626 
> 
> Diff: https://reviews.apache.org/r/47336/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46392: Windows: Added stub implementations of `fcntl.hpp` functions.

[Alex Clemmer]

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

(Updated May 13, 2016, 5:22 p.m.)


Review request for mesos.


Bugs: MESOS-5371
https://issues.apache.org/jira/browse/MESOS-5371


Repository: mesos


Description
---

This commit introduces temporary versions of 2 important functions:
`os::nonblock` and `os::cloexec`. We put them here in a placeholder
commit so that reviewers can make progress on subprocess. In the
immediate term, the plan is to figure out on a callsite-by-callsite
basis how to work around the functionality of `os::cloexec`. When we
collect more data, we will be in a better position to offer a way
forward here.


Diffs (updated)
-

  3rdparty/stout/include/stout/os.hpp 25e93200cf8b95f57931771073f3b6e34fff061b 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
14734317d7fb40053ee808745ac3ba8c706a7669 

Diff: https://reviews.apache.org/r/46392/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47339: Refactored URL path endpoint extraction.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 13, 2016, 2:54 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47339/
> ---
> 
> (Updated May 13, 2016, 2:54 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extracting the HTTP endpoint from `URL.path` is now done in a separate
> function in `Slave::Http`, making `Slave::Http::extractEndpoint`
> consistent with the one in `Master::Http`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
>   src/master/master.hpp bd0fa9870199bb8792f6c2b8c24ad93da4f027e0 
>   src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
> 
> Diff: https://reviews.apache.org/r/47339/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46799: Added capabilities support to mesos-execute.

[Jojy Varghese]

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

(Updated May 13, 2016, 4:46 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

This change introduces two flags: `user` and `capabilities`.

   user:  used to specify the user name for the command.
   capabilities: comma separated list of capabilities the task
 requires.


Diffs (updated)
-

  src/cli/execute.cpp 4711e80314e2fc4cde077bebd9a8167324c3254e 

Diff: https://reviews.apache.org/r/46799/diff/


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 47199: Implemented parsing docker labels in v1 spec.

[Gilbert Song]

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

(Updated May 13, 2016, 1:49 p.m.)


Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and Kevin 
Klues.


Bugs: MESOS-5272
https://issues.apache.org/jira/browse/MESOS-5272


Repository: mesos


Description
---

Implemented parsing docker labels in v1 spec.


Diffs (updated)
-

  src/docker/spec.cpp ac28331a17edb8c2ff81d5a2f79a794f869a3e5d 

Diff: https://reviews.apache.org/r/47199/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47336: Fixed FTS handle leak issue in xfs isolator.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On May 13, 2016, 12:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47336/
> ---
> 
> (Updated May 13, 2016, 12:09 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed FTS handle leak issue in xfs isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 80404c396ae98622a69ec290881226c1753d3626 
> 
> Diff: https://reviews.apache.org/r/47336/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[Benjamin Mahler]

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



This looks good but when you mentioned the consecutive failures in the 
description I was confused. The test should probably just say that we launch a 
task that toggles between healthy and unhealthy, and will never be killed 
because no consecutive health failures occur. That will make it clear that we 
need to ignore subsequent status updates because we'll continue to receive 
healthy/unhealthy updates.

Could you update the description to clarify this?


src/tests/health_check_tests.cpp (line 504)


Why did this change?


- Benjamin Mahler


On May 7, 2016, 8:59 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46307/
> ---
> 
> (Updated May 7, 2016, 8:59 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> Neil Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-1802
> https://issues.apache.org/jira/browse/MESOS-1802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to ignore subsequent status updates in HealthStatusChange
> tests. In our test cases, we set `consecutive_failures` to 3 in
> HealthCheck message definition. But the counter for
> `consecutiveFailures` in `mesos-health-check` would be reset to 0
> after a success check. It is possible to continue to receive status
> updates before we stop the driver.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/46307/diff/
> 
> 
> Testing
> ---
> 
> # I still could not reproduce the problem in old code after repeatedly tests. 
> So seems no way to verify whether my assumption is correct or not.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 47363: Introduced a driver to v1 executor shim/adapter.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Bugs: MESOS-5302
https://issues.apache.org/jira/browse/MESOS-5302


Repository: mesos


Description
---

This change adds a driver to v1 executor shim/adapter. This can
be used by the command executor/docker executor to toggle
between using the new/old API instead of duplicating the
code like we did with `http_command_executor.cpp`.


Diffs
-

  include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/executor/v0_v1executor.hpp PRE-CREATION 
  src/executor/v0_v1executor.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/47363/diff/


Testing
---

make check (Later in the chain, we make the command executor use this interface)
So the existing tests should be able to test this out.


Thanks,

Anand Mazumdar

Review Request 47365: Moved code from HTTP command executor to command executor.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Bugs: MESOS-5302
https://issues.apache.org/jira/browse/MESOS-5302


Repository: mesos


Description
---

This change makes reviewing code later in the chain easier.
Later in the chain, we would make this use the Shim/Adapter
to toggle between using the driver or the v1 API via an
environment variable. `http_command_executor.cpp` would
be deleted.


Diffs
-

  src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 

Diff: https://reviews.apache.org/r/47365/diff/


Testing
---

make check
(this should not be committed on its own)


Thanks,

Anand Mazumdar

Review Request 47366: Made the command executor use the adapter interface.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Bugs: MESOS-5302
https://issues.apache.org/jira/browse/MESOS-5302


Repository: mesos


Description
---

This change adds support to the command executor to toggle
between the driver based adapter/v1 API based on the
`MESOS_HTTP_COMMAND_EXECUTOR` environment variable
set by the agent.


Diffs
-

  src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 

Diff: https://reviews.apache.org/r/47366/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 47198: Added labels to docker v1 spec config.

[Gilbert Song]

> On May 12, 2016, 5:25 a.m., Guangya Liu wrote:
> > include/mesos/docker/v1.proto, lines 56-57
> > 
> >
> > Why both `optional`? I found that other `Label` definition are defining 
> > `key` as required but `value` as `optional` or `required`. I think that for 
> > your case, both `key` and `value` should be required?

The docker label should be independent from the label in our first class proto. 

Even though from docker documentation `Keys should start and end with an alpha 
numeric character`. But I can still generate the followings by some experiments 
using docker build:
```
"Labels": {
"": "something",
"com.example.release-date": "2015-02-12",
"com.example.version": "0.0.1-beta",
"com.example.version.is-beta": "",
"com.example.version.is-production": "",
"vendor": "ACME Incorporated"
}```

So that is the reason we want both optional. Actually `optional` should be more 
safe even if you dont have the issue above.


- Gilbert


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


On May 11, 2016, 8:56 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47198/
> ---
> 
> (Updated May 11, 2016, 8:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added labels to docker v1 spec config.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/v1.proto ff18f8cadfce3315467f194d50d3469fa839f686 
> 
> Diff: https://reviews.apache.org/r/47198/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47089: Waited for first statusUpdate before advance clock in GracePeriod test.

[Benjamin Mahler]

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




src/tests/health_check_tests.cpp (line 1015)


Why did this change?



src/tests/health_check_tests.cpp (line 1057)


Ideally this uses `->` instead of `.get().` but if you'd like to send a 
sweep of this file separately I'll be happy to commit it.



src/tests/health_check_tests.cpp (line 1059)


It looks like the clock needs to be paused before the task is launched? 
Otherwise we may have too much time advance between launching the task and 
pausing the clock, no?


- Benjamin Mahler


On May 7, 2016, 11:46 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47089/
> ---
> 
> (Updated May 7, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Niklas Nielsen, Timothy Chen, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1653
> https://issues.apache.org/jira/browse/MESOS-1653
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Waited for first statusUpdate before advance clock in GracePeriod test.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/47089/diff/
> 
> 
> Testing
> ---
> 
> Test with
> 
> ```
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="HealthCheckTest.GracePeriod" 
> --verbose --gtest_break_on_failure --gtest_repeat=-1
> ```
> 
> more than 1500 iteractions in my slow machine.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47198: Added labels to docker v1 spec config.

[Gilbert Song]

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

(Updated May 13, 2016, 1:48 p.m.)


Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and Kevin 
Klues.


Bugs: MESOS-5272
https://issues.apache.org/jira/browse/MESOS-5272


Repository: mesos


Description
---

Added labels to docker v1 spec config.


Diffs (updated)
-

  include/mesos/docker/v1.proto ff18f8cadfce3315467f194d50d3469fa839f686 

Diff: https://reviews.apache.org/r/47198/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 47168: Windows: Implemented `kill`.

[Daniel Pravat]

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

(Updated May 13, 2016, 8:42 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-5375
https://issues.apache.org/jira/browse/MESOS-5375


Repository: mesos


Description
---

Windows: Implemented `kill`.


Diffs (updated)
-

  3rdparty/stout/include/Makefile.am 9b39ce32c0269479066cf7991afaeed65d8ab547 
  3rdparty/stout/include/stout/os/kill.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/posix/kill.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/kill.hpp PRE-CREATION 
  3rdparty/stout/include/stout/windows.hpp 
a7a59e78575e1456b4e14d18ac97f51dd23d794e 

Diff: https://reviews.apache.org/r/47168/diff/


Testing
---

Windows: build/test w/ Marathon


Thanks,

Daniel Pravat

Re: Review Request 47169: Windows: Implemented `killtree` using NT job objects.

[Daniel Pravat]

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

(Updated May 13, 2016, 8:42 p.m.)


Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
Remoortere, and Michael Park.


Bugs: MESOS-3639
https://issues.apache.org/jira/browse/MESOS-3639


Repository: mesos


Description
---

Windows: Implemented  `killtree` using NT job objects.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/killtree.hpp 
b075d625541ed6c10192e3e98bf399b38b69cdc5 
  3rdparty/stout/include/stout/windows/os.hpp 
261768eace6ab09956f4a80e1ec5dba988d831e1 

Diff: https://reviews.apache.org/r/47169/diff/


Testing
---

Windows: build/test with Marathon


Thanks,

Daniel Pravat

Review Request 47359: Fixed a typo in persistent volume docs.

[Greg Mann]

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

Review request for mesos and Bernd Mathiske.


Repository: mesos


Description
---

Fixed a typo in persistent volume docs.


Diffs
-

  docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 

Diff: https://reviews.apache.org/r/47359/diff/


Testing
---


Thanks,

Greg Mann

Re: Review Request 47199: Implemented parsing docker labels in v1 spec.

[Guangya Liu]

> On 五月 12, 2016, 12:44 p.m., Guangya Liu wrote:
> > src/docker/spec.cpp, lines 226-230
> > 
> >
> > Does there are any possiblity that there are duplicat labels in 
> > `config` and `container_config`? If so, how to handle the case if there are 
> > duplicate labels with `config`?
> 
> Gilbert Song wrote:
> From the docker spec, we are assuming the key should be unique and for 
> each Dockerfile that contains duplicate label for the same key, they would 
> overwrite the privious key. So we dont need to handle duplicates here. Good 
> thoughts!

Good to know, can you please add some comments here to clarify? Another point 
is that as we are `assuming the key should be unique` for each Dockerfile, but 
the Dockerfile is written by 3rd party, does it make sense to add some error 
handling here?


- Guangya


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


On 五月 13, 2016, 8:49 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47199/
> ---
> 
> (Updated 五月 13, 2016, 8:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parsing docker labels in v1 spec.
> 
> 
> Diffs
> -
> 
>   src/docker/spec.cpp ac28331a17edb8c2ff81d5a2f79a794f869a3e5d 
> 
> Diff: https://reviews.apache.org/r/47199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47198: Added labels to docker v1 spec config.

[Guangya Liu]

> On 五月 12, 2016, 12:25 p.m., Guangya Liu wrote:
> > include/mesos/docker/v1.proto, lines 56-57
> > 
> >
> > Why both `optional`? I found that other `Label` definition are defining 
> > `key` as required but `value` as `optional` or `required`. I think that for 
> > your case, both `key` and `value` should be required?
> 
> Gilbert Song wrote:
> The docker label should be independent from the label in our first class 
> proto. 
> 
> Even though from docker documentation `Keys should start and end with an 
> alpha numeric character`. But I can still generate the followings by some 
> experiments using docker build:
> ```
> "Labels": {
> "": "something",
> "com.example.release-date": "2015-02-12",
> "com.example.version": "0.0.1-beta",
> "com.example.version.is-beta": "",
> "com.example.version.is-production": "",
> "vendor": "ACME Incorporated"
> }```
> 
> So that is the reason we want both optional. Actually `optional` should 
> be more safe even if you dont have the issue above.

I think that at least we need to have the `key` as required, otherwise, the 
label as `"": "something",` may have no meaning, how does a label without `key` 
works?


- Guangya


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


On 五月 13, 2016, 8:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47198/
> ---
> 
> (Updated 五月 13, 2016, 8:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added labels to docker v1 spec config.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/v1.proto ff18f8cadfce3315467f194d50d3469fa839f686 
> 
> Diff: https://reviews.apache.org/r/47198/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47374, 47216, 47150, 47215, 47214, 47213, 47212, 47205, 47149]

Failed command: ./support/apply-review.sh -n -r 47213

Error:
2016-05-14 03:36:17 URL:https://reviews.apache.org/r/47213/diff/raw/ 
[1540/1540] -> "47213.patch" [1]
error: 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp: does 
not exist in index

Full log: https://builds.apache.org/job/mesos-reviewbot/13061/console

- Mesos ReviewBot


On May 13, 2016, 11:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 13, 2016, 11:31 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
> Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_CPPFLAGS =   \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47266: Implemented provisioner removing docker whitelist files.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47265, 47266]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 10:16 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47266/
> ---
> 
> (Updated May 13, 2016, 10:16 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5277
> https://issues.apache.org/jira/browse/MESOS-5277
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented provisioner removing docker whitelist files.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
> 
> Diff: https://reviews.apache.org/r/47266/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with docker image containing `.wh.` files.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Qian Zhang]

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




src/slave/slave.cpp (line 1019)


If we do the link here, that means we still establish TCP connection with 
master right after a new master is detected, right? But I think we want to do 
the link after the initial backoff to avoid SYN flood.


- Qian Zhang


On May 13, 2016, 11:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 11:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47362: Added a test for the SSL head-of-line blocking issue in MESOS-5340.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47362]

Failed command: ./support/apply-review.sh -n -r 47362

Error:
2016-05-14 02:14:21 URL:https://reviews.apache.org/r/47362/diff/raw/ 
[2478/2478] -> "47362.patch" [1]
Total errors found: 0
Checking 1 files
Error: No line in the commit message summary may exceed 72 characters.

Full log: https://builds.apache.org/job/mesos-reviewbot/13059/console

- Mesos ReviewBot


On May 13, 2016, 9:46 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47362/
> ---
> 
> (Updated May 13, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-5340
> https://issues.apache.org/jira/browse/MESOS-5340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test works by creating a connection on which no data is sent (the SSL 
> handshake does not complete, nor is the socket downgraded). After which, we 
> expect that an HTTP request should succeed.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 4d237815a03828b915e821c3af78132e2915c610 
> 
> Diff: https://reviews.apache.org/r/47362/diff/
> 
> 
> Testing
> ---
> 
> This test fails before the fix in MESOS-5340.
> 
> ```
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from SSLTest
> [ RUN  ] SSLTest.SilentSocket
> ../../../3rdparty/libprocess/src/tests/ssl_tests.cpp:752: Failure
> Failed to wait 15secs for socket
> [  FAILED  ] SSLTest.SilentSocket (15221 ms)
> [--] 1 test from SSLTest (15221 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (15222 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] SSLTest.SilentSocket
> ```
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 47362: Added a test for the SSL head-of-line blocking issue in MESOS-5340.

[haosdent huang]

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




3rdparty/libprocess/src/tests/ssl_tests.cpp (line 736)


Have a question here, why settle and resume without any operations here?



3rdparty/libprocess/src/tests/ssl_tests.cpp (line 755)


I think should change to

```
  const string buffer =
string("HTTP/1.1 200 OK\r\n") +
"Content-Length : " +
stringify(data.length()) + "\r\n" +
"\r\n" +
data;
```

here?


- haosdent huang


On May 13, 2016, 9:46 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47362/
> ---
> 
> (Updated May 13, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-5340
> https://issues.apache.org/jira/browse/MESOS-5340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test works by creating a connection on which no data is sent (the SSL 
> handshake does not complete, nor is the socket downgraded). After which, we 
> expect that an HTTP request should succeed.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 4d237815a03828b915e821c3af78132e2915c610 
> 
> Diff: https://reviews.apache.org/r/47362/diff/
> 
> 
> Testing
> ---
> 
> This test fails before the fix in MESOS-5340.
> 
> ```
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from SSLTest
> [ RUN  ] SSLTest.SilentSocket
> ../../../3rdparty/libprocess/src/tests/ssl_tests.cpp:752: Failure
> Failed to wait 15secs for socket
> [  FAILED  ] SSLTest.SilentSocket (15221 ms)
> [--] 1 test from SSLTest (15221 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (15222 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] SSLTest.SilentSocket
> ```
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 46498: Add runtime for Appc Spec ex: command, workingdir and environment.

[Srinivas Brahmaroutu]

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

(Updated May 14, 2016, 4:20 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.


Repository: mesos


Description
---

Add runtime for Appc Spec ex: command, workingdir and environment.


Diffs (updated)
-

  include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
  include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 
  src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
  src/slave/containerizer/mesos/containerizer.cpp 
75e5a32a3e70ec60a6800e21a621673184ea0956 
  src/slave/containerizer/mesos/isolators/appc/runtime.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
5c96e9f6603d39889e6bc807874d35d0cb3556be 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 
  src/tests/containerizer/provisioner_appc_tests.cpp 
84fe52b6937c3b7d7628b17a2f045eec2f386b4d 

Diff: https://reviews.apache.org/r/46498/diff/


Testing
---


Thanks,

Srinivas Brahmaroutu

Re: Review Request 47266: Implemented provisioner removing docker whitelist files.

[Guangya Liu]

> On 五月 13, 2016, 6:49 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, line 320
> > 
> >
> > s/bind/bind and overlay
> 
> Gilbert Song wrote:
> The overlay backend supports multi-layered images:)

The overlay also support single layer after https://reviews.apache.org/r/45358/ 
, I think that may be we can deprecate `bind` backend in near feature? ;-)


- Guangya


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


On 五月 13, 2016, 10:16 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47266/
> ---
> 
> (Updated 五月 13, 2016, 10:16 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5277
> https://issues.apache.org/jira/browse/MESOS-5277
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented provisioner removing docker whitelist files.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
> 
> Diff: https://reviews.apache.org/r/47266/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with docker image containing `.wh.` files.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

> On 五月 12, 2016, 10:34 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp, line 
> > 174
> > 
> >
> > since you already have the check above, why dont you return None() 
> > there to avoid checking `forcepull` again?
> 
> Guangya Liu wrote:
> if (forcePullImage) {
>   VLOG(1) << "The mesos containerizer is trying to force pull image '"
>   << imageReference << "' from registry";
> }
> //The above check is mainly for logging.
> 
> if (forcePullImage || !storedImages.contains(imageReference)) {
>   return None();
> }
> 
> Gilbert Song wrote:
> How about:
> 
> ```
> if (!storedImages.contains(imageReference)) {
>   return None();
> }
> 
> if (forcePullImage) {
>   VLOG(1) << "The mesos containerizer is trying to force pull image '"
>   << imageReference << "' from registry";
>   
>   return None();
> }
> ```

Good! ;-)


- Guangya


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


On 五月 13, 2016, 3:26 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44838/
> ---
> 
> (Updated 五月 13, 2016, 3:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled mesos containerizer force_pull_image for docker.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 137af502a66e6a65773c00eaacbe392576376284 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> cf630cc0b67a325529fa04ad2b1708e013b9596a 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> eeec94326a4fd67675df10e0b6a32267e555fa96 
> 
> Diff: https://reviews.apache.org/r/44838/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> It is difficult to test this with unit test, but just test with mesos execute 
> by setting `force_pull_image` as true and found that the iamge was always 
> pulled when `force_pull_image` is true.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47336: Fixed FTS handle leak issue in xfs isolator.

[Guangya Liu]

> On 五月 13, 2016, 5:22 p.m., James Peach wrote:
> > Looks fine. Consider switching to ``unique_ptr``, since we only check the 
> > ``fts_close`` error 1 of 3 times.

@James Peach, can you please show more detail for what do you mean? How to 
switch to `unique_prt`? Do you want me to submit another patch to address your 
comments?

Here the `fts_close` was called 2 times for error handling and 1 time for 
normal close.


- Guangya


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


On 五月 13, 2016, 7:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47336/
> ---
> 
> (Updated 五月 13, 2016, 7:09 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed FTS handle leak issue in xfs isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
> 80404c396ae98622a69ec290881226c1753d3626 
> 
> Diff: https://reviews.apache.org/r/47336/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 47381: Fixed agent to properly handle killTask of unregistered executor.

[Vinod Kone]

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

Review request for mesos and Jie Yu.


Bugs: MESOS-5380
https://issues.apache.org/jira/browse/MESOS-5380


Repository: mesos


Description
---

The agent now shuts down the executor during registration if it does not
have any queued tasks (e.g., framework sent a killTask before
registration).

Note that if the executor doesn't register at all, it will be cleaned up
anyway after the registration timeout value.

Also, note that this doesn't handle the case where the agent restarts
after processing the killTask() but before cleaning up the executor.


Diffs
-

  src/slave/slave.cpp 7c870396b4d6804bfda6169d76d136e95cd839f5 
  src/tests/slave_tests.cpp 3ec670aa75790417ec8b7b96cfdb787492b104e1 

Diff: https://reviews.apache.org/r/47381/diff/


Testing
---

make check

The new test failed w/o the code change.


Thanks,

Vinod Kone

Re: Review Request 46428: Fixed the broken Docker Volume Rootfs Test on Centos7.

[Guangya Liu]

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

(Updated 五月 14, 2016, 1:33 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


Bugs: MESOS-5351
https://issues.apache.org/jira/browse/MESOS-5351


Repository: mesos


Description
---

Fixed the broken Docker Volume Rootfs Test on Centos7.


Diffs (updated)
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp 
c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 

Diff: https://reviews.apache.org/r/46428/diff/


Testing
---

make
make check

[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from DockerVolumeIsolatorTest
[ RUN  ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
[   OK ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
 (1732 ms)
[ RUN  ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
[   OK ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume 
(1708 ms)
[--] 2 tests from DockerVolumeIsolatorTest (3451 ms total)
 
[--] Global test environment tear-down
[==] 2 tests from 1 test case ran. (3460 ms total)
[  PASSED  ] 2 tests.
[root@mesos-24 build]# cat /etc/*release
CentOS Linux release 7.2.1511 (Core) 
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/;
BUG_REPORT_URL="https://bugs.centos.org/;
 
CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"
 
CentOS Linux release 7.2.1511 (Core) 
CentOS Linux release 7.2.1511 (Core) 
[root@mesos-24 build]#


Thanks,

Guangya Liu

Re: Review Request 46107: Add Appc runtime spec for command, working directory and environment.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46107]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 7:52 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46107/
> ---
> 
> (Updated May 13, 2016, 7:52 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Appc runtime spec for command, working directory and environment.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 
> 
> Diff: https://reviews.apache.org/r/46107/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 47360: Updated dynamic reservation and persistent volume docs.

[Greg Mann]

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

(Updated May 14, 2016, 12:39 a.m.)


Review request for mesos, Bernd Mathiske and Neil Conway.


Bugs: MESOS-5215
https://issues.apache.org/jira/browse/MESOS-5215


Repository: mesos


Description
---

This patch updates the documentation of RESERVE and
CREATE operations, both via offer operations and
operator endpoints. Specifically, we clarify the
Mesos master's expectations for the values of the
`principal` fields found in `ReservationInfo` and
`DiskInfo.Persistence`.


Diffs (updated)
-

  docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 
  docs/reservation.md a400d19aec7a48d122ba1c9c23d38d792b8dbe6f 

Diff: https://reviews.apache.org/r/47360/diff/


Testing
---

Viewed with the Mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 44838: Enabled mesos containerizer force_pull_image for docker.

[Guangya Liu]

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

(Updated 五月 14, 2016, 12:58 a.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Bugs: MESOS-4886
https://issues.apache.org/jira/browse/MESOS-4886


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for docker.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
137af502a66e6a65773c00eaacbe392576376284 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
cf630cc0b67a325529fa04ad2b1708e013b9596a 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
eeec94326a4fd67675df10e0b6a32267e555fa96 

Diff: https://reviews.apache.org/r/44838/diff/


Testing
---

make
make check

It is difficult to test this with unit test, but just test with mesos execute 
by setting `force_pull_image` as true and found that the iamge was always 
pulled when `force_pull_image` is true.


Thanks,

Guangya Liu

Re: Review Request 44839: Enabled mesos containerizer force_pull_image for appc.

[Guangya Liu]

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

(Updated 五月 14, 2016, 1:29 a.m.)


Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.


Bugs: MESOS-4886
https://issues.apache.org/jira/browse/MESOS-4886


Repository: mesos


Description
---

Enabled mesos containerizer force_pull_image for appc.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 

Diff: https://reviews.apache.org/r/44839/diff/


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 47169: Windows: Implemented `killtree` using NT job objects.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47052, 47053, 47221, 47054, 41632, 47168, 47169]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 8:42 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47169/
> ---
> 
> (Updated May 13, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3639
> https://issues.apache.org/jira/browse/MESOS-3639
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented  `killtree` using NT job objects.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 
> b075d625541ed6c10192e3e98bf399b38b69cdc5 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 261768eace6ab09956f4a80e1ec5dba988d831e1 
> 
> Diff: https://reviews.apache.org/r/47169/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/test with Marathon
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47199: Implemented parsing docker labels in v1 spec.

[Gilbert Song]

> On May 12, 2016, 5:44 a.m., Guangya Liu wrote:
> > src/docker/spec.cpp, lines 226-230
> > 
> >
> > Does there are any possiblity that there are duplicat labels in 
> > `config` and `container_config`? If so, how to handle the case if there are 
> > duplicate labels with `config`?

>From the docker spec, we are assuming the key should be unique and for each 
>Dockerfile that contains duplicate label for the same key, they would 
>overwrite the privious key. So we dont need to handle duplicates here. Good 
>thoughts!


- Gilbert


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


On May 11, 2016, 8:56 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47199/
> ---
> 
> (Updated May 11, 2016, 8:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5272
> https://issues.apache.org/jira/browse/MESOS-5272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented parsing docker labels in v1 spec.
> 
> 
> Diffs
> -
> 
>   src/docker/spec.cpp ac28331a17edb8c2ff81d5a2f79a794f869a3e5d 
> 
> Diff: https://reviews.apache.org/r/47199/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 47362: Added a test for the SSL head-of-line blocking issue in MESOS-5340.

[Benjamin Mahler]

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

Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and Neil 
Conway.


Bugs: MESOS-5340
https://issues.apache.org/jira/browse/MESOS-5340


Repository: mesos


Description
---

This test works by creating a connection on which no data is sent (the SSL 
handshake does not complete, nor is the socket downgraded). After which, we 
expect that an HTTP request should succeed.


Diffs
-

  3rdparty/libprocess/src/tests/ssl_tests.cpp 
4d237815a03828b915e821c3af78132e2915c610 

Diff: https://reviews.apache.org/r/47362/diff/


Testing
---

This test fails before the fix in MESOS-5340.

```
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from SSLTest
[ RUN  ] SSLTest.SilentSocket
../../../3rdparty/libprocess/src/tests/ssl_tests.cpp:752: Failure
Failed to wait 15secs for socket
[  FAILED  ] SSLTest.SilentSocket (15221 ms)
[--] 1 test from SSLTest (15221 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (15222 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLTest.SilentSocket
```


Thanks,

Benjamin Mahler

Review Request 47360: Updated dynamic reservation and persistent volume docs.

[Greg Mann]

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

Review request for mesos, Bernd Mathiske and Neil Conway.


Bugs: MESOS-5215
https://issues.apache.org/jira/browse/MESOS-5215


Repository: mesos


Description
---

This patch updates the documentation of RESERVE and
CREATE operations, both via offer operations and
operator endpoints. Specifically, we clarify the
Mesos master's expectations for the values of the
`principal` fields found in `ReservationInfo` and
`DiskInfo.Persistence`.


Diffs
-

  docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 
  docs/reservation.md a400d19aec7a48d122ba1c9c23d38d792b8dbe6f 

Diff: https://reviews.apache.org/r/47360/diff/


Testing
---

Viewed with the Mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47274]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 13, 2016, 6:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 13, 2016, 6:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/http.cpp a0d67671c89b9794e721dc3ba012acd3bdc447e3 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47266: Implemented provisioner removing docker whitelist files.

[Gilbert Song]

> On May 12, 2016, 11:49 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, line 320
> > 
> >
> > s/bind/bind and overlay

The overlay backend supports multi-layered images:)


- Gilbert


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


On May 11, 2016, 3:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47266/
> ---
> 
> (Updated May 11, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5277
> https://issues.apache.org/jira/browse/MESOS-5277
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented provisioner removing docker whitelist files.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
> 
> Diff: https://reviews.apache.org/r/47266/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with docker image containing `.wh.` files.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>