On Feb. 9, 2016, 7:13 p.m., Travis Hegner wrote:
> > hanks
> 
> Joerg Schad wrote:
>     Oh btw regarding the commit message:
>     We usually have commit messages stating what changed, so in your case it 
> could be something along the lines of 'Added support for new docker network 
> setting.'
> 
> Travis Hegner wrote:
>     Understood. I will take care of both of these.

A couple of quick questions if I may:

1. Would it be possible or advisable to refactor this code based off of a 
detected docker version? I saw some tests with regard to the docker version, 
but I'm not familiar enough with the project to know if that is a route to go. 
Essentially, if docker version > 1.9.1 use new API otherwise use the old one? 
Perhaps there is a detection for the actual API version as well?
2. How would one test the various execution paths? Would I have to supply a 
sample inspect output for the various versions as part of the test, and then 
validate how the code handles each of the different version samples? I don't 
know how else one would test each possible condition. I did see that some tests 
supplied a sample json blob to test against... would that be advisable in this 
case?
3. Should I be basing this patch off of the master branch, or the latest 
release? If it makes a difference, I am hopeful to get this patch into 0.27.1, 
if it's not already too late for that.


- Travis


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


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
>     https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> -------
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net <name> to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks.<name>.IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Reply via email to