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



src/docker/docker.hpp (line 44)
<https://reviews.apache.org/r/37114/#comment151677>

    End comment with period.



src/docker/docker.hpp (line 46)
<https://reviews.apache.org/r/37114/#comment151678>

    Align this with the last line (one space to the left), also if you look at 
the next method example of our default values in parameters, follow the same 
spacing (space between = and ")



src/docker/docker.hpp (line 161)
<https://reviews.apache.org/r/37114/#comment151680>

    Format spacing correctly, look at other examples in the code.



src/docker/docker.cpp (line 99)
<https://reviews.apache.org/r/37114/#comment151682>

    Format spacing



src/docker/docker.cpp (line 101)
<https://reviews.apache.org/r/37114/#comment151683>

    End comment with period.



src/docker/docker.cpp (line 102)
<https://reviews.apache.org/r/37114/#comment151689>

    Also does this mean if a user put in tcp:/// we're just padding unix:///?
    I think it's safe to assume we should always expect a absolute path to the 
socket, and I'm not sure what other prefix are we allowing here by allowing a 
custom prefix to be passed in.



src/docker/docker.cpp (line 105)
<https://reviews.apache.org/r/37114/#comment151684>

    two spaces



src/docker/docker.cpp (line 107)
<https://reviews.apache.org/r/37114/#comment151685>

    Why paranethsis? Also spacing



src/docker/docker.cpp (line 663)
<https://reviews.apache.org/r/37114/#comment151690>

    Correct formating by moving paranethis aligning with last line



src/docker/executor.hpp (line 45)
<https://reviews.apache.org/r/37114/#comment151691>

    s/host/socket/g


- Timothy Chen


On Aug. 21, 2015, 8:54 p.m., Vaibhav Khanduja wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37114/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2015, 8:54 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
> 
>

Reply via email to