Re: Review Request 42705: Update docs for --weights flag and authorization.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 8, 2016, 8:09 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42705/
> ---
> 
> (Updated March 8, 2016, 8:09 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update docs for --weights flag and authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md c4449d5da4e2ea3ebfaaf4ed5b682e8f7bf56ac3 
>   docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
>   src/master/flags.cpp be981ed6155edce18bdb55188c78d73182159418 
> 
> Diff: https://reviews.apache.org/r/42705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43824: Addressed comments of 41672.

[Adam B]

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




src/tests/hierarchical_allocator_tests.cpp (lines 236 - 239)


Comparing `int` vs. `unsigned` lead to the ReviewBot error you got. I'd 
just stick with `int`, since it's shorter and the compiler will probably 
optimize all this out anyway.


- Adam B


On March 8, 2016, 7:17 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 8, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44408: Remove setting up of ACLs (unneeded) in PersistentVolumeTests.

[Adam B]

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


Fix it, then Ship it!




Looks great! One more little bit of cleanup, and I'll commit it.


src/tests/persistent_volume_tests.cpp (line 683)


`StartMaster()` without any parameters will call `CreateMasterFlags()` 
anyway, so just drop the `CreateMasterFlags()`.


- Adam B


On March 7, 2016, 12:55 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44408/
> ---
> 
> (Updated March 7, 2016, 12:55 p.m.)
> 
> 
> Review request for mesos, Adam B and Joseph Wu.
> 
> 
> Bugs: MESOS-4868
> https://issues.apache.org/jira/browse/MESOS-4868
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix removes the setting up of ACLs in PersistentVolumeTests
> as it is no longer needed any more with implicit roles (MESOS-4868).
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
> 
> Diff: https://reviews.apache.org/r/44408/diff/
> 
> 
> Testing
> ---
> 
> make check (in Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44512: Support to get weights info by /weights.

[Yongqiao Wang]

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

(Updated March 9, 2016, 6:36 a.m.)


Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Support to get weights info by /weights.


Diffs
-

  include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
  src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/weights_handler.cpp PRE-CREATION 

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


Testing (updated)
---

make && make check.

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--authenticate_http --credentials=/opt/credentials.json  >> 
/tmp/mesos-master.log 2>&1 &)

$ curl --user framework1:secret_string1 http://localhost:5050/weights | python 
-mjson.tool
{}

$ curl --user framework1:secret_string1 --data 
"[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
 -X PUT http://127.0.0.1:5050/weights

$ curl --user framework1:secret_string1 http://localhost:5050/weights | python 
-mjson.tool
{
"infos": [
{
"role": "role3",
"weight": 3.4
},
{
"role": "role2",
"weight": 1.0
},
{
"role": "role1",
"weight": 1.8
}
]
}

Has updated all tests of /weights endpoint to check the update results with 
/weights GET request.


Thanks,

Yongqiao Wang

Re: Review Request 44511: Add registry tests for /weights endpoint.

[Yongqiao Wang]

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

(Updated March 9, 2016, 6:34 a.m.)


Review request for mesos and Adam B.


Changes
---

Check with the /weights GET method.


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


Repository: mesos


Description
---

Add registry tests for /weights endpoint.


Diffs (updated)
-

  src/tests/dynamic_weights_tests.cpp PRE-CREATION 

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


Testing
---

make && make check.


Thanks,

Yongqiao Wang

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44531]

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

- Mesos ReviewBot


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

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



I will discard this patch since I have broken it into two:
https://reviews.apache.org/r/44549/
https://reviews.apache.org/r/44555/

- Qian Zhang


On March 8, 2016, 10:42 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 8, 2016, 10:42 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

> On March 9, 2016, 2:09 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, lines 95-288
> > 
> >
> > I would strongly recommend move all `parse` and `validation` operations 
> > to `spec.cpp`, because we may not want the `create` method to be that long.

I agree with you. However after discussed with Jie and Avinash, we all agree 
that we do not need to valid the fields in the CNI network configuration file 
since the CNI plugin will do the validation when "network/cni" isolator invokes 
it. So I removed all those field validation code from `create()` method which 
makes it much shorter. Please see https://reviews.apache.org/r/44555/ for 
details and let me know if you have further comments :-)


- Qian


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


On March 8, 2016, 10:42 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 8, 2016, 10:42 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 44555: Implemented the framework and create() method of "network/cni" isolator.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented the framework and create() method of "network/cni" isolator.


Diffs
-

  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 41790: Add tests for /weights endpoint.

[Yongqiao Wang]

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

(Updated March 9, 2016, 5:52 a.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Check the updated results wiht /weights endpoint.


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


Repository: mesos


Description
---

Add tests for /weights endpoint.


Diffs (updated)
-

  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/tests/dynamic_weights_tests.cpp PRE-CREATION 

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


Testing (updated)
---

Make and Make check successfully!

$ ./src/mesos-tests --gtest_filter=DynamicWeightsTest.*
[==] Running 11 tests from 1 test case.
[--] Global test environment set-up.
[--] 11 tests from DynamicWeightsTest
[ RUN  ] DynamicWeightsTest.PutInvalidRequest
[   OK ] DynamicWeightsTest.PutInvalidRequest (87 ms)
[ RUN  ] DynamicWeightsTest.ZeroWeight
[   OK ] DynamicWeightsTest.ZeroWeight (44 ms)
[ RUN  ] DynamicWeightsTest.NegativeWeight
[   OK ] DynamicWeightsTest.NegativeWeight (42 ms)
[ RUN  ] DynamicWeightsTest.NonNumericWeight
[   OK ] DynamicWeightsTest.NonNumericWeight (37 ms)
[ RUN  ] DynamicWeightsTest.MissingRole
[   OK ] DynamicWeightsTest.MissingRole (42 ms)
[ RUN  ] DynamicWeightsTest.UnknownRole
[   OK ] DynamicWeightsTest.UnknownRole (38 ms)
[ RUN  ] DynamicWeightsTest.UpdateWeightsWithExplictRoles
[   OK ] DynamicWeightsTest.UpdateWeightsWithExplictRoles (38 ms)
[ RUN  ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest
[   OK ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest (35 ms)
[ RUN  ] DynamicWeightsTest.AuthorizedWeightUpdateRequest
[   OK ] DynamicWeightsTest.AuthorizedWeightUpdateRequest (45 ms)
[ RUN  ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal
[   OK ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal 
(38 ms)
[ RUN  ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest
[   OK ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest (35 ms)
[--] 11 tests from DynamicWeightsTest (482 ms total)

[--] Global test environment tear-down
[==] 11 tests from 1 test case ran. (490 ms total)
[  PASSED  ] 11 tests.


Thanks,

Yongqiao Wang

Re: Review Request 44470: Implemented runtime isoaltor default entrypoint test.

[Guangya Liu]

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




src/tests/containerizer/runtime_isolator_tests.cpp (line 302)


Can you please add some comments here saying that the inky entrypint is 
`ENTRYPOINT [echo]`


- Guangya Liu


On 三月 7, 2016, 9:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44470/
> ---
> 
> (Updated 三月 7, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4889
> https://issues.apache.org/jira/browse/MESOS-4889
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented runtime isoaltor default entrypoint test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/runtime_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44470/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

[Guangya Liu]

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

(Updated 三月 9, 2016, 5:38 a.m.)


Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

There is a bug when setting host maintain with http endpoint: 
https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
The logic is as this:
1) Get all host list from maintain window and put it to updated hashmap.
2) If the machine in was in updated was also in master->machines, call master 
updateUnavailability to trigger recoverResources, updateUnavailability etc in 
allocator
3) Otherwise, clear the unavailabity time window for the machine.
4) Update each new machines in updated to call master updateUnavailability

But the logic in step 4) is getting all machines from the schedule windows but 
not the machines that is new to the cluster, this caused master get two 
updateUnavailability calls for a machine in the updated hashmap.

The fix is filter machines in updated hashmap when handling new machines.


Diffs (updated)
-

  src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 

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


Testing
---

make
make check
 ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose


Thanks,

Guangya Liu

Re: Review Request 44320: Moved authorizer.proto to acls.proto.

[Alexander Rojas]

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

(Updated March 9, 2016, 6:35 a.m.)


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


Changes
---

Added missed header includes.


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


Repository: mesos


Description
---

This is the first step towards separating the language used to define
the ACLs from the mechanism to query them.


Diffs (updated)
-

  include/mesos/authorizer/acls.hpp PRE-CREATION 
  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  include/mesos/authorizer/authorizer.proto  
  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/authorizer/acls.cpp PRE-CREATION 
  src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
  src/common/parse.hpp 78c7cf12ca6a475305254177db6b6b2319a1f72f 
  src/examples/persistent_volume_framework.cpp 
4218b1563e10aaefe9abcdc20c90c13651959790 
  src/examples/test_authorizer_module.cpp 
95d77fbff0cdfdb360a8597fbba28404b59d0042 
  src/master/flags.hpp 6f53099eab9b0e5917e508bef24b2c85302b33e2 
  src/master/quota_handler.cpp a41c91f10bc0eedc754425b4de1b3e184c4ffb08 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 44450: Rescind all outstanding offers to satisfy weights update.

[Yongqiao Wang]

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

(Updated March 9, 2016, 5:19 a.m.)


Review request for mesos and Adam B.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Rescind all outstanding offers to satisfy weights update.


Diffs (updated)
-

  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/weights_handler.cpp PRE-CREATION 

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


Testing
---

Make && Make check.

Manual test steps:

- Start Mesos master:
$ ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master

- Start Mesos slave:
$ ./bin/mesos-slave.sh --master=127.0.0.1:5050

- Register a framwork with `curl`
$ curl -v http://127.0.0.1:5050/api/v1/scheduler -H "Content-type: 
application/json" -X POST -d @subscribe.json
$  cat subscribe.json
{
   "type": "SUBSCRIBE",

   "subscribe" : {
  "framework_info": {
  "user" :  "root",
  "name" :  "comsumer c1 HTTP Framework",
  "role" :  "mesos",
  "principal":"wyq"
  },

  "force" : true
  }
}

And this framework will receive an offer:
{"subscribed":{"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"}},"type":"SUBSCRIBED"}20
{"type":"HEARTBEAT"}680
{"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
{"type":"HEARTBEAT"}20

- Update the weight of role `mesos`
$ curl --data "[{\"weight\":1.8,\"role\":\"mesos\"}]" -X PUT 
http://127.0.0.1:5050/weights

Receive an rescind offer, and a new offer received:
{"rescind":{"offer_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"}},"type":"RESCIND"}680
{"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O1"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
{"type":"HEARTBEAT"}20

- Update the weight of role `mesos1`
$ curl --data "[{\"weight\":2.8,\"role\":\"mesos1\"}]" -X PUT 
http://127.0.0.1:5050/weights

- The outstanding offer will not be rescinded.


(TODO) I will add couple of tests for this patch in another JIRA.


Thanks,

Yongqiao Wang

Re: Review Request 44322: Implemented a generalized interface for the authorizer.

[Alexander Rojas]

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

(Updated March 9, 2016, 6:07 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

**WIP: Do not commit**

Implements the later [Generic Authorizer Interface v 
0.3.1](https://docs.google.com/document/d/1-XARWJFUq0r_TgRHz_472NvLZNjbqE4G8c2JL44OSMQ)
proposal.

It still lacks some parts:

- [ ] Doxygen in the interface.
- [ ] Updating `authorizer.md`

Still the basic functionality is there and I don't expect it to change
much. As I mentioned, comments aren't there yet but feel free to point
where they are missed, at this point focusing in the actual content
may not be relevant as the patch may change from its current form.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  include/mesos/authorizer/authorizer.proto PRE-CREATION 
  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
  src/authorizer/local/authorizer.hpp 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
  src/authorizer/local/authorizer.cpp 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
  src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/quota_handler.cpp a41c91f10bc0eedc754425b4de1b3e184c4ffb08 
  src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/mesos.hpp 9c62833e0a64cfd62fce8cffd04f9cdd933646c8 
  src/tests/mesos.cpp 395b23d32b2d03aef446858e197cb9788644eefa 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 

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


Testing
---

make -j4 check


Thanks,

Alexander Rojas

Re: Review Request 44320: Moved authorizer.proto to acls.proto.

[Alexander Rojas]

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

(Updated March 9, 2016, 5:58 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

This is the first step towards separating the language used to define
the ACLs from the mechanism to query them.


Diffs (updated)
-

  include/mesos/authorizer/acls.hpp PRE-CREATION 
  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  include/mesos/authorizer/authorizer.proto  
  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/authorizer/acls.cpp PRE-CREATION 
  src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
  src/common/parse.hpp 78c7cf12ca6a475305254177db6b6b2319a1f72f 
  src/examples/persistent_volume_framework.cpp 
4218b1563e10aaefe9abcdc20c90c13651959790 
  src/examples/test_authorizer_module.cpp 
95d77fbff0cdfdb360a8597fbba28404b59d0042 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 44319: Removed initialize method from the authorizer interface.

[Alexander Rojas]

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

(Updated March 9, 2016, 5:54 a.m.)


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


Changes
---

Adresses issues reported by Vinod.


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


Repository: mesos


Description
---

Removed initialize method from the authorizer interface.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
  src/authorizer/local/authorizer.hpp 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
  src/authorizer/local/authorizer.cpp 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
  src/examples/test_authorizer_module.cpp 
95d77fbff0cdfdb360a8597fbba28404b59d0042 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
  src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 44319: Removed initialize method from the authorizer interface.

[Alexander Rojas]

> On March 8, 2016, 7:16 p.m., Vinod Kone wrote:
> > src/examples/test_authorizer_module.cpp, lines 41-61
> > 
> >
> > why can't you use Authorizer::create() here instead of repeating this 
> > flags extracting/parsing logic?

Refactor the code to `local::Authorizer::create(ACLs)` so I guess this is not 
relevant anymore.


> On March 8, 2016, 7:16 p.m., Vinod Kone wrote:
> > src/local/local.cpp, lines 225-233
> > 
> >
> > I'm a little confused. 
> > 
> > Does this mean we allow specifying non-local authorizer as 
> > "flags.authorizers" but only allow "acls" parameter to it? What if the 
> > module writer want to send extra parameters to their module?

I refactored this code based on offline discussion.


> On March 8, 2016, 7:16 p.m., Vinod Kone wrote:
> > src/master/main.cpp, lines 363-369
> > 
> >
> > I see the repetition now.
> > 
> > How about we create an overload  Authorizer::create(const ACLs& acls) 
> > that calls Authorizer::create(const string& name, const Parameters& 
> > parameters). Add a TODO saying that this overload should be killed once 
> > ACLs can be input as a module param instead of top level "--acls" flag.

I refactored this code based on offline discussion.


> On March 8, 2016, 7:16 p.m., Vinod Kone wrote:
> > src/tests/authorization_tests.cpp, lines 56-75
> > 
> >
> > why not use Authorizer::create() instead of the repetition here?

I refactored this code based on offline discussion.


- Alexander


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


On March 8, 2016, 5:43 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44319/
> ---
> 
> (Updated March 8, 2016, 5:43 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-2950
> https://issues.apache.org/jira/browse/MESOS-2950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed initialize method from the authorizer interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> ec6c9928c55c3096c7de634f900419abbdd00886 
>   src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
>   src/authorizer/local/authorizer.hpp 
> 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
>   src/authorizer/local/authorizer.cpp 
> 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
>   src/examples/test_authorizer_module.cpp 
> 95d77fbff0cdfdb360a8597fbba28404b59d0042 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
> 
> Diff: https://reviews.apache.org/r/44319/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 44289: Added support for contender and detector modules.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: [44289]

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On March 9, 2016, 1:39 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44289/
> ---
> 
> (Updated March 9, 2016, 1:39 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for contender and detector modules.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/contender.hpp PRE-CREATION 
>   include/mesos/module/detector.hpp PRE-CREATION 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/test_contender_module.cpp PRE-CREATION 
>   src/examples/test_detector_module.cpp PRE-CREATION 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contenders/zookeeper.cpp 
> 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detectors/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/module/manager.cpp 6ae99504005581b22a44768949b1d305cec517d9 
>   src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
>   src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 
> 
> Diff: https://reviews.apache.org/r/44289/diff/
> 
> 
> Testing
> ---
> 
> In addition to all unit tests passing, we are currently using this 
> functionality in our environment with a custom consensus stack. In our world, 
> we have a C++ plugin that calls out to an HTTP REST service (implemented in 
> Java/Scala, not that it matters).
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44523: Changed the master's default HTTP authentication realm.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44515, 44523]

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

- Mesos ReviewBot


On March 8, 2016, 7:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44523/
> ---
> 
> (Updated March 8, 2016, 7:01 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the master's default HTTP authentication realm.
> 
> 
> Diffs
> -
> 
>   src/master/constants.cpp e316f9772d880b694faeee6d001dc56bc088c118 
> 
> Diff: https://reviews.apache.org/r/44523/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42705: Update docs for --weights flag and authorization.

[Yongqiao Wang]

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

(Updated March 9, 2016, 4:09 a.m.)


Review request for mesos and Adam B.


Changes
---

Addressed comments of Adam.


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


Repository: mesos


Description
---

Update docs for --weights flag and authorization.


Diffs (updated)
-

  docs/authorization.md c4449d5da4e2ea3ebfaaf4ed5b682e8f7bf56ac3 
  docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
  src/master/flags.cpp be981ed6155edce18bdb55188c78d73182159418 

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


Testing
---


Thanks,

Yongqiao Wang

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Alexander Rojas]

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


Fix it, then Ship it!





src/slave/constants.cpp (line 58)


Can you create a JIRA issue so we make the naming of the realm symetric in 
the master to _mesos-master_. I think only the master's 
`DEFAULT_HTTP_AUTHENTICATION_REALM` should be changed.


- Alexander Rojas


On March 8, 2016, 7:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 8, 2016, 7:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test. However, these flags aren't yet incorporated 
> into any agent endpoints. We should probably wait to merge this patch along 
> with the changes for https://issues.apache.org/jira/browse/MESOS-4850 when 
> they're ready.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44467: Implemented local puller shell command test.

[Guangya Liu]

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




src/tests/containerizer/provisioner_docker_tests.cpp (lines 340 - 343)


Does the `flags.docker_store_dir` needs to be a temp value? If default, 
then all images will be persisted at same directory and some test cases may 
re-use the cached image.


- Guangya Liu


On 三月 7, 2016, 9:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44467/
> ---
> 
> (Updated 三月 7, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4813
> https://issues.apache.org/jira/browse/MESOS-4813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented local puller shell command test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
> 
> Diff: https://reviews.apache.org/r/44467/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43824: Addressed comments of 41672.

[Yongqiao Wang]

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

(Updated March 9, 2016, 3:17 a.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Addressed the comments of Adam.


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


Repository: mesos


Description
---

Addressed comments of 41672.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

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


Testing
---

make && make check successfully.


Thanks,

Yongqiao Wang

Re: Review Request 43824: Addressed comments of 41672.

[Yongqiao Wang]

> On March 8, 2016, 11:20 p.m., Adam B wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 233
> > 
> >
> > `getAllocations` is too general a name for what this function actually 
> > does. More like `handleTwoAllocationsAndRecoverResources`. Any method that 
> > starts with `get` should return something, preferably whatever comes after 
> > `get` (i.e. `Allocations`).
> > If you parameterize the number of iterations of the loop, add a 
> > boolean/enum for whether to recover the resources, and return 
> > `frameworkAllocations`, then you could call it `getAllocations`, and use it 
> > for the 3-iteration loop at the end of the test.
> > That said, only two reuses may not be worth pulling it out (although 3 
> > may be).
> > I give you 3 options (your choice):
> > 1. Rename the method appropriately (may require code changes so it's 
> > not as ugly as `handleTwoAllocationsAndRecoverResources`).
> > 2. Inline the method back into the test in both places.
> > 3. Generalize the method and reuse it for the 3rd instance.

Thanks very much for your detailed description.


- Yongqiao


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


On March 8, 2016, 12:10 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 8, 2016, 12:10 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44467: Implemented local puller shell command test.

[Guangya Liu]

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




src/tests/containerizer/provisioner_docker_tests.cpp (line 335)


This does not affect much but most test cases are now using `alpine` 
instead, it is better udate the image ame to `alpine` to keep consistent.

ditto for the following.


- Guangya Liu


On 三月 7, 2016, 9:46 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44467/
> ---
> 
> (Updated 三月 7, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4813
> https://issues.apache.org/jira/browse/MESOS-4813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented local puller shell command test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
> 
> Diff: https://reviews.apache.org/r/44467/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 44549: Introduced a protobuf message "NetworkConfig".

[Qian Zhang]

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

Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Introduced a protobuf message "NetworkConfig".


Diffs
-

  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/slave/containerizer/mesos/isolators/network/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/spec.proto PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Guangya Liu]

> On 三月 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.

I was using 1.10.0, and the default value of net is `--net=default` and it also 
works if I run the command `docker run -it --net default ubuntu:14.04 /bin/bash`


- Guangya


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


On 三月 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated 三月 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44456: Added Appc provisioner integration test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44533, 44534, 44299, 44455, 44456]

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

- Mesos ReviewBot


On March 8, 2016, 10:50 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44456/
> ---
> 
> (Updated March 8, 2016, 10:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4818
> https://issues.apache.org/jira/browse/MESOS-4818
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Appc provisioner integration test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 6c8087e17aa8b7139ba12337d5be472b7099e77f 
> 
> Diff: https://reviews.apache.org/r/44456/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Travis Hegner]

> On March 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.

I've found this to be true with at least docker 1.10.2.

Is it possible to launch a container with "--net default" (or ommitted) and 
have it not be a "bridge" in general with docker? I haven't determined that 
yet. If not, then we could statically test for "default" and set the string to 
"bridge" every time. Another thing to consider, if mesos /always/ launches a 
container with an explicit "--net bridge" or "--net host" parameter (until UDNs 
are implemented anyway), then we aren't really in danger of exposing the 
behavior described in this issue. Launching the container with an explicit 
"--net bridge" sets the NetworkMode field to "bridge" as well.


- Travis


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Guangya Liu]

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




src/docker/docker.cpp (lines 306 - 307)


I found that the NetworkMode is `default` when using `bridge` mode, so the 
value of `networkModeValue` would be `default` but not `bridge`, and this will 
cause line 315 constrcut a invalid addressLocation as 
`NetworkSettings.Networks.default.IPAddress` which will always failed and thus 
fall to old API.


- Guangya Liu


On 三月 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated 三月 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44439: Added device support in cgroups abstraction.

[Ben Mahler]

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




src/linux/cgroups.hpp (lines 628 - 633)


Hm.. we probably do not want to add this, because there is already a 
`cgroups::create` and the caller is free to compose `cgroups::create` along 
with additional calls to `cgroups::devices::*` as appropriate.



src/linux/cgroups.hpp (lines 635 - 654)


The naming convention in this file is to mirror the cgroups controls, so 
these would be:

```
cgroups::devices::allow(...)
cgroups::devices::deny(...)
cgroups::devices::list(...)
```

The other reason we've added these helpers is to provide better types to 
make the code more readable, notice how the memory controls and cpu controls in 
this file use Bytes and Duration, respectively.

Here, I think we need to do something a bit more involved. To be more 
specific, if I read the caller code:

```
cgroups::devices::allow(
hierarchy,
cgroup, 
"c 1:3 rm");
```

It's not easy for the reader to understand what `"c 1:3 rm"` means. So 
adding types here would help, for example:

```
// This is /dev/null.
cgroups::device::Selector selector;
devices.type = CHARACTER;
devices.major = 1;
devices.minor = ALL;

cgroups::device::Access access;
access.read = true;
access.write = true;
access.mknod = true;

Try allow = cgroups::devices::allow(
hierarchy,
cgroup,
selector,
access);
```

For cgroups::devices::list, Kevin and I played around and weren't able to 
see the contents of the file change at all, did you have any luck getting 
different results out of the devices.list file?



src/tests/containerizer/cgroups_tests.cpp (lines 1278 - 1309)


Does this test pass? Did you make sure this test ran? Note that you have to 
run the tests under 'sudo' on Linux for this test to run because it has the 
ROOT and CGROUPS strings in the test name.


- Ben Mahler


On March 7, 2016, 6:27 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44439/
> ---
> 
> (Updated March 7, 2016, 6:27 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Felix Abecassis, Kevin Klues, and 
> Niklas Nielsen.
> 
> 
> Bugs: MESOS-3368
> https://issues.apache.org/jira/browse/MESOS-3368
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There are some helper methods added to support device cgroups in cgroups 
> abstraction to aid isolators controlling access to devices.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 51ccefd1fd2d0989b9bd31342d3d1e3701f88ed2 
>   src/linux/cgroups.cpp df18ed46a2a96871f67c7eb4233c3b4c27b7aa1c 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
> 
> Diff: https://reviews.apache.org/r/44439/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

> On March 3, 2016, 10:15 p.m., Joseph Wu wrote:
> > src/master/contender.hpp, lines 17-18
> > 
> >
> > This whole file seems like a pretty substantial change.  I'd recommend 
> > pulling it out into a separate review (rather than hiding it in this 
> > review).
> > 
> > Also, you'll want to consider making folders "contenders" and 
> > "detectors".  Then renaming this file "standalone.hpp".
> 
> Anurag Singh wrote:
> Ok. Although then zookeeper's classes should also go into their own files.
> 
> Anurag Singh wrote:
> Looking at this again, the changes in this file are really just namespace 
> changes (the only thing substantial is the removal of the MasterContender 
> class definition) - I can't put them into a different commit without breaking 
> the build (I'm trying to make sure individual commits don't break builds, 
> which I think is a sensible goal). However, I can leave this commit as is but 
> create a separate commit to create the contenders/detectors directories. Is 
> that acceptable to you?
> 
> Anurag Singh wrote:
> Ping ... will the suggestion I made work for you?
> 
> Joseph Wu wrote:
> Don't worry about having atomic commits (especially for verbose changes).
> 
> There are a couple of changes to consider here, each of which might 
> deserve its own review:
> - Pulling out the interface deletions.  (You also consider pulling it 
> into the previous review, as you're really moving the code from private to 
> public headers.)
> - Adding a folder for each module you are adding ("contender" and 
> "detector").
> - Breaking apart the Standalone vs Zookeeper logic.
> 
> Anurag Singh wrote:
> While separating the zookeeper and standalone detectors and moving them 
> into detectors, I realized that the functions in the 'promises' namespace 
> will be duplicated unless I move them into their own header file. I can 
> either put them in include/mesos and change the namespace to mesos 
> mesos::internal::promises, or just  leave the header file in detectors. Which 
> one is preferable? bmahler had a todo for doing this  so I'm not sure if your 
> team already has some work planned in this area.
> 
> Joseph Wu wrote:
> I would *tentatively* recommend putting those helpers inside 
> `3rdparty/libprocess/include/process/collect.hpp`, where we have a bunch of 
> other helpers that operate on groups of `Futures`.  We definitely want those 
> helpers in a libprocess header somewhere, but I'm not sure what the most 
> acceptable place would be.  (I'll ask BenM about it.)

The logic separation change has turned into a major change - if you see a lot 
of issues with the commits, we should spin the separation change into an issue 
by itself (I can work on that if needed). It will allow this review to proceed 
faster - unfortunately we're blocked on our module changes until this review 
completes.


- Anurag


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


On March 9, 2016, 1:35 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 9, 2016, 1:35 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp b010a819132fb80810e7f8ce96778109f2e8b35e 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> 

Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

[Guangya Liu]

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

(Updated 三月 9, 2016, 1:47 a.m.)


Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

There is a bug when setting host maintain with http endpoint: 
https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
The logic is as this:
1) Get all host list from maintain window and put it to updated hashmap.
2) If the machine in was in updated was also in master->machines, call master 
updateUnavailability to trigger recoverResources, updateUnavailability etc in 
allocator
3) Otherwise, clear the unavailabity time window for the machine.
4) Update each new machines in updated to call master updateUnavailability

But the logic in step 4) is getting all machines from the schedule windows but 
not the machines that is new to the cluster, this caused master get two 
updateUnavailability calls for a machine in the updated hashmap.

The fix is filter machines in updated hashmap when handling new machines.


Diffs (updated)
-

  src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 

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


Testing
---

make
make check
 ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose


Thanks,

Guangya Liu

Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

[fan du]

> On March 7, 2016, 10:43 p.m., Jie Yu wrote:
> > src/master/master.cpp, line 2846
> > 
> >
> > I looked weird to me that we increase the metrics for reserve resources 
> > in 'authorizeXXX' function. Can you do that in the callers of authorizeXXX 
> > function instead?

Thanks for the reviewing :)
It intended to reduce code duplication, do the statistics counting here in the 
common path from both http endpoint and normal framework RECEIVE message. If 
you insist, I'm willing to do it seprately.


- fan


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


On March 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 3, 2016, 5:40 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4492
> https://issues.apache.org/jira/browse/MESOS-4492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
> 
> Diff: https://reviews.apache.org/r/44255/diff/
> 
> 
> Testing
> ---
> 
> ChangLog:
> v2:
>   - Documenting those metrics
>   - Add test code for MetricsTest as suggested by Guangya
>   - post-review.py does not update original 
> RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
> rebased.
> 
> 
> Tests:
> 1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
> (3.10.0-123.el7.x86_640)
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MetricsTest
> [ RUN  ] MetricsTest.Master
> [   OK ] MetricsTest.Master (211 ms)
> [--] 1 test from MetricsTest (211 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (244 ms total)
> [  PASSED  ] 1 test
> 
> 2. Verify its functionality with 'reserve' http endpoint as an test case
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 0.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> # curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ 
> { "name": "cpus", "type": "SCALAR","scalar": { "value": 1 
> },"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
> ipdc02-kvm-guest2:5050/master/reserve
> HTTP/1.1 200 OK
> Date: Fri, 26 Feb 2016 19:59:01 GMT
> Content-Length: 0
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 1.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

> On March 3, 2016, 10:15 p.m., Joseph Wu wrote:
> > include/mesos/scheduler.hpp, lines 44-49
> > 
> >
> > A forward declaration isn't required anymore.  You can just include the 
> > header(s) from the previous review.

I tried doing this and the build failed because the glog/logging.h header was 
not found. I did not debug this further. The failure has nothing to do with my 
change, however. Even if I try including future.hpp (which in turn includes 
glog/logging.h) here, the build breaks.


- Anurag


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


On March 9, 2016, 1:35 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 9, 2016, 1:35 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp b010a819132fb80810e7f8ce96778109f2e8b35e 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> d193897e636efd0e3ef67bf67fcd6255a3de0341 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
>   src/tests/scheduler_event_call_tests.cpp 
> 8c02ceeb3ec1783cb2f63f100700508e70f586e4 
>   src/tests/scheduler_http_api_tests.cpp 
> dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
>   src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
>   src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
> 
> Diff: https://reviews.apache.org/r/44288/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>

Re: Review Request 44289: Added support for contender and detector modules.

[Anurag Singh]

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

(Updated March 9, 2016, 1:39 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added support for contender and detector modules.


Diffs (updated)
-

  include/mesos/module/contender.hpp PRE-CREATION 
  include/mesos/module/detector.hpp PRE-CREATION 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/examples/test_contender_module.cpp PRE-CREATION 
  src/examples/test_detector_module.cpp PRE-CREATION 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/contenders/zookeeper.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detectors/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/module/manager.cpp 6ae99504005581b22a44768949b1d305cec517d9 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 

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


Testing
---

In addition to all unit tests passing, we are currently using this 
functionality in our environment with a custom consensus stack. In our world, 
we have a C++ plugin that calls out to an HTTP REST service (implemented in 
Java/Scala, not that it matters).


Thanks,

Anurag Singh

Review Request 44546: Moved functions in promises to a common header file.

[Anurag Singh]

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

Review request for mesos.


Repository: mesos


Description
---

Moved functions in promises to a common header file.


Diffs
-

  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/detectors/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 

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


Testing
---


Thanks,

Anurag Singh

Review Request 44547: Added functions in promises to the collect header.

[Anurag Singh]

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

Review request for mesos.


Repository: mesos


Description
---

Added functions in promises to the collect header.


Diffs
-

  3rdparty/libprocess/include/process/collect.hpp 
5a92b72eb7668494dc832ec446a41b3d673a20cc 

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


Testing
---


Thanks,

Anurag Singh

Review Request 44545: Separated standalone and zookeeper classes.

[Anurag Singh]

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

Review request for mesos.


Repository: mesos


Description
---

Instead of keeping standalone and zookeper contender/detector class
definitions and implementations in the same file, separated them. Also
made the necessary changes in users of class headers to point to the
new locations.


Diffs
-

  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/contenders/standalone.cpp PRE-CREATION 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 
  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/isolator_tests.cpp 
342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
  src/tests/fault_tolerance_tests.cpp d193897e636efd0e3ef67bf67fcd6255a3de0341 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 
  src/tests/mesos.hpp 9c62833e0a64cfd62fce8cffd04f9cdd933646c8 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing
---


Thanks,

Anurag Singh

Review Request 44544: Moved contender and detector definitions into separate directories.

[Anurag Singh]

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

Review request for mesos.


Repository: mesos


Description
---

Moved contender and detector definitions into separate directories.


Diffs
-

  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing
---


Thanks,

Anurag Singh

Review Request 44543: Removed unnecessary MasterContender and MasterDetector definitions.

[Anurag Singh]

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

Review request for mesos.


Repository: mesos


Description
---

MasterContender and MasterDetector are now defined in
include/mesos/master/contender.hpp and detector.hpp.


Diffs
-

  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 

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


Testing
---


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

[Anurag Singh]

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

(Updated March 9, 2016, 1:34 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

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

(Updated March 9, 2016, 1:35 a.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Also modified users of MasterContender and MasterDetector to use this
namespace.


Diffs (updated)
-

  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
  src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp b010a819132fb80810e7f8ce96778109f2e8b35e 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
  src/tests/fault_tolerance_tests.cpp d193897e636efd0e3ef67bf67fcd6255a3de0341 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh

Re: Review Request 44366: Added GPUs as an explicit resource.

[Ben Mahler]

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



Could you update the description to reflect the new state of the code?


src/slave/containerizer/containerizer.cpp (line 96)


Can we also prevent fractional GPUs here?

We'd also need to do fractional validation in the launch task path, inside 
`update` (as we discussed `prepare` is calling update so no need to stick it in 
both functions).



src/slave/containerizer/containerizer.cpp (lines 97 - 102)


Since it's unlikely we'll have a default number of GPUs in the future, 
perhaps we just have a comment here that describes why we don't use a default?



src/slave/containerizer/containerizer.cpp (line 114)


Needs to be in the ifdef, right?



src/slave/containerizer/containerizer.cpp (lines 114 - 135)


Can you remove the periods at the end of the error messages?



src/slave/containerizer/containerizer.cpp (lines 115 - 118)


Our pattern for error messages is to avoid the periods and multi-sentence 
messages. We'll try to write messages that can compose as follows:

Failed to : 

Where reason can also correspond to a composed message, so you can end up 
with things like:

Failed to launch container: Failed to prepare container: Fractional 'gpus' 
not supported.

So when we've needed multi-sentence messages, we've tended to use 
semi-colons.



src/slave/containerizer/containerizer.cpp (line 118)


These should be plural, so 'gpus'



src/slave/containerizer/containerizer.cpp (lines 121 - 130)


We can get rid of the parsing code here based on the suggestion in the 
previous review to add to common/parse.hpp.

Note that we would usually use const& in the loop here.



src/v1/resources.cpp (line 1238)


Don't feel obligated, but would be nice to do a blanket s/`.get().`/`->`/ 
cleanup in the resources / values files, separate from this patch.


- Ben Mahler


On March 8, 2016, 10:47 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44366/
> ---
> 
> (Updated March 8, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4865
> https://issues.apache.org/jira/browse/MESOS-4865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we can either infer the number of GPUs to offer as resources
> from the --nvidia_gpus flag, or pass the amount explicity as a "gpus"
> resource. In the future, we will generalize this via autodiscovery of
> gpus and support for gpus types other than Nvidia.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 388c4f383b77be490cb2debb64a734c6d6e4a176 
>   include/mesos/v1/resources.hpp 64ad8bf08230aeaa173325364c91b765f091210e 
>   src/common/resources.cpp cf0707209143084d86aff5e4427846f461479c54 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/containerizer/containerizer.cpp 
> f6fc7863d0c215611f170dc0c89aa229407b5137 
>   src/v1/resources.cpp 296c4c2392384a2fcc4f2c1843980ff97e71114d 
> 
> Diff: https://reviews.apache.org/r/44366/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44365: Added flag to specify available Nvidia GPUs on an agent's command line.

[Ben Mahler]

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




src/slave/flags.hpp (line 92)


Can you put this in an ifdef?



src/slave/flags.cpp (lines 597 - 607)


-s/nvidia_gpus/nvidia_gpu_devices/
-capitalize NVML
-s/as a resource/as resources/
-maybe put the nvidia-smi comment in a parenthetical?
-The third sentence seems to be already expressed in the first? Perhaps 
clarify that the isolator will be used only if the `--isolation` flag contains 
the right value.
-We can probably omit or re-pharse the last sentence since this flag is 
omitted from the program if the configure flag is not present.



src/slave/flags.cpp (lines 606 - 607)


I was going to suggest we add a validator here, but even better would be to 
add a parse specialization to src/common/parse.hpp and just avoid the need for 
the extra validation altogether.


- Ben Mahler


On March 8, 2016, 10:47 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44365/
> ---
> 
> (Updated March 8, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4864
> https://issues.apache.org/jira/browse/MESOS-4864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added flag to specify available Nvidia GPUs on an agent's command line.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44365/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Dan Osborne]

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


Ship it!




Ship It!

- Dan Osborne


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44363: Added stubs for the Nvidia GPU device isolator.

[Vikrama Ditya]

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


Ship it!




Ship It!

- Vikrama Ditya


On March 8, 2016, 10:46 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44363/
> ---
> 
> (Updated March 8, 2016, 10:46 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4623
> https://issues.apache.org/jira/browse/MESOS-4623
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the first isolator to fall under the category of cgroup
> devices. However, we do not yet have a generic cgroup device isolator
> (nor will we in the very near future). As such, I have preemptively
> created the nvidia gpu isolator in the directory hierarchy under the
> path:
> 
> src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/
> 
> in order to easily allow them to fall under the category of gpu
> devices later on.
> 
> In this stub implementation, initialization of the agent will
> fail if the nvidia gpu isolator is enabled via the agent --isolation
> flag. That is --isolation="cgroups/devices/gpu/nvidia". In a
> subsequent commit we will fill in the guts to actually enable the
> proper isolation.
> 
> The flags documentation has been udpated accordingly.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
>   src/Makefile.am 5b54fe04734afc5faa1bba3b84f43a1b8efe8753 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> PRE-CREATION 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44363/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44523: Changed the master's default HTTP authentication realm.

[Adam B]

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




src/master/constants.cpp (lines 52 - 53)


Would the same hold true for any machine that runs both a master and an 
agent process?
Or is it only when master and agent are inside the same linux process?


- Adam B


On March 8, 2016, 11:01 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44523/
> ---
> 
> (Updated March 8, 2016, 11:01 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed the master's default HTTP authentication realm.
> 
> 
> Diffs
> -
> 
>   src/master/constants.cpp e316f9772d880b694faeee6d001dc56bc088c118 
> 
> Diff: https://reviews.apache.org/r/44523/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44513: Added missing flag to authentication docs.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 8, 2016, 7:15 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44513/
> ---
> 
> (Updated March 8, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added missing flag to authentication docs.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 2de0eedd75e6f34549351c0ace4fee9aba7f2fd1 
> 
> Diff: https://reviews.apache.org/r/44513/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42705: Update docs for --weights flag and authorization.

[Adam B]

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



Need to update the --weights flags help message in flags.cpp too.
(This patch also needs a minor rebase)


docs/configuration.md (lines 761 - 763)


These same changes need to be made in `src/master/flags.cpp` so they show 
up in command-line help.


- Adam B


On Feb. 13, 2016, 7:01 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42705/
> ---
> 
> (Updated Feb. 13, 2016, 7:01 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update docs for --weights flag and authorization.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md bbb4f2adc9348cb1686e6af78f5604d8cf7651ab 
>   docs/configuration.md eea985cc251d6edd8476ade174b10a58aebefab1 
> 
> Diff: https://reviews.apache.org/r/42705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

[Anand Mazumdar]

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




src/tests/master_maintenance_tests.cpp (line 481)


hmmm .. shouldn't we do a `Clock::pause/settle` here to ensure that the 
master does not send another inverse offer to us i.e. the original issue after 
L481? 

Something like this:

```cpp
event = events.get();

Clock::pause();
Clock::settle();

ASSERT_TRUE(event.isPending());
```

What do you think?


- Anand Mazumdar


On March 5, 2016, 1:23 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44258/
> ---
> 
> (Updated March 5, 2016, 1:23 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4831
> https://issues.apache.org/jira/browse/MESOS-4831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is a bug when setting host maintain with http endpoint: 
> https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
> The logic is as this:
> 1) Get all host list from maintain window and put it to updated hashmap.
> 2) If the machine in was in updated was also in master->machines, call master 
> updateUnavailability to trigger recoverResources, updateUnavailability etc in 
> allocator
> 3) Otherwise, clear the unavailabity time window for the machine.
> 4) Update each new machines in updated to call master updateUnavailability
> 
> But the logic in step 4) is getting all machines from the schedule windows 
> but not the machines that is new to the cluster, this caused master get two 
> updateUnavailability calls for a machine in the updated hashmap.
> 
> The fix is filter machines in updated hashmap when handling new machines.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8276baa538eb4d2aaf54cc1aa516bffaadacc4dd 
>   src/tests/master_maintenance_tests.cpp 
> 3faa8136cf57276295553910319480028f433e4c 
> 
> Diff: https://reviews.apache.org/r/44258/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
>  ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44299: Added unit test for file URI fetcher.

[Jojy Varghese]

> On March 8, 2016, 1:57 a.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 590-591
> > 
> >
> > Looking at the original code, why do we need this id? Should that 
> > always be the same? Why do we need to pass it in?

moved the image id up to `prepareImage` method.


- Jojy


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


On March 8, 2016, 10:49 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44299/
> ---
> 
> (Updated March 8, 2016, 10:49 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit test for file URI fetcher.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 6c8087e17aa8b7139ba12337d5be472b7099e77f 
> 
> Diff: https://reviews.apache.org/r/44299/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44364: Added infrastructure for Nvidia GPU specific tests.

[Ben Mahler]

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




src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (lines 30 - 32)


How about we have a DISABLED test that just starts and stops an agent with 
the gpu isolation flags?


- Ben Mahler


On March 8, 2016, 10:47 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44364/
> ---
> 
> (Updated March 8, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4863
> https://issues.apache.org/jira/browse/MESOS-4863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added infrastructure for Nvidia GPU specific tests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5b54fe04734afc5faa1bba3b84f43a1b8efe8753 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44364/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 43824: Addressed comments of 41672.

[Adam B]

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




src/tests/hierarchical_allocator_tests.cpp (line 233)


`getAllocations` is too general a name for what this function actually 
does. More like `handleTwoAllocationsAndRecoverResources`. Any method that 
starts with `get` should return something, preferably whatever comes after 
`get` (i.e. `Allocations`).
If you parameterize the number of iterations of the loop, add a 
boolean/enum for whether to recover the resources, and return 
`frameworkAllocations`, then you could call it `getAllocations`, and use it for 
the 3-iteration loop at the end of the test.
That said, only two reuses may not be worth pulling it out (although 3 may 
be).
I give you 3 options (your choice):
1. Rename the method appropriately (may require code changes so it's not as 
ugly as `handleTwoAllocationsAndRecoverResources`).
2. Inline the method back into the test in both places.
3. Generalize the method and reuse it for the 3rd instance.


- Adam B


On March 8, 2016, 4:10 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 8, 2016, 4:10 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44514: Implemented NetworkCniIsolatorProcess::prepare().

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44004, 44200, 44269, 44514]

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

- Mesos ReviewBot


On March 8, 2016, 4:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 8, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented NetworkCniIsolatorProcess::prepare().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44514/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 43920: Added a helper function to stout : os/which.hpp.

[Joseph Wu]

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



Looks good.  Before I give a "Ship It!", your next steps should be:

- Add a unit test.
- Modify the following classes in src/tests/environment.cpp:
  - CurlFilter
  - LogrotateFilter
  - NetcatFilter
  - PerfCPUCyclesFilter
  - PerfFilter
- Modify PortMappingIsolatorTest::SetUpTestCase in 
src/tests/containerizer/port_mapping_tests.cpp
- Modify the `sha512` command in src/common/command_utils.cpp
- Modify the `logrotate_path` flag validation logic in 
src/slave/container_loggers/lib_logrotate.hpp and logrotate.hpp


3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp (line 25)


Nit: s/os{/os {/



3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp (lines 27 - 28)


Suggestion:

Searches for an executable `command` on the environment variable `PATH`.  
If found, returns the absolute path to the command.  Otherwise, returns `None`.



3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp (line 51)


Nit: no indent here.



3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp (line 52)


Nit: add one newline preceding this line.


- Joseph Wu


On March 8, 2016, 6:22 a.m., Disha  Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43920/
> ---
> 
> (Updated March 8, 2016, 6:22 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper function to stout : os/which.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43920/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Disha  Singh
> 
>

Re: Review Request 44456: Added Appc provisioner integration test.

[Jojy Varghese]

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

(Updated March 8, 2016, 10:50 p.m.)


Review request for mesos and Jie Yu.


Changes
---

rebased.


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


Repository: mesos


Description (updated)
---

Added Appc provisioner integration test.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
6c8087e17aa8b7139ba12337d5be472b7099e77f 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44299: Added unit test for file URI fetcher.

[Jojy Varghese]

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

(Updated March 8, 2016, 10:49 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added unit test for file URI fetcher.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
6c8087e17aa8b7139ba12337d5be472b7099e77f 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 44534: Refactored AppcImageFetcherTest.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Changes:
  - Changed TestAppcImageServer mock methods to real implementation that
  serves http requests. This will allow the TestAppcImageServer to serve
  multiple images in a test.
  - Made TestAppcImageServer loadable so that it can serve images from its
  'images' directory (currently defaulted to 'server').
  - Renamed prepareServerImage to prepareImage to reflect the semantics.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
6c8087e17aa8b7139ba12337d5be472b7099e77f 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44366: Added GPUs as an explicit resource.

[Vikrama Ditya]

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




src/slave/containerizer/containerizer.cpp (line 107)


It will be good to comment that --nvidia-gpus flag consists of devices 
numbers as comes out from nvidia-smi output. Good to give example here.


- Vikrama Ditya


On March 4, 2016, 1:11 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44366/
> ---
> 
> (Updated March 4, 2016, 1:11 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Rob Todd.
> 
> 
> Bugs: MESOS-4865
> https://issues.apache.org/jira/browse/MESOS-4865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we can either infer the number of GPUs to offer as resources
> from the --nvidia_gpus flag, or pass the amount explicity as a "gpus"
> resource. In the future, we will generalize this via autodiscovery of
> gpus and support for gpus types other than Nvidia.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 388c4f383b77be490cb2debb64a734c6d6e4a176 
>   include/mesos/v1/resources.hpp 64ad8bf08230aeaa173325364c91b765f091210e 
>   src/common/resources.cpp cf0707209143084d86aff5e4427846f461479c54 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
>   src/slave/containerizer/containerizer.cpp 
> f6fc7863d0c215611f170dc0c89aa229407b5137 
>   src/v1/resources.cpp 296c4c2392384a2fcc4f2c1843980ff97e71114d 
> 
> Diff: https://reviews.apache.org/r/44366/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 44533: Fixed AppcStoreTest fixture to remove imageId from test image.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed AppcStoreTest fixture to remove imageId from test image.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
6c8087e17aa8b7139ba12337d5be472b7099e77f 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44455: Fixed AppcImageFetcherTest for manifest formatting.

[Jojy Varghese]

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

(Updated March 8, 2016, 10:45 p.m.)


Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed AppcImageFetcherTest for manifest formatting.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
6c8087e17aa8b7139ba12337d5be472b7099e77f 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On March 4, 2016, 5:23 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44258/
> ---
> 
> (Updated March 4, 2016, 5:23 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4831
> https://issues.apache.org/jira/browse/MESOS-4831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is a bug when setting host maintain with http endpoint: 
> https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
> The logic is as this:
> 1) Get all host list from maintain window and put it to updated hashmap.
> 2) If the machine in was in updated was also in master->machines, call master 
> updateUnavailability to trigger recoverResources, updateUnavailability etc in 
> allocator
> 3) Otherwise, clear the unavailabity time window for the machine.
> 4) Update each new machines in updated to call master updateUnavailability
> 
> But the logic in step 4) is getting all machines from the schedule windows 
> but not the machines that is new to the cluster, this caused master get two 
> updateUnavailability calls for a machine in the updated hashmap.
> 
> The fix is filter machines in updated hashmap when handling new machines.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8276baa538eb4d2aaf54cc1aa516bffaadacc4dd 
>   src/tests/master_maintenance_tests.cpp 
> 3faa8136cf57276295553910319480028f433e4c 
> 
> Diff: https://reviews.apache.org/r/44258/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
>  ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Joseph Wu]

> On March 3, 2016, 2:15 p.m., Joseph Wu wrote:
> > src/master/contender.hpp, lines 17-18
> > 
> >
> > This whole file seems like a pretty substantial change.  I'd recommend 
> > pulling it out into a separate review (rather than hiding it in this 
> > review).
> > 
> > Also, you'll want to consider making folders "contenders" and 
> > "detectors".  Then renaming this file "standalone.hpp".
> 
> Anurag Singh wrote:
> Ok. Although then zookeeper's classes should also go into their own files.
> 
> Anurag Singh wrote:
> Looking at this again, the changes in this file are really just namespace 
> changes (the only thing substantial is the removal of the MasterContender 
> class definition) - I can't put them into a different commit without breaking 
> the build (I'm trying to make sure individual commits don't break builds, 
> which I think is a sensible goal). However, I can leave this commit as is but 
> create a separate commit to create the contenders/detectors directories. Is 
> that acceptable to you?
> 
> Anurag Singh wrote:
> Ping ... will the suggestion I made work for you?
> 
> Joseph Wu wrote:
> Don't worry about having atomic commits (especially for verbose changes).
> 
> There are a couple of changes to consider here, each of which might 
> deserve its own review:
> - Pulling out the interface deletions.  (You also consider pulling it 
> into the previous review, as you're really moving the code from private to 
> public headers.)
> - Adding a folder for each module you are adding ("contender" and 
> "detector").
> - Breaking apart the Standalone vs Zookeeper logic.
> 
> Anurag Singh wrote:
> While separating the zookeeper and standalone detectors and moving them 
> into detectors, I realized that the functions in the 'promises' namespace 
> will be duplicated unless I move them into their own header file. I can 
> either put them in include/mesos and change the namespace to mesos 
> mesos::internal::promises, or just  leave the header file in detectors. Which 
> one is preferable? bmahler had a todo for doing this  so I'm not sure if your 
> team already has some work planned in this area.

I would *tentatively* recommend putting those helpers inside 
`3rdparty/libprocess/include/process/collect.hpp`, where we have a bunch of 
other helpers that operate on groups of `Futures`.  We definitely want those 
helpers in a libprocess header somewhere, but I'm not sure what the most 
acceptable place would be.  (I'll ask BenM about it.)


- Joseph


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


On March 3, 2016, 9:30 a.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 3, 2016, 9:30 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Joseph Wu]

> On March 8, 2016, 3:50 a.m., Bernd Mathiske wrote:
> > src/tests/scheduler_event_call_tests.cpp, line 367
> > 
> >
> > In most other places you have the blank line before the detector.

True.  Fixed four places in:
- scheduler_driver_tests.cpp
- scheduler_event_call_tests.cpp
- slave_tests.cpp


> On March 8, 2016, 3:50 a.m., Bernd Mathiske wrote:
> > src/tests/slave_tests.cpp, line 180
> > 
> >
> > With this line you have now introduced a 3rd way to write this stretch 
> > of code. Please pick one. This one is probably the least controversial. :-)

I see the following patterns:
```
  Owned detector = master.get()->detector();

  Try slave = StartSlave(detector.get(), ...);
  ASSERT_SOME(slave);
```
---
```
  // There may or may not be a comment here. i.e. "Start a slave."
  Owned detector = master.get()->detector();
  Try slave = StartSlave(detector.get(), ...);
  ASSERT_SOME(slave);
```
---
```
  slave::Flags flags = CreateSlaveFlags();
  Owned detector = master.get()->detector();

  Try slave = StartSlave(detector.get(), ...);
  ASSERT_SOME(slave);
```

I'll standardize to:
```
  // Maybe: slave::Flags flags = CreateSlaveFlags();

  Owned detector = master.get()->detector();
  
  // Maybe a comment here.
  Try slave = StartSlave(detector.get(), ...);
  ASSERT_SOME(slave);
```


- Joseph


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


On March 8, 2016, 2:33 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated March 8, 2016, 2:33 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Owned`.  And 
> `Try` with `Owned`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 00f4129e46aa9268fbb66da25b34e61004fa87b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6aecd912fc84b72d2b64f7a842891fddcbc469ac 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
>   src/tests/containerizer/isolator_tests.cpp 
> 342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 03879d99c371f296f8d9904666911b34209c114d 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 983a6be160aefe5a32acb6111bb3c85230ec 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 2fc0893f5f5e80a783296fb31b30abe86d92df1b 
>   src/tests/fault_tolerance_tests.cpp 
> d193897e636efd0e3ef67bf67fcd6255a3de0341 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp d0fd27fd8a6b48511ef8cafab5dff59f65729d9f 
>   src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_quota_tests.cpp 

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Joseph Wu]

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

(Updated March 8, 2016, 2:33 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Rebased and addressed conflicts in FaultToleranceTests, HealthCheckTests, and 
MasterTests.

Also made some whitespace changes to standardize the appearance of the 
MasterDetector in tests.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Includes the following changes:

* Added the `` header where appropriate.
* Added the namespace `using process::Owned;` where appropriate.
* Generally replaced `Try` with `Owned`.  And 
`Try` with `Owned`.
* Added the (now required) `MasterDetector` argument to all slaves.  Before, 
this was fetched from the first master in `Cluster`.
* Removed `Shutdown();` from all tests.
* Replaced `Stop(...)` with the appropriate master/slave destruction calls.
* Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
launchers, etc).
* Replace `CHECK` in tests with `ASSERT`.


Diffs (updated)
-

  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/command_executor_tests.cpp 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
  src/tests/container_logger_tests.cpp 00f4129e46aa9268fbb66da25b34e61004fa87b2 
  src/tests/containerizer/docker_containerizer_tests.cpp 
6aecd912fc84b72d2b64f7a842891fddcbc469ac 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
e72239a55724f1aeeec5362cc370c93dbeca7164 
  src/tests/containerizer/isolator_tests.cpp 
342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
  src/tests/containerizer/memory_pressure_tests.cpp 
03879d99c371f296f8d9904666911b34209c114d 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
  src/tests/containerizer/port_mapping_tests.cpp 
983a6be160aefe5a32acb6111bb3c85230ec 
  src/tests/containerizer/provisioner_docker_tests.cpp 
5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
  src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
  src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
  src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
  src/tests/executor_http_api_tests.cpp 
2fc0893f5f5e80a783296fb31b30abe86d92df1b 
  src/tests/fault_tolerance_tests.cpp d193897e636efd0e3ef67bf67fcd6255a3de0341 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/health_check_tests.cpp d0fd27fd8a6b48511ef8cafab5dff59f65729d9f 
  src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_quota_tests.cpp 4fabc1473ec3e048afe7171abbb8d6e49e863847 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 
  src/tests/master_validation_tests.cpp 
c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
  src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
  src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_endpoints_tests.cpp 
81185a161498394020a27f1f5bf747bac5425f43 
  src/tests/persistent_volume_tests.cpp 
bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
  src/tests/rate_limiting_tests.cpp 352a5e7a58b500c25c7c8a421047c4e79434e38e 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/registrar_zookeeper_tests.cpp 
3df9779ee5d076e16f6a538326693a36f986b6d0 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/reservation_endpoints_tests.cpp 
f95ae7a32c3809d150adf1e9e515a3b527e61699 
  src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/scheduler_driver_tests.cpp f6dc25d82ae5f1e77fc6ede7ff2660ed0d9ea039 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 
  src/tests/teardown_tests.cpp 

Re: Review Request 43630: Especially updated scheduler tests to use the updated MesosTest helpers.

[Joseph Wu]

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

(Updated March 8, 2016, 2:32 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Whitespace change from a comment in the next review.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Continuation of https://reviews.apache.org/r/43615/ with re-ordering of some 
local variables due to the order of destruction.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 
  src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 

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


Testing
---

Tests are run at the end of this review chain.


Thanks,

Joseph Wu

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Timothy Chen]

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

(Updated March 8, 2016, 9:58 p.m.)


Review request for mesos, Jie Yu and Travis Hegner.


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


Repository: mesos


Description
---

Fixed parsing network ip address with docker.


Diffs
-

  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 44363: Added stubs for the Nvidia GPU device isolator.

[Ben Mahler]

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


Fix it, then Ship it!




Looks good thanks, can you ask Jie about the naming scheme for the flag value?


src/Makefile.am (lines 907 - 911)


I was going to ask that we update cmake with these additional files, but it 
seems cmake doesn't include the handling of the network isolation stuff yet, so 
let's punt for now.



src/slave/containerizer/mesos/containerizer.cpp (lines 220 - 222)


Can we get some feedback from Jie about the naming we've chosen here?



src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp (line 
19)


Only one newline here



src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp (line 
55)


No periods at the end of error / failure messages, can you do a sweep here?


- Ben Mahler


On March 4, 2016, 1:10 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44363/
> ---
> 
> (Updated March 4, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Rob Todd.
> 
> 
> Bugs: MESOS-4623
> https://issues.apache.org/jira/browse/MESOS-4623
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the first isolator to fall under the category of cgroup
> devices. However, we do not yet have a generic cgroup device isolator
> (nor will we in the very near future). As such, I have preemptively
> created the nvidia gpu isolator in the directory hierarchy under the
> path:
> 
> src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/
> 
> in order to easily allow them to fall under the category of gpu
> devices later on.
> 
> In this stub implementation, initialization of the agent will
> fail if the nvidia gpu isolator is enabled via the agent --isolation
> flag. That is --isolation="cgroups/devices/gpu/nvidia". In a
> subsequent commit we will fill in the guts to actually enable the
> proper isolation.
> 
> The flags documentation has been udpated accordingly.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 305ba2c801c2060db6dcb4ef83c1043aaa7d520c 
>   src/Makefile.am 5b54fe04734afc5faa1bba3b84f43a1b8efe8753 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> PRE-CREATION 
>   src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
> 
> Diff: https://reviews.apache.org/r/44363/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 44531: Fixed parsing network ip address with docker.

[Timothy Chen]

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

Review request for mesos, Jie Yu and Travis Hegner.


Repository: mesos


Description
---

Fixed parsing network ip address with docker.


Diffs
-

  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Timothy Chen]

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

(Updated March 8, 2016, 9:58 p.m.)


Review request for mesos, Jie Yu and Travis Hegner.


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


Repository: mesos


Description
---

Fixed parsing network ip address with docker.


Diffs
-

  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 44513: Added missing flag to authentication docs.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44513]

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

- Mesos ReviewBot


On March 8, 2016, 3:15 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44513/
> ---
> 
> (Updated March 8, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added missing flag to authentication docs.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md 2de0eedd75e6f34549351c0ace4fee9aba7f2fd1 
> 
> Diff: https://reviews.apache.org/r/44513/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44361: Added configure flags to build with Nvidia GPU support.

[Ben Mahler]

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


Fix it, then Ship it!




Looks good, thanks!


configure.ac (lines 215 - 216)


Could you do a sweep and capitalize NVML, Nvidia, and GPU in the comments? 
The convention is to do acronyms in all caps, but we unfortunately do this for 
classes as well, like URL, which we should change at some point since it leads 
to confusing name boundaries (e.g. HTTPConnection would be better named as 
HttpConnection, this doesn't exist but it's just an example. Another example is 
http:: vs JSON::, ideally namespace names are only lower case).



configure.ac (lines 957 - 976)


Could we flatten this to make it a bit easier to read?


- Ben Mahler


On March 4, 2016, 1:09 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44361/
> ---
> 
> (Updated March 4, 2016, 1:09 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Rob Todd.
> 
> 
> Bugs: MESOS-4861
> https://issues.apache.org/jira/browse/MESOS-4861
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the initial commit to begin adding native support for GPUs in
> Mesos. This initial version will only include support for Nvidia GPUs
> that can be managed by the Nvidia Management Library (nvml).
> 
> The configure flags added in this commit can be used to enable Nvidia
> GPU support, as well as specify the installation directories of the
> nvml header and library files if not already installed in standard
> include/library paths on the system.
> 
> In a subsequent commit, we will use these configure flags to
> conditionally build support for Nvidia GPUs into Mesos.
> 
> 
> Diffs
> -
> 
>   configure.ac b045d3c68a2d440bed4d1b3e6ab21a1bbe063517 
> 
> Diff: https://reviews.apache.org/r/44361/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44512: Support to get weights info by /weights.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [41681, 43863, 44512]

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

- Mesos ReviewBot


On March 8, 2016, 2:52 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44512/
> ---
> 
> (Updated March 8, 2016, 2:52 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4316
> https://issues.apache.org/jira/browse/MESOS-4316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support to get weights info by /weights.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
>   src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/weights_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44512/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
> --authenticate_http --credentials=/opt/credentials.json  >> 
> /tmp/mesos-master.log 2>&1 &)
> 
> $ curl --user framework1:secret_string1 http://localhost:5050/weights | 
> python -mjson.tool
> {}
> 
> $ curl --user framework1:secret_string1 --data 
> "[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
>  -X PUT http://127.0.0.1:5050/weights
> 
> $ curl --user framework1:secret_string1 http://localhost:5050/weights | 
> python -mjson.tool
> {
> "infos": [
> {
> "role": "role3",
> "weight": 3.4
> },
> {
> "role": "role2",
> "weight": 1.0
> },
> {
> "role": "role1",
> "weight": 1.8
> }
> ]
> }
> 
> (TODO) Add couple of tests in another patch.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44360: Added a script to install the Nvidia GDK on a host.

[Ben Mahler]

> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > Looks good, main thing is just to add some context to the installer script 
> > so that others understand why it exists.

Could you update the testing done so that others can tell how you tested this?


- Ben


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


On March 4, 2016, 1:21 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44360/
> ---
> 
> (Updated March 4, 2016, 1:21 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Rob Todd.
> 
> 
> Bugs: MESOS-4860
> https://issues.apache.org/jira/browse/MESOS-4860
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script can be used to install the Nvidia GDK for Cuda 7.5 on a
> mesos development machine. The purpose of the Nvidia GDK is to provide
> all the necessary header files (nvml.h) and library files
> (libnvidia-ml.so) necessary to build mesos with Nvidia GPU support.
> 
> If the machine on which Mesos is being compiled doesn't have any GPUs,
> then libnvidia-ml.so consists only of stubs, allowing Mesos to build
> and run, but not actually do anything useful under the hood. This
> enables us to build a GPU-enabled mesos on a development machine
> without GPUs and then deploy it to a production machine with GPUs and
> be reasonably sure it will work.
> 
> The installation script itself takes a few parameters.
> Run it as support/install-nvidia-gdk.sh --help to see its usage
> semantics.
> 
> 
> Diffs
> -
> 
>   support/install-nvidia-gdk.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44360: Added a script to install the Nvidia GDK on a host.

[Ben Mahler]

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



Looks good, main thing is just to add some context to the installer script so 
that others understand why it exists.


support/install-nvidia-gdk.sh (lines 1 - 2)


Could we pull down some of the content from the description into a block 
comment here? A few things that would be great to have:

-What is the GDK?
-Why do we include this installer script?
-How to uninstall.
-That apparently nvidia will stop publishing GDKs? We should check with Rob 
on that.

Would be helpful also to mention why we tie this to a particular version.



support/install-nvidia-gdk.sh (line 21)


Should this include the ldcache flag as well?



support/install-nvidia-gdk.sh (line 89)


Can you check for errors on wget and make sure that it exits nicely? (e.g. 
test it out with a bad URL).



support/install-nvidia-gdk.sh (line 95)


I probably wouldn't know what "invalid" means in this context. :)



support/install-nvidia-gdk.sh (lines 99 - 100)


Maybe a little more context that nvidia's installer crashes when it 
encounters this file during re-installation?



support/install-nvidia-gdk.sh (lines 102 - 104)


Maybe a small note about why we use `--silent` here?



support/install-nvidia-gdk.sh (line 108)


Could we move this up to the top so that it's clearer that this is a side 
effect file after installation?


- Ben Mahler


On March 4, 2016, 1:21 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44360/
> ---
> 
> (Updated March 4, 2016, 1:21 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Rob Todd.
> 
> 
> Bugs: MESOS-4860
> https://issues.apache.org/jira/browse/MESOS-4860
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script can be used to install the Nvidia GDK for Cuda 7.5 on a
> mesos development machine. The purpose of the Nvidia GDK is to provide
> all the necessary header files (nvml.h) and library files
> (libnvidia-ml.so) necessary to build mesos with Nvidia GPU support.
> 
> If the machine on which Mesos is being compiled doesn't have any GPUs,
> then libnvidia-ml.so consists only of stubs, allowing Mesos to build
> and run, but not actually do anything useful under the hood. This
> enables us to build a GPU-enabled mesos on a development machine
> without GPUs and then deploy it to a production machine with GPUs and
> be reasonably sure it will work.
> 
> The installation script itself takes a few parameters.
> Run it as support/install-nvidia-gdk.sh --help to see its usage
> semantics.
> 
> 
> Diffs
> -
> 
>   support/install-nvidia-gdk.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 44523: Changed the master's default HTTP authentication realm.

[Greg Mann]

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

Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


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


Repository: mesos


Description
---

Changed the master's default HTTP authentication realm.


Diffs
-

  src/master/constants.cpp e316f9772d880b694faeee6d001dc56bc088c118 

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


Testing
---

`make check`


Thanks,

Greg Mann

Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

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

Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


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


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs
-

  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/constants.cpp 0f0d8e4b079d136d250f83cfc768de8c98b8bee2 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 6e3fd69c06eefd40bc0e5c222ea72f34144c5534 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

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


Testing
---

`make check` was used to test. However, these flags aren't yet incorporated 
into any agent endpoints. We should probably wait to merge this patch along 
with the changes for https://issues.apache.org/jira/browse/MESOS-4850 when 
they're ready.


Thanks,

Greg Mann

Re: Review Request 41049: New python lib with only the executor driver.

[Steve Niemitz]

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



Sorry for the spam, I added -static-libstdc++ to allow better compatiblity, but 
if it's going to cause problems we could drop it.

- Steve Niemitz


On March 8, 2016, 6:25 p.m., Steve Niemitz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41049/
> ---
> 
> (Updated March 8, 2016, 6:25 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Bugs: MESOS-4090
> https://issues.apache.org/jira/browse/MESOS-4090
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New python lib with only the executor driver.
> 
> This patch produces a new python egg, mesos.executor, which contains only the 
> code needed to create a MesosExecutorDriver.  By doing so, the linker can 
> remove unused code in libmesos_no_3rdparty.a, and therefor not include any 
> external dependencies in the resulting _mesos.so.
> 
> 
> Diffs
> -
> 
>   configure.ac a20382e8d425eb297492a6e6c2c75ea59be097c2 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/python/executor/setup.py.in PRE-CREATION 
>   src/python/executor/src/mesos/__init__.py PRE-CREATION 
>   src/python/executor/src/mesos/executor/__init__.py PRE-CREATION 
>   src/python/executor/src/mesos/executor/module.cpp PRE-CREATION 
>   src/python/native/ext_modules.py.in 
> eb93864733713dddad66141c6b8b6cd895f41484 
>   src/python/native/setup.py.in 49ed61293281f65d6597470ce3697326ac769032 
>   src/python/native/src/mesos/native/__init__.py 
> 226f94357cd81b700706edd68c94de461647fa1b 
>   src/python/native/src/mesos/native/mesos_executor_driver_impl.hpp  
>   src/python/native/src/mesos/native/mesos_executor_driver_impl.cpp 
> 7838a07ac12034a5eed2f459bd11e5e0a07e94de 
>   src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp  
>   src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp 
> f8be49bf5feb6675c3c8e6a1df75a5e079d09fcc 
>   src/python/native/src/mesos/native/module.hpp 
> 2cf7b57077d10c071bd4b8f36e2cb43041fc168d 
>   src/python/native/src/mesos/native/module.cpp 
> 92373611b8d3c54d8af97b55064074bd423726ee 
>   src/python/native/src/mesos/native/proxy_executor.hpp  
>   src/python/native/src/mesos/native/proxy_executor.cpp 
> 706f417b547bb9878346902b2a2f98b1afade5ad 
>   src/python/native/src/mesos/native/proxy_scheduler.hpp  
>   src/python/native/src/mesos/native/proxy_scheduler.cpp 
> 8afb3380d99b4dc9c4f7b3d926f4a5b1d6e94c20 
>   src/python/native_common/ext_modules.py.in PRE-CREATION 
>   src/python/scheduler/setup.py.in PRE-CREATION 
>   src/python/scheduler/src/mesos/__init__.py PRE-CREATION 
>   src/python/scheduler/src/mesos/scheduler/__init__.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41049/diff/
> 
> 
> Testing
> ---
> 
> On CentOS 7, GCC 4.8.5:
> - make distcheck
> - Tested with aurora executor
> - Made sure mesos.scheduler still worked
> - Made sure mesos.native still worked
> 
> 
> Thanks,
> 
> Steve Niemitz
> 
>

Re: Review Request 41049: New python lib with only the executor driver.

[Steve Niemitz]

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

(Updated March 8, 2016, 6:25 p.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


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


Repository: mesos


Description
---

New python lib with only the executor driver.

This patch produces a new python egg, mesos.executor, which contains only the 
code needed to create a MesosExecutorDriver.  By doing so, the linker can 
remove unused code in libmesos_no_3rdparty.a, and therefor not include any 
external dependencies in the resulting _mesos.so.


Diffs (updated)
-

  configure.ac a20382e8d425eb297492a6e6c2c75ea59be097c2 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/python/executor/setup.py.in PRE-CREATION 
  src/python/executor/src/mesos/__init__.py PRE-CREATION 
  src/python/executor/src/mesos/executor/__init__.py PRE-CREATION 
  src/python/executor/src/mesos/executor/module.cpp PRE-CREATION 
  src/python/native/ext_modules.py.in eb93864733713dddad66141c6b8b6cd895f41484 
  src/python/native/setup.py.in 49ed61293281f65d6597470ce3697326ac769032 
  src/python/native/src/mesos/native/__init__.py 
226f94357cd81b700706edd68c94de461647fa1b 
  src/python/native/src/mesos/native/mesos_executor_driver_impl.hpp  
  src/python/native/src/mesos/native/mesos_executor_driver_impl.cpp 
7838a07ac12034a5eed2f459bd11e5e0a07e94de 
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp  
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp 
f8be49bf5feb6675c3c8e6a1df75a5e079d09fcc 
  src/python/native/src/mesos/native/module.hpp 
2cf7b57077d10c071bd4b8f36e2cb43041fc168d 
  src/python/native/src/mesos/native/module.cpp 
92373611b8d3c54d8af97b55064074bd423726ee 
  src/python/native/src/mesos/native/proxy_executor.hpp  
  src/python/native/src/mesos/native/proxy_executor.cpp 
706f417b547bb9878346902b2a2f98b1afade5ad 
  src/python/native/src/mesos/native/proxy_scheduler.hpp  
  src/python/native/src/mesos/native/proxy_scheduler.cpp 
8afb3380d99b4dc9c4f7b3d926f4a5b1d6e94c20 
  src/python/native_common/ext_modules.py.in PRE-CREATION 
  src/python/scheduler/setup.py.in PRE-CREATION 
  src/python/scheduler/src/mesos/__init__.py PRE-CREATION 
  src/python/scheduler/src/mesos/scheduler/__init__.py PRE-CREATION 

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


Testing
---

On CentOS 7, GCC 4.8.5:
- make distcheck
- Tested with aurora executor
- Made sure mesos.scheduler still worked
- Made sure mesos.native still worked


Thanks,

Steve Niemitz

Re: Review Request 44319: Removed initialize method from the authorizer interface.

[Vinod Kone]

> On March 8, 2016, 1:08 a.m., Vinod Kone wrote:
> > include/mesos/authorizer/authorizer.hpp, line 58
> > 
> >
> > seems weird that this interface takes ACLs as a param. can we make it 
> > take Parameters instead? I think it gets rid of a bunch of boilerplate code 
> > below.
> 
> Alexander Rojas wrote:
> It won't get rid of any boiler plate code, and I will actually have to 
> add some extra code to translate flags to acls, since the place where this 
> constructor is used is done as `Try create = 
> Authorizer::create(authorizerName, flags.acls);`, Not that this works mostly 
> as a dispatcher which will either call `LocalAuthorizer::create(const ACLs& 
> acls)` or `modules::ModuleManager::create(name);`.
> 
> I considered using a constructor which looks as `Authorizer::create(const 
> Flags&)` but the flags are `internal` which means they are not part of the 
> exported headers.
> 
> However, I will change it to parameters mostly for flexibility! since 
> this header doesn't belong neither to master nor to the agent but can be used 
> by both.

I see. Lets add a separate overload for Authorizer::create() that takes ACLs 
then. See my comments in the next review.


> On March 8, 2016, 1:08 a.m., Vinod Kone wrote:
> > src/examples/test_authorizer_module.cpp, line 41
> > 
> >
> > s/rawAcls/stringAcls/

actually, lets call this "acls_".


> On March 8, 2016, 1:08 a.m., Vinod Kone wrote:
> > src/tests/authorization_tests.cpp, line 51
> > 
> >
> > have we used this pattern elsewhere?
> 
> Alexander Rojas wrote:
> We have a similar patter with the 
> [`http::Authenticator`](https://github.com/apache/mesos/blob/95faeac356d7918803beeaa77098569383539a16/include/mesos/authentication/http/basic_authenticator_factory.hpp),
>  althrough there the factory is part of Mesos (we had to do such a thing 
> because the interface to be a module was part of libprocess, so we couldn't 
> create a mesos friendly `create()` method there, so we extracted to its own 
> factory class).
> 
> The reason I went for this pattern was so I didn't have to create an 
> extra factory function in the `LocalAuthorizer` which takes parameters only 
> for  
> 
> The only alternative I saw to this was to add a constructor which takes 
> `Parameters` to the `LocaAuthorizer` which will be used only for testing 
> purposes, which I didn't want to have. But if you think that path is the 
> right one, please let me know.

LocalAuthorizer::create(const Parameters& parameters) sounds fine to me.


- Vinod


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


On March 8, 2016, 4:43 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44319/
> ---
> 
> (Updated March 8, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-2950
> https://issues.apache.org/jira/browse/MESOS-2950
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed initialize method from the authorizer interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> ec6c9928c55c3096c7de634f900419abbdd00886 
>   src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
>   src/authorizer/local/authorizer.hpp 
> 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
>   src/authorizer/local/authorizer.cpp 
> 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
>   src/examples/test_authorizer_module.cpp 
> 95d77fbff0cdfdb360a8597fbba28404b59d0042 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
> 
> Diff: https://reviews.apache.org/r/44319/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 41049: New python lib with only the executor driver.

[Steve Niemitz]

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

(Updated March 8, 2016, 6:15 p.m.)


Review request for mesos, Till Toenshoff and Vinod Kone.


Changes
---

oops, some extra flags I had been using to debug with snuck into that last 
diff, reverted.


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


Repository: mesos


Description
---

New python lib with only the executor driver.

This patch produces a new python egg, mesos.executor, which contains only the 
code needed to create a MesosExecutorDriver.  By doing so, the linker can 
remove unused code in libmesos_no_3rdparty.a, and therefor not include any 
external dependencies in the resulting _mesos.so.


Diffs (updated)
-

  configure.ac a20382e8d425eb297492a6e6c2c75ea59be097c2 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/python/executor/setup.py.in PRE-CREATION 
  src/python/executor/src/mesos/__init__.py PRE-CREATION 
  src/python/executor/src/mesos/executor/__init__.py PRE-CREATION 
  src/python/executor/src/mesos/executor/module.cpp PRE-CREATION 
  src/python/native/ext_modules.py.in eb93864733713dddad66141c6b8b6cd895f41484 
  src/python/native/setup.py.in 49ed61293281f65d6597470ce3697326ac769032 
  src/python/native/src/mesos/native/__init__.py 
226f94357cd81b700706edd68c94de461647fa1b 
  src/python/native/src/mesos/native/mesos_executor_driver_impl.hpp  
  src/python/native/src/mesos/native/mesos_executor_driver_impl.cpp 
7838a07ac12034a5eed2f459bd11e5e0a07e94de 
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.hpp  
  src/python/native/src/mesos/native/mesos_scheduler_driver_impl.cpp 
f8be49bf5feb6675c3c8e6a1df75a5e079d09fcc 
  src/python/native/src/mesos/native/module.hpp 
2cf7b57077d10c071bd4b8f36e2cb43041fc168d 
  src/python/native/src/mesos/native/module.cpp 
92373611b8d3c54d8af97b55064074bd423726ee 
  src/python/native/src/mesos/native/proxy_executor.hpp  
  src/python/native/src/mesos/native/proxy_executor.cpp 
706f417b547bb9878346902b2a2f98b1afade5ad 
  src/python/native/src/mesos/native/proxy_scheduler.hpp  
  src/python/native/src/mesos/native/proxy_scheduler.cpp 
8afb3380d99b4dc9c4f7b3d926f4a5b1d6e94c20 
  src/python/native_common/ext_modules.py.in PRE-CREATION 
  src/python/scheduler/setup.py.in PRE-CREATION 
  src/python/scheduler/src/mesos/__init__.py PRE-CREATION 
  src/python/scheduler/src/mesos/scheduler/__init__.py PRE-CREATION 

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


Testing
---

On CentOS 7, GCC 4.8.5:
- make distcheck
- Tested with aurora executor
- Made sure mesos.scheduler still worked
- Made sure mesos.native still worked


Thanks,

Steve Niemitz

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

[Daniel Pravat]

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

(Updated March 8, 2016, 6:15 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

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

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


Testing
---

OSX: make check
Windows: make


Thanks,

Daniel Pravat

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

[Daniel Pravat]

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

(Updated March 8, 2016, 6:10 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Summary (updated)
-

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


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


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

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

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


Testing
---

OSX: make check
Windows: make


Thanks,

Daniel Pravat

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/network/cni.cpp (line 49)


How about using `'--network_cni_plugins_dir'`?



src/slave/containerizer/mesos/isolators/network/cni.cpp (line 54)


ditto.



src/slave/containerizer/mesos/isolators/network/cni.cpp (lines 58 - 60)


return Error(
"The Network CNI plugins directory '" +
flags.network_cni_plugins_dir.get() + "' does not exist");



src/slave/containerizer/mesos/isolators/network/cni.cpp (lines 65 - 66)


ditto.



src/slave/containerizer/mesos/isolators/network/cni.cpp (lines 95 - 288)


I would strongly recommend move all `parse` and `validation` operations to 
`spec.cpp`, because we may not want the `create` method to be that long.


- Gilbert Song


On March 8, 2016, 6:42 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 8, 2016, 6:42 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 43920: Added a helper function to stout : os/which.hpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43920]

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

- Mesos ReviewBot


On March 8, 2016, 2:22 p.m., Disha  Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43920/
> ---
> 
> (Updated March 8, 2016, 2:22 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper function to stout : os/which.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43920/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Disha  Singh
> 
>

Re: Review Request 44474: Improve master tasks metrics.

[Cong Wang]

> On March 7, 2016, 11:17 p.m., Neil Conway wrote:
> > src/master/master.cpp, line 6461
> > 
> >
> > Seems like we use post-increment elsewhere in this RR?

I am not sure I understand your question, but post-increment is supported by 
Counter even before my patches.


- Cong


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


On March 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44474: Improve master tasks metrics.

[Cong Wang]

> On March 8, 2016, 12:37 a.m., Ian Downes wrote:
> > src/master/metrics.cpp, line 215
> > 
> >
> > foreach?

I don't think we can use foreach over enum, can we?


- Cong


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


On March 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44474: Improve master tasks metrics.

[Cong Wang]

> On March 8, 2016, 12:37 a.m., Ian Downes wrote:
> > src/master/master.cpp, line 6461
> > 
> >
> > I'm not familiar with this code but it appears to be changing the 
> > behavior substantially. 
> > 
> > The original code sums across all slaves that are registered at the 
> > time of the metric call. The new code tracks counters for the various 
> > states when a task on a registered slave changes. IIUC, this is a 
> > fundamental change in how the metrics are determined because the slave 
> > registration state is considered at different times. For example, a task 
> > may change state on a registered slave and it gets counted, then the slave 
> > becomes unregistered, then the metric endpoint is queried. Old and new code 
> > will give different numbers?
> > 
> > If my understanding is correct, then perhaps (somewhat) more performant 
> > code could be achieved by maintaining counters at the slave level, and then 
> > aggregating the counters from *registered* slaves?

So are you saying I miss the slave unregistration event? But when we unregister 
a slave, we remove all the tasks from that slave too, and the counter is 
updated in removeTask(). Or I misunderstand your point?

As the slaves/tasks states are complicated, I am not surprised at all I miss 
some case, please point it out explicitly so that I can fix it. ;)


> On March 8, 2016, 12:37 a.m., Ian Downes wrote:
> > src/master/metrics.cpp, lines 38-46
> > 
> >
> > Please keep the original order which more closely matches the 
> > progression of states, i.e., staging before starting.

Nope, we use TASK_* as an index for this array, therefore it has to be in this 
order.


- Cong


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


On March 7, 2016, 10:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44474/
> ---
> 
> (Updated March 7, 2016, 10:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Vinod Kone.
> 
> 
> Bugs: MESOS-4740
> https://issues.apache.org/jira/browse/MESOS-4740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid iterate the list of slaves, instead just maintain some counters.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
> 
> Diff: https://reviews.apache.org/r/44474/diff/
> 
> 
> Testing
> ---
> 
> Manual check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 44322: Implemented a generalized interface for the authorizer.

[Alexander Rojas]

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

(Updated March 8, 2016, 5:50 p.m.)


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


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


Repository: mesos


Description
---

**WIP: Do not commit**

Implements the later [Generic Authorizer Interface v 
0.3.1](https://docs.google.com/document/d/1-XARWJFUq0r_TgRHz_472NvLZNjbqE4G8c2JL44OSMQ)
proposal.

It still lacks some parts:

- [ ] Doxygen in the interface.
- [ ] Updating `authorizer.md`

Still the basic functionality is there and I don't expect it to change
much. As I mentioned, comments aren't there yet but feel free to point
where they are missed, at this point focusing in the actual content
may not be relevant as the patch may change from its current form.


Diffs
-

  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  include/mesos/authorizer/authorizer.proto PRE-CREATION 
  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
  src/authorizer/local/authorizer.hpp 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
  src/authorizer/local/authorizer.cpp 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
  src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/quota_handler.cpp a41c91f10bc0eedc754425b4de1b3e184c4ffb08 
  src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/mesos.hpp 9c62833e0a64cfd62fce8cffd04f9cdd933646c8 
  src/tests/mesos.cpp 395b23d32b2d03aef446858e197cb9788644eefa 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 

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


Testing
---

make -j4 check


Thanks,

Alexander Rojas

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

> On March 3, 2016, 10:15 p.m., Joseph Wu wrote:
> > src/master/contender.hpp, lines 17-18
> > 
> >
> > This whole file seems like a pretty substantial change.  I'd recommend 
> > pulling it out into a separate review (rather than hiding it in this 
> > review).
> > 
> > Also, you'll want to consider making folders "contenders" and 
> > "detectors".  Then renaming this file "standalone.hpp".
> 
> Anurag Singh wrote:
> Ok. Although then zookeeper's classes should also go into their own files.
> 
> Anurag Singh wrote:
> Looking at this again, the changes in this file are really just namespace 
> changes (the only thing substantial is the removal of the MasterContender 
> class definition) - I can't put them into a different commit without breaking 
> the build (I'm trying to make sure individual commits don't break builds, 
> which I think is a sensible goal). However, I can leave this commit as is but 
> create a separate commit to create the contenders/detectors directories. Is 
> that acceptable to you?
> 
> Anurag Singh wrote:
> Ping ... will the suggestion I made work for you?
> 
> Joseph Wu wrote:
> Don't worry about having atomic commits (especially for verbose changes).
> 
> There are a couple of changes to consider here, each of which might 
> deserve its own review:
> - Pulling out the interface deletions.  (You also consider pulling it 
> into the previous review, as you're really moving the code from private to 
> public headers.)
> - Adding a folder for each module you are adding ("contender" and 
> "detector").
> - Breaking apart the Standalone vs Zookeeper logic.

While separating the zookeeper and standalone detectors and moving them into 
detectors, I realized that the functions in the 'promises' namespace will be 
duplicated unless I move them into their own header file. I can either put them 
in include/mesos and change the namespace to mesos mesos::internal::promises, 
or just  leave the header file in detectors. Which one is preferable? bmahler 
had a todo for doing this  so I'm not sure if your team already has some work 
planned in this area.


- Anurag


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


On March 3, 2016, 5:30 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> ---
> 
> (Updated March 3, 2016, 5:30 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
> https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 

Re: Review Request 44320: Moved authorizer.proto to acls.proto.

[Alexander Rojas]

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

(Updated March 8, 2016, 5:44 p.m.)


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


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


Repository: mesos


Description
---

This is the first step towards separating the language used to define
the ACLs from the mechanism to query them.


Diffs
-

  include/mesos/authorizer/acls.hpp PRE-CREATION 
  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  include/mesos/authorizer/authorizer.proto  
  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/authorizer/acls.cpp PRE-CREATION 
  src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
  src/common/parse.hpp 78c7cf12ca6a475305254177db6b6b2319a1f72f 
  src/examples/persistent_volume_framework.cpp 
4218b1563e10aaefe9abcdc20c90c13651959790 
  src/examples/test_authorizer_module.cpp 
95d77fbff0cdfdb360a8597fbba28404b59d0042 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 44319: Removed initialize method from the authorizer interface.

[Alexander Rojas]

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

(Updated March 8, 2016, 5:43 p.m.)


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


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


Repository: mesos


Description
---

Removed initialize method from the authorizer interface.


Diffs
-

  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 
  src/authorizer/local/authorizer.hpp 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
  src/authorizer/local/authorizer.cpp 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
  src/examples/test_authorizer_module.cpp 
95d77fbff0cdfdb360a8597fbba28404b59d0042 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
  src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

> On March 7, 2016, 5:36 p.m., Timothy Chen wrote:
> > Are you still be able to work on this? We like to get this merged, so if 
> > you can't or don't reply we will create a new patch based on this.
> 
> Travis Hegner wrote:
> Hi Timothy,
> 
> I've been stalling this for https://reviews.apache.org/r/42516, as that 
> patch will change the way this patch should be written. It only works now out 
> of pure luck in the way docker intereprets multiple "--net" parameters, since 
> mesos doesn't yet officially support user defined networks.
> 
> The linked patch will make this patch support it properly, as well as 
> make this patch easier to write, as the network name will not have to be 
> queried at all from json; it will be available as a variable.
> 
> Timothy Chen wrote:
> I don't quite understand the pure luck part, IIUC your patch is updating 
> how we parse the Docker inspect output for Network IPAddress, which shouldn't 
> matter how we actually launched the container right? How is the linked patch 
> related?
> 
> Timothy Chen wrote:
> At this moment this is actually like becomes a blocker for 0.28 as we're 
> not returning the right IP address with the latest Docker daemon, so I'd like 
> to get this in ASAP. For now I'll create a new patch based on your changes, 
> and once the dependend patch goes in will love if you can continue the work.
> 
> Travis Hegner wrote:
> I think I understand the miscommunication better now. As you know, 
> beginning with docker 1.9 the docker inspect output changed the location of 
> the IPAddress. The original location was still populated for backwards 
> compatability, but only for the common "bridge" and "host" network types. 
> Mesos is written to fail with any other network type. With the new user 
> defined networks feature, the old location was not populated. My patch was 
> originally intended to address the fact that with user defined networks, the 
> original ip location was null.
> 
> In order to utilize user defined networks in my environment, we are 
> passing arbitraty docker parameters to mesos with the docker containerizer 
> from marathon. This results in multiple "--net" parameters passed to docker. 
> The luck comes into play because mesos interperets the first --net parameter 
> of "bridge" and succeeds, and docker interperets the second --net parameter 
> of my UDN, and connects to the right network. I would consider this behavior 
> unstable at best.
> 
> Based on the sudden up-tick in interest in this patch, I am speculating 
> that docker 1.10 is no longer populating the original ip address field (I 
> would be un-aware, because I've been running my cluster with this patch), 
> which this patch will successfully fix, and even be stable for the typical 
> "host" and "bridge" networks.
> 
> All that said, I can see why this patch is now more important, even 
> though it should be re-structured after review 42516 is implemented. I'll see 
> if I can spend some time today and address the remaining issues with this 
> patch.
> 
> Timothy Chen wrote:
> Yes IP Address is now being populated in a different field now, so 
> basically the patch has to handle two changes in the API, one is the addition 
> of network mode and another is the change of IPAddress.
> 
> I'm not sure your availability is so I'm having a patch similiar to 
> yours. Sorry for this but we like to see this get into 0.28.0.

I've been working this, and I believe the appropriate way to fix it is by 
checking the docker version, and behaving accordingly. The problem is that the 
validateVersion() function is not available within the 
Docker::Container::create() scope... (have a look at 
https://github.com/travishegner/mesos/blob/networkInfo/src/docker/docker.cpp#L305)

Any ideas on the best way to check the docker version within this scope? I 
didn't want to pass a reference in to the Docker object, as that requires 
changes to some of the tests as well.

Of course, if we don't want to maintain backwards compatability with older 
docker versions, then we can eliminate the need for the version check.

@Timothy, are you on IRC? We may be able to work together on this...


- Travis


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


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> 

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Timothy Chen]

> On March 7, 2016, 5:36 p.m., Timothy Chen wrote:
> > Are you still be able to work on this? We like to get this merged, so if 
> > you can't or don't reply we will create a new patch based on this.
> 
> Travis Hegner wrote:
> Hi Timothy,
> 
> I've been stalling this for https://reviews.apache.org/r/42516, as that 
> patch will change the way this patch should be written. It only works now out 
> of pure luck in the way docker intereprets multiple "--net" parameters, since 
> mesos doesn't yet officially support user defined networks.
> 
> The linked patch will make this patch support it properly, as well as 
> make this patch easier to write, as the network name will not have to be 
> queried at all from json; it will be available as a variable.
> 
> Timothy Chen wrote:
> I don't quite understand the pure luck part, IIUC your patch is updating 
> how we parse the Docker inspect output for Network IPAddress, which shouldn't 
> matter how we actually launched the container right? How is the linked patch 
> related?
> 
> Timothy Chen wrote:
> At this moment this is actually like becomes a blocker for 0.28 as we're 
> not returning the right IP address with the latest Docker daemon, so I'd like 
> to get this in ASAP. For now I'll create a new patch based on your changes, 
> and once the dependend patch goes in will love if you can continue the work.
> 
> Travis Hegner wrote:
> I think I understand the miscommunication better now. As you know, 
> beginning with docker 1.9 the docker inspect output changed the location of 
> the IPAddress. The original location was still populated for backwards 
> compatability, but only for the common "bridge" and "host" network types. 
> Mesos is written to fail with any other network type. With the new user 
> defined networks feature, the old location was not populated. My patch was 
> originally intended to address the fact that with user defined networks, the 
> original ip location was null.
> 
> In order to utilize user defined networks in my environment, we are 
> passing arbitraty docker parameters to mesos with the docker containerizer 
> from marathon. This results in multiple "--net" parameters passed to docker. 
> The luck comes into play because mesos interperets the first --net parameter 
> of "bridge" and succeeds, and docker interperets the second --net parameter 
> of my UDN, and connects to the right network. I would consider this behavior 
> unstable at best.
> 
> Based on the sudden up-tick in interest in this patch, I am speculating 
> that docker 1.10 is no longer populating the original ip address field (I 
> would be un-aware, because I've been running my cluster with this patch), 
> which this patch will successfully fix, and even be stable for the typical 
> "host" and "bridge" networks.
> 
> All that said, I can see why this patch is now more important, even 
> though it should be re-structured after review 42516 is implemented. I'll see 
> if I can spend some time today and address the remaining issues with this 
> patch.

Yes IP Address is now being populated in a different field now, so basically 
the patch has to handle two changes in the API, one is the addition of network 
mode and another is the change of IPAddress.

I'm not sure your availability is so I'm having a patch similiar to yours. 
Sorry for this but we like to see this get into 0.28.0.


- Timothy


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


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API 

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 4:18 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md e888b233351b2da05a5e5c63138de5f60708afea 

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


Testing
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Rendered Website
  
https://reviews.apache.org/media/uploaded/files/2016/03/08/dae6b1ea-a627-4198-b5ad-8e744120ab68__Screen_Shot_2016-03-08_at_15.06.55.png


Thanks,

Joerg Schad

Re: Review Request 43798: Added overview section to upgrades.md.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43798, 43792]

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

Error:
2016-03-08 16:12:50 URL:https://reviews.apache.org/r/43798/diff/raw/ 
[11321/11321] -> "43798.patch" [1]
error: patch failed: docs/upgrades.md:7
error: docs/upgrades.md: patch does not apply

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

- Mesos ReviewBot


On March 8, 2016, 2:08 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43798/
> ---
> 
> (Updated March 8, 2016, 2:08 p.m.)
> 
> 
> Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.
> 
> 
> Bugs: MESOS-4381
> https://issues.apache.org/jira/browse/MESOS-4381
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added overview section to upgrades.md.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 
> 
> Diff: https://reviews.apache.org/r/43798/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
> docker website container.
> 
> 
> File Attachments
> 
> 
> overview
>   
> https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
> Screen Shot 2016-02-29 at 3.15.12 AM.png
>   
> https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
> Screen Shot 2016-02-29 at 3.15.12 AM.png
>   
> https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png
> Rendered Website
>   
> https://reviews.apache.org/media/uploaded/files/2016/03/08/dae6b1ea-a627-4198-b5ad-8e744120ab68__Screen_Shot_2016-03-08_at_15.06.55.png
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 44514: Implemented NetworkCniIsolatorProcess::prepare().

[Qian Zhang]

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

Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Implemented NetworkCniIsolatorProcess::prepare().


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44511: Add registry tests for /weights endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [41681, 43863, 41790, 44511]

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

- Mesos ReviewBot


On March 8, 2016, 1:58 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44511/
> ---
> 
> (Updated March 8, 2016, 1:58 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4797
> https://issues.apache.org/jira/browse/MESOS-4797
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add registry tests for /weights endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44511/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>