Re: Review Request 37159: Delegated the container root filesystem provisioning to the filesystem isolator.
On Aug. 6, 2015, 5:38 a.m., Timothy Chen wrote: src/slave/containerizer/mesos/containerizer.cpp, line 661 https://reviews.apache.org/r/37159/diff/1/?file=1033221#file1033221line661 Where does the provisioner go? I see it's removed here but I don't see where else we're calling provisioner to populate the rootfs? Yes. It'll be moved the the linux filesystem isolator. I am working on that right now and will submit that shortly. - Jie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37159/#review94357 --- On Aug. 6, 2015, 4:20 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37159/ --- (Updated Aug. 6, 2015, 4:20 a.m.) Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu. Repository: mesos Description --- Delegated the container root filesystem provisioning to the filesystem isolator. The motivation is that: currently, container rootfs provisioning is done by the containerizer while preparing the rest of the filesystem (i.e., bind mount volumes) is done by the filesystem isolator. It'll be more natural if all filesystem related preparation is done by one component. Another reason is that we are going to provision images specified in the volumes as well. So provisining rootfs in filesystem isolator makes it more easy to implement. Turns out that this change simplify the containerizer quite a bit. Diffs - include/mesos/slave/isolator.hpp 22f1e3686f50c3b9290561aa7e5073e24a702824 include/mesos/slave/isolator.proto 3d9222be5e9bd9e9f665fb2e57db6b7e925c8fbd src/slave/containerizer/isolator.hpp 710c584f95d60c1931b40ca041409aa819a06cba src/slave/containerizer/isolator.cpp ed610f9f8fe328fb3b73f620858dc632725e51f8 src/slave/containerizer/isolators/cgroups/cpushare.hpp 6b980f26fe8bb51dd989a0578337bc13dbd087ad src/slave/containerizer/isolators/cgroups/cpushare.cpp 907d7e78bfb591197e150ac053bb857d15a1e6dc src/slave/containerizer/isolators/cgroups/mem.hpp e831878ab47b8455a4831ebe305373130b194a40 src/slave/containerizer/isolators/cgroups/mem.cpp e343d0b9751b46bc5a4a8ccd32c0b2745e110e6b src/slave/containerizer/isolators/cgroups/perf_event.hpp 73f245bc9166e1f7550466ddd97113c63ce44e73 src/slave/containerizer/isolators/cgroups/perf_event.cpp 0e421cb6ad3e04b71746033ab15d0f1695fcd5e7 src/slave/containerizer/isolators/filesystem/posix.hpp 4c7a6f2b7530c88c34d533dba9593006ad5284b2 src/slave/containerizer/isolators/filesystem/posix.cpp 4861ee13fc34eef03d28f26d57a7d11aebed81a6 src/slave/containerizer/isolators/filesystem/shared.hpp 45e4ba09993e7b77f2df45a5c86bc00fa2d83977 src/slave/containerizer/isolators/filesystem/shared.cpp b30ab3fd0013045a2843fe1e8843cc120ce180c6 src/slave/containerizer/isolators/namespaces/pid.hpp 858e43683c88ac62abfc74ff28e8073895cf6f64 src/slave/containerizer/isolators/namespaces/pid.cpp 8e643f4afae8c24cd4d68aa349148b6f402b286b src/slave/containerizer/isolators/network/port_mapping.hpp 2599c9800e3edf12ec883b31c280324b24b195c5 src/slave/containerizer/isolators/network/port_mapping.cpp 8244c345b84108af7fa18d20e71401d6e1a0aeb0 src/slave/containerizer/isolators/posix.hpp ef19749c0d5b795fee54d67cfc0d983b0f7084ec src/slave/containerizer/isolators/posix/disk.hpp 9fa584ff4a2f3c90c4d81aecefbcba57fa2294ad src/slave/containerizer/isolators/posix/disk.cpp 6dda77bad7ab135b6d339a80b98a291ea7120e95 src/slave/containerizer/mesos/containerizer.hpp 8851d30af56b4f9fb95450ac1f42ab550e3df9ff src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/tests/containerizer/isolator.hpp fa2fc9bd6a59de130870f1ab199e05e85579d8dd src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/mesos_containerizer_tests.cpp 213fa4b0b9c50eba941ef6b52497eb32d539 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/37159/diff/ Testing --- sudo make check Thanks, Jie Yu
Re: Review Request 37190: WIP: Added POST support for /maintenance endpoint. Performed initial verification of the input.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37190/#review94446 --- Bad patch! Reviews applied: [36321, 36571, 37052, 37190] Failed command: ./support/apply-review.sh -n -r 37190 Error: 2015-08-06 21:16:36 URL:https://reviews.apache.org/r/37190/diff/raw/ [7339/7339] - 37190.patch [1] 37190.patch:39: trailing whitespace. error: patch failed: src/master/maintenance.hpp:32 error: src/master/maintenance.hpp: patch does not apply error: patch failed: src/master/maintenance.cpp:59 error: src/master/maintenance.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On Aug. 6, 2015, 6:27 p.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37190/ --- (Updated Aug. 6, 2015, 6:27 p.m.) Review request for mesos, Benjamin Hindman, Joris Van Remoortere, and Joseph Wu. Bugs: MESOS-2067 https://issues.apache.org/jira/browse/MESOS-2067 Repository: mesos Description --- See summary. Diffs - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 30a2550c606ca528ec5b69fc9efedd698d67c5f2 src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/37190/diff/ Testing --- Manual testing of the endpoint. Thanks, Artem Harutyunyan
Review Request 37196: Add Docker Image type to Container Image protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37196/ --- Review request for mesos and Timothy Chen. Bugs: MESOS-2851 https://issues.apache.org/jira/browse/MESOS-2851 Repository: mesos Description --- Add Docker Image type to Container Image protobuf. Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba Diff: https://reviews.apache.org/r/37196/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 6, 2015, 9:03 p.m.) Review request for mesos. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing (updated) --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 36979: Updating all references to os::shell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/#review94452 --- Patch looks great! Reviews applied: [36978, 36979] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 6:24 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/ --- (Updated Aug. 6, 2015, 6:24 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Updating all references to os::shell For more details see MESOS-3142. Diffs - src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/slave/containerizer/isolators/network/port_mapping.cpp 8244c345b84108af7fa18d20e71401d6e1a0aeb0 src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/36979/diff/ Testing --- make check *Note*: this patch fixes breakages introduce by the refactoring in: https://reviews.apache.org/r/36978 Thanks, Marco Massenzio
Re: Review Request 37165: Introduced v1 API.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37165/#review94436
---
Ship it!
LGTM. Please rebase and let the review bot give an OK before committing.
src/Makefile.am (lines 237 - 245)
https://reviews.apache.org/r/37165/#comment149037
Add a comment for why you did -mv?
src/Makefile.am (lines 467 - 472)
https://reviews.apache.org/r/37165/#comment149036
reorder alphabetically.
src/Makefile.am (lines 852 - 855)
https://reviews.apache.org/r/37165/#comment149038
alphabetical.
src/Makefile.am (lines 1158 - 1161)
https://reviews.apache.org/r/37165/#comment149039
alphabetical.
src/examples/event_call_framework.cpp (line 25)
https://reviews.apache.org/r/37165/#comment149041
use v1/resources.hpp
src/examples/event_call_framework.cpp (lines 239 - 242)
https://reviews.apache.org/r/37165/#comment149042
use v1 resources.
src/internal/devolve.hpp (line 37)
https://reviews.apache.org/r/37165/#comment149043
lets kill unused ones.
src/internal/evolve.hpp (line 39)
https://reviews.apache.org/r/37165/#comment149044
kill unused ones.
src/master/master.hpp (line 1346)
https://reviews.apache.org/r/37165/#comment149045
Add a comment to be more clear on what is happening here?
src/scheduler/scheduler.cpp (line 238)
https://reviews.apache.org/r/37165/#comment149048
Can you add a comment somewhere that the master needs to write a v1
MasterInfo into ZK?
- Vinod Kone
On Aug. 6, 2015, 6:31 a.m., Benjamin Hindman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37165/
---
(Updated Aug. 6, 2015, 6:31 a.m.)
Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
Repository: mesos
Description
---
Perhaps the right thing is to move internal/{d|e}volve.hpp|cpp to
v1/{d|e}volve.hpp|cpp?
Note that Anand can fix up src/scheduler/scheduler.cpp to just use the HTTP
API once it's finished and can kill all authenticating code and 'install',
'send', 'evolve', 'devolve' code and update src/tests/scheduler_tests.cpp as
well.
Diffs
-
include/mesos/scheduler.hpp cd235a11e63a5df742057be8e27629db4cf9
include/mesos/v1/attributes.hpp PRE-CREATION
include/mesos/v1/mesos.hpp PRE-CREATION
include/mesos/v1/mesos.proto PRE-CREATION
include/mesos/v1/resources.hpp PRE-CREATION
include/mesos/v1/scheduler.hpp PRE-CREATION
include/mesos/v1/scheduler/scheduler.hpp PRE-CREATION
include/mesos/v1/scheduler/scheduler.proto PRE-CREATION
include/mesos/v1/values.hpp PRE-CREATION
src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec
src/common/protobuf_utils.hpp a4708ed286ef237f17d9fd7813be2f6e7218b42a
src/common/protobuf_utils.cpp 3cb684598d0492a2f57b46fabcf13565ff42f27a
src/examples/event_call_framework.cpp
8054068fa746f8635f1133ceac530e04eaa0e1d7
src/internal/devolve.hpp PRE-CREATION
src/internal/devolve.cpp PRE-CREATION
src/internal/evolve.hpp PRE-CREATION
src/internal/evolve.cpp PRE-CREATION
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/scheduler/scheduler.cpp 97fa2c063db506dec69ff1edd851c96b4e1219a4
src/tests/mesos.hpp 93d87c78e5665b8104dbbc3d1e8c92e515cc67ab
src/tests/scheduler_driver_tests.cpp PRE-CREATION
src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71
src/tests/slave_tests.cpp cb5a01ed771e66d75091ca33523dbe673e16a86e
src/v1/attributes.cpp PRE-CREATION
src/v1/mesos.cpp PRE-CREATION
src/v1/resources.cpp PRE-CREATION
src/v1/values.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37165/diff/
Testing
---
make check
Thanks,
Benjamin Hindman
Review Request 37200: Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/ --- Review request for mesos and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers. Diffs - src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37200/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 36979: Updating all references to os::shell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/#review94453 --- src/hdfs/hdfs.hpp (line 110) https://reviews.apache.org/r/36979/#comment149063 Why not use the following? return !out.get().empty(); - Guangya Liu On 八月 6, 2015, 6:24 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/ --- (Updated 八月 6, 2015, 6:24 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Updating all references to os::shell For more details see MESOS-3142. Diffs - src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/slave/containerizer/isolators/network/port_mapping.cpp 8244c345b84108af7fa18d20e71401d6e1a0aeb0 src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/36979/diff/ Testing --- make check *Note*: this patch fixes breakages introduce by the refactoring in: https://reviews.apache.org/r/36978 Thanks, Marco Massenzio
Review Request 37199: Added store interface and moved store implementation to LocalStore subclass.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37199/ --- Review request for mesos and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Added store interface and moved store implementation to LocalStore subclass. Diffs - src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37199/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/#review94448 --- src/slave/containerizer/provisioners/docker/store.hpp (line 123) https://reviews.apache.org/r/37197/#comment149056 End comment with period. And why not use ImageName as keys, or basically everywhere else in the provisioner? src/slave/containerizer/provisioners/docker/store.hpp (line 125) https://reviews.apache.org/r/37197/#comment149057 What hash is this? end comment with period too. src/slave/containerizer/provisioners/docker/store.cpp (line 264) https://reviews.apache.org/r/37197/#comment149058 Non-zero exit for tar subprocess for uri ' + uri + ', error: + stringify(status.get()) - Timothy Chen On Aug. 6, 2015, 8:34 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 6, 2015, 8:34 p.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 36978: MESOS-3142 Refactoring os::shell - patch 1/2
On Aug. 5, 2015, 2:54 a.m., Artem Harutyunyan wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, line 50 https://reviews.apache.org/r/36978/diff/1/?file=1026032#file1026032line50 should the variable be called `_cmd`? Marco Massenzio wrote: note this is neither a private member, not a constructor arg - it's a temp var: AFAIK there are no guidelines (apart from the obvious naming it sensibly). Renamed `command` Artem Harutyunyan wrote: It does not really matter since you renamed, but there is a guideline for function arguments: ``` We prepend constructor and function arguments with a leading underscore to avoid ambiguity and / or shadowing: ``` ah! It *does* matter: I know something that I didn't, but should have :) Thanks! - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36978/#review94162 --- On Aug. 6, 2015, 6:20 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36978/ --- (Updated Aug. 6, 2015, 6:20 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Refactoring os::shell. See MESOS-3142 for more details. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp ab767ccc4553cc5f61e4fe1b67110a9b5b32f2bc 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 53f14e1869ed7a6e1ac7cc8a82c558ed77907dc9 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 4b7a7bafe1c64183d021b39cf12523250491f0ee 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 2556bd428cc8990659e30e804b9c96c1659ef4a1 Diff: https://reviews.apache.org/r/36978/diff/ Testing --- make check *Note*: this patch by itself breaks mesos - this only fixes the `stout` part: see also https://reviews.apache.org/r/36979/ Thanks, Marco Massenzio
Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- Review request for mesos and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37179: Enhance the bootstrap script
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37179/#review94385 --- Ship it! Ship It! - Yong Qiao Wang On Aug. 6, 2015, 9:28 a.m., Zhiwei Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37179/ --- (Updated Aug. 6, 2015, 9:28 a.m.) Review request for mesos and Yong Qiao Wang. Bugs: MESOS-3218 https://issues.apache.org/jira/browse/MESOS-3218 Repository: mesos Description --- Force override the link file when it is invalid. Diffs - bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d Diff: https://reviews.apache.org/r/37179/diff/ Testing --- Thanks, Zhiwei Chen
Re: Review Request 37108: Remove unused failover functionality in scheduler library
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37108/#review94387 --- Ship it! Ship It! - Alexander Rojas On Aug. 5, 2015, 3:19 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37108/ --- (Updated Aug. 5, 2015, 3:19 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- We now leave it upto the client to set the force option when making the subscribe call. This existed in the pre-Call era as force(failover) was not exposed to the scheduler. ( was part of FrameworkReRegisterMessage not exposed to the scheduler ) We anyways were not using this as of now. Diffs - src/scheduler/scheduler.cpp 97fa2c063db506dec69ff1edd851c96b4e1219a4 Diff: https://reviews.apache.org/r/37108/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37075: Protobuf definitions instructing the fetcher cache about checksums and their validation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37075/#review94254 --- include/mesos/mesos.proto (line 304) https://reviews.apache.org/r/37075/#comment148801 s/not/no/ - Till Toenshoff On Aug. 5, 2015, 11:37 a.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37075/ --- (Updated Aug. 5, 2015, 11:37 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy Chen. Bugs: MESOS-3208 https://issues.apache.org/jira/browse/MESOS-3208 Repository: mesos Description --- Protobuf definitions instructing the fetcher cache about checksums and their validation. First in a series of patches implementing phase 1 of MESOS-2073 (see comments in the ticket). There are references to docs in this patch, but updating docs will be in the last patch. Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba Diff: https://reviews.apache.org/r/37075/diff/ Testing --- Thanks, Bernd Mathiske
Review Request 37168: MESOS-3063
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37168/ --- 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 37065: Fixed MemIsolatorTest failure on OSX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/#review94376 --- Ship it! Thanks for the exploration, just a few comments on comments. src/tests/containerizer/memory_test_helper.cpp (line 76) https://reviews.apache.org/r/37065/#comment148954 This doesn't sound accurate to me: `to make sure allocated memory is mapped`. I think maybe something like `to make sure the mapped memory is locked` would be more accurate? src/tests/containerizer/memory_test_helper.cpp (lines 80 - 82) https://reviews.apache.org/r/37065/#comment148953 `mlockall` is not what we want ideally right? Can we mention that it's not ideal but that it exists temporarily to address `MESOS-3197`? src/tests/containerizer/memory_test_helper.cpp (lines 97 - 98) https://reviews.apache.org/r/37065/#comment148952 Could we add some additional comment here to describe why we need to do this? Something like: ``` Since `mlockall` is left unimplemented in OS X, we need to use `mlock` instead. ``` - Michael Park On Aug. 6, 2015, 5:54 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/ --- (Updated Aug. 6, 2015, 5:54 a.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3197 https://issues.apache.org/jira/browse/MESOS-3197 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/memory_test_helper.cpp 5e40b747f4266e7532baf8fd02ea5db0955124d2 Diff: https://reviews.apache.org/r/37065/diff/ Testing --- make check Thanks, Artem Harutyunyan
Re: Review Request 37109: Removed ability to mutate user from scheduler library
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37109/#review94389 --- Ship it! - Alexander Rojas On Aug. 5, 2015, 3:23 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37109/ --- (Updated Aug. 5, 2015, 3:23 a.m.) Review request for mesos and Vinod Kone. Repository: mesos Description --- The scheduler used to mutate the user field to the os::user if it was not set. This patch removes this functionality and leaves it upto the client to provide a user as part of SUBSCRIBE. Diffs - src/examples/event_call_framework.cpp 8054068fa746f8635f1133ceac530e04eaa0e1d7 src/scheduler/scheduler.cpp 97fa2c063db506dec69ff1edd851c96b4e1219a4 Diff: https://reviews.apache.org/r/37109/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37097: Fix 'Accept-Encoding' parsing
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37097/#review94386 --- 3rdparty/libprocess/src/http.cpp (lines 133 - 138) https://reviews.apache.org/r/37097/#comment148962 Can you leave the RFC number here? I find it is a good practice even if it is repeatedly used. 3rdparty/libprocess/src/http.cpp (lines 134 - 138) https://reviews.apache.org/r/37097/#comment148963 This paragraph doesn't seem to come from the RFC. Can you move it before the headline or after the contents from the rfc? 3rdparty/libprocess/src/http.cpp (line 159) https://reviews.apache.org/r/37097/#comment148964 I don't think we use `cout` in libprocess but `VLOG` (check `decode` in this file). - Alexander Rojas On Aug. 5, 2015, 9 p.m., Isabel Jimenez wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37097/ --- (Updated Aug. 5, 2015, 9 p.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- Currently parsing only compares the begining of the header making 'gzipbug' match with candidate 'gzip' Diffs - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/encoder_tests.cpp 0032137 Diff: https://reviews.apache.org/r/37097/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36847: Added HTTP Delete Method.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/#review94388 --- 3rdparty/libprocess/include/process/http.hpp (line 754) https://reviews.apache.org/r/36847/#comment148965 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. - Alexander Rojas On Aug. 4, 2015, 12:57 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36847/ --- (Updated Aug. 4, 2015, 12:57 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 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 37168: MESOS-3063
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37168/#review94390 --- Patch looks great! Reviews applied: [37168] All tests passed. - Mesos ReviewBot On Aug. 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 Aug. 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 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/#review94362 --- Patch looks great! Reviews applied: [37162] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 5:12 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/ --- (Updated Aug. 6, 2015, 5:12 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs - configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 Diff: https://reviews.apache.org/r/37162/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 35874: Added template parameters and constructors to hashset to match the signature of hashmap
On June 30, 2015, 11:55 p.m., Joris Van Remoortere wrote:
Hi Alex,
I just wanted to follow up on our conversation.
As discussed, I think it makes sense to refactor to use
`std::unordered_set` first.
After that, I think you can simplify this code significantly:
```
hashset(const std::setElem set) : std::unordered_setElem, Hash,
Equal(set.begin(), set.end(), std::ceil(list.size() / max_load_factor()))
{}
hashset(std::initializer_listElem list) : std::unordered_setElem, Hash,
Equal(list.begin(), list.end(), std::ceil(list.size() /
max_load_factor())) {}
```
The move constructor is a little trickier, let's discuss that one :-)
Hey Joris, I've filed
[MESOS-3217](https://issues.apache.org/jira/browse/MESOS-3217) to capture the
work involved in our transition from boost `unordered_{set,map}` and `hash` to
their standard counterparts. Meanwhile, I've committed this patch to keep
`hashset` and `hashmap` APIs consistent.
- Michael
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35874/#review89982
---
On July 7, 2015, 8:51 a.m., Alexander Rojas wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35874/
---
(Updated July 7, 2015, 8:51 a.m.)
Review request for mesos, Bernd Mathiske, Joerg Schad, Michael Park, and Till
Toenshoff.
Bugs: MESOS-2924
https://issues.apache.org/jira/browse/MESOS-2924
Repository: mesos
Description
---
Adds extra template parameters to hashset as well as implicit constructors
from
`std::set` and a initializer list constructor.
These changes keep hashset up to date with the changes in hashmap.
Diffs
-
3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp
75ed9db54dc9ab502e978f06c55a621cacb56b91
3rdparty/libprocess/3rdparty/stout/tests/hashset_tests.cpp
3c4b732432c0c155451d34ecd5f985318d118fe5
Diff: https://reviews.apache.org/r/35874/diff/
Testing
---
make check
Thanks,
Alexander Rojas
Review Request 37198: Add Docker image provisioner and copy backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37198/ --- Review request for mesos and Timothy Chen. Bugs: MESOS-2850 https://issues.apache.org/jira/browse/MESOS-2850 Repository: mesos Description --- Add Docker image provisioner and copy backend. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/slave/containerizer/provisioner.hpp cb4d511e8189b65df9b9803f23812dd98edc44ac src/slave/containerizer/provisioner.cpp df52e36b23ad3cd28f50e96865d0b163cc245cb2 src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37198/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On Aug. 6, 2015, 6:39 p.m., Ben Mahler wrote: src/master/master.cpp, line 4990 https://reviews.apache.org/r/36720/diff/9/?file=1033626#file1033626line4990 Don't we still want this since we call pid.get()? Persisted the CHECK_SOME. Removed the misleading message though as we have now implemented http framework failover. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94428 --- On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:25 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/#review94494 --- src/slave/containerizer/provisioners/docker/store.cpp (line 403) https://reviews.apache.org/r/37197/#comment149127 prefer using explicit captures. - Jojy Varghese On Aug. 6, 2015, 10:51 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 6, 2015, 10:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 7, 2015, 5:04 a.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Changes --- removed the discarded depends on review to unblock reviewbot -- @vinodkone Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 37192: More basic call validation tests for http api
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/#review94497 --- Patch looks great! Reviews applied: [37080, 36720, 37082, 37192] All tests passed. - Mesos ReviewBot On Aug. 7, 2015, 3:05 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/ --- (Updated Aug. 7, 2015, 3:05 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- Added more basic call validation tests around malformed body, invalid media types etc for the http api Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37192/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 7, 2015, 2:27 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Modified signature for subscribe(...), added _failoverFramework. Modified some TODO messages to be more explicit. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36837: Update gmock to 1.7.0.
On Aug. 7, 2015, midnight, Michael Park wrote: 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake, line 36 https://reviews.apache.org/r/36837/diff/7/?file=1033437#file1033437line36 `s/1.6.0/1.7.0/` Thank you very much, let me update. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36837/#review94466 --- On Aug. 6, 2015, 9:20 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36837/ --- (Updated Aug. 6, 2015, 9:20 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/CMakeLists.txt 711809e808ebd0ed95d62270220e016ba6f41dca 3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz d45d9894b0214f5f02a88f6da5c258327110efd8 3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 3rdparty/libprocess/3rdparty/versions.am 97727537778511ca5a10be4f3c25cd21d919 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a Diff: https://reviews.apache.org/r/36837/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/#review94488 --- src/slave/containerizer/provisioners/docker/store.hpp (line 128) https://reviews.apache.org/r/37197/#comment149114 What is the ownership model of Fetcher member? How do you know that the pointer is valid through the life of StoreProcess object? - Jojy Varghese On Aug. 6, 2015, 10:51 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 6, 2015, 10:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 7, 2015, 2:56 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Review comments by Vinod Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs (updated) - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
On Aug. 7, 2015, 12:21 a.m., Vinod Kone wrote: src/tests/http_api_tests.cpp, line 187 https://reviews.apache.org/r/37082/diff/3-4/?file=1032065#file1032065line187 No CHECK's in test code please. It will crash the program. Use ASSERT_SOME() instead. here and everywhere else. Whoops, my bad. Fixed. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/#review94468 --- On Aug. 6, 2015, 4:26 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 6, 2015, 4:26 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 36837: Update gmock to 1.7.0.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36837/ --- (Updated Aug. 7, 2015, 3:24 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs (updated) - 3rdparty/libprocess/3rdparty/CMakeLists.txt 711809e808ebd0ed95d62270220e016ba6f41dca 3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz d45d9894b0214f5f02a88f6da5c258327110efd8 3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 3rdparty/libprocess/3rdparty/versions.am 97727537778511ca5a10be4f3c25cd21d919 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a Diff: https://reviews.apache.org/r/36837/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/#review94491 --- src/slave/containerizer/provisioners/docker/store.hpp (line 72) https://reviews.apache.org/r/37197/#comment149117 Why not use the new c++11 delete keyword? - Jojy Varghese On Aug. 6, 2015, 10:51 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 6, 2015, 10:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/#review94489 --- src/slave/containerizer/provisioners/docker/store.hpp (line 54) https://reviews.apache.org/r/37197/#comment149115 I think new files should have javadoc style comments. - Jojy Varghese On Aug. 6, 2015, 10:51 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 6, 2015, 10:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/ --- (Updated Aug. 7, 2015, 3:30 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs - LICENSE 8757c208f91e9fa000574489bdb7432a0777d923 configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 Diff: https://reviews.apache.org/r/37162/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/ --- (Updated Aug. 7, 2015, 3:30 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs (updated) - LICENSE 8757c208f91e9fa000574489bdb7432a0777d923 configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 Diff: https://reviews.apache.org/r/37162/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.
On Aug. 7, 2015, 12:01 a.m., Michael Park wrote: We should also update the `LICENSE` file to reflect this update. ``` == -For gmock-1.6.0 (3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz): +For gmock-1.7.0 (3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz): == ``` Thank you, I updated it now. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/#review94467 --- On Aug. 7, 2015, 3:30 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/ --- (Updated Aug. 7, 2015, 3:30 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs - LICENSE 8757c208f91e9fa000574489bdb7432a0777d923 configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 Diff: https://reviews.apache.org/r/37162/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37198: Add Docker image provisioner and copy backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37198/#review94485 --- Ship it! LGTM - Guangya Liu On 八月 6, 2015, 8:37 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37198/ --- (Updated 八月 6, 2015, 8:37 p.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-2850 https://issues.apache.org/jira/browse/MESOS-2850 Repository: mesos Description --- Add Docker image provisioner and copy backend. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/slave/containerizer/provisioner.hpp cb4d511e8189b65df9b9803f23812dd98edc44ac src/slave/containerizer/provisioner.cpp df52e36b23ad3cd28f50e96865d0b163cc245cb2 src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37198/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37197: Docker image store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/#review94493 --- src/slave/containerizer/provisioners/docker/store.cpp (line 169) https://reviews.apache.org/r/37197/#comment149125 Why extra newline? - Jojy Varghese On Aug. 6, 2015, 10:51 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37197/ --- (Updated Aug. 6, 2015, 10:51 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Stored images currently kept indefinitely. Diffs - src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37197/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37196: Add Docker Image type to Container Image protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37196/#review94480 --- Ship it! Ship It! - Guangya Liu On 八月 6, 2015, 9:27 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37196/ --- (Updated 八月 6, 2015, 9:27 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-2851 https://issues.apache.org/jira/browse/MESOS-2851 Repository: mesos Description --- Add Docker Image type to Container Image protobuf. Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba Diff: https://reviews.apache.org/r/37196/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37192: More basic call validation tests for http api
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/ --- (Updated Aug. 7, 2015, 3:05 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Review comments from Vinod Repository: mesos Description --- Added more basic call validation tests around malformed body, invalid media types etc for the http api Diffs (updated) - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37192/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37194: Fixed a bug in the MesosContainerizer creation logic.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37194/#review94463 --- Patch looks great! Reviews applied: [36929, 36930, 36954, 36956, 37054, 37055, 37091, 37105, 37142, 37159, 37194] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 6:19 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37194/ --- (Updated Aug. 6, 2015, 6:19 p.m.) Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu. Repository: mesos Description --- Fixed a bug in the MesosContainerizer creation logic. I did a few small style fixes and added a few more comments in the patch as well. The bug is that currently we only put filesystem isolator to the front if it's coming from modules. Diffs - src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 Diff: https://reviews.apache.org/r/37194/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 36837: Update gmock to 1.7.0.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36837/#review94466 --- 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake (line 36) https://reviews.apache.org/r/36837/#comment149082 `s/1.6.0/1.7.0/` - Michael Park On Aug. 6, 2015, 9:20 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36837/ --- (Updated Aug. 6, 2015, 9:20 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/CMakeLists.txt 711809e808ebd0ed95d62270220e016ba6f41dca 3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz d45d9894b0214f5f02a88f6da5c258327110efd8 3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 3rdparty/libprocess/3rdparty/versions.am 97727537778511ca5a10be4f3c25cd21d919 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a Diff: https://reviews.apache.org/r/36837/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 37202: Fixed flaky MasterAuthorizationTest.DuplicateRegistration test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37202/#review94464 --- Ship it! Ship It! - Ben Mahler On Aug. 6, 2015, 11:52 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37202/ --- (Updated Aug. 6, 2015, 11:52 p.m.) Review request for mesos. Bugs: MESOS-3203 https://issues.apache.org/jira/browse/MESOS-3203 Repository: mesos Description --- Added a Clock::pause(). See the bug for details. Diffs - src/tests/master_authorization_tests.cpp e2dc093d708f778f9f3dbe9a7f122ddc51e34389 Diff: https://reviews.apache.org/r/37202/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 37192: More basic call validation tests for http api
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/#review94432 --- Ship it! src/tests/http_api_tests.cpp (line 137) https://reviews.apache.org/r/37192/#comment149033 s/though// s/server/master/ src/tests/http_api_tests.cpp (line 145) https://reviews.apache.org/r/37192/#comment149085 s/strings/string/ ? src/tests/http_api_tests.cpp (line 161) https://reviews.apache.org/r/37192/#comment149087 s/server/master/ src/tests/http_api_tests.cpp (line 199) https://reviews.apache.org/r/37192/#comment149088 did you need to pull this into a temp variable? why not just use it directly in #202? - Vinod Kone On Aug. 6, 2015, 5:06 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/ --- (Updated Aug. 6, 2015, 5:06 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- Added more basic call validation tests around malformed body, invalid media types etc for the http api Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37192/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/#review94468 --- src/tests/http_api_tests.cpp (line 182) https://reviews.apache.org/r/37082/#comment149089 AWAIT_READY guarantees that event.isReady(). no need for this check? here and everywhere. src/tests/http_api_tests.cpp (line 183) https://reviews.apache.org/r/37082/#comment149091 No CHECK's in test code please. It will crash the program. Use ASSERT_SOME() instead. here and everywhere else. - Vinod Kone On Aug. 6, 2015, 4:26 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 6, 2015, 4:26 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37114: MESOS-3187, support docker host command line option
On Aug. 7, 2015, 1:31 a.m., Timothy Chen wrote: We already have a flag called docker_socket that I think we can reuse instead of doing this. Thanks .. I will make the change ... On Aug. 7, 2015, 1:31 a.m., Timothy Chen wrote: src/docker/docker.hpp, line 45 https://reviews.apache.org/r/37114/diff/1/?file=1032204#file1032204line45 The new flag can just have a default value, no need to do this. My thought was it will make the Docker class complete .. but can do the change - Vaibhav --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review94472 --- On Aug. 6, 2015, 9:10 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 6, 2015, 9:10 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Review Request 37202: Fixed flaky MasterAuthorizationTest.DuplicateRegistration test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37202/ --- Review request for mesos. Bugs: MESOS-3203 https://issues.apache.org/jira/browse/MESOS-3203 Repository: mesos Description --- Added a Clock::pause(). See the bug for details. Diffs - src/tests/master_authorization_tests.cpp e2dc093d708f778f9f3dbe9a7f122ddc51e34389 Diff: https://reviews.apache.org/r/37202/diff/ Testing --- make check Thanks, Vinod Kone
Re: Review Request 37162: Add GTEST_LANG_CXX11 to configure.ac when compile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/#review94467 --- We should also update the `LICENSE` file to reflect this update. ``` == -For gmock-1.6.0 (3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz): +For gmock-1.7.0 (3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz): == ``` - Michael Park On Aug. 6, 2015, 5:12 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37162/ --- (Updated Aug. 6, 2015, 5:12 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs - configure.ac 230e90d3165618e8cf50e8d34bdfc41119ab24a3 Diff: https://reviews.apache.org/r/37162/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37200: Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/#review94456 --- Patch looks great! Reviews applied: [37196, 37197, 37198, 37199, 37200] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 8:40 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/ --- (Updated Aug. 6, 2015, 8:40 p.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers. Diffs - src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e Diff: https://reviews.apache.org/r/37200/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37105: Removed the code of checkpointing container root filesystem path.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37105/#review94153 --- src/slave/containerizer/mesos/containerizer.hpp (line 184) https://reviews.apache.org/r/37105/#comment149014 Simple documentation on the return value? src/slave/containerizer/mesos/containerizer.cpp (line 632) https://reviews.apache.org/r/37105/#comment149072 Why can't `prepare()` be chained by `then()` anymore? src/slave/containerizer/mesos/containerizer.cpp (lines 674 - 675) https://reviews.apache.org/r/37105/#comment149060 This doesn't get implicitly converted to Option? - Jiang Yan Xu On Aug. 5, 2015, 2:16 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37105/ --- (Updated Aug. 5, 2015, 2:16 p.m.) Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu. Bugs: MESOS-3205 https://issues.apache.org/jira/browse/MESOS-3205 Repository: mesos Description --- Removed the code of checkpointing container root filesystem path. See ticket for the motivation. Diffs - include/mesos/slave/isolator.proto 3d9222be5e9bd9e9f665fb2e57db6b7e925c8fbd src/common/protobuf_utils.hpp a4708ed286ef237f17d9fd7813be2f6e7218b42a src/common/protobuf_utils.cpp 3cb684598d0492a2f57b46fabcf13565ff42f27a src/slave/containerizer/mesos/containerizer.hpp 8851d30af56b4f9fb95450ac1f42ab550e3df9ff src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 src/slave/containerizer/provisioner.hpp cb4d511e8189b65df9b9803f23812dd98edc44ac src/slave/paths.cpp 404c2143e70771747d356b15eac9137495fd9a75 src/slave/state.hpp cecf200e6e79172af57ae195a73a5161be7e604a src/slave/state.cpp b9f2d8a0d6395b92bd50f1e0927f3ae9fd04b81c Diff: https://reviews.apache.org/r/37105/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94460 --- Before we go too far down this path, can you please see https://reviews.apache.org/r/36424/ and see whether it does what you are trying to achieve here? (maybe not, I'm not familiar at all with `cli` - just asking) - Marco Massenzio On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated Aug. 4, 2015, 9:33 a.m.) Review request for mesos, Adam B and Michael Park. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94459 --- Before we go too far down this path, can you please see https://reviews.apache.org/r/36424/ and see whether it does what you are trying to achieve here? (maybe not, I'm not familiar at all with `cli` - just asking) - Marco Massenzio On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated Aug. 4, 2015, 9:33 a.m.) Review request for mesos, Adam B and Michael Park. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94461 --- - Marco Massenzio On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated Aug. 4, 2015, 9:33 a.m.) Review request for mesos, Adam B and Michael Park. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 37190: WIP: Added POST support for /maintenance endpoint. Performed initial verification of the input.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37190/ --- (Updated Aug. 6, 2015, 6:34 p.m.) Review request for Benjamin Hindman, Joris Van Remoortere and Joseph Wu. Changes --- Uploading WIP. Bugs: MESOS-2067 https://issues.apache.org/jira/browse/MESOS-2067 Repository: mesos Description --- See summary. Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 53420ca7d503296fbe11b1ea0795afe2ebf86255 src/master/master.cpp d699e4bc3cf734a516a6baf329919e04744b5702 Diff: https://reviews.apache.org/r/37190/diff/ Testing --- Manual testing of the endpoint. Thanks, Artem Harutyunyan
Re: Review Request 37196: Add Docker Image type to Container Image protobuf.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37196/#review94474 --- Ship it! Ship It! - Timothy Chen On Aug. 6, 2015, 9:27 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37196/ --- (Updated Aug. 6, 2015, 9:27 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, and Timothy Chen. Bugs: MESOS-2851 https://issues.apache.org/jira/browse/MESOS-2851 Repository: mesos Description --- Add Docker Image type to Container Image protobuf. Diffs - include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba Diff: https://reviews.apache.org/r/37196/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review94472 --- We already have a flag called docker_socket that I think we can reuse instead of doing this. src/docker/docker.hpp (line 45) https://reviews.apache.org/r/37114/#comment149099 The new flag can just have a default value, no need to do this. - Timothy Chen On Aug. 6, 2015, 9:10 p.m., Vaibhav Khanduja wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- (Updated Aug. 6, 2015, 9:10 p.m.) Review request for mesos, Benjamin Hindman, Ben Mahler, Timothy Chen, and Vinod Kone. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value - make check Thanks, Vaibhav Khanduja
Re: Review Request 36837: Update gmock to 1.7.0.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36837/ --- (Updated Aug. 6, 2015, 6:26 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs (updated) - 3rdparty/libprocess/3rdparty/CMakeLists.txt 711809e808ebd0ed95d62270220e016ba6f41dca 3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz d45d9894b0214f5f02a88f6da5c258327110efd8 3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 3rdparty/libprocess/3rdparty/versions.am 97727537778511ca5a10be4f3c25cd21d919 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a Diff: https://reviews.apache.org/r/36837/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 37179: Enhance the bootstrap script
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37179/#review94397 --- Patch looks great! Reviews applied: [37179] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 9:37 a.m., Zhiwei Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37179/ --- (Updated Aug. 6, 2015, 9:37 a.m.) Review request for mesos and Yong Qiao Wang. Bugs: MESOS-3218 https://issues.apache.org/jira/browse/MESOS-3218 Repository: mesos Description --- Force override the link file when it is invalid. Diffs - bootstrap 779b33cdcb88b2417769bb046a17b47cd6042f2d Diff: https://reviews.apache.org/r/37179/diff/ Testing --- make check Thanks, Zhiwei Chen
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/#review94393 --- Patch looks great! Reviews applied: [37065] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 5:54 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/ --- (Updated Aug. 6, 2015, 5:54 a.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3197 https://issues.apache.org/jira/browse/MESOS-3197 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/memory_test_helper.cpp 5e40b747f4266e7532baf8fd02ea5db0955124d2 Diff: https://reviews.apache.org/r/37065/diff/ Testing --- make check Thanks, Artem Harutyunyan
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/#review94394 --- src/tests/containerizer/memory_test_helper.cpp (lines 75 - 76) https://reviews.apache.org/r/37065/#comment148980 Not a big fan of explain how something will be used in the method description. What if we remove references to `mlockall()` and `mlock()` here. Something a long the lines of: This helper allocates memory and locks it to the physical memory so it won't get swapped by the OS. src/tests/containerizer/memory_test_helper.cpp (lines 79 - 86) https://reviews.apache.org/r/37065/#comment148981 After reading the man pages, `mlock()` exists in Linux too and frankly it looks safer to use than `mlockall()`. `mlock()` will only lock the memory passed to the call in the physical memory while `mlockall()` will lock every new allocation after we made this call (and that includes stack, heap and code sections) until we make a `munlockall()` call, which we don't. - Alexander Rojas On Aug. 6, 2015, 7:54 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/ --- (Updated Aug. 6, 2015, 7:54 a.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3197 https://issues.apache.org/jira/browse/MESOS-3197 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/memory_test_helper.cpp 5e40b747f4266e7532baf8fd02ea5db0955124d2 Diff: https://reviews.apache.org/r/37065/diff/ Testing --- make check Thanks, Artem Harutyunyan
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. 6, 2015, 1:59 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 (updated) - include/mesos/authorizer/authorizer.hpp PRE-CREATION include/mesos/authorizer/authorizer.proto PRE-CREATION include/mesos/mesos.proto a6748d1cd82238f005c6a49c70d22d095462f1ba include/mesos/type_utils.hpp f1cb5e279073c5195fc41dada307a10d00c84955 src/Makefile.am 35ebbbd0bd9c9dd059c02ce3dc22c780b929be81 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 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/master/master.hpp 30a2550c606ca528ec5b69fc9efedd698d67c5f2 src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 src/tests/authorization_tests.cpp 99bb06c1ee73a90abaeeabb742e45aa188c21a87 src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b src/tests/mesos.hpp 20418d4fbd2f4ae35ee0c707472cbf37125883b0 src/tests/mesos.cpp a2a469e2a581dc6c566dafd4acd2a95c0238399f Diff: https://reviews.apache.org/r/36048/diff/ Testing --- make check Thanks, Alexander Rojas
Review Request 37192: More basic call validation tests for http api
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/ --- Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- Added more basic call validation tests around malformed body, invalid media types etc for the http api Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37192/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37097: Fix 'Accept-Encoding' parsing
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37097/ --- (Updated Aug. 6, 2015, 5:35 p.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- Currently parsing only compares the begining of the header making 'gzipbug' match with candidate 'gzip' Diffs (updated) - 3rdparty/libprocess/include/process/http.hpp b8d9300 3rdparty/libprocess/src/http.cpp 4dcbd74 3rdparty/libprocess/src/tests/encoder_tests.cpp 0032137 Diff: https://reviews.apache.org/r/37097/diff/ Testing --- make check Thanks, Isabel Jimenez
Re: Review Request 36979: Updating all references to os::shell
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/ --- (Updated Aug. 6, 2015, 6:24 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Changes --- Updated call signatures. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Updating all references to os::shell For more details see MESOS-3142. Diffs (updated) - src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/slave/containerizer/isolators/network/port_mapping.cpp 8244c345b84108af7fa18d20e71401d6e1a0aeb0 src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/36979/diff/ Testing --- make check *Note*: this patch fixes breakages introduce by the refactoring in: https://reviews.apache.org/r/36978 Thanks, Marco Massenzio
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/#review94431 --- Patch looks great! Reviews applied: [37065] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 5:34 p.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/ --- (Updated Aug. 6, 2015, 5:34 p.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3197 https://issues.apache.org/jira/browse/MESOS-3197 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/memory_test_helper.cpp 5e40b747f4266e7532baf8fd02ea5db0955124d2 Diff: https://reviews.apache.org/r/37065/diff/ Testing --- make check Thanks, Artem Harutyunyan
Re: Review Request 36978: MESOS-3142 Refactoring os::shell - patch 1/2
On Aug. 4, 2015, 7:54 p.m., Artem Harutyunyan wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, line 50 https://reviews.apache.org/r/36978/diff/1/?file=1026032#file1026032line50 should the variable be called `_cmd`? Marco Massenzio wrote: note this is neither a private member, not a constructor arg - it's a temp var: AFAIK there are no guidelines (apart from the obvious naming it sensibly). Renamed `command` It does not really matter since you renamed, but there is a guideline for function arguments: ``` We prepend constructor and function arguments with a leading underscore to avoid ambiguity and / or shadowing: ``` - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36978/#review94162 --- On Aug. 6, 2015, 11:20 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36978/ --- (Updated Aug. 6, 2015, 11:20 a.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Refactoring os::shell. See MESOS-3142 for more details. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp ab767ccc4553cc5f61e4fe1b67110a9b5b32f2bc 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 53f14e1869ed7a6e1ac7cc8a82c558ed77907dc9 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 4b7a7bafe1c64183d021b39cf12523250491f0ee 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 2556bd428cc8990659e30e804b9c96c1659ef4a1 Diff: https://reviews.apache.org/r/36978/diff/ Testing --- make check *Note*: this patch by itself breaks mesos - this only fixes the `stout` part: see also https://reviews.apache.org/r/36979/ Thanks, Marco Massenzio
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/#review94422 --- Patch looks great! Reviews applied: [37080, 36720, 37082] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 4:26 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 6, 2015, 4:26 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Review Request 37194: Fixed a bug in the MesosContainerizer creation logic.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37194/ --- Review request for mesos, Ian Downes, Timothy Chen, Vinod Kone, and Jiang Yan Xu. Repository: mesos Description --- Fixed a bug in the MesosContainerizer creation logic. I did a few small style fixes and added a few more comments in the patch as well. The bug is that currently we only put filesystem isolator to the front if it's coming from modules. Diffs - src/slave/containerizer/mesos/containerizer.cpp 6d07ff151770bac4eeeb7cd8c9d03f54f2e78ec1 Diff: https://reviews.apache.org/r/37194/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 36979: Updating all references to os::shell
On Aug. 5, 2015, 4:16 a.m., Artem Harutyunyan wrote: src/tests/containerizer/port_mapping_tests.cpp, line 986 https://reviews.apache.org/r/36979/diff/1/?file=1026038#file1026038line986 ditto. + extra newline. Marco Massenzio wrote: Having looked at both tests, I was being unnecessarily pedantic IMO: checking for the error code (256) to be present in the error string seems to me to be more than sufficient (and self-explanatory too - but added a comment all the same). What thinks you? Artem Harutyunyan wrote: After think a bit more about it, I find it a bit unusual that the user has to perform a string search in order to get out the integer error code. In cases when you expect a certain kind of failure from a certain command it's easy (like in your test case), but what if the cause of failure is unknown, or if there are several possible error codes expected. It looks to me that one will need to involve a regex parser to be able to reliably(?) get the signal and the error code out. This might drive delopers away from this function, and cause proliferation of similar code in the codebase (something that this was meant to facilitate avoiding). Returning a primtive struct(or a union) with a couple of fields could easily help to avoid that. Marco Massenzio wrote: Well, as a data point, as mentioned, the use case you mention, was **never** used in our current (what, 2-3year old?) code base. And, in any event, if anyone wants a rich outcome, they can use `process::subprocess` (which I'm updating next) and they'll get pretty much everything they ever wanted to know about the outcome. This is really meant to be an easy-to-use, low-boilerplate API for the simple use case: I want to run `cmd` and only care whether it succeeded or not. see `CommandResult` in https://reviews.apache.org/r/36424/diff/6#index_header - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/#review94177 --- On Aug. 6, 2015, 6:24 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/ --- (Updated Aug. 6, 2015, 6:24 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Updating all references to os::shell For more details see MESOS-3142. Diffs - src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/slave/containerizer/isolators/network/port_mapping.cpp 8244c345b84108af7fa18d20e71401d6e1a0aeb0 src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/36979/diff/ Testing --- make check *Note*: this patch fixes breakages introduce by the refactoring in: https://reviews.apache.org/r/36978 Thanks, Marco Massenzio
Re: Review Request 37192: More basic call validation tests for http api
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/#review94427 --- Patch looks great! Reviews applied: [37080, 36720, 37082, 37192] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 5:06 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37192/ --- (Updated Aug. 6, 2015, 5:06 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos Description --- Added more basic call validation tests around malformed body, invalid media types etc for the http api Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37192/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
On Aug. 6, 2015, 4:47 a.m., Alexander Rojas wrote: src/tests/containerizer/memory_test_helper.cpp, lines 79-87 https://reviews.apache.org/r/37065/diff/3/?file=1033300#file1033300line79 After reading the man pages, `mlock()` exists in Linux too and frankly it looks safer to use than `mlockall()`. `mlock()` will only lock the memory passed to the call in the physical memory while `mlockall()` will lock every new allocation after we made this call (and that includes stack, heap and code sections) until we make a `munlockall()` call, which we don't. It does exist on Linux, it just doesn't realiably work for our use case on Ubuntu. I am aware of the difference of mlock() and mlockall(), in the context of the OOM test it does not really mattter because these are short lived test processes that are either getting killed or are going away. On Aug. 6, 2015, 4:47 a.m., Alexander Rojas wrote: src/tests/containerizer/memory_test_helper.cpp, lines 75-76 https://reviews.apache.org/r/37065/diff/3/?file=1033300#file1033300line75 Not a big fan of explain how something will be used in the method description. What if we remove references to `mlockall()` and `mlock()` here. Something a long the lines of: This helper allocates memory and locks it to the physical memory so it won't get swapped by the OS. I've removed references to `mlockall()` and `mlock()`. - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/#review94394 --- On Aug. 6, 2015, 10:34 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/ --- (Updated Aug. 6, 2015, 10:34 a.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3197 https://issues.apache.org/jira/browse/MESOS-3197 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/memory_test_helper.cpp 5e40b747f4266e7532baf8fd02ea5db0955124d2 Diff: https://reviews.apache.org/r/37065/diff/ Testing --- make check Thanks, Artem Harutyunyan
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/ --- (Updated Aug. 6, 2015, 10:34 a.m.) Review request for mesos, Benjamin Hindman and Michael Park. Changes --- Addressed comments. Bugs: MESOS-3197 https://issues.apache.org/jira/browse/MESOS-3197 Repository: mesos Description --- See summary. Diffs (updated) - src/tests/containerizer/memory_test_helper.cpp 5e40b747f4266e7532baf8fd02ea5db0955124d2 Diff: https://reviews.apache.org/r/37065/diff/ Testing --- make check Thanks, Artem Harutyunyan
Re: Review Request 37065: Fixed MemIsolatorTest failure on OSX.
On Aug. 6, 2015, 1:21 a.m., Michael Park wrote: src/tests/containerizer/memory_test_helper.cpp, lines 81-83 https://reviews.apache.org/r/37065/diff/3/?file=1033300#file1033300line81 `mlockall` is not what we want ideally right? Can we mention that it's not ideal but that it exists temporarily to address `MESOS-3197`? I am not sure. For the purpose of this test (which is to artifficiialy drive memory utilization) mlockall() seems to be a safer choice. - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/#review94376 --- On Aug. 6, 2015, 10:34 a.m., Artem Harutyunyan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37065/ --- (Updated Aug. 6, 2015, 10:34 a.m.) Review request for mesos, Benjamin Hindman and Michael Park. Bugs: MESOS-3197 https://issues.apache.org/jira/browse/MESOS-3197 Repository: mesos Description --- See summary. Diffs - src/tests/containerizer/memory_test_helper.cpp 5e40b747f4266e7532baf8fd02ea5db0955124d2 Diff: https://reviews.apache.org/r/37065/diff/ Testing --- make check Thanks, Artem Harutyunyan
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94428 --- Thanks, I'll get the HttpConnection change committed, left some notes about things I updated. Also opened some issues in the subscribe path that I noticed while doing this. src/common/http.hpp (line 50) https://reviews.apache.org/r/36720/#comment149022 Hm.. let's at least say that it's based on the content type. src/common/http.cpp (line 49) https://reviews.apache.org/r/36720/#comment149021 No need for std:: here src/common/http.cpp (lines 61 - 63) https://reviews.apache.org/r/36720/#comment149020 How about we just put UNREACHABLE at the bottom outside the switch? That way, we don't lose the switch warning for missing enum cases, right? src/master/master.hpp (lines 1542 - 1544) https://reviews.apache.org/r/36720/#comment149023 Let's have the master decide when to set up this callback. On that note, how about we add a `closed()` on the HttpConnection as well? src/master/master.cpp (line 2216) https://reviews.apache.org/r/36720/#comment149024 This change looks like a regression, now we're sending to '`framework-pid`' instead of '`from`'? src/master/master.cpp https://reviews.apache.org/r/36720/#comment149027 Don't we still want this since we call pid.get()? - Ben Mahler On Aug. 6, 2015, 4:25 p.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:25 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36979: Updating all references to os::shell
On Aug. 5, 2015, 4:16 a.m., Artem Harutyunyan wrote: src/tests/containerizer/port_mapping_tests.cpp, line 986 https://reviews.apache.org/r/36979/diff/1/?file=1026038#file1026038line986 ditto. + extra newline. Marco Massenzio wrote: Having looked at both tests, I was being unnecessarily pedantic IMO: checking for the error code (256) to be present in the error string seems to me to be more than sufficient (and self-explanatory too - but added a comment all the same). What thinks you? Artem Harutyunyan wrote: After think a bit more about it, I find it a bit unusual that the user has to perform a string search in order to get out the integer error code. In cases when you expect a certain kind of failure from a certain command it's easy (like in your test case), but what if the cause of failure is unknown, or if there are several possible error codes expected. It looks to me that one will need to involve a regex parser to be able to reliably(?) get the signal and the error code out. This might drive delopers away from this function, and cause proliferation of similar code in the codebase (something that this was meant to facilitate avoiding). Returning a primtive struct(or a union) with a couple of fields could easily help to avoid that. Well, as a data point, as mentioned, the use case you mention, was **never** used in our current (what, 2-3year old?) code base. And, in any event, if anyone wants a rich outcome, they can use `process::subprocess` (which I'm updating next) and they'll get pretty much everything they ever wanted to know about the outcome. This is really meant to be an easy-to-use, low-boilerplate API for the simple use case: I want to run `cmd` and only care whether it succeeded or not. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/#review94177 --- On Aug. 6, 2015, 6:24 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/ --- (Updated Aug. 6, 2015, 6:24 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Updating all references to os::shell For more details see MESOS-3142. Diffs - src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/slave/containerizer/isolators/network/port_mapping.cpp 8244c345b84108af7fa18d20e71401d6e1a0aeb0 src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/36979/diff/ Testing --- make check *Note*: this patch fixes breakages introduce by the refactoring in: https://reviews.apache.org/r/36978 Thanks, Marco Massenzio
Review Request 37114: MESOS-3187, support docker host command line option
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/ --- Review request for mesos. Bugs: MESOS-3187 https://issues.apache.org/jira/browse/MESOS-3187 Repository: mesos Description --- MESOS-3187, support docker host command line option. Docker daemon supports starting on a non-default port. Such scenarios would needed when starting Docker daemon on TCP or non-default unix port. Mesos slave does not work if Docker daemon is started on any of such non-default port. The code change is needed in Mesos slave to accept this parameter so as connect for its operations to the right Docker daemon. The change is made in Mesos slave, so as it is available to any framework making using Docker executor. The code is added to start slave binary with --docker_host, instructing it to connect on port as specified in the parameter. The default value of --default_host is unix:///var/run/docker.sock, which is default port for Docker daemon. The main class src/docker.cpp/.hpp is kept backward compartible to make Docker cli execute on default Docker port. Diffs - src/docker/docker.hpp 38e5299ad38b9e20501387f2193b0fa448e49e3e src/docker/docker.cpp 1367de8a7bbbda6348a30e4ef4c616378e450250 src/docker/executor.hpp fa13b6e9905051eef27d3a51b75a5c86fdad0dd7 src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/slave/flags.hpp 881d494c06fea5c382d27b357d65c1baf201ae46 src/slave/flags.cpp 82b6cf47af26f0533ff603a67240777e9a9b986e src/tests/containerizer/docker_tests.cpp a4a2725c05ae0cb88426c587f7ded0da77154edc src/tests/flags.hpp 364495695c5915e54257014aeebb1e212d3da6fc Diff: https://reviews.apache.org/r/37114/diff/ Testing --- Following scenarios were executed to test the code changes. Kindly suggest if more test-cases are required: a) Mesos slave with unix port : unix:///var/run/docker_myport.sock i) Start slave with --docker_host parameter unix:///var/run/docker_myport.sock ii) Using a framework, in my case Marathon, post a Docker job iii) The docker job does get started on the slave, confirmed with docker ps command output docker -H unix:///var/run/docker_myport.sock ps CONTAINER IDIMAGE COMMANDCREATED STATUS PORTS NAMES 07fc4ec86bacmygoserver /bin/sh -c /mygoser 19 minutes ago Up 19 minutes */tcp, */udp mesos-20150731-104052-1051068938-5050-7913-S33.17b355cd-2754-4fb2-a558-66820dff033c iv) Stop or destroy the job from Marathon GUI b) Two mesos slave with non-default docker port i) On two different hosts, start slave, with one running on default port and other non-default. The start slaves with attributes - default and or non-default. ii) Give jobs to these slaves, using Marathon UNIQUE attribute, selecting slave - non-default default iii) Stop/destroy the jobs d) Modified unit test-case taking docker port value Thanks, Vaibhav Khanduja
Re: Review Request 36978: MESOS-3142 Refactoring os::shell - patch 1/2
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36978/ --- (Updated Aug. 6, 2015, 6:20 p.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Changes --- Following Ben's comments, reverted the call signature to accept varargs. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Refactoring os::shell. See MESOS-3142 for more details. Diffs (updated) - 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp ab767ccc4553cc5f61e4fe1b67110a9b5b32f2bc 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 53f14e1869ed7a6e1ac7cc8a82c558ed77907dc9 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 4b7a7bafe1c64183d021b39cf12523250491f0ee 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 2556bd428cc8990659e30e804b9c96c1659ef4a1 Diff: https://reviews.apache.org/r/36978/diff/ Testing --- make check *Note*: this patch by itself breaks mesos - this only fixes the `stout` part: see also https://reviews.apache.org/r/36979/ Thanks, Marco Massenzio
Re: Review Request 37190: WIP: Added POST support for /maintenance endpoint. Performed initial verification of the input.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37190/ --- (Updated Aug. 6, 2015, 11:27 a.m.) Review request for mesos, Benjamin Hindman, Joris Van Remoortere, and Joseph Wu. Changes --- Addressed some of the comments. Bugs: MESOS-2067 https://issues.apache.org/jira/browse/MESOS-2067 Repository: mesos Description --- See summary. Diffs (updated) - src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/maintenance.hpp PRE-CREATION src/master/maintenance.cpp PRE-CREATION src/master/master.hpp 30a2550c606ca528ec5b69fc9efedd698d67c5f2 src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/37190/diff/ Testing --- Manual testing of the endpoint. Thanks, Artem Harutyunyan
Re: Review Request 37160: Add GTEST_LANG_CXX11 to StoutTestsConfigure when compile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37160/#review94372 --- Patch looks great! Reviews applied: [37160] All tests passed. - Mesos ReviewBot On Aug. 6, 2015, 5:13 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37160/ --- (Updated Aug. 6, 2015, 5:13 a.m.) Review request for mesos and Michael Park. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs - 3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake d69920689932d2afc8416d0f8ba289695444d8b2 Diff: https://reviews.apache.org/r/37160/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37074: Added process id prefix for benchmarks and process tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37074/#review94383 --- Ship it! Ship It! - Alexander Rojas On Aug. 5, 2015, 4:52 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37074/ --- (Updated Aug. 5, 2015, 4:52 p.m.) Review request for mesos. Bugs: MESOS-1457 https://issues.apache.org/jira/browse/MESOS-1457 Repository: mesos Description --- This patch was originally proposed by Palak Choudhary via https://reviews.apache.org/r/31076/. This version is just rebased onto the current master. Diffs - 3rdparty/libprocess/src/tests/benchmarks.cpp 97c81b8e61a75771e5bf7d46505cec4e0c0f404a 3rdparty/libprocess/src/tests/process_tests.cpp 95e3257b030128e9d03dde9aa048602c68c6a446 Diff: https://reviews.apache.org/r/37074/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 37073: Added process id prefix for cram_md5_authentication_tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37073/#review94384 --- Ship it! Ship It! - Alexander Rojas On Aug. 5, 2015, 4:53 p.m., Till Toenshoff wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37073/ --- (Updated Aug. 5, 2015, 4:53 p.m.) Review request for mesos. Bugs: MESOS-1457 https://issues.apache.org/jira/browse/MESOS-1457 Repository: mesos Description --- This patch was originally proposed by Palak Choudhary via https://reviews.apache.org/r/31075/. This version is just rebased onto the current master. Diffs - src/tests/cram_md5_authentication_tests.cpp b216ae4821c5bd967ea1b0e6787270246ff224f4 Diff: https://reviews.apache.org/r/37073/diff/ Testing --- make check Thanks, Till Toenshoff
Re: Review Request 36837: Update gmock to 1.7.0.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36837/ --- (Updated Aug. 6, 2015, 9:20 a.m.) Review request for mesos and Michael Park. Changes --- osx default tar would append some file have `_` prefix to package, this let distcheck failed. I use gnu tar to repackage it again. Bugs: MESOS-3141 https://issues.apache.org/jira/browse/MESOS-3141 Repository: mesos Description --- See summary. Diffs (updated) - 3rdparty/libprocess/3rdparty/CMakeLists.txt 711809e808ebd0ed95d62270220e016ba6f41dca 3rdparty/libprocess/3rdparty/gmock-1.6.0.tar.gz d45d9894b0214f5f02a88f6da5c258327110efd8 3rdparty/libprocess/3rdparty/gmock-1.7.0.tar.gz PRE-CREATION 3rdparty/libprocess/3rdparty/versions.am 97727537778511ca5a10be4f3c25cd21d919 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 3c1bb0bfed7e31440dc4be5ee9e3df4ae9152c5c 3rdparty/libprocess/configure.ac 40f344c6847424ea9b68e3d368497bf2763b4c6a Diff: https://reviews.apache.org/r/36837/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 37165: Introduced v1 API.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37165/#review94381
---
Bad patch!
Reviews applied: [37165]
Failed command: ./support/apply-review.sh -n -r 37165
Error:
2015-08-06 09:27:43 URL:https://reviews.apache.org/r/37165/diff/raw/
[253063/253063] - 37165.patch [1]
error: patch failed: src/tests/scheduler_tests.cpp:275
error: src/tests/scheduler_tests.cpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Aug. 6, 2015, 6:31 a.m., Benjamin Hindman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37165/
---
(Updated Aug. 6, 2015, 6:31 a.m.)
Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.
Repository: mesos
Description
---
Perhaps the right thing is to move internal/{d|e}volve.hpp|cpp to
v1/{d|e}volve.hpp|cpp?
Note that Anand can fix up src/scheduler/scheduler.cpp to just use the HTTP
API once it's finished and can kill all authenticating code and 'install',
'send', 'evolve', 'devolve' code and update src/tests/scheduler_tests.cpp as
well.
Diffs
-
include/mesos/scheduler.hpp cd235a11e63a5df742057be8e27629db4cf9
include/mesos/v1/attributes.hpp PRE-CREATION
include/mesos/v1/mesos.hpp PRE-CREATION
include/mesos/v1/mesos.proto PRE-CREATION
include/mesos/v1/resources.hpp PRE-CREATION
include/mesos/v1/scheduler.hpp PRE-CREATION
include/mesos/v1/scheduler/scheduler.hpp PRE-CREATION
include/mesos/v1/scheduler/scheduler.proto PRE-CREATION
include/mesos/v1/values.hpp PRE-CREATION
src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec
src/common/protobuf_utils.hpp a4708ed286ef237f17d9fd7813be2f6e7218b42a
src/common/protobuf_utils.cpp 3cb684598d0492a2f57b46fabcf13565ff42f27a
src/examples/event_call_framework.cpp
8054068fa746f8635f1133ceac530e04eaa0e1d7
src/internal/devolve.hpp PRE-CREATION
src/internal/devolve.cpp PRE-CREATION
src/internal/evolve.hpp PRE-CREATION
src/internal/evolve.cpp PRE-CREATION
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/scheduler/scheduler.cpp 97fa2c063db506dec69ff1edd851c96b4e1219a4
src/tests/mesos.hpp 93d87c78e5665b8104dbbc3d1e8c92e515cc67ab
src/tests/scheduler_driver_tests.cpp PRE-CREATION
src/tests/scheduler_tests.cpp 98fc70bf43ba99b54064a236795c7e1269004b71
src/tests/slave_tests.cpp cb5a01ed771e66d75091ca33523dbe673e16a86e
src/v1/attributes.cpp PRE-CREATION
src/v1/mesos.cpp PRE-CREATION
src/v1/resources.cpp PRE-CREATION
src/v1/values.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37165/diff/
Testing
---
make check
Thanks,
Benjamin Hindman
Re: Review Request 36978: MESOS-3142 Refactoring os::shell - patch 1/2
On Aug. 6, 2015, 5:48 a.m., Benjamin Hindman wrote:
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, line 44
https://reviews.apache.org/r/36978/diff/2/?file=1032323#file1032323line44
s/cmd/command/
Reverted.
On Aug. 6, 2015, 5:48 a.m., Benjamin Hindman wrote:
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, line 45
https://reviews.apache.org/r/36978/diff/2/?file=1032323#file1032323line45
s/cmd/command/
s/empyt/empty/
Reverted.
On Aug. 6, 2015, 5:48 a.m., Benjamin Hindman wrote:
3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp, line
31
https://reviews.apache.org/r/36978/diff/2/?file=1032324#file1032324line31
s/cmd/command/ (matches the declaration).
reverted.
On Aug. 6, 2015, 5:48 a.m., Benjamin Hindman wrote:
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp, lines
55-57
https://reviews.apache.org/r/36978/diff/2/?file=1032323#file1032323line55
A couple of comments:
(1) This is a good place for a 'std::initializer_list' to clean up the
call sites, i.e.,
os::shell(echo, {hello, world});
versus:
os::shell(echo, vectorstring{hello, world});
(2) I'm a little confounded by the 'std::vector' approach (even after
replacing with 'std::initializer_list'). In particular, it seems like the
value it adds is that it keeps a programmer from having to do
'strings::join( , args)' but they still have to stringify their arguments
which makes me wonder why you wouldn't just use 'strings::format' outside
of the call everywhere? Or why not keep the original printf version and
call 'strings::format' internally instead of 'strings::join'? With the
'std::initializer_list' approach I'll imagine we'll see a mix of:
(A) os::shell(echo, {hello, stringify(arg)});
or:
(B) os::shell(echo hello + stringify(arg));
or:
(C) os::shell(strings::join( , echo, hello, stringify(arg)));
or:
(D) os::shell(strings::format(echo hello %s, arg));
The existing version (or an improved variadic template version) would
have looked something like:
(E) os::shell(echo hello %s, arg);
I acknowledge that the 'std::initializer_list' version let's you add a
third 'ignoreErrors' parameter (which you couldn't do if instead of 'args'
you used variadic parameters). But, do we really need the 'ignoreErrors'
parameter? Why not just ask people to append '|| exit 0' to their command
just like we ask people to append '21'? I like the idea of just giving
people a conduit to the shell, and however they'd do stuff in the shell
world they do here too.
Now, if we were _not_ using 'popen' under the covers or passing a
string to '/bin/sh' then I would definitely see the value in a
'std::initializer_list' because then I wouldn't need to quote the command!
But unfortunately we're going to have to force people to quote the command
no matter what.
Thus, my suggestion is to ask people to append '|| exit 0' and then go
with a variadic template implementation, i.e., (E), or if you want to be
dead simple do (D) and then use 'strings::format' everywhere in the next
review.
(Note that we use '%s' with our 'strings::format' for all types kind of
like we use '{}' in Python string templates.)
hahaha the `... || true` was the first thing I thought (and used to prove the
failure) then went like OMG, they'll make fun of me and used `ignoreErrors`
instead - totally up for it!
As for why I changed the call signature, I am not quite sure either... the more
I think about it, it must have been a reflective Java-ism: varargs (and
arrays) feel very pre-1.5, so I just get rid of them whenever I see them: this
clearly doesn't apply to C++, though!
Reverted back to the original signature (minus `stdout`, obviously) and, you
are totally right: from a caller's perspective is a way superior user
experience!
- Marco
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36978/#review94348
---
On Aug. 5, 2015, 7:56 a.m., Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36978/
---
(Updated Aug. 5, 2015, 7:56 a.m.)
Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
Bugs: MESOS-3142
https://issues.apache.org/jira/browse/MESOS-3142
Repository: mesos
Description
---
Refactoring os::shell.
See MESOS-3142 for more details.
Diffs
-
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94413 --- src/common/http.cpp (line 60) https://reviews.apache.org/r/36720/#comment148999 need a return here to aviod compiler warning. - Vinod Kone On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:08 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
On Aug. 6, 2015, 4:10 p.m., Vinod Kone wrote: src/common/http.cpp, line 60 https://reviews.apache.org/r/36720/diff/8/?file=1033227#file1033227line60 need a return here to aviod compiler warning. I am putting in a fix for that. This looks to be as a GCC bug. With -Werror=uninitialized, -Wswitch turned on in our code. It shouldn't ever complain. Clang rightfully figures this out but GCC does not. Would go ahead and put in a default: UNREACHABLE() workaround here for now. - Anand --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/#review94413 --- On Aug. 6, 2015, 4:08 a.m., Anand Mazumdar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:08 a.m.) Review request for mesos, Ben Mahler and Vinod Kone. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 36720: Add subscribe- subscribed workflow for http frameworks
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36720/ --- (Updated Aug. 6, 2015, 4:25 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Added a UNREACHABLE for default case to put off GCC for now. Bugs: MESOS-2294 https://issues.apache.org/jira/browse/MESOS-2294 Repository: mesos Description --- Split review out of r36318. This change adds the functionality of making a http call for subscribe and the master responding with a subscribed event on the persistent stream. Also added functionality for framework failover equivalent of re-register. It should now be possible to merge the subscribed(...) introduced in this review and the re-factor that happened in MESOS-3182. - Made a new function for exited()/failoverFramework for http frameworks that invoke into the common continuation function for pid/http frameworks thereafter. - The re-register functionality equivalent goes in _subscribe(...) Diffs (updated) - src/common/http.hpp 9e4290f9e1f72730f63466d498a953cc5031a92b src/common/http.cpp a74c51d9392d0b0a67d51a0143eb314cfca11245 src/master/http.cpp 76e70801925041f08bc94f0ca18c86f1a573b2b3 src/master/master.hpp e44174976aa64176916827bec4c911333c9a91db src/master/master.cpp 50b98248463fc4cd48962890c14c7ad64f2b6f43 Diff: https://reviews.apache.org/r/36720/diff/ Testing --- make check + adding tests in a different patch. Thanks, Anand Mazumdar
Re: Review Request 37082: Tests for subscribe/failover functionality for http based framework
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37082/ --- (Updated Aug. 6, 2015, 4:26 p.m.) Review request for mesos, Ben Mahler and Vinod Kone. Changes --- Rerun reviewbot again with the enum fix for gcc in r36720. Repository: mesos Description --- This implements the tests for http framework subscribe/failover/upgrade from a pid based framework. The test are parameterized on content type and hence test both json/protobuf responses. Diffs - src/tests/http_api_tests.cpp 586d11288828fe9997e54f5dbd7d28c200e973f5 Diff: https://reviews.apache.org/r/37082/diff/ Testing --- make check Thanks, Anand Mazumdar
Re: Review Request 36979: Updating all references to os::shell
On Aug. 4, 2015, 9:16 p.m., Artem Harutyunyan wrote: src/tests/containerizer/port_mapping_tests.cpp, line 986 https://reviews.apache.org/r/36979/diff/1/?file=1026038#file1026038line986 ditto. + extra newline. Marco Massenzio wrote: Having looked at both tests, I was being unnecessarily pedantic IMO: checking for the error code (256) to be present in the error string seems to me to be more than sufficient (and self-explanatory too - but added a comment all the same). What thinks you? After think a bit more about it, I find it a bit unusual that the user has to perform a string search in order to get out the integer error code. In cases when you expect a certain kind of failure from a certain command it's easy (like in your test case), but what if the cause of failure is unknown, or if there are several possible error codes expected. It looks to me that one will need to involve a regex parser to be able to reliably(?) get the signal and the error code out. This might drive delopers away from this function, and cause proliferation of similar code in the codebase (something that this was meant to facilitate avoiding). Returning a primtive struct(or a union) with a couple of fields could easily help to avoid that. - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/#review94177 --- On Aug. 5, 2015, 10:10 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36979/ --- (Updated Aug. 5, 2015, 10:10 a.m.) Review request for mesos, Benjamin Hindman and Artem Harutyunyan. Bugs: MESOS-3142 https://issues.apache.org/jira/browse/MESOS-3142 Repository: mesos Description --- Updating all references to os::shell For more details see MESOS-3142. Diffs - src/hdfs/hdfs.hpp a070c3200f0a0ac48ec86451749c7faf10c7f6a7 src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce src/slave/containerizer/isolators/network/port_mapping.cpp 8244c345b84108af7fa18d20e71401d6e1a0aeb0 src/tests/containerizer/isolator_tests.cpp ff6e2b7e190a58a4809d6e71addb15dabe418e17 src/tests/containerizer/port_mapping_tests.cpp 4bee74acba2b1472c80cabbc9d0384bd04c543aa Diff: https://reviews.apache.org/r/36979/diff/ Testing --- make check *Note*: this patch fixes breakages introduce by the refactoring in: https://reviews.apache.org/r/36978 Thanks, Marco Massenzio
