Re: Review Request 46497: Added authentication to the '/profiler/*' endpoints.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On April 25, 2016, 4:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46497/
> ---
> 
> (Updated April 25, 2016, 4:02 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authentication to the '/profiler/*' endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/profiler.hpp 
> b305f1148ad6b623c75facc4abd0269c13017bab 
>   3rdparty/libprocess/src/process.cpp 
> 8727eb202e9699f0ac3c95788257cf1a22b0da7b 
>   3rdparty/libprocess/src/profiler.cpp 
> 58cf2fec119e65a74a3c75561ba0866ec88897d0 
> 
> Diff: https://reviews.apache.org/r/46497/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX. The endpoints were also tested by configuring with 
> `../configure --enable-perftools`, and then hitting `/profiler/start` and 
> `/profiler/stop` with the `LIBPROCESS_ENABLE_PROFILER` environment variable 
> set to 1.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46259: Added authentication to `/metrics/snapshot` endpoint.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 25, 2016, 10:21 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46259/
> ---
> 
> (Updated April 25, 2016, 10:21 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds authentication to the `/metrics/snapshot`
> endpoint on the master and agent.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 7abfadf6d030c52b7e03e773b67c74db6ad5b5df 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 7677dffed98153bc6276aad492a6815e67740ea3 
>   3rdparty/libprocess/src/process.cpp 
> 8727eb202e9699f0ac3c95788257cf1a22b0da7b 
> 
> Diff: https://reviews.apache.org/r/46259/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 45922: Added agent authorization flags.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On April 22, 2016, 12:11 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> ---
> 
> (Updated April 22, 2016, 12:11 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956 
>   src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea 
>   src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
>   src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4 
>   src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
>   src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 
> 
> Diff: https://reviews.apache.org/r/45922/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.

[Benjamin Bannier]

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

(Updated April 25, 2016, 4:16 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.


Changes
---

Clearly separated async/sync parts of handling request for statistics.


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


Repository: mesos


Description
---

Added authorization to agents' `/monitor/statistics` endpoints.


Diffs (updated)
-

  src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check (OS X, clang w/o optimization)


Thanks,

Benjamin Bannier

Re: Review Request 46569: Parameterized agent endpoint authorization tests on tested endpoint.

[Benjamin Bannier]

> On April 25, 2016, 1:31 p.m., Adam B wrote:
> > src/tests/slave_authorization_tests.cpp, line 176
> > 
> >
> > Shouldn't you give this a more generic name now that it can handle any 
> > (agent) endpoint?

Done.


- Benjamin


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


On April 25, 2016, 4:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46569/
> ---
> 
> (Updated April 25, 2016, 4:16 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> At the moment we only provide a harness for `GET` methods, but will
> generalize this further once we authorize `POST` requests against
> agent endpoints as well.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46569/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang trunk w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.

[Benjamin Bannier]

> On April 25, 2016, 12:57 p.m., Alexander Rojas wrote:
> > src/slave/http.cpp, lines 647-649
> > 
> >
> > I was thinking that it makes sense to pull up the limiter related logic 
> > to `statistics()` so that method prepares everything up, while 
> > `_statistics()` only has logic related to build the response.

Good point! This also allows us to make `_statistics` purely synchronously 
which simplifies handling of `Future` failure scenarios.


- Benjamin


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


On April 25, 2016, 4:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46319/
> ---
> 
> (Updated April 25, 2016, 4:16 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization to agents' `/monitor/statistics` endpoints.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46319/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.

[Benjamin Bannier]

> On April 22, 2016, 10:22 a.m., Adam B wrote:
> > src/tests/slave_tests.cpp, lines 1926-1927
> > 
> >
> > I would expect you to await/validate the response after validating the 
> > request.
> 
> Benjamin Bannier wrote:
> Wouldn't that be mixing two concerns in the same test (and e.g., lead to 
> bloat hiding what is the crux of the test)? I think since here I check only 
> whether an endpoint is authorized looking at the status code should be enough.
> 
> Adam B wrote:
> Sure, I didn't mean you need to validate more of the response's body. I 
> just meant that you currently `AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, 
> response)` then `AWAIT_READY(request)` (and validate its 
> subject/action/object), where I would expect the logical order to be to 
> await/validate the request first, then AWAIT_EXPECT_RESPONSE after.
> Request first, response after.

Thanks for being extra verbose; I have no idea how I could misread what you 
wrote originally.


- Benjamin


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


On April 25, 2016, 4:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46319/
> ---
> 
> (Updated April 25, 2016, 4:16 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization to agents' `/monitor/statistics` endpoints.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46319/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46569: Parameterized agent endpoint authorization tests on tested endpoint.

[Benjamin Bannier]

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

(Updated April 25, 2016, 4:16 p.m.)


Review request for mesos and Adam B.


Changes
---

Renamed a var.


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


Repository: mesos


Description
---

At the moment we only provide a harness for `GET` methods, but will
generalize this further once we authorize `POST` requests against
agent endpoints as well.


Diffs (updated)
-

  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check (OS X, clang trunk w/o optimization)


Thanks,

Benjamin Bannier

Re: Review Request 46620: Refactored Flags to store name in `Name` struct instead of string.

[Benjamin Bannier]

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




3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp (line 45)


If this was `const` it would be much easier to reason how this works with 
all the usage sites.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (line 379)


Here and at similar call sites: since you provide an `operator<<` directly 
using `stringify(name)` here and elsewhere seems more consistent to me.


- Benjamin Bannier


On April 25, 2016, 7:43 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46620/
> ---
> 
> (Updated April 25, 2016, 7:43 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5271
> https://issues.apache.org/jira/browse/MESOS-5271
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sets up the stage to add metadata to the name (e.g., deprecated).
> This will be used to add deprecation support subsequently.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
> 2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 
> 
> Diff: https://reviews.apache.org/r/46620/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 46497: Added authentication to the '/profiler/*' endpoints.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 25, 2016, 10:02 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46497/
> ---
> 
> (Updated April 25, 2016, 10:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authentication to the '/profiler/*' endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/profiler.hpp 
> b305f1148ad6b623c75facc4abd0269c13017bab 
>   3rdparty/libprocess/src/process.cpp 
> 8727eb202e9699f0ac3c95788257cf1a22b0da7b 
>   3rdparty/libprocess/src/profiler.cpp 
> 58cf2fec119e65a74a3c75561ba0866ec88897d0 
> 
> Diff: https://reviews.apache.org/r/46497/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX. The endpoints were also tested by configuring with 
> `../configure --enable-perftools`, and then hitting `/profiler/start` and 
> `/profiler/stop` with the `LIBPROCESS_ENABLE_PROFILER` environment variable 
> set to 1.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46259: Added authentication to `/metrics/snapshot` endpoint.

[Greg Mann]

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

(Updated April 25, 2016, 2:21 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


Changes
---

Commented out unused `principal` parameters.


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


Repository: mesos


Description
---

This patch adds authentication to the `/metrics/snapshot`
endpoint on the master and agent.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
7abfadf6d030c52b7e03e773b67c74db6ad5b5df 
  3rdparty/libprocess/src/metrics/metrics.cpp 
7677dffed98153bc6276aad492a6815e67740ea3 
  3rdparty/libprocess/src/process.cpp 8727eb202e9699f0ac3c95788257cf1a22b0da7b 

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


Testing
---

`sudo make check` on OSX


Thanks,

Greg Mann

Re: Review Request 46262: Added a LoggingTest with authentication.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 15, 2016, 3:01 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46262/
> ---
> 
> (Updated April 15, 2016, 3:01 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test `LoggingTest.ToggleAuthenticationEnabled`
> is added in this patch.
> 
> 
> Diffs
> -
> 
>   src/tests/logging_tests.cpp 675a3f852baf5b4ffd10b674a0abc82c88a34c47 
> 
> Diff: https://reviews.apache.org/r/46262/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46261: Added new MetricsTests with authentication to Mesos tests.

[Kapil Arya]

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


Ship it!





src/tests/metrics_tests.cpp (line 85)


Newline?


- Kapil Arya


On April 21, 2016, 1:58 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46261/
> ---
> 
> (Updated April 21, 2016, 1:58 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The tests `MetricsTest.AgentAuthenticationEnabled` and
> `MetricsTest.MasterAuthenticationEnabled` are added in
> this patch.
> 
> 
> Diffs
> -
> 
>   src/tests/metrics_tests.cpp eacff678d06da7ba8afee6ab68261968561dffc3 
> 
> Diff: https://reviews.apache.org/r/46261/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46260: Added a MetricsTest with authentication to libprocess.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 22, 2016, 3:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46260/
> ---
> 
> (Updated April 22, 2016, 3:14 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test `MetricsTest.SnapshotAuthenticationEnabled`
> is added to the libprocess tests in this patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> b84dc8d858f58bc9f52b218b7153510417cf34c2 
> 
> Diff: https://reviews.apache.org/r/46260/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46497: Added authentication to the '/profiler/*' endpoints.

[Greg Mann]

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

(Updated April 25, 2016, 2:02 p.m.)


Review request for mesos, Alexander Rojas and Kapil Arya.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added authentication to the '/profiler/*' endpoints.


Diffs (updated)
-

  3rdparty/libprocess/include/process/profiler.hpp 
b305f1148ad6b623c75facc4abd0269c13017bab 
  3rdparty/libprocess/src/process.cpp 8727eb202e9699f0ac3c95788257cf1a22b0da7b 
  3rdparty/libprocess/src/profiler.cpp 58cf2fec119e65a74a3c75561ba0866ec88897d0 

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


Testing (updated)
---

`sudo make check` on OSX. The endpoints were also tested by configuring with 
`../configure --enable-perftools`, and then hitting `/profiler/start` and 
`/profiler/stop` with the `LIBPROCESS_ENABLE_PROFILER` environment variable set 
to 1.


Thanks,

Greg Mann

Re: Review Request 46497: Added authentication to the '/profiler/*' endpoints.

[Greg Mann]

> On April 25, 2016, 12:37 p.m., Alexander Rojas wrote:
> >

Whoops! Thanks Alexander :-)


- Greg


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


On April 25, 2016, 2:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46497/
> ---
> 
> (Updated April 25, 2016, 2:02 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authentication to the '/profiler/*' endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/profiler.hpp 
> b305f1148ad6b623c75facc4abd0269c13017bab 
>   3rdparty/libprocess/src/process.cpp 
> 8727eb202e9699f0ac3c95788257cf1a22b0da7b 
>   3rdparty/libprocess/src/profiler.cpp 
> 58cf2fec119e65a74a3c75561ba0866ec88897d0 
> 
> Diff: https://reviews.apache.org/r/46497/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX. The endpoints were also tested by configuring with 
> `../configure --enable-perftools`, and then hitting `/profiler/start` and 
> `/profiler/stop` with the `LIBPROCESS_ENABLE_PROFILER` environment variable 
> set to 1.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46258: Added authentication to `/logging/toggle` endpoint.

[Greg Mann]

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

(Updated April 25, 2016, 2:20 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


Changes
---

Commented out unused `principal` parameters.


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


Repository: mesos


Description
---

This patch adds authentication to the `/logging/toggle`
HTTP endpoint on the master and agent.


Diffs (updated)
-

  3rdparty/libprocess/include/process/logging.hpp 
0c0774647d8048a9d8d395141a5864eadc64f8ef 
  3rdparty/libprocess/src/logging.cpp bdbf716ee46a459f1004f0f4b72c23aae135f347 
  3rdparty/libprocess/src/process.cpp 8727eb202e9699f0ac3c95788257cf1a22b0da7b 

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


Testing
---

`sudo make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 46593: Added test for containerizer destroy while provisioning race.

[Neil Conway]

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


Fix it, then Ship it!





src/tests/containerizer/mesos_containerizer_tests.cpp (line 958)


Extra set of parentheses here doesn't seem useful. (Can you fix this where 
it occurs elsewhere in the test file?)


- Neil Conway


On April 22, 2016, 11:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46593/
> ---
> 
> (Updated April 22, 2016, 11:48 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for containerizer destroy while provisioning race.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 09742ff21513dc2570684d384b257868dd57a9ce 
> 
> Diff: https://reviews.apache.org/r/46593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Verified that the test is 90% chance to be segfault if we reverted the fix.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 46621: Add alias support for flags.

[Benjamin Bannier]

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



Did a quick review since I spent some time on working on a fix for MESOS-3335. 
It would be great if we wouldn't aggreviate that problem further.

What seems unclear to me ATM is how alias'ed flags can override each other, or 
what the preferred order would be.


3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp (line 64)


Does it makes sense to support multiple aliases?



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp (line 81)


We could return a const ref here.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 152 - 
171)


These suffer from the problem described in MESOS-3335.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 192 - 
200)


This also suffers from the problem described in MESOS-3335.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 212 - 
216)


This also suffers from the problem described in MESOS-3335.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 232 - 
250)


These signatures do not in general suffer from the problem of MESOS-3335 
since they use a member pointer instead of a pointer to (at some point) random 
memory.



3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp (lines 272 - 
273)


Signature also safe re:MESOS-3335.


- Benjamin Bannier


On April 25, 2016, 7:44 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46621/
> ---
> 
> (Updated April 25, 2016, 7:44 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5271
> https://issues.apache.org/jira/browse/MESOS-5271
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add alias support for flags.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
> 2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 
> 
> Diff: https://reviews.apache.org/r/46621/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 46187: Terminate when receiving the ACK of terminal status update.

[Vinod Kone]

> On April 14, 2016, 4:49 p.m., Vinod Kone wrote:
> > src/launcher/http_command_executor.cpp, line 749
> > 
> >
> > Looking at slave::statusUpdate() code there are several scenarios where 
> > the slave ignores a status update sent by the executor; this means this 
> > executor could end up not terminating forever.
> > 
> > Can you do the following:
> > 
> > --> Enque a message in the queue to self terminate after some timeout 
> > (you can use the delay() function) to be safe.
> > 
> > --> Add a TODO that we do this to be safe and also because slave 
> > sometimes doesn't ACK a status update. Link to a ticket that fixes the 
> > slave status update semantics to always ACK a status update sent by an 
> > executor.
> > 
> > sounds good?
> 
> Vinod Kone wrote:
> @Qian, any update on this? If this particular review is going to take 
> some time, I think it is still useful two commit the other 2 reviews in this 
> chain. AFAICT, they are independent of this review?
> 
> Qian Zhang wrote:
> @Vinod, sorry for the late. I have filed a ticket 
> (https://issues.apache.org/jira/browse/MESOS-5262) for enhancing 
> `slave::statusUpdate()` to always ACK the status update sent by executor.
> 
> And can you please elaborate about the specific scenarios this executor 
> could not terminate forever. Originially I thought the scenario should be: 
> executor sends a terminal status upate to slave when the corresponding 
> framework is in `TERMINATING` state (e.g., operator tears down the 
> framework), then in `Slave::statusUpdate()`, this status update will be 
> ignored, so the executor will not get the ACK. But after testing, I found in 
> this case the executor can still terminate, because the container 
> corresponded to this executor will be destroyed by 
> `Slave::shutdownExecutorTimeout()` -> `MesosContainerizer::destroy()`, so 
> after `--executor_shutdown_grace_period`, the executor can still terminate. 
> So I am not in which case the executor will never terminate.
> 
> And yes, the other 2 patches are independent of this one, I will make 
> them not depending on this one in the review board, thanks!
> 
> Qian Zhang wrote:
> After more thinking, I see one scenario the executor could never 
> terminate is: agent is down right after it sends SHUTDOWN event to executor. 
> In this case, when handling the SHUTDOWN event, executor will kill the task 
> and send TASK_KILLED status update to agent, but it will not get ACK since 
> agent is already down, so the executor will still run. But I think once agent 
> is started again, executor will receive the ACK and then terminate. I am not 
> sure if this behavior is OK, or we want executor to terminate once it 
> receives the SHUTDOWN event rather than wait for agent is started again?

I think it is the right thing for the executor to stay up until the agent comes 
back up and sends an ACK. Otherwise, which is currently the case, the agent has 
no idea about the exit status of the executor.

So the cases where I see no ACKs from the agent are:

1) `update` doesn't have UUID.
  --> Is this possible with the HTTP executor? If not, we should probably 
shutdown the executor instead of just ignoring.

2) Framework is unknown.
  --> Don't think we can do much here because we don't have access to Executor 
object?

3) Framework is terminating.
  --> Per your observation the agent is guaranteed to destroy the container. We 
need to add a comment here explainining this.

4) Executor is unknown. While we forward the status update to the framework, 
the ACK is dropped by `_statusUpdate` and `___statusUpdate()`
   --> If the update is generated by the agent (`killTask()`, `_runTask()`) we 
should be fine because there is no executor.
   --> If the update is sent by an executor for a task belonging to another 
executor, that another executor is hopefully already dead. So we should be fine.
   --> Is it possible for the update to be sent by an executor that is not 
known to the agent? Don't think we can do much here since we don't have access 
to the Executor object.


- Vinod


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


On April 25, 2016, 8:34 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46187/
> ---
> 
> (Updated April 25, 2016, 8:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-4855
> https://issues.apache.org/jira/browse/MESOS-4855
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Terminate when receiving the 

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46618]

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 April 25, 2016, 5:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46618/
> ---
> 
> (Updated April 25, 2016, 5:56 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added containerId to failure message when there are duplicate volumes
> in one container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 4675c767a562a3ca7f99265c9198a37786933c2d 
> 
> Diff: https://reviews.apache.org/r/46618/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46497: Added authentication to the '/profiler/*' endpoints.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46254, 46255, 46258, 46259, 46260, 46261, 46262, 46461, 
46462, 46497]

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 April 25, 2016, 2:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46497/
> ---
> 
> (Updated April 25, 2016, 2:02 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authentication to the '/profiler/*' endpoints.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/profiler.hpp 
> b305f1148ad6b623c75facc4abd0269c13017bab 
>   3rdparty/libprocess/src/process.cpp 
> 8727eb202e9699f0ac3c95788257cf1a22b0da7b 
>   3rdparty/libprocess/src/profiler.cpp 
> 58cf2fec119e65a74a3c75561ba0866ec88897d0 
> 
> Diff: https://reviews.apache.org/r/46497/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX. The endpoints were also tested by configuring with 
> `../configure --enable-perftools`, and then hitting `/profiler/start` and 
> `/profiler/stop` with the `LIBPROCESS_ENABLE_PROFILER` environment variable 
> set to 1.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 46643: Fixed the build when perftools is enabled.

[Greg Mann]

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

Review request for mesos and Kapil Arya.


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


Repository: mesos


Description
---

Fixed the build when perftools is enabled.


Diffs
-

  3rdparty/libprocess/configure.ac d27e46e37d4f341258cfa983b4b066973bab89fa 
  3rdparty/libprocess/src/profiler.cpp 58cf2fec119e65a74a3c75561ba0866ec88897d0 

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


Testing
---

`sudo make check` on OSX, after both `../configure` and `../configure 
--enable-perftools`.


Thanks,

Greg Mann

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 300)


Please remove the quote.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 547)


Quote is not needed for container id because it's generated by Mesos.


- Jie Yu


On April 25, 2016, 5:26 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46618/
> ---
> 
> (Updated April 25, 2016, 5:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added containerId to failure message when there are duplicate volumes
> in one container. Also quotaed the containerId in _cleanup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 4675c767a562a3ca7f99265c9198a37786933c2d 
> 
> Diff: https://reviews.apache.org/r/46618/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46461: Updated gperftools to version 2.5 (libprocess).

[Greg Mann]

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

(Updated April 25, 2016, 5:25 p.m.)


Review request for mesos and Kapil Arya.


Changes
---

Split build system changes into a separate commit.


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


Repository: mesos


Description
---

Updated gperftools to version 2.5 (libprocess).


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/gperftools-2.0.tar.gz 
13b03cae44e5e6dcfdec4756b975f425b2ea73cf 
  3rdparty/libprocess/3rdparty/gperftools-2.5.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 5506eb3 

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


Testing
---

`sudo make check`


Thanks,

Greg Mann

Review Request 46641: Added tests for the Profiler to libprocess.

[Greg Mann]

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

Review request for mesos, Alexander Rojas and Kapil Arya.


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


Repository: mesos


Description
---

Added tests for the Profiler to libprocess.


Diffs
-

  3rdparty/libprocess/Makefile.am c51c31eb91dd7be4cff8a188942ea77b3b361d45 
  3rdparty/libprocess/src/tests/profiler_tests.cpp PRE-CREATION 

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


Testing
---

`make check` in each of the following configurations:
* after `../configure`
* after `../configure --enable-perftools && export LIBPROCESS_ENABLE_PROFILER=0`
* after `../configure --enable-perftools && export LIBPROCESS_ENABLE_PROFILER=1`


Thanks,

Greg Mann

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Guangya Liu]

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

(Updated 四月 25, 2016, 5:56 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description (updated)
---

Added containerId to failure message when there are duplicate volumes
in one container.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
4675c767a562a3ca7f99265c9198a37786933c2d 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Guangya Liu]

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

(Updated 四月 25, 2016, 5:56 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Added containerId to failure message when there are duplicate volumes
in one container. Also quotaed the containerId in _cleanup.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
4675c767a562a3ca7f99265c9198a37786933c2d 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 45958: Updated protobuf Resource to mark the resource as shareable.

[Anindya Sinha]

> On April 18, 2016, 6:47 a.m., Jiang Yan Xu wrote:
> > include/mesos/mesos.proto, line 762
> > 
> >
> > How about `optional SharedInfo shared`?
> > 
> > [shareable](http://www.merriam-webster.com/dictionary/shareable): 
> > capable of being shared
> > 
> > The following sentences read pretty clear to me.
> > - Some resource types in Mesos are shareable.
> > - Currently only persistent volumes are shareable. (This has nothing to 
> > do with whether `SHARE` operation has been applied, just that this type of 
> > resource can be made shared.)
> > - The `SHARE` operation makes a nonshared resource **shared**. 
> > - The `UNSHARE` operation makes **shared** resource nonshared.
> > - `SharedInfo` is currently empty but in the future we may add policies 
> > around how this resource should be **shared**.
> > 
> > Plus we can compare this with `shared_ptr` which is semantically very 
> > similar.
> > 
> > If we agree to this please also change the use of these words elsewhere 
> > appropriately.

I think ShareInfo seems fine to me. However, I think if there is a strong 
opinion regarding this, I think Shareable is better simply because it describes 
the resource (ie. adjective) and is on the same principles as Revocable (as 
pointed by Guangya Liu).
However, I stringly believe ShareInfo should be fine.


- Anindya


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


On April 8, 2016, 11:16 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45958/
> ---
> 
> (Updated April 8, 2016, 11:16 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ShareInfo in Resource protobuf to allow for sharing of resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861 
>   include/mesos/v1/mesos.proto a60a834e2538d54db7f257a0d4adfbb503ec1b0f 
> 
> Diff: https://reviews.apache.org/r/45958/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 46261: Added new MetricsTests with authentication to Mesos tests.

[Greg Mann]

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

(Updated April 25, 2016, 5:37 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


Changes
---

Added missing newline.


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


Repository: mesos


Description
---

The tests `MetricsTest.AgentAuthenticationEnabled` and
`MetricsTest.MasterAuthenticationEnabled` are added in
this patch.


Diffs (updated)
-

  src/tests/metrics_tests.cpp eacff678d06da7ba8afee6ab68261968561dffc3 

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


Testing
---

`sudo make check` on OSX.


Thanks,

Greg Mann

Review Request 46647: Added the '--enable-perftools' flag to configure docs.

[Greg Mann]

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

Review request for mesos and Kapil Arya.


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


Repository: mesos


Description
---

Added the '--enable-perftools' flag to configure docs.


Diffs
-

  docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 46461: Updated gperftools to version 2.5 (libprocess).

[Kapil Arya]

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



LGTM modulo the comment. Also, have you done a grep for HAS_GPERFTOOLS to 
ensure that there aren't any leftovers?


3rdparty/libprocess/configure.ac (line 853)


I am wondering if we should split out this patch into two: one that updates 
bundled version, and second that does the HAS_GPERTOOL->ENABLE_GPERFTOOLS 
transition.


- Kapil Arya


On April 21, 2016, 4:29 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46461/
> ---
> 
> (Updated April 21, 2016, 4:29 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3319
> https://issues.apache.org/jira/browse/MESOS-3319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated gperftools to version 2.5 (libprocess).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/gperftools-2.0.tar.gz 
> 13b03cae44e5e6dcfdec4756b975f425b2ea73cf 
>   3rdparty/libprocess/3rdparty/gperftools-2.5.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 5506eb3 
>   3rdparty/libprocess/configure.ac d27e46e 
>   3rdparty/libprocess/src/profiler.cpp 58cf2fe 
> 
> Diff: https://reviews.apache.org/r/46461/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46462: Updated gperftools to version 2.5 (Mesos).

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 21, 2016, 12:22 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46462/
> ---
> 
> (Updated April 21, 2016, 12:22 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3319
> https://issues.apache.org/jira/browse/MESOS-3319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated gperftools to version 2.5 (Mesos).
> 
> 
> Diffs
> -
> 
>   src/python/native_common/ext_modules.py.in 
> ad98a576ee3662ccb5b8900a339e530089f41355 
> 
> Diff: https://reviews.apache.org/r/46462/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46461: Updated gperftools to version 2.5 (libprocess).

[Greg Mann]

> On April 25, 2016, 4:01 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/configure.ac, line 853
> > 
> >
> > I am wondering if we should split out this patch into two: one that 
> > updates bundled version, and second that does the 
> > HAS_GPERTOOL->ENABLE_GPERFTOOLS transition.

Good call; I split off a second review here: https://reviews.apache.org/r/46643/


- Greg


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


On April 25, 2016, 5:25 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46461/
> ---
> 
> (Updated April 25, 2016, 5:25 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3319
> https://issues.apache.org/jira/browse/MESOS-3319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated gperftools to version 2.5 (libprocess).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/gperftools-2.0.tar.gz 
> 13b03cae44e5e6dcfdec4756b975f425b2ea73cf 
>   3rdparty/libprocess/3rdparty/gperftools-2.5.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 5506eb3 
> 
> Diff: https://reviews.apache.org/r/46461/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Joseph Wu]

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



FYI: I will be making some changes to the fetcher underneath your change here.  
See the new issues added to the epic: MESOS-3918


src/uri/fetcher.hpp (lines 40 - 50)


This flag-based initialization method for the fetcher plugins doesn't make 
too much sense.  Instead, we should probably go for something like the 
`--isolation` agent flag.

i.e. 
`--fetcher_plugins=builtin/copy;builtin/curl;builtin/hadoop;builtin/docker;;...`

That would allow re-ordering of plugins as well as multiple custom plugins.

---

Also, all these flags are empty, other than `HadoopFetcherPlugin::Flags`.  
Now would be a good opportunity to get rid of these flags.  The Hadoop plugin 
would be a good candidate for an example Fetcher plugin implementation.

(Parameters/Flags for plugins would be specified in the `--modules` flag.)


- Joseph Wu


On April 22, 2016, 7:33 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42806/
> ---
> 
> (Updated April 22, 2016, 7:33 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 e024c6d65608a55765e527a8668c415723dcfcca 
>   src/examples/test_fetcher_plugin_module.cpp PRE-CREATION 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/tests/module.hpp e661d95fa44fc1aedfe83c564c826d5b7d32c85b 
>   src/tests/module.cpp 5b83fd6358ddea4c9d849b8992e1a6040ef74505 
>   src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
>   src/uri/fetcher.cpp aa9df5d0256a26b8684934c2bd37b82a069088f7 
> 
> Diff: https://reviews.apache.org/r/42806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 46647: Added the '--enable-perftools' flag to configure docs.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 25, 2016, 1:45 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46647/
> ---
> 
> (Updated April 25, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3319
> https://issues.apache.org/jira/browse/MESOS-3319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the '--enable-perftools' flag to configure docs.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
> 
> Diff: https://reviews.apache.org/r/46647/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46643: Fixed the build when perftools is enabled.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 25, 2016, 1:19 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46643/
> ---
> 
> (Updated April 25, 2016, 1:19 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-3319
> https://issues.apache.org/jira/browse/MESOS-3319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the build when perftools is enabled.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac d27e46e37d4f341258cfa983b4b066973bab89fa 
>   3rdparty/libprocess/src/profiler.cpp 
> 58cf2fec119e65a74a3c75561ba0866ec88897d0 
> 
> Diff: https://reviews.apache.org/r/46643/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX, after both `../configure` and `../configure 
> --enable-perftools`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40266: Libprocess Reinit: Cleanup SocketManager alongside ProcessManager.

[Joseph Wu]

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



Some inline TODO's from an offline review (with Joris & Neil).


3rdparty/libprocess/src/process.cpp (lines 781 - 788)


Write a test for this.  i.e. Try to connect to this socket (with plain 
sockets) while `process::finalize` is running.



3rdparty/libprocess/src/process.cpp (line 1106)


Consider moving to `::finalize`.  The unproxy code should mostly remain 
in-place.



3rdparty/libprocess/src/process.cpp (lines 2036 - 2040)


Save `PID` and terminate via `PID`.  This is a better guarantee against 
asynchronous/multiple termination events.



3rdparty/libprocess/src/process.cpp (line 2279)


Rename to something that doesn't semantically collide with 
`process::finalize`.  Not as explicit as `clean_up_all_but_gc()`, but along 
those lines.


- Joseph Wu


On April 14, 2016, 1:28 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40266/
> ---
> 
> (Updated April 14, 2016, 1:28 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3910
> https://issues.apache.org/jira/browse/MESOS-3910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `SocketManager` and `ProcessManager` are highly inter-dependent, 
> which requires some untangling in `process::finalize`.
> 
> * Logic originally found in `~ProcessManager` has been split into 
>   `ProcessManager::finalize` due to what happens during cleanup.
> * The future from `__s__->accept()` must be explicitly discarded as 
>   libevent does not detect a locally closed socket.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> afeddec20495bb9621c3e26b0d425c9419654739 
> 
> Diff: https://reviews.apache.org/r/40266/diff/
> 
> 
> Testing
> ---
> 
> `make check` (libev)
> `make check` (--enable-libevent --enable-ssl)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46588: Added URI struct to stout.

[Joseph Wu]

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

(Updated April 25, 2016, 4:13 p.m.)


Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


Changes
---

Add roundtrip equality to the test.  Handle some nuances in the "path" part of 
the URI.


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


Repository: mesos


Description
---

This will replace the `mesos::URI` protobuf currently used by the 
`uri::Fetcher`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
08ede41dcedc755933d656de58d93796e657749d 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/uri.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/uri_tests.cpp PRE-CREATION 

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


Testing
---

make check (OSX) GTEST_FILTER="URITest*"


Ran a clean build (make check) on:

* Ubuntu 12, 14, 15
* CentOS 6, 7
* Debian 8


Thanks,

Joseph Wu

Review Request 46669: Added deprecation support to Flag name.

[Vinod Kone]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Added deprecation support to Flag name.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 

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


Testing
---

Tested manually by running ./bin/mesos-master.sh --authenticate

The one caveat is that the deprecation warning might be printed before logging 
the library is initialized (e.g., master/main.cpp calls flags.load() before 
logging::initialize(flags)).


Thanks,

Vinod Kone

Re: Review Request 46665: Added 0.26.2 to the CHANGELOG.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46663, 46664, 46665]

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 April 25, 2016, 11:08 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46665/
> ---
> 
> (Updated April 25, 2016, 11:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reflects the backport of MESOS-4705 to the 0.26.x branch.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 
> 
> Diff: https://reviews.apache.org/r/46665/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 45668: Enable CMake build for Linux as a BUILDTOOL option.

[Vinod Kone]

> On April 18, 2016, 6:18 p.m., Vinod Kone wrote:
> > support/docker_build.sh, lines 127-140
> > 
> >
> > Hmm. I was hoping for something more generic than hard coding these 3 
> > configurations. But if it's not easy to auto translate and the 
> > configuration options for CMAKE are truly different from Autotools, I'm ok 
> > with not doing this change here and instead have 2 different Jenkins jobs 
> > (one for Mesos w/ Auto-tools and one for Mesos w/ CMake); was just hoping 
> > we could get away with having just one Jenkins job (parameterized).
> 
> Juan Larriba wrote:
> I will try to submit a more generic proposal to convert from autotools to 
> cmake parameters so the same Jenkins job can be used for building, I have had 
> a lot of work this past week and couldn't do anything. However, I still do 
> not understand why the BUILDTOOL or the COMPILER parameters can change 
> between job executions and the CONFIGURATION parameter cannot change at the 
> same time.

The ASF CI job is setup as a paremeterized job 
(https://builds.apache.org/view/M-R/view/Mesos/job/Mesos/). The parameters are 
the environment varialbes (BUILDTOOL, COMPILER, CONFIGURATION etc). So all the 
combinations of the environmental varialbles should work.

For example if we have the following envs,

BUILDTOOL = "'autotools' 'cmake'"
COMPILER = "'gcc' 'clang'"
CONFIGURATION = "'--verbose' '--enable_libevent'"

Then all the combinations of these should work.

autotools x gcc x --verbose
autotools x gcc x --enable_libevent
autotools x clang x --verbose
autotools x clang x --enable_libevent
cmake x gcc x --verbose
cmake x gcc x --enable_libevent
cmake x clang x --verbose
cmake x clang x --enable_libevent

Hope this makes sense?


- Vinod


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


On April 18, 2016, 1:58 p.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated April 18, 2016, 1:58 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-5101
> https://issues.apache.org/jira/browse/MESOS-5101
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable CMake build for Linux as a BUILDTOOL option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh c5917bcce4cf1f98a1808ceabe340648edd7d2a9 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04 using both 
> cmake and autotools. In ubuntu:14.04 was built using gcc and clang, in 
> centos:7 only gcc.
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>

Re: Review Request 46621: Add alias support for flags.

[Vinod Kone]

> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > Did a quick review since I spent some time on working on a fix for 
> > MESOS-3335. It would be great if we wouldn't aggreviate that problem 
> > further.
> > 
> > What seems unclear to me ATM is how alias'ed flags can override each other, 
> > or what the preferred order would be.

There is no override or preference order. Only one of name/alias is allowed. 
Otherwise we throw an error.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp, line 64
> > 
> >
> > Does it makes sense to support multiple aliases?

that was how the original patch was in my branch. but reverted it because i 
didn't have any use cases to use multiple aliases. we can add it later if we 
need it.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp, line 81
> > 
> >
> > We could return a const ref here.

just as an optmization? don't think we do that in our code base.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 
> > 152-171
> > 
> >
> > These suffer from the problem described in MESOS-3335.

yes. but IIUC this issue is not introduced here>? i would rather we fix it in 
one swoop when we fix MESOS-3335.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 
> > 192-200
> > 
> >
> > This also suffers from the problem described in MESOS-3335.

see above.


> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, lines 
> > 212-216
> > 
> >
> > This also suffers from the problem described in MESOS-3335.

see above.


- Vinod


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


On April 25, 2016, 5:44 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46621/
> ---
> 
> (Updated April 25, 2016, 5:44 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5271
> https://issues.apache.org/jira/browse/MESOS-5271
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add alias support for flags.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
> 2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 
> 
> Diff: https://reviews.apache.org/r/46621/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 46621: Added alias support for flags.

[Vinod Kone]

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

(Updated April 26, 2016, 1:04 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased and updated flag.load() signature to support deprecation later in the 
review chain.


Summary (updated)
-

Added alias support for flags.


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


Repository: mesos


Description (updated)
---

Added alias support for flags.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 

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


Testing
---

make check


Thanks,

Vinod Kone

Review Request 46663: Added 0.28.2 to the CHANGELOG.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This reflects the backport of MESOS-4705 to the 0.28.x branch.


Diffs
-

  CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 

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


Testing
---

N/A


Thanks,

Ben Mahler

Review Request 46665: Added 0.26.2 to the CHANGELOG.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This reflects the backport of MESOS-4705 to the 0.26.x branch.


Diffs
-

  CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 

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


Testing
---

N/A


Thanks,

Ben Mahler

Review Request 46664: Added 0.27.3 to the CHANGELOG.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This reflects the backport of MESOS-4705 to the 0.27.x branch.


Diffs
-

  CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 

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


Testing
---

N/A


Thanks,

Ben Mahler

Re: Review Request 45958: Updated protobuf Resource to mark the resource as shareable.

[Guangya Liu]

> On April 18, 2016, 6:47 a.m., Jiang Yan Xu wrote:
> > include/mesos/mesos.proto, line 762
> > 
> >
> > How about `optional SharedInfo shared`?
> > 
> > [shareable](http://www.merriam-webster.com/dictionary/shareable): 
> > capable of being shared
> > 
> > The following sentences read pretty clear to me.
> > - Some resource types in Mesos are shareable.
> > - Currently only persistent volumes are shareable. (This has nothing to 
> > do with whether `SHARE` operation has been applied, just that this type of 
> > resource can be made shared.)
> > - The `SHARE` operation makes a nonshared resource **shared**. 
> > - The `UNSHARE` operation makes **shared** resource nonshared.
> > - `SharedInfo` is currently empty but in the future we may add policies 
> > around how this resource should be **shared**.
> > 
> > Plus we can compare this with `shared_ptr` which is semantically very 
> > similar.
> > 
> > If we agree to this please also change the use of these words elsewhere 
> > appropriately.
> 
> Anindya Sinha wrote:
> I think ShareInfo seems fine to me. However, I think if there is a strong 
> opinion regarding this, I think Shareable is better simply because it 
> describes the resource (ie. adjective) and is on the same principles as 
> Revocable (as pointed by Guangya Liu).
> However, I stringly believe ShareInfo should be fine.

In my understaind, I think that this is similar with `RevocableInfo` as 
following:

  message RevocableInfo {}

  // If this is set, the resources are revocable, i.e., any tasks or
  // executors launched using these resources could get preempted or
  // throttled at any time. This could be used by frameworks to run
  // best effort tasks that do not need strict uptime or performance
  // guarantees. Note that if this is set, 'disk' or 'reservation'
  // cannot be set.
  optional RevocableInfo revocable = 9;

Agree with Anindya, using the concept of `Shareable` will have same principal 
with `Revocable`.


- Guangya


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


On April 8, 2016, 11:16 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45958/
> ---
> 
> (Updated April 8, 2016, 11:16 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ShareInfo in Resource protobuf to allow for sharing of resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 63c181ae0a1e350fc27e36b1698e02db100b8861 
>   include/mesos/v1/mesos.proto a60a834e2538d54db7f257a0d4adfbb503ec1b0f 
> 
> Diff: https://reviews.apache.org/r/45958/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Review Request 46670: Added deprecated alias for `--authenticate_frameworks` master flag.

[Vinod Kone]

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

Review request for mesos.


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


Repository: mesos


Description
---

Added deprecated alias for `--authenticate_frameworks` master flag.


Diffs
-

  src/master/flags.cpp c0c9e924e876175b75a174e375a4c993d97e18ee 

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


Testing
---

make check

Teste manually by running ./bin/mesos-master.sh --authenticate


Thanks,

Vinod Kone

Re: Review Request 46580: Added uriparser as a bundled dependency.

[Joseph Wu]

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

(Updated April 25, 2016, 4:12 p.m.)


Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


Changes
---

Rebase on a change right above one of mine (merge conflict).


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


Repository: mesos


Description
---

The source for `uriparser` can be found here:
http://uriparser.sourceforge.net/


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 83bd38a 
  3rdparty/libprocess/3rdparty/uriparser-0.8.4.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 5506eb3 
  3rdparty/libprocess/Makefile.am c51c31e 
  3rdparty/libprocess/configure.ac d27e46e 

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


Testing
---

On OSX:

../configure
make
make check


Thanks,

Joseph Wu

Re: Review Request 46370: Introduced linux capabilities API.

[Jojy Varghese]

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

(Updated April 26, 2016, 1:23 a.m.)


Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

This change introduces basic API for linux capabilities. This is not a
comprehensive API but is strictly limited to the need for securing Mesos
containers using linux capabilities.


Diffs (updated)
-

  src/CMakeLists.txt 8e3eecb172e6c6b5256313f0937dc1156e91 
  src/Makefile.am 08d5279bbc0c763255fda3c3360d8f24f41bccda 
  src/linux/capabilities.hpp PRE-CREATION 
  src/linux/capabilities.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 46569: Parameterized agent endpoint authorization tests on tested endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45922, 46318, 46203, 46319, 46569]

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 April 25, 2016, 2:16 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46569/
> ---
> 
> (Updated April 25, 2016, 2:16 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> At the moment we only provide a harness for `GET` methods, but will
> generalize this further once we authorize `POST` requests against
> agent endpoints as well.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46569/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang trunk w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46462: Updated gperftools to version 2.5 (Mesos).

[Greg Mann]

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

(Updated April 25, 2016, 5:26 p.m.)


Review request for mesos and Kapil Arya.


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


Repository: mesos


Description
---

Updated gperftools to version 2.5 (Mesos).


Diffs
-

  src/python/native_common/ext_modules.py.in 
ad98a576ee3662ccb5b8900a339e530089f41355 

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


Testing
---

`sudo make check`


Thanks,

Greg Mann

Re: Review Request 46261: Added new MetricsTests with authentication to Mesos tests.

[Greg Mann]

> On April 25, 2016, 3:32 p.m., Kapil Arya wrote:
> > src/tests/metrics_tests.cpp, line 85
> > 
> >
> > Newline?

Good catch! Fixed.


- Greg


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


On April 25, 2016, 5:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46261/
> ---
> 
> (Updated April 25, 2016, 5:37 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The tests `MetricsTest.AgentAuthenticationEnabled` and
> `MetricsTest.MasterAuthenticationEnabled` are added in
> this patch.
> 
> 
> Diffs
> -
> 
>   src/tests/metrics_tests.cpp eacff678d06da7ba8afee6ab68261968561dffc3 
> 
> Diff: https://reviews.apache.org/r/46261/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44379: Use tokens size to parse perf stat format.

[Ben Mahler]

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


Ship it!




Thanks for the comment!

I've added a commit description and made some minor adjustments that I'll 
commit with your change:

```
commit a5c81d4077400892cd3a5c306143f16903aac62c
Author: fan du 
Date:   Mon Apr 25 13:50:50 2016 -0700

Fixed the 'perf' parsing logic.

Previously the 'perf' parsing logic used the kernel version to
determine the token ordering. However, this approach breaks
when distributions backport perf parsing changes onto older
kernel versions. This updates the parsing logic to understand
all existing formats.

Co-authored with haosdent.

Review: https://reviews.apache.org/r/44379/

diff --git a/src/linux/perf.cpp b/src/linux/perf.cpp
index 749e676..2364ab5 100644
--- a/src/linux/perf.cpp
+++ b/src/linux/perf.cpp
@@ -343,28 +343,24 @@ struct Sample
 // because the unit field can be empty.
 vector tokens = strings::split(line, PERF_DELIMITER);

-if (version >= Version(4, 0, 0)) {
-  // Optional running time and ratio were introduced in Linux v4.0,
-  // which make the format either:
-  //   value,unit,event,cgroup
-  //   value,unit,event,cgroup,running,ratio
-  if ((tokens.size() == 4) || (tokens.size() == 6)) {
-return Sample({tokens[0], internal::normalize(tokens[2]), tokens[3]});
-  }
-} else if (version >= Version(3, 13, 0)) {
-  // Unit was added in Linux v3.13, making the format:
-  //   value,unit,event,cgroup
-  if (tokens.size() == 4) {
-return Sample({tokens[0], internal::normalize(tokens[2]), tokens[3]});
-  }
-} else {
-  // Expected format for Linux kernel <= 3.12 is:
-  //   value,event,cgroup
-  if (tokens.size() == 3) {
-return Sample({tokens[0], internal::normalize(tokens[1]), tokens[2]});
-  }
+// The following formats are possible:
+//   (1) value,event,cgroup (since Linux v2.6.39)
+//   (2) value,unit,event,cgroup (since Linux v3.14)
+//   (3) value,unit,event,cgroup,running,ratio (since Linux v4.1)
+//
+// Note that we do not use the kernel version when parsing
+// because OS vendors often backport perf tool functionality
+// into older kernel versions.
+
+if (tokens.size() == 3) {
+  return Sample({tokens[0], internal::normalize(tokens[1]), tokens[2]});
+}
+
+if (tokens.size() == 4 || tokens.size() == 6) {
+  return Sample({tokens[0], internal::normalize(tokens[2]), tokens[3]});
 }

+// Bail out if the format is not recognized.
 return Error("Unexpected number of fields");
   }
 };
```

- Ben Mahler


On April 18, 2016, 5:41 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
> ---
> 
> (Updated April 18, 2016, 5:41 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4705
> https://issues.apache.org/jira/browse/MESOS-4705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Co-authored with haosdent.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 749e676aaf2ce639dd976f2b23e323300c6114c5 
> 
> Diff: https://reviews.apache.org/r/44379/diff/
> 
> 
> Testing
> ---
> 
> 1. {Found and Test} with Serenity, ema filter could get perf event statistics 
> correctly as expected.
> 2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
> --log_dir=/tmp/mesos/
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 46641: Added tests for the Profiler to libprocess.

[Greg Mann]

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

(Updated April 25, 2016, 8:47 p.m.)


Review request for mesos, Alexander Rojas and Kapil Arya.


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


Repository: mesos


Description
---

Added tests for the Profiler to libprocess.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c51c31eb91dd7be4cff8a188942ea77b3b361d45 
  3rdparty/libprocess/src/tests/profiler_tests.cpp PRE-CREATION 

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


Testing
---

`make check` in each of the following configurations:
* after `../configure`
* after `../configure --enable-perftools && export LIBPROCESS_ENABLE_PROFILER=0`
* after `../configure --enable-perftools && export LIBPROCESS_ENABLE_PROFILER=1`


Thanks,

Greg Mann

Re: Review Request 46260: Added a MetricsTest with authentication to libprocess.

[Greg Mann]

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

(Updated April 25, 2016, 8:47 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description
---

The test `MetricsTest.SnapshotAuthenticationEnabled`
is added to the libprocess tests in this patch.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/metrics_tests.cpp 
b84dc8d858f58bc9f52b218b7153510417cf34c2 

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


Testing
---

`sudo make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 46187: Terminate when receiving the ACK of terminal status update.

[Qian Zhang]

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

(Updated April 25, 2016, 8:34 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Updated the bug id -- @vinodkone


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


Repository: mesos


Description
---

Terminate when receiving the ACK of terminal status update.


Diffs
-

  src/launcher/http_command_executor.cpp 
0b4136c040ec9cf585c0d38f8471495a855cd640 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 40266: Libprocess Reinit: Cleanup SocketManager alongside ProcessManager.

[Neil Conway]

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




3rdparty/libprocess/src/process.cpp (line 1356)


"each socket"



3rdparty/libprocess/src/process.cpp (line 1357)


"interdependency" => "a dependency"


- Neil Conway


On April 14, 2016, 8:28 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40266/
> ---
> 
> (Updated April 14, 2016, 8:28 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3910
> https://issues.apache.org/jira/browse/MESOS-3910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `SocketManager` and `ProcessManager` are highly inter-dependent, 
> which requires some untangling in `process::finalize`.
> 
> * Logic originally found in `~ProcessManager` has been split into 
>   `ProcessManager::finalize` due to what happens during cleanup.
> * The future from `__s__->accept()` must be explicitly discarded as 
>   libevent does not detect a locally closed socket.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> afeddec20495bb9621c3e26b0d425c9419654739 
> 
> Diff: https://reviews.apache.org/r/40266/diff/
> 
> 
> Testing
> ---
> 
> `make check` (libev)
> `make check` (--enable-libevent --enable-ssl)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40512: Libprocess Reinit: Add a test-only method to reinitialize libprocess.

[Neil Conway]

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




3rdparty/libprocess/src/process.cpp (line 475)


Typo.


- Neil Conway


On April 14, 2016, 8:28 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40512/
> ---
> 
> (Updated April 14, 2016, 8:28 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3820
> https://issues.apache.org/jira/browse/MESOS-3820
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This builds upon earlier changes to complete `process::finalize`.  
> Tests which need to reconfigure libprocess, such as some SSL-related 
> tests, can use `process::reinitialize` to do so.  
> 
> This method may also be useful for providing additional isolation 
> between tests, such as cleaning up all processes after each test case.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> 523b9740a45bdae3fa9a1a1423108e6815fbf872 
>   3rdparty/libprocess/src/process.cpp 
> afeddec20495bb9621c3e26b0d425c9419654739 
> 
> Diff: https://reviews.apache.org/r/40512/diff/
> 
> 
> Testing
> ---
> 
> Defined `process::reinitialize` in several tests, such as `HTTPTest`, 
> `MetricsTest`, and `ReapTest` can called `process::reinitialize` before and 
> after tests.  Confirmed that this did not have side effects on other tests 
> (See https://reviews.apache.org/r/40453/ ).
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46564: Fixed a typo in Docker containerizer.

[Qian Zhang]

> On April 25, 2016, 9:48 a.m., Timothy Chen wrote:
> > Can you rebase your patch? Thanks!

Sorry, I do not quite understand. I think the file I updated in this patch was 
not updated by others after I posted this patch, so there should be no 
conflict, right? :-)


- Qian


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


On April 22, 2016, 6:40 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46564/
> ---
> 
> (Updated April 22, 2016, 6:40 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in Docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 30f85a13f70fc2b1e04dbf0ca7f47bff806e4672 
> 
> Diff: https://reviews.apache.org/r/46564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46564: Fixed a typo in Docker containerizer.

[Qian Zhang]

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

(Updated April 25, 2016, 4:15 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Rebase


Repository: mesos


Description
---

Fixed a typo in Docker containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 30f85a13f70fc2b1e04dbf0ca7f47bff806e4672 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46618]

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 April 25, 2016, 5:26 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46618/
> ---
> 
> (Updated April 25, 2016, 5:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added containerId to failure message when there are duplicate volumes
> in one container. Also quotaed the containerId in _cleanup.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 4675c767a562a3ca7f99265c9198a37786933c2d 
> 
> Diff: https://reviews.apache.org/r/46618/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46564: Fixed a typo in Docker containerizer.

[Qian Zhang]

> On April 25, 2016, 9:48 a.m., Timothy Chen wrote:
> > Can you rebase your patch? Thanks!
> 
> Qian Zhang wrote:
> Sorry, I do not quite understand. I think the file I updated in this 
> patch was not updated by others after I posted this patch, so there should be 
> no conflict, right? :-)
> 
> Timothy Chen wrote:
> I tried to apply it on latest master but it has conflict, so you need to 
> rebase on the latest master and update the patch again.

OK, I have rebased on the latest master, but it seems the updated patch is same 
with the previous one.

BTW, can you please help review https://reviews.apache.org/r/46264/? It is also 
a simple fix to a typo :-)


- Qian


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


On April 25, 2016, 4:15 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46564/
> ---
> 
> (Updated April 25, 2016, 4:15 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in Docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 30f85a13f70fc2b1e04dbf0ca7f47bff806e4672 
> 
> Diff: https://reviews.apache.org/r/46564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46564: Fixed a typo in Docker containerizer.

[Timothy Chen]

> On April 25, 2016, 1:48 a.m., Timothy Chen wrote:
> > Can you rebase your patch? Thanks!
> 
> Qian Zhang wrote:
> Sorry, I do not quite understand. I think the file I updated in this 
> patch was not updated by others after I posted this patch, so there should be 
> no conflict, right? :-)

I tried to apply it on latest master but it has conflict, so you need to rebase 
on the latest master and update the patch again.


- Timothy


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


On April 22, 2016, 10:40 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46564/
> ---
> 
> (Updated April 22, 2016, 10:40 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in Docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 30f85a13f70fc2b1e04dbf0ca7f47bff806e4672 
> 
> Diff: https://reviews.apache.org/r/46564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

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

(Updated April 25, 2016, 10:19 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.


Changes
---

Changed dependency chain.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
  include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
  include/mesos/authorizer/authorizer.proto 
40d93ea257d1df8d22eee8a21667db90d579a8fe 
  src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
  src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 
  src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
  src/slave/http.cpp f887a71684304a82ff0d81dbd240e29aa7e2ac67 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.

[Benjamin Bannier]

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

(Updated April 25, 2016, 10:19 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.


Changes
---

Reorder deps on Jan's request.


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


Repository: mesos


Description
---

Added authorization to agents' `/monitor/statistics` endpoints.


Diffs
-

  src/slave/http.cpp f887a71684304a82ff0d81dbd240e29aa7e2ac67 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check (OS X, clang w/o optimization)


Thanks,

Benjamin Bannier

Re: Review Request 46318: Added helper to create test agent with injected `Authorizer`.

[Benjamin Bannier]

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

(Updated April 25, 2016, 10:19 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.


Changes
---

Reorder deps on Jan's request.


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


Repository: mesos


Description
---

In addition to the fully generic interface we do provide a number of
short hand functions for creating agents in tests which allow injecting
just a single component. Add one such short hand function for creating
a test agent with an injected `Authorizer` which we will use in a
subsequent patch.


Diffs
-

  src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
  src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 

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


Testing
---

make check (OS X, clang w/o optimization)


Thanks,

Benjamin Bannier

Re: Review Request 45668: Enable CMake build for Linux as a BUILDTOOL option.

[Juan Larriba]

> On Abr. 18, 2016, 6:18 p.m., Vinod Kone wrote:
> > support/docker_build.sh, line 125
> > 
> >
> > What is this for?

It tells CMake the generator that it needs to use to generate build files.


> On Abr. 18, 2016, 6:18 p.m., Vinod Kone wrote:
> > support/docker_build.sh, lines 127-140
> > 
> >
> > Hmm. I was hoping for something more generic than hard coding these 3 
> > configurations. But if it's not easy to auto translate and the 
> > configuration options for CMAKE are truly different from Autotools, I'm ok 
> > with not doing this change here and instead have 2 different Jenkins jobs 
> > (one for Mesos w/ Auto-tools and one for Mesos w/ CMake); was just hoping 
> > we could get away with having just one Jenkins job (parameterized).

I will try to submit a more generic proposal to convert from autotools to cmake 
parameters so the same Jenkins job can be used for building, I have had a lot 
of work this past week and couldn't do anything. However, I still do not 
understand why the BUILDTOOL or the COMPILER parameters can change between job 
executions and the CONFIGURATION parameter cannot change at the same time.


- Juan


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


On Abr. 18, 2016, 1:58 p.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated Abr. 18, 2016, 1:58 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joerg Schad, and Vinod Kone.
> 
> 
> Bugs: MESOS-5101
> https://issues.apache.org/jira/browse/MESOS-5101
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable CMake build for Linux as a BUILDTOOL option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh c5917bcce4cf1f98a1808ceabe340648edd7d2a9 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04 using both 
> cmake and autotools. In ubuntu:14.04 was built using gcc and clang, in 
> centos:7 only gcc.
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>

Re: Review Request 46624: Added support to mesos to work with flag alias.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46620, 46621, 46622, 46623, 46624]

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 April 25, 2016, 5:46 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46624/
> ---
> 
> (Updated April 25, 2016, 5:46 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5271
> https://issues.apache.org/jira/browse/MESOS-5271
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support to mesos to work with flag alias.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4f489fb9e5bc77b839cc4a44d7ec36af06f73e4a 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
> 
> Diff: https://reviews.apache.org/r/46624/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.

[Benjamin Bannier]

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

(Updated April 25, 2016, 11:52 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.


Changes
---

Rebased onto Jan's last changes. Expected to fail until he fixes his 
`authorizeEndpoint` to support agent paths containing multiple `/`.


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


Repository: mesos


Description
---

Added authorization to agents' `/monitor/statistics` endpoints.


Diffs (updated)
-

  src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check (OS X, clang w/o optimization)


Thanks,

Benjamin Bannier

Re: Review Request 46569: Parameterized agent endpoint authorization tests on tested endpoint.

[Benjamin Bannier]

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

(Updated April 25, 2016, 11:52 a.m.)


Review request for mesos and Adam B.


Changes
---

Rebased.


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


Repository: mesos


Description
---

At the moment we only provide a harness for `GET` methods, but will
generalize this further once we authorize `POST` requests against
agent endpoints as well.


Diffs (updated)
-

  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check (OS X, clang trunk w/o optimization)


Thanks,

Benjamin Bannier

Re: Review Request 46564: Fixed a typo in Docker containerizer.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46564]

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 April 25, 2016, 8:15 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46564/
> ---
> 
> (Updated April 25, 2016, 8:15 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in Docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 30f85a13f70fc2b1e04dbf0ca7f47bff806e4672 
> 
> Diff: https://reviews.apache.org/r/46564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Benjamin Bannier]

> On April 20, 2016, 10:35 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 658-660
> > 
> >
> > Where did you come up with the magic number 3? What if we reorganize 
> > the operator endpoints in the (1.0) future? How will we know what the new 
> > value should be here?
> > What if the user setup a reverse proxy (like in dcos) and these 
> > requests are actually coming from a different base url than expected?

@adam: The three here is needed so that this just strips the agent part of the 
path, not everything up to the last `/`. An example endpoint would be 
`/slave(1)/monitor/statistics`.


- Benjamin


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


On April 25, 2016, 10:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 10:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Benjamin Bannier]

> On April 20, 2016, 10:35 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 658-660
> > 
> >
> > Where did you come up with the magic number 3? What if we reorganize 
> > the operator endpoints in the (1.0) future? How will we know what the new 
> > value should be here?
> > What if the user setup a reverse proxy (like in dcos) and these 
> > requests are actually coming from a different base url than expected?
> 
> Benjamin Bannier wrote:
> @adam: The three here is needed so that this just strips the agent part 
> of the path, not everything up to the last `/`. An example endpoint would be 
> `/slave(1)/monitor/statistics`.
> 
> Jan Schlicht wrote:
> Seems like a hard problem to fully support both requirements. Maybe 
> reverting back to using `std::string` instead of `http::URL` as the function 
> parameter for `endpoint` could resolve this.

Please use some typed entity that the usual endpoint handlers are aware of. 
They currently have a `Request`, but e.g., have no idea how they are being 
routed.


- Benjamin


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


On April 25, 2016, 10:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 10:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

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

(Updated April 25, 2016, 10:30 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
  include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
  include/mesos/authorizer/authorizer.proto 
40d93ea257d1df8d22eee8a21667db90d579a8fe 
  src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
  src/authorizer/local/authorizer.cpp 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
  src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
  src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

> On April 20, 2016, 10:35 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 658-660
> > 
> >
> > Where did you come up with the magic number 3? What if we reorganize 
> > the operator endpoints in the (1.0) future? How will we know what the new 
> > value should be here?
> > What if the user setup a reverse proxy (like in dcos) and these 
> > requests are actually coming from a different base url than expected?
> 
> Benjamin Bannier wrote:
> @adam: The three here is needed so that this just strips the agent part 
> of the path, not everything up to the last `/`. An example endpoint would be 
> `/slave(1)/monitor/statistics`.

Seems like a hard problem to fully support both requirements. Maybe reverting 
back to using `std::string` instead of `http::URL` as the function parameter 
for `endpoint` could resolve this.


- Jan


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


On April 25, 2016, 10:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 10:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46264: Fixed a typo in docker_containerizer_tests.cpp.

[Qian Zhang]

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

(Updated April 25, 2016, 4:23 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Rebase


Repository: mesos


Description
---

Fixed a typo in docker_containerizer_tests.cpp.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
f43165388f29513ab8be6594ab6647e8f9eb5a93 

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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

> On April 20, 2016, 10:35 a.m., Adam B wrote:
> > docs/configuration.md, line 897
> > 
> >
> > We're going to have to start documenting which endpoints can/must be 
> > authorized this way, similar to how Joerg added authentication to endpoint 
> > help. Can you create a new JIRA for this?

I've added [MESOS-5273](https://issues.apache.org/jira/browse/MESOS-5142) for 
that task.


- Jan


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


On April 25, 2016, 10:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 10:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.

[Alexander Rojas]

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




src/slave/http.cpp (lines 647 - 649)


I was thinking that it makes sense to pull up the limiter related logic to 
`statistics()` so that method prepares everything up, while `_statistics()` 
only has logic related to build the response.


- Alexander Rojas


On April 25, 2016, 11:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46319/
> ---
> 
> (Updated April 25, 2016, 11:52 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization to agents' `/monitor/statistics` endpoints.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46319/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46139: Add positive tests for /weights endpoint.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On April 19, 2016, 6:42 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46139/
> ---
> 
> (Updated April 19, 2016, 6:42 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4316
> https://issues.apache.org/jira/browse/MESOS-4316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add positive tests for /weights endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp 
> f89b89dd2553220161e28ec4784eb0bbdfdb65fe 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46139/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
> --gtest_filter=DynamicWeightsTest.*
> [==] Running 13 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 13 tests from DynamicWeightsTest
> [ RUN  ] DynamicWeightsTest.PutInvalidRequest
> [   OK ] DynamicWeightsTest.PutInvalidRequest (89 ms)
> [ RUN  ] DynamicWeightsTest.ZeroWeight
> [   OK ] DynamicWeightsTest.ZeroWeight (39 ms)
> [ RUN  ] DynamicWeightsTest.NegativeWeight
> [   OK ] DynamicWeightsTest.NegativeWeight (46 ms)
> [ RUN  ] DynamicWeightsTest.NonNumericWeight
> [   OK ] DynamicWeightsTest.NonNumericWeight (39 ms)
> [ RUN  ] DynamicWeightsTest.MissingRole
> [   OK ] DynamicWeightsTest.MissingRole (37 ms)
> [ RUN  ] DynamicWeightsTest.UnknownRole
> [   OK ] DynamicWeightsTest.UnknownRole (32 ms)
> [ RUN  ] DynamicWeightsTest.UpdateWeightsWithExplictRoles
> [   OK ] DynamicWeightsTest.UpdateWeightsWithExplictRoles (44 ms)
> [ RUN  ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest
> [   OK ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest (40 ms)
> [ RUN  ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest
> [   OK ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest (35 ms)
> [ RUN  ] DynamicWeightsTest.AuthorizedWeightUpdateRequest
> [   OK ] DynamicWeightsTest.AuthorizedWeightUpdateRequest (42 ms)
> [ RUN  ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal
> [   OK ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal 
> (41 ms)
> [ RUN  ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest
> [   OK ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest (40 ms)
> [ RUN  ] DynamicWeightsTest.RecoveredWeightsFromRegistry
> [   OK ] DynamicWeightsTest.RecoveredWeightsFromRegistry (135 ms)
> [--] 13 tests from DynamicWeightsTest (659 ms total)
> 
> [--] Global test environment tear-down
> [==] 13 tests from 1 test case ran. (669 ms total)
> [  PASSED  ] 13 tests.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

> On April 20, 2016, 10:35 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 658-660
> > 
> >
> > Where did you come up with the magic number 3? What if we reorganize 
> > the operator endpoints in the (1.0) future? How will we know what the new 
> > value should be here?
> > What if the user setup a reverse proxy (like in dcos) and these 
> > requests are actually coming from a different base url than expected?
> 
> Benjamin Bannier wrote:
> @adam: The three here is needed so that this just strips the agent part 
> of the path, not everything up to the last `/`. An example endpoint would be 
> `/slave(1)/monitor/statistics`.
> 
> Jan Schlicht wrote:
> Seems like a hard problem to fully support both requirements. Maybe 
> reverting back to using `std::string` instead of `http::URL` as the function 
> parameter for `endpoint` could resolve this.
> 
> Benjamin Bannier wrote:
> Please use some typed entity that the usual endpoint handlers are aware 
> of. They currently have a `Request`, but e.g., have no idea how they are 
> being routed.

I'll go back to using the "magic number 3". At this point `URL::path` will look 
like this: "/slave(n)/name/of/endpoint". By splitting into 3 components we get 
rid of the "/slave(n)/". The path is not the full URL that has been requested, 
hence reverse proxies shouldn't be an issue here. I'll add a comment, 
explaining this.


- Jan


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


On April 25, 2016, 10:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 10:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46318: Added helper to create test agent with injected `Authorizer`.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On April 25, 2016, 10:19 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46318/
> ---
> 
> (Updated April 25, 2016, 10:19 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In addition to the fully generic interface we do provide a number of
> short hand functions for creating agents in tests which allow injecting
> just a single component. Add one such short hand function for creating
> a test agent with an injected `Authorizer` which we will use in a
> subsequent patch.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
>   src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 
> 
> Diff: https://reviews.apache.org/r/46318/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 45922: Added agent authorization flags.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On April 22, 2016, 3:11 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> ---
> 
> (Updated April 22, 2016, 3:11 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956 
>   src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea 
>   src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
>   src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4 
>   src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
>   src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 
> 
> Diff: https://reviews.apache.org/r/45922/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46318: Added helper to create test agent with injected `Authorizer`.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On April 25, 2016, 1:19 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46318/
> ---
> 
> (Updated April 25, 2016, 1:19 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In addition to the fully generic interface we do provide a number of
> short hand functions for creating agents in tests which allow injecting
> just a single component. Add one such short hand function for creating
> a test agent with an injected `Authorizer` which we will use in a
> subsequent patch.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
>   src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 
> 
> Diff: https://reviews.apache.org/r/46318/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.

[Adam B]

> On April 22, 2016, 1:22 a.m., Adam B wrote:
> > src/tests/slave_tests.cpp, lines 1926-1927
> > 
> >
> > I would expect you to await/validate the response after validating the 
> > request.
> 
> Benjamin Bannier wrote:
> Wouldn't that be mixing two concerns in the same test (and e.g., lead to 
> bloat hiding what is the crux of the test)? I think since here I check only 
> whether an endpoint is authorized looking at the status code should be enough.

Sure, I didn't mean you need to validate more of the response's body. I just 
meant that you currently `AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, 
response)` then `AWAIT_READY(request)` (and validate its 
subject/action/object), where I would expect the logical order to be to 
await/validate the request first, then AWAIT_EXPECT_RESPONSE after.
Request first, response after.


- Adam


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


On April 25, 2016, 2:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46319/
> ---
> 
> (Updated April 25, 2016, 2:52 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization to agents' `/monitor/statistics` endpoints.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46319/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Adam B]

> On April 20, 2016, 1:35 a.m., Adam B wrote:
> > src/tests/slave_authorization_tests.cpp, lines 110-114
> > 
> >
> > This seems wrong. You don't even bother to reset the authenticator 
> > after you're done?
> > Why do you even need to unset the authenticator when you already set 
> > `slaveFlags.authenticate_http = false`?
> 
> Jan Schlicht wrote:
> Unfortunately the libprocess environment is shared between tests. An 
> agent sets the libprocess-wide authenticator when initialized but doesn't 
> unset it -- which is good, because otherwise tests instanciating multiple 
> agents may not work correctly. As a consequence, the authenticator may still 
> be set when this test case is started, forcing us to have to explicitly unset 
> it here.
> 
> Adam B wrote:
> And then you don't need to reset it at the end of this test because the 
> next agent that starts will set it anew?
> 
> Jan Schlicht wrote:
> Yes, exactly so.
> But resetting it would be the right way, i.e. all tests should do it like 
> that. Then we wouldn't need to unset it here. That's what's done for the 
> master in `cluster.cpp` (see `Master::~Master`) but it would restrict us to 
> using a single agent in the tests. Better would be something like a reference 
> pointed pointer to some object that unsets the authenticator after all agents 
> used in the test have been destroyed.
> 
> Jan Schlicht wrote:
> s/reference pointed pointer/reference counted pointer/ ... = 
> `std::shared_ptr`.

Ok, please add a TODO to fix this weird unset behavior somehow. Then we can 
drop this review issue.


- Adam


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


On April 25, 2016, 1:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 1:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Adam B]

> On April 20, 2016, 1:35 a.m., Adam B wrote:
> > include/mesos/authorizer/acls.proto, line 151
> > 
> >
> > Let's consider calling this `GetEndpoint`, to match the HTTP verb? 
> > There may be some users that are allowed to GET the endpoint at a 
> > particular path, but cannot POST to it.
> 
> Jan Schlicht wrote:
> You're right, follow up tickets will want to authorize `POST` actions and 
> operators pretty sure will want them to be different from their `GET` acl.
> Will rename it to `GetEndpoint`. As a consequence `ACLs.acces_endpoints` 
> will be renamed to `ACLs.get_endpoints` and `ACCESS_ENDPOINT_WITH_PATH` to 
> `GET_ENDPOINTS_WITH_PATH`.

+1


> On April 20, 2016, 1:35 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 658-660
> > 
> >
> > Where did you come up with the magic number 3? What if we reorganize 
> > the operator endpoints in the (1.0) future? How will we know what the new 
> > value should be here?
> > What if the user setup a reverse proxy (like in dcos) and these 
> > requests are actually coming from a different base url than expected?
> 
> Benjamin Bannier wrote:
> @adam: The three here is needed so that this just strips the agent part 
> of the path, not everything up to the last `/`. An example endpoint would be 
> `/slave(1)/monitor/statistics`.
> 
> Jan Schlicht wrote:
> Seems like a hard problem to fully support both requirements. Maybe 
> reverting back to using `std::string` instead of `http::URL` as the function 
> parameter for `endpoint` could resolve this.
> 
> Benjamin Bannier wrote:
> Please use some typed entity that the usual endpoint handlers are aware 
> of. They currently have a `Request`, but e.g., have no idea how they are 
> being routed.
> 
> Jan Schlicht wrote:
> I'll go back to using the "magic number 3". At this point `URL::path` 
> will look like this: "/slave(n)/name/of/endpoint". By splitting into 3 
> components we get rid of the "/slave(n)/". The path is not the full URL that 
> has been requested, hence reverse proxies shouldn't be an issue here. I'll 
> add a comment, explaining this.

I see. And will this value be the same for the master's endpoints?
Good to hear that reverse proxies won't be affected since it's not a full URL.


- Adam


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


On April 25, 2016, 1:30 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 1:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46569: Parameterized agent endpoint authorization tests on tested endpoint.

[Adam B]

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


Fix it, then Ship it!




Looks like a great generalization!
What magic. You must be a wizard.


src/tests/slave_authorization_tests.cpp (line 175)


Shouldn't you give this a more generic name now that it can handle any 
(agent) endpoint?


- Adam B


On April 25, 2016, 2:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46569/
> ---
> 
> (Updated April 25, 2016, 2:52 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-5164
> https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> At the moment we only provide a harness for `GET` methods, but will
> generalize this further once we authorize `POST` requests against
> agent endpoints as well.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46569/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang trunk w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Alexander Rojas]

> On April 20, 2016, 10:35 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 658-660
> > 
> >
> > Where did you come up with the magic number 3? What if we reorganize 
> > the operator endpoints in the (1.0) future? How will we know what the new 
> > value should be here?
> > What if the user setup a reverse proxy (like in dcos) and these 
> > requests are actually coming from a different base url than expected?
> 
> Benjamin Bannier wrote:
> @adam: The three here is needed so that this just strips the agent part 
> of the path, not everything up to the last `/`. An example endpoint would be 
> `/slave(1)/monitor/statistics`.
> 
> Jan Schlicht wrote:
> Seems like a hard problem to fully support both requirements. Maybe 
> reverting back to using `std::string` instead of `http::URL` as the function 
> parameter for `endpoint` could resolve this.
> 
> Benjamin Bannier wrote:
> Please use some typed entity that the usual endpoint handlers are aware 
> of. They currently have a `Request`, but e.g., have no idea how they are 
> being routed.
> 
> Jan Schlicht wrote:
> I'll go back to using the "magic number 3". At this point `URL::path` 
> will look like this: "/slave(n)/name/of/endpoint". By splitting into 3 
> components we get rid of the "/slave(n)/". The path is not the full URL that 
> has been requested, hence reverse proxies shouldn't be an issue here. I'll 
> add a comment, explaining this.
> 
> Adam B wrote:
> I see. And will this value be the same for the master's endpoints?
> Good to hear that reverse proxies won't be affected since it's not a full 
> URL.
> 
> Jan Schlicht wrote:
> This values won't work for the master's endpoint. In that case 
> `URL::path` will be "/name/of/endpoint" and we wouldn't need to split. 
> Because we're in `Slave::Http` we can expect that this code is only called 
> for agents.
> 
> Alexander Rojas wrote:
> So here is my issue wit this, you break it into three, and pass only the 
> second one to the authorizer, but that just sets a bad precedent. There are 
> endpoint that added with more components, e.g. `/api/v1/scheduler`. The right 
> way to solve this is to do something like:
> 
> ```c++
> // … code to handle when `url.path` is empty.
> 
> std::string path = url.path;
> std::size_t position = path.find('/', 1); 
> if (position != std::string::npos) {
>   path = path.substr(position);
> }
> 
> // Call the authorizer.
> ```
> 
> And we can add code to the authorizer module instead on how to handle 
> objects which encode paths (just like we dispatch to the endpoint handlers).

Forget what I said, I now understand what split with parameter does.


- Alexander


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


On April 25, 2016, 2:50 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> ---
> 
> (Updated April 25, 2016, 2:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto 
> c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 
> 40d93ea257d1df8d22eee8a21667db90d579a8fe 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 
> 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46670: Added deprecated alias for `--authenticate_frameworks` master flag.

[Vinod Kone]

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

(Updated April 26, 2016, 2:24 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

empty diff update to just re-kick reviewbot.


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


Repository: mesos


Description
---

Added deprecated alias for `--authenticate_frameworks` master flag.


Diffs (updated)
-

  src/master/flags.cpp c0c9e924e876175b75a174e375a4c993d97e18ee 

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


Testing
---

make check

Teste manually by running ./bin/mesos-master.sh --authenticate


Thanks,

Vinod Kone

Re: Review Request 46665: Added 0.26.2 to the CHANGELOG.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 25, 2016, 11:08 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46665/
> ---
> 
> (Updated April 25, 2016, 11:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reflects the backport of MESOS-4705 to the 0.26.x branch.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 
> 
> Diff: https://reviews.apache.org/r/46665/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 46664: Added 0.27.3 to the CHANGELOG.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 25, 2016, 11:08 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46664/
> ---
> 
> (Updated April 25, 2016, 11:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reflects the backport of MESOS-4705 to the 0.27.x branch.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 
> 
> Diff: https://reviews.apache.org/r/46664/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 46663: Added 0.28.2 to the CHANGELOG.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 25, 2016, 11:08 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46663/
> ---
> 
> (Updated April 25, 2016, 11:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reflects the backport of MESOS-4705 to the 0.28.x branch.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 
> 
> Diff: https://reviews.apache.org/r/46663/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 46620: Refactored Flags to store name in `Name` struct instead of string.

[Vinod Kone]

> On April 25, 2016, 2:48 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp, line 45
> > 
> >
> > If this was `const` it would be much easier to reason how this works 
> > with all the usage sites.

const makes it harder to use the Flag (which contains the Name) in a map; 
because the assignment operator is deleted.


- Vinod


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


On April 25, 2016, 5:43 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46620/
> ---
> 
> (Updated April 25, 2016, 5:43 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-5271
> https://issues.apache.org/jira/browse/MESOS-5271
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sets up the stage to add metadata to the name (e.g., deprecated).
> This will be used to add deprecation support subsequently.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
> 2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 
> 
> Diff: https://reviews.apache.org/r/46620/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 46620: Refactored Flags to store name in `Name` struct instead of string.

[Vinod Kone]

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

(Updated April 26, 2016, 12:54 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Used stringify(name) instead of name.value in string concatenations.


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


Repository: mesos


Description
---

This sets up the stage to add metadata to the name (e.g., deprecated).
This will be used to add deprecation support subsequently.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flag.hpp 
2502d9e8a515b9adc1b3aa2f719e5710a0e7ef29 
  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
c3cbcdb781e1c282d381de1ad2bf4f386ee1db21 

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


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 46588: Added URI struct to stout.

[Joseph Wu]

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

(Updated April 25, 2016, 5:59 p.m.)


Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


Changes
---

Fixed user_info parsing (which is much more expressive than the previous diff 
allowed).
Added leading slash for `path`, which is more intuitive.
Added surrounding brackets for IPv6 hosts.
Fixed typo for fragment parsing.

Added a ton of tests.


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


Repository: mesos


Description
---

This will replace the `mesos::URI` protobuf currently used by the 
`uri::Fetcher`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
08ede41dcedc755933d656de58d93796e657749d 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/uri.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/uri_tests.cpp PRE-CREATION 

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


Testing
---

make check (OSX) GTEST_FILTER="URITest*"


Ran a clean build (make check) on:

* Ubuntu 12, 14, 15
* CentOS 6, 7
* Debian 8


Thanks,

Joseph Wu

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On April 25, 2016, 10:56 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46618/
> ---
> 
> (Updated April 25, 2016, 10:56 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added containerId to failure message when there are duplicate volumes
> in one container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 4675c767a562a3ca7f99265c9198a37786933c2d 
> 
> Diff: https://reviews.apache.org/r/46618/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46501: Updated authorization.md to reflect current changes.

[Greg Mann]

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




docs/authorization.md (line 12)


s/JSON based/JSON-based/

It might be helpful at the beginning to give a broad picture of how these 
ACLs work, something like: "When authorizing an action, Mesos proceeds through 
the list of relevant ACLs until it finds one that can either grant or deny 
permission to the  subject making the request."



docs/authorization.md (line 14)


I would prefer to avoid mentioning 'acls.proto' here; I think that we 
should try to create documentation that provides adequate guidance for users 
without referring them to the code itself.

I think that we could provide a schema here, or we might be able to get rid 
of this passage (except for the explanation of the `permissive` field). The two 
passages following this one do a good job of explaining the structure of the 
ACLs.

What do you think?



docs/authorization.md (lines 16 - 18)


s/consist on an array/consists of an array/
s/Each of this objects/Each of these objects/

I would recommend splitting the second sentence into three: "Each of these 
objects has two entries. The first, `principals`, is common to all actions and 
describes the subjects which wish to perform the given action. The second entry 
varies among actions and describes the object on which the action will be 
executed."

I would prefer to avoid mentioning the `Entity` protobuf message directly. 
I think it would be easier to understand if we simply describe the 
characteristics of the subject and object fields, as you do in line 18, without 
mentioning `Entity`. What do you think?



docs/authorization.md (line 21)


s/which is default/which is the default/
s/In case permissive/If permissive/
s/set to false all/set to false, all/
s/non-matching request/non-matching requests/



docs/authorization.md (line 24)


The ACL doesn't imply that `foo` can register in no other role besides 
`analytics`. If `foo` tries to register in another role, it won't match any 
ACL, and since `permissive` is true by default, the request will be authorized.



docs/authorization.md (line 49)


s/set to `false` and hence principals/set to `false`; hence, principals/

/but not as any other user/and not as any other user/



docs/authorization.md (lines 51 - 61)


I think the doc would benefit from a couple more examples here. An example 
showcasing the order of ACLs might be helpful (i.e., a case where the order of 
the ACLs really matters). Also, perhaps an example with multiple 
principals/objects in those JSON lists?



docs/authorization.md (line 65)


s/Currently _local authorizer_/Currently the _local authorizer_/

s/for following/for the following/



docs/authorization.md (line 92)


I would change the actions to be consistent in tense. The preceding items 
use the progressive tense - i.e., "Destroying quotas" - while this one doesn't 
- "Reserve resources".



docs/authorization.md (line 94)


Could also be an operator



docs/authorization.md (line 99)


Can also be an operator



docs/authorization.md (line 104)


Can also be an operator



docs/authorization.md (line 109)


Can also be an operator



docs/authorization.md (line 117)


s/Implementing Authorizer/Implementing an Authorizer/



docs/authorization.md (line 119)


Is this section targeted at people who will write their own authorizer 
modules? If so, we could make this more explicit like so: "In case you plan to 
implement your own authorizer module, the authorization interface consists of 
two parts:"



docs/authorization.md (line 120)


s/First the/First, the/
s/interface defining the/interface defines the/
s/and _local authorizer_/and the _local authorizer_/
s/In case the request ... otherwise./This function should return `false` if 
the request is rejected and `true` otherwise./



docs/authorization.md (line 122)

Re: Review Request 46220: Added documentation for Nvidia GPU support.

[Qian Zhang]

> On April 22, 2016, 5:55 p.m., Qian Zhang wrote:
> > docs/gpu-support.md, line 440
> > 
> >
> > Does this limitation mean that currently we do not support container 
> > with an image (e.g., Docker image, Appc image)? Instead we only support 
> > image-less container since such kind of containers will be able to directly 
> > access agent host filesystem so that they can access the needed GPU library 
> > to perform GPU-aware jobs.
> 
> Kevin Klues wrote:
> Yes. This is what it means currently.  However, I am planning to remove 
> this restriction before the 0.29 release.
> 
> Qian Zhang wrote:
> Can you please let me know a bit more about why removing this restriction 
> before 0.29 release? Do we already have a solution to make the container with 
> an image can access the GPU library?

@Kevin, any comments? :-)


- Qian


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


On April 15, 2016, 4:33 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46220/
> ---
> 
> (Updated April 15, 2016, 4:33 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5221
> https://issues.apache.org/jira/browse/MESOS-5221
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for Nvidia GPU support.
> 
> 
> Diffs
> -
> 
>   docs/gpu-support.md PRE-CREATION 
>   docs/home.md dc41fc4479e6c23650cd8ac78dcc4b9161d00721 
> 
> Diff: https://reviews.apache.org/r/46220/diff/
> 
> 
> Testing
> ---
> 
> Ran a local copy of the documentation site to make sure everything looks OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>