----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37114/#review96408 -----------------------------------------------------------
Content-wise, LGTM. Style-wise, there's a bit of jaggedness in your function definitions and calls (marked a subset of them). See the "Function Definition/Invocation" section in http://mesos.apache.org/documentation/latest/mesos-c++-style-guide/ src/docker/docker.hpp (lines 49 - 50) <https://reviews.apache.org/r/37114/#comment151747> Indent of 4 spaces. src/docker/docker.cpp (lines 131 - 132) <https://reviews.apache.org/r/37114/#comment151748> Indent of 4 spaces. src/docker/docker.cpp (lines 682 - 683) <https://reviews.apache.org/r/37114/#comment151749> Indent of 4 spaces. src/slave/containerizer/docker.cpp (line 129) <https://reviews.apache.org/r/37114/#comment151750> Extra newline. src/tests/flags.hpp (line 83) <https://reviews.apache.org/r/37114/#comment151745> Extra space before `\n`. - Joseph Wu On Aug. 25, 2015, 12:27 p.m., Vaibhav Khanduja wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37114/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2015, 12:27 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/tests/containerizer/docker_containerizer_tests.cpp > 80ed60e2b0fa39e8302867a7cb6a7388c25f9a40 > src/tests/containerizer/docker_tests.cpp > a4a2725c05ae0cb88426c587f7ded0da77154edc > src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f > 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 ID IMAGE COMMAND CREATED > STATUS PORTS NAMES > 07fc4ec86bac mygoserver "/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 > >
