date:20170216

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

2017-02-16 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [56778]

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, 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 56619: Updated Mesos handlers to use 'AuthenticationContext'.

2017-02-16 Thread Greg Mann



> On Feb. 16, 2017, 3:56 p.m., Alexander Rojas wrote:
> > I personally find this patch way too big, so at the end I found myself just 
> > going through the changeset, perhaps breaking it into the _master_, _agent_ 
> > and _files_ components will help reviewers. What do you think?

Good idea, I'll split this up.


> On Feb. 16, 2017, 3:56 p.m., Alexander Rojas wrote:
> > src/master/http.cpp, line 520
> > 
> >
> > Any readon why `validation::master::call::validate(...)` cannot take 
> > the whole context as parameter?

I considered changing the signatures of the validation helpers as well, but 
ended up leaving them as-is, since they only require the principal currently. 
However, it would be less brittle to pass the context instead. Perhaps I'll 
post separate patches for the validation code, since these reviews are already 
so large.


> 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.

Good call, will do.


> On Feb. 16, 2017, 3:56 p.m., Alexander Rojas wrote:
> > src/master/weights_handler.cpp, line 52
> > 
> >
> > Not sure if using `using` here is the way to go, we don't seem to use 
> > it for `http::Request`. This goes with my idea of just using 
> > `http::authentication::Context` as the name type.

I left the name as-is for the moment, but am open to changing it if we come up 
with something better. Let's consider our options and I can do a sweep and 
update all the reviews if we decide to change it.


- Greg


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


On Feb. 17, 2017, 6:03 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56619/
> ---
> 
> (Updated Feb. 17, 2017, 6:03 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 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'.

2017-02-16 Thread Greg Mann


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

(Updated Feb. 17, 2017, 6:03 a.m.)


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


Changes
---

Addressed comments.


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 56623: Implemented 'AuthenticationContext' in libprocess.

2017-02-16 Thread Greg Mann


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

(Updated Feb. 17, 2017, 5: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 (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 56778: Fixed a bug around executor not able to use reserved resources.

2017-02-16 Thread Anand Mazumdar


---
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.


Changes
---

Updated bug number.


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

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

2017-02-16 Thread Anand Mazumdar


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

Review request for mesos and Benjamin Mahler.


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


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 56623: Implemented 'AuthenticationContext' in libprocess.

2017-02-16 Thread Greg Mann


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

(Updated Feb. 17, 2017, 5:20 a.m.)


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


Changes
---

Addressed comments.


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 56052: Added the 'Secret' protobuf message.

2017-02-16 Thread Vinod Kone


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


Ship it!




Ship It!

- Vinod Kone


On Feb. 16, 2017, 5:52 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56052/
> ---
> 
> (Updated Feb. 16, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Kapil Arya, Jan Schlicht, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-6996
> https://issues.apache.org/jira/browse/MESOS-6996
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `Secret` protobuf message which
> is designed to serve as a generic mechanism for passing
> priviledged information within Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 34a288b6f5c2fca090a7aa7a61798e3255d6663a 
>   include/mesos/v1/mesos.proto 6638111d10f4a36cdf91dfce1019871e9839c306 
>   src/common/validation.hpp 7946f06cf17c05893ff80179c9d14b5f1f4023ef 
>   src/common/validation.cpp 0f1a02286d8431acfee6136e8ada49b0ac746897 
>   src/tests/slave_validation_tests.cpp 
> 3d17799ed04951fb56524db0f5d89347192300b2 
> 
> Diff: https://reviews.apache.org/r/56052/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56392: Tightened test expecations in reservation-related tests.

2017-02-16 Thread Benjamin Mahler


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


Ship it!




Ship It!

- Benjamin Mahler


On Feb. 16, 2017, 10:24 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56392/
> ---
> 
> (Updated Feb. 16, 2017, 10:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change makes expected error strings in reservation
> validation-related tests explicit. This is to make sure that the
> observed errors match the expected ones.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> 83c88ae3a1f54eab29de79c0f34ade0cdb410341 
> 
> Diff: https://reviews.apache.org/r/56392/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 56594: Explicitly marked protobuf files as using v2 syntax.

2017-02-16 Thread Joseph Wu


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


Ship it!




Effectively the same as: https://reviews.apache.org/r/56237/

So I'll close both and attribute the patch to Anthony.

- Joseph Wu


On Feb. 16, 2017, 2:58 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56594/
> ---
> 
> (Updated Feb. 16, 2017, 2:58 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-6138
> https://issues.apache.org/jira/browse/MESOS-6138
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `protoc` will complain about `.proto` files that do not explicitly
> specify the version of PB syntax they're using. On Windows, this
> registers as a build warning.
> 
> To ameliorate this, we explicitly specify the syntax in all `.proto`
> files.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.proto 
> 229ac256b087d55ecc95636254261ecd12829187 
> 
> Diff: https://reviews.apache.org/r/56594/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 56593: Explicitly marked protobuf files as using v2 syntax.

2017-02-16 Thread Joseph Wu


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


Ship it!




Effectively the same as: https://reviews.apache.org/r/56236/
(but slightly more up-to-date)

So I'll close both and attribute the patch to Anthony.

- Joseph Wu


On Feb. 16, 2017, 2:58 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56593/
> ---
> 
> (Updated Feb. 16, 2017, 2:58 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-6138
> https://issues.apache.org/jira/browse/MESOS-6138
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `protoc` will complain about `.proto` files that do not explicitly
> specify the version of PB syntax they're using. On Windows, this
> registers as a build warning.
> 
> To ameliorate this, we explicitly specify the syntax in all `.proto`
> files.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 775c14b70c94c90d31d0123f45188fd29651f9f4 
>   include/mesos/allocator/allocator.proto 
> 9fc8baccb135c5aa58f64847307978139139a46e 
>   include/mesos/appc/spec.proto 2dbb3504596dda229c42353098879350c3a7c960 
>   include/mesos/authentication/authentication.proto 
> a24d53512a8ec27e97cf543cd64129c8a096ac67 
>   include/mesos/authorizer/acls.proto 
> fd25e5449ad659ee0269cfc3f47b9939ac022fb4 
>   include/mesos/authorizer/authorizer.proto 
> 8b860a3e8e0b1c660a8fefc97f10f5acc0501920 
>   include/mesos/docker/spec.proto 734395cbde81bc4ff99f1310fc4b913beeb5b11f 
>   include/mesos/docker/v1.proto 209fb4afe5e4ea1cc776bfe9cb36865c9b05aaf0 
>   include/mesos/docker/v2.proto 9b5bfb5d03ae8115c1b20a8c640640d6dbaa257d 
>   include/mesos/executor/executor.proto 
> e746608176245bcabf265925111b8df9cab8ca55 
>   include/mesos/fetcher/fetcher.proto 
> 7f204e1a0b70ecdcd4247e1c0699585243a97b40 
>   include/mesos/maintenance/maintenance.proto 
> 4afae5c6958a5da91b3e649b496b9757d951ca0c 
>   include/mesos/master/master.proto a2228db8902c297f137b8106a7b3d2babbc35017 
>   include/mesos/mesos.proto 34a288b6f5c2fca090a7aa7a61798e3255d6663a 
>   include/mesos/module/hook.proto f7b91f813f7919098763790211d3b90aed0b8340 
>   include/mesos/module/module.proto 1c6c4b0762f5a4e71cf64fd9bc859e380222b591 
>   include/mesos/oci/spec.proto f3083dd129fe0902760bc85aa93dfe6985358bcb 
>   include/mesos/quota/quota.proto 78e12d56c4f41613ec6b731d86093f48a23b4509 
>   include/mesos/scheduler/scheduler.proto 
> da2c67af41c9b9d4fdf6d79e84d0187cb9873ad3 
>   include/mesos/slave/containerizer.proto 
> c70d437ac3f8f813fcb71c04275cc8eeeb946065 
>   include/mesos/slave/oversubscription.proto 
> e7346089d5699194b761c81ab9610ad33d83ba55 
>   include/mesos/state/state.proto 7a7d68e6abc85c0ead04f5771e878d10348f44b8 
>   include/mesos/uri/uri.proto 55023b681665e712990d45ff4a1dbc4ccf64edbf 
>   include/mesos/v1/agent/agent.proto a98acb7b0c66f7e17dcecf61ed22bfde70a33651 
>   include/mesos/v1/allocator/allocator.proto 
> 093f18f554470b14905db157bcc1e33303423eaa 
>   include/mesos/v1/executor/executor.proto 
> 754e62aee5f822526eb9661aa255444c3f84dd1d 
>   include/mesos/v1/maintenance/maintenance.proto 
> 27f7c2b3460fe3a7e1a845cd962cb9979a9ccc0b 
>   include/mesos/v1/master/master.proto 
> cfdca7400d98233c6320eebd8784e4f51c43ebbd 
>   include/mesos/v1/mesos.proto 6638111d10f4a36cdf91dfce1019871e9839c306 
>   include/mesos/v1/quota/quota.proto 424e6b4bf0064d146cfeeb19de77f0cce67882b9 
>   include/mesos/v1/scheduler/scheduler.proto 
> 65ccfa76c7029b546533d123d3c9be136b8c9124 
>   src/master/registry.proto eab9821b6d159d14bf62b347d6188608ae5c 
>   src/messages/flags.proto 08406b69acaa52eba58e6091b9c8b3bc03981044 
>   src/messages/log.proto 12c2d83d88bb5b282bc44d49655a67e671b12ba2 
>   src/messages/messages.proto 7a2f37b78a8edcd372558f77f15e6b249742e321 
>   src/messages/state.proto 857a88873681c9cd7263d0063e8f17f08c5cd64b 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
> 4c5b03c1a802c177f64ea8bb1e31fa58603991bb 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto 
> 4b3850037ec01969cabf16b94745c1802bf4de62 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> c93c7a92ec152bd9747a70392adfe6a0e863e839 
> 
> Diff: https://reviews.apache.org/r/56593/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 56593: Explicitly marked protobuf files as using v2 syntax.

2017-02-16 Thread Alex Clemmer


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

(Updated Feb. 16, 2017, 2:58 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Changes
---

Added JIRA -- Joseph Wu.


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


Repository: mesos


Description
---

`protoc` will complain about `.proto` files that do not explicitly
specify the version of PB syntax they're using. On Windows, this
registers as a build warning.

To ameliorate this, we explicitly specify the syntax in all `.proto`
files.


Diffs
-

  include/mesos/agent/agent.proto 775c14b70c94c90d31d0123f45188fd29651f9f4 
  include/mesos/allocator/allocator.proto 
9fc8baccb135c5aa58f64847307978139139a46e 
  include/mesos/appc/spec.proto 2dbb3504596dda229c42353098879350c3a7c960 
  include/mesos/authentication/authentication.proto 
a24d53512a8ec27e97cf543cd64129c8a096ac67 
  include/mesos/authorizer/acls.proto fd25e5449ad659ee0269cfc3f47b9939ac022fb4 
  include/mesos/authorizer/authorizer.proto 
8b860a3e8e0b1c660a8fefc97f10f5acc0501920 
  include/mesos/docker/spec.proto 734395cbde81bc4ff99f1310fc4b913beeb5b11f 
  include/mesos/docker/v1.proto 209fb4afe5e4ea1cc776bfe9cb36865c9b05aaf0 
  include/mesos/docker/v2.proto 9b5bfb5d03ae8115c1b20a8c640640d6dbaa257d 
  include/mesos/executor/executor.proto 
e746608176245bcabf265925111b8df9cab8ca55 
  include/mesos/fetcher/fetcher.proto 7f204e1a0b70ecdcd4247e1c0699585243a97b40 
  include/mesos/maintenance/maintenance.proto 
4afae5c6958a5da91b3e649b496b9757d951ca0c 
  include/mesos/master/master.proto a2228db8902c297f137b8106a7b3d2babbc35017 
  include/mesos/mesos.proto 34a288b6f5c2fca090a7aa7a61798e3255d6663a 
  include/mesos/module/hook.proto f7b91f813f7919098763790211d3b90aed0b8340 
  include/mesos/module/module.proto 1c6c4b0762f5a4e71cf64fd9bc859e380222b591 
  include/mesos/oci/spec.proto f3083dd129fe0902760bc85aa93dfe6985358bcb 
  include/mesos/quota/quota.proto 78e12d56c4f41613ec6b731d86093f48a23b4509 
  include/mesos/scheduler/scheduler.proto 
da2c67af41c9b9d4fdf6d79e84d0187cb9873ad3 
  include/mesos/slave/containerizer.proto 
c70d437ac3f8f813fcb71c04275cc8eeeb946065 
  include/mesos/slave/oversubscription.proto 
e7346089d5699194b761c81ab9610ad33d83ba55 
  include/mesos/state/state.proto 7a7d68e6abc85c0ead04f5771e878d10348f44b8 
  include/mesos/uri/uri.proto 55023b681665e712990d45ff4a1dbc4ccf64edbf 
  include/mesos/v1/agent/agent.proto a98acb7b0c66f7e17dcecf61ed22bfde70a33651 
  include/mesos/v1/allocator/allocator.proto 
093f18f554470b14905db157bcc1e33303423eaa 
  include/mesos/v1/executor/executor.proto 
754e62aee5f822526eb9661aa255444c3f84dd1d 
  include/mesos/v1/maintenance/maintenance.proto 
27f7c2b3460fe3a7e1a845cd962cb9979a9ccc0b 
  include/mesos/v1/master/master.proto cfdca7400d98233c6320eebd8784e4f51c43ebbd 
  include/mesos/v1/mesos.proto 6638111d10f4a36cdf91dfce1019871e9839c306 
  include/mesos/v1/quota/quota.proto 424e6b4bf0064d146cfeeb19de77f0cce67882b9 
  include/mesos/v1/scheduler/scheduler.proto 
65ccfa76c7029b546533d123d3c9be136b8c9124 
  src/master/registry.proto eab9821b6d159d14bf62b347d6188608ae5c 
  src/messages/flags.proto 08406b69acaa52eba58e6091b9c8b3bc03981044 
  src/messages/log.proto 12c2d83d88bb5b282bc44d49655a67e671b12ba2 
  src/messages/messages.proto 7a2f37b78a8edcd372558f77f15e6b249742e321 
  src/messages/state.proto 857a88873681c9cd7263d0063e8f17f08c5cd64b 
  src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
4c5b03c1a802c177f64ea8bb1e31fa58603991bb 
  src/slave/containerizer/mesos/isolators/network/cni/spec.proto 
4b3850037ec01969cabf16b94745c1802bf4de62 
  src/slave/containerizer/mesos/provisioner/docker/message.proto 
c93c7a92ec152bd9747a70392adfe6a0e863e839 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 56594: Explicitly marked protobuf files as using v2 syntax.

2017-02-16 Thread Alex Clemmer


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

(Updated Feb. 16, 2017, 2:58 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Changes
---

Added JIRA -- Joseph Wu.


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


Repository: mesos


Description
---

`protoc` will complain about `.proto` files that do not explicitly
specify the version of PB syntax they're using. On Windows, this
registers as a build warning.

To ameliorate this, we explicitly specify the syntax in all `.proto`
files.


Diffs
-

  3rdparty/stout/tests/protobuf_tests.proto 
229ac256b087d55ecc95636254261ecd12829187 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 56592: Libprocess: Removed MSVC compiler warnings.

2017-02-16 Thread Joseph Wu

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

Fix it, then Ship it!

3rdparty/libprocess/include/process/network.hpp (line 50)

Note: This is a note, rather than an issue.

This is an implicit cast from `size_t` (usually uint64_t) to `socklen_t` 
(usually uint32_t).  As a down-cast, we may sometimes want to check against 
`std::numeric_limits`, but in this case we don't need to.

`Address::size` is the amount of bytes a socket address struct requires.  
If we actually had a struct bigger than 4GB, there would be much more 
fundamental issues to solve :D

So this line LGTM.

3rdparty/libprocess/src/encoder.hpp (line 247)

For an unsigned -> signed (size_t -> off_t) cast, we need to check if `size 
< std::numeric_limits::max()`.

I plan to add this to the constructor:
```
// NOTE: For files, we expect the size to be derived from `stat`-ing
// the file.  The `struct stat` returns the size in `off_t` form,
// meaning that it is a programmer error to construct the `FileEncoder`
// with a size greater the max value of `off_t`.
CHECK_LE(_size, std::numeric_limits::max());
```

- Joseph Wu

On Feb. 14, 2017, 12:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56592/
> ---
> 
> (Updated Feb. 14, 2017, 12:58 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Removed MSVC compiler warnings.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
>   3rdparty/libprocess/include/process/network.hpp 
> 0590e7a67275b9e37af2a49c050daab5eeaee7a5 
>   3rdparty/libprocess/src/encoder.hpp 
> b019bf90c0aabbce50d90f5ed6f3fd25d725e542 
> 
> Diff: https://reviews.apache.org/r/56592/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 56591: Stout: Removed MSVC compiler warnings.

2017-02-16 Thread Joseph Wu


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


Ship it!




- Joseph Wu


On Feb. 15, 2017, 1:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56591/
> ---
> 
> (Updated Feb. 15, 2017, 1:36 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Removed MSVC compiler warnings.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/abort.hpp 
> b7dba032f579ead83f4e04c942239e721632eeb4 
>   3rdparty/stout/include/stout/duration.hpp 
> 0a30a09678c20937caa6f094c3c63a326e357932 
>   3rdparty/stout/include/stout/gzip.hpp 
> 7040a3275370e7447e843c2471f35e2ba26166e4 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 1fc4d55393aa617d1b67178c945ac0af91c30c99 
>   3rdparty/stout/include/stout/os/windows/environment.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> fddaaa54deaea5e6ef3947142870c7fef76e76aa 
>   3rdparty/stout/include/stout/protobuf.hpp 
> c0f03bc547cddba29309c429b34a0c1e6015c8ea 
>   3rdparty/stout/include/stout/windows/os.hpp 
> b5172fca96c4151f4b1ebb6d343022558f45fc34 
> 
> Diff: https://reviews.apache.org/r/56591/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 56702: Windows: Handle environment variable inheritance uniformly in Mesos.

2017-02-16 Thread Joseph Wu


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


Ship it!




LGTM.

I will discard https://reviews.apache.org/r/53712/ and 
https://reviews.apache.org/r/55388/ in favor of this patch.

- Joseph Wu


On Feb. 15, 2017, 2:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56702/
> ---
> 
> (Updated Feb. 15, 2017, 2:58 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review is a planned regression on MESOS-6816. In other parts of the
> codebase, we copy all environment variables from the system before
> launching a process, overwriting user specified environment variables.
> 
> This is not correct behavior, but a half-measure that is necessary,
> because Windows does not inherit environment variables by default. In
> this commit, we make this behavior uniform across all places where we
> are creating a process, because it is better for behavior to be
> consistent before we get around to fixing it. We do have concrete plans
> to come back and resolve this issue.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 886528f0f706ef2a3c07246406f3ee1ebc45565d 
>   src/slave/containerizer/mesos/launch.cpp 
> 4dd81b47ca4654f5e783a4f2227834e938bc8bb3 
> 
> Diff: https://reviews.apache.org/r/56702/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 56505: Added Windows support for Docker executor.

2017-02-16 Thread Joseph Wu


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


Ship it!




LGTM.  

I will discard https://reviews.apache.org/r/52364/ in favor of this patch.

- Joseph Wu


On Feb. 15, 2017, 2:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56505/
> ---
> 
> (Updated Feb. 15, 2017, 2:58 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-3098
> https://issues.apache.org/jira/browse/MESOS-3098
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit will introduce the changes to the Mesos Docker interfaces
> required to use the Docker executor to run Windows Containers in the
> Mesos agent. In particular, since Windows Containers use named pipes to
> connect do the Docker host, rather than a socket, we introduce the
> plumbing to default to a named pipe connection when invoking the
> `docker` command.
> 
> This work constitutes the beginning of the end of the work that will
> eventually result in Mesos support of Windows Containers.
> 
> This review is partially based on r/52364, which was the work of Daniel
> Pravat.
> 
> Lastly, this review has a planned regression in MESOS-6816. In other
> parts of the codebase, we copy all environment variables from the system
> before launching a process, overwriting user specified environment
> variables. This is not correct behavior, but a half-measure that is
> necessary, because Windows does not inherit environment variables by
> default. In this commit, we make this behavior uniform across all places
> where we are creating a process, because it is better for behavior to be
> consistent before we get around to fixing it. We do have concrete plans
> to come back and resolve this issue.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 68042fd6f3b19c12cddd0d2e74ae064ceceb6a8c 
>   src/docker/executor.hpp 8385631fd170f97b28c4ca3596255ab0546774d6 
>   src/slave/constants.hpp 1159ac3b1f8797a847a6f5679045434064a519e9 
>   src/slave/containerizer/docker.cpp 886528f0f706ef2a3c07246406f3ee1ebc45565d 
>   src/slave/flags.cpp 71935dea0f898d4594de9a5d8a3d1c7fc1a21606 
>   src/tests/flags.hpp cab24162e26be9fe79c0c65f24e35e3c3d735906 
> 
> Diff: https://reviews.apache.org/r/56505/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 56740: Update Allegro url on Powered By page.

2017-02-16 Thread Benjamin Mahler


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


Ship it!




Ship It!

- Benjamin Mahler


On Feb. 16, 2017, 8:27 a.m., Kamil Wargula wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56740/
> ---
> 
> (Updated Feb. 16, 2017, 8:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update Allegro url on Powered By page.
> 
> 
> Diffs
> -
> 
>   docs/powered-by-mesos.md 669c183ddd30556849b0a95bdb854e2d5e84bc1a 
> 
> Diff: https://reviews.apache.org/r/56740/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kamil Wargula
> 
>

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

2017-02-16 Thread Mesos Reviewbot


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



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. 16, 2017, 3:37 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56413/
> ---
> 
> (Updated Feb. 16, 2017, 3:37 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 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

2017-02-16 Thread Greg Mann



> On Feb. 16, 2017, 12:10 a.m., Vinod Kone wrote:
> > src/tests/slave_validation_tests.cpp, line 145
> > 
> >
> > Why is this test here instead of master validationt ests? Doesn't this 
> > validation happen in master?
> 
> Vinod Kone wrote:
> didn't intend this to be an issue. just curious.

Both the master and agent use that validation code - I picked the agent 
validation tests as the location somewhat arbitrarily.


- Greg


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


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 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

2017-02-16 Thread Greg Mann


---
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.


Changes
---

Updated comments, and changed `Secret.value.value` to `Secret.value.data`.


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

  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 56052: Added the 'Secret' protobuf message.

2017-02-16 Thread Greg Mann


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

(Updated Feb. 16, 2017, 5:52 p.m.)


Review request for mesos, Adam B, Jie Yu, Kapil Arya, Jan Schlicht, and Vinod 
Kone.


Changes
---

Changed the `Value.value` field to `Value.data`.


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


Repository: mesos


Description
---

This patch adds a new `Secret` protobuf message which
is designed to serve as a generic mechanism for passing
priviledged information within Mesos.


Diffs (updated)
-

  include/mesos/mesos.proto 34a288b6f5c2fca090a7aa7a61798e3255d6663a 
  include/mesos/v1/mesos.proto 6638111d10f4a36cdf91dfce1019871e9839c306 
  src/common/validation.hpp 7946f06cf17c05893ff80179c9d14b5f1f4023ef 
  src/common/validation.cpp 0f1a02286d8431acfee6136e8ada49b0ac746897 
  src/tests/slave_validation_tests.cpp 3d17799ed04951fb56524db0f5d89347192300b2 

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 56711: Fixed fetcher to not pick up environment variables it should not see.

2017-02-16 Thread Till Toenshoff

> On Feb. 16, 2017, 9:39 a.m., Adam B wrote:
> > src/slave/containerizer/fetcher.cpp, lines 827-828
> > 
> >
> > How confident are you that the fetcher doesn't actually need any of 
> > these env vars? How can we test/prove it?

So far I simply tried to identify such possible conflicts by looking at the 
code.

The `mesos-fetcher` itself is offering support for 
`mesos::internal::logging::Flags` which add the following options:
```
  add(::quiet,
  add(::logging_level,
  add(::log_dir,
  add(::logbufsecs,
  add(::initialize_driver_logging,
  add(::external_log_file,
```
All of the above are certainly supported by use if `MESOS_` prefixed 
environment variable names. Other than that, the `mesos-fetcher` has support 
for `MESOS_FETCHER_INFO` which we explicitly pass on even after that patch we 
are discussing here. 
So the things to look at should be limited towards the candidates covered by 
those `Flags`. To be entirely clean we might want to consider passing those 
candidates through explicitly.

- Till

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

On Feb. 16, 2017, 1:31 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56711/
> ---
> 
> (Updated Feb. 16, 2017, 1:31 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, and Greg Mann.
> 
> 
> Bugs: MESOS-6751 and MESOS-7133
> https://issues.apache.org/jira/browse/MESOS-6751
> https://issues.apache.org/jira/browse/MESOS-7133
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 9ec38dc95dddfcd990369d0146986e20b15da1a0 
> 
> Diff: https://reviews.apache.org/r/56711/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

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

2017-02-16 Thread Alexander Rojas

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

I personally find this patch way too big, so at the end I found myself just 
going through the changeset, perhaps breaking it into the _master_, _agent_ and 
_files_ components will help reviewers. What do you think?

src/files/files.hpp (line 89)

Could you indent this more... or less... not sure how, but it is hard to 
tell wether `authorized` is part of the type declaration or the name of the 
parameter.

src/files/files.cpp (line 188)

Same here, it is not clear that `authorizations` is not part of type. 
Although this is what clang format returns I think either:

```c++
hashmap<
  string,
  lambda::function>
  authorizations;
```

or

```c++
hashmap<
  string,
  lambda::function
> authorizations;
```

are more readable.

src/master/http.cpp (line 520)

Any readon why `validation::master::call::validate(...)` cannot take the 
whole context as parameter?

src/master/http.cpp (lines 655 - 657)

Note to self: This may be a bug that violates the semantics of `Subject`, 
write a test.

src/master/http.cpp (lines 795 - 797)

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.

src/master/http.cpp (lines 1477 - 1479)

likewise, this looks like a prime candidate to be abstracted away.

src/master/weights_handler.cpp (line 52)

Not sure if using `using` here is the way to go, we don't seem to use it 
for `http::Request`. This goes with my idea of just using 
`http::authentication::Context` as the name type.

- Alexander Rojas

On Feb. 14, 2017, 1:14 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56619/
> ---
> 
> (Updated Feb. 14, 2017, 1:14 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 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 c5324abc0db82275fd65d3f7d361ad8ee9e017d1 
>   src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e 
>   src/master/master.cpp dff320acd1a13995d51cc0c303edea0cdfaa336d 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 
>   src/slave/http.cpp ccd489d700a605dc41037e00dbd6a7cb3b6edcc6 
>   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 56623: Implemented 'AuthenticationContext' in libprocess.

2017-02-16 Thread Jan Schlicht


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




3rdparty/libprocess/include/process/http.hpp (lines 75 - 78)


Make this a const function.

You could also make this a free function, to have better ordering.


- Jan Schlicht


On Feb. 14, 2017, 12:23 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56623/
> ---
> 
> (Updated Feb. 14, 2017, 12:23 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
> 
>

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

2017-02-16 Thread Gastón Kleiman


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

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 55901: Added support for command health checks to the default executor.

2017-02-16 Thread Gastón Kleiman


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

(Updated Feb. 16, 2017, 2:59 p.m.)


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


Changes
---

Addresed Vinod's 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 56288: Improved the wording of what's logged on command health check timeouts.

2017-02-16 Thread Gastón Kleiman


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

(Updated Feb. 16, 2017, 2:58 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
---

Improved the wording of what's logged on command health check timeouts.


Diffs (updated)
-

  src/checks/health_checker.cpp 6c97369fd9a567ba16dd92085bf142d43f71eaf1 

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


Testing
---

None, not a functional change.


Thanks,

Gastón Kleiman

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

2017-02-16 Thread Gastón Kleiman



> On Feb. 15, 2017, 9:40 p.m., Vinod Kone wrote:
> > src/checks/health_checker.cpp, line 664
> > 
> >
> > won't we be losing the info about why wait failed?

Yes, but the health check timed out anyway, we call `WaitNestedContainer`, only 
to ensure that the next `LaunchNestedContainerSession` call won't fail, so I 
don't think that a user would care about the details of the 
`WaitNestedContainer` call. We could log the response `VLOG(1)` or `VLOG(2)`, 
to make it easier to debug potential Mesos bugs.

In our offline discussions we also realized that reusing the `ContainerID` is 
not always safe, so we'll have to replace this wait call with a wait + 
cleanupContainer.


> On Feb. 15, 2017, 9:40 p.m., Vinod Kone wrote:
> > src/checks/health_checker.cpp, line 616
> > 
> >
> > is this deferred only because you want to access `taskId`? why not just 
> > pass taskId to the lambda directly and not-defer?

Because the compiler complains about not being able to capture a non-variable:

```
../../src/checks/health_checker.cpp:584:26: error: capture of non-variable 
‘mesos::internal::checks::HealthCheckerProcess::taskId’
```


> On Feb. 15, 2017, 9:40 p.m., Vinod Kone wrote:
> > src/checks/health_checker.cpp, line 654
> > 
> >
> > do you want to add a TODO here to not re-use the ContainerID?

I don't know if we should commit this with that TODO, or if I should wait until 
MESOS-7120 is resolved, and then update this patch to not re-use the 
ContainerID.


- Gastón


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


On Feb. 10, 2017, 6:40 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55901/
> ---
> 
> (Updated Feb. 10, 2017, 6:40 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
> ---
> 
> Added support for command health checks to the default executor.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp f1f2834b3429fb00cc49c179fa9a3de328f597b5 
>   src/checks/health_checker.cpp a5225ff1f4b071ed4182d41fa8ecc705fa4dbe00 
>   src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a 
>   src/tests/health_check_tests.cpp 7b6a803a28b2e4f6c27e9a0c4f668350ec2d5a81 
> 
> 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 56754: Implemented a JWT secret generator.

2017-02-16 Thread Jan Schlicht


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

(Updated Feb. 16, 2017, 3:55 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Extract the SecretGenerator interface/module.


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


Repository: mesos


Description
---

This can be used to create a 'Secret' from an 'AuthenticationContext'.
The resulting secret will provide a JWT.


Diffs (updated)
-

  src/Makefile.am c21a073bdbefbb4547e45ed13b7a8a563854fd82 
  src/authentication/executor/jwt_secret_generator.hpp PRE-CREATION 
  src/authentication/executor/jwt_secret_generator.cpp PRE-CREATION 
  src/tests/secret_generator_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 56757: Added the SecretGenerator module interface.

2017-02-16 Thread Jan Schlicht


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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

This interface will be used by agents to create credentials for the
default executor.


Diffs
-

  include/mesos/authentication/secret_generator.hpp PRE-CREATION 
  include/mesos/module/secret_generator.hpp PRE-CREATION 
  src/Makefile.am c21a073bdbefbb4547e45ed13b7a8a563854fd82 

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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 56754: Implemented a JWT secret generator.

2017-02-16 Thread Jan Schlicht


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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

This can be used to create a 'Secret' from an 'AuthenticationContext'.
The resulting secret will provide a JWT.


Diffs
-

  src/Makefile.am c21a073bdbefbb4547e45ed13b7a8a563854fd82 
  src/authentication/executor/secret_generator.hpp PRE-CREATION 
  src/authentication/executor/secret_generator.cpp PRE-CREATION 
  src/tests/secret_generator_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 56753: Implemented the JWT authenticator.

2017-02-16 Thread Jan Schlicht


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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

This HTTP authenticator extracts a JWT from the requests' authorization
header using the 'Bearer' schema and validates it against a secret using
HMAC SHA256. The 'sub' claim of the JWT is the extracted principal, all
other claims will be additional labels of the 'AuthenticationContext'.


Diffs
-

  3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
  3rdparty/libprocess/include/process/authenticator.hpp 
e5489c6cb4adc8a822e7dd4515542618c36136f9 
  3rdparty/libprocess/src/authenticator.cpp 
cfedb6f7674e0f6690e77a633cdd1bd494c7d2c7 
  3rdparty/libprocess/src/jwt_authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
fb4da9aecff0370d97a15269c5d8fffb30e0478f 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 56530: Prevent offers for old agents being sent to MULTI_ROLE frameworks.

2017-02-16 Thread Benjamin Bannier

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

src/master/master.cpp (lines 6943 - 6944)

It seems like if we recover resources here, there is nothing preventing the 
allocator from offering the exact same resources again immediately. In that 
case allocation progress could be blocked indefinitely as the allocator 
prepares an offer which the master immediately filters.

Another possibility might be to directly filter frameworks in the allocator 
loop like we already do for GPUs, 
https://github.com/apache/mesos/blob/7012ee28a5a91f4e6427bc134a99feb9536cb4e0/src/master/allocator/mesos/hierarchical.cpp#L1525-L1531.
 For that we'd need to store agent capabilities in 
`HierarchicalAllocatorProcess::Slave` to perform the filtering.

The downside of that approach is of course that all of this handling would 
be specifically patched into the hierarchical allocator making writing proper 
custom allocators even harder, but I cannot see another way to perform 
filtering in a deadlock-safe way. Do you have an idea?

- Benjamin Bannier

On Feb. 15, 2017, 7:44 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56530/
> ---
> 
> (Updated Feb. 15, 2017, 7:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Guangya Liu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6940
> https://issues.apache.org/jira/browse/MESOS-6940
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In a mixed cluster, when an old agent (not MULTI_ROLE capable)
> registers with new master (MULTI_ROLE capable), it cannot correctly
> handle tasks from a MULTI_ROLE framework. Therefore, we should
> disallow offers from these agents being sent to MULTI_ROLE frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp dff320acd1a13995d51cc0c303edea0cdfaa336d 
>   src/tests/master_tests.cpp c8ef9d6bd9f4f8d631ac87d5e5f8dbe679a70fd1 
> 
> Diff: https://reviews.apache.org/r/56530/diff/
> 
> 
> Testing
> ---
> 
> ./bin/mesos-tests.sh 
> --gtest_filter="MasterTest.MultiRoleFrameworkDoesNotReceiveOfferFromOldAgent" 
> --gtest_break_on_failure --gtest_repeat=-1
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 56213: Added check tests for command executor.

2017-02-16 Thread Gastón Kleiman



> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote:
> > src/tests/check_tests.cpp, line 177
> > 
> >
> > s/agent/slave/ 
> > 
> > here and everywhere else.
> > 
> > we are not doing this change yet.
> 
> Gastón Kleiman wrote:
> We do this in `health_check_tests.cpp`:
> 
> ```
> $ grep "agent = " health_check_tests.cpp | wc -l
>   15
> $ grep "slave = " health_check_tests.cpp | wc -l
>0
> $
> ```
> 
> Vinod Kone wrote:
> I think we should fix `health_check_tests.cpp` as well then.

I used `health_check_tests.cpp` just as an example, but we're using this in a 
lot of places:

```
$ git grep "agent[^ ]* =" | cut -d: -f1 | sort | uniq
api_tests.cpp
authentication_tests.cpp
containerizer/cgroups_isolator_tests.cpp
containerizer/docker_containerizer_tests.cpp
containerizer/io_switchboard_tests.cpp
containerizer/memory_pressure_tests.cpp
fault_tolerance_tests.cpp
health_check_tests.cpp
hierarchical_allocator_tests.cpp
hook_tests.cpp
master_allocator_tests.cpp
master_tests.cpp
master_validation_tests.cpp
mesos.cpp
mesos.hpp
metrics_tests.cpp
oversubscription_tests.cpp
partition_tests.cpp
reconciliation_tests.cpp
reservation_endpoints_tests.cpp
reservation_tests.cpp
slave_authorization_tests.cpp
slave_recovery_tests.cpp
slave_tests.cpp
sorter_tests.cpp
$ git grep "agent[^ ]* =" | wc -l
 207
$
```


- Gastón


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


On Feb. 8, 2017, 4:56 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56213/
> ---
> 
> (Updated Feb. 8, 2017, 4:56 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp b5a5d8e80c80b480992a3c8160ee7d0e4443111c 
>   src/tests/mesos.hpp b450a04dfbf3bbeeb6b605fb78097dca390cbdbe 
> 
> Diff: https://reviews.apache.org/r/56213/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/56218/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 56601: Used single quotes instead of backticks in error message.

2017-02-16 Thread Gastón Kleiman



> On Feb. 14, 2017, 1:48 p.m., Gastón Kleiman wrote:
> > I think that we should also update `checks/checker.cpp` and 
> > `checks/health_checker.cpp`:
> > 
> > ```
> > $ ag --cpp --nomultiline '^\s*[^/"]+".*`'
> > checks/checker.cpp
> > 60:"Check's `CommandInfo` is invalid: " + error->message);
> > 
> > checks/health_checker.cpp
> > :"Health check's `CommandInfo` is invalid: " + 
> > error->message);
> > ```
> 
> Benjamin Bannier wrote:
> I didn't intend to make a full sweep in this patch, but rather to address 
> a single such instance in the reservation validation code I touched.
> 
> I believe a full sweep should fix a much larger number of places (in 
> addition to error messages probably also in e.g., help strings),
> 
> $ git grep -n '".*`' | grep -v '^.*\/\/' | grep -E '(cpp:|hpp:)' | wc 
> -l
>  328
> 
> Benjamin Mahler wrote:
> Wow, I did not realize the extent to which backticks have made it into 
> our logging, error, and help messages. This would warrant a thread on the dev 
> list to make sure everyone is on the same page, because from what I can tell 
> it has become inconsistent. I'm curious to know if there were any reasons to 
> also include backticks in logging, error, and help messages (other than being 
> consistent with comment formatting style).

I agree that it has become inconsistent and that we'd probably require a dev 
thread and a full sweep to make all the logging, error, and help messages 
consistent.

However I think that making all the validation errors consistent wouldn't be so 
hard. As far as I can see, it'd require only to fix the 4 issues I opened here 
and to update `checker.cpp` and `health_checker.cpp`, which I would volunteer 
to do.


- Gastón


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


On Feb. 16, 2017, 10:24 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56601/
> ---
> 
> (Updated Feb. 16, 2017, 10:24 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f 
>   src/tests/master_validation_tests.cpp 
> 83c88ae3a1f54eab29de79c0f34ade0cdb410341 
> 
> Diff: https://reviews.apache.org/r/56601/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 56052: Added the 'Secret' protobuf message.

2017-02-16 Thread Jan Schlicht


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




src/common/validation.cpp (line 113)


Please include ``.


- Jan Schlicht


On Feb. 15, 2017, 11:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56052/
> ---
> 
> (Updated Feb. 15, 2017, 11:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Kapil Arya, Jan Schlicht, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-6996
> https://issues.apache.org/jira/browse/MESOS-6996
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new `Secret` protobuf message which
> is designed to serve as a generic mechanism for passing
> priviledged information within Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 34a288b6f5c2fca090a7aa7a61798e3255d6663a 
>   include/mesos/v1/mesos.proto 6638111d10f4a36cdf91dfce1019871e9839c306 
>   src/common/validation.hpp 7946f06cf17c05893ff80179c9d14b5f1f4023ef 
>   src/common/validation.cpp 0f1a02286d8431acfee6136e8ada49b0ac746897 
>   src/tests/slave_validation_tests.cpp 
> 3d17799ed04951fb56524db0f5d89347192300b2 
> 
> Diff: https://reviews.apache.org/r/56052/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56601: Used single quotes instead of backticks in error message.

2017-02-16 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [55461, 56391, 55462, 56392, 56601]

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. 16, 2017, 10:24 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56601/
> ---
> 
> (Updated Feb. 16, 2017, 10:24 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f 
>   src/tests/master_validation_tests.cpp 
> 83c88ae3a1f54eab29de79c0f34ade0cdb410341 
> 
> Diff: https://reviews.apache.org/r/56601/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

2017-02-16 Thread Benjamin Bannier


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


Fix it, then Ship it!





src/tests/master_validation_tests.cpp (line 3402)


Would it make sense to add a comment clarifying why this is needed here, 
but not below? Something like

// Since we launched tasks, stopping the driver will trigger
// a shutdown request to the executor.


- Benjamin Bannier


On Feb. 15, 2017, 1:05 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56692/
> ---
> 
> (Updated Feb. 15, 2017, 1:05 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 
> fd1f4a6cf0661351e265e50da1bd6ea04ed13d26 
> 
> 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 56618: Updated common Mesos code to use 'AuthenticationContext'.

2017-02-16 Thread Jan Schlicht



> On Feb. 14, 2017, 4: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.

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.


- Jan


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


On Feb. 14, 2017, 12:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56618/
> ---
> 
> (Updated Feb. 14, 2017, 12:46 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 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 56742: Updated CHANGELOG for Mesos 1.2.0 release.

2017-02-16 Thread Mesos Reviewbot


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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On Feb. 16, 2017, 9:49 a.m., Adam B wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56742/
> ---
> 
> (Updated Feb. 16, 2017, 9:49 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CHANGELOG for Mesos 1.2.0 release.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 
> 
> Diff: https://reviews.apache.org/r/56742/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam B
> 
>

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

2017-02-16 Thread Alexander Rojas



> On Feb. 14, 2017, 4: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?

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.


- Alexander


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


On Feb. 14, 2017, 12:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56618/
> ---
> 
> (Updated Feb. 14, 2017, 12:46 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 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 56618: Updated common Mesos code to use 'AuthenticationContext'.

2017-02-16 Thread Alexander Rojas


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




src/common/http.hpp (line 133)


Given that we always use the complete name 
`http::authentication::AuthenticationContext` I was wondering if it makes any 
sense of calling this type `AuthenticationContext`  or if it is redundant given 
its namespace.

I'm not asking you to change it, but to think about it.



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.


- Alexander Rojas


On Feb. 14, 2017, 12:46 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56618/
> ---
> 
> (Updated Feb. 14, 2017, 12:46 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 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 56617: Updated libprocess handlers to use 'AuthenticationContext'.

2017-02-16 Thread Alexander Rojas


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


Ship it!




Ship It!

- Alexander Rojas


On Feb. 14, 2017, 4:41 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56617/
> ---
> 
> (Updated Feb. 14, 2017, 4:41 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 endpoint handlers in libprocess
> to make use of the `AuthenticationContext` instead of an
> `Option& principal`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> b5c2515605d5a9fab304f1425d29082f6167cea4 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> d9c64b7a4660182a5a23beed50a8e42b89d8b2aa 
>   3rdparty/libprocess/include/process/profiler.hpp 
> d19d647aa0d1172f09940fae1d1dfa83fd07b924 
>   3rdparty/libprocess/src/logging.cpp 
> cf7b43d2339df57ce899677419968d7e00ad8c38 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 29c8c1da4e1424f029ccdf9fab890e8c1ec0ccb8 
>   3rdparty/libprocess/src/profiler.cpp 
> 2b3b6d5220aed9568f9fef51045cf322aa2934d7 
> 
> Diff: https://reviews.apache.org/r/56617/diff/
> 
> 
> Testing
> ---
> 
> Testing information can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-16 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [56665, 5, 56667]

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. 16, 2017, 9:35 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56667/
> ---
> 
> (Updated Feb. 16, 2017, 9:35 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-7001
> https://issues.apache.org/jira/browse/MESOS-7001
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> JSON Web Tokens can be used to create claim-based access tokens and is
> typically used for HTTP authentication.
> This implementation is intended for internal use, e.g. Mesos is supposed
> to only parse tokens that it also created. It doesn't fully comply with
> RFC 7519. Currently the only supported cryptographic algorithm is HMAC
> with SHA-256.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
>   3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
>   3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/56667/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

2017-02-16 Thread Alexander Rojas


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




3rdparty/libprocess/include/process/http.hpp (line 22)


This include is not needed since `iosfwd` already includes forward 
declarations for `std::ostream`.

So I would say, unless you actually use it here, stick to the forward 
declaration.



3rdparty/libprocess/include/process/http.hpp (line 68)


In order to keep things localized, I would prefer this declaration to be in 
`3rdparty/libprocess/include/process/authenticator.hpp`



3rdparty/libprocess/include/process/http.hpp (line 123)


for lambda declarations you don't need to name the parameters, so I tend to 
omit them if their meaning is not clear from the context, but in this case I 
think is clear that it is an `AuthenticationContext`.

Plus you don't name the parameters anywhere else where you use this 
callback type.



3rdparty/libprocess/src/authenticator_manager.cpp (line 100)


Not yours (actually mine) but this message is strange if one doesn't have 
knowledge of the Mesos internals. How about:

```
HTTP Authentication expects either a 'context', an 'unauthorized' response 
or a 'forbidden' response to be set.
```



3rdparty/libprocess/src/http.cpp (line 85)


To keep things localized, this is better suited to 
`3rdparty/libprocess/src/authenticator.cpp`



3rdparty/libprocess/src/http.cpp (lines 87 - 109)


Why not going the fancy way and use `jsonify()`?


- Alexander Rojas


On Feb. 14, 2017, 12:23 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56623/
> ---
> 
> (Updated Feb. 14, 2017, 12:23 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 56601: Used single quotes instead of backticks in error message.

2017-02-16 Thread Benjamin Bannier


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

(Updated Feb. 16, 2017, 11:24 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/validation.cpp b8c6b49ca5ca1f15e58a2967adc335436a35071f 
  src/tests/master_validation_tests.cpp 
83c88ae3a1f54eab29de79c0f34ade0cdb410341 

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


Testing
---

make check (OS X)


Thanks,

Benjamin Bannier

Re: Review Request 56392: Tightened test expecations in reservation-related tests.

2017-02-16 Thread Benjamin Bannier


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

(Updated Feb. 16, 2017, 11:24 a.m.)


Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.


Changes
---

Rebased.


Repository: mesos


Description
---

This change makes expected error strings in reservation
validation-related tests explicit. This is to make sure that the
observed errors match the expected ones.


Diffs (updated)
-

  src/tests/master_validation_tests.cpp 
83c88ae3a1f54eab29de79c0f34ade0cdb410341 

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


Testing
---

make check (OS X)


Thanks,

Benjamin Bannier

Review Request 56742: Updated CHANGELOG for Mesos 1.2.0 release.

2017-02-16 Thread Adam B


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

Review request for mesos.


Repository: mesos


Description
---

Updated CHANGELOG for Mesos 1.2.0 release.


Diffs
-

  CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 

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


Testing
---


Thanks,

Adam B

Re: Review Request 56711: Fixed fetcher to not pick up environment variables it should not see.

2017-02-16 Thread Adam B


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




src/slave/containerizer/fetcher.cpp (lines 827 - 828)


How confident are you that the fetcher doesn't actually need any of these 
env vars? How can we test/prove it?


- Adam B


On Feb. 15, 2017, 5:31 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56711/
> ---
> 
> (Updated Feb. 15, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, and Greg Mann.
> 
> 
> Bugs: MESOS-6751 and MESOS-7133
> https://issues.apache.org/jira/browse/MESOS-6751
> https://issues.apache.org/jira/browse/MESOS-7133
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 9ec38dc95dddfcd990369d0146986e20b15da1a0 
> 
> Diff: https://reviews.apache.org/r/56711/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-16 Thread Jan Schlicht


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

(Updated Feb. 16, 2017, 10:35 a.m.)


Review request for mesos and Greg Mann.


Summary (updated)
-

Added support for JSON Web Tokens.


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


Repository: mesos


Description (updated)
---

JSON Web Tokens can be used to create claim-based access tokens and is
typically used for HTTP authentication.
This implementation is intended for internal use, e.g. Mesos is supposed
to only parse tokens that it also created. It doesn't fully comply with
RFC 7519. Currently the only supported cryptographic algorithm is HMAC
with SHA-256.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 75386184108214e67a58c328258ec204099d638c 
  3rdparty/libprocess/include/process/jwt.hpp PRE-CREATION 
  3rdparty/libprocess/src/jwt.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/jwt_tests.cpp PRE-CREATION 

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


Testing (updated)
---

make check


Thanks,

Jan Schlicht

Re: Review Request 56666: Added a HMAC SHA256 generator.

2017-02-16 Thread Jan Schlicht


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

(Updated Feb. 16, 2017, 10:34 a.m.)


Review request for mesos and Greg Mann.


Changes
---

Removed redundancy.


Summary (updated)
-

Added a HMAC SHA256 generator.


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


Repository: mesos


Description (updated)
---

HMAC SHA256 can be used to create or verify message signatures.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/utilities.hpp 
c2f64a91a505675d568ddf5aa081125e4e32fe17 
  3rdparty/libprocess/src/ssl/utilities.cpp 
8aec613312eee3dd11d9df8c3828a5185407e073 

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


Testing (updated)
---

make check


Thanks,

Jan Schlicht

Re: Review Request 56665: Added a URL-safe base64 implementation.

2017-02-16 Thread Jan Schlicht


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

(Updated Feb. 16, 2017, 10:33 a.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description (updated)
---

Base64 has many variants that use different alphabets for encoding.
"Base 64 Encoding with URL and Filename Safe Alphabet" is a variant
described in RFC 4648.


Diffs (updated)
-

  3rdparty/stout/include/stout/base64.hpp 
2ac04c4602bc919633a2a480dd2168b7aa301bd7 
  3rdparty/stout/tests/base64_tests.cpp 
32e516861d44c7e3a36e1a29b4d1fe5960684e3b 

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


Testing (updated)
---

make check


Thanks,

Jan Schlicht

Re: Review Request 56740: Update Allegro url on Powered By page.

2017-02-16 Thread Mesos Reviewbot


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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On Feb. 16, 2017, 7:23 a.m., Kamil Wargula wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56740/
> ---
> 
> (Updated Feb. 16, 2017, 7:23 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update Allegro url on Powered By page.
> 
> 
> Diffs
> -
> 
>   docs/powered-by-mesos.md 669c183ddd30556849b0a95bdb854e2d5e84bc1a 
> 
> Diff: https://reviews.apache.org/r/56740/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kamil Wargula
> 
>

54 matches

Mail list logo