Re: Review Request 37427: Docker registry: adding TokenManager.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37427/#review95238
---
src/slave/containerizer/provisioners/docker/token_manager.hpp (line 51)
https://reviews.apache.org/r/37427/#comment150088
space between // and TODO
src/slave/containerizer/provisioners/docker/token_manager.hpp (line 53)
https://reviews.apache.org/r/37427/#comment150087
Do javadoc style comments begin with /** ?
I think the same would apply to other javadoc style comments throughout
src/slave/containerizer/provisioners/docker/token_manager.cpp (lines 37 - 46)
https://reviews.apache.org/r/37427/#comment150097
no need to put underscore in front of parameters being passed in
src/slave/containerizer/provisioners/docker/token_manager.cpp (line 135)
https://reviews.apache.org/r/37427/#comment150095
space between ) and {
src/slave/containerizer/provisioners/docker/token_manager.cpp (line 230)
https://reviews.apache.org/r/37427/#comment150093
pass in token by const reference?
src/slave/containerizer/provisioners/docker/token_manager.cpp (line 252)
https://reviews.apache.org/r/37427/#comment150094
space between () {}, you could also probably just do this in the header file
src/tests/containerizer/docker_containerizer_tests.cpp (line 2998)
https://reviews.apache.org/r/37427/#comment150089
DockerContainerizerTest tests the Docker Containerizer. The TokenManager is
a component of the Mesos Containerizer, and would therefore be a part of a
Docker provisioner test file.
- Lily Chen
On Aug. 13, 2015, 4:47 a.m., Jojy Varghese wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37427/
---
(Updated Aug. 13, 2015, 4:47 a.m.)
Review request for mesos, Lily Chen and Timothy Chen.
Repository: mesos
Description
---
Changes:
- Added Token implementation (RFC 7519).
- Added TokenManager implementation. This component keeps a cache of tokens
requested for any future requests.
Diffs
-
src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960
src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION
src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION
src/tests/containerizer/docker_containerizer_tests.cpp
c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea
Diff: https://reviews.apache.org/r/37427/diff/
Testing
---
make check.
Thanks,
Jojy Varghese
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 13, 2015, 10:57 a.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Changes --- Addresses Bernd's and Tim's issues. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a4a9c6f3afa2441e958e472b264c57b5c5d18eb1 include/mesos/type_utils.hpp 6ff061c94e9d3dbb55bd047840b2012d11e3a0f2 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 579c009a1b43bc25c294366de29a56dacc734791 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 4e2947003247c3d7bf407d6d709faf2e878418f3 src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37328: Remove namespace ambiguity
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37328/#review95241 --- Patch looks great! Reviews applied: [37302, 37303, 37328] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 5:43 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37328/ --- (Updated Aug. 13, 2015, 5:43 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- Remove namespace ambiguity. Needed for r37303 Diffs - src/tests/common/http_tests.cpp 38d062b2b4062e0a2fc912bad0cc2d73339eb66e Diff: https://reviews.apache.org/r/37328/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 8:29 a.m.) Review request for mesos, Lily Chen and Timothy Chen. Changes --- build fix; review comments addressed. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs (updated) - src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95243 --- Patch looks great! Reviews applied: [37426, 37427] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 8:29 a.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 8:29 a.m.) Review request for mesos, Lily Chen and Timothy Chen. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs - src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 13, 2015, 1:20 p.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Changes --- Fixed Typo. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a4a9c6f3afa2441e958e472b264c57b5c5d18eb1 include/mesos/type_utils.hpp 6ff061c94e9d3dbb55bd047840b2012d11e3a0f2 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 579c009a1b43bc25c294366de29a56dacc734791 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 4e2947003247c3d7bf407d6d709faf2e878418f3 src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36050: Added test authorizer module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36050/#review95251 --- Ship it! Ship It! - Bernd Mathiske On Aug. 12, 2015, 6:52 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36050/ --- (Updated Aug. 12, 2015, 6:52 a.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds a test authorizer module. Updates the authorization tests in order to perform typed tests on the default authorizer implementation and on the test authorizer module. Diffs - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/examples/test_authorizer_module.cpp PRE-CREATION src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/module.hpp 03756a2536ca3e662ba422e96d121a6c39bb8c84 src/tests/module.cpp 61d4753f0f098005f56dd4a24984e30405c32558 Diff: https://reviews.apache.org/r/36050/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/#review95249 --- Ship it! Fix minor typo, then ship it! (We know that this is not the eventual intended shape of authz, but it is a self-contained, consistent step in the right direction.) include/mesos/authorizer/authorizer.hpp (line 61) https://reviews.apache.org/r/36048/#comment150104 s/is/if - Bernd Mathiske On Aug. 13, 2015, 1:57 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 13, 2015, 1:57 a.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a4a9c6f3afa2441e958e472b264c57b5c5d18eb1 include/mesos/type_utils.hpp 6ff061c94e9d3dbb55bd047840b2012d11e3a0f2 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 579c009a1b43bc25c294366de29a56dacc734791 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 4e2947003247c3d7bf407d6d709faf2e878418f3 src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37303: Moved scheduler library to http
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/#review95240 --- Ship it! Looks good modulo minor fixes. I'll fix them and commit for you. src/scheduler/scheduler.cpp (line 329) https://reviews.apache.org/r/37303/#comment150090 indentation. src/scheduler/scheduler.cpp (line 393) https://reviews.apache.org/r/37303/#comment150091 No comment here? src/scheduler/scheduler.cpp (line 412) https://reviews.apache.org/r/37303/#comment150092 indentation. - Vinod Kone On Aug. 13, 2015, 5:39 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37303/ --- (Updated Aug. 13, 2015, 5:39 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2552 https://issues.apache.org/jira/browse/MESOS-2552 Repository: mesos Description --- This changes moves scheduler library to http. The change to remove auth + install,receive handlers are in other reviews. Diffs - src/common/http.hpp 98a12709ae5fcbf4ce3e37d164bff43818096838 src/scheduler/scheduler.cpp 3fbe3831cf93c29180bc7e433fd57c6108988316 Diff: https://reviews.apache.org/r/37303/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37420: Added note regarding glog and gflags to documentation
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37420/#review95245 --- Thanks for taking this on Greg. docs/getting-started.md (line 1) https://reviews.apache.org/r/37420/#comment150101 Whooopsi?! :) docs/getting-started.md (line 174) https://reviews.apache.org/r/37420/#comment150105 The current patch for glog is not related to this very issue - that old patch that I recently resurrected does. Our complete glog patch does other things that are mostly fixing C++11 stuff and standard library issues. So we might want to be a bit more precise here - maybe by even referencing the ticket and the exact patch. What do you think? - Till Toenshoff On Aug. 12, 2015, 10:25 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37420/ --- (Updated Aug. 12, 2015, 10:25 p.m.) Review request for mesos, Joris Van Remoortere and Till Toenshoff. Bugs: MESOS-1010 https://issues.apache.org/jira/browse/MESOS-1010 Repository: mesos Description --- Added note regarding glog and gflags to documentation Diffs - docs/getting-started.md 1203ab88fde1c4a72c1bef6e85293f4d9abb2e24 Diff: https://reviews.apache.org/r/37420/diff/ Testing --- Thanks, Greg Mann
Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated Aug. 13, 2015, 1:52 p.m.) Review request for mesos, Benjamin Hindman and Timothy Chen. Changes --- Addressed comment and rebased. Bugs: MESOS-3086 https://issues.apache.org/jira/browse/MESOS-3086 Repository: mesos Description --- WIP Added Non-Freezeer Task Killer. Diffs (updated) - src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f Diff: https://reviews.apache.org/r/36620/diff/ Testing (updated) --- sudo make check + manual tests Thanks, Joerg Schad
Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.
On July 28, 2015, 7:20 p.m., Timothy Chen wrote: I notice there are no new tests added for this, can you add a test to verify the new change works? Timothy Chen wrote: Are you able to add this? Otherwise let's add a TODO and get this in. Created MESOS-3255 to follow up with tests. - Joerg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/#review93329 --- On July 24, 2015, 1:07 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated July 24, 2015, 1:07 p.m.) Review request for mesos, Benjamin Hindman and Timothy Chen. Bugs: MESOS-3086 https://issues.apache.org/jira/browse/MESOS-3086 Repository: mesos Description --- WIP Added Non-Freezeer Task Killer. Diffs - src/linux/cgroups.cpp e062fcbd56315f11882fe0ccb615c490dd719934 Diff: https://reviews.apache.org/r/36620/diff/ Testing --- sudo make check Thanks, Joerg Schad
Re: Review Request 34128: Enable different IP/Port for external access.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34128/#review95256 --- Can this be merged now unless there are more comments? - Anindya Sinha On July 30, 2015, 10:36 p.m., Anindya Sinha wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34128/ --- (Updated July 30, 2015, 10:36 p.m.) Review request for mesos. Bugs: MESOS-809 https://issues.apache.org/jira/browse/MESOS-809 Repository: mesos Description --- Expose environment variables LIBPROCESS_ADVERTISE_IP and LIBPROCESS_ADVERTISE_PORT as the IP and port which libprocess would advertise (if set). If not set, it defaults to the IP and port on which it binded to. Diffs - 3rdparty/libprocess/src/process.cpp 6d3609d06d017d8e50d7935b335fda7ebecbd04c Diff: https://reviews.apache.org/r/34128/diff/ Testing --- Testing: make test Thanks, Anindya Sinha
Re: Review Request 34129: Add 2 optional args advertise_ip and advertise_port for libprocess to advertise.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/#review95257 --- Can this be merged now unless there are more comments? Also needs https://reviews.apache.org/r/34128 - Anindya Sinha On July 30, 2015, 10:36 p.m., Anindya Sinha wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/ --- (Updated July 30, 2015, 10:36 p.m.) Review request for mesos and Cosmin Lehene. Bugs: MESOS-809 https://issues.apache.org/jira/browse/MESOS-809 Repository: mesos Description --- If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done. Command line arguments added: advertise_ip: IP address advertised to reach mesos master. advertise_port: Port advertised to reach mesos master (used with advertise_ip). Diffs - docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff docs/operational-guide.md ef81db672bd5c8fab172336956ab03461ae35064 src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce Diff: https://reviews.apache.org/r/34129/diff/ Testing --- Testing: make test Thanks, Anindya Sinha
Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/#review95259 --- Patch looks great! Reviews applied: [36612, 36620] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 1:52 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated Aug. 13, 2015, 1:52 p.m.) Review request for mesos, Benjamin Hindman and Timothy Chen. Bugs: MESOS-3086 https://issues.apache.org/jira/browse/MESOS-3086 Repository: mesos Description --- WIP Added Non-Freezeer Task Killer. Diffs - src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f Diff: https://reviews.apache.org/r/36620/diff/ Testing --- sudo make check + manual tests Thanks, Joerg Schad
Re: Review Request 37427: Docker registry: adding TokenManager.
On Aug. 13, 2015, 6:24 a.m., Lily Chen wrote: src/slave/containerizer/provisioners/docker/token_manager.cpp, lines 37-46 https://reviews.apache.org/r/37427/diff/2/?file=1039212#file1039212line37 no need to put underscore in front of parameters being passed in As per style docs, its an acceptable way to disambiguate. On Aug. 13, 2015, 6:24 a.m., Lily Chen wrote: src/tests/containerizer/docker_containerizer_tests.cpp, line 2998 https://reviews.apache.org/r/37427/diff/2/?file=1039213#file1039213line2998 DockerContainerizerTest tests the Docker Containerizer. The TokenManager is a component of the Mesos Containerizer, and would therefore be a part of a Docker provisioner test file. I understand. Since other part of proviosioner feature is being worked on in parallel, I have put these tests in this file, as I didnt see a separate file for provisioner tests. I have added a TODO in the test file to move these tests to the new file when it is dropped. Also, these new tests are isolated from the already existing tests in the file. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95238 --- On Aug. 13, 2015, 8:29 a.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 8:29 a.m.) Review request for mesos, Lily Chen and Timothy Chen. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs - src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 36847: Added HTTP Delete Method.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/#review95277 --- Patch looks great! Reviews applied: [36847] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 4:19 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/ --- (Updated Aug. 13, 2015, 4:19 p.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Bugs: MESOS-3152 https://issues.apache.org/jira/browse/MESOS-3152 Repository: mesos Description --- see summary. Diffs - 3rdparty/libprocess/include/process/http.hpp b829dad9d1c5d98f09a3a3c7cc04e7659f6616a4 3rdparty/libprocess/src/http.cpp b70e2fa0f500216c15f20907816874b9486826b8 3rdparty/libprocess/src/tests/http_tests.cpp 7351fa44e69743ab82e9cada7ba7f797a6899bab Diff: https://reviews.apache.org/r/36847/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 34129: Add 2 optional args advertise_ip and advertise_port for libprocess to advertise.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/#review95287 --- Ship it! Ship It! - Vinod Kone On July 30, 2015, 10:36 p.m., Anindya Sinha wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/ --- (Updated July 30, 2015, 10:36 p.m.) Review request for mesos and Cosmin Lehene. Bugs: MESOS-809 https://issues.apache.org/jira/browse/MESOS-809 Repository: mesos Description --- If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done. Command line arguments added: advertise_ip: IP address advertised to reach mesos master. advertise_port: Port advertised to reach mesos master (used with advertise_ip). Diffs - docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff docs/operational-guide.md ef81db672bd5c8fab172336956ab03461ae35064 src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce Diff: https://reviews.apache.org/r/34129/diff/ Testing --- Testing: make test Thanks, Anindya Sinha
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review95303 --- Ship it! LGTM module the issue below. BTW, do we know if anyone is using the `mesos-cli` tool? I had a discussion with ThomasR a while ago and he suggested that the `mesos-cli` tool should be removed from mesos source repo entirely. src/cli/python/mesos/__init__.py (line 25) https://reviews.apache.org/r/36819/#comment150189 Don't we need a `mesos.cli` here as well? - Kapil Arya On Aug. 7, 2015, 11:08 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 7, 2015, 11:08 a.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 0794279dd2e23b5b593e7e388bd6d04e17c746a6 src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
Re: Review Request 36819: Use setup.py in python cli package.
On Aug. 13, 2015, 6:27 p.m., Kapil Arya wrote: src/cli/python/mesos/__init__.py, line 27 https://reviews.apache.org/r/36819/diff/6/?file=1034329#file1034329line27 Don't we need a `mesos.cli` here as well? Kartic Krish wrote: Looks like the module structure has been changed, so just 'packages': [ 'mesos' ] should be OK. Yes?not need mesos.cli here. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review95303 --- On Aug. 7, 2015, 3:08 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 7, 2015, 3:08 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 0794279dd2e23b5b593e7e388bd6d04e17c746a6 src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
Re: Review Request 37414: Fix flaky ExamplesTest.JavaLog
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37414/ --- (Updated Aug. 13, 2015, 4:49 p.m.) Review request for mesos, Joris Van Remoortere, Joseph Wu, and Till Toenshoff. Bugs: MESOS-1013 https://issues.apache.org/jira/browse/MESOS-1013 Repository: mesos Description --- Fix flaky ExamplesTest.JavaLog Diffs (updated) - src/examples/java/TestLog.java 9dd4630745e4638f5c347c8a69eec30fdad3af56 Diff: https://reviews.apache.org/r/37414/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaLog bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Re: Review Request 36819: Use setup.py in python cli package.
On Aug. 13, 2015, 4 p.m., Marco Massenzio wrote: Today we are cutting a 0.24 RC (according to @Vinod's recent email) - it would be great if we could have this fix in, which solves an issue with the Python installers. Can anyone please do anything about this one? Thanks! T_T? thank you very much @marco. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review95262 --- On Aug. 7, 2015, 3:08 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 7, 2015, 3:08 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 0794279dd2e23b5b593e7e388bd6d04e17c746a6 src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
Re: Review Request 37415: Fix flaky ExamplesTest.JavaFramework
On Aug. 13, 2015, 9:22 a.m., Till Toenshoff wrote: src/examples/java/TestFramework.java, line 275 https://reviews.apache.org/r/37415/diff/1/?file=1038842#file1038842line275 Why 500 - not 200 or 1000? If I am not mistaken, then using 500 worked fine on your test-machines but chances are that someone comes in with a heavily loaded machine where those 500ms might not do, correct? Let us be honest and let the audience know that this is just a value chosen out of some test runs done - again, if I did not miss something :) I think it would also be a great idea to add a todo here that captures what you wrote within the JIRA issue already; TODO(*): Ideally, we would inspect the status of the driver and its associated tasks via the Java API and wait until their teardown is complete to exit. Greg Mann wrote: You are correct! :-) Thanks, comments addressed. Agreed. I don't think a sleep if a good enough solution. (Note: This next part is just supposition.) If you take a look at the comment here: https://github.com/apache/mesos/blob/437e7018e184128c9ec7c4113da09bbf9914721f/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp#L485 You'll notice that the SchedulerDriver is instantiated as a weak reference. So the GC is relied upon to destroy the object. See: https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#weak Have you tried changing it to a normal global reference and adding the explicit cleanup code into the `stop` function? - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/#review95264 --- On Aug. 13, 2015, 9:49 a.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/ --- (Updated Aug. 13, 2015, 9:49 a.m.) Review request for mesos, Joris Van Remoortere, Joseph Wu, and Till Toenshoff. Bugs: MESOS-830 https://issues.apache.org/jira/browse/MESOS-830 Repository: mesos Description --- Fix flaky ExamplesTest.JavaFramework Diffs - src/examples/java/TestFramework.java 265005b9d1a6a053b812c133a4817c3d9e61179e Diff: https://reviews.apache.org/r/37415/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaFramework bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 13, 2015, 6:46 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Changes --- Add shebang to all setup.py.in Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs (updated) - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac b9ecafd1813d87d604364d354994e4dba386f958 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/setup.py.in 880e2a6fe4470e50bbfa805967fb983fb8a9987f src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/setup.py.in 9fc9ad2bdeaf1ad3f007e36f24bbe5b10dedc77b src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/setup.py.in 72cb770bda659957c03853c3c99ba6c53f606df8 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/setup.py.in 8ffde68e60e8c968f196e6e0790aeef47d0fb662 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
Re: Review Request 37415: Fix flaky ExamplesTest.JavaFramework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/#review95299 --- Patch looks great! Reviews applied: [37414, 37415] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 4:49 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/ --- (Updated Aug. 13, 2015, 4:49 p.m.) Review request for mesos, Joris Van Remoortere, Joseph Wu, and Till Toenshoff. Bugs: MESOS-830 https://issues.apache.org/jira/browse/MESOS-830 Repository: mesos Description --- Fix flaky ExamplesTest.JavaFramework Diffs - src/examples/java/TestFramework.java 265005b9d1a6a053b812c133a4817c3d9e61179e Diff: https://reviews.apache.org/r/37415/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaFramework bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Re: Review Request 37415: Fix flaky ExamplesTest.JavaFramework
On Aug. 13, 2015, 4:22 p.m., Till Toenshoff wrote: src/examples/java/TestFramework.java, line 275 https://reviews.apache.org/r/37415/diff/1/?file=1038842#file1038842line275 Why 500 - not 200 or 1000? If I am not mistaken, then using 500 worked fine on your test-machines but chances are that someone comes in with a heavily loaded machine where those 500ms might not do, correct? Let us be honest and let the audience know that this is just a value chosen out of some test runs done - again, if I did not miss something :) I think it would also be a great idea to add a todo here that captures what you wrote within the JIRA issue already; TODO(*): Ideally, we would inspect the status of the driver and its associated tasks via the Java API and wait until their teardown is complete to exit. You are correct! :-) Thanks, comments addressed. - Greg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/#review95264 --- On Aug. 13, 2015, 4:49 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/ --- (Updated Aug. 13, 2015, 4:49 p.m.) Review request for mesos, Joris Van Remoortere, Joseph Wu, and Till Toenshoff. Bugs: MESOS-830 https://issues.apache.org/jira/browse/MESOS-830 Repository: mesos Description --- Fix flaky ExamplesTest.JavaFramework Diffs - src/examples/java/TestFramework.java 265005b9d1a6a053b812c133a4817c3d9e61179e Diff: https://reviews.apache.org/r/37415/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaFramework bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Re: Review Request 36819: Use setup.py in python cli package.
On Aug. 13, 2015, 6:27 p.m., Kapil Arya wrote: src/cli/python/mesos/__init__.py, line 27 https://reviews.apache.org/r/36819/diff/6/?file=1034329#file1034329line27 Don't we need a `mesos.cli` here as well? Looks like the module structure has been changed, so just 'packages': [ 'mesos' ] should be OK. - Kartic --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review95303 --- On Aug. 7, 2015, 3:08 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 7, 2015, 3:08 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 0794279dd2e23b5b593e7e388bd6d04e17c746a6 src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review95305 --- Looks good. Please include the she-bang in setup.py (as per previous reviewers comment). - Kartic Krish On Aug. 7, 2015, 3:08 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 7, 2015, 3:08 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 0794279dd2e23b5b593e7e388bd6d04e17c746a6 src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37416/ --- (Updated Aug. 13, 2015, 6:40 p.m.) Review request for mesos and Ben Mahler. Changes --- Address reviewer comments. Repository: mesos Description --- Perf supported() should be based on the version of perf, not the version of the kernel. Diffs (updated) - src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 Diff: https://reviews.apache.org/r/37416/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37426: MESOS-3251 : Fixing host field of request header.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37426/#review95281 --- I wonder if we should add a test for this so that we can assert that the fix works ? One easy way I can think about it, looking at the tests in process_tests.cpp ( you should be able to use the already existing HttpEndpointProcess there ), set up a route for a mock handler. Send in a http::Request to this mock route endpoint with URL(http://TEST_URL) , the handler on the server side should check the host header to be TEST_URL in the http::Request ? - Anand Mazumdar On Aug. 13, 2015, 12:46 a.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37426/ --- (Updated Aug. 13, 2015, 12:46 a.m.) Review request for mesos, Anand Mazumdar and Timothy Chen. Bugs: MESOS-3251 https://issues.apache.org/jira/browse/MESOS-3251 Repository: mesos Description --- Currently libprocess http API sets the Host header field from the peer socket address (IP:port). The problem is that socket address might not be right HTTP server and might be just a proxy. Changed the logic to look for http server URL's domain first to populate the Host field. Diffs - 3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 Diff: https://reviews.apache.org/r/37426/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 13, 2015, 7:08 p.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Changes --- Addresses Tim's issues. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a4a9c6f3afa2441e958e472b264c57b5c5d18eb1 include/mesos/type_utils.hpp 6ff061c94e9d3dbb55bd047840b2012d11e3a0f2 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 579c009a1b43bc25c294366de29a56dacc734791 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 4e2947003247c3d7bf407d6d709faf2e878418f3 src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
On Aug. 13, 2015, 6:43 p.m., Timothy Chen wrote: src/authorizer/authorizer.cpp, line 19 https://reviews.apache.org/r/36048/diff/18/?file=1039308#file1039308line19 Is this our new style guide rule too to put this up before all other imports? Otherwise I would have still sort it the way we used to It comes from the google style guide, which we are taking more seriously: the first include should be that of the companion header. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/#review95271 --- On Aug. 13, 2015, 7:08 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 13, 2015, 7:08 p.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a4a9c6f3afa2441e958e472b264c57b5c5d18eb1 include/mesos/type_utils.hpp 6ff061c94e9d3dbb55bd047840b2012d11e3a0f2 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 579c009a1b43bc25c294366de29a56dacc734791 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 4e2947003247c3d7bf407d6d709faf2e878418f3 src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37426: MESOS-3251 : Fixing host field of request header.
On Aug. 13, 2015, 4:56 p.m., Anand Mazumdar wrote: I wonder if we should add a test for this so that we can assert that the fix works ? One easy way I can think about it, looking at the tests in process_tests.cpp ( you should be able to use the already existing HttpEndpointProcess there ), set up a route for a mock handler. Send in a http::Request to this mock route endpoint with URL(http://TEST_URL) , the handler on the server side should check the host header to be TEST_URL in the http::Request ? We have many HTTP(S) tests already in place. Those tests validate this change. Any test that uses URL for a GET request validates this change. There are a few simple HTTPs tests for get and also the new docker provisioner tests validate it. - Jojy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37426/#review95281 --- On Aug. 13, 2015, 12:46 a.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37426/ --- (Updated Aug. 13, 2015, 12:46 a.m.) Review request for mesos, Anand Mazumdar and Timothy Chen. Bugs: MESOS-3251 https://issues.apache.org/jira/browse/MESOS-3251 Repository: mesos Description --- Currently libprocess http API sets the Host header field from the peer socket address (IP:port). The problem is that socket address might not be right HTTP server and might be just a proxy. Changed the logic to look for http server URL's domain first to populate the Host field. Diffs - 3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 Diff: https://reviews.apache.org/r/37426/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 10:26 p.m.) Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen. Changes --- addressed review comments. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs (updated) - src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Review Request 37449: Fixed scheduler tests to work with heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37449/ --- Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-3260 https://issues.apache.org/jira/browse/MESOS-3260 Repository: mesos Description --- As a short term fix stopped enqueuing heartbeat events in the queue. Diffs - src/tests/scheduler_tests.cpp 837d9e8471548dfe4ab3c9ec9822947f51848ab4 Diff: https://reviews.apache.org/r/37449/diff/ Testing --- make check Ran the scheduler tests in a loop. Thanks, Vinod Kone
Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/#review95321 --- I re-created the issues from the previous avatar (https://reviews.apache.org/r/36867). src/tests/fault_tolerance_tests.cpp (lines 1834 - 1837) https://reviews.apache.org/r/37443/#comment150207 You should be able to replace these lines with: `framework1.mutable_labels()-add_labels()-CopyFrom(createLabel(foo, bar));` src/tests/fault_tolerance_tests.cpp (lines 1869 - 1872) https://reviews.apache.org/r/37443/#comment150208 same as above src/tests/fault_tolerance_tests.cpp (lines 1944 - 1950) https://reviews.apache.org/r/37443/#comment150209 This block can be replaced with: EXPECT_EQ( JSON::Value(JSON::Protobuf(createLabel(foo, bar))), labelsObject.values[0]); src/tests/master_tests.cpp (line 3604) https://reviews.apache.org/r/37443/#comment150218 Unused. Remove. src/tests/master_tests.cpp (line 3608) https://reviews.apache.org/r/37443/#comment150211 There are only two labels below. src/tests/master_tests.cpp (lines 3612 - 3622) https://reviews.apache.org/r/37443/#comment150210 same as above. src/tests/master_tests.cpp (line 3637) https://reviews.apache.org/r/37443/#comment150216 two space indentation here. src/tests/master_tests.cpp (lines 3650 - 3651) https://reviews.apache.org/r/37443/#comment150213 Indent by four space. src/tests/master_tests.cpp (lines 3654 - 3655) https://reviews.apache.org/r/37443/#comment150214 Indent by four spaces. src/tests/master_tests.cpp (lines 3658 - 3659) https://reviews.apache.org/r/37443/#comment150215 Indent by four spaces. - Kapil Arya On Aug. 13, 2015, 3:50 p.m., James DeFelice wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- (Updated Aug. 13, 2015, 3:50 p.m.) Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 36049: Added support for modularized Authorizer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review95328 --- Ship it! src/local/local.cpp (line 219) https://reviews.apache.org/r/36049/#comment150242 s/authenticator/authorizer/ - will fix while committing. src/local/local.cpp (line 222) https://reviews.apache.org/r/36049/#comment150243 See above. src/master/main.cpp (line 349) https://reviews.apache.org/r/36049/#comment150244 See above. src/master/main.cpp (line 352) https://reviews.apache.org/r/36049/#comment150245 See above. - Till Toenshoff On Aug. 12, 2015, 1:52 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/ --- (Updated Aug. 12, 2015, 1:52 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds and integrates helper classes needed to support an `Authorizer` module. Also adds a flag to the master, allowing the selection of an `Authorizer` module. Diffs - docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/module/authorizer.hpp PRE-CREATION src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/authorizer/authorizer.cpp PRE-CREATION src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd Diff: https://reviews.apache.org/r/36049/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37449: Fixed scheduler tests to work with heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37449/#review95349 --- Ship it! LGTM ! src/tests/scheduler_tests.cpp (line 103) https://reviews.apache.org/r/37449/#comment150260 micro nit : let's just do Event::HEARTBEAT - Anand Mazumdar On Aug. 13, 2015, 11:23 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37449/ --- (Updated Aug. 13, 2015, 11:23 p.m.) Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-3260 https://issues.apache.org/jira/browse/MESOS-3260 Repository: mesos Description --- As a short term fix stopped enqueuing heartbeat events in the queue. Diffs - src/tests/scheduler_tests.cpp 837d9e8471548dfe4ab3c9ec9822947f51848ab4 Diff: https://reviews.apache.org/r/37449/diff/ Testing --- make check Ran the scheduler tests in a loop. Thanks, Vinod Kone
Re: Review Request 37311: Implemented a 'read-only' Appc image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37311/ --- (Updated Aug. 13, 2015, 2:39 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Changes --- Comments. Bugs: MESOS-3194 https://issues.apache.org/jira/browse/MESOS-3194 Repository: mesos Description --- Implemented a 'read-only' Appc image store. Diffs (updated) - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37311/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37310: Added Appc spec validation utility.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/#review95333 --- Ship it! src/slave/containerizer/provisioners/appc/spec.hpp (lines 41 - 42) https://reviews.apache.org/r/37310/#comment150246 We wrap comments in 70 char width. Please make sure this is the case:) - Jie Yu On Aug. 13, 2015, 9:33 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/ --- (Updated Aug. 13, 2015, 9:33 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Repository: mesos Description --- Added Appc spec validation utility. Diffs - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37310/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
On Aug. 13, 2015, 8:06 p.m., Till Toenshoff wrote: Thanks for your patience Alexander - I know it has been a long journey - in the end, I think things are in great shape minus some nits I just discovered on my final pass. Please note that I will commit while fixing the noted issues but I will leave them open for your attention :) - close once you saw them. - Till --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/#review95266 --- On Aug. 13, 2015, 5:08 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 13, 2015, 5:08 p.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a4a9c6f3afa2441e958e472b264c57b5c5d18eb1 include/mesos/type_utils.hpp 6ff061c94e9d3dbb55bd047840b2012d11e3a0f2 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 579c009a1b43bc25c294366de29a56dacc734791 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 4e2947003247c3d7bf407d6d709faf2e878418f3 src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37310: Added Appc spec validation utility.
On Aug. 13, 2015, 9:39 p.m., Jie Yu wrote: src/slave/containerizer/provisioners/appc/spec.hpp, lines 41-42 https://reviews.apache.org/r/37310/diff/4/?file=1039588#file1039588line41 We wrap comments in 70 char width. Please make sure this is the case:) Jiang Yan Xu wrote: I adjusted the line wrapping due to the discussion here: http://www.mail-archive.com/dev@mesos.apache.org/msg33053.html, which I support. What do you think? We should of course make it official on the style guide. Then we should at least make it less jagged. This particular comment is very jagged (with only one word in the next line). - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/#review95333 --- On Aug. 13, 2015, 9:33 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/ --- (Updated Aug. 13, 2015, 9:33 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Repository: mesos Description --- Added Appc spec validation utility. Diffs - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37310/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Review Request 37442: Factor out the token extraction rules in prepartion for extending them to cope with multiple versions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37442/ --- Review request for mesos and Ben Mahler. Repository: mesos Description --- Factor out the token extraction rules in prepartion for extending them to cope with multiple versions. Diffs - src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 Diff: https://reviews.apache.org/r/37442/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37311: Implemented a 'read-only' Appc image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37311/ --- (Updated Aug. 13, 2015, 3:11 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Changes --- Comments. NNFR. Bugs: MESOS-3194 https://issues.apache.org/jira/browse/MESOS-3194 Repository: mesos Description --- Implemented a 'read-only' Appc image store. Diffs (updated) - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37311/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37310: Added Appc spec validation utility.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/ --- (Updated Aug. 13, 2015, 3:12 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Changes --- Comments. NNFR. Repository: mesos Description --- Added Appc spec validation utility. Diffs (updated) - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37310/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37449: Fixed scheduler tests to work with heartbeats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37449/#review95348 --- Ship it! src/tests/scheduler_tests.cpp (line 106) https://reviews.apache.org/r/37449/#comment150259 nit: looks like a tab slipped in? (or may just be an RB artifact) - Marco Massenzio On Aug. 13, 2015, 11:23 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37449/ --- (Updated Aug. 13, 2015, 11:23 p.m.) Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-3260 https://issues.apache.org/jira/browse/MESOS-3260 Repository: mesos Description --- As a short term fix stopped enqueuing heartbeat events in the queue. Diffs - src/tests/scheduler_tests.cpp 837d9e8471548dfe4ab3c9ec9822947f51848ab4 Diff: https://reviews.apache.org/r/37449/diff/ Testing --- make check Ran the scheduler tests in a loop. Thanks, Vinod Kone
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 14, 2015, 12:09 a.m.) Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen. Changes --- added more tests. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs (updated) - src/Makefile.am b5dbdc316cad179cd265bd81900999fab35940b9 src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 37311: Implemented a 'read-only' Appc image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37311/#review95340 --- Ship it! src/slave/containerizer/provisioners/appc/store.cpp (lines 74 - 75) https://reviews.apache.org/r/37311/#comment150251 Please wrap the comments so that it's less jagged (e.g., wrapping at 70 char) src/slave/containerizer/provisioners/appc/store.cpp (lines 78 - 79) https://reviews.apache.org/r/37311/#comment150252 Ditto here. src/slave/containerizer/provisioners/appc/store.cpp (lines 119 - 120) https://reviews.apache.org/r/37311/#comment150250 Please make the comment here less jagged (e.g., wrapping at 70 char). src/slave/containerizer/provisioners/appc/store.cpp (line 121) https://reviews.apache.org/r/37311/#comment150254 s/createEntry/createImage/? - Jie Yu On Aug. 13, 2015, 9:39 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37311/ --- (Updated Aug. 13, 2015, 9:39 p.m.) Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-3194 https://issues.apache.org/jira/browse/MESOS-3194 Repository: mesos Description --- Implemented a 'read-only' Appc image store. Diffs - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37311/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Review Request 37445: Fix typos in style guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37445/ --- Review request for mesos. Repository: mesos Description --- Fix typos in style guide. Diffs - docs/mesos-c++-style-guide.md 9c1a00c32043fa10038e38bd7cbc561aafcd6ea0 Diff: https://reviews.apache.org/r/37445/diff/ Testing --- Thanks, Neil Conway
Re: Review Request 37289: Corrected the comments for DRFSorter::dirty.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37289/ --- (Updated Aug. 14, 2015, 12:06 a.m.) Review request for mesos and Alexander Rukletsov. Bugs: MESOS-3245 https://issues.apache.org/jira/browse/MESOS-3245 Repository: mesos Description --- Corrected the comments for DRFSorter::dirty. Diffs - src/master/allocator/sorter/drf/sorter.hpp f66ade06c6a5b4bf816839477cec2d18036c7b1a Diff: https://reviews.apache.org/r/37289/diff/ Testing (updated) --- make check Thanks, Qian Zhang
Re: Review Request 37310: Added Appc spec validation utility.
On Aug. 13, 2015, 2:39 p.m., Jie Yu wrote: src/slave/containerizer/provisioners/appc/spec.hpp, lines 41-42 https://reviews.apache.org/r/37310/diff/4/?file=1039588#file1039588line41 We wrap comments in 70 char width. Please make sure this is the case:) I adjusted the line wrapping due to the discussion here: http://www.mail-archive.com/dev@mesos.apache.org/msg33053.html, which I support. What do you think? We should of course make it official on the style guide. - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/#review95333 --- On Aug. 13, 2015, 2:33 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/ --- (Updated Aug. 13, 2015, 2:33 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Repository: mesos Description --- Added Appc spec validation utility. Diffs - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37310/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37257: Add SUPPRESS call interface to the scheduler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37257/#review95351 --- Bad patch! Reviews applied: [37257] Failed command: ./support/apply-review.sh -n -r 37257 Error: 2015-08-13 23:39:06 URL:https://reviews.apache.org/r/37257/diff/raw/ [5394/5394] - 37257.patch [1] error: patch failed: src/tests/scheduler_tests.cpp:903 error: src/tests/scheduler_tests.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On Aug. 13, 2015, 7:44 p.m., Guangya Liu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37257/ --- (Updated Aug. 13, 2015, 7:44 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3037 https://issues.apache.org/jira/browse/MESOS-3037 Repository: mesos Description --- This is just part of MESOS-3037, this patch only add the interface of SUPPRESS call. Diffs - include/mesos/scheduler.hpp cd235a11e63a5df742057be8e27629db4cf9 include/mesos/scheduler/scheduler.proto 89daf8a6b74057ee156b3ad691397e76fcb835b8 src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 src/tests/scheduler_tests.cpp 9d29d1ab0cb2914f4749e05af95c1c21e88908ca Diff: https://reviews.apache.org/r/37257/diff/ Testing --- Thanks, Guangya Liu
Review Request 37457: Add --ip_discovery_command to Agent.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37457/
---
Review request for mesos, Cody Maloney and Timothy Chen.
Repository: mesos
Description
---
Similarly to the Master, we want to enable the Agent too to be initialized
with an IP address that is discovered by an arbitrary script.
This is particularly useful for Mesos deployments on public (or private) clouds
where DNS resolution is either limited or not feasible.
Jira: MESOS-3154
Diffs
-
src/slave/main.cpp 8008430e98773d8be9ba5ac6385cffb2e351932a
Diff: https://reviews.apache.org/r/37457/diff/
Testing
---
make check
run `./bin/mesos-slave --ip_discovery_command=/.../discover-ip.py ...`
and verified it binds to the expected address:
```
I0813 17:18:24.871472 2046800640 main.cpp:272] Starting Mesos slave
I0813 17:18:24.873610 292802560 slave.cpp:190] Slave started on
1)@192.168.33.1:5051
I0813 17:18:24.873633 292802560 slave.cpp:191] Flags at startup:
--appc_store_dir=/tmp/mesos/store/appc --authenticatee=crammd5
--container_disk_watch_interval=15secs --containerizers=mesos
--default_role=* --disk_watch_interval=1mins --docker=docker
--docker_kill_orphans=true --docker_remove_delay=6hrs
--docker_socket=/var/run/docker.sock --docker_stop_timeout=0ns
--enforce_container_disk_quota=false --executor_registration_timeout=1mins
--executor_shutdown_grace_period=5secs --fetcher_cache_dir=/tmp/mesos/fetch
--fetcher_cache_size=2GB --frameworks_home= --gc_delay=1weeks
--gc_disk_headroom=0.1 --hadoop_home= --help=false
--initialize_driver_logging=true
--ip_discovery_command=~/dev/discovery-ip.py
--isolation=posix/cpu,posix/mem
--launcher_dir=/Users/marco/dev/workspace/mesos/build/src --logbufsecs=0
--logging_level=INFO --master=127.0.0.1:5050 --modules=libraries {
file: /Users/marco/dev/workspace/modules/execute/build/Debug/libexecmod.so
modules {
name: com_mesosphere_mesos_RemoteExecutionAnonymous
}
}
--oversubscribed_resources_interval=15secs --port=5051
--qos_correction_interval_min=0ns --quiet=false --recover=reconnect
--recovery_timeout=15mins --registration_backoff_factor=1secs
--resource_monitoring_interval=1secs --sandbox_directory=/mnt/mesos/sandbox
--strict=true --switch_user=true --version=false --work_dir=/tmp/slave
I0813 17:18:24.875107 292802560 slave.cpp:354] Slave resources: cpus(*):8;
mem(*):15360; disk(*):470858; ports(*):[31000-32000]
I0813 17:18:24.881863 292802560 slave.cpp:384] Slave hostname: 192.168.33.1
...
```
Thanks,
Marco Massenzio
Re: Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/#review95362 --- Patch looks great! Reviews applied: [37443] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 8:20 p.m., James DeFelice wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- (Updated Aug. 13, 2015, 8:20 p.m.) Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 37403: Using AcceptMediaType Request method to validate Accept header
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37403/#review95296 --- Ship it! Some minor comments and noticed that we were not setting the response content type! I'll get these cleaned up and commit src/tests/http_api_tests.cpp (line 41) https://reviews.apache.org/r/37403/#comment150168 Why the move? src/tests/http_api_tests.cpp (line 563) https://reviews.apache.org/r/37403/#comment150172 How about 'NotAcceptable'? src/tests/http_api_tests.cpp (line 572) https://reviews.apache.org/r/37403/#comment150169 No need for the std:: prefix here? src/tests/http_api_tests.cpp (line 575) https://reviews.apache.org/r/37403/#comment150186 It's pretty implicit but http::streaming::post is already being passed the content type and will overwrite it, so this doesn't do anything. src/tests/http_api_tests.cpp (line 576) https://reviews.apache.org/r/37403/#comment150171 How about we write a valid format? e.g. text/html Note that these are supposed to be formatted as type/subtype src/tests/http_api_tests.cpp (line 606) https://reviews.apache.org/r/37403/#comment150187 This get overwritten to the parameterized content type when we pass 'contentType' into http::streaming::post. Otherwise this test would fail because we encode as either PROTOBUF or JSON depending on the test parameter. src/tests/http_api_tests.cpp (line 660) https://reviews.apache.org/r/37403/#comment150188 In all of these tests, how about we also validate the content type coming back in the response? Seems an important thing to test for 'Accept' working correctly. In particular, these don't test the behavior that JSON is the default? By adding this, I found a bug :) We aren't setting the content type of the response! - Ben Mahler On Aug. 12, 2015, 11:12 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37403/ --- (Updated Aug. 12, 2015, 11:12 p.m.) Review request for mesos and Ben Mahler. Bugs: MESOS-2497 https://issues.apache.org/jira/browse/MESOS-2497 Repository: mesos Description --- Using AcceptMediaType Request method to validate request 'Accept' HTTP header in Scheduler HTTP API Diffs - src/master/http.cpp 579c009 src/tests/http_api_tests.cpp aef3c4b Diff: https://reviews.apache.org/r/37403/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- (Updated Aug. 13, 2015, 7:50 p.m.) Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Changes --- Previous diff was uploaded with a parent diff, I think I confused reviewboard Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs (updated) - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/#review95325 --- Ship it! Ship It! - Kapil Arya On Aug. 13, 2015, 4:20 p.m., James DeFelice wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- (Updated Aug. 13, 2015, 4:20 p.m.) Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 37308: Added AppcImageManifest protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37308/ --- (Updated Aug. 13, 2015, 2:24 p.m.) Review request for mesos, Ian Downes and Jie Yu. Changes --- Comments. Bugs: MESOS-3194 https://issues.apache.org/jira/browse/MESOS-3194 Repository: mesos Description --- Derived from /r/34142/. Diffs (updated) - include/mesos/mesos.proto 80f56ac2178b24ff19f57c1ace13f567843c7807 Diff: https://reviews.apache.org/r/37308/diff/ Testing --- Tested along with /r/37310/. Thanks, Jiang Yan Xu
Re: Review Request 36050: Added test authorizer module.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36050/#review95330 --- Ship it! Ship It! - Till Toenshoff On Aug. 12, 2015, 1:52 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36050/ --- (Updated Aug. 12, 2015, 1:52 p.m.) Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2947 https://issues.apache.org/jira/browse/MESOS-2947 Repository: mesos Description --- Adds a test authorizer module. Updates the authorization tests in order to perform typed tests on the default authorizer implementation and on the test authorizer module. Diffs - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/examples/test_authorizer_module.cpp PRE-CREATION src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/module.hpp 03756a2536ca3e662ba422e96d121a6c39bb8c84 src/tests/module.cpp 61d4753f0f098005f56dd4a24984e30405c32558 Diff: https://reviews.apache.org/r/36050/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/#review95320 --- src/tests/master_tests.cpp (line 3608) https://reviews.apache.org/r/37443/#comment150206 This comment is out of date. Will fix ASAP - James DeFelice On Aug. 13, 2015, 7:39 p.m., James DeFelice wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- (Updated Aug. 13, 2015, 7:39 p.m.) Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 37415: Fix flaky ExamplesTest.JavaFramework
On Aug. 13, 2015, 4:22 p.m., Till Toenshoff wrote: src/examples/java/TestFramework.java, line 275 https://reviews.apache.org/r/37415/diff/1/?file=1038842#file1038842line275 Why 500 - not 200 or 1000? If I am not mistaken, then using 500 worked fine on your test-machines but chances are that someone comes in with a heavily loaded machine where those 500ms might not do, correct? Let us be honest and let the audience know that this is just a value chosen out of some test runs done - again, if I did not miss something :) I think it would also be a great idea to add a todo here that captures what you wrote within the JIRA issue already; TODO(*): Ideally, we would inspect the status of the driver and its associated tasks via the Java API and wait until their teardown is complete to exit. Greg Mann wrote: You are correct! :-) Thanks, comments addressed. Joseph Wu wrote: Agreed. I don't think a sleep if a good enough solution. (Note: This next part is just supposition.) If you take a look at the comment here: https://github.com/apache/mesos/blob/437e7018e184128c9ec7c4113da09bbf9914721f/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp#L485 You'll notice that the SchedulerDriver is instantiated as a weak reference. So the GC is relied upon to destroy the object. See: https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#weak Have you tried changing it to a normal global reference and adding the explicit cleanup code into the `stop` function? Greg Mann wrote: Yea, I'm not crazy about the sleep solution either. I just tried your suggestion, Joseph, putting a DeleteGlobalRef() call in the stop() method, and it didn't work unfortunately. I think that short of an explicit confirmation message upon teardown completion (which doesn't currently exist, and is perhaps not advisable anyway), I don't see a way of resolving this in an elegant fashion. Even if we're sure that the SchedulerDriver is gone before we call System.exit(), the associated Executors and the Slave are doing logging as they shut down, and it's the premature destruction of the mutexes used by glog that is causing the problem. Well, @joseph - the comment you point to, pretty much negates that option, doesn't it? // Create a weak global reference to the MesosSchedulerDriver // instance (we want a global reference so the GC doesn't collect // the instance but we make it weak so the JVM can exit). Bearing in mind that this is test code, which is only run at exit (ie, one-off and not in a performance-critical section) I think that adding a sleep is acceptable (if not exciting). Sure, a callback that says hey, I'm done now, you can go away would be great, but I'd guess, probably overkill. Totally +1 to the comment and the TODO - hard-coded 'magic numbers are A Bad Thing, so definitely worth going the extra mile and explaining them. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/#review95264 --- On Aug. 13, 2015, 4:49 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/ --- (Updated Aug. 13, 2015, 4:49 p.m.) Review request for mesos, Joris Van Remoortere, Joseph Wu, and Till Toenshoff. Bugs: MESOS-830 https://issues.apache.org/jira/browse/MESOS-830 Repository: mesos Description --- Fix flaky ExamplesTest.JavaFramework Diffs - src/examples/java/TestFramework.java 265005b9d1a6a053b812c133a4817c3d9e61179e Diff: https://reviews.apache.org/r/37415/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaFramework bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Re: Review Request 36048: Updated authorizer to allow for modularized implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/#review95266 --- Ship it! Thanks for your patience Alexander - I know it has been a long journey - in the end, I think things are in great shape minus some nits I just discovered on my final pass. include/mesos/authorizer/authorizer.proto (line 85) https://reviews.apache.org/r/36048/#comment150212 This iff did suprise me for a moment. if and only if was meant, learned something new :) - thanks for pointing this out Alexander. src/authorizer/authorizer.hpp (line 26) https://reviews.apache.org/r/36048/#comment150219 Missing: `#include stout/nothing.hpp` `#include stout/option.hpp` - will add these while committing. src/authorizer/authorizer.cpp (line 30) https://reviews.apache.org/r/36048/#comment150221 Not used - will remove while committing. src/local/local.cpp (line 41) https://reviews.apache.org/r/36048/#comment150222 `#include stout/nothing.hpp` missing - will add while committing. src/tests/mesos.hpp (line 53) https://reviews.apache.org/r/36048/#comment150224 `#include stout/nothing.hpp` missing - will add while committing. - Till Toenshoff On Aug. 13, 2015, 5:08 p.m., Alexander Rojas wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36048/ --- (Updated Aug. 13, 2015, 5:08 p.m.) Review request for mesos, Adam B, Alexander Rukletsov, Bernd Mathiske, Kapil Arya, Jan Schlicht, and Till Toenshoff. Bugs: MESOS-2946 https://issues.apache.org/jira/browse/MESOS-2946 Repository: mesos Description --- Splits and updates the original declaration of the `Authorizer` into its interface and a default implementation, the `LocalAuthorizer`. Following the pattern of the modularized `Authenticator`, it generates a default constructor which is required when writing a `TYPED_TEST` in a follow up patch. Additionally, an initialize method has been added, needed for passing in the current ACL definitions as provided via flags. Other changes are just updates to allow for compilation. Diffs - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a4a9c6f3afa2441e958e472b264c57b5c5d18eb1 include/mesos/type_utils.hpp 6ff061c94e9d3dbb55bd047840b2012d11e3a0f2 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/authorizer/authorizer.hpp c039d9412780aa199db169b31991bf9f45b07d0f src/authorizer/authorizer.cpp 21e97e315478a4ca9442af83732665f85eb2f8fc src/common/parse.hpp 8d7ddd6819dad98cd96d5aaae8fe57caf1ee7098 src/examples/persistent_volume_framework.cpp c6d6ed337bfca91dc146cb31298cabebdbb13509 src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f src/master/http.cpp 579c009a1b43bc25c294366de29a56dacc734791 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 4e2947003247c3d7bf407d6d709faf2e878418f3 src/master/master.cpp 398203d9367f85340166e66ecc34b9a33dd81048 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd src/tests/mesos.hpp 8b486145dfd2a554689f6e84d76b50508979fea2 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Re: Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- (Updated Aug. 13, 2015, 8:20 p.m.) Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Changes --- addressed additional review comments Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs (updated) - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 37308: Added AppcImageManifest protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37308/#review95331 --- Ship it! Ship It! - Jie Yu On Aug. 13, 2015, 9:24 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37308/ --- (Updated Aug. 13, 2015, 9:24 p.m.) Review request for mesos, Ian Downes and Jie Yu. Bugs: MESOS-3194 https://issues.apache.org/jira/browse/MESOS-3194 Repository: mesos Description --- Derived from /r/34142/. Diffs - include/mesos/mesos.proto 80f56ac2178b24ff19f57c1ace13f567843c7807 Diff: https://reviews.apache.org/r/37308/diff/ Testing --- Tested along with /r/37310/. Thanks, Jiang Yan Xu
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95326 --- src/slave/containerizer/provisioners/docker/token_manager.hpp (line 48) https://reviews.apache.org/r/37427/#comment150235 Move class variables after the method declarations src/slave/containerizer/provisioners/docker/token_manager.hpp (line 82) https://reviews.apache.org/r/37427/#comment150231 Name the parameters, also 4 space indent and one parameter each line. src/slave/containerizer/provisioners/docker/token_manager.hpp (line 93) https://reviews.apache.org/r/37427/#comment150232 Nit: I think we usually comment Forward declarations (although I don't think it's in the style guide) src/slave/containerizer/provisioners/docker/token_manager.cpp (line 120) https://reviews.apache.org/r/37427/#comment150237 Let's not use auto here, spell out the type. src/slave/containerizer/provisioners/docker/token_manager.cpp (line 122) https://reviews.apache.org/r/37427/#comment150236 This should fit in one line src/tests/containerizer/docker_containerizer_tests.cpp (line 86) https://reviews.apache.org/r/37427/#comment150239 Add a TODO around these that we can move this into a common test SSL server utility. src/tests/containerizer/docker_containerizer_tests.cpp (line 2998) https://reviews.apache.org/r/37427/#comment150238 Let's just create a new Docker provisioner test file for this. - Timothy Chen On Aug. 13, 2015, 8:55 p.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 13, 2015, 8:55 p.m.) Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs - src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 37443: Add labels to FrameworkInfo (v2)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37443/ --- (Updated Aug. 13, 2015, 7:49 p.m.) Review request for mesos, Adam B, Connor Doyle, and Niklas Nielsen. Changes --- fixed out of date comment Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We want processes running in the cluster to be able to discover these capabilities; however, we don't want to commit to a fixed set of capabilities or how those capabilities should be represented. Hence, this commit represents this information using freeform key-value pairs, similar to the labels mechanism already in use elsewhere. Jira: MESOS-2841 supercedes #36867 ; addressed outstanding review comments in that review Diffs (updated) - include/mesos/mesos.proto a4a9c6f src/master/http.cpp 579c009 src/master/master.hpp 4e29470 src/tests/fault_tolerance_tests.cpp c63599a src/tests/master_tests.cpp a4703af Diff: https://reviews.apache.org/r/37443/diff/ Testing --- make check Thanks, James DeFelice
Re: Review Request 37415: Fix flaky ExamplesTest.JavaFramework
On Aug. 13, 2015, 4:22 p.m., Till Toenshoff wrote: src/examples/java/TestFramework.java, line 275 https://reviews.apache.org/r/37415/diff/1/?file=1038842#file1038842line275 Why 500 - not 200 or 1000? If I am not mistaken, then using 500 worked fine on your test-machines but chances are that someone comes in with a heavily loaded machine where those 500ms might not do, correct? Let us be honest and let the audience know that this is just a value chosen out of some test runs done - again, if I did not miss something :) I think it would also be a great idea to add a todo here that captures what you wrote within the JIRA issue already; TODO(*): Ideally, we would inspect the status of the driver and its associated tasks via the Java API and wait until their teardown is complete to exit. Greg Mann wrote: You are correct! :-) Thanks, comments addressed. Joseph Wu wrote: Agreed. I don't think a sleep if a good enough solution. (Note: This next part is just supposition.) If you take a look at the comment here: https://github.com/apache/mesos/blob/437e7018e184128c9ec7c4113da09bbf9914721f/src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp#L485 You'll notice that the SchedulerDriver is instantiated as a weak reference. So the GC is relied upon to destroy the object. See: https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/functions.html#weak Have you tried changing it to a normal global reference and adding the explicit cleanup code into the `stop` function? Yea, I'm not crazy about the sleep solution either. I just tried your suggestion, Joseph, putting a DeleteGlobalRef() call in the stop() method, and it didn't work unfortunately. I think that short of an explicit confirmation message upon teardown completion (which doesn't currently exist, and is perhaps not advisable anyway), I don't see a way of resolving this in an elegant fashion. Even if we're sure that the SchedulerDriver is gone before we call System.exit(), the associated Executors and the Slave are doing logging as they shut down, and it's the premature destruction of the mutexes used by glog that is causing the problem. - Greg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/#review95264 --- On Aug. 13, 2015, 4:49 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/ --- (Updated Aug. 13, 2015, 4:49 p.m.) Review request for mesos, Joris Van Remoortere, Joseph Wu, and Till Toenshoff. Bugs: MESOS-830 https://issues.apache.org/jira/browse/MESOS-830 Repository: mesos Description --- Fix flaky ExamplesTest.JavaFramework Diffs - src/examples/java/TestFramework.java 265005b9d1a6a053b812c133a4817c3d9e61179e Diff: https://reviews.apache.org/r/37415/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaFramework bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Re: Review Request 37257: Add SUPPRESS call interface to the scheduler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37257/ --- (Updated 八月 13, 2015, 7:44 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3037 https://issues.apache.org/jira/browse/MESOS-3037 Repository: mesos Description --- This is just part of MESOS-3037, this patch only add the interface of SUPPRESS call. Diffs (updated) - include/mesos/scheduler.hpp cd235a11e63a5df742057be8e27629db4cf9 include/mesos/scheduler/scheduler.proto 89daf8a6b74057ee156b3ad691397e76fcb835b8 src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 src/tests/scheduler_tests.cpp 9d29d1ab0cb2914f4749e05af95c1c21e88908ca Diff: https://reviews.apache.org/r/37257/diff/ Testing --- Thanks, Guangya Liu
Re: Review Request 37310: Added Appc spec validation utility.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37310/ --- (Updated Aug. 13, 2015, 2:33 p.m.) Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu. Changes --- Comments. Repository: mesos Description --- Added Appc spec validation utility. Diffs (updated) - src/Makefile.am 942003149b071a322933e2c085d9122903f69713 src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37310/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Review Request 37467: Fixed scheduler library to send calls in order.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37467/ --- Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-2552 https://issues.apache.org/jira/browse/MESOS-2552 Repository: mesos Description --- Add a queue to guarantee that calls are delivered in order on the master. Ideally, http:post() would allow us to do pipelining on the same connection. Diffs - src/scheduler/scheduler.cpp 3fbe3831cf93c29180bc7e433fd57c6108988316 Diff: https://reviews.apache.org/r/37467/diff/ Testing --- make check Thanks, Vinod Kone
Review Request 37460: Prevent perf test failures from killing the test harness.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37460/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Prevent perf test failures from killing the test harness. Diffs - src/tests/containerizer/perf_tests.cpp 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 Diff: https://reviews.apache.org/r/37460/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37442: Factor out the token extraction rules in prepartion for extending them to cope with multiple versions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37442/#review95371 --- Patch looks great! Reviews applied: [37423, 37424, 37417, 37416, 37442] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 9:59 p.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37442/ --- (Updated Aug. 13, 2015, 9:59 p.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- Factor out the token extraction rules in prepartion for extending them to cope with multiple versions. Diffs - src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 Diff: https://reviews.apache.org/r/37442/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review95374 --- Ship it! Ship It! - Till Toenshoff On Aug. 13, 2015, 6:46 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 13, 2015, 6:46 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac b9ecafd1813d87d604364d354994e4dba386f958 src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/setup.py.in 880e2a6fe4470e50bbfa805967fb983fb8a9987f src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/setup.py.in 9fc9ad2bdeaf1ad3f007e36f24bbe5b10dedc77b src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/setup.py.in 72cb770bda659957c03853c3c99ba6c53f606df8 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/setup.py.in 8ffde68e60e8c968f196e6e0790aeef47d0fb662 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
Re: Review Request 37457: Add --ip_discovery_command to Agent.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37457/#review95377
---
Patch looks great!
Reviews applied: [36978, 36979, 37457]
All tests passed.
- Mesos ReviewBot
On Aug. 14, 2015, 12:21 a.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37457/
---
(Updated Aug. 14, 2015, 12:21 a.m.)
Review request for mesos, Cody Maloney and Timothy Chen.
Repository: mesos
Description
---
Similarly to the Master, we want to enable the Agent too to be initialized
with an IP address that is discovered by an arbitrary script.
This is particularly useful for Mesos deployments on public (or private)
clouds
where DNS resolution is either limited or not feasible.
Jira: MESOS-3154
Diffs
-
src/slave/main.cpp 8008430e98773d8be9ba5ac6385cffb2e351932a
Diff: https://reviews.apache.org/r/37457/diff/
Testing
---
make check
run `./bin/mesos-slave --ip_discovery_command=/.../discover-ip.py ...`
and verified it binds to the expected address:
```
I0813 17:18:24.871472 2046800640 main.cpp:272] Starting Mesos slave
I0813 17:18:24.873610 292802560 slave.cpp:190] Slave started on
1)@192.168.33.1:5051
I0813 17:18:24.873633 292802560 slave.cpp:191] Flags at startup:
--appc_store_dir=/tmp/mesos/store/appc --authenticatee=crammd5
--container_disk_watch_interval=15secs --containerizers=mesos
--default_role=* --disk_watch_interval=1mins --docker=docker
--docker_kill_orphans=true --docker_remove_delay=6hrs
--docker_socket=/var/run/docker.sock --docker_stop_timeout=0ns
--enforce_container_disk_quota=false
--executor_registration_timeout=1mins
--executor_shutdown_grace_period=5secs
--fetcher_cache_dir=/tmp/mesos/fetch --fetcher_cache_size=2GB
--frameworks_home= --gc_delay=1weeks --gc_disk_headroom=0.1
--hadoop_home= --help=false --initialize_driver_logging=true
--ip_discovery_command=~/dev/discovery-ip.py
--isolation=posix/cpu,posix/mem
--launcher_dir=/Users/marco/dev/workspace/mesos/build/src --logbufsecs=0
--logging_level=INFO --master=127.0.0.1:5050 --modules=libraries {
file: /Users/marco/dev/workspace/modules/execute/build/Debug/libexecmod.so
modules {
name: com_mesosphere_mesos_RemoteExecutionAnonymous
}
}
--oversubscribed_resources_interval=15secs --port=5051
--qos_correction_interval_min=0ns --quiet=false --recover=reconnect
--recovery_timeout=15mins --registration_backoff_factor=1secs
--resource_monitoring_interval=1secs
--sandbox_directory=/mnt/mesos/sandbox --strict=true --switch_user=true
--version=false --work_dir=/tmp/slave
I0813 17:18:24.875107 292802560 slave.cpp:354] Slave resources: cpus(*):8;
mem(*):15360; disk(*):470858; ports(*):[31000-32000]
I0813 17:18:24.881863 292802560 slave.cpp:384] Slave hostname: 192.168.33.1
...
```
Thanks,
Marco Massenzio
Review Request 37466: Update perf tests to including testing the supported perf output formats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37466/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Update perf tests to including testing the supported perf output formats. Diffs - src/tests/containerizer/perf_tests.cpp 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 Diff: https://reviews.apache.org/r/37466/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37445: Fix typos in style guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37445/#review95364 --- Patch looks great! Reviews applied: [37445] All tests passed. - Mesos ReviewBot On Aug. 13, 2015, 9:58 p.m., Neil Conway wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37445/ --- (Updated Aug. 13, 2015, 9:58 p.m.) Review request for mesos. Repository: mesos Description --- Fix typos in style guide. Diffs - docs/mesos-c++-style-guide.md 9c1a00c32043fa10038e38bd7cbc561aafcd6ea0 Diff: https://reviews.apache.org/r/37445/diff/ Testing --- Thanks, Neil Conway
Review Request 37465: Revert Revert Updated scheduler library to HTTP.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37465/ --- Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-2552 https://issues.apache.org/jira/browse/MESOS-2552 Repository: mesos Description --- This reverts commit ddbd429708f17caca593b4ba52cb10661066a39e. This is revert of revert. NNFR. Diffs - src/common/http.hpp 6f4f68638a4fe78cddf5e2fdbac94b168d0e6be0 src/scheduler/scheduler.cpp 3fbe3831cf93c29180bc7e433fd57c6108988316 Diff: https://reviews.apache.org/r/37465/diff/ Testing --- make check Thanks, Vinod Kone
Review Request 37464: Revert Revert Deleted old style message handling from the scheduler library.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37464/ --- Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-2552 https://issues.apache.org/jira/browse/MESOS-2552 Repository: mesos Description --- This reverts commit 2926208c4108a8467cba00d6d49b549e7286f5a1. This is a revert of revert. No need for review. Diffs - src/scheduler/scheduler.cpp 3fbe3831cf93c29180bc7e433fd57c6108988316 Diff: https://reviews.apache.org/r/37464/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 37168: MESOS-3063
On 八月 13, 2015, 2:56 a.m., haosdent huang wrote: src/examples/dynamic_reservation_framework.cpp, line 343 https://reviews.apache.org/r/37168/diff/1/?file=1033410#file1033410line343 Does we need add ``` logging::initialize(argv[0], flags, true); ``` here? logging is an internal tools. In this example, i'm trying to avoid using internal classes; so user can build this example from installed Mesos, which is better practice for the user :). On 八月 13, 2015, 2:56 a.m., haosdent huang wrote: src/examples/dynamic_reservation_framework.cpp, line 278 https://reviews.apache.org/r/37168/diff/1/?file=1033410#file1033410line278 Could we change the style here to make it looks better? :-) sure, I copied this from other example; anyway, I'll check the style :). - Klaus --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37168/#review95227 --- On 八月 6, 2015, 8:03 a.m., Klaus Ma wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37168/ --- (Updated 八月 6, 2015, 8:03 a.m.) Review request for mesos. Bugs: MESOS-3063 https://issues.apache.org/jira/browse/MESOS-3063 Repository: mesos Description --- MESOS-3063 (Add an example framework using dynamic reservation) Diffs - src/examples/dynamic_reservation_executor.cpp PRE-CREATION src/examples/dynamic_reservation_framework.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37168/diff/ Testing --- Test Steps Build the example and run in local Mesos cluster Example Output I0806 15:19:33.955976 17033 sched.cpp:640] Framework registered with 20150805-204038-16842879-5050-14289-0015 Registered! Received offer 20150805-204038-16842879-5050-14289-O161 with mem(*):2862; disk(*):460240; ports(*):[31000-32000]; cpus(*):4 Reserve resources cpus(test, dynamic-reservation-framework-cpp):1; mem(test, dynamic-reservation-framework-cpp):128 Received offer 20150805-204038-16842879-5050-14289-O162 with mem(*):2734; disk(*):460240; ports(*):[31000-32000]; cpus(*):3; cpus(test, dynamic- reservation-framework-cpp):1; mem(test, dynamic- reservation-framework-cpp):128 Launching task 0 using offer 20150805-204038-16842879-5050-14289-O162 Task 0 is in state TASK_FINISHED Received offer 20150805-204038-16842879-5050-14289-O163 with mem(*):2734; disk(*):460240; ports(*):[31000-32000]; cpus(*):3; cpus(test, dynamic- reservation-framework-cpp):1; mem(test, dynamic-reservation-framework-cpp):128 Launching task 1 using offer 20150805-204038-16842879-5050-14289-O163 Task 1 is in state TASK_FINISHED Received offer 20150805-204038-16842879-5050-14289-O164 with mem(*):2734; disk(*):460240; ports(*):[31000-32000]; cpus(*):3; cpus(test, dynamic- reservation-framework-cpp):1; mem(test, dynamic-reservation-framework-cpp):128 Launching task 2 using offer 20150805-204038-16842879-5050-14289-O164 Task 2 is in state TASK_FINISHED Received offer 20150805-204038-16842879-5050-14289-O165 with mem(*):2734; disk(*):460240; ports(*):[31000-32000]; cpus(*):3; cpus(test, dynamic- reservation-framework-cpp):1; mem(test, dynamic-reservation-framework-cpp):128 Launching task 3 using offer 20150805-204038-16842879-5050-14289-O165 Task 3 is in state TASK_FINISHED Received offer 20150805-204038-16842879-5050-14289-O166 with mem(*):2734; disk(*):460240; ports(*):[31000-32000]; cpus(*):3; cpus(test, dynamic- reservation-framework-cpp):1; mem(test, dynamic-reservation-framework-cpp):128 Launching task 4 using offer 20150805-204038-16842879-5050-14289-O166 Task 4 is in state TASK_FINISHED Received offer 20150805-204038-16842879-5050-14289-O167 with mem(*):2734; disk(*):460240; ports(*):[31000-32000]; cpus(*):3; cpus(test, dynamic- reservation-framework-cpp):1; mem(test, dynamic-reservation-framework-cpp):128 Unreserve resources cpus(test, dynamic-reservation-framework-cpp):1; mem(test, dynamic-reservation-framework-cpp):128 Received offer 20150805-204038-16842879-5050-14289-O168 with mem(*):2862; disk(*):460240; ports(*):[31000-32000]; cpus(*):4 I0806 15:19:49.515024 17031 sched.cpp:1748] Asked to stop the driver I0806 15:19:49.515115 17031 sched.cpp:1032] Stopping framework '20150805-204038-16842879-5050-14289-0015' I0806 15:19:49.515630 17027 sched.cpp:1748] Asked to stop the driver 2015-08-06 15:19:49,516:17027(0x7f23f610d700):ZOO_INFO@zookeeper_close@2505: Closing zookeeper sessionId=0x34da910aab20058 to [9.111.159.76:2181] Thanks, Klaus Ma
Re: Review Request 37467: Fixed scheduler library to send calls in order.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37467/#review95370 --- Ship it! LGTM ! Would we do a test run on Jenkins before committing this in just to be sure since the last time review bot did seem to show a false positive on r37328 ? src/scheduler/scheduler.cpp (line 382) https://reviews.apache.org/r/37467/#comment150319 Micro nit : How about just CHECK(!calls.empty()) ? - Anand Mazumdar On Aug. 14, 2015, 1:59 a.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37467/ --- (Updated Aug. 14, 2015, 1:59 a.m.) Review request for mesos, Anand Mazumdar and Ben Mahler. Bugs: MESOS-2552 https://issues.apache.org/jira/browse/MESOS-2552 Repository: mesos Description --- Add a queue to guarantee that calls are delivered in order on the master. Ideally, http:post() would allow us to do pipelining on the same connection. Diffs - src/scheduler/scheduler.cpp 3fbe3831cf93c29180bc7e433fd57c6108988316 Diff: https://reviews.apache.org/r/37467/diff/ Testing --- make check Thanks, Vinod Kone
Review Request 37462: Add support for version detection and parsing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37462/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Add support for version detection and parsing. Diffs - src/linux/perf.cpp cb1a13d0b1754a50f0121bfda522056ff8c3e3c8 Diff: https://reviews.apache.org/r/37462/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37468: Removed allocation types to mesos::master namespace
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37468/#review95378 --- Bad patch! Reviews applied: [37468] Failed command: ./support/apply-review.sh -n -r 37468 Error: 2015-08-14 04:22:30 URL:https://reviews.apache.org/r/37468/diff/raw/ [23334/23334] - 37468.patch [1] Traceback (most recent call last): File ./support/jsonurl.py, line 25, in module print data UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in position 3: ordinal not in range(128) Successfully applied: Removed allocation types to mesos::master namespace The allocation-related types was moved to mesos::master namespace. MESOS-2516 Review: https://reviews.apache.org/r/37468 fatal: empty ident name (for guilherme@gmail.com) not allowed Failed to commit patch - Mesos ReviewBot On Aug. 14, 2015, 2:01 a.m., José Guilherme Vanz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37468/ --- (Updated Aug. 14, 2015, 2:01 a.m.) Review request for mesos. Bugs: MESOS-2516 https://issues.apache.org/jira/browse/MESOS-2516 Repository: mesos Description --- The allocation-related types was moved to mesos::master namespace. MESOS-2516 Diffs - docs/allocation-module.md 4ba5ba8c27cccebf0e5fc5064fdc1d6af05d38f0 include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a include/mesos/module/allocator.hpp 376eb4860322582f911d7a07253b79b1c9aa9292 src/examples/test_allocator_module.cpp 9679fd9f9a998834bc5329efa07d46a0403ab3f6 src/local/local.hpp 6bb25f16982d7be067b5f1307d5eca7745820e81 src/local/local.cpp 4d98bf23705027f3ba0cbb571289f21b288fe7db src/master/allocator/allocator.cpp 42214048e83cb50bdb669d79fa49401c1716de87 src/master/allocator/mesos/allocator.hpp aa55755a9c3250579e9366bdbc17a2449e95d659 src/master/allocator/mesos/hierarchical.hpp e278139f856888d6c6f538f7c0f664087e97f629 src/master/allocator/sorter/drf/sorter.hpp f66ade06c6a5b4bf816839477cec2d18036c7b1a src/master/allocator/sorter/drf/sorter.cpp bfc273493419fe46a4d907f4f7fa282cff71b800 src/master/allocator/sorter/sorter.hpp 536a7ad9a2d661bc8aa352d2e0ae41115b1e8a04 src/master/main.cpp e024a2e4beeb58438accd07e1b87ea239fb1d88b src/master/master.hpp 1e12f650796acafcee83cf9bd0b85f57d359333c src/master/master.cpp c5e6c6f3304060d4c92d52851951f10bc432500e src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 src/tests/fault_tolerance_tests.cpp c63599ad36d883d25a23f5bab1929c4e2dad4960 src/tests/hierarchical_allocator_tests.cpp 9748ca0b3fee25dcec51c64d8ba84dbd4aaf src/tests/master_allocator_tests.cpp 89331965553505f6b7eebf39ad27d943df816a24 src/tests/master_authorization_tests.cpp f3f0cc81ab958e5b9a2bc458f5c38b3e12202514 src/tests/master_tests.cpp a4703afc331244a9e959c48652342f31ef787288 src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c src/tests/mesos.cpp 9bff4a66604a67d9add5a05247548e0162f3cda7 src/tests/partition_tests.cpp b7030adcfb1bfb128040aff20f12461412b5382c src/tests/rate_limiting_tests.cpp f3aeddee00c7bb7905092aa8a760603468063126 src/tests/reservation_tests.cpp aeee36752573e3f401d3dca7d2d69c90d0e8bd6b src/tests/resource_offers_tests.cpp 882a9ff4d09aace486182828bf43b643b0d0c519 src/tests/scheduler_driver_tests.cpp 4963f5d672737e4bb8f173f0cbd6c504a5d91b71 src/tests/scheduler_tests.cpp 7842f27673cc0fc11864916dda4ca4bbb66d9dbd src/tests/slave_recovery_tests.cpp e1392a2235ff51dac7a3cb7cd3e8edf8406864fc src/tests/sorter_tests.cpp 0db9a046007c1e89edb2af1e5d63cbc40934e2af Diff: https://reviews.apache.org/r/37468/diff/ Testing --- Thanks, José Guilherme Vanz
Re: Review Request 37415: Fix flaky ExamplesTest.JavaFramework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/ --- (Updated Aug. 14, 2015, 4:56 a.m.) Review request for mesos, Joris Van Remoortere, Joseph Wu, and Till Toenshoff. Bugs: MESOS-830 https://issues.apache.org/jira/browse/MESOS-830 Repository: mesos Description --- Fix flaky ExamplesTest.JavaFramework Diffs - src/examples/java/TestFramework.java 265005b9d1a6a053b812c133a4817c3d9e61179e Diff: https://reviews.apache.org/r/37415/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaFramework bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Re: Review Request 36847: Added HTTP Delete Method.
On Aug. 6, 2015, 9:56 a.m., Alexander Rojas wrote: 3rdparty/libprocess/include/process/http.hpp, line 754 https://reviews.apache.org/r/36847/diff/7/?file=1028524#file1028524line754 I'm rather late to the party, but AFAIK Mesos appreciates consistency over doing some things right and some wrong. The Mesos way would be to add the method in a consistent way and then do a sweep change for all the other http calls. That being said, the way I have seen other http libraries getting around the keyword issue is by using an underscore at some point. Marco Massenzio wrote: To paraphrase a well-known maxim... I'd rather be sometimes right, than consistently wrong :) (and, as per my usual, adding here my favorite [demotivator](http://despair.com/collections/demotivators/products/consistency) :D More seriously, good point about underscore, the problem here is that wherever you put it, you end up violating a style rule and/or doing something ugly - a real conumdrum! Bernd Mathiske wrote: There is NO consistent way to add the method and it MUST be added. Therefore I prefer a way that is, halas, inconsistent, but at least not wrong when regarded in isolation in any additional ways. Then, as an additional issue/patch we may/should choose to bring the other calls into line. (An underscore does not achieve any of these objectives.) Bernd Mathiske wrote: All that said, I propose that we choose this name for the method: requestDelete. Reasons: - Avoids keyword problem. - Is short enough to be applied to all other HTTP requests: requestGet, requestAuth, etc.; clarifies what these do. - Reads as a verb. - Avoids send as part of the name, which is kinda wrong, since we do more than just sending. We also wait for the response via a future. Marco Massenzio wrote: +1 I added MESOS-3256 to track the renaming effort. - Joerg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/#review94388 --- On Aug. 4, 2015, 10:57 a.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/ --- (Updated Aug. 4, 2015, 10:57 a.m.) Review request for mesos, Alexander Rukletsov and Bernd Mathiske. Bugs: MESOS-3152 https://issues.apache.org/jira/browse/MESOS-3152 Repository: mesos Description --- see summary. Diffs - 3rdparty/libprocess/include/process/http.hpp b8d930058aa3ae9d21ae0f7b420d49f2448f84b1 3rdparty/libprocess/src/http.cpp 4dcbd74b894b483c4d166c23fac55ac8dba75166 3rdparty/libprocess/src/tests/http_tests.cpp ecbcbd552ac834659860627c82628ed38e6139b3 Diff: https://reviews.apache.org/r/36847/diff/ Testing --- make check Thanks, Joerg Schad
Re: Review Request 37427: Docker registry: adding TokenManager.
On Aug. 13, 2015, 6:24 a.m., Lily Chen wrote:
src/slave/containerizer/provisioners/docker/token_manager.cpp, line 252
https://reviews.apache.org/r/37427/diff/2/?file=1039212#file1039212line252
space between () {}, you could also probably just do this in the header
file
The reason we dont add definition in header file is that compiler might add the
definition in each compilation unit the header file is included. This will
bloat the binary (and other issues that comes along with it).
- Jojy
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37427/#review95238
---
On Aug. 13, 2015, 8:29 a.m., Jojy Varghese wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37427/
---
(Updated Aug. 13, 2015, 8:29 a.m.)
Review request for mesos, Lily Chen and Timothy Chen.
Repository: mesos
Description
---
Changes:
- Added Token implementation (RFC 7519).
- Added TokenManager implementation. This component keeps a cache of tokens
requested for any future requests.
Diffs
-
src/Makefile.am 111aed92820689b12ee4073269ce34db7be30960
src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION
src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION
src/tests/containerizer/docker_containerizer_tests.cpp
c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea
Diff: https://reviews.apache.org/r/37427/diff/
Testing
---
make check.
Thanks,
Jojy Varghese
Re: Review Request 37415: Fix flaky ExamplesTest.JavaFramework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/#review95264 --- src/examples/java/TestFramework.java (line 275) https://reviews.apache.org/r/37415/#comment150115 Why 500 - not 200 or 1000? If I am not mistaken, then using 500 worked fine on your test-machines but chances are that someone comes in with a heavily loaded machine where those 500ms might not do, correct? Let us be honest and let the audience know that this is just a value chosen out of some test runs done - again, if I did not miss something :) I think it would also be a great idea to add a todo here that captures what you wrote within the JIRA issue already; TODO(*): Ideally, we would inspect the status of the driver and its associated tasks via the Java API and wait until their teardown is complete to exit. - Till Toenshoff On Aug. 12, 2015, 9:07 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37415/ --- (Updated Aug. 12, 2015, 9:07 p.m.) Review request for mesos, Joris Van Remoortere and Till Toenshoff. Bugs: MESOS-830 https://issues.apache.org/jira/browse/MESOS-830 Repository: mesos Description --- Fix flaky ExamplesTest.JavaFramework Diffs - src/examples/java/TestFramework.java 265005b9d1a6a053b812c133a4817c3d9e61179e Diff: https://reviews.apache.org/r/37415/diff/ Testing --- This test may fail only infrequently, and the bug is made worse by verbose logging, so to verify correctness (or reproduce the bug before patching) try: GLOG_v=2 GTEST_FILTER=ExamplesTest.JavaFramework bin/mesos-tests.sh --verbose --gtest_repeat=100 2/dev/null | grep \[ Thanks, Greg Mann
Review Request 37471: Fix EventCallFramework test to not ignore heartbeat events
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37471/ --- Review request for mesos and Vinod Kone. Repository: mesos Description --- See summary and related CR Diffs - src/examples/event_call_framework.cpp 019e5c1f19a9e455b73ea9916b4f4270dad6d3da Diff: https://reviews.apache.org/r/37471/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 36819: Use setup.py in python cli package.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/#review95262 --- Ship it! Today we are cutting a 0.24 RC (according to @Vinod's recent email) - it would be great if we could have this fix in, which solves an issue with the Python installers. Can anyone please do anything about this one? Thanks! - Marco Massenzio On Aug. 7, 2015, 3:08 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36819/ --- (Updated Aug. 7, 2015, 3:08 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Marco Massenzio. Bugs: MESOS-3149 https://issues.apache.org/jira/browse/MESOS-3149 Repository: mesos Description --- Use setup.py in python cli package. Diffs - Makefile.am f8e958d376efa55ace7c5727a31b8e747403641e bin/mesos.sh.in 5cbeac4330a9f45fc6d54b8c2d383f48e4098f95 configure.ac 0794279dd2e23b5b593e7e388bd6d04e17c746a6 src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/cli/python/mesos/__init__.py 028b0d27fb193bac96f2a6a3201ee4cc8fd369ef src/cli/python/mesos/cli.py src/cli/python/mesos/futures.py src/cli/python/mesos/http.py src/python/cli/src/mesos/__init__.py PRE-CREATION src/python/interface/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/native/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 src/python/protocol/src/mesos/__init__.py f48ad10528712b2b8960f1863d156b88ed1ce311 Diff: https://reviews.apache.org/r/36819/diff/ Testing --- make check # Test in CentOS 6.6, OS X 10.10, Ubuntu 14.04 ## test steps: 1. sudo make install 2. export PYTHONPATH 3. python -c 'import mesos; from mesos import cli, http, futures' Thanks, haosdent huang
