Re: Review Request 49313: Added the VIEW_FLAGS authorization action.

[Adam B]

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




include/mesos/authorizer/acls.proto (lines 254 - 255)


Ooh, so in a future world, you could allow/deny access to individual 
flags.. nice



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


s/whole flags set/entire set of flags/



src/common/http.cpp (line 710)


Indentation



src/master/http.cpp (line 1226)


s/query the path/view all flags/



src/master/http.cpp (lines 1247 - 1248)


double-blank line



src/slave/http.cpp (line 552)


s/query this endpoint/view all flags/



src/slave/http.cpp (lines 573 - 574)


double-blank line


- Adam B


On June 28, 2016, 3:51 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49313/
> ---
> 
> (Updated June 28, 2016, 3:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-5705
> https://issues.apache.org/jira/browse/MESOS-5705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds again authorization for flags. Instead of being part of
> `get_endpoints` it uses its own action `VIEW_FLAGS` which is
> used to restrict access to the `/flags` endpoint, as well as
> to filter the results of the `/state` endpoint on both master
> and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> a6d93cd2cb9161a98565b22e50b06aac4931a671 
>   include/mesos/authorizer/authorizer.proto 
> fc76796022a6fa3d36a1447c476980868d42c2d0 
>   src/authorizer/local/authorizer.cpp 
> 3fade4168face1cb80b30c9b69b31d9eb4126222 
>   src/common/http.hpp 55bd0ac81af80c656a4a80766a3e4b21db9cf0cf 
>   src/common/http.cpp 95e8fb672b49a00860c64f818cc734fa22cf3516 
>   src/master/http.cpp e089fe960779f39b3321f2ec81ab2acc17d53641 
>   src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
>   src/tests/authorization_tests.cpp 9b99da138fa27a725738d70bd99e889b108b44ae 
>   src/tests/master_authorization_tests.cpp 
> 81804e0522fd6b26155732af08e33c4d0bb0a8fe 
>   src/tests/slave_authorization_tests.cpp 
> 78221e200d9b7880cc474f1acef92c5dec1c8e25 
> 
> Diff: https://reviews.apache.org/r/49313/diff/
> 
> 
> Testing
> ---
> 
> - `make check`
> - manual tests with browsers.
> - Used the script:
>  
> ```sh
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "view_flags" : [
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "ANY" }
>},
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "NONE" }
>}
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>  --authenticate_http \
>  --log_dir=/tmp/mesos/logs/master \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> 
> # Returns a 200 OK Response with the contents of the flags
> # in JSON object
> http GET http://127.0.0.1:5050/flags -a foo:bar
> http GET http://127.0.0.1:5051/flags -a foo:bar
> 
> # Returned JSON contains a `flags` entry with all the flags.
> http GET http://127.0.0.1:5050/state -a foo:bar
> http GET http://127.0.0.1:5051/state -a foo:bar
> 
> # 403 Forbidden response
> http GET http://127.0.0.1:5050/flags -a baz:bar
> http GET http://127.0.0.1:5051/flags -a baz:bar
> 
> # Returned JSON doesn't include flags information.
> http GET http://127.0.0.1:5050/state -a baz:bar
> http GET http://127.0.0.1:5051/state -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 49353: Removed unused parameters in command executor.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On June 29, 2016, 5:32 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49353/
> ---
> 
> (Updated June 29, 2016, 5:32 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused parameters in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 207a101cb466c015d1b3544f0093c784fa69e7b9 
>   src/launcher/posix/executor.hpp 9d26cd2be58297095d38c3042f0bec9883d7eea6 
>   src/launcher/posix/executor.cpp 0c181ecd8a7361cf1321816f7c089783fc506ab9 
>   src/launcher/windows/executor.hpp cc39f7b0ecca5ae4c64c14d2094e4c24eeb1165f 
>   src/launcher/windows/executor.cpp d6eaec3b9d1b50c731d7849656a70abe85dad5f5 
> 
> Diff: https://reviews.apache.org/r/49353/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 49353: Removed unused parameters in command executor.

[Jie Yu]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Removed unused parameters in command executor.


Diffs
-

  src/launcher/executor.cpp 207a101cb466c015d1b3544f0093c784fa69e7b9 
  src/launcher/posix/executor.hpp 9d26cd2be58297095d38c3042f0bec9883d7eea6 
  src/launcher/posix/executor.cpp 0c181ecd8a7361cf1321816f7c089783fc506ab9 
  src/launcher/windows/executor.hpp cc39f7b0ecca5ae4c64c14d2094e4c24eeb1165f 
  src/launcher/windows/executor.cpp d6eaec3b9d1b50c731d7849656a70abe85dad5f5 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 49207: Added proto message definitions to support appc runtime.

[Jie Yu]

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




include/mesos/appc/spec.proto (line 43)


+1

Thanks Guangya, I missed that


- Jie Yu


On June 28, 2016, 9:45 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49207/
> ---
> 
> (Updated June 28, 2016, 9:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proto message definitions to support appc runtime.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
> 
> Diff: https://reviews.apache.org/r/49207/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49313: Added the VIEW_FLAGS authorization action.

[Jie Yu]

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




src/authorizer/local/authorizer.cpp (line 203)


Not yours. `{` should be in the next line.



src/master/http.cpp (lines 2080 - 2082)


This looks like a flag too? If you do not want to treat that as a flag, add 
a note at least?


- Jie Yu


On June 28, 2016, 10:51 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49313/
> ---
> 
> (Updated June 28, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-5705
> https://issues.apache.org/jira/browse/MESOS-5705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds again authorization for flags. Instead of being part of
> `get_endpoints` it uses its own action `VIEW_FLAGS` which is
> used to restrict access to the `/flags` endpoint, as well as
> to filter the results of the `/state` endpoint on both master
> and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> a6d93cd2cb9161a98565b22e50b06aac4931a671 
>   include/mesos/authorizer/authorizer.proto 
> fc76796022a6fa3d36a1447c476980868d42c2d0 
>   src/authorizer/local/authorizer.cpp 
> 3fade4168face1cb80b30c9b69b31d9eb4126222 
>   src/common/http.hpp 55bd0ac81af80c656a4a80766a3e4b21db9cf0cf 
>   src/common/http.cpp 95e8fb672b49a00860c64f818cc734fa22cf3516 
>   src/master/http.cpp e089fe960779f39b3321f2ec81ab2acc17d53641 
>   src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
>   src/tests/authorization_tests.cpp 9b99da138fa27a725738d70bd99e889b108b44ae 
>   src/tests/master_authorization_tests.cpp 
> 81804e0522fd6b26155732af08e33c4d0bb0a8fe 
>   src/tests/slave_authorization_tests.cpp 
> 78221e200d9b7880cc474f1acef92c5dec1c8e25 
> 
> Diff: https://reviews.apache.org/r/49313/diff/
> 
> 
> Testing
> ---
> 
> - `make check`
> - manual tests with browsers.
> - Used the script:
>  
> ```sh
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "view_flags" : [
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "ANY" }
>},
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "NONE" }
>}
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>  --authenticate_http \
>  --log_dir=/tmp/mesos/logs/master \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> 
> # Returns a 200 OK Response with the contents of the flags
> # in JSON object
> http GET http://127.0.0.1:5050/flags -a foo:bar
> http GET http://127.0.0.1:5051/flags -a foo:bar
> 
> # Returned JSON contains a `flags` entry with all the flags.
> http GET http://127.0.0.1:5050/state -a foo:bar
> http GET http://127.0.0.1:5051/state -a foo:bar
> 
> # 403 Forbidden response
> http GET http://127.0.0.1:5050/flags -a baz:bar
> http GET http://127.0.0.1:5051/flags -a baz:bar
> 
> # Returned JSON doesn't include flags information.
> http GET http://127.0.0.1:5050/state -a baz:bar
> http GET http://127.0.0.1:5051/state -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 49297: Use defer() to garantee getTasks lambda execution context.

[Jay Guo]

> On June 28, 2016, 8:45 p.m., Vinod Kone wrote:
> > Maybe it's better for getTasks() to have it's own independation 
> > implementation instead of depending on the factored out `_tasks()`. 
> > getAgents() and getFrameworks() have their own implementations anyway. We 
> > can also revert the `_tasks()` split. We need to add comments for why we 
> > have own independent implementations for getTasks(), getAgents() and 
> > getFrameworks() for posterity.
> > 
> > What do you think?

We could also take json and evolve it to v1Response, like `getContainers`. But 
I guess standalone implementation makes more sense if we want to embed 
`getTasks` into other responses.


- Jay


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


On June 28, 2016, 7:23 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49297/
> ---
> 
> (Updated June 28, 2016, 7:23 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use defer() to garantee getTasks lambda execution context.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d583bbeb186974135999eaddbab96e69635dcc7c 
> 
> Diff: https://reviews.apache.org/r/49297/diff/
> 
> 
> Testing
> ---
> 
> WIP
> 
> make check
> 
> Still need to figure out how to handle invalid task*
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

[Timothy Chen]

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



Can you add a test?

- Timothy Chen


On June 29, 2016, 4:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36816/
> ---
> 
> (Updated June 29, 2016, 4:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2533
> https://issues.apache.org/jira/browse/MESOS-2533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported HTTP/HTTPS in health check.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/health-check/main.cpp 4cc9dde8927acd4594477476f1b1bdc43963d789 
> 
> Diff: https://reviews.apache.org/r/36816/diff/
> 
> 
> Testing
> ---
> 
> * Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
> HealthCheckTest.HealthyTaskNotMatchHttpStatuses
> make check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 49351: Added wrapper function for health check in docker executor.

[haosdent huang]

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

Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
Timothy Chen.


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


Repository: mesos


Description
---

Added wrapper function for health check in docker executor.


Diffs
-

  src/docker/executor.cpp 88b7fc4c36ed3974ac6b103a29e1d975619f0c69 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

[haosdent huang]

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

(Updated June 29, 2016, 4:45 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Mahler, and 
Timothy Chen.


Changes
---

Rebase.


Summary (updated)
-

Supported HTTP/HTTPS in health check.


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


Repository: mesos


Description (updated)
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/health-check/main.cpp 4cc9dde8927acd4594477476f1b1bdc43963d789 

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


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 49313: Added the VIEW_FLAGS authorization action.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49313]

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

- Mesos ReviewBot


On June 28, 2016, 10:51 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49313/
> ---
> 
> (Updated June 28, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-5705
> https://issues.apache.org/jira/browse/MESOS-5705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds again authorization for flags. Instead of being part of
> `get_endpoints` it uses its own action `VIEW_FLAGS` which is
> used to restrict access to the `/flags` endpoint, as well as
> to filter the results of the `/state` endpoint on both master
> and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> a6d93cd2cb9161a98565b22e50b06aac4931a671 
>   include/mesos/authorizer/authorizer.proto 
> fc76796022a6fa3d36a1447c476980868d42c2d0 
>   src/authorizer/local/authorizer.cpp 
> 3fade4168face1cb80b30c9b69b31d9eb4126222 
>   src/common/http.hpp 55bd0ac81af80c656a4a80766a3e4b21db9cf0cf 
>   src/common/http.cpp 95e8fb672b49a00860c64f818cc734fa22cf3516 
>   src/master/http.cpp e089fe960779f39b3321f2ec81ab2acc17d53641 
>   src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
>   src/tests/authorization_tests.cpp 9b99da138fa27a725738d70bd99e889b108b44ae 
>   src/tests/master_authorization_tests.cpp 
> 81804e0522fd6b26155732af08e33c4d0bb0a8fe 
>   src/tests/slave_authorization_tests.cpp 
> 78221e200d9b7880cc474f1acef92c5dec1c8e25 
> 
> Diff: https://reviews.apache.org/r/49313/diff/
> 
> 
> Testing
> ---
> 
> - `make check`
> - manual tests with browsers.
> - Used the script:
>  
> ```sh
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "view_flags" : [
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "ANY" }
>},
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "NONE" }
>}
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>  --authenticate_http \
>  --log_dir=/tmp/mesos/logs/master \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> 
> # Returns a 200 OK Response with the contents of the flags
> # in JSON object
> http GET http://127.0.0.1:5050/flags -a foo:bar
> http GET http://127.0.0.1:5051/flags -a foo:bar
> 
> # Returned JSON contains a `flags` entry with all the flags.
> http GET http://127.0.0.1:5050/state -a foo:bar
> http GET http://127.0.0.1:5051/state -a foo:bar
> 
> # 403 Forbidden response
> http GET http://127.0.0.1:5050/flags -a baz:bar
> http GET http://127.0.0.1:5051/flags -a baz:bar
> 
> # Returned JSON doesn't include flags information.
> http GET http://127.0.0.1:5050/state -a baz:bar
> http GET http://127.0.0.1:5051/state -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 49336: Renamed `agent_id` fields to `slave_id` for unversioned master.proto.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49336/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change renames the `agent_id` to `slave_id` for the
> unversioned protos to keep it consistent to other unversioned
> protobuf files. Also, fixed a couple of minor issues of not
> capturing aliases by const references for consistency.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto b9936521628238fd3d215c6a05423ebbda1e94d7 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 49335: Moved the agent exists check to `_reserve/_unreserve`.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49335/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moved the agent exists check that is common to
> both `/reserve` and the new API to `_reserve`. This is
> consistent to how the `/create-volumes` already does it.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49335/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 49346: Changed the replog's network abstraction to use 'relink'.

[Benjamin Mahler]

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


Ship it!




What is a replog?

```
$ grep -R replog mesos | wc -l
$ 0
```

I'm kidding :) but might as well avoid the abbreviation here as I doubt 
everyone will recognize it.


src/log/network.hpp (lines 159 - 165)


I tend to like a newline between each comment "block" (above the NOTE 
here). I think here I would be more interested in understanding exactly what a 
stale socket is. I did some digging to find pointers and made a suggestion 
below. The idea is that even though this code isn't really the right place to 
document stale connections, it's going to serve as an example of using 
RECONNECT and we may consider adopting some of this documentation within the 
`link` documentation so that callers understand this pitfall:

```
// Link in order to keep a socket open (more efficient).
//
// We force a reconnect to avoid sending on a "stale" socket. In
// general when linking to a remote process, the underlying TCP
// connection may become "stale". RFC 793 refers to this as a
// "half-open" connection: the RST is not sent upon the death
// of the peer and a RST will only be received once further
// data is sent on the socket.
// 
// "Half-open" (aka "stale") connections are typically addressed
// via keep-alives (see RFC 1122 4.2.3.6) to periodically probe
// the connection. In our case here we can rely on the re-addition
// of the network member to force a new connection.
link(pid, RemoteConnection::RECONNECT);
```

Note the `s/Try and/Link in order to/` to make this a bit clearer.


- Benjamin Mahler


On June 29, 2016, 12:48 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49346/
> ---
> 
> (Updated June 29, 2016, 12:48 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
> Gilbert Song, Artem Harutyunyan, and Jie Yu.
> 
> 
> Bugs: MESOS-5576
> https://issues.apache.org/jira/browse/MESOS-5576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/log/network.hpp 56c5dbb38eaeaa70735c47a2266b0dbebc42aa35 
> 
> Diff: https://reviews.apache.org/r/49346/diff/
> 
> 
> Testing
> ---
> 
> The new tests currently fail on Debian 8 and Ubuntu 12, 14, 15, 16.  But pass 
> on CentOS 6, 7.  Also breaks the cmake tests.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 49207: Added proto message definitions to support appc runtime.

[Guangya Liu]

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




include/mesos/appc/spec.proto (line 43)


Srini, I think that you need to kill this line in following patches.


- Guangya Liu


On 六月 28, 2016, 9:45 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49207/
> ---
> 
> (Updated 六月 28, 2016, 9:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proto message definitions to support appc runtime.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
> 
> Diff: https://reviews.apache.org/r/49207/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49175: Added tests for libprocess linking and unlinking behavior.

[Joseph Wu]

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

(Updated June 28, 2016, 7:28 p.m.)


Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
Artem Harutyunyan, and Jie Yu.


Changes
---

Change JIRA.


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


Repository: mesos


Description
---

Adds tests which exercise "link" semantics against remote processes.
This includes detection of `ExitedEvents` when the process exits
as well as mixing "link" semantics.

Includes a test case that emulates the failure observed in MESOS-5576.


Diffs
-

  3rdparty/libprocess/Makefile.am 3c3249fe9799ba919ac7bd13e2ddb07a306737f0 
  3rdparty/libprocess/src/tests/process_tests.cpp 
3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
  3rdparty/libprocess/src/tests/test_linkee.cpp PRE-CREATION 

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


Testing (updated)
---

See end of chain.

Need to fix a race in the tests...


Thanks,

Joseph Wu

Re: Review Request 49177: Added 'relink' semantics to ProcessBase::link.

[Joseph Wu]

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

(Updated June 28, 2016, 7:28 p.m.)


Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
Artem Harutyunyan, and Jie Yu.


Changes
---

Change JIRA.


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


Repository: mesos


Description
---

The `REMOTE_RELINK` option for `ProcessBase::link` will force the
`SocketManager` to create a new socket if a persistent link already
exists.


Diffs
-

  3rdparty/libprocess/include/process/process.hpp 
2946e50081c4a418a868083806a26f995c295007 
  3rdparty/libprocess/src/process.cpp 9bae71246e751e491be5a989eea8aca29c9aa751 

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


Testing
---

make check (OSX)


Thanks,

Joseph Wu

Re: Review Request 49174: Added test-only function for retrieving link sockets from libprocess.

[Joseph Wu]

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

(Updated June 28, 2016, 7:28 p.m.)


Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
Artem Harutyunyan, and Jie Yu.


Changes
---

Change JIRA.


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


Repository: mesos


Description
---

This can be used in a test to "break" a socket without libprocess's
explicit knowledge.  For example, we can disable transmission on a
persistent link.  The next message sent over that link will be dropped.


Diffs
-

  3rdparty/libprocess/src/process.cpp 9bae71246e751e491be5a989eea8aca29c9aa751 

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


Testing
---

See end of chain.


Thanks,

Joseph Wu

Re: Review Request 49321: GetState response protobuf.

[haosdent huang]

> On June 28, 2016, 10:50 p.m., Zhitao Li wrote:
> > include/mesos/master/master.proto, line 282
> > 
> >
> > Can elected_time actually be optional? Assuming we only serve from 
> > elected leader or error out in response, this could be required to be 
> > consistent with other fields.
> > 
> > It remains a question whether we want to make `GetFrameworks 
> > get_frameworks` optional: making it required might make future 
> > implementation of filters difficult, depending on how filters will be 
> > implemented.

Got it, let me drop :)


- haosdent


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


On June 28, 2016, 3:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49321/
> ---
> 
> (Updated June 28, 2016, 3:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GetState response protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
>   include/mesos/v1/master/master.proto 
> 681ea1e638777059a6bf792435cbe526bc252f7a 
> 
> Diff: https://reviews.apache.org/r/49321/diff/
> 
> 
> Testing
> ---
> 
> `make` on Mac
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49245: Implement READ_FILE for agent operator API.

[zhou xing]

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

(Updated 六月 29, 2016, 2:06 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

rebase and update code chain dependency


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

Implemented readFile method for agent operator API.


Diffs (updated)
-

  src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
  src/slave/slave.hpp 2afd7d152dcd2f5390014cd7bd4e926b62c292d1 
  src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49244: Implement READ_FILE for master operator API.

[zhou xing]

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

(Updated 六月 29, 2016, 2:06 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

rebase and update code chain dependency


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

Implemented readFile method for master operator API.


Diffs (updated)
-

  src/master/http.cpp 311db1a9400ab533f4536e7a7412122275a7044d 
  src/master/master.hpp e2ab2110fe5a287ab16ac9ef4222fed633e02ebe 
  src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49243: Create readFile method in FilesProcess.

[zhou xing]

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

(Updated 六月 29, 2016, 2:05 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

rebase and update code chain dependency


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

This method helps to readFiles based on offset, length
and path, which can be used for implementing master/agent's
READ_FILE operator v1 API.


Diffs (updated)
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49242: Add ReadFile protobuf message.

[zhou xing]

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

(Updated 六月 29, 2016, 2:04 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

rebase and update code chain dependency


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description
---

Add ReadFile message in the Response message of master.proto
, v1/master.proto, agent.proto, v1/agent.proto, mesos.proto
and v1/mesos.proto.


Diffs (updated)
-

  include/mesos/agent/agent.proto 538d12f71df1943f91bafb99650625aa910affaa 
  include/mesos/master/master.proto 798b536927cc707eb2912b81a924a77c932461db 
  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/agent/agent.proto 48f15173fe62b9ce7f648f6b54d74ec62f797c55 
  include/mesos/v1/master/master.proto 93157d57dcc53b54fed2ebbc4772c689ddba2119 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49301: Added FilesError class.

[zhou xing]

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

(Updated 六月 29, 2016, 2:01 a.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


Changes
---

update code per Anand's comments


Bugs: mesos-5515
https://issues.apache.org/jira/browse/mesos-5515


Repository: mesos


Description (updated)
---

FilesError class represents the various errors that can be
returned by methods on the `Files` class via a `Try` that
has failed.


Diffs (updated)
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 

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


Testing
---

make
make check


Thanks,

zhou xing

Re: Review Request 49336: Renamed `agent_id` fields to `slave_id` for unversioned master.proto.

[Jay Guo]

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


Ship it!




Ship It!

- Jay Guo


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49336/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change renames the `agent_id` to `slave_id` for the
> unversioned protos to keep it consistent to other unversioned
> protobuf files. Also, fixed a couple of minor issues of not
> capturing aliases by const references for consistency.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto b9936521628238fd3d215c6a05423ebbda1e94d7 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 49315: Avoided unnecessary cast of `dispatch` in deferred.hpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46686, 46687, 49315]

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

- Mesos ReviewBot


On June 28, 2016, 6:49 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49315/
> ---
> 
> (Updated June 28, 2016, 6:49 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Michael Park.
> 
> 
> Bugs: MESOS-4611
> https://issues.apache.org/jira/browse/MESOS-4611
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided unnecessary cast of `dispatch` in deferred.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/deferred.hpp 
> 18ba979b4ee4bb417a80710c647f7d24e43deeeb 
> 
> Diff: https://reviews.apache.org/r/49315/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49337: Moved code to group old/v1 operator API implementations together.

[Jay Guo]

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


Ship it!




Ship It!

- Jay Guo


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49337/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moves the create/destroy volumes, reserve/unreserve
> for the v1 API to be closer to the corresponding implementations
> for the old API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49337/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 49335: Moved the agent exists check to `_reserve/_unreserve`.

[Jay Guo]

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


Ship it!




Ship It!

- Jay Guo


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49335/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moved the agent exists check that is common to
> both `/reserve` and the new API to `_reserve`. This is
> consistent to how the `/create-volumes` already does it.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49335/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 49346: Changed the replog's network abstraction to use 'relink'.

[Joseph Wu]

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

(Updated June 28, 2016, 5:48 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
Gilbert Song, Artem Harutyunyan, and Jie Yu.


Changes
---

Update testing.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/log/network.hpp 56c5dbb38eaeaa70735c47a2266b0dbebc42aa35 

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


Testing (updated)
---

The new tests currently fail on Debian 8 and Ubuntu 12, 14, 15, 16.  But pass 
on CentOS 6, 7.  Also breaks the cmake tests.


Thanks,

Joseph Wu

Re: Review Request 48374: Added class to share Nvidia-specific components between containerizers.

[Kevin Klues]

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

(Updated June 29, 2016, 12:25 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased


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


Repository: mesos


Description (updated)
---

Previously, the `NvidiaGpuAllocator` component was created directly
inside the `NvidiaGpuIsolatorProcess` even though it was designed to
be used by multiple containerizers at the same time. To allow for
simultaneous access, we created a new `NvidiaComponents` class to hold
an `NvidiaGpuAllocator` instance and pass it to each containerizer as
it is created. This component can easily be extended to include more
cross-containerizer components later on.

We create the `NvidiaComponents` instance inside
`Containerizer::create()` and pass it to both the docker containerizer
and the mesos containerizer when they are created.  The docker
containerizer currently doesn't do anything with this information, but
its interface has been updated to accomodate it. The mesos
containerizer has been updated to pass this all the way down to the
`NvidiaGpuIsolatorProcess` and exploit it.


Diffs (updated)
-

  src/Makefile.am 86c39fdf379ada470c9b1f86be263ef71dc47c41 
  src/slave/containerizer/containerizer.cpp 
2449526061f7d6ac46ff42260ccdd0278694d9d4 
  src/slave/containerizer/docker.hpp 9b0a5a76e457006119e657ee8f627dcd1326c0ce 
  src/slave/containerizer/docker.cpp 514268fd84507eafa11fc57d38a23b0502f80ef8 
  src/slave/containerizer/mesos/containerizer.hpp 
a1a00020668f6da8d611f26e5637afffc87d09ba 
  src/slave/containerizer/mesos/containerizer.cpp 
d984efd4742ec084d66538c48a36ea768832324d 
  src/slave/containerizer/mesos/isolators/gpu/components.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
e35a55fdf44ec01d49d6a9e396a88f4163ef79c3 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
b8753ec8fc0afc8f2b389250de57d5095287acf9 

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


Testing
---

GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 49308: Added agent and scheduler authentication backoff.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49308]

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

- Mesos ReviewBot


On June 28, 2016, 1:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49308/
> ---
> 
> (Updated June 28, 2016, 1:16 p.m.)
> 
> 
> Review request for mesos, Adam B and Vinod Kone.
> 
> 
> Bugs: MESOS-2043
> https://issues.apache.org/jira/browse/MESOS-2043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The backoff follows to existing pattern for backoff used during agent
> or scheduler registration where we backoff for some random time in an
> interval of increasing length, capped by
> `REGISTER_RETRY_INTERVAL_MAX`.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 9f561d73a2e591afdc3ba4adb35a11763dced402 
>   src/slave/slave.hpp 2afd7d152dcd2f5390014cd7bd4e926b62c292d1 
>   src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
> 
> Diff: https://reviews.apache.org/r/49308/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X w/o optimizations).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 49223: Enhanced Value parsing.

[Klaus Ma]

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

(Updated June 29, 2016, 7:58 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update JIRA #.


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


Repository: mesos


Description
---

Enhanced Value parsing:

1. Did not support `[1-2, [3-4]]` as Ranges; it should be `[1-2, 3-4]`.
2. Did not support `{a{b, c}d}` as Set; it should be `{ab, cd}`
3. Add check for Text against `[a-zA-Z0-9_/.-]`


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/attributes_tests.cpp cb71be5ecead322d90943146f54f8a0c915eba1c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 49346: Changed the replog's network abstraction to use 'relink'.

[Joseph Wu]

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, Benjamin Mahler, 
Gilbert Song, Artem Harutyunyan, and Jie Yu.


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


Repository: mesos


Description
---

Changed the replog's network abstraction to use 'relink'.


Diffs
-

  src/log/network.hpp 56c5dbb38eaeaa70735c47a2266b0dbebc42aa35 

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


Testing
---


Thanks,

Joseph Wu

Re: Review Request 49175: Added tests for libprocess linking and unlinking behavior.

[Joseph Wu]

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

(Updated June 28, 2016, 4:30 p.m.)


Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
Artem Harutyunyan, and Jie Yu.


Changes
---

Addressed comments: greatly improved test robustness and comments.


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


Repository: mesos


Description
---

Adds tests which exercise "link" semantics against remote processes.
This includes detection of `ExitedEvents` when the process exits
as well as mixing "link" semantics.

Includes a test case that emulates the failure observed in MESOS-5576.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 3c3249fe9799ba919ac7bd13e2ddb07a306737f0 
  3rdparty/libprocess/src/tests/process_tests.cpp 
3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
  3rdparty/libprocess/src/tests/test_linkee.cpp PRE-CREATION 

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


Testing
---

See end of chain.


Thanks,

Joseph Wu

Re: Review Request 49175: Added tests for libprocess linking and unlinking behavior.

[Joseph Wu]

> On June 27, 2016, 3:41 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, line 745
> > 
> >
> > Let's not bother with this anymore because it seems to have 
> > proliferated via being copied around and we should be checking this at the 
> > start of the libprocess test main. However, I'm not sure we'd even compile 
> > if this was false given it seems to correspond to whether we have pthread 
> > support.

Cleaned up the `ProcessTest.Exited` and `ProcessTest.InjectExited` too.


> On June 27, 2016, 3:41 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, lines 805-815
> > 
> >
> > The status being satisfied does not necessarily entail that the exited 
> > message is delivered since settling the clock does not ensure that socket 
> > events are processed.
> > 
> > Can you use a Future instead of the atomic_bool?

Cleaned up the `ProcessTest.Exited` and `ProcessTest.InjectExited` too.


> On June 27, 2016, 3:41 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp, lines 860-861
> > 
> >
> > Can you put these inside initialize, here and below?

I ended up consolidating the various helper actors into one 
`RemoteLinkTestProcess`.  I use this by simply calling `linkup` (can't conflict 
with `link`), `relink`, or `ping_linkee`.


- Joseph


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


On June 28, 2016, 4:30 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49175/
> ---
> 
> (Updated June 28, 2016, 4:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
> Artem Harutyunyan, and Jie Yu.
> 
> 
> Bugs: MESOS-5576
> https://issues.apache.org/jira/browse/MESOS-5576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds tests which exercise "link" semantics against remote processes.
> This includes detection of `ExitedEvents` when the process exits
> as well as mixing "link" semantics.
> 
> Includes a test case that emulates the failure observed in MESOS-5576.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 3c3249fe9799ba919ac7bd13e2ddb07a306737f0 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
>   3rdparty/libprocess/src/tests/test_linkee.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49175/diff/
> 
> 
> Testing
> ---
> 
> See end of chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 49177: Added 'relink' semantics to ProcessBase::link.

[Joseph Wu]

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

(Updated June 28, 2016, 4:30 p.m.)


Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
Artem Harutyunyan, and Jie Yu.


Changes
---

Rework the enum naming and comments.  And fix the mistaken indenting of the 
link code (would have broken things otherwise :)


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


Repository: mesos


Description
---

The `REMOTE_RELINK` option for `ProcessBase::link` will force the
`SocketManager` to create a new socket if a persistent link already
exists.


Diffs (updated)
-

  3rdparty/libprocess/include/process/process.hpp 
2946e50081c4a418a868083806a26f995c295007 
  3rdparty/libprocess/src/process.cpp 9bae71246e751e491be5a989eea8aca29c9aa751 

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


Testing
---

make check (OSX)


Thanks,

Joseph Wu

Re: Review Request 49174: Added test-only function for retrieving link sockets from libprocess.

[Joseph Wu]

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

(Updated June 28, 2016, 4:30 p.m.)


Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
Artem Harutyunyan, and Jie Yu.


Changes
---

s/[...]/.at(...)/


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


Repository: mesos


Description
---

This can be used in a test to "break" a socket without libprocess's
explicit knowledge.  For example, we can disable transmission on a
persistent link.  The next message sent over that link will be dropped.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 9bae71246e751e491be5a989eea8aca29c9aa751 

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


Testing
---

See end of chain.


Thanks,

Joseph Wu

Review Request 49348: Added basic test to invoke Appc Runtime Isolator.

[Srinivas Brahmaroutu]

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

Review request for mesos.


Repository: mesos


Description
---

Added basic test to invoke Appc Runtime Isolator.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
061f80c62319817b22a5c1880a4858fdafbfb72a 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49344: Removed 'override' flag in command executor.

[Jie Yu]

> On June 28, 2016, 10:48 p.m., Benjamin Mahler wrote:
> > src/launcher/posix/executor.cpp, lines 74-78
> > 
> >
> > The naked `.get()` seems unfortunate but this does make the format 
> > easier to read. Wonder if you want to do:
> > 
> > ```
> > Try format = strings::format(
> > "%s %s '%s'",
> > os::Shell::arg0,
> > os::Shell::arg1,
> > command.value());
> > 
> > CHECK_SOME(format);
> > 
> > commandString = format.get();
> > ```
> > 
> > Almost like we need an equivalent of `T* CHECK_NOTNULL(T*)` to make 
> > this easier:
> > 
> > ```
> > commandString = CHECK_NOTERROR(strings::format(
> > "%s %s '%s'",
> > os::Shell::arg0,
> > os::Shell::arg1,
> > command.value()));
> > ```
> > 
> > Just some food for thought.

Good idea. Created https://issues.apache.org/jira/browse/MESOS-5738 to track


- Jie


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


On June 28, 2016, 10:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49344/
> ---
> 
> (Updated June 28, 2016, 10:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, Ian Downes, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed 'override' flag in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 939a16571bd7123b9561483444880e12a04f4cd0 
>   src/launcher/posix/executor.hpp 76072454a93a4ecb58ff3666923fc593a4f26b2a 
>   src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 
>   src/launcher/windows/executor.hpp eb37f929fc5f5994e109642c7bd56c148c68fd43 
>   src/launcher/windows/executor.cpp 81ae4b0a53b128f2d9621537bf359880aff625e9 
>   src/tests/slave_tests.cpp 6d80b2d819660123454c78f52e995b32926b0e5d 
> 
> Diff: https://reviews.apache.org/r/49344/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49321: GetState response protobuf.

[Zhitao Li]

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




include/mesos/master/master.proto (line 282)


Can elected_time actually be optional? Assuming we only serve from elected 
leader or error out in response, this could be required to be consistent with 
other fields.

It remains a question whether we want to make `GetFrameworks 
get_frameworks` optional: making it required might make future implementation 
of filters difficult, depending on how filters will be implemented.



include/mesos/master/master.proto (lines 283 - 284)


I still prefer the approach in current patch because it makes it clear that 
`GetState` is just composed with other `Get*` responses, and there should be no 
symantic or behavior difference (e.g. authorization, filter, etc).

It seems like `orphan_tasks` are only reported in `GetState` but no other 
calls right now. Maybe we should consider add that, to be consistent with other 
calls?



include/mesos/v1/master/master.proto (line 283)


See comments above.


- Zhitao Li


On June 28, 2016, 3:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49321/
> ---
> 
> (Updated June 28, 2016, 3:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GetState response protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
>   include/mesos/v1/master/master.proto 
> 681ea1e638777059a6bf792435cbe526bc252f7a 
> 
> Diff: https://reviews.apache.org/r/49321/diff/
> 
> 
> Testing
> ---
> 
> `make` on Mac
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49219: Added runtime isolator interface to run appc containers.

[Srinivas Brahmaroutu]

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

(Updated June 28, 2016, 10:53 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added runtime isolator interface to run appc containers.


Diffs (updated)
-

  src/CMakeLists.txt 9c80a613818302ad3b417d582e9a91a59a6f666d 
  src/Makefile.am 86c39fdf379ada470c9b1f86be263ef71dc47c41 
  src/slave/containerizer/mesos/isolators/appc/runtime.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 

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


Testing
---

Make Check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

[Srinivas Brahmaroutu]

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

(Updated June 28, 2016, 10:53 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
  src/slave/containerizer/mesos/containerizer.cpp 
d984efd4742ec084d66538c48a36ea768832324d 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
249acad49122d988e44744384bcf840b941c0997 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

[Srinivas Brahmaroutu]

> On June 28, 2016, 3:38 p.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp, lines 334-335
> > 
> >
> > Can we simplify the logic as:
> > 
> > if (imageInfo.layers.size() == 1) {
> >   return ProvisionInfo{
> > rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};
> > }

I try not to generalize. I like to keep the checks in case other containerizers 
supported.


- Srinivas


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


On June 28, 2016, 6:39 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated June 28, 2016, 6:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49313: Added the VIEW_FLAGS authorization action.

[Vinod Kone]

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




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


s/command/command-line/



src/master/http.cpp (lines 1241 - 1262)


flags specific authorization should be done in `_flags()` so that the v1 
API can get the benefit automatically. 

please move this logic inside `_flags()`. you might need to adjust its 
return type and add a `__flags()`.


- Vinod Kone


On June 28, 2016, 12:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49313/
> ---
> 
> (Updated June 28, 2016, 12:07 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-5705
> https://issues.apache.org/jira/browse/MESOS-5705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds again authorization for flags. Instead of being part of
> `get_endpoints` it uses its own action `VIEW_TASKS` which is
> used to restrict access to the `/flags` endpoint, as well as
> to filter the results of the `/state` endpoint on both master
> and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> a6d93cd2cb9161a98565b22e50b06aac4931a671 
>   include/mesos/authorizer/authorizer.proto 
> fc76796022a6fa3d36a1447c476980868d42c2d0 
>   src/authorizer/local/authorizer.cpp 
> 3fade4168face1cb80b30c9b69b31d9eb4126222 
>   src/common/http.hpp 55bd0ac81af80c656a4a80766a3e4b21db9cf0cf 
>   src/common/http.cpp 95e8fb672b49a00860c64f818cc734fa22cf3516 
>   src/master/http.cpp e089fe960779f39b3321f2ec81ab2acc17d53641 
>   src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
>   src/tests/authorization_tests.cpp 9b99da138fa27a725738d70bd99e889b108b44ae 
>   src/tests/master_authorization_tests.cpp 
> 81804e0522fd6b26155732af08e33c4d0bb0a8fe 
>   src/tests/slave_authorization_tests.cpp 
> 78221e200d9b7880cc474f1acef92c5dec1c8e25 
> 
> Diff: https://reviews.apache.org/r/49313/diff/
> 
> 
> Testing
> ---
> 
> - `make check`
> - manual tests with browsers.
> - Used the script:
>  
> ```sh
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "view_flags" : [
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "ANY" }
>},
>{
>  "principals" : { "values" : ["foo"] },
>  "flags" : { "type" : "NONE" }
>}
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>  --authenticate_http \
>  --log_dir=/tmp/mesos/logs/master \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> 
> # Returns a 200 OK Response with the contents of the flags
> # in JSON object
> http GET http://127.0.0.1:5050/flags -a foo:bar
> http GET http://127.0.0.1:5051/flags -a foo:bar
> 
> # Returned JSON contains a `flags` entry with all the flags.
> http GET http://127.0.0.1:5050/state -a foo:bar
> http GET http://127.0.0.1:5051/state -a foo:bar
> 
> # 403 Forbidden response
> http GET http://127.0.0.1:5050/flags -a baz:bar
> http GET http://127.0.0.1:5051/flags -a baz:bar
> 
> # Returned JSON doesn't include flags information.
> http GET http://127.0.0.1:5050/state -a baz:bar
> http GET http://127.0.0.1:5051/state -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 49313: Added the VIEW_FLAGS authorization action.

[Alexander Rojas]

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

(Updated June 28, 2016, 10:51 p.m.)


Review request for mesos, Adam B, Joerg Schad, Till Toenshoff, and Vinod Kone.


Changes
---

fixed a typo in the description -- @vinodkone


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


Repository: mesos


Description (updated)
---

Adds again authorization for flags. Instead of being part of
`get_endpoints` it uses its own action `VIEW_FLAGS` which is
used to restrict access to the `/flags` endpoint, as well as
to filter the results of the `/state` endpoint on both master
and agents.


Diffs
-

  include/mesos/authorizer/acls.proto a6d93cd2cb9161a98565b22e50b06aac4931a671 
  include/mesos/authorizer/authorizer.proto 
fc76796022a6fa3d36a1447c476980868d42c2d0 
  src/authorizer/local/authorizer.cpp 3fade4168face1cb80b30c9b69b31d9eb4126222 
  src/common/http.hpp 55bd0ac81af80c656a4a80766a3e4b21db9cf0cf 
  src/common/http.cpp 95e8fb672b49a00860c64f818cc734fa22cf3516 
  src/master/http.cpp e089fe960779f39b3321f2ec81ab2acc17d53641 
  src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
  src/tests/authorization_tests.cpp 9b99da138fa27a725738d70bd99e889b108b44ae 
  src/tests/master_authorization_tests.cpp 
81804e0522fd6b26155732af08e33c4d0bb0a8fe 
  src/tests/slave_authorization_tests.cpp 
78221e200d9b7880cc474f1acef92c5dec1c8e25 

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


Testing
---

- `make check`
- manual tests with browsers.
- Used the script:
 
```sh
#! /usr/bin/env bash

rm -rf /tmp/mesos/*

cat < /tmp/credentials.txt
foo bar
baz bar
EOF

cat < /tmp/acls.json
{
  "permissive": false,
  "view_flags" : [
   {
 "principals" : { "values" : ["foo"] },
 "flags" : { "type" : "ANY" }
   },
   {
 "principals" : { "values" : ["foo"] },
 "flags" : { "type" : "NONE" }
   }
  ]
}
EOF

./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
 --authenticate_http \
 --log_dir=/tmp/mesos/logs/master \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &
./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
 --master=127.0.0.1:5050 \
 --authenticate_http \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json &

# Returns a 200 OK Response with the contents of the flags
# in JSON object
http GET http://127.0.0.1:5050/flags -a foo:bar
http GET http://127.0.0.1:5051/flags -a foo:bar

# Returned JSON contains a `flags` entry with all the flags.
http GET http://127.0.0.1:5050/state -a foo:bar
http GET http://127.0.0.1:5051/state -a foo:bar

# 403 Forbidden response
http GET http://127.0.0.1:5050/flags -a baz:bar
http GET http://127.0.0.1:5051/flags -a baz:bar

# Returned JSON doesn't include flags information.
http GET http://127.0.0.1:5050/state -a baz:bar
http GET http://127.0.0.1:5051/state -a baz:bar
```


Thanks,

Alexander Rojas

Re: Review Request 49344: Removed 'override' flag in command executor.

[Benjamin Mahler]

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


Ship it!





src/launcher/executor.cpp (lines 363 - 364)


Mind putting the quotes on a single line?



src/launcher/posix/executor.cpp (lines 73 - 77)


The naked `.get()` seems unfortunate but this does make the format easier 
to read. Wonder if you want to do:

```
Try format = strings::format(
"%s %s '%s'",
os::Shell::arg0,
os::Shell::arg1,
command.value());

CHECK_SOME(format);

commandString = format.get();
```

Almost like we need an equivalent of `T* CHECK_NOTNULL(T*)` to make this 
easier:

```
commandString = CHECK_NOTERROR(strings::format(
"%s %s '%s'",
os::Shell::arg0,
os::Shell::arg1,
command.value()));
```

Just some food for thought.


- Benjamin Mahler


On June 28, 2016, 10:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49344/
> ---
> 
> (Updated June 28, 2016, 10:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, Ian Downes, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed 'override' flag in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 939a16571bd7123b9561483444880e12a04f4cd0 
>   src/launcher/posix/executor.hpp 76072454a93a4ecb58ff3666923fc593a4f26b2a 
>   src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 
>   src/launcher/windows/executor.hpp eb37f929fc5f5994e109642c7bd56c148c68fd43 
>   src/launcher/windows/executor.cpp 81ae4b0a53b128f2d9621537bf359880aff625e9 
>   src/tests/slave_tests.cpp 6d80b2d819660123454c78f52e995b32926b0e5d 
> 
> Diff: https://reviews.apache.org/r/49344/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49201: Added validation for the `get_endpoints` ACL.

[Vinod Kone]

> On June 27, 2016, 11:11 p.m., Vinod Kone wrote:
> > src/authorizer/local/authorizer.cpp, lines 765-770
> > 
> >
> > i'm afraid this will be hard to keep this hashset in sync with the 
> > reality. is there a way to enforce that this is up to date?
> 
> Alexander Rojas wrote:
> As things are implemented right now, there is no way to enforce that the 
> list is up to date. While a couple of endpoints (`/logging/toggle` and 
> `/metrics/snapshot`) use the `createAuthorizationCallbacks()` function, and 
> therefore a central registry in the way of the returned callbacks. 
> Application level methods either do it manually or through the helper 
> function `Slave::Http::authorizeEndpoint`. So really, the `validPaths` here 
> would be the ultimate source of truth unless we redesign the whole system.

AFAICT, `authorizeEndpoint()` and `createAuthorizationCallbacks()` are the only 
two methods (three if you consider the former in both master and slave) that 
need access to this map, in addition to the local authorizer. I don't see any 
other methods that verify this authorization manually? So I would suggest to 
have a `AUTHORIZABLE_ENDPOINTS` in common/http.cpp and have the above two 
functions (and local authorizer) check that the path being authorized is a 
valid path. Note that this helps us being defensive against the case when using 
custom authorizer is being used.

Also, this merits a comment in authorizer.proto.


- Vinod


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


On June 28, 2016, 8:46 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49201/
> ---
> 
> (Updated June 28, 2016, 8:46 a.m.)
> 
> 
> Review request for mesos, Adam B, Jan Schlicht, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5707
> https://issues.apache.org/jira/browse/MESOS-5707
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The fact that not all endpoints can be secure through ACLs, yet
> the ACL is called `get_endpoints`, can be confusing for operators.
> Therefore, if an operator tries to launch an agent/master with an
> invalid configuration an error is generated.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 2c20a3069dc000b6674ac15046edd9213e79a632 
>   src/tests/authorization_tests.cpp 9b99da138fa27a725738d70bd99e889b108b44ae 
> 
> Diff: https://reviews.apache.org/r/49201/diff/
> 
> 
> Testing
> ---
> 
> `make check` and following scripts:
> 
> ```sh
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "get_endpoints" : [
>{
>  "principals" : { "values" : ["foo"] },
>  "paths" : { "values" : ["/frameworks"] }
>}
>   ]
> }
> EOF
> 
> # Fails to start up with a message saying that `/frameworks`
> # ins't supported.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> 
> ```
> 
> and
> 
> 
> ```sh
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "get_endpoints" : [
>{
>  "principals" : { "values" : ["foo"] },
>  "paths" : { "values" : ["/monitor/statistics", 
> "/monitor/statistics.json", "/containers"] }
>}
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>  --authenticate_http \
>  --log_dir=/tmp/mesos/logs/master \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json &
> 
> # Following requests succeed (200 OK response)
> http http://localhost:5051/monitor/statistics -a foo:bar
> http http://localhost:5051/monitor/statistics.json -a foo:bar
> http http://localhost:5051/monitor/containers -a foo:bar
> 
> # Following requests fail (403 forbidden response)
> http http://localhost:5051/monitor/statistics -a baz:bar
> http http://localhost:5051/monitor/statistics.json -a baz:bar
> http http://localhost:5051/monitor/containers -a baz:bar
> 
> ```
> 
> 
> Thanks,
> 
> 

Re: Review Request 49343: Fixed an indentation issue in shell.hpp.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 28, 2016, 10:15 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49343/
> ---
> 
> (Updated June 28, 2016, 10:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed an indentation issue in shell.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/shell.hpp 
> d4753cf4a9896c0948c02faf7a9bdaf05c2f584e 
> 
> Diff: https://reviews.apache.org/r/49343/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49341: Simplified setsid logic in command executor.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/launcher/posix/executor.cpp (line 103)


Looks like the `strerror(errrno)` was lost (used to be captured via 
`perror`.


- Benjamin Mahler


On June 28, 2016, 10:14 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49341/
> ---
> 
> (Updated June 28, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The setsid loop is not necessary. POSIX standard suggests that the pid
> of the child should not be equal to any active process group id.
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 
> 
> Diff: https://reviews.apache.org/r/49341/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49342: Fixed an indentation issue in command executor.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 28, 2016, 10:14 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49342/
> ---
> 
> (Updated June 28, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed an indentation issue in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 
> 
> Diff: https://reviews.apache.org/r/49342/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49311: Synchronized endpoint documentation by running script.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49311]

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

- Mesos ReviewBot


On June 28, 2016, 11:45 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49311/
> ---
> 
> (Updated June 28, 2016, 11:45 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Synchronized endpoint documentation by running script.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/flags.md 850b6f86ddd92af512932aec5af9bdc600cf92d8 
>   docs/endpoints/slave/api/v1.md c867d795104e053e5aae819df3deea68269228bf 
>   docs/endpoints/slave/flags.md 3ea38c160670fc302a42ae98d7ebe0679b5bdc92 
> 
> Diff: https://reviews.apache.org/r/49311/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 49344: Removed 'override' flag in command executor.

[Jie Yu]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, Ian Downes, and Vinod 
Kone.


Repository: mesos


Description
---

Removed 'override' flag in command executor.


Diffs
-

  src/launcher/executor.cpp 939a16571bd7123b9561483444880e12a04f4cd0 
  src/launcher/posix/executor.hpp 76072454a93a4ecb58ff3666923fc593a4f26b2a 
  src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 
  src/launcher/windows/executor.hpp eb37f929fc5f5994e109642c7bd56c148c68fd43 
  src/launcher/windows/executor.cpp 81ae4b0a53b128f2d9621537bf359880aff625e9 
  src/tests/slave_tests.cpp 6d80b2d819660123454c78f52e995b32926b0e5d 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 49343: Fixed an indentation issue in shell.hpp.

[Jie Yu]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.


Repository: mesos


Description
---

Fixed an indentation issue in shell.hpp.


Diffs
-

  3rdparty/stout/include/stout/os/posix/shell.hpp 
d4753cf4a9896c0948c02faf7a9bdaf05c2f584e 

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


Testing
---

make check


Thanks,

Jie Yu

Review Request 49342: Fixed an indentation issue in command executor.

[Jie Yu]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.


Repository: mesos


Description
---

Fixed an indentation issue in command executor.


Diffs
-

  src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 49341: Simplified setsid logic in command executor.

[Jie Yu]

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

(Updated June 28, 2016, 10:14 p.m.)


Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.


Repository: mesos


Description
---

The setsid loop is not necessary. POSIX standard suggests that the pid
of the child should not be equal to any active process group id.


Diffs
-

  src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 

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


Testing (updated)
---

make check


Thanks,

Jie Yu

Review Request 49341: Simplified setsid logic in command executor.

[Jie Yu]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.


Repository: mesos


Description
---

The setsid loop is not necessary. POSIX standard suggests that the pid
of the child should not be equal to any active process group id.


Diffs
-

  src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 

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


Testing
---


Thanks,

Jie Yu

Re: Review Request 49095: Enabled fine-grained authorization in the master's frameworks endpoint.

[Vinod Kone]

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


Fix it, then Ship it!




i'll fix the raised issue while committing.


src/master/http.cpp (lines 1207 - 1208)


move `Framework*` to the above line and indent accordingly.


- Vinod Kone


On June 28, 2016, 1:17 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49095/
> ---
> 
> (Updated June 28, 2016, 1:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff.
> 
> 
> Bugs: mesos-5704
> https://issues.apache.org/jira/browse/mesos-5704
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Even if ACLs were defined for the actions `VIEW_FRAMEWORKS`,
> `VIEW_EXECUTORS` and `VIEW_TASKS`, the data these actions were
> supposed to protect, could still leaked through the master's
> `/frameworks` endpoint, since it didn't enable any authorization
> mechanism.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e089fe960779f39b3321f2ec81ab2acc17d53641 
>   src/tests/master_authorization_tests.cpp 
> 81804e0522fd6b26155732af08e33c4d0bb0a8fe 
> 
> Diff: https://reviews.apache.org/r/49095/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 49208: Added tests to check if appc spec is properly parsed.

[Srinivas Brahmaroutu]

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

(Updated June 28, 2016, 10:13 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added tests to check if appc spec is properly parsed.


Diffs (updated)
-

  src/Makefile.am 86c39fdf379ada470c9b1f86be263ef71dc47c41 
  src/tests/containerizer/appc_spec_tests.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_appc_tests.cpp 
061f80c62319817b22a5c1880a4858fdafbfb72a 

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


Testing
---

Make check


Thanks,

Srinivas Brahmaroutu

Review Request 49340: Used ABORT consistently in command executor.

[Jie Yu]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.


Repository: mesos


Description
---

Used ABORT consistently in command executor.


Diffs
-

  src/launcher/executor.cpp 939a16571bd7123b9561483444880e12a04f4cd0 
  src/launcher/posix/executor.cpp b393db6c0631b5b5c43fb4d3d9183d9100e31049 
  src/launcher/windows/executor.cpp 81ae4b0a53b128f2d9621537bf359880aff625e9 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 49207: Added proto message definitions to support appc runtime.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 28, 2016, 9:45 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49207/
> ---
> 
> (Updated June 28, 2016, 9:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proto message definitions to support appc runtime.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
> 
> Diff: https://reviews.apache.org/r/49207/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49321: GetState response protobuf.

[Vinod Kone]

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




include/mesos/master/master.proto (lines 283 - 284)


hmm. seeing `get_frameworks`, `get_agents` and then `orphan_tasks` seems a 
bit weird. maybe the one we were discussing earlier reads better?

```
message GetState {

required TimeInfo start_time = 1;
required TimeInfo elected_time = 2;

repeated GetFrameworks.framework frameworks = 3;
repeated GetFrameworks.framework completed_frameworks = 3;
repeated FrameworkID unregistered_frameworks = 4;

repeated GetAgents.Agent agents = 5;

repeated Task orphan_tasks = 6;

}

```

also i'm not really a big fan of `GetFrameworks.framework` and 
`GetAgents.Agent`. not sure if that is weird or pulling `Framework` and `Agent` 
into top level protos (at the same level as `Call` and `Response` or top level 
inside `Response`) is more weird.



include/mesos/master/master.proto (line 319)


i updated these. can you rebase?


- Vinod Kone


On June 28, 2016, 3:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49321/
> ---
> 
> (Updated June 28, 2016, 3:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GetState response protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
>   include/mesos/v1/master/master.proto 
> 681ea1e638777059a6bf792435cbe526bc252f7a 
> 
> Diff: https://reviews.apache.org/r/49321/diff/
> 
> 
> Testing
> ---
> 
> `make` on Mac
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49207: Added proto message definitions to support appc runtime.

[Srinivas Brahmaroutu]

> On June 25, 2016, 9:16 a.m., Guangya Liu wrote:
> >

@gyliu, This spec is referred at the top of the file. I am dropping this issue.


- Srinivas


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


On June 28, 2016, 9:45 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49207/
> ---
> 
> (Updated June 28, 2016, 9:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proto message definitions to support appc runtime.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
> 
> Diff: https://reviews.apache.org/r/49207/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49207: Added proto message definitions to support appc runtime.

[Srinivas Brahmaroutu]

> On June 28, 2016, 1:11 p.m., Guangya Liu wrote:
> > include/mesos/appc/spec.proto, lines 44-46
> > 
> >
> > I saw that you dropped previous comments, but it would be great if you 
> > can show some comments to explain when you want to drop some comments ;-)
> > 
> > I would suggest that you add a link as following here as comment so 
> > that people will clear where does those field from.
> >  
> > https://github.com/appc/spec/blob/master/spec/aci.md#image-manifest-schema
> >  
> > Other comments are:
> > 1) Please always add a period for all of your comments.
> > 2) Always start with a upper case charactor for a sentense, for your 
> > case, it would be /*TODO(srbrahma):  Would x*/ , but I think the 
> > comments here needs to be updated.

@Gyliu, I dropped the comments because it does not make sense. I did add a 
comment to explain that. Since the whole proto conforms to Appc spec I think 
calling out missing fields is not necessary.


- Srinivas


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


On June 28, 2016, 9:45 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49207/
> ---
> 
> (Updated June 28, 2016, 9:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proto message definitions to support appc runtime.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
> 
> Diff: https://reviews.apache.org/r/49207/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49207: Added proto message definitions to support appc runtime.

[Srinivas Brahmaroutu]

> On June 26, 2016, 1:57 a.m., Guangya Liu wrote:
> > include/mesos/appc/spec.proto, line 50
> > 
> >
> > What about `optional Environment environment = 3;`

We thought of duplicating "Label" as another message "Environment". For 
simplicity we keep it like this for now.


- Srinivas


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


On June 28, 2016, 9:45 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49207/
> ---
> 
> (Updated June 28, 2016, 9:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proto message definitions to support appc runtime.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
> 
> Diff: https://reviews.apache.org/r/49207/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49337: Moved code to group old/v1 operator API implementations together.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49337/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moves the create/destroy volumes, reserve/unreserve
> for the v1 API to be closer to the corresponding implementations
> for the old API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49337/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 49336: Renamed `agent_id` fields to `slave_id` for unversioned master.proto.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49336/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change renames the `agent_id` to `slave_id` for the
> unversioned protos to keep it consistent to other unversioned
> protobuf files. Also, fixed a couple of minor issues of not
> capturing aliases by const references for consistency.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto b9936521628238fd3d215c6a05423ebbda1e94d7 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 49335: Moved the agent exists check to `_reserve/_unreserve`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 28, 2016, 8:58 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49335/
> ---
> 
> (Updated June 28, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change moved the agent exists check that is common to
> both `/reserve` and the new API to `_reserve`. This is
> consistent to how the `/create-volumes` already does it.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 
> 
> Diff: https://reviews.apache.org/r/49335/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 49336: Renamed `agent_id` fields to `slave_id` for unversioned master.proto.

[Anand Mazumdar]

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

Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.


Repository: mesos


Description
---

This change renames the `agent_id` to `slave_id` for the
unversioned protos to keep it consistent to other unversioned
protobuf files. Also, fixed a couple of minor issues of not
capturing aliases by const references for consistency.


Diffs
-

  include/mesos/master/master.proto b9936521628238fd3d215c6a05423ebbda1e94d7 
  src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 49337: Moved code to group old/v1 operator API implementations together.

[Anand Mazumdar]

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

Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.


Repository: mesos


Description
---

This change moves the create/destroy volumes, reserve/unreserve
for the v1 API to be closer to the corresponding implementations
for the old API.


Diffs
-

  src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 49335: Moved the agent exists check to `_reserve/_unreserve`.

[Anand Mazumdar]

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

Review request for mesos, Abhishek Dasgupta, Jay Guo, and Vinod Kone.


Repository: mesos


Description
---

This change moved the agent exists check that is common to
both `/reserve` and the new API to `_reserve`. This is
consistent to how the `/create-volumes` already does it.


Diffs
-

  src/master/http.cpp e827e6948a7eec51c914975e8b77a094844cb63f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 49297: Use defer() to garantee getTasks lambda execution context.

[Vinod Kone]

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



Maybe it's better for getTasks() to have it's own independation implementation 
instead of depending on the factored out `_tasks()`. getAgents() and 
getFrameworks() have their own implementations anyway. We can also revert the 
`_tasks()` split. We need to add comments for why we have own independent 
implementations for getTasks(), getAgents() and getFrameworks() for posterity.

What do you think?

- Vinod Kone


On June 28, 2016, 7:23 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49297/
> ---
> 
> (Updated June 28, 2016, 7:23 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use defer() to garantee getTasks lambda execution context.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d583bbeb186974135999eaddbab96e69635dcc7c 
> 
> Diff: https://reviews.apache.org/r/49297/diff/
> 
> 
> Testing
> ---
> 
> WIP
> 
> make check
> 
> Still need to figure out how to handle invalid task*
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 49245: Implement READ_FILE for agent operator API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49242, 49301, 49243, 49244, 49245]

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

- Mesos ReviewBot


On June 28, 2016, 11:15 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49245/
> ---
> 
> (Updated June 28, 2016, 11:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5515
> https://issues.apache.org/jira/browse/mesos-5515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented readFile method for agent operator API.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f325bc6b7c92adfb47c1a636b7d385a00e461afb 
>   src/slave/slave.hpp 2afd7d152dcd2f5390014cd7bd4e926b62c292d1 
>   src/tests/api_tests.cpp 7b5c761751e4b28e3966ac8180fb19ce00fc8f30 
> 
> Diff: https://reviews.apache.org/r/49245/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49064: Implement v1 operator update weights API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 28, 2016, 9:15 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49064/
> ---
> 
> (Updated June 28, 2016, 9:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5496
> https://issues.apache.org/jira/browse/mesos-5496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented updateWeights method in WeightsHandler.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f63cc5a9dc379e4e264fe1f08fdc06d8c16d7384 
>   include/mesos/v1/master/master.proto 
> d98281d58daa461ce622af49adff9f748b920b7f 
>   src/master/http.cpp d583bbeb186974135999eaddbab96e69635dcc7c 
>   src/master/master.hpp ba271e7a175529c59abd8bb92536eb5bf0136a40 
>   src/master/validation.cpp 9120b71fc7725bdf7094aac6619d8aadcc352df5 
>   src/master/weights_handler.cpp 0332d86570724e841f97348087808081b2b890af 
>   src/tests/api_tests.cpp 7b5c761751e4b28e3966ac8180fb19ce00fc8f30 
> 
> Diff: https://reviews.apache.org/r/49064/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49136: Add Framework protobuf message.

[Vinod Kone]

> On June 28, 2016, 6:39 p.m., Vinod Kone wrote:
> > include/mesos/master/master.proto, line 316
> > 
> >
> > s/agent_id/slave_id/
> > 
> > also should this be required?

i've fixed these while committing.


- Vinod


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


On June 28, 2016, 8:09 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49136/
> ---
> 
> (Updated June 28, 2016, 8:09 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5492
> https://issues.apache.org/jira/browse/mesos-5492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'Framework' protobuf message to master/master.proto &
> v1/master/master.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f63cc5a9dc379e4e264fe1f08fdc06d8c16d7384 
>   include/mesos/v1/master/master.proto 
> d98281d58daa461ce622af49adff9f748b920b7f 
> 
> Diff: https://reviews.apache.org/r/49136/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49137: Implement v1 operator API GET_FRAMEWORK call.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49136, 49137]

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

- Mesos ReviewBot


On June 28, 2016, 8:11 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49137/
> ---
> 
> (Updated June 28, 2016, 8:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5492
> https://issues.apache.org/jira/browse/mesos-5492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented getFrameworks method in http.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d583bbeb186974135999eaddbab96e69635dcc7c 
>   src/master/master.hpp ba271e7a175529c59abd8bb92536eb5bf0136a40 
>   src/tests/api_tests.cpp 7b5c761751e4b28e3966ac8180fb19ce00fc8f30 
> 
> Diff: https://reviews.apache.org/r/49137/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49280: Fixed FD inheritance leak when SSL is enabled.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On June 28, 2016, 1:34 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49280/
> ---
> 
> (Updated June 28, 2016, 1:34 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Benjamin Mahler, Artem 
> Harutyunyan, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5723
> https://issues.apache.org/jira/browse/MESOS-5723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Incoming sockets are leaked when the agent forks because incoming 
> sockets are not modified with the CLOEXEC option.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 3f80e863fdbb62d4cf7ce1b46c468a09f0694f56 
> 
> Diff: https://reviews.apache.org/r/49280/diff/
> 
> 
> Testing
> ---
> 
> make check (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46687: Updated `dispatch` calls to use lambda in http_tests.cpp.

[haosdent huang]

> On June 26, 2016, 7:37 a.m., Michael Park wrote:
> > Could you specify that this only updates `http_tests.cpp`? There are other 
> > places in the codebase where we explicitly construct a `std::function` 
> > first, but we don't update all of those in this patch.
> 
> Michael Park wrote:
> Just in the summary and/or description, that is.

Hi, @mcypark Thanks a lot for your detail comments. I found the remain legacy 
codes are only exists in `deferred.hpp`. I post a sperate patch in 
https://reviews.apache.org/r/49315/


- haosdent


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


On June 28, 2016, 9:43 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46687/
> ---
> 
> (Updated June 28, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Michael Park.
> 
> 
> Bugs: MESOS-4611
> https://issues.apache.org/jira/browse/MESOS-4611
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `dispatch` calls to use lambda in http_tests.cpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 4337e6028a3a6e5279793c7c6f73bb9a4f60cb0a 
> 
> Diff: https://reviews.apache.org/r/46687/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 49315: Avoided unnecessary cast of `dispatch` in deferred.hpp.

[haosdent huang]

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

Review request for mesos, Kevin Klues and Michael Park.


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


Repository: mesos


Description
---

Avoided unnecessary cast of `dispatch` in deferred.hpp.


Diffs
-

  3rdparty/libprocess/include/process/deferred.hpp 
18ba979b4ee4bb417a80710c647f7d24e43deeeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49321: GetState response protobuf.

[haosdent huang]

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




include/mesos/master/master.proto (line 282)


This should be `optional`?



include/mesos/v1/master/master.proto (line 283)


This should be `optional`?


- haosdent huang


On June 28, 2016, 3:07 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49321/
> ---
> 
> (Updated June 28, 2016, 3:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> GetState response protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
>   include/mesos/v1/master/master.proto 
> 681ea1e638777059a6bf792435cbe526bc252f7a 
> 
> Diff: https://reviews.apache.org/r/49321/diff/
> 
> 
> Testing
> ---
> 
> `make` on Mac
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49136: Add Framework protobuf message.

[Vinod Kone]

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


Fix it, then Ship it!





include/mesos/master/master.proto (line 316)


s/agent_id/slave_id/

also should this be required?


- Vinod Kone


On June 28, 2016, 8:09 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49136/
> ---
> 
> (Updated June 28, 2016, 8:09 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5492
> https://issues.apache.org/jira/browse/mesos-5492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'Framework' protobuf message to master/master.proto &
> v1/master/master.proto.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f63cc5a9dc379e4e264fe1f08fdc06d8c16d7384 
>   include/mesos/v1/master/master.proto 
> d98281d58daa461ce622af49adff9f748b920b7f 
> 
> Diff: https://reviews.apache.org/r/49136/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49137: Implement v1 operator API GET_FRAMEWORK call.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 28, 2016, 8:11 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49137/
> ---
> 
> (Updated June 28, 2016, 8:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5492
> https://issues.apache.org/jira/browse/mesos-5492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented getFrameworks method in http.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d583bbeb186974135999eaddbab96e69635dcc7c 
>   src/master/master.hpp ba271e7a175529c59abd8bb92536eb5bf0136a40 
>   src/tests/api_tests.cpp 7b5c761751e4b28e3966ac8180fb19ce00fc8f30 
> 
> Diff: https://reviews.apache.org/r/49137/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49137: Implement v1 operator API GET_FRAMEWORK call.

[Vinod Kone]

> On June 28, 2016, 6:14 a.m., Vinod Kone wrote:
> > src/master/http.cpp, line 1358
> > 
> >
> > don't use `auto` here because the type is not clear from this 
> > expression.
> 
> zhou xing wrote:
> If I reaplace auto with hashmap, I got the 
> following compile error:
> 
> '''
> ../../src/master/http.cpp:1274:16: error: too many arguments provided to
>   function-like macro invocation
>framework.executors) {
>^
> ../../3rdparty/stout/include/stout/foreach.hpp:51:9: note: macro 
> 'foreachpair'
>   defined here
> #define foreachpair(KEY, VALUE, ELEMS)
> '''
> 
> I checked the code of foreachpair, seems it does not accept more than 1 
> template params?
> 
> zhou xing wrote:
> another solution I can see is to use the following code to go through all 
> the executors:
> 
> ```
>   const hashset& slaveIds = framework.executors.keys();
>   foreach (const SlaveID& slaveId, slaveIds) {
> const hashmap& executorsMap =
> framework.executors.get(slaveId).get();
> 
> foreachvalue (const ExecutorInfo& info, executorsMap) {
>   // Skip unauthorized executors.
>   if (!approveViewExecutorInfo(executorsApprover,
>info,
>framework.info)) {
> continue;
>   }
> 
>   mesos::master::Response::GetFrameworks::Executor executor;
>   executor.mutable_info()->CopyFrom(executor);
>   executor.mutable_agent_id()->set_value(slaveId.value());
> 
>   _framework.mutable_executors()->Add()->CopyFrom(executor);
> }
>   }
>   ```
>   I prefer to keep 'auto', as it is not that complex.   What do you think?
> 
> haosdent huang wrote:
> Looks like the comma in `const hashmap&` make 
> compiler confuse.
> 
> haosdent huang wrote:
> Refer to 
> http://stackoverflow.com/questions/5414619/boost-c-macro-argument-count-error 
> , I suggest to use `auto` here.

I see. Dropping the issue.


- Vinod


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


On June 28, 2016, 8:11 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49137/
> ---
> 
> (Updated June 28, 2016, 8:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5492
> https://issues.apache.org/jira/browse/mesos-5492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented getFrameworks method in http.cpp.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp d583bbeb186974135999eaddbab96e69635dcc7c 
>   src/master/master.hpp ba271e7a175529c59abd8bb92536eb5bf0136a40 
>   src/tests/api_tests.cpp 7b5c761751e4b28e3966ac8180fb19ce00fc8f30 
> 
> Diff: https://reviews.apache.org/r/49137/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49301: Added FilesError class.

[Anand Mazumdar]

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



LGTM minus a few minor issues around style/code comments.


src/files/files.hpp (lines 53 - 55)


hmm ..  `FilesError` can be used by more than master/agent actors. How 
about adding a more generic comment on the use case of this error class e.g., 
something like:

```cpp
// Represents the various errors that can be returned by methods on the 
`Files` class via a `Try` that has failed.
```



src/files/files.hpp (line 61)


Missing period at the end. Ditto for all the other 3 occurences.

s/Request arguments are invalid/Invalid argument



src/files/files.hpp (line 62)


s/could not be/not



src/files/files.hpp (line 71)


`const std::string&`

Also, no need for the `explicit` keyword on a multi arg constructor. 
Though, C++11 allows you to use the brace initialization syntax, AFAICT, we 
don't use this widely in our code yet and would be inconsistent.



src/files/files.hpp (lines 72 - 74)


Should fit in one line, no?


- Anand Mazumdar


On June 28, 2016, 11:13 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49301/
> ---
> 
> (Updated June 28, 2016, 11:13 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5515
> https://issues.apache.org/jira/browse/mesos-5515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added FilesError class to identify the errors happened during
> the handling of files relevant requests. FilesError is a type
> deriving from Error that the master/agent can use to construct
> the response.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
> 
> Diff: https://reviews.apache.org/r/49301/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 46687: Updated `dispatch` calls to use lambda in http_tests.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46686, 46687]

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

- Mesos ReviewBot


On June 28, 2016, 9:43 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46687/
> ---
> 
> (Updated June 28, 2016, 9:43 a.m.)
> 
> 
> Review request for mesos, Kevin Klues and Michael Park.
> 
> 
> Bugs: MESOS-4611
> https://issues.apache.org/jira/browse/MESOS-4611
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `dispatch` calls to use lambda in http_tests.cpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 4337e6028a3a6e5279793c7c6f73bb9a4f60cb0a 
> 
> Diff: https://reviews.apache.org/r/46687/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48373: Integrated the `NvidiaGpuAllocator` into the `NvidiaGpuIsolator`.

[Kevin Klues]

> On June 28, 2016, 2:55 a.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp, lines 442-446
> > 
> >
> > Unfortunately this won't accomplish your intent of providing context in 
> > the failure message because the .onFailed method does not provide 
> > composition, your returned Failure here will fall into the void.
> > 
> > To accomplish this, we need a .then which can accept a Future in 
> > addition to being able to take a T.

Yes, I see this now. The .then is just chained onto the .onFailed. In fact, the 
way I had it before, I think we never would have actually cleaned up anything 
(the .onFailed() was the only thing chained to the original Future).


> On June 28, 2016, 2:55 a.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp, line 375
> > 
> >
> > Hm.. seems we should do another lookup of the 'info' since we are 
> > returning to the isolator context and things may have occurred in the 
> > interim. Perhaps it's worth pulling out an `_update` continuation to force 
> > this (no captures possible).

I see you committed this with:
```
  .then(defer(PID(this),
  ::_update,
  containerId,
  lambda::_1));
```

Why `PID(this)` instead of `self()`?

Also, I see why it makes sense to re-lookup `info`, but I'm unclear on why the 
continuation had to be pulled out into a separate function.  The logic feels 
very disjoint now when adding GPUs vs. taking them away.  Maybe it's just the 
name `_update` since it only actually gets triggered in the "adding" GPUs case.


> On June 28, 2016, 2:55 a.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp, line 447
> > 
> >
> > This needs to defer to self since the infos map is managed by the 
> > isolator actor. In general, capturing and using 'this' within a 
> > continuation without defering to self is dangerous.

Yes. This was an oversight. When I started thsi patch I was unclear on the need 
for `defer()` inside the `.then()`, and looks like I missed one when doing a 
pass over the code to add it.


> On June 28, 2016, 2:55 a.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp, line 261
> > 
> >
> > Hm.. wonder why we didn't just use foreach here?

I believe the original code needed a normal `for` look for some reason and the 
new code just inherited this form.  I agree -- a `foreach` is more appropriate 
here now though.


> On June 28, 2016, 2:55 a.m., Benjamin Mahler wrote:
> > src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp, lines 448-451
> > 
> >
> > To guard against races, how about:
> > 
> > ```
> >   CHECK(infos.contains(containerId));
> >   delete infos.at(containerId);
> >   infos.erase(containerId);
> > 
> >   return Nothing();
> > ```

Makes sense.  It's the same reasoning as for `update()` with needing to 
re-lookup `info` (only this time we don't need to store a temporary to it since 
we are just deleting it).


- Kevin


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


On June 23, 2016, 4:34 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48373/
> ---
> 
> (Updated June 23, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5559
> https://issues.apache.org/jira/browse/MESOS-5559
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> All logic to do GPU allocation is now conducted asynchronously through
> the `NvidiaGpuAllocator` component.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
> 28ad0b3d1d8e7642a93e4ebb0fcc399e182be393 
>   src/slave/containerizer/mesos/isolators/gpu/nvidia.cpp 
> cfb23244d0e6f6e0d94685a35826b35f3297c6fe 
> 
> Diff: https://reviews.apache.org/r/48373/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check && sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49247: Implemented REMOVE_QUOTA Call in v1 master API.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On June 28, 2016, 4:56 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49247/
> ---
> 
> (Updated June 28, 2016, 4:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5510
> https://issues.apache.org/jira/browse/MESOS-5510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented REMOVE_QUOTA Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9e6d291aff77bb39462375dfed5dc75f3719806f 
>   src/master/master.hpp f16bccd0b72a8785d03290d11cd9b28d2990fd24 
>   src/master/quota_handler.cpp 4fa7533cb6a109fd060050c7814de440bd34c8ed 
>   src/tests/api_tests.cpp 7eb179f300f7a7dc6bdc3425268e7cd2fc8299fe 
> 
> Diff: https://reviews.apache.org/r/49247/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04 :
> sudo GTEST_FILTER="*MasterAPITest.RemoveQuota*" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 49325: Documented that reserved resources count against quota.

[Neil Conway]

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

Review request for mesos, Alexander Rukletsov and Joerg Schad.


Repository: mesos


Description
---

Made some other edits and cleanups to quota docs.


Diffs
-

  docs/quota.md 61c6be0d94466b40933d75d53d2b4464243e4796 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 49247: Implemented REMOVE_QUOTA Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 28, 2016, 4:56 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented REMOVE_QUOTA Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 9e6d291aff77bb39462375dfed5dc75f3719806f 
  src/master/master.hpp f16bccd0b72a8785d03290d11cd9b28d2990fd24 
  src/master/quota_handler.cpp 4fa7533cb6a109fd060050c7814de440bd34c8ed 
  src/tests/api_tests.cpp 7eb179f300f7a7dc6bdc3425268e7cd2fc8299fe 

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


Testing
---

On Ubuntu 16.04 :
sudo GTEST_FILTER="*MasterAPITest.RemoveQuota*" make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49206: Implemented GET_QUOTA Call in v1 master API.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On June 28, 2016, 4:48 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49206/
> ---
> 
> (Updated June 28, 2016, 4:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5508
> https://issues.apache.org/jira/browse/MESOS-5508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented GET_QUOTA Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
>   include/mesos/v1/master/master.proto 
> 681ea1e638777059a6bf792435cbe526bc252f7a 
>   src/master/http.cpp 9e6d291aff77bb39462375dfed5dc75f3719806f 
>   src/master/master.hpp f16bccd0b72a8785d03290d11cd9b28d2990fd24 
>   src/master/quota_handler.cpp 4fa7533cb6a109fd060050c7814de440bd34c8ed 
>   src/tests/api_tests.cpp 7eb179f300f7a7dc6bdc3425268e7cd2fc8299fe 
> 
> Diff: https://reviews.apache.org/r/49206/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04 :
> 
> sudo GTEST_FILTER="*MasterAPITest.GetQuota*" make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 49326: Fixed typo in authorization test description.

[Joerg Schad]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Fixed typo in authorization test description.


Diffs
-

  src/tests/master_authorization_tests.cpp 
81804e0522fd6b26155732af08e33c4d0bb0a8fe 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 49206: Implemented GET_QUOTA Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 28, 2016, 4:48 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented GET_QUOTA Call in v1 master API.


Diffs (updated)
-

  include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
  include/mesos/v1/master/master.proto 681ea1e638777059a6bf792435cbe526bc252f7a 
  src/master/http.cpp 9e6d291aff77bb39462375dfed5dc75f3719806f 
  src/master/master.hpp f16bccd0b72a8785d03290d11cd9b28d2990fd24 
  src/master/quota_handler.cpp 4fa7533cb6a109fd060050c7814de440bd34c8ed 
  src/tests/api_tests.cpp 7eb179f300f7a7dc6bdc3425268e7cd2fc8299fe 

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


Testing
---

On Ubuntu 16.04 :

sudo GTEST_FILTER="*MasterAPITest.GetQuota*" make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49247: Implemented REMOVE_QUOTA Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 28, 2016, 4:13 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented REMOVE_QUOTA Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp 9e6d291aff77bb39462375dfed5dc75f3719806f 
  src/master/master.hpp f16bccd0b72a8785d03290d11cd9b28d2990fd24 
  src/master/quota_handler.cpp 4fa7533cb6a109fd060050c7814de440bd34c8ed 
  src/tests/api_tests.cpp 7eb179f300f7a7dc6bdc3425268e7cd2fc8299fe 

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


Testing
---

On Ubuntu 16.04 :
sudo GTEST_FILTER="*MasterAPITest.RemoveQuota*" make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49064: Implement v1 operator update weights API.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48924, 48925, 48940, 49064]

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

- Mesos ReviewBot


On June 28, 2016, 9:15 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49064/
> ---
> 
> (Updated June 28, 2016, 9:15 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5496
> https://issues.apache.org/jira/browse/mesos-5496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented updateWeights method in WeightsHandler.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto f63cc5a9dc379e4e264fe1f08fdc06d8c16d7384 
>   include/mesos/v1/master/master.proto 
> d98281d58daa461ce622af49adff9f748b920b7f 
>   src/master/http.cpp d583bbeb186974135999eaddbab96e69635dcc7c 
>   src/master/master.hpp ba271e7a175529c59abd8bb92536eb5bf0136a40 
>   src/master/validation.cpp 9120b71fc7725bdf7094aac6619d8aadcc352df5 
>   src/master/weights_handler.cpp 0332d86570724e841f97348087808081b2b890af 
>   src/tests/api_tests.cpp 7b5c761751e4b28e3966ac8180fb19ce00fc8f30 
> 
> Diff: https://reviews.apache.org/r/49064/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49206: Implemented GET_QUOTA Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated June 28, 2016, 3:58 p.m.)


Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented GET_QUOTA Call in v1 master API.


Diffs (updated)
-

  include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
  include/mesos/v1/master/master.proto 681ea1e638777059a6bf792435cbe526bc252f7a 
  src/master/http.cpp 9e6d291aff77bb39462375dfed5dc75f3719806f 
  src/master/master.hpp f16bccd0b72a8785d03290d11cd9b28d2990fd24 
  src/master/quota_handler.cpp 4fa7533cb6a109fd060050c7814de440bd34c8ed 
  src/tests/api_tests.cpp 7eb179f300f7a7dc6bdc3425268e7cd2fc8299fe 

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


Testing
---

On Ubuntu 16.04 :

sudo GTEST_FILTER="*MasterAPITest.GetQuota*" make -j4 check


Thanks,

Abhishek Dasgupta

Review Request 49323: Added test that combines the two ways of creating volumes.

[Neil Conway]

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

Review request for mesos.


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


Repository: mesos


Description
---

This test checks that dynamic reservations and persistent volumes can be
created via the offer API and then removed via the HTTP endpoints.


Diffs
-

  src/tests/persistent_volume_endpoints_tests.cpp 
6c85e19eeaa69bf3a4e3077261331191db6eec06 

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


Testing
---

make check

This test passes without any Mesos changes. i.e., it doesn't reproduce the bug 
by itself, but having more test coverage for this scenario seems wise.


Thanks,

Neil Conway

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

[Guangya Liu]

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




include/mesos/appc/spec.proto 


This comment should be addressed in https://reviews.apache.org/r/49207/

I saw that you fixed some commentes for other patches, it is better address 
the comments in the original patch.

Ditto for the update of include/mesos/slave/isolator.proto.



src/slave/containerizer/mesos/containerizer.cpp (line 317)


Put `appc/runtime` above `docker/runtime`?



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 212 - 213)


VLOG(1) << "Failed to get manifest for AppC image  '"
<< appc.SerializeAsString() << "'";

Please make sure the `<<` keep aligned.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 334 - 335)


Can we simplify the logic as:

if (imageInfo.layers.size() == 1) {
  return ProvisionInfo{
rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};
}



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 403 - 404)


What about 

return ProvisionInfo{
  rootfs, imageInfo.dockerManifest, imageInfo.appcManifest};


- Guangya Liu


On 六月 28, 2016, 6:39 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49232/
> ---
> 
> (Updated 六月 28, 2016, 6:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added appcManifest to ImageInfo and ProvisionInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d984efd4742ec084d66538c48a36ea768832324d 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 48a05059969e068a0ee0d38b61be9e7104e3188d 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
> 
> Diff: https://reviews.apache.org/r/49232/diff/
> 
> 
> Testing
> ---
> 
> Make Check.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49257: Added documentation on coarse grain authorization for endpoints.

[Alexander Rojas]

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

(Updated June 28, 2016, 5:12 p.m.)


Review request for mesos, Adam B, Joerg Schad, Jan Schlicht, and Till Toenshoff.


Bugs: MESOS-5707 and MESOS-5712
https://issues.apache.org/jira/browse/MESOS-5707
https://issues.apache.org/jira/browse/MESOS-5712


Repository: mesos


Description
---

Coarse grained authorization for endpoints landed a while ago, however
no documentation is available.


Diffs (updated)
-

  docs/authorization.md 9bd6031d2a3aef921fa4e3f6683cc5c234832d47 
  docs/upgrades.md 079e04f9e886da4e28ef79b61f0fef82e927e771 

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


Testing
---

Not a functional change.


Thanks,

Alexander Rojas

Re: Review Request 49257: Added documentation on coarse grain authorization for endpoints.

[Alexander Rojas]

> On June 28, 2016, 1:14 a.m., Vinod Kone wrote:
> > CHANGELOG, line 44
> > 
> >
> > Can you expand on this?

I removed this line since below there is the following text:

```
  * [MESOS-4843, MESOS-5150, MESOS-5286, MESOS-5335, MESOS-5336] - Authorization
has been added to the '/metrics/snapshot', '/logging/toggle', '/quota',
'/files/browse', '/files/download', '/files/read', '/flags', and
'/containers' endpoints. If a Mesos cluster has authorization enabled, these
endpoints now require that ACLs be set to authorize principals to access
them. Note that the '/metrics/snapshot' and '/files/*' endpoints are used by
the web UI, and thus using the web UI in a cluster with authorization
enabled will require that ACLs be set appropriately.
```

Is kind of important to not that if 
[r/49313/](https://reviews.apache.org/r/49313/) doesn't land, _'/flags'_ should 
be removed from that list.


- Alexander


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


On June 28, 2016, 2:57 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49257/
> ---
> 
> (Updated June 28, 2016, 2:57 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Jan Schlicht, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5707 and MESOS-5712
> https://issues.apache.org/jira/browse/MESOS-5707
> https://issues.apache.org/jira/browse/MESOS-5712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Coarse grained authorization for endpoints landed a while ago, however
> no documentation is available.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 7a44422e47a0453d2079070857c5537b63f4ae0a 
>   docs/authorization.md abadee4ae51ee2d2e160c7883b4f518593f2f5b3 
>   docs/upgrades.md 079e04f9e886da4e28ef79b61f0fef82e927e771 
> 
> Diff: https://reviews.apache.org/r/49257/diff/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 49257: Added documentation on coarse grain authorization for endpoints.

[Alexander Rojas]

> On June 28, 2016, 3:05 p.m., Joerg Schad wrote:
> > CHANGELOG, line 151
> > 
> >
> > Doesn't this duplicate your addition above?

Yup, fixed.


- Alexander


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


On June 28, 2016, 2:57 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49257/
> ---
> 
> (Updated June 28, 2016, 2:57 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, Jan Schlicht, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5707 and MESOS-5712
> https://issues.apache.org/jira/browse/MESOS-5707
> https://issues.apache.org/jira/browse/MESOS-5712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Coarse grained authorization for endpoints landed a while ago, however
> no documentation is available.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 7a44422e47a0453d2079070857c5537b63f4ae0a 
>   docs/authorization.md abadee4ae51ee2d2e160c7883b4f518593f2f5b3 
>   docs/upgrades.md 079e04f9e886da4e28ef79b61f0fef82e927e771 
> 
> Diff: https://reviews.apache.org/r/49257/diff/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 49321: GetState response protobuf.

[Zhitao Li]

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

(Updated June 28, 2016, 3:07 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

GetState response protobuf.


Diffs
-

  include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
  include/mesos/v1/master/master.proto 681ea1e638777059a6bf792435cbe526bc252f7a 

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


Testing
---

`make` on Mac


Thanks,

Zhitao Li

Review Request 49321: GetState response protobuf.

[Zhitao Li]

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

Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

GetState response protobuf.


Diffs
-

  include/mesos/master/master.proto 332261918a069f4e581bfc4d0fc14ce8c86c2617 
  include/mesos/v1/master/master.proto 681ea1e638777059a6bf792435cbe526bc252f7a 

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


Testing
---

`make` on Mac


Thanks,

Zhitao Li