On 二月 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. > > Travis Hegner wrote: > 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.
For 1), I think that you can take a look at https://reviews.apache.org/r/42516/diff/11#2 for how to handle different versions. Yes, checking version may be better, if version >= 1.9.1, use new way; otherwise, use old way. For 2) You may want to supply a sample inspect output for the various versions as part of the test and then validate the code. - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43093/#review118436 ----------------------------------------------------------- On 二月 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 二月 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 > >
