Re: Review Request 55710: Added Agent capabilities in the response of /state endpoint.

2017-01-23 Thread Benjamin Mahler


> On Jan. 23, 2017, 10:33 p.m., Michael Park wrote:
> > Ship It!
> 
> Michael Park wrote:
> Do we have a patch to add `capabilities` to Agent `/state` endpoint as 
> well?

FYI

I was looking into setting the MULTI_ROLE capability in the agent code, and I'd 
like to update what we committed to follow the approach we took for sending the 
agent's version. The version is communicated through the registration and 
re-registration messages as a separate field rather than being set inside 
`SlaveInfo`. This was because we don't handle an updated `SlaveInfo` currently 
(the agent considers `SlaveInfo` changes during restart to be incompatible, and 
the master does not update the `SlaveInfo` in the registry if it were to 
change). So, to avoid tackling these problems now, we can start with just 
having capabilities follow the same approach as what we did for the agent 
version. Make sense? Can you send patches for this? Note that it doesn't need 
to block any of your outstanding patches from going in AFAICT.


- Benjamin


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


On Jan. 20, 2017, 5:29 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55710/
> ---
> 
> (Updated Jan. 20, 2017, 5:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Bugs: MESOS-6902
> https://issues.apache.org/jira/browse/MESOS-6902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
> It is automatically wrapped into response of GetState v1 API. This
> patch added this field to /state v0 API for consistency.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
>   src/tests/master_tests.cpp da7094dbbafbb0ab1153a0a4a6fcabd63888d67a 
> 
> Diff: https://reviews.apache.org/r/55710/diff/
> 
> 
> Testing
> ---
> 
> make check GTEST_FILTER="MasterTest.StateEndpointAgentCapabilities"
> [   OK ] MasterTest.StateEndpointAgentCapabilities (85 ms)
> [--] 1 test from MasterTest (94 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (122 ms total)
> 
> make check on Ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>



Re: Review Request 55710: Added Agent capabilities in the response of /state endpoint.

2017-01-23 Thread Michael Park


> On Jan. 23, 2017, 2:33 p.m., Michael Park wrote:
> > Ship It!

Do we have a patch to add `capabilities` to Agent `/state` endpoint as well?


- Michael


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


On Jan. 19, 2017, 9:29 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55710/
> ---
> 
> (Updated Jan. 19, 2017, 9:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Bugs: MESOS-6902
> https://issues.apache.org/jira/browse/MESOS-6902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
> It is automatically wrapped into response of GetState v1 API. This
> patch added this field to /state v0 API for consistency.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
>   src/tests/master_tests.cpp da7094dbbafbb0ab1153a0a4a6fcabd63888d67a 
> 
> Diff: https://reviews.apache.org/r/55710/diff/
> 
> 
> Testing
> ---
> 
> make check GTEST_FILTER="MasterTest.StateEndpointAgentCapabilities"
> [   OK ] MasterTest.StateEndpointAgentCapabilities (85 ms)
> [--] 1 test from MasterTest (94 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (122 ms total)
> 
> make check on Ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>



Re: Review Request 55710: Added Agent capabilities in the response of /state endpoint.

2017-01-23 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Jan. 19, 2017, 9:29 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55710/
> ---
> 
> (Updated Jan. 19, 2017, 9:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Bugs: MESOS-6902
> https://issues.apache.org/jira/browse/MESOS-6902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
> It is automatically wrapped into response of GetState v1 API. This
> patch added this field to /state v0 API for consistency.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
>   src/tests/master_tests.cpp da7094dbbafbb0ab1153a0a4a6fcabd63888d67a 
> 
> Diff: https://reviews.apache.org/r/55710/diff/
> 
> 
> Testing
> ---
> 
> make check GTEST_FILTER="MasterTest.StateEndpointAgentCapabilities"
> [   OK ] MasterTest.StateEndpointAgentCapabilities (85 ms)
> [--] 1 test from MasterTest (94 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (122 ms total)
> 
> make check on Ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>



Re: Review Request 55710: Added Agent capabilities in the response of /state endpoint.

2017-01-20 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [55710]

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 Jan. 20, 2017, 5:29 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55710/
> ---
> 
> (Updated Jan. 20, 2017, 5:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.
> 
> 
> Bugs: MESOS-6902
> https://issues.apache.org/jira/browse/MESOS-6902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
> It is automatically wrapped into response of GetState v1 API. This
> patch added this field to /state v0 API for consistency.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
>   src/tests/master_tests.cpp da7094dbbafbb0ab1153a0a4a6fcabd63888d67a 
> 
> Diff: https://reviews.apache.org/r/55710/diff/
> 
> 
> Testing
> ---
> 
> make check GTEST_FILTER="MasterTest.StateEndpointAgentCapabilities"
> [   OK ] MasterTest.StateEndpointAgentCapabilities (85 ms)
> [--] 1 test from MasterTest (94 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (122 ms total)
> 
> make check on Ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>



Re: Review Request 55710: Added Agent capabilities in the response of /state endpoint.

2017-01-19 Thread Jay Guo

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

(Updated Jan. 20, 2017, 1:29 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


Changes
---

Added a test to check `capablities` is included in `/state` response.


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


Repository: mesos


Description
---

We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
It is automatically wrapped into response of GetState v1 API. This
patch added this field to /state v0 API for consistency.


Diffs (updated)
-

  src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
  src/tests/master_tests.cpp da7094dbbafbb0ab1153a0a4a6fcabd63888d67a 

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


Testing (updated)
---

make check GTEST_FILTER="MasterTest.StateEndpointAgentCapabilities"
[   OK ] MasterTest.StateEndpointAgentCapabilities (85 ms)
[--] 1 test from MasterTest (94 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (122 ms total)

make check on Ubuntu 14.04


Thanks,

Jay Guo



Review Request 55710: Added Agent capabilities in the response of /state endpoint.

2017-01-19 Thread Jay Guo

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


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


Repository: mesos


Description
---

We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
It is automatically wrapped into response of GetState v1 API. This
patch added this field to /state v0 API for consistency.


Diffs
-

  src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 

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


Testing
---

WIP. We need to figure out how to inject `SlaveInfo` during Agent registration 
for testing purpose, before we actually implement Agent Capabilities.


Thanks,

Jay Guo