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

[fan du]

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

(Updated 三月 14, 2016, 5:50 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

Makethe metrics counting for both NG cases(sanity check and validation check) 
and OK cases


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


Repository: mesos


Description
---

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
  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:

v3:
  - Move the couting out of common code path to http endpoint and master accept 
call separately to reflect its logic.

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 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

[fan du]

> On 三月 12, 2016, 2:11 a.m., Greg Mann wrote:
> > src/master/http.cpp, line 787
> > 
> >
> > This increment statement occurs after some invalidation logic, and 
> > directly before the authorization call; is this precisely where we want to 
> > be incrementing? We end up not counting invalid operations, but counting 
> > unauthorized operations.
> > 
> > Looking through the other metrics code in master.cpp, it seems we 
> > aren't entirely consistent with respect to when we increment the 
> > `messages_XXX` metrics. We increment them both before and after operation 
> > validation, for example.
> > 
> > In master.cpp, there are some existing `messages_XXX` metrics in the 
> > `accept()` function which get incremented together; those occur before 
> > validation and before authorization. To be consistent, you should probably 
> > do the same here. Currently, the metric gets incremented after `validate()` 
> > is called.
> > 
> > We should create a separate JIRA for making all of the messages metrics 
> > consistent with respect to when they get incremented.

I think the rule is clear to do the counting at the function entry call to 
include all other NG cases(sanity check and validation) as well.
Thanks for the point, and pls review the updated RR.

I will create another JIRA to track other metrics issue, and discuss with you 
later.


- fan


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


On 三月 10, 2016, 2:42 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated 三月 10, 2016, 2:42 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/http.cpp 950206baf7f3a1cccdc49d810126473966d8d021 
>   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:
> 
> v3:
>   - Move the couting out of common code path to http endpoint and master 
> accept call separately to reflect its logic.
> 
> 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 44315: Do not traverse offer list if there is only one offer.

[Guangya Liu]

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

(Updated 三月 14, 2016, 4:44 a.m.)


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


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


Repository: mesos


Description
---

Do not traverse offer list if there is only one offer.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp 
5a421d4e8740b5105518283943087869025cbfa6 

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


Testing
---

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


Thanks,

Guangya Liu

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

[Greg Mann]

> On March 13, 2016, 4:26 p.m., Joerg Schad wrote:
> > src/master/constants.hpp, line 136
> > 
> >
> > Should we make it "master HTTP authentication realm."?

Good idea; done!


- Greg


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


On March 14, 2016, 4:18 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44523/
> ---
> 
> (Updated March 14, 2016, 4:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, 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.hpp a188da30a8b55e884acc7e2f570ef0a9272e7472 
>   src/master/constants.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44523/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44553: Added authentication to agent HTTP endpoints.

[Greg Mann]

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

(Updated March 14, 2016, 4:18 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added authentication to agent HTTP endpoints.

This patch adds HTTP authentication to the `/state`, `/state.json`, and 
`/flags` endpoints. Tests are also updated to use authentication when hitting 
these endpoints, and a new test was added to probe these endpoints' behavior 
when bad credentials are supplied: `SlaveTest.HTTPEndpointsBadAuthentication`.


Diffs (updated)
-

  src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/containerizer/docker_containerizer_tests.cpp 
8afaa4dab3984e9866b7b223e8e2e70ef83a39dc 
  src/tests/fault_tolerance_tests.cpp 2d1bac2aa788a2f6c8b647c43d8b731ea02d4a24 
  src/tests/health_check_tests.cpp 9605859b665645654bbdb2688455cae2d692a057 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing
---

Tests were updated to use authentication when hitting the affected agent 
endpoints, and new tests were added to probe the endpoint behavior when invalid 
credentials are supplied.

`sudo make check` was used to test on both OSX and Ubuntu 14.04. The new test 
was run 1000 times to look for flakiness.


Thanks,

Greg Mann

Re: 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/
---

(Updated March 14, 2016, 4:18 a.m.)


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


Changes
---

Addressed comment.


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


Repository: mesos


Description
---

Changed the master's default HTTP authentication realm.


Diffs (updated)
-

  src/master/constants.hpp a188da30a8b55e884acc7e2f570ef0a9272e7472 
  src/master/constants.cpp PRE-CREATION 

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: 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/
---

(Updated March 14, 2016, 4:17 a.m.)


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


Changes
---

Rebase.


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 (updated)
-

  src/slave/constants.hpp 4110061d576d5e20e512e84bb049b6ac910ceab0 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 

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


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44703: Added Doxygen docs for basic HTTP authenticator.

[Greg Mann]

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

(Updated March 14, 2016, 4:17 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added Doxygen docs for basic HTTP authenticator.

The structure of the module parameters for the basic HTTP authenticator has 
changed, so Doxygen comments were added to the module's header file to provide 
an example.


Diffs (updated)
-

  include/mesos/authentication/http/basic_authenticator_factory.hpp 
c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 42386: Updated `createFrameworkInfo` for hierarchical_allocator_tests.cpp.

[Guangya Liu]

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

(Updated 三月 14, 2016, 4:03 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Updated `createFrameworkInfo` for hierarchical_allocator_tests.cpp.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
459e02576f6d05abbbcc83ae5cabac5c66e93f05 

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


Testing
---

make
make tests
GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose


Thanks,

Guangya Liu

Re: Review Request 40632: Enabled oversubscribed resources for reservations in allocator.

[Guangya Liu]

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

(Updated 三月 14, 2016, 4:03 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Enabled oversubscribed resources for reservations in allocator.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
70291075c00a9a557529c2562dedcfc6c6c3ec32 
  src/tests/hierarchical_allocator_tests.cpp 
459e02576f6d05abbbcc83ae5cabac5c66e93f05 

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


Testing
---

make
make check
GLOG_v=2  ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
--verbose --gtest_repeat=100 --gtest_shuffle


Thanks,

Guangya Liu

Re: Review Request 44678: Modified basic HTTP authenticator creator to accept realm.

[Greg Mann]

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

(Updated March 14, 2016, 4:02 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Modified basic HTTP authenticator creator to accept realm.

To accommodate different authentication realms for the master and agent, the 
default basic HTTP authenticator needs to accept its authentication realm as a 
parameter. This patch adds this parameter and modifies the HTTP authentication 
tests to validate it appropriately.


Diffs (updated)
-

  include/mesos/authentication/http/basic_authenticator_factory.hpp 
c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 
  src/authentication/http/basic_authenticator_factory.cpp 
62f851685db3b42c52bbcb7cff3e4f4703004ed7 
  src/master/master.cpp 255b4d1ba029d7f9cfb86138c66a25d0dffa30e0 
  src/tests/http_authentication_tests.cpp 
cf2bb762272fa38e04e5c26aef2858300bbd0459 

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


Testing
---

`make check` was used to test on both OSX and CentOS 7.


Thanks,

Greg Mann

Re: Review Request 44554: Added agent HTTP authentication to the docs.

[Greg Mann]

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

(Updated March 14, 2016, 4 a.m.)


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


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


Repository: mesos


Description
---

Added agent HTTP authentication to the docs.


Diffs (updated)
-

  docs/authentication.md dd538b512c6f822cd742fc6e2f3d4f7fbffadc06 
  docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
  docs/home.md fd7794f56b4a95268dbb82288d99cab19a89f5cd 

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


Testing
---

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


Thanks,

Greg Mann

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44758]

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 14, 2016, 1:09 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 14, 2016, 1:09 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44767: Added authentication information to master endpoints.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44767, 44766, 44768]

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

Error:
2016-03-14 01:53:00 URL:https://reviews.apache.org/r/44767/diff/raw/ 
[28637/28637] -> "44767.patch" [1]
error: patch failed: docs/endpoints/master/flags.md:13
error: docs/endpoints/master/flags.md: patch does not apply
error: patch failed: docs/endpoints/master/frameworks.md:13
error: docs/endpoints/master/frameworks.md: patch does not apply
error: patch failed: src/master/http.cpp:735
error: src/master/http.cpp: patch does not apply

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

- Mesos ReviewBot


On March 13, 2016, 10:33 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44767/
> ---
> 
> (Updated March 13, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master endpoints now display information whether authentication (if enabled) 
> is required in HELP.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/api/v1/scheduler.md 
> 7a75fd75eef458f26f9e5067b792329f310d5d24 
>   docs/endpoints/master/create-volumes.md 
> 2ab116a16581fc5cfc0fc5cf70c6e001a28d8493 
>   docs/endpoints/master/destroy-volumes.md 
> 88679fe12baf7642b63e71688b90031aaedab1ef 
>   docs/endpoints/master/flags.md 739d6bfae5ae35223d6bc7c4782ff0f16b868018 
>   docs/endpoints/master/frameworks.md 
> 95c1f0e63f6a61d0e6622a8ec9d821a646656cfc 
>   docs/endpoints/master/health.md 7f9352025a6364b812b91e9ccbe380a3a69ba4e1 
>   docs/endpoints/master/machine/down.md 
> 6eec9ecc6e92fd1299b27499f78055c6f4f68e81 
>   docs/endpoints/master/machine/up.md 
> 7cbbf04859a218f90672453e5037480676f79fe5 
>   docs/endpoints/master/maintenance/schedule.md 
> 2b5e782554136c1624483d8155a96472ec0660dd 
>   docs/endpoints/master/maintenance/status.md 
> 4d0c7551acb89fb375834fd703c406d68f8bdcfc 
>   docs/endpoints/master/observe.md fae1ee062350d7b25775bef98a0bebda1892ab62 
>   docs/endpoints/master/quota.md 812874d2dd6c3548887e3044ba1f3c3c8c9d1dd6 
>   docs/endpoints/master/redirect.md ac9d0fa3eae485726b10a3ac756228d0cb5aeb27 
>   docs/endpoints/master/reserve.md 1a4f67961baf761f79a693780f42a1a8ce2244fc 
>   docs/endpoints/master/roles.json.md 
> 863715386791ef9192f38bf390fcd2b31d988547 
>   docs/endpoints/master/roles.md 171e0163c4c2db34bf34c8303846793f3c29bdf5 
>   docs/endpoints/master/slaves.md ec169c15507d3c731b8833f2656949caf0efcc33 
>   docs/endpoints/master/state-summary.md 
> fb10ac7db5b94ea7982c245f1d884312d818c6b5 
>   docs/endpoints/master/state.json.md 
> 0415cfdd63cee0b350b3e33c496bacfc1ba53f8a 
>   docs/endpoints/master/state.md ae1ec718b8d718718727dbd2e7ce4739cca41680 
>   docs/endpoints/master/tasks.json.md 
> 46a1253d33cdfdca0a7158808d1e29da1a634933 
>   docs/endpoints/master/tasks.md 2ada97b5dfdaf5f1f7578fa42eb4e61233e6a753 
>   docs/endpoints/master/teardown.md f68d083a4271ed7dd0ddcc87da104a43eb40c7ec 
>   docs/endpoints/master/unreserve.md e059d8d888343c5036346f7a615f04375f44f517 
>   docs/endpoints/master/weights.md PRE-CREATION 
>   src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 
> 
> Diff: https://reviews.apache.org/r/44767/diff/
> 
> 
> Testing
> ---
> 
> Viewed master endpoint help in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44380: Change IOTest.BufferedRead to write to the temporary directory.

[Yong Tang]

> On March 4, 2016, 7:42 a.m., Benjamin Bannier wrote:
> > I think using a `TemporaryDirectoryTest` fixture is the right approach, but 
> > what I find unfortunate about your approach is that with this patch we'd 
> > create a temporary directory and incur the overhead for all tests in this 
> > suite, even ones not creating any files. If that's something we wouldn't 
> > want, what about using separate suites for tests creating files (that would 
> > be `BufferedRead` and `Redirect`), and other ones?
> 
> Yong Tang wrote:
> Hi Benjamin, thanks for the suggestion. I splits IOTest into two test 
> suites. Those tests that will not write files to disk remains the same while 
> the rests (BufferedRead and Redirect) have been moved to 
> TemporaryDirectoryIOTest via TemporaryDirectoryTest fixture. Let me see if 
> there are any issues and I will fix it.

Hi Benjamin, are there any additional issues that need to be addressed in this 
review request? Feedback is appreciated.


- Yong


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


On March 4, 2016, 6:56 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44380/
> ---
> 
> (Updated March 4, 2016, 6:56 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4807
> https://issues.apache.org/jira/browse/MESOS-4807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit changes IOTest.BufferedRead so that this specific
> test (not all IOTest) could be executed from temporary
> directories via TemporaryDirectoryTest fixture (MESOS-4807).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 2bffc7cd9c3aa204a1d1b8eb45f0bff12f49ca62 
> 
> Diff: https://reviews.apache.org/r/44380/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

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

[Kevin Klues]

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

(Updated March 14, 2016, 1:12 a.m.)


Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


Changes
---

Addressed all of bmahler's comments.


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 (updated)
-

  configure.ac 8e4f03593df4a8ba13f00292963e351acc3f71c1 

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


Testing (updated)
---

I ran `bootstrap` to generate configure.

I then ran:

```
mkdir build; cd build
../configure --enable-nvidia-gpu-support
../configure --enable-nvidia-gpu-support --with-nvml-include=
../configure --enable-nvidia-gpu-support --with-nvml-include=
../configure --enable-nvidia-gpu-support --with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
../configure --enable-nvidia-gpu-support --with-nvml-include= 
--with-nvml-lib=
```

And verified the proper errors / successes in each case (only the last one is a 
success).

The exact command I ran in the success case for my configuration was:
```
../configure --enable-nvidia-gpu-support 
--with-nvml-include=/opt/nvidia-gdk/usr/include 
--with-nvml-lib=/opt/nvidia-gdk/usr/src/gdk/nvml/lib
```


Thanks,

Kevin Klues

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

[Kevin Klues]

> On March 8, 2016, 7:33 p.m., Ben Mahler wrote:
> > configure.ac, lines 957-976
> > 
> >
> > Could we flatten this to make it a bit easier to read?

There are alot of examples in configure.ac that still do it this way (e.g.:

```
AC_CHECK_HEADERS([event2/event.h],
 [AC_CHECK_LIB([event],
 ...
```

However, I agree that splitting them up is clearer and should still be correct 
since we always error if we can't find the headers.

Updated.


> On March 8, 2016, 7:33 p.m., Ben Mahler wrote:
> > 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).

Done.


- Kevin


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


On March 14, 2016, 1:12 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44361/
> ---
> 
> (Updated March 14, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> 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 8e4f03593df4a8ba13f00292963e351acc3f71c1 
> 
> Diff: https://reviews.apache.org/r/44361/diff/
> 
> 
> Testing
> ---
> 
> I ran `bootstrap` to generate configure.
> 
> I then ran:
> 
> ```
> mkdir build; cd build
> ../configure --enable-nvidia-gpu-support
> ../configure --enable-nvidia-gpu-support --with-nvml-include=
> ../configure --enable-nvidia-gpu-support --with-nvml-include=
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support --with-nvml-include= 
> --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include= --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support --with-nvml-include= 
> --with-nvml-lib=
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include= --with-nvml-lib=
> ```
> 
> And verified the proper errors / successes in each case (only the last one is 
> a success).
> 
> The exact command I ran in the success case for my configuration was:
> ```
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include=/opt/nvidia-gdk/usr/include 
> --with-nvml-lib=/opt/nvidia-gdk/usr/src/gdk/nvml/lib
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Yong Tang]

> On March 13, 2016, 6:47 p.m., Benjamin Bannier wrote:
> > support/clang-format, line 2
> > 
> >
> > It would be great if you could use this opportunity and explicitly 
> > expand the values of the `Google` style we do rely on. This avoids the 
> > problem of upstream changing their definition of `Google` style, and then 
> > users on our end using different styles, depending on their clang version. 
> > This was an issue in the past with clang-format from trunk.
> > 
> > You should use `clang-format -dump-config` with some 3.5 and 3.8 
> > versions and adjust values where needed. As a starting point, I am using 
> > this config with clang-format from trunk (but cannot vouch for its quality),
> > 
> >   https://gist.github.com/bbannier/6e47d32eec9e0093711a

Thanks for the feedback Benjamin. I updated the clang-format file based on the 
Google style value expansion in 3.8. Please let me know if there are any other 
issues.


- Yong


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


On March 14, 2016, 1:09 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 14, 2016, 1:09 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Yong Tang]

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

(Updated March 14, 2016, 1:09 a.m.)


Review request for mesos and Michael Park.


Changes
---

Update clang-format by explicitly expanding the values of the Google style.


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


Repository: mesos


Description
---

Upgrade to clang-format-3.8 (MESOS-4906).


Diffs (updated)
-

  docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
  support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 

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


Testing
---

make check


Thanks,

Yong Tang

Re: Review Request 44765: Added description to endpoint help for frameworks and flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44286, 44186, 44621, 44711, 44764, 44765]

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 13, 2016, 9:29 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44765/
> ---
> 
> (Updated March 13, 2016, 9:29 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added description to endpoint help for frameworks and flags.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/flags.md 739d6bfae5ae35223d6bc7c4782ff0f16b868018 
>   docs/endpoints/master/frameworks.md 
> 95c1f0e63f6a61d0e6622a8ec9d821a646656cfc 
>   src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 
> 
> Diff: https://reviews.apache.org/r/44765/diff/
> 
> 
> Testing
> ---
> 
> Viewed in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44583: Reran `generate-endpoint-help.py` script for `/weights` endpoint.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On March 9, 2016, 6:04 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44583/
> ---
> 
> (Updated March 9, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reran `generate-endpoint-help.py` script for `/weights` endpoint.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md bd70819b186c33718f66830034979d415767a2a3 
>   docs/endpoints/master/weights.md PRE-CREATION 
>   src/master/http.cpp 7304bfd5350d763d9ed1d5acdc285874b6d8f5df 
> 
> Diff: https://reviews.apache.org/r/44583/diff/
> 
> 
> Testing
> ---
> 
> Previewed via site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Kevin Klues]

> 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.
> 
> Ben Mahler wrote:
> Could you update the testing done so that others can tell how you tested 
> this?

Done.


> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > 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.

I added everything you mentioned here in a comment except for mentioning why we 
tie this to a particular version. Instead, I mention that the script downloads 
the GDK for Nvidia's latest drivers (which should always be backwards 
compatible). So long as we keep the script up to date, we shouldn't have to 
worry about versioning.

Also, given what I said above, I think we can hold off mentioning anything 
about Nvidia discontinuing the GDK in favor of just updating this script to 
properly install the equivalent stub libraries when the time comes.


> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > support/install-nvidia-gdk.sh, line 21
> > 
> >
> > Should this include the ldcache flag as well?

Done.


> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > 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).

Done.


> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > support/install-nvidia-gdk.sh, line 95
> > 
> >
> > I probably wouldn't know what "invalid" means in this context. :)

Updated.


> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > 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?

Done.


> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > support/install-nvidia-gdk.sh, lines 102-104
> > 
> >
> > Maybe a small note about why we use `--silent` here?

Done.


> On March 8, 2016, 7:15 p.m., Ben Mahler wrote:
> > 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?

Done.


- Kevin


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


On March 14, 2016, 12:25 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44360/
> ---
> 
> (Updated March 14, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4860
> https://issues.apache.org/jira/browse/MESOS-4860
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script can be used to install the Nvidia GPU Deployment Kit (GDK)
> on a mesos development machine. The purpose of the GDK is to provide
> stubs for compiling code against Nvidia's latest drivers.  It includes
> all the necessary header files (nvml.h) and library files
> (libnvidia-ml.so) necessary to build mesos with Nvidia GPU support.
> However, it does not actually contain any driver code, and it can't be
> used to interact with any GPUs installed on a system. For that, you
> will have to find the latest released drivers themselves.
> 
> We provide this script only for convenience, so that developers
> working on non-GPU equipped systems can build code with Nvidia GPU
> support. It is a simple wrapper around the official GDK installer
> script provided by Nvidia, which can be downloaded here:
> 
>   https://developer.nvidia.com/gpu-deployment-kit
> 
> The 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 

Re: Review Request 44768: Included weights endpoint in endpoint documentation.

[Neil Conway]

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



I already have a patch out for this -- https://reviews.apache.org/r/44583/

- Neil Conway


On March 13, 2016, 9:37 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44768/
> ---
> 
> (Updated March 13, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos, Adam B, Kevin Klues, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Even though the weights endpoint had been added with a corresponding HELP(), 
> the documentation had not been regenerated.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md 27b178efa1c2d1a00f557018e3343293ebfb3430 
>   docs/endpoints/master/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44768/diff/
> 
> 
> Testing
> ---
> 
> Viewed generated docs.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

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

[Kevin Klues]

> On March 6, 2016, 12:51 a.m., Klaus Ma wrote:
> > support/install-nvidia-gdk.sh, lines 3-13
> > 
> >
> > Are we going to provide documents about those vars? e.g. which version 
> > are we going to support?

>From talking to Nvidia, this is actually the last version of the GDK they plan 
>to release in this form.  It's only purpose is to provide a stub library so 
>that you can build code without a full CUDA install.  Moreover, this is just a 
>helper to make it a little easier to get the tools installed on your system. 
>In fact, if you already have CUDA installed, I would recommend *against* using 
>this script.  I am adding a big comment at the top of the script to mention 
>all of this.


- Kevin


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


On March 14, 2016, 12:25 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44360/
> ---
> 
> (Updated March 14, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4860
> https://issues.apache.org/jira/browse/MESOS-4860
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This script can be used to install the Nvidia GPU Deployment Kit (GDK)
> on a mesos development machine. The purpose of the GDK is to provide
> stubs for compiling code against Nvidia's latest drivers.  It includes
> all the necessary header files (nvml.h) and library files
> (libnvidia-ml.so) necessary to build mesos with Nvidia GPU support.
> However, it does not actually contain any driver code, and it can't be
> used to interact with any GPUs installed on a system. For that, you
> will have to find the latest released drivers themselves.
> 
> We provide this script only for convenience, so that developers
> working on non-GPU equipped systems can build code with Nvidia GPU
> support. It is a simple wrapper around the official GDK installer
> script provided by Nvidia, which can be downloaded here:
> 
>   https://developer.nvidia.com/gpu-deployment-kit
> 
> The 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
> ---
> 
> I ran the script on my Mac to verify that it fails out cleanly.
> 
> I ran it on a linux box to exercise each of the different flags that can be 
> passed to it.
> ```
> install-nvidia-gdk.sh --install-dir=
> install-nvidia-gdk.sh --install-dir=
> install-nvidia-gdk.sh --install-dir= --update-ldcache
> install-nvidia-gdk.sh --install-dir= --update-ldcache
> ```
> 
> With each combination, I verified that the proper error messages were 
> reported, or that the installation was successful.
> 
> As per one of the comments below, I also temporarily changed the download URL 
> for the installer to make sure that the `wget` call errored out cleanly.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

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

[Kevin Klues]

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

(Updated March 14, 2016, 12:25 a.m.)


Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.


Changes
---

Addressed all of klaus1982 and bmahler's comment.


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


Repository: mesos


Description (updated)
---

This script can be used to install the Nvidia GPU Deployment Kit (GDK)
on a mesos development machine. The purpose of the GDK is to provide
stubs for compiling code against Nvidia's latest drivers.  It includes
all the necessary header files (nvml.h) and library files
(libnvidia-ml.so) necessary to build mesos with Nvidia GPU support.
However, it does not actually contain any driver code, and it can't be
used to interact with any GPUs installed on a system. For that, you
will have to find the latest released drivers themselves.

We provide this script only for convenience, so that developers
working on non-GPU equipped systems can build code with Nvidia GPU
support. It is a simple wrapper around the official GDK installer
script provided by Nvidia, which can be downloaded here:

  https://developer.nvidia.com/gpu-deployment-kit

The script itself takes a few parameters. Run it as
'support/install-nvidia-gdk.sh --help' to see its usage semantics.


Diffs (updated)
-

  support/install-nvidia-gdk.sh PRE-CREATION 

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


Testing (updated)
---

I ran the script on my Mac to verify that it fails out cleanly.

I ran it on a linux box to exercise each of the different flags that can be 
passed to it.
```
install-nvidia-gdk.sh --install-dir=
install-nvidia-gdk.sh --install-dir=
install-nvidia-gdk.sh --install-dir= --update-ldcache
install-nvidia-gdk.sh --install-dir= --update-ldcache
```

With each combination, I verified that the proper error messages were reported, 
or that the installation was successful.

As per one of the comments below, I also temporarily changed the download URL 
for the installer to make sure that the `wget` call errored out cleanly.


Thanks,

Kevin Klues

Re: Review Request 44768: Included weights endpoint in endpoint documentation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44768]

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 13, 2016, 9:37 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44768/
> ---
> 
> (Updated March 13, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos, Adam B, Kevin Klues, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Even though the weights endpoint had been added with a corresponding HELP(), 
> the documentation had not been regenerated.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md 27b178efa1c2d1a00f557018e3343293ebfb3430 
>   docs/endpoints/master/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44768/diff/
> 
> 
> Testing
> ---
> 
> Viewed generated docs.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44768: Included weights endpoint in endpoint documentation.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On March 13, 2016, 9:37 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44768/
> ---
> 
> (Updated March 13, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos, Adam B, Kevin Klues, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Even though the weights endpoint had been added with a corresponding HELP(), 
> the documentation had not been regenerated.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md 27b178efa1c2d1a00f557018e3343293ebfb3430 
>   docs/endpoints/master/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44768/diff/
> 
> 
> Testing
> ---
> 
> Viewed generated docs.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44767: Added authentication information to master endpoints.

[Joerg Schad]

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

(Updated March 13, 2016, 10:33 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Added mesos.


Repository: mesos


Description
---

Master endpoints now display information whether authentication (if enabled) is 
required in HELP.


Diffs
-

  docs/endpoints/master/api/v1/scheduler.md 
7a75fd75eef458f26f9e5067b792329f310d5d24 
  docs/endpoints/master/create-volumes.md 
2ab116a16581fc5cfc0fc5cf70c6e001a28d8493 
  docs/endpoints/master/destroy-volumes.md 
88679fe12baf7642b63e71688b90031aaedab1ef 
  docs/endpoints/master/flags.md 739d6bfae5ae35223d6bc7c4782ff0f16b868018 
  docs/endpoints/master/frameworks.md 95c1f0e63f6a61d0e6622a8ec9d821a646656cfc 
  docs/endpoints/master/health.md 7f9352025a6364b812b91e9ccbe380a3a69ba4e1 
  docs/endpoints/master/machine/down.md 
6eec9ecc6e92fd1299b27499f78055c6f4f68e81 
  docs/endpoints/master/machine/up.md 7cbbf04859a218f90672453e5037480676f79fe5 
  docs/endpoints/master/maintenance/schedule.md 
2b5e782554136c1624483d8155a96472ec0660dd 
  docs/endpoints/master/maintenance/status.md 
4d0c7551acb89fb375834fd703c406d68f8bdcfc 
  docs/endpoints/master/observe.md fae1ee062350d7b25775bef98a0bebda1892ab62 
  docs/endpoints/master/quota.md 812874d2dd6c3548887e3044ba1f3c3c8c9d1dd6 
  docs/endpoints/master/redirect.md ac9d0fa3eae485726b10a3ac756228d0cb5aeb27 
  docs/endpoints/master/reserve.md 1a4f67961baf761f79a693780f42a1a8ce2244fc 
  docs/endpoints/master/roles.json.md 863715386791ef9192f38bf390fcd2b31d988547 
  docs/endpoints/master/roles.md 171e0163c4c2db34bf34c8303846793f3c29bdf5 
  docs/endpoints/master/slaves.md ec169c15507d3c731b8833f2656949caf0efcc33 
  docs/endpoints/master/state-summary.md 
fb10ac7db5b94ea7982c245f1d884312d818c6b5 
  docs/endpoints/master/state.json.md 0415cfdd63cee0b350b3e33c496bacfc1ba53f8a 
  docs/endpoints/master/state.md ae1ec718b8d718718727dbd2e7ce4739cca41680 
  docs/endpoints/master/tasks.json.md 46a1253d33cdfdca0a7158808d1e29da1a634933 
  docs/endpoints/master/tasks.md 2ada97b5dfdaf5f1f7578fa42eb4e61233e6a753 
  docs/endpoints/master/teardown.md f68d083a4271ed7dd0ddcc87da104a43eb40c7ec 
  docs/endpoints/master/unreserve.md e059d8d888343c5036346f7a615f04375f44f517 
  docs/endpoints/master/weights.md PRE-CREATION 
  src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 

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


Testing
---

Viewed master endpoint help in browser.


Thanks,

Joerg Schad

Re: Review Request 44766: Enabled Authentication information in endpoint HELP.

[Joerg Schad]

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

(Updated March 13, 2016, 10:31 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Corrected order in documentation.


Repository: mesos


Description
---

Enabled endpoint HELP string to display information about authentication 
requirements.


Diffs (updated)
-

  3rdparty/libprocess/include/process/help.hpp 
783304e2fc78db70f1a5fccbf5e96fcc76a88fd8 
  3rdparty/libprocess/src/help.cpp 5f368801affecacb0d1daaeb6ccf5895ccb231d2 
  3rdparty/libprocess/src/logging.cpp 015a43db77fd7015aeee0c45fa10c292f3e9cf58 

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


Testing
---

Viewed master endpoint help in browser.


Thanks,

Joerg Schad

Review Request 44768: Included weights endpoint in endpoint documentation.

[Joerg Schad]

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

Review request for mesos, Adam B, Kevin Klues, and Neil Conway.


Repository: mesos


Description
---

Even though the weights endpoint had been added with a corresponding HELP(), 
the documentation had not been regenerated.


Diffs
-

  docs/endpoints/index.md 27b178efa1c2d1a00f557018e3343293ebfb3430 
  docs/endpoints/master/weights.md PRE-CREATION 

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


Testing
---

Viewed generated docs.


Thanks,

Joerg Schad

Re: Review Request 44765: Added description to endpoint help for frameworks and flags.

[Joerg Schad]

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

(Updated March 13, 2016, 9:29 p.m.)


Review request for mesos, Adam B and Neil Conway.


Changes
---

Generated endpoint documentation.


Repository: mesos


Description (updated)
---

Added description to endpoint help for frameworks and flags.


Diffs (updated)
-

  docs/endpoints/master/flags.md 739d6bfae5ae35223d6bc7c4782ff0f16b868018 
  docs/endpoints/master/frameworks.md 95c1f0e63f6a61d0e6622a8ec9d821a646656cfc 
  src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 

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


Testing
---

Viewed in browser.


Thanks,

Joerg Schad

Re: Review Request 44765: Added description to endpoint help for frameworks and flags.

[Joerg Schad]

> On March 13, 2016, 8:40 p.m., Kevin Klues wrote:
> > Can you also run the support/generate-endpoint-ehlp.py sript to regenerate 
> > the mardown files for the endpoints help?

Already considered that :-).


- Joerg


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


On March 13, 2016, 8:21 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44765/
> ---
> 
> (Updated March 13, 2016, 8:21 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> All other endpoints provide a description which specifies at least the type 
> of object returned.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 
> 
> Diff: https://reviews.apache.org/r/44765/diff/
> 
> 
> Testing
> ---
> 
> Viewed in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44765: Added description to endpoint help for frameworks and flags.

[Kevin Klues]

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



Can you also run the support/generate-endpoint-ehlp.py sript to regenerate the 
mardown files for the endpoints help?

- Kevin Klues


On March 13, 2016, 8:21 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44765/
> ---
> 
> (Updated March 13, 2016, 8:21 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> All other endpoints provide a description which specifies at least the type 
> of object returned.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 
> 
> Diff: https://reviews.apache.org/r/44765/diff/
> 
> 
> Testing
> ---
> 
> Viewed in browser.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 44765: Added description to endpoint help for frameworks and flags.

[Joerg Schad]

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

Review request for mesos, Adam B and Neil Conway.


Repository: mesos


Description
---

All other endpoints provide a description which specifies at least the type of 
object returned.


Diffs
-

  src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 

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


Testing
---

Viewed in browser.


Thanks,

Joerg Schad

Review Request 44764: Made 'framework' endpoint help string consistent.

[Joerg Schad]

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

Review request for mesos, Adam B and Neil Conway.


Repository: mesos


Description
---

All other endpoint HELP functions are already named 'endpoint'_HELP().


Diffs
-

  src/master/http.cpp 6dec322ca4f7d81c2f070db4f54a0575a2b5c842 
  src/master/master.hpp 7b5139115f306d8faef7e41bb231e1e1671b8120 
  src/master/master.cpp 255b4d1ba029d7f9cfb86138c66a25d0dffa30e0 

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


Testing
---

Viewed endpoint help in browser.


Thanks,

Joerg Schad

Re: Review Request 44711: Updated authentication.md after most endpoints enable authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44286, 44186, 44621, 44711]

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 13, 2016, 6:54 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44711/
> ---
> 
> (Updated March 13, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-4844
> https://issues.apache.org/jira/browse/MESOS-4844
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reflected default authentication of endpoints in documentation.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md dd538b512c6f822cd742fc6e2f3d4f7fbffadc06 
> 
> Diff: https://reviews.apache.org/r/44711/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44711: Updated authentication.md after most endpoints enable authentication.

[Joerg Schad]

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

(Updated March 13, 2016, 6:54 p.m.)


Review request for mesos, Adam B and Greg Mann.


Changes
---

Addressed review.


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


Repository: mesos


Description
---

Reflected default authentication of endpoints in documentation.


Diffs (updated)
-

  docs/authentication.md dd538b512c6f822cd742fc6e2f3d4f7fbffadc06 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 44621: Added tests for http endpoints with bad authentication.

[Joerg Schad]

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

(Updated March 13, 2016, 6:50 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


Changes
---

Addressed review.


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


Repository: mesos


Description
---

With enabling http authentication for http endpoints we should also add tests 
to check
that http request to those endpoints return "401 Unauthorized" if queried 
without or with
bad credentials.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp 
5a421d4e8740b5105518283943087869025cbfa6 
  src/tests/master_tests.cpp e9ddd360fd87aed091f12a35f92df3f4e9c4190b 
  src/tests/repair_tests.cpp cb38bb1b6717c3eb003323278cf8f6902394be7c 
  src/tests/role_tests.cpp 2e23926143a2c09c6e1fee961a4ede6f7b3a275e 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Benjamin Bannier]

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




support/clang-format (line 2)


It would be great if you could use this opportunity and explicitly expand 
the values of the `Google` style we do rely on. This avoids the problem of 
upstream changing their definition of `Google` style, and then users on our end 
using different styles, depending on their clang version. This was an issue in 
the past with clang-format from trunk.

You should use `clang-format -dump-config` with some 3.5 and 3.8 versions 
and adjust values where needed. As a starting point, I am using this config 
with clang-format from trunk (but cannot vouch for its quality),

  https://gist.github.com/bbannier/6e47d32eec9e0093711a


- Benjamin Bannier


On March 13, 2016, 5:15 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 13, 2016, 5:15 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44762: [WIP][PROPOSAL_1]Add CgroupsIsolator. This is only used for discussion.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44762]

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

Error:
2016-03-13 18:26:30 URL:https://reviews.apache.org/r/44762/diff/raw/ 
[36134/36134] -> "44762.patch" [1]
Total errors found: 0
Checking 6 files
Error: Commit message summary (the first line) must start with a capital letter.

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

- Mesos ReviewBot


On March 13, 2016, 5:50 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44762/
> ---
> 
> (Updated March 13, 2016, 5:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4697
> https://issues.apache.org/jira/browse/MESOS-4697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CgroupsIsolator. This is only used for discussion.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/info.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/info.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44762/diff/
> 
> 
> Testing
> ---
> 
> The relations between classes in this proposal is:
> 
> ```
> ++
> ||
> | CgroupsIsolatorProcess <-+
> || |
> +^---+ +
>  |   Belongs to
>  + +
> Belongs to |
>  + |
>  | |
>+-+-+   +---+-+
>|   |   | |
>| Subsystem <---+Used by+---+ CgroupsIsolatorInfo |
>|   |   | |
>+---+   +-+
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 44762: [WIP][PROPOSAL_1]Add CgroupsIsolator. This is only used for discussion.

[haosdent huang]

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




src/slave/containerizer/mesos/isolators/cgroups/info.hpp (line 55)


Another problem here is could not avoid different threads to modify a same 
`CgroupsIsolatorInfo` object in a easy way.


- haosdent huang


On March 13, 2016, 5:50 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44762/
> ---
> 
> (Updated March 13, 2016, 5:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4697
> https://issues.apache.org/jira/browse/MESOS-4697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CgroupsIsolator. This is only used for discussion.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/info.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/info.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44762/diff/
> 
> 
> Testing
> ---
> 
> The relations between classes in this proposal is:
> 
> ```
> ++
> ||
> | CgroupsIsolatorProcess <-+
> || |
> +^---+ +
>  |   Belongs to
>  + +
> Belongs to |
>  + |
>  | |
>+-+-+   +---+-+
>|   |   | |
>| Subsystem <---+Used by+---+ CgroupsIsolatorInfo |
>|   |   | |
>+---+   +-+
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 44761: [WIP][PROPOSAL_2]Add CgroupsIsolator. This is only used for discussion.

[haosdent huang]

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

(Updated March 13, 2016, 6:09 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Add CgroupsIsolator. This is only used for discussion.


Diffs
-

  src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
  src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing (updated)
---

The relations between classes in this proposal is:
```
   ++
   ||
   | CgroupsIsolatorProcess |
   ||
   +---^+
   |
   |
 Belongs to
   |
+--+---+
|  |
| CgroupsIsolatorProcess::Info |
|  |
+--^---+
   |
 Belongs to
   |
 +-+-+
 |   |
 | Subsystem |
 |   |
 +---+
```


Thanks,

haosdent huang

Re: Review Request 44762: [WIP][PROPOSAL_1]Add CgroupsIsolator. This is only used for discussion.

[haosdent huang]

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




src/slave/containerizer/mesos/isolators/cgroups/info.hpp (line 55)


Because both CgroupsIsolatorProcess and Subsystem need access 
CgroupsIsolatorInfo, I use struct here.



src/slave/containerizer/mesos/isolators/cgroups/info.hpp (line 75)


Could not use something like 
```
vector
```

here.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 63)


Because we set ContainerLimiation when oom or in some scenarios, I use 
WatchCallback here.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 93)


Because besides `assign`, sometimes we would like to register oom listener 
in mem subsytem, I move it from `CgroupsIsolatorProcess` to `Subsystem` as a 
default implementation. But it would assign to same hierarchy multiple times, 
should I move it back to `CgroupsIsolatorProcess`?


- haosdent huang


On March 13, 2016, 5:50 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44762/
> ---
> 
> (Updated March 13, 2016, 5:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4697
> https://issues.apache.org/jira/browse/MESOS-4697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CgroupsIsolator. This is only used for discussion.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/info.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/info.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44762/diff/
> 
> 
> Testing
> ---
> 
> The relations between classes in this proposal is:
> 
> ```
> ++
> ||
> | CgroupsIsolatorProcess <-+
> || |
> +^---+ +
>  |   Belongs to
>  + +
> Belongs to |
>  + |
>  | |
>+-+-+   +---+-+
>|   |   | |
>| Subsystem <---+Used by+---+ CgroupsIsolatorInfo |
>|   |   | |
>+---+   +-+
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 44761: [WIP][PROPOSAL_2]Add CgroupsIsolator. This is only used for discussion.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44761]

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

Error:
2016-03-13 17:56:36 URL:https://reviews.apache.org/r/44761/diff/raw/ 
[33841/33841] -> "44761.patch" [1]
Total errors found: 0
Checking 4 files
Error: Commit message summary (the first line) must start with a capital letter.

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

- Mesos ReviewBot


On March 13, 2016, 5:35 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44761/
> ---
> 
> (Updated March 13, 2016, 5:35 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4697
> https://issues.apache.org/jira/browse/MESOS-4697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CgroupsIsolator. This is only used for discussion.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44761/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 44762: [WIP][PROPOSAL_1]Add CgroupsIsolator. This is only used for discussion.

[haosdent huang]

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

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


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Add CgroupsIsolator. This is only used for discussion.


Diffs
-

  src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
  src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/info.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/info.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing (updated)
---

The relations between classes in this proposal is:

```
++
||
| CgroupsIsolatorProcess <-+
|| |
+^---+ +
 |   Belongs to
 + +
Belongs to |
 + |
 | |
   +-+-+   +---+-+
   |   |   | |
   | Subsystem <---+Used by+---+ CgroupsIsolatorInfo |
   |   |   | |
   +---+   +-+
```


Thanks,

haosdent huang

Review Request 44762: [WIP][PROPOSAL_1]Add CgroupsIsolator. This is only used for discussion.

[haosdent huang]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Add CgroupsIsolator. This is only used for discussion.


Diffs
-

  src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
  src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/info.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/info.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Review Request 44761: [WIP][PROPOSAL_2]Add CgroupsIsolator. This is only used for discussion.

[haosdent huang]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Add CgroupsIsolator. This is only used for discussion.


Diffs
-

  src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
  src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44621: Added tests for http endpoints with bad authentication.

[Joerg Schad]

> On March 13, 2016, 8:26 a.m., Adam B wrote:
> > src/tests/master_maintenance_tests.cpp, line 1786
> > 
> >
> > You can't use `badAuthnHeaders` here, because of the content-type?

Yes.


> On March 13, 2016, 8:26 a.m., Adam B wrote:
> > src/tests/master_tests.cpp, line 4265
> > 
> >
> > There is no GET request allowed on /weights (yet), so it's interesting 
> > to me that this part of the test passes. I would expect it to return 405 
> > MethodNotAllowed. Or I guess authentication happens before we even get to 
> > the handler to check the action?

I removed the test for now (especially as there is no http::put yet). Will add 
that with a follow up patch.


- Joerg


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


On March 11, 2016, 7:49 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44621/
> ---
> 
> (Updated March 11, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4844
> https://issues.apache.org/jira/browse/MESOS-4844
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With enabling http authentication for http endpoints we should also add tests 
> to check
> that http request to those endpoints return "401 Unauthorized" if queried 
> without or with
> bad credentials.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp 
> 3c7024cfbd7e5bef75f092eace8d0e8ca423 
>   src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 
>   src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
>   src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
> 
> Diff: https://reviews.apache.org/r/44621/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

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

[Joerg Schad]

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


Fix it, then Ship it!





src/master/constants.hpp (line 136)


Should we make it "master HTTP authentication realm."?


- Joerg Schad


On March 11, 2016, 10:04 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44523/
> ---
> 
> (Updated March 11, 2016, 10:04 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, 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.hpp 2c3299bc1aaa0888f7e47a71965c56ada8ecc21f 
>   src/master/constants.cpp e316f9772d880b694faeee6d001dc56bc088c118 
> 
> Diff: https://reviews.apache.org/r/44523/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42806: Added the fetcher plugin module interface.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42806]

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 13, 2016, 1:09 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42806/
> ---
> 
> (Updated March 13, 2016, 1:09 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3926
> https://issues.apache.org/jira/browse/MESOS-3926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the fetcher plugin module interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/fetcher_plugin.hpp PRE-CREATION 
>   src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
>   src/examples/test_fetcher_plugin_module.cpp PRE-CREATION 
>   src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
>   src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
>   src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 
>   src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
>   src/uri/fetcher.cpp 8645b66f6c64c76b6c02ef0b9827a7d694d5ba97 
> 
> Diff: https://reviews.apache.org/r/42806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 44514: Implemented prepare() method of "network/cni" isolator.

[Qian Zhang]

> On March 12, 2016, 2:19 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 228
> > 
> >
> > Can there be a use case where you want multiple NICs to be attached to 
> > the same network? Servers use this configuration when they want to utilize 
> > NIC bonding. To aggregate the bandwidth available on the NICs

Good point! I think it makes sense that user enables NIC bonding by creating a 
bond device (e.g., bond0) as the master of the normal ethernet devices (e.g., 
eth0 and eth1), and both eth0 and eth1 are set up by CNI plugin and get 
assigned IP from CNI plugin in the same subnet. My only concern is, how to 
configure the IP for bond0, maybe just use IP of either eth0 or eth1 as its IP?


> On March 12, 2016, 2:19 a.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 236
> > 
> >
> > If the CNI configuration is host-local then we will be calling the 
> > host-local plugin during `isolate`. So why do we need this?

The purpose of this comment is, when we support to explicitly request an IP for 
container in future, here we need to get `NetworkInfo.ip_addresses` to know 
which IP the container want to request, and keep it in the isolator (e.g., in 
the Info struct for the container), and later in isolate() method, we need to 
set the value of the environment variable `CNI_ARGS` to that IP when invoking 
plugin, see 
https://github.com/appc/cni/blob/master/Documentation/host-local.md#supported-arguments
 for more details.


- Qian


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


On March 10, 2016, 10:20 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44514/
> ---
> 
> (Updated March 10, 2016, 10:20 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 prepare() method of "network/cni" isolator.
> 
> 
> 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 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Joerg Schad]

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

(Updated March 13, 2016, 1:29 p.m.)


Review request for mesos, Adam B and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Previously the app-framework-development-guide only explained the scheduler 
callback interface. Equally important when developing frameworks is the 
knowledge if potential actions a scheduler can trigger via the SchedulerDriver.


Diffs (updated)
-

  docs/app-framework-development-guide.md 
55f09c7cf2262dda98dcc8b63b1e36ab78296141 

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


Testing
---

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


Thanks,

Joerg Schad

Re: Review Request 42806: Added the fetcher plugin module interface.

[Shuai Lin]

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

(Updated March 13, 2016, 1:09 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased to lastest master.


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


Repository: mesos


Description
---

Added the fetcher plugin module interface.


Diffs (updated)
-

  include/mesos/module/fetcher_plugin.hpp PRE-CREATION 
  src/Makefile.am f2a592dba3e963c99c48c4b9045b4a04d173cb22 
  src/examples/test_fetcher_plugin_module.cpp PRE-CREATION 
  src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 
  src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
  src/uri/fetcher.cpp 8645b66f6c64c76b6c02ef0b9827a7d694d5ba97 

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


Testing
---

make check


Thanks,

Shuai Lin

Review Request 44590: Tests fixup.

[Greg Mann]

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

Review request for mesos.


Repository: mesos


Description
---

Tests fixup.


Diffs
-


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 44583: Reran `generate-endpoint-help.py` script for `/weights` endpoint.

[Adam B]

> On March 13, 2016, 4:31 a.m., Adam B wrote:
> > Ship It!

Sorry, needs a rebase after https://reviews.apache.org/r/44693/


- Adam


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


On March 9, 2016, 10:04 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44583/
> ---
> 
> (Updated March 9, 2016, 10:04 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reran `generate-endpoint-help.py` script for `/weights` endpoint.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md bd70819b186c33718f66830034979d415767a2a3 
>   docs/endpoints/master/weights.md PRE-CREATION 
>   src/master/http.cpp 7304bfd5350d763d9ed1d5acdc285874b6d8f5df 
> 
> Diff: https://reviews.apache.org/r/44583/diff/
> 
> 
> Testing
> ---
> 
> Previewed via site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44584: Improved docs for dynamic weights.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 10, 2016, 10:40 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44584/
> ---
> 
> (Updated March 10, 2016, 10:40 a.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved docs for dynamic weights.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md f6e84023b90e560594429826ed7163310d62b265 
>   docs/roles.md 65d6ddf46ac394389c70a3faf5cc85c5cf892478 
>   docs/weights.md dec2ddd6516d2d3a9926f6410f8309eb2de40c3c 
>   src/master/flags.cpp c1dd127109f1ba96a8f9b95f3eb99dfeb43f7d28 
> 
> Diff: https://reviews.apache.org/r/44584/diff/
> 
> 
> Testing
> ---
> 
> Previewed via site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44583: Reran `generate-endpoint-help.py` script for `/weights` endpoint.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On March 9, 2016, 10:04 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44583/
> ---
> 
> (Updated March 9, 2016, 10:04 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reran `generate-endpoint-help.py` script for `/weights` endpoint.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md bd70819b186c33718f66830034979d415767a2a3 
>   docs/endpoints/master/weights.md PRE-CREATION 
>   src/master/http.cpp 7304bfd5350d763d9ed1d5acdc285874b6d8f5df 
> 
> Diff: https://reviews.apache.org/r/44583/diff/
> 
> 
> Testing
> ---
> 
> Previewed via site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43819: Added Scheduler-Driver API to app-framework-development-guide.md.

[Adam B]

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


Fix it, then Ship it!




Looks great. Fix it, then I'll commit.


docs/app-framework-development-guide.md (line 136)


s/necessarily/usually/ It's only implemented by a framework if it's not 
using libmesos, right?



docs/app-framework-development-guide.md (lines 324 - 325)


Looks like this fits on one line now. I wonder if others will too.


- Adam B


On March 7, 2016, 12:14 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43819/
> ---
> 
> (Updated March 7, 2016, 12:14 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-4726
> https://issues.apache.org/jira/browse/MESOS-4726
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the app-framework-development-guide only explained the scheduler 
> callback interface. Equally important when developing frameworks is the 
> knowledge if potential actions a scheduler can trigger via the 
> SchedulerDriver.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> 55f09c7cf2262dda98dcc8b63b1e36ab78296141 
> 
> Diff: https://reviews.apache.org/r/43819/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist (https://gist.github.com/joerg84/b4bf279a55e1b62051e6) and 
> via docker website container.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44620: Documented how to make executors work with SSL.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44620]

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 11, 2016, 7:11 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44620/
> ---
> 
> (Updated March 11, 2016, 7:11 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4750
> https://issues.apache.org/jira/browse/MESOS-4750
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented how to make executors work with SSL.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md 3de2a3e931091e002dc4b259c70eadd89a52b059 
> 
> Diff: https://reviews.apache.org/r/44620/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44672: Added normalize method to registry puller.

[Guangya Liu]

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



Can you please link https://issues.apache.org/jira/browse/MESOS-4877 to this RR?

- Guangya Liu


On 三月 10, 2016, 11:49 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44672/
> ---
> 
> (Updated 三月 10, 2016, 11:49 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added normalize method to registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6d637ed14f35feb554c8fcc63a7a7e046aaca574 
> 
> Diff: https://reviews.apache.org/r/44672/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh 
> --gtest_filter="*ProvisionerDockerRegistryPullerTest*"
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

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/#review123306
---


Ship it!




Ship It!

- Adam B


On March 11, 2016, 2:04 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44523/
> ---
> 
> (Updated March 11, 2016, 2:04 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, 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.hpp 2c3299bc1aaa0888f7e47a71965c56ada8ecc21f 
>   src/master/constants.cpp e316f9772d880b694faeee6d001dc56bc088c118 
> 
> Diff: https://reviews.apache.org/r/44523/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44620: Documented how to make executors work with SSL.

[Adam B]

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


Ship it!




Looks good, but I'll clean it up a bit before committing, if you don't mind:

Executors must be able to access the SSL environment variables and the files 
referred to by those variables. Environment variables can be provided to an 
executor by specifying `CommandInfo.environment` or by using the slave's 
`--executor_environment_variables` command line flag. If the slave and the 
executor are running in separate containers, `ContainerInfo.volumes` can be 
used to mount SSL files from the host into the executor's container.

- Adam B


On March 11, 2016, 11:11 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44620/
> ---
> 
> (Updated March 11, 2016, 11:11 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4750
> https://issues.apache.org/jira/browse/MESOS-4750
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented how to make executors work with SSL.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md 3de2a3e931091e002dc4b259c70eadd89a52b059 
> 
> Diff: https://reviews.apache.org/r/44620/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44711: Updated authentication.md after most endpoints enable authentication.

[Adam B]

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



Slow down, partner. Even after Greg's patch to add authn to basic agent HTTP 
endpoints, there are still a lot of unauthenticated endpoints.


docs/authentication.md (line 12)


I wouldn't call it "most" yet. Maybe "many". By my count, we're still 
excluding all the agent endpoints, plus:
/registrar(id)/registry
/files/*
/logging
/metrics/snapshot 
/profiler/*
/system/stats.json
/help/*

Can we also update the HTTP endpoint help to specify that authentication is 
required for endpoints when it is? (Maybe even base the answer on the actual 
--authenticate_http value)


- Adam B


On March 11, 2016, 8:48 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44711/
> ---
> 
> (Updated March 11, 2016, 8:48 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Bugs: MESOS-4844
> https://issues.apache.org/jira/browse/MESOS-4844
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reflected default authentication of endpoints in documentation.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md dd538b512c6f822cd742fc6e2f3d4f7fbffadc06 
> 
> Diff: https://reviews.apache.org/r/44711/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 44621: Added tests for http endpoints with bad authentication.

[Adam B]

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



Looks good. Just some minor style points to clean up, and then we can ship it


src/tests/master_maintenance_tests.cpp (line 1749)


s/POST's/POSTs/ - it's plural, not possessive



src/tests/master_maintenance_tests.cpp (line 1750)


Sorry, but we don't like abbrevs in variable names. How about 
"unauthenticatedHeaders" or "anonymousHeaders"?



src/tests/master_maintenance_tests.cpp (line 1757)


Seems weird that this one doesn't get a pre-definition comment.



src/tests/master_maintenance_tests.cpp (line 1761)


s/POST's/POSTs/



src/tests/master_maintenance_tests.cpp (line 1762)


No abbreviations, so "badAuthenticationHeaders"?



src/tests/master_maintenance_tests.cpp (lines 1767 - 1768)


Comment: // Post the maintenance schedule without authentication.



src/tests/master_maintenance_tests.cpp (lines 1768 - 1781)


Could you please be consistent about testing POST first or GET first?



src/tests/master_maintenance_tests.cpp (line 1786)


You can't use `badAuthnHeaders` here, because of the content-type?



src/tests/master_maintenance_tests.cpp (line 1821)


s/up/down/



src/tests/master_tests.cpp (line 4195)


"Test get requests on various endpoints without..."



src/tests/master_tests.cpp (line 4265)


There is no GET request allowed on /weights (yet), so it's interesting to 
me that this part of the test passes. I would expect it to return 405 
MethodNotAllowed. Or I guess authentication happens before we even get to the 
handler to check the action?



src/tests/repair_tests.cpp (line 225)


Any reason you only test get, but not post?



src/tests/repair_tests.cpp (line 226)


s/EndpointBadAuthentication/ObserveEndpointBadAuthentication/ since 
`HealthTest` sounds too generic to just apply to the observe endpoint.


- Adam B


On March 11, 2016, 11:49 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44621/
> ---
> 
> (Updated March 11, 2016, 11:49 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4844
> https://issues.apache.org/jira/browse/MESOS-4844
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With enabling http authentication for http endpoints we should also add tests 
> to check
> that http request to those endpoints return "401 Unauthorized" if queried 
> without or with
> bad credentials.
> 
> 
> Diffs
> -
> 
>   src/tests/master_maintenance_tests.cpp 
> 3c7024cfbd7e5bef75f092eace8d0e8ca423 
>   src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 
>   src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
>   src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
> 
> Diff: https://reviews.apache.org/r/44621/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>