Re: Review Request 56809: Added test for nested container machine reboot case.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56808, 56809]

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

- Mesos Reviewbot


On Feb. 18, 2017, 1:55 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56809/
> ---
> 
> (Updated Feb. 18, 2017, 1:55 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for nested container machine reboot case.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> ea01fe55a28d17105157004d8cf0976202a49b7c 
> 
> Diff: https://reviews.apache.org/r/56809/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 56806: Fixed some clang-tidy warnings.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56806]

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

- Mesos Reviewbot


On Feb. 18, 2017, 12:18 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56806/
> ---
> 
> (Updated Feb. 18, 2017, 12:18 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mostly avoiding spurious copies.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 442184920521f372c535426f49155cb19980bf6d 
>   src/master/detector/detector.cpp 1ebe5af4219a7e1ec1cbf22b440dd3ecaad4b4a9 
>   src/slave/slave.cpp 7564e8d39530794131dbbc928fcbc59fb65ef471 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 31d63b1f239055d82470ace9024b584a2096dce4 
>   src/tests/fetcher_cache_tests.cpp e5eee9bce56edacb74ad80ff859ddbc5048c73d0 
>   src/tests/hook_tests.cpp 237df8102941cd143c4d61016963e62a69c43382 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> ec8df33a3f0d4ad790d5cc753e1691c5517c39c0 
> 
> Diff: https://reviews.apache.org/r/56806/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 56624: Updated libprocess tests to use 'AuthenticationContext'.

[Greg Mann]

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

(Updated Feb. 18, 2017, 3:04 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the HTTP-related libprocess
tests to use authenticated handlers which accept
an `AuthenticationContext` instead of an
`Option principal`.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
fb4da9aecff0370d97a15269c5d8fffb30e0478f 

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


Testing
---

Testing information can be found at the end of this review chain.


Thanks,

Greg Mann

Review Request 56813: Updated master handlers to use 'AuthenticationContext'.

[Greg Mann]

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

Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the HTTP endpoint handlers in the
agent process to accept an `AuthenticationContext`
instead of an `Option& principal`.


Diffs
-

  src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 0f9fc39c1bd2af032f908d0263ed6a592ef1771c 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 

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


Testing
---

Testing details can be found at the end of this review chain.


Thanks,

Greg Mann

Review Request 56812: Updated agent handlers to use 'AuthenticationContext'.

[Greg Mann]

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

Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the HTTP endpoint handlers in the
agent process to accept an `AuthenticationContext`
instead of an `Option& principal`.


Diffs
-

  src/slave/http.cpp af70b6f294a04f23b04cd1d8c36c1c3e86d7d5e6 
  src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 
  src/slave/slave.cpp 7564e8d39530794131dbbc928fcbc59fb65ef471 

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


Testing
---

Testing details can be found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 56619: Updated 'Files' handlers to use 'AuthenticationContext'.

[Greg Mann]

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

(Updated Feb. 18, 2017, 3 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


Changes
---

Split endpoint changes into 3 separate patches.


Summary (updated)
-

Updated 'Files' handlers to use 'AuthenticationContext'.


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


Repository: mesos


Description (updated)
---

This patch updates the HTTP endpoint handlers in the
Mesos `Files` process to accept an `AuthenticationContext`
instead of an `Option& principal`.


Diffs (updated)
-

  src/files/files.hpp 8cffc26fc7d674187e55663f23f1e10bed40229e 
  src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a 

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


Testing
---

Testing information can be found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 56623: Implemented 'AuthenticationContext' in libprocess.

[Greg Mann]

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

(Updated Feb. 18, 2017, 2:54 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


Changes
---

Made use of `jsonify` in `operator<<`.


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


Repository: mesos


Description
---

This patch adds a new struct, `AuthenticationContext`, to
libprocess to represent an authenticated entity in the system.
The new type contains a `principal` and a map containing
arbitrary key-value pairs.


Diffs (updated)
-

  3rdparty/libprocess/include/process/authenticator.hpp 
e5489c6cb4adc8a822e7dd4515542618c36136f9 
  3rdparty/libprocess/include/process/http.hpp 
3a406963c1defc037d58971f2400e40855503fbb 
  3rdparty/libprocess/include/process/process.hpp 
ceedbe86ccba5c24bf1e9e7f032d7dbcf6acb0f3 
  3rdparty/libprocess/src/authenticator.cpp 
cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7 
  3rdparty/libprocess/src/authenticator_manager.cpp 
a22acd026a001788dc39b8005a56577e33c6800b 
  3rdparty/libprocess/src/http.cpp 0e37936eec5b2910e54d40788c26f98665cbe1f1 
  3rdparty/libprocess/src/process.cpp 3ad485f9136cd9edf0a6db76ff1f475039236391 

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


Testing
---

Testing information can be found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 56623: Implemented 'AuthenticationContext' in libprocess.

[Greg Mann]

> On Feb. 16, 2017, 10:45 a.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 87-109
> > 
> >
> > Why not going the fancy way and use `jsonify()`?
> 
> Alexander Rojas wrote:
> Just add this function in the same namespace as the 
> `AuthorizationContext`:
> 
> ```c++
> static void json(JSON::ObjectWriter* writer, const AuthorizationContext& 
> context)
> {
>   if (context.principal.isSome()) {
> writer->field("principal", context.principal.get());
>   }
>   if (context.labels.isSome()) {
> writer->field("labels", context.labels.get());
>   }
> }
> ```
> 
> Then you can implement the output stream operator as:
> 
> ```c++
> std::ostream& operator<<(
> std::ostream& stream, const AuthenticationContext& context)
> {
>   stream << jsonify(context);
>   return streaml
> }
> ```

Awesome, thx Alexander!!


- Greg


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


On Feb. 17, 2017, 10:33 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56623/
> ---
> 
> (Updated Feb. 17, 2017, 10:33 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new struct, `AuthenticationContext`, to
> libprocess to represent an authenticated entity in the system.
> The new type contains a `principal` and a map containing
> arbitrary key-value pairs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> e5489c6cb4adc8a822e7dd4515542618c36136f9 
>   3rdparty/libprocess/include/process/http.hpp 
> 3a406963c1defc037d58971f2400e40855503fbb 
>   3rdparty/libprocess/include/process/process.hpp 
> ceedbe86ccba5c24bf1e9e7f032d7dbcf6acb0f3 
>   3rdparty/libprocess/src/authenticator.cpp 
> cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7 
>   3rdparty/libprocess/src/authenticator_manager.cpp 
> a22acd026a001788dc39b8005a56577e33c6800b 
>   3rdparty/libprocess/src/http.cpp 0e37936eec5b2910e54d40788c26f98665cbe1f1 
>   3rdparty/libprocess/src/process.cpp 
> 3ad485f9136cd9edf0a6db76ff1f475039236391 
> 
> Diff: https://reviews.apache.org/r/56623/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56238: Allowed using protobuf 3 in python bindings.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 18, 2017, 2:23 a.m., Anthony Sottile wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56238/
> ---
> 
> (Updated Feb. 18, 2017, 2:23 a.m.)
> 
> 
> Review request for mesos, Deshi Xiao, Yong Tang, and Zhitao Li.
> 
> 
> Bugs: MESOS-5186 and MESOS-6138
> https://issues.apache.org/jira/browse/MESOS-5186
> https://issues.apache.org/jira/browse/MESOS-6138
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed using protobuf 3 in python bindings.
> 
> 
> Diffs
> -
> 
>   src/python/interface/setup.py.in 037c2ec8e63f497f7029a847a7a0d7b72e6f36fa 
>   src/python/protocol/setup.py.in 5dd4a0a0f67c4003b58319d0d6c7bb227e8a13a1 
> 
> Diff: https://reviews.apache.org/r/56238/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Sottile
> 
>

Re: Review Request 56238: Allowed using protobuf 3 in python bindings.

[Anthony Sottile]

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

(Updated Feb. 18, 2017, 2:23 a.m.)


Review request for mesos, Deshi Xiao, Yong Tang, and Zhitao Li.


Bugs: MESOS-5186 and MESOS-6138
https://issues.apache.org/jira/browse/MESOS-5186
https://issues.apache.org/jira/browse/MESOS-6138


Repository: mesos


Description (updated)
---

Allowed using protobuf 3 in python bindings.


Diffs
-

  src/python/interface/setup.py.in 037c2ec8e63f497f7029a847a7a0d7b72e6f36fa 
  src/python/protocol/setup.py.in 5dd4a0a0f67c4003b58319d0d6c7bb227e8a13a1 

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


Testing
---


Thanks,

Anthony Sottile

Re: Review Request 56238: Allowed using protobuf 3 in python bindings.

[Anthony Sottile]

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

(Updated Feb. 18, 2017, 2:23 a.m.)


Review request for mesos, Deshi Xiao, Yong Tang, and Zhitao Li.


Summary (updated)
-

Allowed using protobuf 3 in python bindings.


Bugs: MESOS-5186 and MESOS-6138
https://issues.apache.org/jira/browse/MESOS-5186
https://issues.apache.org/jira/browse/MESOS-6138


Repository: mesos


Description
---

Allow a newer version of google protobuf.


Diffs
-

  src/python/interface/setup.py.in 037c2ec8e63f497f7029a847a7a0d7b72e6f36fa 
  src/python/protocol/setup.py.in 5dd4a0a0f67c4003b58319d0d6c7bb227e8a13a1 

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


Testing
---


Thanks,

Anthony Sottile

Review Request 56809: Added test for nested container machine reboot case.

[Gilbert Song]

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

Review request for mesos, Avinash sridharan, Jie Yu, and Kevin Klues.


Repository: mesos


Description
---

Added test for nested container machine reboot case.


Diffs
-

  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
ea01fe55a28d17105157004d8cf0976202a49b7c 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 56808: Fixed nested container agent flapping issue after reboot.

[Gilbert Song]

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

Review request for mesos, Avinash sridharan, Jie Yu, and Kevin Klues.


Repository: mesos


Description
---

Fixed nested container agent flapping issue after reboot.


Diffs
-

  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
ff52e35ac44fea70fe2031e6ac537c613c57f50f 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
8c20d4077859d437999467d045dfd500c1e576dd 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 56805: Simplified interface for setting weights in allocator.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56804, 56805]

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

- Mesos Reviewbot


On Feb. 18, 2017, 12:14 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56805/
> ---
> 
> (Updated Feb. 18, 2017, 12:14 a.m.)
> 
> 
> Review request for mesos, Adam B and Yongqiao Wang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now that weights can change dynamically, passing a list of weights to
> the allocator at initialization-time is unnecessary and confusing (e.g.,
> the initial weights might be wrong, if a different set of weight values
> are recovered from the registry).
> 
> Instead, require that weights are communicated to the allocator via the
> existing `updateWeights` method.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 23413d1f9d76009c999b35d2bc98afc52c136404 
>   src/master/allocator/mesos/allocator.hpp 
> 38fbf16a0500fc9f6ce7a498a6af3d81d63fc215 
>   src/master/allocator/mesos/hierarchical.hpp 
> d33306745a7287b750cb4a5242c7527369d58d65 
>   src/master/allocator/mesos/hierarchical.cpp 
> eeb44fe89d4bfd26900b11833c1182157e5c7e5c 
>   src/master/master.cpp 5534674fa2740b0a6e37fc1df34b582f1e691133 
>   src/tests/allocator.hpp c741a985660e1540ad8e3a9c387d513247e56714 
>   src/tests/api_tests.cpp 7715da9072e7be81eb19e742857ff7ab3fd60ea5 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5441fa9d1fad1ca7819038db49c6d88e40571e4a 
>   src/tests/master_allocator_tests.cpp 
> 25c67d32eec5fede78eb9fcbc1009eeff7da0dad 
>   src/tests/master_quota_tests.cpp d15c7aacb85596cdee7cf59c0c179247ba624fe5 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> ec8df33a3f0d4ad790d5cc753e1691c5517c39c0 
>   src/tests/reservation_endpoints_tests.cpp 
> 7432d752120b43560aa96cad8bf3981ee8102e67 
>   src/tests/reservation_tests.cpp 309ce8b9acc9131110198c14c655427d7fe9d603 
>   src/tests/resource_offers_tests.cpp 
> 74dacf140e49e402a4ad02ce7751e7c7b2f78ee1 
>   src/tests/slave_recovery_tests.cpp 0e295915fea0a7314e173857249bd8726eeccd76 
> 
> Diff: https://reviews.apache.org/r/56805/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 56806: Fixed some clang-tidy warnings.

[Neil Conway]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Mostly avoiding spurious copies.


Diffs
-

  src/common/protobuf_utils.cpp 442184920521f372c535426f49155cb19980bf6d 
  src/master/detector/detector.cpp 1ebe5af4219a7e1ec1cbf22b440dd3ecaad4b4a9 
  src/slave/slave.cpp 7564e8d39530794131dbbc928fcbc59fb65ef471 
  src/tests/containerizer/docker_containerizer_tests.cpp 
31d63b1f239055d82470ace9024b584a2096dce4 
  src/tests/fetcher_cache_tests.cpp e5eee9bce56edacb74ad80ff859ddbc5048c73d0 
  src/tests/hook_tests.cpp 237df8102941cd143c4d61016963e62a69c43382 
  src/tests/persistent_volume_endpoints_tests.cpp 
ec8df33a3f0d4ad790d5cc753e1691c5517c39c0 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 56805: Simplified interface for setting weights in allocator.

[Neil Conway]

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

Review request for mesos, Adam B and Yongqiao Wang.


Repository: mesos


Description
---

Now that weights can change dynamically, passing a list of weights to
the allocator at initialization-time is unnecessary and confusing (e.g.,
the initial weights might be wrong, if a different set of weight values
are recovered from the registry).

Instead, require that weights are communicated to the allocator via the
existing `updateWeights` method.


Diffs
-

  include/mesos/allocator/allocator.hpp 
23413d1f9d76009c999b35d2bc98afc52c136404 
  src/master/allocator/mesos/allocator.hpp 
38fbf16a0500fc9f6ce7a498a6af3d81d63fc215 
  src/master/allocator/mesos/hierarchical.hpp 
d33306745a7287b750cb4a5242c7527369d58d65 
  src/master/allocator/mesos/hierarchical.cpp 
eeb44fe89d4bfd26900b11833c1182157e5c7e5c 
  src/master/master.cpp 5534674fa2740b0a6e37fc1df34b582f1e691133 
  src/tests/allocator.hpp c741a985660e1540ad8e3a9c387d513247e56714 
  src/tests/api_tests.cpp 7715da9072e7be81eb19e742857ff7ab3fd60ea5 
  src/tests/hierarchical_allocator_tests.cpp 
5441fa9d1fad1ca7819038db49c6d88e40571e4a 
  src/tests/master_allocator_tests.cpp 25c67d32eec5fede78eb9fcbc1009eeff7da0dad 
  src/tests/master_quota_tests.cpp d15c7aacb85596cdee7cf59c0c179247ba624fe5 
  src/tests/persistent_volume_endpoints_tests.cpp 
ec8df33a3f0d4ad790d5cc753e1691c5517c39c0 
  src/tests/reservation_endpoints_tests.cpp 
7432d752120b43560aa96cad8bf3981ee8102e67 
  src/tests/reservation_tests.cpp 309ce8b9acc9131110198c14c655427d7fe9d603 
  src/tests/resource_offers_tests.cpp 74dacf140e49e402a4ad02ce7751e7c7b2f78ee1 
  src/tests/slave_recovery_tests.cpp 0e295915fea0a7314e173857249bd8726eeccd76 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 56804: Cleaned up weights handling code.

[Neil Conway]

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

Review request for mesos, Adam B and Yongqiao Wang.


Repository: mesos


Description
---

Removed a redundant statement, fixed log message style, cleaned up
comments.


Diffs
-

  src/master/master.cpp 5534674fa2740b0a6e37fc1df34b582f1e691133 
  src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 
  src/tests/dynamic_weights_tests.cpp ce577ce6c71bbfffba7db7c72523a9fd2d68141b 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 1597)


We don't add period in log lines.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 1601 - 
1602)


I would still return Error here.


- Jie Yu


On Feb. 17, 2017, 3:51 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56493/
> ---
> 
> (Updated Feb. 17, 2017, 3:51 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7074
> https://issues.apache.org/jira/browse/MESOS-7074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since a lot of environments do not provide interface speed in
> /sys/class/net//speed`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> f6f2bfe1d5d3c113036ad44a480f97bbb462a269 
> 
> Diff: https://reviews.apache.org/r/56493/diff/
> 
> 
> Testing
> ---
> 
> ./configure
> make -j 8
> make check
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 56778: Fixed a bug around executor not able to use reserved resources.

[Jie Yu]

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



@anand, can you follow up on fixing the tests?


src/tests/default_executor_tests.cpp (lines 1321 - 1326)


Consistently using v1?



src/tests/default_executor_tests.cpp (line 1388)


Ditto.



src/tests/default_executor_tests.cpp (lines 1391 - 1392)


Why we still use evolve here? I think we already support v1::createTask?


- Jie Yu


On Feb. 17, 2017, 8:15 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56778/
> ---
> 
> (Updated Feb. 17, 2017, 8:15 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7137
> https://issues.apache.org/jira/browse/MESOS-7137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We were not unallocating the resources before checking if the
> executor resources were contained in the checkpointed resources
> on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ebba8e16bc9ec45781183e78cb5a3c351a5f65f5 
>   src/tests/default_executor_tests.cpp 
> ffb69e9d6745c267704195056500edd4d8a4ca3f 
> 
> Diff: https://reviews.apache.org/r/56778/diff/
> 
> 
> Testing
> ---
> 
> make check + added test that fails without the change.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 56619: Updated Mesos handlers to use 'AuthenticationContext'.

[Greg Mann]

> On Feb. 16, 2017, 3:56 p.m., Alexander Rojas wrote:
> > src/master/http.cpp, lines 796-798
> > 
> >
> > This appears here, there and everywhere, that I'm considering it may be 
> > a good idea to abstract it to something like:
> > 
> > ```
> > static Option AuthenticationContext::extractPrincipal(const 
> > Option& context);
> > ```
> > 
> > That will make it easier if one day we need to modify how the 
> > extraction works.
> 
> Greg Mann wrote:
> Good call, will do.

I think I would actually prefer to update the validation code to accept the 
`AuthenticationContext` instead of adding this helper method.


> On Feb. 16, 2017, 3:56 p.m., Alexander Rojas wrote:
> > src/master/http.cpp, lines 1479-1481
> > 
> >
> > likewise, this looks like a prime candidate to be abstracted away.

As I suggested in https://reviews.apache.org/r/56618/, let's avoid passing 
default-initialized `Subject`s at all, and instead use a helper which returns 
`Option`.


- Greg


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


On Feb. 17, 2017, 10:36 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56619/
> ---
> 
> (Updated Feb. 17, 2017, 10:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the HTTP endpoint handlers in Mesos
> to accept an `AuthenticationContext` instead of an
> `Option& principal`.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp 8cffc26fc7d674187e55663f23f1e10bed40229e 
>   src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a 
>   src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp 5534674fa2740b0a6e37fc1df34b582f1e691133 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 
>   src/slave/http.cpp af70b6f294a04f23b04cd1d8c36c1c3e86d7d5e6 
>   src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 
>   src/slave/slave.cpp ebba8e16bc9ec45781183e78cb5a3c351a5f65f5 
> 
> Diff: https://reviews.apache.org/r/56619/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56619: Updated Mesos handlers to use 'AuthenticationContext'.

[Greg Mann]

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

(Updated Feb. 17, 2017, 10:36 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the HTTP endpoint handlers in Mesos
to accept an `AuthenticationContext` instead of an
`Option& principal`.


Diffs (updated)
-

  src/files/files.hpp 8cffc26fc7d674187e55663f23f1e10bed40229e 
  src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a 
  src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 
  src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
  src/master/master.cpp 5534674fa2740b0a6e37fc1df34b582f1e691133 
  src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
  src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
  src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 
  src/slave/http.cpp af70b6f294a04f23b04cd1d8c36c1c3e86d7d5e6 
  src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 
  src/slave/slave.cpp ebba8e16bc9ec45781183e78cb5a3c351a5f65f5 

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


Testing
---

Testing information can be found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 56618: Updated common Mesos code to use 'AuthenticationContext'.

[Greg Mann]

> On Feb. 14, 2017, 3:12 p.m., Jan Schlicht wrote:
> > src/common/http.hpp, line 133
> > 
> >
> > How about using an `Option` here and returning 
> > `Subject()` in the case of `context.isNone()`?
> > All calls to this functions in the following patch are either
> > ```
> > authorization::Subject subject = context.isSome()
> >   ? createAuthorizationSubject(context.get())
> >   : authorization::Subject();
> > ```
> > or
> > ```
> > if (context.isSome()) {
> >   
> > request.mutable_subject()->CopyFrom(createAuthorizationSubject(context.get()));
> > }
> > ```
> > At least the first form would look much simpler and concise when 
> > changing the function signature this way:
> > ```
> > authorization::Subject subject = createAuthorizationSubject(context)
> > ```
> > What do you think?
> 
> Alexander Rojas wrote:
> We discussed this, and the semantics of a default created 
> `authorization::Subject` are different from that of a non setted subject, 
> therefore this is a bad idea. However, there may be a related bug already 
> inside Mesos authorizer for not paying attention to the different semantics. 
> We still need to build a test for it.
> 
> Jan Schlicht wrote:
> Okay, so the first example wouldn't be concerned by this, because it is 
> always creating an `authorization::Subject`. Of course, this is true for the 
> second example, we want to differ between `Request::subject` being set or 
> not. Still, changing the signature and doing
> ```
> if (context.isSome()) {
>   
> request.mutable_subject()->CopyFrom(createAuthorizationSubject(context));
> }
> ```
> would make sure of that.

After looking at the callsites in our handlers a bit more, I think we have the 
following two cases:

1) We want to call `getObjectApprover`, which accepts an 
`Option`
2) We want to set the `subject` of an authorization request conditionally, only 
when `context.isSome()`

To accommodate these two cases, I think it's actually beneficial to have two 
different helpers; one of which returns an `authorization::Subject`, while the 
other returns `Option`. I've updated the patches to 
include two functions, `createSubject` and `createOptionalSubject`, to handle 
these cases. Let me know what you guys think!


- Greg


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


On Feb. 17, 2017, 10:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56618/
> ---
> 
> (Updated Feb. 17, 2017, 10:34 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates common Mesos HTTP-related helpers,
> as well as the `authorization::Subject` protobuf
> message, to make use of the `AuthenticationContext`
> type instead of an `Option principal`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 9cc75b0db17b2d0bab3f449f795cbf505c5b0f15 
>   src/common/http.hpp 3d5ab59ddc4dce4d791c1b439f5e1459d1a724a4 
>   src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e 
> 
> Diff: https://reviews.apache.org/r/56618/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56618: Updated common Mesos code to use 'AuthenticationContext'.

[Greg Mann]

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

(Updated Feb. 17, 2017, 10:34 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates common Mesos HTTP-related helpers,
as well as the `authorization::Subject` protobuf
message, to make use of the `AuthenticationContext`
type instead of an `Option principal`.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
9cc75b0db17b2d0bab3f449f795cbf505c5b0f15 
  src/common/http.hpp 3d5ab59ddc4dce4d791c1b439f5e1459d1a724a4 
  src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e 

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


Testing
---

Testing information can be found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 56623: Implemented 'AuthenticationContext' in libprocess.

[Greg Mann]

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

(Updated Feb. 17, 2017, 10:33 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
Toenshoff, and Vinod Kone.


Changes
---

Added a comment.


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


Repository: mesos


Description
---

This patch adds a new struct, `AuthenticationContext`, to
libprocess to represent an authenticated entity in the system.
The new type contains a `principal` and a map containing
arbitrary key-value pairs.


Diffs (updated)
-

  3rdparty/libprocess/include/process/authenticator.hpp 
e5489c6cb4adc8a822e7dd4515542618c36136f9 
  3rdparty/libprocess/include/process/http.hpp 
3a406963c1defc037d58971f2400e40855503fbb 
  3rdparty/libprocess/include/process/process.hpp 
ceedbe86ccba5c24bf1e9e7f032d7dbcf6acb0f3 
  3rdparty/libprocess/src/authenticator.cpp 
cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7 
  3rdparty/libprocess/src/authenticator_manager.cpp 
a22acd026a001788dc39b8005a56577e33c6800b 
  3rdparty/libprocess/src/http.cpp 0e37936eec5b2910e54d40788c26f98665cbe1f1 
  3rdparty/libprocess/src/process.cpp 3ad485f9136cd9edf0a6db76ff1f475039236391 

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


Testing
---

Testing information can be found at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 56611: Relax perf version check for Arch Linux.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On Feb. 14, 2017, 3:59 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56611/
> ---
> 
> (Updated Feb. 14, 2017, 3:59 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-6982
> https://issues.apache.org/jira/browse/MESOS-6982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Relax perf version check for Arch Linux.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.hpp 9c4330b6086abb18f03660fe89a6fb01d8ed 
>   src/linux/perf.cpp 3141e5ee9eee78e974625791f362dc345c682ebb 
>   src/tests/containerizer/perf_tests.cpp 
> d536ecc5cb24787bc3487efb146573cd4f3ded43 
> 
> Diff: https://reviews.apache.org/r/56611/diff/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56618: Updated common Mesos code to use 'AuthenticationContext'.

[Greg Mann]

> On Feb. 16, 2017, 10:56 a.m., Alexander Rojas wrote:
> > src/common/http.cpp, line 708
> > 
> >
> > I have the feeling that `extractAuthorizationSubject` may be a better 
> > way of describing what is happening here.
> > 
> > I wonder if there is a way of introducing this to the protobuf 
> > generated `Subject` class so this is not a free function.

I'm not really a fan of `extractAuthorizationSubject`, since there isn't an 
actual `authorization::Subject` within the context type, we're constructing a 
new one and returning it.

I don't think it's worth messing with the protobuf classes to avoid a free 
function. I'll namespace the creation functions to improve the isolation.


- Greg


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


On Feb. 13, 2017, 11:46 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56618/
> ---
> 
> (Updated Feb. 13, 2017, 11:46 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates common Mesos HTTP-related helpers,
> as well as the `authorization::Subject` protobuf
> message, to make use of the `AuthenticationContext`
> type instead of an `Option principal`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 8b860a3e8e0b1c660a8fefc97f10f5acc0501920 
>   src/common/http.hpp 3d5ab59ddc4dce4d791c1b439f5e1459d1a724a4 
>   src/common/http.cpp abfbf7248beb2d4068be06b7f5f829d7617f943e 
> 
> Diff: https://reviews.apache.org/r/56618/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 50974: Documented all the API calls for Operator HTTP API.

[Vinod Kone]

> On Jan. 31, 2017, 7:44 p.m., Vinod Kone wrote:
> >

I've commited this and fixed the issues myself. Thanks for the patch!


- Vinod


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


On Nov. 28, 2016, 6:50 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50974/
> ---
> 
> (Updated Nov. 28, 2016, 6:50 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5992
> https://issues.apache.org/jira/browse/MESOS-5992
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented all the API calls for Operator HTTP API.
> 
> 
> Diffs
> -
> 
>   docs/operator-http-api.md 4f4c39e7b4b6de32af1933c34eba21f126fae8ac 
> 
> Diff: https://reviews.apache.org/r/50974/diff/
> 
> 
> Testing
> ---
> 
> Checked the generated page through "rake dev".
> Validated and formatted all the JSON snippets with:
> http://jsonlint.com/
> http://jsonviewer.stack.hu/
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 56778: Fixed a bug around executor not able to use reserved resources.

[Anand Mazumdar]

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

(Updated Feb. 17, 2017, 8:15 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Review comments, NNFR.


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


Repository: mesos


Description
---

We were not unallocating the resources before checking if the
executor resources were contained in the checkpointed resources
on the agent.


Diffs (updated)
-

  src/slave/slave.cpp ebba8e16bc9ec45781183e78cb5a3c351a5f65f5 
  src/tests/default_executor_tests.cpp ffb69e9d6745c267704195056500edd4d8a4ca3f 

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


Testing
---

make check + added test that fails without the change.


Thanks,

Anand Mazumdar

Re: Review Request 56778: Fixed a bug around executor not able to use reserved resources.

[Anand Mazumdar]

> On Feb. 17, 2017, 7:41 p.m., Vinod Kone wrote:
> > src/tests/default_executor_tests.cpp, lines 1416-1417
> > 
> >
> > hmm. why didn't you just start with a v1::ExecutorInfo like you did 
> > with v1::TaskGroup?

Mostly to be consistent with the other tests in the file. Would do a sweep 
later.


> On Feb. 17, 2017, 7:41 p.m., Vinod Kone wrote:
> > src/tests/default_executor_tests.cpp, line 1317
> > 
> >
> > Is this still needed? Thought we allowed implicit roles now?

good catch, I copy/pasted it from the reservation tests. Would fix it here and 
thereafter need to do a sweep there too.


> On Feb. 17, 2017, 7:41 p.m., Vinod Kone wrote:
> > src/tests/default_executor_tests.cpp, lines 1324-1335
> > 
> >
> > move this to #1380

All the other tests in this file are consistent with doing it here though. 
Would do a sweep.


- Anand


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


On Feb. 17, 2017, 5:24 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56778/
> ---
> 
> (Updated Feb. 17, 2017, 5:24 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7137
> https://issues.apache.org/jira/browse/MESOS-7137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We were not unallocating the resources before checking if the
> executor resources were contained in the checkpointed resources
> on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ebba8e16bc9ec45781183e78cb5a3c351a5f65f5 
>   src/tests/default_executor_tests.cpp 
> ffb69e9d6745c267704195056500edd4d8a4ca3f 
> 
> Diff: https://reviews.apache.org/r/56778/diff/
> 
> 
> Testing
> ---
> 
> make check + added test that fails without the change.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 56692: Silenced a GMock warning in a test.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Feb. 17, 2017, 11:35 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56692/
> ---
> 
> (Updated Feb. 17, 2017, 11:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Silenced a GMock warning in a test.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 717673875cd468c23065766438723c3e0fa0627e 
> 
> Diff: https://reviews.apache.org/r/56692/diff/
> 
> 
> Testing
> ---
> 
> Without this change, the test consistently produces this warning on my system:
> 
> ```
> GMOCK WARNING:
> Uninteresting mock function call - returning directly.
> Function call: shutdown(0x7fe1f0e7d330)
> ```
> 
> With this change, the warning was not observed in 500 test runs.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 56781: Fixed compilation on VS 2017.

[Michael Park]

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

(Updated Feb. 17, 2017, 12:07 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Joseph Wu.


Changes
---

Addressed Alex's comment.


Repository: mesos


Description (updated)
---

Fixed compilation on VS 2017.


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
819ee5ceb8f2900087c2a06d5b1df0a1aeb413f6 

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


Testing
---

Tested on VS 2015 and 2017.


Thanks,

Michael Park

Re: Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 16, 2017, 5:57 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56053/
> ---
> 
> (Updated Feb. 16, 2017, 5:57 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Kapil Arya, Jan Schlicht, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7009
> https://issues.apache.org/jira/browse/MESOS-7009
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a field of type `Secret` to the
> `Environment` protobuf message, enabling the passing
> of secrets into the environments of executors and
> tasks. Additional validation and test code is added
> as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 34a288b6f5c2fca090a7aa7a61798e3255d6663a 
>   include/mesos/v1/mesos.proto 6638111d10f4a36cdf91dfce1019871e9839c306 
>   src/common/validation.hpp 7946f06cf17c05893ff80179c9d14b5f1f4023ef 
>   src/common/validation.cpp 0f1a02286d8431acfee6136e8ada49b0ac746897 
>   src/tests/check_tests.cpp b5a5d8e80c80b480992a3c8160ee7d0e4443111c 
>   src/tests/health_check_tests.cpp 476e04a12f885e5060a6e838b82ee599594c96f7 
>   src/tests/master_validation_tests.cpp 
> fd1f4a6cf0661351e265e50da1bd6ea04ed13d26 
>   src/tests/slave_validation_tests.cpp 
> 3d17799ed04951fb56524db0f5d89347192300b2 
> 
> Diff: https://reviews.apache.org/r/56053/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56778: Fixed a bug around executor not able to use reserved resources.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/default_executor_tests.cpp (line 1317)


Is this still needed? Thought we allowed implicit roles now?



src/tests/default_executor_tests.cpp (lines 1324 - 1335)


move this to #1380



src/tests/default_executor_tests.cpp (lines 1416 - 1417)


hmm. why didn't you just start with a v1::ExecutorInfo like you did with 
v1::TaskGroup?


- Vinod Kone


On Feb. 17, 2017, 5:24 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56778/
> ---
> 
> (Updated Feb. 17, 2017, 5:24 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7137
> https://issues.apache.org/jira/browse/MESOS-7137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We were not unallocating the resources before checking if the
> executor resources were contained in the checkpointed resources
> on the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ebba8e16bc9ec45781183e78cb5a3c351a5f65f5 
>   src/tests/default_executor_tests.cpp 
> ffb69e9d6745c267704195056500edd4d8a4ca3f 
> 
> Diff: https://reviews.apache.org/r/56778/diff/
> 
> 
> Testing
> ---
> 
> make check + added test that fails without the change.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 56692: Silenced a GMock warning in a test.

[Neil Conway]

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

(Updated Feb. 17, 2017, 7:35 p.m.)


Review request for mesos, Benjamin Bannier and Michael Park.


Changes
---

Add comment, per review.


Repository: mesos


Description
---

Silenced a GMock warning in a test.


Diffs (updated)
-

  src/tests/master_validation_tests.cpp 
717673875cd468c23065766438723c3e0fa0627e 

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


Testing
---

Without this change, the test consistently produces this warning on my system:

```
GMOCK WARNING:
Uninteresting mock function call - returning directly.
Function call: shutdown(0x7fe1f0e7d330)
```

With this change, the warning was not observed in 500 test runs.


Thanks,

Neil Conway

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56493]

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

- Mesos Reviewbot


On Feb. 17, 2017, 3:51 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56493/
> ---
> 
> (Updated Feb. 17, 2017, 3:51 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7074
> https://issues.apache.org/jira/browse/MESOS-7074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since a lot of environments do not provide interface speed in
> /sys/class/net//speed`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> f6f2bfe1d5d3c113036ad44a480f97bbb462a269 
> 
> Diff: https://reviews.apache.org/r/56493/diff/
> 
> 
> Testing
> ---
> 
> ./configure
> make -j 8
> make check
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 56493: Port_mapping isolator: do not depend on interface speed.

[Pierre Cheynier]

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

(Updated Feb. 17, 2017, 3:51 p.m.)


Review request for mesos and Jie Yu.


Changes
---

One of the case was not well handled due to a syntax error (missing comma)


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


Repository: mesos


Description
---

Since a lot of environments do not provide interface speed in
/sys/class/net//speed`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
f6f2bfe1d5d3c113036ad44a480f97bbb462a269 

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


Testing
---

./configure
make -j 8
make check


Thanks,

Pierre Cheynier

Re: Review Request 56413: WIP Support pausing/resuming health checks.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55900, 56288, 55901, 56413]

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

- Mesos Reviewbot


On Feb. 17, 2017, 12:01 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56413/
> ---
> 
> (Updated Feb. 17, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6280
> https://issues.apache.org/jira/browse/MESOS-6280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP Support pausing/resuming health checks.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp f1f2834b3429fb00cc49c179fa9a3de328f597b5 
>   src/checks/health_checker.cpp 6c97369fd9a567ba16dd92085bf142d43f71eaf1 
>   src/docker/executor.cpp d8c72b081fb9a37b3927b73cea3b47956a076691 
>   src/launcher/default_executor.cpp d97324b0e5eb3f145ee93aa0821205c5eefbfce0 
>   src/launcher/executor.cpp d9417ce1d5b108f5292a66010eab80f11552a969 
> 
> Diff: https://reviews.apache.org/r/56413/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 56413: WIP Support pausing/resuming health checks.

[Gastón Kleiman]

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

(Updated Feb. 17, 2017, 12:01 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, 
and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

WIP Support pausing/resuming health checks.


Diffs (updated)
-

  src/checks/health_checker.hpp f1f2834b3429fb00cc49c179fa9a3de328f597b5 
  src/checks/health_checker.cpp 6c97369fd9a567ba16dd92085bf142d43f71eaf1 
  src/docker/executor.cpp d8c72b081fb9a37b3927b73cea3b47956a076691 
  src/launcher/default_executor.cpp d97324b0e5eb3f145ee93aa0821205c5eefbfce0 
  src/launcher/executor.cpp d9417ce1d5b108f5292a66010eab80f11552a969 

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


Testing
---


Thanks,

Gastón Kleiman

Re: Review Request 55901: Added support for command health checks to the default executor.

[Gastón Kleiman]

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

(Updated Feb. 17, 2017, 11:59 a.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, 
and Vinod Kone.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

Added support for command health checks to the default executor.


Diffs (updated)
-

  src/checks/health_checker.hpp f1f2834b3429fb00cc49c179fa9a3de328f597b5 
  src/checks/health_checker.cpp 6c97369fd9a567ba16dd92085bf142d43f71eaf1 
  src/launcher/default_executor.cpp d97324b0e5eb3f145ee93aa0821205c5eefbfce0 
  src/tests/health_check_tests.cpp 476e04a12f885e5060a6e838b82ee599594c96f7 

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


Testing
---

Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
passes on Linux, but not on macOS, because of MESOS-7050.


Thanks,

Gastón Kleiman

Re: Review Request 56537: Added a test to ensure framework can upgrade to support MULTI_ROLE.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/tests/master_tests.cpp (line 6406)


`s/slave/agent/`



src/tests/master_tests.cpp (line 6416)


Should we make explicit that this future will be ready, e.g., by awaiting 
it right before we await `offers`?



src/tests/master_tests.cpp (line 6428)


I believe this needs to be an assertion, e.g.,

ASSERT_FALSE(offers->empty());



src/tests/master_tests.cpp (lines 6433 - 6435)


Since we are working on the default-constructed fields, `MergeFrom` is 
identical to `CopyFrom` here, right?



src/tests/master_tests.cpp (line 6438)


You can drop `Times(1)` here as it is the default.



src/tests/master_tests.cpp (lines 6450 - 6452)


`s/status.get()./status->/g'



src/tests/master_tests.cpp (lines 6455 - 6456)


We can make explicit that we keep the same role and avoid a hardcoded value 
by inverting setting `roles` and clearing `role`, e.g.,

frameworkInfo.add_roles(frameworkInfo.role());
frameworkInfo.clear_role();



src/tests/master_tests.cpp (lines 6482 - 6489)


Seems like just using

driver2.reconcileTasks({status.get()});

would be identical, but hardcode less information.



src/tests/master_tests.cpp (line 6492)


`explicitReconciliation->state()`



src/tests/master_tests.cpp (lines 6494 - 6495)


`driver1` would be inactive as soon as `registered2` above is ready. We 
could move this block just after that to create a bracket around the second 
registration,

EXPECT_CALL(exec, shutdown(_));

driver2.start();

AWAIT_READY(registered2);
  
driver1.stop();
driver1.join();

I believe stopping `driver1` here makes sure that `exec` will be around 
long enough so it actually can see its `shutdown` call allowing us to expect 
exactly one call (`.Times(1)` is the default for `EXPECT_CALL`).


- Benjamin Bannier


On Feb. 14, 2017, 2:49 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56537/
> ---
> 
> (Updated Feb. 14, 2017, 2:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-7035
> https://issues.apache.org/jira/browse/MESOS-7035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A framework can be upgraded to be MULTI_ROLE capable even with task
> running, as long as it does not change role while upgrading.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 9a7a953adbbbed0b37082bb0166d03b1388c80ae 
> 
> Diff: https://reviews.apache.org/r/56537/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 56623: Implemented 'AuthenticationContext' in libprocess.

[Alexander Rojas]

> On Feb. 16, 2017, 11:45 a.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/src/http.cpp, lines 87-109
> > 
> >
> > Why not going the fancy way and use `jsonify()`?

Just add this function in the same namespace as the `AuthorizationContext`:

```c++
static void json(JSON::ObjectWriter* writer, const AuthorizationContext& 
context)
{
  if (context.principal.isSome()) {
writer->field("principal", context.principal.get());
  }
  if (context.labels.isSome()) {
writer->field("labels", context.labels.get());
  }
}
```

Then you can implement the output stream operator as:

```c++
std::ostream& operator<<(
std::ostream& stream, const AuthenticationContext& context)
{
  stream << jsonify(context);
  return streaml
}
```


- Alexander


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


On Feb. 17, 2017, 6:31 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56623/
> ---
> 
> (Updated Feb. 17, 2017, 6:31 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7003
> https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new struct, `AuthenticationContext`, to
> libprocess to represent an authenticated entity in the system.
> The new type contains a `principal` and a map containing
> arbitrary key-value pairs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> e5489c6cb4adc8a822e7dd4515542618c36136f9 
>   3rdparty/libprocess/include/process/http.hpp 
> 3a406963c1defc037d58971f2400e40855503fbb 
>   3rdparty/libprocess/include/process/process.hpp 
> ceedbe86ccba5c24bf1e9e7f032d7dbcf6acb0f3 
>   3rdparty/libprocess/src/authenticator.cpp 
> cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7 
>   3rdparty/libprocess/src/authenticator_manager.cpp 
> a22acd026a001788dc39b8005a56577e33c6800b 
>   3rdparty/libprocess/src/http.cpp 0e37936eec5b2910e54d40788c26f98665cbe1f1 
>   3rdparty/libprocess/src/process.cpp 
> 3ad485f9136cd9edf0a6db76ff1f475039236391 
> 
> Diff: https://reviews.apache.org/r/56623/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 55576: Fixes FutureTest.After3 flakiness.

[Alexander Rojas]

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

(Updated Feb. 17, 2017, 10:51 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, and Joris Van 
Remoortere.


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


Repository: mesos


Description (updated)
---

Fixed a flakiness in `FutureTest.After3` caused by the way
libprocess processes timers. 

Timers are executed in batch and also discarded in batch, 
so if a given timer's thunk is executed as the first of its
batch, the callback and all objects in the callback will
be kept around until the last timer's thunk in the batch
is executed.

To get around the issue, we ensure that the first batch
is executed, by waiting on the timer's `Future` to be set,
then schedule a second timer and wait until it begins to
be executed.

Note that a solution based in `Clock` would not work since
the timers are executed in, either `libev` or `libevent` 
loops for which libprocess has no control over.


Diffs
-

  3rdparty/libprocess/src/tests/future_tests.cpp 
380755f9a2329d548969cfb2332c79aacbf7fff2 

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


Testing
---

```sh
# Put the machine under stress.
$ stress -c 4 -t 2600 -d 2 -i 2 &
$ ./${MESOS_SRC_ROOT}/build/3rdparty/libprocess/libprocess-tests \
  --gtest_filter="FutureTest.After3" --gtest_repeat=100 
--gtest_break_on_failure
$ ./${MESOS_SRC_ROOT}/build/build/src/mesos-tests
```


Thanks,

Alexander Rojas

Re: Review Request 56781: Fixed compilation on VS 2017.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [56781]

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

- Mesos Reviewbot


On Feb. 17, 2017, 8:12 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56781/
> ---
> 
> (Updated Feb. 17, 2017, 8:12 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 819ee5ceb8f2900087c2a06d5b1df0a1aeb413f6 
> 
> Diff: https://reviews.apache.org/r/56781/diff/
> 
> 
> Testing
> ---
> 
> Tested on VS 2015 and 2017.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 56781: Fixed compilation on VS 2017.

[Alex Clemmer]

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/future.hpp (line 412)


Should this be `#endif // __WINDOWS__`?


- Alex Clemmer


On Feb. 17, 2017, 8:12 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56781/
> ---
> 
> (Updated Feb. 17, 2017, 8:12 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 819ee5ceb8f2900087c2a06d5b1df0a1aeb413f6 
> 
> Diff: https://reviews.apache.org/r/56781/diff/
> 
> 
> Testing
> ---
> 
> Tested on VS 2015 and 2017.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 56781: Fixed compilation on VS 2017.

[Michael Park]

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

Review request for mesos, Andrew Schwartzmeyer, Alex Clemmer, and Joseph Wu.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
819ee5ceb8f2900087c2a06d5b1df0a1aeb413f6 

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


Testing
---

Tested on VS 2015 and 2017.


Thanks,

Michael Park