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. > > 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. > > Guangya Liu wrote: > 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.
I'm glad you sent that link. The progress for supporting user defined networks will change this patch completely. We are using user defined networks currently, and that is the point of this patch that I'm working on. Your link just helped me to realize that mesos still believes that our containers are running in bridged mode, while docker believes we are using user defined networks. Basically, we are using the "parameters" field in marathon to set up our custom "--net" parameter, even though marathon also passes "--net=bridge". Mesos seems to be honoring the first "--net" parameter, while docker honors the second. I should really wait for that patch, or work with that author to ensure it will work for our environment as well. - 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 > >
