> On Feb. 3, 2016, 8:58 a.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 326
> > <https://reviews.apache.org/r/43093/diff/1/?file=1229714#file1229714line326>
> >
> >     If we could add some assert code about ip value in `TEST_F(DockerTest, 
> > ROOT_DOCKER_interface)` would make this patch looks better.
> >     
> >     ```
> >     TEST_F(DockerTest, ROOT_DOCKER_interface)
> >     ...
> >       // Test some fields of the container.
> >       EXPECT_NE("", inspect.get().id);
> >       EXPECT_EQ("/" + containerName, inspect.get().name);
> >       EXPECT_SOME(inspect.get().pid);
> >     ...
> >     ```

I am unsure of what you are looking for in this case. I can't really assert any 
of the fields related to this patch, as they will have different values or not 
exist depending on the docker API version being queried.

With the new API, "NetworkSettings.IPAddress" is expected to be empty (perhaps 
not exist in the future), but not with the old API. Similarly, with the old API 
"NetworkSettings.Networks.<network_name>.IPAddress" will never exist, but 
exists in the new API.

Any testing of these fields would be mutually exclusive.


- Travis


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


On Feb. 3, 2016, 7:04 p.m., Travis Hegner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2016, 7:04 p.m.)
> 
> 
> Review request for mesos 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 a831726 
> 
> 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