> On Feb. 11, 2016, 5:34 p.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > <https://reviews.apache.org/r/42516/diff/9/?file=1239370#file1239370line529>
> >
> >     Also check whether `network_name()` is empty string `""`.
> 
> Ezra Silvera wrote:
>     I think we already covered this issue in previous review comments. Is 
> there anything special you think is missing ?
>     
>     Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)
>     
>         Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
>         We added a new value "USER" which is identical in terms of code flow 
> to HOST and NONE so I'm not sure we should add a special test for USER while 
> all other values are not tested.
>         Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
>         The same is true for port mapping - maybe we had to better clarify it 
> on the comments - we didn't change any functionality at all and didn't touch 
> any code related to port mapping or network functionality. We are simply 
> passing a new value for the  --net parameter to the docker engine, nothing 
> else has changed.
>         What do you think?
> 
> Ezra Silvera wrote:
>     My comment was for teh testing. We will add a test for "" string (we 
> actually started with that and switch to the current test due to review 
> comments. :-)

For the test, using user-defined network is fine (similar to test using 
BRIDGE). For the empty string, current code is only check whether 
`network_name()` is set; if it's set to `""`, I'm not sure the behaviour but I 
think we it's better to return error before launching docker command line with 
empty string.


- Klaus


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


On Feb. 10, 2016, 4:34 p.m., Ezra Silvera wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
>     https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Signed-off-by: Ezra Silvera <e...@il.ibm.com>
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> -------
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork...."
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Reply via email to