mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Niklas Nielsen" <...@qni.dk>
Subject Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.
Date Mon, 19 Oct 2015 23:22:42 GMT

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

Ship it!


See comments below :)


src/tests/containerizer/docker_containerizer_tests.cpp (line 504)
<https://reviews.apache.org/r/39388/#comment161157>

    Mind adding a comment about the test sequence? Why does the 'test $LIBPROCESS_IP = \"foobar"\'
work, etc.



src/tests/containerizer/docker_containerizer_tests.cpp (lines 509 - 510)
<https://reviews.apache.org/r/39388/#comment161155>

    No biggie; but could we have done the creation of the mock docker inline in line 512?
The first thing that came to my mind was that we were leaking the object.



src/tests/containerizer/docker_containerizer_tests.cpp (line 518)
<https://reviews.apache.org/r/39388/#comment161156>

    Can we make this assumption (do we have other places where we assume localhost is always
127.0.0.1)?


- Niklas Nielsen


On Oct. 17, 2015, 4:18 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2015, 4:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
>     https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 4bb65afd0ee61cafef68e064a697fdce65d60058

> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> -------
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which fails without
the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>


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