Re: Review Request 60013: Resources: Updated the validation logic.

[Michael Park]

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

(Updated June 14, 2017, 8:20 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Addressed rest of bmahler's comments.


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


Repository: mesos


Description
---

Resources: Updated the validation logic.


Diffs (updated)
-

  src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
  src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
  src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 
  src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 


Diff: https://reviews.apache.org/r/60013/diff/8/

Changes: https://reviews.apache.org/r/60013/diff/7-8/


Testing
---


Thanks,

Michael Park

Re: Review Request 60013: Resources: Updated the validation logic.

[Michael Park]

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

(Updated June 14, 2017, 7:37 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Addressed the comment around the complex loop.


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


Repository: mesos


Description
---

Resources: Updated the validation logic.


Diffs (updated)
-

  src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
  src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
  src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 
  src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 


Diff: https://reviews.apache.org/r/60013/diff/7/

Changes: https://reviews.apache.org/r/60013/diff/6-7/


Testing
---


Thanks,

Michael Park

Re: Review Request 60013: Resources: Updated the validation logic.

[Michael Park]

> On June 13, 2017, 10:58 a.m., Benjamin Mahler wrote:
> > src/common/resources.cpp
> > Lines 844-845 (original), 864-868 (patched)
> > 
> >
> > Do you want to add a function to roles.hpp for checking this?
> > 
> > ```
> > bool isSubRole(const string& parentRole, const string& subRole);
> > ```

https://reviews.apache.org/r/60063


- Michael


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


On June 13, 2017, 1:34 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60013/
> ---
> 
> (Updated June 13, 2017, 1:34 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7655
> https://issues.apache.org/jira/browse/MESOS-7655
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resources: Updated the validation logic.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
>   src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
>   src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
>   src/tests/master_validation_tests.cpp 
> 4e7ce74edf0069b9317f869b299694a45e2a62f2 
>   src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 
> 
> 
> Diff: https://reviews.apache.org/r/60013/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59950: Added tests for `DefaultExecutor` setting `MESOS_CONTAINER_IP`.

[Avinash sridharan]

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

(Updated June 15, 2017, 12:41 a.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Addressed Jie's comments and rebased.


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


Repository: mesos


Description
---

Added tests for `DefaultExecutor` setting `MESOS_CONTAINER_IP`.


Diffs (updated)
-

  src/tests/containerizer/cni_isolator_tests.cpp 
505439faefe991b22e6c3435274bd18d01b73251 
  src/tests/mesos.hpp 48072a976cdbe6e655dc6f5f258abc6d737ed068 


Diff: https://reviews.apache.org/r/59950/diff/3/

Changes: https://reviews.apache.org/r/59950/diff/2-3/


Testing
---

make check

[==] Running 2 tests from 2 test cases.
[--] Global test environment set-up.
[--] 1 test from HostNetwork/DefaultExecutorCniTest
[ RUN  ] HostNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0
I0609 18:16:12.417310 22692 executor.cpp:192] Version: 1.4.0
I0609 18:16:12.435683 22694 default_executor.cpp:182] Received SUBSCRIBED event
I0609 18:16:12.441695 22694 default_executor.cpp:186] Subscribed executor on 
centos7
I0609 18:16:12.442006 22694 default_executor.cpp:182] Received LAUNCH_GROUP 
event
W0609 18:16:12.47 22691 default_executor.cpp:443] Setting 
`MESOS_CONTAINER_IP` to: 127.0.0.1
I0609 18:16:12.469161 22691 default_executor.cpp:605] Successfully launched 
tasks [ 34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6 ] in child containers [ 
1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e ]
I0609 18:16:12.472266 22693 default_executor.cpp:678] Waiting for child 
container 
1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e of 
task '34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6'
I0609 18:16:12.481412 22689 default_executor.cpp:182] Received ACKNOWLEDGED 
event
I0609 18:16:12.575834 22694 default_executor.cpp:823] Child container 
1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e of 
task '34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6' in state TASK_FINISHED exited with 
status 0
I0609 18:16:12.575960 22694 default_executor.cpp:945] Terminating after 1secs
[   OK ] HostNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 (2366 
ms)
[--] 1 test from HostNetwork/DefaultExecutorCniTest (2368 ms total)

[--] 1 test from CniNetwork/DefaultExecutorCniTest
[ RUN  ] CniNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0
I0609 18:16:13.042444 22763 executor.cpp:192] Version: 1.4.0
I0609 18:16:13.059947 22759 default_executor.cpp:182] Received SUBSCRIBED event
I0609 18:16:13.065265 22759 default_executor.cpp:186] Subscribed executor on 
centos7
I0609 18:16:13.065783 22759 default_executor.cpp:182] Received LAUNCH_GROUP 
event
W0609 18:16:13.067163 22763 default_executor.cpp:443] Setting 
`MESOS_CONTAINER_IP` to: 10.0.2.15
I0609 18:16:13.181131 22765 default_executor.cpp:605] Successfully launched 
tasks [ 1d265a47-6422-458e-b350-9a1091130c40 ] in child containers [ 
48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 ]
I0609 18:16:13.183614 22764 default_executor.cpp:678] Waiting for child 
container 
48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 of 
task '1d265a47-6422-458e-b350-9a1091130c40'
I0609 18:16:13.191012 22760 default_executor.cpp:182] Received ACKNOWLEDGED 
event
I0609 18:16:13.290139 22763 default_executor.cpp:823] Child container 
48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 of 
task '1d265a47-6422-458e-b350-9a1091130c40' in state TASK_FINISHED exited with 
status 0
I0609 18:16:13.290249 22763 default_executor.cpp:945] Terminating after 1secs
[   OK ] CniNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 (728 ms)
[--] 1 test from CniNetwork/DefaultExecutorCniTest (729 ms total)

[--] Global test environment tear-down
[==] 2 tests from 2 test cases ran. (3105 ms total)
[  PASSED  ] 2 tests.


Thanks,

Avinash sridharan

Re: Review Request 59949: Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.

[Avinash sridharan]

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

(Updated June 15, 2017, 12:38 a.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.


Diffs (updated)
-

  src/launcher/default_executor.cpp 91a4e1ebed3ff979bd3c0f7f46559cec24f61c2c 


Diff: https://reviews.apache.org/r/59949/diff/5/

Changes: https://reviews.apache.org/r/59949/diff/4-5/


Testing
---

make check


Thanks,

Avinash sridharan

Review Request 60104: Added rebooted flag to RecoveryInfo and State.

[Megha Sharma]

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

Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added rebooted flag to State and Slave.RecoveryInfo to remember
if the agent has rebooted.


Diffs
-

  src/slave/slave.hpp 7ffaed14035a05259ec72c70532ee4f0affa1f5d 
  src/slave/slave.cpp 7d147ac6609933ac884bfc29032dba572a0952c6 
  src/slave/state.hpp a497ce1f58fb8dc7718ee5bb10bc62dd7479efa5 
  src/slave/state.cpp 18b790d2cc4f537cc9b0c3cca59b9cbaac0eda10 


Diff: https://reviews.apache.org/r/60104/diff/1/


Testing
---


Thanks,

Megha Sharma

Review Request 60103: Changed variable name _ack to statusUpdateAck.

[Megha Sharma]

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

Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Changed variable name _ack to statusUpdateAck which
correctly identifies with the purpose of this variable.


Diffs
-

  src/tests/slave_recovery_tests.cpp 38502584186793686f78ff5f4e03f36a3bf7ad1c 


Diff: https://reviews.apache.org/r/60103/diff/1/


Testing
---


Thanks,

Megha Sharma

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 3:26 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Added TODOs for deprecation.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 


Diff: https://reviews.apache.org/r/59861/diff/7/

Changes: https://reviews.apache.org/r/59861/diff/6-7/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 3:21 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Addressed bmahler's comments.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 


Diff: https://reviews.apache.org/r/59861/diff/6/

Changes: https://reviews.apache.org/r/59861/diff/5-6/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59949: Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.

[Avinash sridharan]

> On June 13, 2017, 6:42 p.m., Jie Yu wrote:
> > src/launcher/default_executor.cpp
> > Lines 371 (patched)
> > 
> >
> > I'd add a TODO here. Because for bridge mode, `self().address.ip` might 
> > be the advertise IP (i.e., agent IP), which is not what we've wanted here.
> 
> Avinash sridharan wrote:
> From a CNI perspective the BRIDGE mode is just another CNI network, so 
> the self().address.ip will be the IP on the BRIDGE network which is what is 
> expected. So for the time being dropping this comment. Please open if you 
> still feel this is an issue.
> 
> Jie Yu wrote:
> i am just giving an example here. Any container that set 
> LIBPROCESS_ADVERTISE_IP env var will change `self().address.ip`. And this is 
> not the IP we want to set for `MESOS_CONTAINER_IP` env var.

Re-opening this issue based on Jie's comments above.


- Avinash


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


On June 14, 2017, 1:57 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59949/
> ---
> 
> (Updated June 14, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Bugs: MESOS-7631
> https://issues.apache.org/jira/browse/MESOS-7631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 91a4e1ebed3ff979bd3c0f7f46559cec24f61c2c 
> 
> 
> Diff: https://reviews.apache.org/r/59949/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59950: Added tests for `DefaultExecutor` setting `MESOS_CONTAINER_IP`.

[Avinash sridharan]

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

(Updated June 14, 2017, 9:58 p.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Fixed Jie's comments.


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


Repository: mesos


Description
---

Added tests for `DefaultExecutor` setting `MESOS_CONTAINER_IP`.


Diffs (updated)
-

  src/tests/containerizer/cni_isolator_tests.cpp 
3a5f4ebd4b05947dbc1e3b38c3fbbf70e6eec24b 
  src/tests/mesos.hpp 48072a976cdbe6e655dc6f5f258abc6d737ed068 


Diff: https://reviews.apache.org/r/59950/diff/2/

Changes: https://reviews.apache.org/r/59950/diff/1-2/


Testing
---

make check

[==] Running 2 tests from 2 test cases.
[--] Global test environment set-up.
[--] 1 test from HostNetwork/DefaultExecutorCniTest
[ RUN  ] HostNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0
I0609 18:16:12.417310 22692 executor.cpp:192] Version: 1.4.0
I0609 18:16:12.435683 22694 default_executor.cpp:182] Received SUBSCRIBED event
I0609 18:16:12.441695 22694 default_executor.cpp:186] Subscribed executor on 
centos7
I0609 18:16:12.442006 22694 default_executor.cpp:182] Received LAUNCH_GROUP 
event
W0609 18:16:12.47 22691 default_executor.cpp:443] Setting 
`MESOS_CONTAINER_IP` to: 127.0.0.1
I0609 18:16:12.469161 22691 default_executor.cpp:605] Successfully launched 
tasks [ 34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6 ] in child containers [ 
1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e ]
I0609 18:16:12.472266 22693 default_executor.cpp:678] Waiting for child 
container 
1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e of 
task '34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6'
I0609 18:16:12.481412 22689 default_executor.cpp:182] Received ACKNOWLEDGED 
event
I0609 18:16:12.575834 22694 default_executor.cpp:823] Child container 
1a6e3ada-7e68-43c7-95e2-88978c055c7d.1f0a6c53-8189-4989-8dd1-4e38f28f7f5e of 
task '34ddb6bc-bf93-42ff-aab1-cb06d25c5fc6' in state TASK_FINISHED exited with 
status 0
I0609 18:16:12.575960 22694 default_executor.cpp:945] Terminating after 1secs
[   OK ] HostNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 (2366 
ms)
[--] 1 test from HostNetwork/DefaultExecutorCniTest (2368 ms total)

[--] 1 test from CniNetwork/DefaultExecutorCniTest
[ RUN  ] CniNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0
I0609 18:16:13.042444 22763 executor.cpp:192] Version: 1.4.0
I0609 18:16:13.059947 22759 default_executor.cpp:182] Received SUBSCRIBED event
I0609 18:16:13.065265 22759 default_executor.cpp:186] Subscribed executor on 
centos7
I0609 18:16:13.065783 22759 default_executor.cpp:182] Received LAUNCH_GROUP 
event
W0609 18:16:13.067163 22763 default_executor.cpp:443] Setting 
`MESOS_CONTAINER_IP` to: 10.0.2.15
I0609 18:16:13.181131 22765 default_executor.cpp:605] Successfully launched 
tasks [ 1d265a47-6422-458e-b350-9a1091130c40 ] in child containers [ 
48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 ]
I0609 18:16:13.183614 22764 default_executor.cpp:678] Waiting for child 
container 
48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 of 
task '1d265a47-6422-458e-b350-9a1091130c40'
I0609 18:16:13.191012 22760 default_executor.cpp:182] Received ACKNOWLEDGED 
event
I0609 18:16:13.290139 22763 default_executor.cpp:823] Child container 
48fc4118-8637-4564-9a8f-73de5712e277.69464142-e313-4c8e-859c-cd1d2cce4e26 of 
task '1d265a47-6422-458e-b350-9a1091130c40' in state TASK_FINISHED exited with 
status 0
I0609 18:16:13.290249 22763 default_executor.cpp:945] Terminating after 1secs
[   OK ] CniNetwork/DefaultExecutorCniTest.ROOT_VerifyContainerIP/0 (728 ms)
[--] 1 test from CniNetwork/DefaultExecutorCniTest (729 ms total)

[--] Global test environment tear-down
[==] 2 tests from 2 test cases ran. (3105 ms total)
[  PASSED  ] 2 tests.


Thanks,

Avinash sridharan

Re: Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

[Michael Park]

> On June 14, 2017, 9:30 a.m., Neil Conway wrote:
> > A bit more detail in the commit message would be good: for example, how 
> > does this change relate to ca8d33ab12d702612492928c0a5080bd459830bd ?

Added more detail! Thanks.


> On June 14, 2017, 9:30 a.m., Neil Conway wrote:
> > src/master/validation.cpp
> > Lines 1830 (patched)
> > 
> >
> > Seems unfortunate to have to pass in the `SlaveID` here just for error 
> > messages. I wonder if the callers of `validate` can include the `SlaveID` 
> > in the error message they return when validation fails?

Removed from here and moved up to the caller.


> On June 14, 2017, 9:30 a.m., Neil Conway wrote:
> > src/master/validation.cpp
> > Lines 1856 (patched)
> > 
> >
> > We don't use single-quotes for capability names elsewhere.

Removed.


> On June 14, 2017, 9:30 a.m., Neil Conway wrote:
> > src/tests/master_validation_tests.cpp
> > Lines 279 (patched)
> > 
> >
> > So we're passing an invalid (unset) `SlaveID` to the `validate` call 
> > below? Seems confusing.

There's no longer an agent id parameter, so this is gone now :)


> On June 14, 2017, 9:30 a.m., Neil Conway wrote:
> > src/tests/master_validation_tests.cpp
> > Lines 331 (patched)
> > 
> >
> > Setting this to `true` suggests to me that the test requires the 
> > hierarchical agent capability, which isn't the case. I wonder if we want 
> > something like a `DEFAULT_AGENT_CAPABILITIES` constant?
> > 
> > For now I'd just pass in a default-initialized `slave::Capabilities`, 
> > without `hierarchicalRole` set.

Left them all default constructed within the test cases except for the 
`AgentHierarchicalRoleCapability` tests.


- Michael


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


On June 14, 2017, 2:39 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60062/
> ---
> 
> (Updated June 14, 2017, 2:39 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes it such that reserve / create operations involving
> resources with a hierarchical role are invalid if they are attempted on
> an agent without a HIERARCHICAL_ROLE capability. Such attempts from an
> operator endpoint will result in a `BadRequest` response. On the
> frameworks side, we've opted to not offer resources from
> a non-HIERARCHICAL_ROLE agent to hierarchical roles (ca8d33ab).
> 
> If we were to undo ca8d33ab, an attempt from a framework to perform such
> an operation would be considered invalid due to this patch and the
> operation would be dropped by the master.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
>   src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58 
>   src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
>   src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
>   src/tests/master_validation_tests.cpp 
> 4e7ce74edf0069b9317f869b299694a45e2a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/60062/diff/4/
> 
> 
> Testing
> ---
> 
> Added new tests to `master_validation_tests.cpp` + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

[Michael Park]

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

(Updated June 14, 2017, 2:39 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Addressed Neil's comments.


Repository: mesos


Description (updated)
---

This patch makes it such that reserve / create operations involving
resources with a hierarchical role are invalid if they are attempted on
an agent without a HIERARCHICAL_ROLE capability. Such attempts from an
operator endpoint will result in a `BadRequest` response. On the
frameworks side, we've opted to not offer resources from
a non-HIERARCHICAL_ROLE agent to hierarchical roles (ca8d33ab).

If we were to undo ca8d33ab, an attempt from a framework to perform such
an operation would be considered invalid due to this patch and the
operation would be dropped by the master.


Diffs (updated)
-

  src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
  src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58 
  src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
  src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 


Diff: https://reviews.apache.org/r/60062/diff/4/

Changes: https://reviews.apache.org/r/60062/diff/3-4/


Testing
---

Added new tests to `master_validation_tests.cpp` + `make check`


Thanks,

Michael Park

Review Request 60101: Prevent the fetcher from setting overly-permissive fs permissions.

[Silas Snider]

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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Prevent the fetcher from setting overly-permissive fs permissions.


Diffs
-

  src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 


Diff: https://reviews.apache.org/r/60101/diff/1/


Testing
---


Thanks,

Silas Snider

Review Request 60095: Add filtering feature for unversioned /master/slaves endpoint.

[Quinn Leng]

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.


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


Repository: mesos


Description
---

Add filtering feature for unversioned /master/slaves endpoint.

Specifically, we can use "/master/slaves?slave_id=[slave id]" to 
query the info for that slave. One thing to mention is that the 
format for the response is the same as before, thus we still have
two categories "slaves" and "recovered_slaves" in the response JSON


Diffs
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 
  src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
  src/tests/master_tests.cpp 543c4a3afab818578c3124331a092cb1057776af 


Diff: https://reviews.apache.org/r/60095/diff/1/


Testing
---

Passed "make check"
Passed "GLOG_v=1 ./bin/mesos-tests.sh 
--gtest_filter=MasterTest.SlavesEndpointQuerySlave --gtest_repeat=1000 
--gtest_break_on_failure"


Thanks,

Quinn Leng

Re: Review Request 60038: Prevented allocating reservation refinements to non-capable frameworks.

[Benjamin Mahler]

> On June 14, 2017, 7:04 p.m., Benjamin Mahler wrote:
> >

Also, could you add a description / commit message here with some of the 
subtleties from our discussion? E.g.

```
Prevented allocating reservation refinements to non-capable frameworks.

When reservation refinements are present, old frameworks without the
RESERVATION_REFINEMENT capability won't be able to understand the new
format. While it's possible to translate the refined reservations into
the old format by "hiding" the intermediate reservations in the "stack",
this leads to ambiguity when processing RESERVE and UNRESERVE operations.
This is due to the loss of information when we drop the intermediate
reservations. Therefore, for now we simply filter out resources with
refined reservations if the framework does not have the capability.
```

Should probably document this in the code as well?


- Benjamin


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


On June 13, 2017, 8:44 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60038/
> ---
> 
> (Updated June 13, 2017, 8:44 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevented allocating reservation refinements to non-capable frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 3428edfca116ffc625b683b6bead48300c2750c4 
> 
> 
> Diff: https://reviews.apache.org/r/60038/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60041: Devolved reserved resources for the HTTP endpoints.

[Benjamin Mahler]

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


Fix it, then Ship it!




This warrants a review description / commit message. How about:

```
Updated the v0 HTTP endpoints for reservation refinements.

The introduction of the RESERVATION_REFINEMENT capability changed
the format of `Resource` by making `Resource.role` and
`Resource.reservation` unused in favor of `Resource.reservations`.
To preserve backwards compatibility in the v0 HTTP endpoints that
expose JSON, we must "downgrade" to the old format if reservation
refinements are not being used within a resource.
```


src/common/http.cpp
Lines 344-347 (patched)


Can you add a comment across all of these touch points? E.g.

```
The introduction of the RESERVATION_REFINEMENT capability changed
the format of `Resource` by making `Resource.role` and
`Resource.reservation` unused in favor of `Resource.reservations`.
To preserve backwards compatibility, we must "downgrade" to the
old format if reservation refinement is not being used.

TODO(mpark): Remove this in 2.x? See MESOS-.
```

Would be great to reference a ticket that is about removing this.



src/slave/http.cpp
Lines 1295-1297 (original), 1298-1301 (patched)


Seems odd to introduce the formatting change here, can you just commit that 
separately to keep the diff minimal?


- Benjamin Mahler


On June 13, 2017, 8:43 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60041/
> ---
> 
> (Updated June 13, 2017, 8:43 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Devolved reserved resources for the HTTP endpoints.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 
>   src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
>   src/slave/http.cpp 78b35865e465ff1e8e7e4950fdb60e3a48b916b6 
> 
> 
> Diff: https://reviews.apache.org/r/60041/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59949: Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.

[Jie Yu]

> On June 13, 2017, 6:42 p.m., Jie Yu wrote:
> > src/launcher/default_executor.cpp
> > Lines 371 (patched)
> > 
> >
> > I'd add a TODO here. Because for bridge mode, `self().address.ip` might 
> > be the advertise IP (i.e., agent IP), which is not what we've wanted here.
> 
> Avinash sridharan wrote:
> From a CNI perspective the BRIDGE mode is just another CNI network, so 
> the self().address.ip will be the IP on the BRIDGE network which is what is 
> expected. So for the time being dropping this comment. Please open if you 
> still feel this is an issue.

i am just giving an example here. Any container that set 
LIBPROCESS_ADVERTISE_IP env var will change `self().address.ip`. And this is 
not the IP we want to set for `MESOS_CONTAINER_IP` env var.


- Jie


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


On June 14, 2017, 1:57 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59949/
> ---
> 
> (Updated June 14, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Bugs: MESOS-7631
> https://issues.apache.org/jira/browse/MESOS-7631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 91a4e1ebed3ff979bd3c0f7f46559cec24f61c2c 
> 
> 
> Diff: https://reviews.apache.org/r/59949/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 60038: Prevented allocating reservation refinements to non-capable frameworks.

[Benjamin Mahler]

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


Ship it!




Looks good. As a follow up I was thinking about logging since this is pretty 
silent and is going to wind up surprising people. I wonder if we could print 
out at the start of the allocation run a set of warnings. E.g.

```
I Performing allocation on N agents
W Framework F is MULTI_ROLE capable but some agents are not
W Framweork F is not RESERVATION_REFINEMENT capable; will not see refined 
reservations
etc
```

- Benjamin Mahler


On June 13, 2017, 8:44 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60038/
> ---
> 
> (Updated June 13, 2017, 8:44 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevented allocating reservation refinements to non-capable frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 3428edfca116ffc625b683b6bead48300c2750c4 
> 
> 
> Diff: https://reviews.apache.org/r/60038/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60074: Updated authorization tests.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 14, 2017, 10 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60074/
> ---
> 
> (Updated June 14, 2017, 10 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7670
> https://issues.apache.org/jira/browse/MESOS-7670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated authorization tests.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 
> 
> 
> Diff: https://reviews.apache.org/r/60074/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60038: Prevented allocating reservation refinements to non-capable frameworks.

[Benjamin Mahler]

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




src/master/allocator/mesos/hierarchical.cpp
Lines 1851-1857 (patched)


Actually, I think you need to touch both the quota and fair share "loops", 
this only touches the fair share loop?


- Benjamin Mahler


On June 13, 2017, 8:44 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60038/
> ---
> 
> (Updated June 13, 2017, 8:44 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevented allocating reservation refinements to non-capable frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 3428edfca116ffc625b683b6bead48300c2750c4 
> 
> 
> Diff: https://reviews.apache.org/r/60038/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60073: Added `RESERVATION_REFINEMENT` capability to tests.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 14, 2017, 10 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60073/
> ---
> 
> (Updated June 14, 2017, 10 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7670
> https://issues.apache.org/jira/browse/MESOS-7670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `RESERVATION_REFINEMENT` capability to tests.
> 
> 
> Diffs
> -
> 
>   src/examples/dynamic_reservation_framework.cpp 
> 41228145cf2249e383166a47a3ac33fb2bee27c9 
>   src/tests/api_tests.cpp 91b3473452b8e65cab9f2e873837d64a0edf4b54 
>   src/tests/default_executor_tests.cpp 
> a76ece5a2832d3a006476672e443573d60d991e9 
>   src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
>   src/tests/hierarchical_allocator_tests.cpp 
> d6c5904b6e8af6ed8e8387de5e635fb4c920dd08 
>   src/tests/master_validation_tests.cpp 
> 4e7ce74edf0069b9317f869b299694a45e2a62f2 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 0cb969cf06a123504720440f436ecb027d1e138a 
>   src/tests/persistent_volume_tests.cpp 
> ce716bc78a7f7f180fca896e780c74ef206a612b 
>   src/tests/reservation_endpoints_tests.cpp 
> ba283ee74fe27998a01c43e1ab6a8ce0089181fa 
>   src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5 
> 
> 
> Diff: https://reviews.apache.org/r/60073/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60072: Updated tests to use the test utilities with reservation refinement.

[Vinod Kone]

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




src/cli/execute.cpp
Lines 502 (patched)


does this work even if role is `*` ?



src/examples/persistent_volume_framework.cpp
Lines 583 (patched)


why is this the only test framework that needs the capability? curious.



src/examples/test_framework.cpp
Line 111 (original), 111 (patched)


curious why "*" needs to be tackled differently here but not in 
test_http_framework.cpp and cli/execute.cpp?



src/tests/master_quota_tests.cpp
Lines 315 (patched)


can you add a comment here on why you are if def'ing this out?



src/tests/mesos.hpp
Lines 1703-1708 (original), 1703-1706 (patched)


hmm. why this change?



src/tests/reservation_tests.cpp
Lines 2236-2240 (original), 2227-2231 (patched)


a bit weird that `createFrameworkInfo` sets role and this over-rides it 
here. maybe the helper can take a role as argument?



src/tests/resources_tests.cpp
Line 331 (original), 329 (patched)


so the top level `role` in JSON never went into a release and that's why 
it's ok to break the format?



src/tests/slave_recovery_tests.cpp
Line 148 (original), 148 (patched)


ASSERT_SOME(actual_) ?


- Vinod Kone


On June 14, 2017, 10:44 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60072/
> ---
> 
> (Updated June 14, 2017, 10:44 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7670
> https://issues.apache.org/jira/browse/MESOS-7670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests to use the test utilities with reservation refinement.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 2b55847be79c7930b598ef31c932e8aca0fa73db 
>   include/mesos/v1/resources.hpp 7c6e8569b990ad34fd6c449b31140f844dae222f 
>   src/cli/execute.cpp 11a2569f788a2ee1ceaf13ab0d40d1d1b275f27a 
>   src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
>   src/examples/dynamic_reservation_framework.cpp 
> 41228145cf2249e383166a47a3ac33fb2bee27c9 
>   src/examples/persistent_volume_framework.cpp 
> ab4597d1fb61f631cd3f52479ae68dcc5e4cd394 
>   src/examples/test_framework.cpp 05ddc89d953d9b968b78f98fec819aeda7f79e26 
>   src/examples/test_http_framework.cpp 
> 471835c349e0da031a540ed48881227a25887ba7 
>   src/tests/api_tests.cpp 91b3473452b8e65cab9f2e873837d64a0edf4b54 
>   src/tests/default_executor_tests.cpp 
> a76ece5a2832d3a006476672e443573d60d991e9 
>   src/tests/hierarchical_allocator_tests.cpp 
> d6c5904b6e8af6ed8e8387de5e635fb4c920dd08 
>   src/tests/master_quota_tests.cpp 6ca45706f47b7d75db519f98af64e6a52091eb84 
>   src/tests/master_validation_tests.cpp 
> 4e7ce74edf0069b9317f869b299694a45e2a62f2 
>   src/tests/mesos.hpp 48072a976cdbe6e655dc6f5f258abc6d737ed068 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 0cb969cf06a123504720440f436ecb027d1e138a 
>   src/tests/reservation_endpoints_tests.cpp 
> ba283ee74fe27998a01c43e1ab6a8ce0089181fa 
>   src/tests/reservation_tests.cpp 6e9c215382ef41700921a673669ac1a7975e9b7f 
>   src/tests/resources_tests.cpp b5913860bf4908d0a8fa2c3b9060823bce41bc25 
>   src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5 
>   src/tests/slave_recovery_tests.cpp 38502584186793686f78ff5f4e03f36a3bf7ad1c 
>   src/tests/slave_tests.cpp 26901f4294ee74e9f8ac57f2b506d369ea540a15 
>   src/tests/upgrade_tests.cpp dfe77e706e3a8cf0897a8895f747b9d51d47128e 
>   src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 
> 
> 
> Diff: https://reviews.apache.org/r/60072/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.

[Silas Snider]

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

(Updated June 14, 2017, 6:31 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Test that bind-mounted host network configuration is mounted readonly.


Diffs (updated)
-

  src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 


Diff: https://reviews.apache.org/r/58250/diff/7/

Changes: https://reviews.apache.org/r/58250/diff/6-7/


Testing
---


Thanks,

Silas Snider

Re: Review Request 60071: Updated test utilities for reservation refinement.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/mesos.hpp
Lines 732 (patched)


2 blank lines.



src/tests/mesos.hpp
Line 875 (original), 894-895 (patched)


do we prefer non-const reference to a pointer here?



src/tests/mesos.hpp
Line 875 (original), 894-900 (patched)


can you move this to #911 and move `reservation` inside the if condition; 
to be consistent with how you did it in #944



src/tests/mesos.hpp
Line 924 (original), 945 (patched)


ditto.


- Vinod Kone


On June 14, 2017, 9:58 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60071/
> ---
> 
> (Updated June 14, 2017, 9:58 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7669
> https://issues.apache.org/jira/browse/MESOS-7669
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated test utilities for reservation refinement.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 48072a976cdbe6e655dc6f5f258abc6d737ed068 
> 
> 
> Diff: https://reviews.apache.org/r/60071/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60021: Updated the use of `toUnreserved()`.

[Benjamin Mahler]

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


Fix it, then Ship it!




Looks good, although this merits a description, how about:

```
Updated (UN)RESERVE operation application to handle reservation refinement.

The logic for applying RESERVE operations now needs to ensure that the
pre-reserved resources are being refined. In other words, the RESERVE
operation should be "pushing" a single reservation to the stack. Note
that we only support "pushing" a single reservation at a time.

For UNRESERVE, the operation should "pop" the last reservation off the
stack. We only support "popping" a single reservation at a time.
```


src/common/resources.cpp
Line 1474 (original), 1474 (patched)


Can you add a note here saying we only allow "pushing" a single reservation 
at a time to the "stack" of reservations?

I noted we should document this under 
[MESOS-7663](https://issues.apache.org/jira/browse/MESOS-7663).



src/common/resources.cpp
Line 1505 (original), 1505 (patched)


Ditto here, can you add a note here saying we only allow "popping" a single 
reservation at a time to the "stack" of reservations?



src/master/http.cpp
Lines 2245-2248 (original), 2245-2248 (patched)


Do you want to re-write this note to reflect that we only allow the 
operator to push one reservation at a time?

Also, not yours, but reading this code I'm left confused as to why we need 
to pass the popped resources of the operation and the operation into the 
continuation. I think the following would clear both of these up?

```
// We only allow pushing a single reservation at a time, so
// we require the resources with one reservation "popped" to
// be present on the agent.
Resources required = resources.popReservation();

return _operation(slaveId, required, operation);
```



src/master/http.cpp
Lines 2245-2249 (original), 2245-2250 (patched)


Shouldn't I see a similar change in `Master::Http::_unreserve` logic? 
Striked me as a little odd that they both didn't need a change.



src/master/http.cpp
Lines 2250 (patched)


Why did you need to wrap with Resources() here? Did you originally have 
popReservation as a mutating fucntion and this is a leftover?


- Benjamin Mahler


On June 13, 2017, 8:37 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60021/
> ---
> 
> (Updated June 13, 2017, 8:37 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7655
> https://issues.apache.org/jira/browse/MESOS-7655
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the use of `toUnreserved()`.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
>   src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
>   src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 
> 
> 
> Diff: https://reviews.apache.org/r/60021/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 60068: Updated the authorizer to look at the correct fields.

[Vinod Kone]

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


Fix it, then Ship it!





src/authorizer/local/authorizer.cpp
Line 558 (original), 561 (patched)


for extracting reservation principal above, you pulled it into a temp 
variable and did a CHECK_SOME, whereas here you are directly doing a `.get()`. 
why the inconsistency?


- Vinod Kone


On June 14, 2017, 9:55 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60068/
> ---
> 
> (Updated June 14, 2017, 9:55 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the authorizer to look at the correct fields.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> 1f2a9902e88705cf412af834c127c3afe6d3bef4 
> 
> 
> Diff: https://reviews.apache.org/r/60068/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 54693: Add ProtoBuf schema for Blkio cgroup subsystem

[Gilbert Song]

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



@Jason, could you reopen this patch for review? I want to apply it to my chain. 
Thanks.

- Gilbert Song


On Dec. 21, 2016, 12:06 p.m., Jason Lai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54693/
> ---
> 
> (Updated Dec. 21, 2016, 12:06 p.m.)
> 
> 
> Review request for mesos, Xiaojian Huang, Gilbert Song, haosdent huang, Jie 
> Yu, Kunal Thakar, and Zhitao Li.
> 
> 
> Bugs: MESOS-6162
> https://issues.apache.org/jira/browse/MESOS-6162
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add ProtoBuf schema for Blkio cgroups per the kernel Blkio controller doc.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0017d3d9d46433b391703025f611ce437bbc7ebe 
> 
> 
> Diff: https://reviews.apache.org/r/54693/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Lai
> 
>

Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.

[Jie Yu]

> On June 14, 2017, 4:41 a.m., James Peach wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp
> > Lines 1451 (patched)
> > 
> >
> > This should be `ROOT_INTERNET_ReadOnlyBindMounts` because it doesn't 
> > need `curl`.

You still need CURL filter because downloading docker image needs curl 
(unfortunate, but this is the case for now).


- Jie


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


On June 14, 2017, 2:12 p.m., Silas Snider wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58250/
> ---
> 
> (Updated June 14, 2017, 2:12 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Test that bind-mounted host network configuration is mounted readonly.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 
> 
> 
> Diff: https://reviews.apache.org/r/58250/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Silas Snider
> 
>

Re: Review Request 57884: Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 14, 2017, 1:06 a.m., Silas Snider wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57884/
> ---
> 
> (Updated June 14, 2017, 1:06 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7268
> https://issues.apache.org/jira/browse/MESOS-7268
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure that host /etc/* files are mounted RDONLY by the CNI Isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 36217d2e5 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 3aecd8c5a 
> 
> 
> Diff: https://reviews.apache.org/r/57884/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Silas Snider
> 
>

Re: Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

[Neil Conway]

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



A bit more detail in the commit message would be good: for example, how does 
this change relate to ca8d33ab12d702612492928c0a5080bd459830bd ?


src/master/validation.cpp
Lines 1830 (patched)


Seems unfortunate to have to pass in the `SlaveID` here just for error 
messages. I wonder if the callers of `validate` can include the `SlaveID` in 
the error message they return when validation fails?



src/master/validation.cpp
Lines 1856 (patched)


We don't use single-quotes for capability names elsewhere.



src/tests/master_validation_tests.cpp
Lines 279 (patched)


So we're passing an invalid (unset) `SlaveID` to the `validate` call below? 
Seems confusing.



src/tests/master_validation_tests.cpp
Lines 331 (patched)


Setting this to `true` suggests to me that the test requires the 
hierarchical agent capability, which isn't the case. I wonder if we want 
something like a `DEFAULT_AGENT_CAPABILITIES` constant?

For now I'd just pass in a default-initialized `slave::Capabilities`, 
without `hierarchicalRole` set.


- Neil Conway


On June 14, 2017, 9:52 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60062/
> ---
> 
> (Updated June 14, 2017, 9:52 a.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevented reserve/create with hierarchical roles on a non-capable agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
>   src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58 
>   src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
>   src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
>   src/tests/master_validation_tests.cpp 
> 4e7ce74edf0069b9317f869b299694a45e2a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/60062/diff/3/
> 
> 
> Testing
> ---
> 
> Added new tests to `master_validation_tests.cpp` + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 60088: CLI: added 'agent' key as an acceptable key in config.toml.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This key is a dictionnary accepting two keys: ip and port.
It will be used by future plugins.


Diffs
-

  src/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
  src/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5 


Diff: https://reviews.apache.org/r/60088/diff/1/


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Review Request 60089: CLI: Added a 'mesos container ps' command.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This command displays the containers running on a specific agent.


Diffs
-

  src/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
  src/cli_new/lib/cli/plugins/container/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/plugins/container/main.py PRE-CREATION 
  src/cli_new/lib/cli/tests/__init__.py 
0daf28869e107263c51653ace39e3b1826871048 
  src/cli_new/lib/cli/tests/container.py PRE-CREATION 
  src/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 


Diff: https://reviews.apache.org/r/60089/diff/1/


Testing
---

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet

Review Request 60087: CLI: Added the ability for a plugin command to have an alias.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

This will be used by future plugins.


Diffs
-

  src/cli_new/lib/cli/plugins/base.py edbe1ec3ce733faea0fd416c51a0884645da3dff 


Diff: https://reviews.apache.org/r/60087/diff/1/


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Re: Review Request 58250: Test that bind-mounted host network configuration is mounted readonly.

[Silas Snider]

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

(Updated June 14, 2017, 2:12 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Test that bind-mounted host network configuration is mounted readonly.


Diffs (updated)
-

  src/tests/containerizer/cni_isolator_tests.cpp 3a5f4ebd4 


Diff: https://reviews.apache.org/r/58250/diff/6/

Changes: https://reviews.apache.org/r/58250/diff/5-6/


Testing
---


Thanks,

Silas Snider

Re: Review Request 59949: Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.

[Avinash sridharan]

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

(Updated June 14, 2017, 1:57 p.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Addressed Jie's comments on adding a TODO.


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


Repository: mesos


Description
---

Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.


Diffs (updated)
-

  src/launcher/default_executor.cpp 91a4e1ebed3ff979bd3c0f7f46559cec24f61c2c 


Diff: https://reviews.apache.org/r/59949/diff/4/

Changes: https://reviews.apache.org/r/59949/diff/3-4/


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 59949: Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.

[Avinash sridharan]

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

(Updated June 14, 2017, 1:53 p.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Addressed Anand's comments.


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


Repository: mesos


Description
---

Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.


Diffs (updated)
-

  src/launcher/default_executor.cpp 91a4e1ebed3ff979bd3c0f7f46559cec24f61c2c 


Diff: https://reviews.apache.org/r/59949/diff/3/

Changes: https://reviews.apache.org/r/59949/diff/2-3/


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 59949: Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.

[Avinash sridharan]

> On June 13, 2017, 6:42 p.m., Jie Yu wrote:
> > src/launcher/default_executor.cpp
> > Lines 371 (patched)
> > 
> >
> > I'd add a TODO here. Because for bridge mode, `self().address.ip` might 
> > be the advertise IP (i.e., agent IP), which is not what we've wanted here.

>From a CNI perspective the BRIDGE mode is just another CNI network, so the 
>self().address.ip will be the IP on the BRIDGE network which is what is 
>expected. So for the time being dropping this comment. Please open if you 
>still feel this is an issue.


> On June 13, 2017, 6:42 p.m., Jie Yu wrote:
> > src/launcher/default_executor.cpp
> > Lines 438 (patched)
> > 
> >
> > Let's add a TODO to document this Downward API.

Instead of a TODO, should we just create a JIRA and add it to the CNI 
documentation and the executor documentation?


- Avinash


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


On June 9, 2017, 6:39 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59949/
> ---
> 
> (Updated June 9, 2017, 6:39 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Bugs: MESOS-7631
> https://issues.apache.org/jira/browse/MESOS-7631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `MESOS_CONTAINER_IP` to tasks launched by `DefaultExecutor`.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 1a60e3bb2b0e8312b349cddedb4cd55716c1b574 
> 
> 
> Diff: https://reviews.apache.org/r/59949/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59186: Additional linux/capabilities isolator documentation.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59547, 59548, 59549, 59550, 59551, 59552, 59185, 59553, 
59554, 59806, 59186]

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 5, 2017, 10:29 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59186/
> ---
> 
> (Updated June 5, 2017, 10:29 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7477
> https://issues.apache.org/jira/browse/MESOS-7477
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Linux capabilities are somewhat involved, so add some additional
> exposition of how the linux/capabilites isolator handles them.
> 
> 
> Diffs
> -
> 
>   docs/linux_capabilities.md b588aff6842a14bbf7ff5c35931cac61f9019805 
> 
> 
> Diff: https://reviews.apache.org/r/59186/diff/6/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 60072: Updated tests to use the test utilities with reservation refinement.

[Michael Park]

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

(Updated June 14, 2017, 3:44 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Updated tests to use the test utilities with reservation refinement.


Diffs (updated)
-

  include/mesos/resources.hpp 2b55847be79c7930b598ef31c932e8aca0fa73db 
  include/mesos/v1/resources.hpp 7c6e8569b990ad34fd6c449b31140f844dae222f 
  src/cli/execute.cpp 11a2569f788a2ee1ceaf13ab0d40d1d1b275f27a 
  src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
  src/examples/dynamic_reservation_framework.cpp 
41228145cf2249e383166a47a3ac33fb2bee27c9 
  src/examples/persistent_volume_framework.cpp 
ab4597d1fb61f631cd3f52479ae68dcc5e4cd394 
  src/examples/test_framework.cpp 05ddc89d953d9b968b78f98fec819aeda7f79e26 
  src/examples/test_http_framework.cpp 471835c349e0da031a540ed48881227a25887ba7 
  src/tests/api_tests.cpp 91b3473452b8e65cab9f2e873837d64a0edf4b54 
  src/tests/default_executor_tests.cpp a76ece5a2832d3a006476672e443573d60d991e9 
  src/tests/hierarchical_allocator_tests.cpp 
d6c5904b6e8af6ed8e8387de5e635fb4c920dd08 
  src/tests/master_quota_tests.cpp 6ca45706f47b7d75db519f98af64e6a52091eb84 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 
  src/tests/mesos.hpp 48072a976cdbe6e655dc6f5f258abc6d737ed068 
  src/tests/persistent_volume_endpoints_tests.cpp 
0cb969cf06a123504720440f436ecb027d1e138a 
  src/tests/reservation_endpoints_tests.cpp 
ba283ee74fe27998a01c43e1ab6a8ce0089181fa 
  src/tests/reservation_tests.cpp 6e9c215382ef41700921a673669ac1a7975e9b7f 
  src/tests/resources_tests.cpp b5913860bf4908d0a8fa2c3b9060823bce41bc25 
  src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5 
  src/tests/slave_recovery_tests.cpp 38502584186793686f78ff5f4e03f36a3bf7ad1c 
  src/tests/slave_tests.cpp 26901f4294ee74e9f8ac57f2b506d369ea540a15 
  src/tests/upgrade_tests.cpp dfe77e706e3a8cf0897a8895f747b9d51d47128e 
  src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 


Diff: https://reviews.apache.org/r/60072/diff/4/

Changes: https://reviews.apache.org/r/60072/diff/3-4/


Testing
---


Thanks,

Michael Park

Review Request 60073: Added `RESERVATION_REFINEMENT` capability to tests.

[Michael Park]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added `RESERVATION_REFINEMENT` capability to tests.


Diffs
-

  src/examples/dynamic_reservation_framework.cpp 
41228145cf2249e383166a47a3ac33fb2bee27c9 
  src/tests/api_tests.cpp 91b3473452b8e65cab9f2e873837d64a0edf4b54 
  src/tests/default_executor_tests.cpp a76ece5a2832d3a006476672e443573d60d991e9 
  src/tests/disk_quota_tests.cpp 3bf0508238a228d86737d6cc899fa68e2046f2e2 
  src/tests/hierarchical_allocator_tests.cpp 
d6c5904b6e8af6ed8e8387de5e635fb4c920dd08 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 
  src/tests/persistent_volume_endpoints_tests.cpp 
0cb969cf06a123504720440f436ecb027d1e138a 
  src/tests/persistent_volume_tests.cpp 
ce716bc78a7f7f180fca896e780c74ef206a612b 
  src/tests/reservation_endpoints_tests.cpp 
ba283ee74fe27998a01c43e1ab6a8ce0089181fa 
  src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5 


Diff: https://reviews.apache.org/r/60073/diff/1/


Testing
---


Thanks,

Michael Park

Review Request 60074: Updated authorization tests.

[Michael Park]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Updated authorization tests.


Diffs
-

  src/tests/authorization_tests.cpp 6d85a5452d50390f96e0b11c2bac5c96722f6eac 


Diff: https://reviews.apache.org/r/60074/diff/1/


Testing
---


Thanks,

Michael Park

Review Request 60072: Updated tests to use the test utilities with reservation refinement.

[Michael Park]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Updated tests to use the test utilities with reservation refinement.


Diffs
-

  include/mesos/resources.hpp 2b55847be79c7930b598ef31c932e8aca0fa73db 
  include/mesos/v1/resources.hpp 7c6e8569b990ad34fd6c449b31140f844dae222f 
  src/cli/execute.cpp 11a2569f788a2ee1ceaf13ab0d40d1d1b275f27a 
  src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
  src/examples/dynamic_reservation_framework.cpp 
41228145cf2249e383166a47a3ac33fb2bee27c9 
  src/examples/persistent_volume_framework.cpp 
ab4597d1fb61f631cd3f52479ae68dcc5e4cd394 
  src/examples/test_framework.cpp 05ddc89d953d9b968b78f98fec819aeda7f79e26 
  src/examples/test_http_framework.cpp 471835c349e0da031a540ed48881227a25887ba7 
  src/tests/api_tests.cpp 91b3473452b8e65cab9f2e873837d64a0edf4b54 
  src/tests/default_executor_tests.cpp a76ece5a2832d3a006476672e443573d60d991e9 
  src/tests/hierarchical_allocator_tests.cpp 
d6c5904b6e8af6ed8e8387de5e635fb4c920dd08 
  src/tests/master_quota_tests.cpp 6ca45706f47b7d75db519f98af64e6a52091eb84 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 
  src/tests/mesos.hpp 48072a976cdbe6e655dc6f5f258abc6d737ed068 
  src/tests/persistent_volume_endpoints_tests.cpp 
0cb969cf06a123504720440f436ecb027d1e138a 
  src/tests/reservation_endpoints_tests.cpp 
ba283ee74fe27998a01c43e1ab6a8ce0089181fa 
  src/tests/reservation_tests.cpp 6e9c215382ef41700921a673669ac1a7975e9b7f 
  src/tests/resources_tests.cpp b5913860bf4908d0a8fa2c3b9060823bce41bc25 
  src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5 
  src/tests/slave_recovery_tests.cpp 38502584186793686f78ff5f4e03f36a3bf7ad1c 
  src/tests/slave_tests.cpp 26901f4294ee74e9f8ac57f2b506d369ea540a15 
  src/tests/upgrade_tests.cpp dfe77e706e3a8cf0897a8895f747b9d51d47128e 
  src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 


Diff: https://reviews.apache.org/r/60072/diff/1/


Testing
---


Thanks,

Michael Park

Review Request 60071: Updated test utilities for reservation refinement.

[Michael Park]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Updated test utilities for reservation refinement.


Diffs
-

  src/tests/mesos.hpp 48072a976cdbe6e655dc6f5f258abc6d737ed068 


Diff: https://reviews.apache.org/r/60071/diff/1/


Testing
---


Thanks,

Michael Park

Review Request 60070: Updated the agent to account for the new resources format.

[Michael Park]

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

Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Updated the agent to account for the new resources format.


Diffs
-

  src/slave/paths.cpp b2709ad3b111f102ff223182a30c993a0c643230 
  src/slave/slave.hpp 7ffaed14035a05259ec72c70532ee4f0affa1f5d 
  src/slave/slave.cpp 7d147ac6609933ac884bfc29032dba572a0952c6 
  src/slave/state.hpp a497ce1f58fb8dc7718ee5bb10bc62dd7479efa5 
  src/slave/state.cpp 18b790d2cc4f537cc9b0c3cca59b9cbaac0eda10 


Diff: https://reviews.apache.org/r/60070/diff/1/


Testing
---


Thanks,

Michael Park

Review Request 60069: Updated the master to account for the new resources format.

[Michael Park]

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

Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Updated the master to account for the new resources format.


Diffs
-

  src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
  src/master/master.hpp e8f273256b14cde1cac390163f948241757f 
  src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58 
  src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7 
  src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 


Diff: https://reviews.apache.org/r/60069/diff/1/


Testing
---


Thanks,

Michael Park

Review Request 60068: Updated the authorizer to look at the correct fields.

[Michael Park]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Updated the authorizer to look at the correct fields.


Diffs
-

  src/authorizer/local/authorizer.cpp 1f2a9902e88705cf412af834c127c3afe6d3bef4 


Diff: https://reviews.apache.org/r/60068/diff/1/


Testing
---


Thanks,

Michael Park

Re: Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

[Michael Park]

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

(Updated June 14, 2017, 2:52 a.m.)


Review request for mesos and Neil Conway.


Repository: mesos


Description (updated)
---

Prevented reserve/create with hierarchical roles on a non-capable agent.


Diffs (updated)
-

  src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
  src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58 
  src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
  src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 


Diff: https://reviews.apache.org/r/60062/diff/2/

Changes: https://reviews.apache.org/r/60062/diff/1-2/


Testing
---

Added new tests to `master_validation_tests.cpp` + `make check`


Thanks,

Michael Park

Review Request 60067: Updated of documents including authorization in operator endpoints.

[Alexander Rojas]

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

Review request for mesos, Greg Mann and Till Toenshoff.


Repository: mesos


Description
---

Automatically generated by running
`./support/generate-endpoint-help.py`


Diffs
-

  docs/endpoints/master/machine/down.md 
a605a99f6b3a5b05ddc6cdd9e71bc326c4f96c27 
  docs/endpoints/master/machine/up.md 67cfc159b579740f9c82edc35fe1a70165efb36d 
  docs/endpoints/master/maintenance/schedule.md 
d3d7154ed863d5c9f39099f945d30a5ef2d3475f 
  docs/endpoints/master/maintenance/status.md 
1a35eba2c4b9ae236df1266fffa8ed007ccb4d6e 


Diff: https://reviews.apache.org/r/60067/diff/1/


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 59988: Added a new protobuf message `MapMessage` for protobuf tests.

[Benjamin Bannier]

> On June 12, 2017, 5:41 p.m., Zhitao Li wrote:
> > It seems that  majority of this patch is generated code. Is the `.proto` 
> > change the only real code change? If so, should we write some c++ test code 
> > to use the map based fields and json parsing of map?
> > 
> > Also, explaining which files are generated in summary will help reviewers 
> > to know what changes we can safekly skip reading?
> 
> Qian Zhang wrote:
> Yes, the `.proto` change is the only real code changed, I have updated 
> the description to mention that. And for the test, please refer to 
> https://reviews.apache.org/r/59989/.
> 
> Benjamin Bannier wrote:
> Could you have a look and try to find out how hard it would be to 
> generate the `*.pb.*` files during the build instead of checking them in, 
> https://issues.apache.org/jira/browse/MESOS-6115?
> 
> Qian Zhang wrote:
> In MESOS-6115, I see you mentioned:
> > We should try to remove them from the source tree; in fact 
> protobuf_tests.cpp already documents ways this could be achieved.
> 
> I am not sure what you meant for `protobuf_tests.cpp already documents 
> ways this could be achieved.`, did you mean the comments here: 
> https://github.com/apache/mesos/blob/1.3.0/3rdparty/stout/tests/protobuf_tests.cpp#L88:L104
>  which seems the way to generate protobuf message dynamically in the runtime 
> rather than during the build.

I am not sure what I was referring to there, so I removed that line from the 
ticket. It seems just adding a rule to generate these output files and adding 
them to the test binary like we already do would do the job.


- Benjamin


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


On June 13, 2017, 10:41 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59988/
> ---
> 
> (Updated June 13, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The real code changes is adding `MapMessage` to protobuf_tests.proto,
> for the other two files in this patch, they are automatically generated
> by running protobuf-3.3.0 compiler `protoc` on protobuf_tests.proto.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.pb.h 
> 2e4ffe17a07ce2360ec618e936ae4557e9dc8e62 
>   3rdparty/stout/tests/protobuf_tests.pb.cc 
> ad6eff779d1cc0e7d037ea77565533c3ebb0b2d6 
>   3rdparty/stout/tests/protobuf_tests.proto 
> d16726aa8060aea2b830040b20dbdd467c801483 
> 
> 
> Diff: https://reviews.apache.org/r/59988/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 60036: Added a hack to avoid printing out the Resource.role field.

[Michael Park]

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

(Updated June 14, 2017, 1:18 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased.


Summary (updated)
-

Added a hack to avoid printing out the Resource.role field.


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


Repository: mesos


Description (updated)
---

Added a hack to avoid printing out the Resource.role field.


Diffs (updated)
-

  include/mesos/resources.hpp 2b55847be79c7930b598ef31c932e8aca0fa73db 
  src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
  src/slave/http.cpp 78b35865e465ff1e8e7e4950fdb60e3a48b916b6 


Diff: https://reviews.apache.org/r/60036/diff/2/

Changes: https://reviews.apache.org/r/60036/diff/1-2/


Testing
---


Thanks,

Michael Park

Re: Review Request 60035: Added support for printing JSON values via `jsonify`.

[Michael Park]

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

(Updated June 14, 2017, 1:17 a.m.)


Review request for mesos, Benjamin Hindman and Benjamin Mahler.


Repository: mesos


Description
---

Added support for printing JSON values via `jsonify`.


Diffs
-

  3rdparty/stout/include/stout/json.hpp 
aa581c0345679b6f9617fb986d744110a728f042 
  3rdparty/stout/include/stout/jsonify.hpp 
7d84bac18abbe9f5bcf67a377d4e742e31b4c94d 
  3rdparty/stout/tests/jsonify_tests.cpp 
d06eba31e6b7f252b120a10066e4d7c9a25ce4b5 


Diff: https://reviews.apache.org/r/60035/diff/4/


Testing
---

Added new tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 1:15 a.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
  src/tests/master_tests.cpp 543c4a3afab818578c3124331a092cb1057776af 
  src/tests/slave_tests.cpp 26901f4294ee74e9f8ac57f2b506d369ea540a15 


Diff: https://reviews.apache.org/r/59861/diff/4/

Changes: https://reviews.apache.org/r/59861/diff/3-4/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 1:06 a.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Addressed some of bmahler's comments.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
  src/tests/master_tests.cpp 543c4a3afab818578c3124331a092cb1057776af 
  src/tests/slave_tests.cpp 26901f4294ee74e9f8ac57f2b506d369ea540a15 


Diff: https://reviews.apache.org/r/59861/diff/3/

Changes: https://reviews.apache.org/r/59861/diff/2-3/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59988: Added a new protobuf message `MapMessage` for protobuf tests.

[Qian Zhang]

> On June 12, 2017, 11:41 p.m., Zhitao Li wrote:
> > It seems that  majority of this patch is generated code. Is the `.proto` 
> > change the only real code change? If so, should we write some c++ test code 
> > to use the map based fields and json parsing of map?
> > 
> > Also, explaining which files are generated in summary will help reviewers 
> > to know what changes we can safekly skip reading?
> 
> Qian Zhang wrote:
> Yes, the `.proto` change is the only real code changed, I have updated 
> the description to mention that. And for the test, please refer to 
> https://reviews.apache.org/r/59989/.
> 
> Benjamin Bannier wrote:
> Could you have a look and try to find out how hard it would be to 
> generate the `*.pb.*` files during the build instead of checking them in, 
> https://issues.apache.org/jira/browse/MESOS-6115?

In MESOS-6115, I see you mentioned:
> We should try to remove them from the source tree; in fact protobuf_tests.cpp 
> already documents ways this could be achieved.

I am not sure what you meant for `protobuf_tests.cpp already documents ways 
this could be achieved.`, did you mean the comments here: 
https://github.com/apache/mesos/blob/1.3.0/3rdparty/stout/tests/protobuf_tests.cpp#L88:L104
 which seems the way to generate protobuf message dynamically in the runtime 
rather than during the build.


- Qian


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


On June 13, 2017, 4:41 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59988/
> ---
> 
> (Updated June 13, 2017, 4:41 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The real code changes is adding `MapMessage` to protobuf_tests.proto,
> for the other two files in this patch, they are automatically generated
> by running protobuf-3.3.0 compiler `protoc` on protobuf_tests.proto.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.pb.h 
> 2e4ffe17a07ce2360ec618e936ae4557e9dc8e62 
>   3rdparty/stout/tests/protobuf_tests.pb.cc 
> ad6eff779d1cc0e7d037ea77565533c3ebb0b2d6 
>   3rdparty/stout/tests/protobuf_tests.proto 
> d16726aa8060aea2b830040b20dbdd467c801483 
> 
> 
> Diff: https://reviews.apache.org/r/59988/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 60064: Updated `isAllocatableTo` to use `isSubroleOf` utility.

[Michael Park]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/common/resources.cpp 7b3fae1a34150964ba3831a3aef2f868e338aef8 
  src/v1/resources.cpp 236fe17730918479a33314e51d14f3cc1679d432 


Diff: https://reviews.apache.org/r/60064/diff/1/


Testing
---

`make check`


Thanks,

Michael Park

Review Request 60063: Introduced `roles::isSubroleOf` abstraction.

[Michael Park]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/roles.hpp 9a74182bb1462c022e3f06380418383fcd324168 
  src/common/roles.cpp 123e99073f8acbf4eb50e257e349e5485fba1cc6 
  src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5 


Diff: https://reviews.apache.org/r/60063/diff/1/


Testing
---

Added new tests in `role_tests.cpp` + `make check`


Thanks,

Michael Park

Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

[Michael Park]

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

Review request for mesos and Neil Conway.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
  src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58 
  src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
  src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 


Diff: https://reviews.apache.org/r/60062/diff/1/


Testing
---

Added new tests to `master_validation_tests.cpp` + `make check`


Thanks,

Michael Park