mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guangya Liu <gyliu...@gmail.com>
Subject Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker
Date Fri, 12 Feb 2016 14:17:50 GMT


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message