mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Park" <mcyp...@gmail.com>
Subject Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.
Date Tue, 20 Oct 2015 00:43:06 GMT


> On Oct. 19, 2015, 11:22 p.m., Niklas Nielsen wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, line 504
> > <https://reviews.apache.org/r/39388/diff/2/?file=1100524#file1100524line504>
> >
> >     Mind adding a comment about the test sequence? Why does the 'test $LIBPROCESS_IP
= \"foobar"\' work, etc.

Yep, will do.


> On Oct. 19, 2015, 11:22 p.m., Niklas Nielsen wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, lines 509-510
> > <https://reviews.apache.org/r/39388/diff/2/?file=1100524#file1100524line509>
> >
> >     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.

Probably, I'll look into it a bit more. I followed the pattern of existing tests so I'll perhaps
update those correspondingly.


> On Oct. 19, 2015, 11:22 p.m., Niklas Nielsen wrote:
> > src/tests/containerizer/docker_containerizer_tests.cpp, line 518
> > <https://reviews.apache.org/r/39388/diff/2/?file=1100524#file1100524line518>
> >
> >     Can we make this assumption (do we have other places where we assume localhost
is always 127.0.0.1)?

The choice of `127.0.0.1` is arbitrary here. We could've assigned it to `x`, and performed
`test $LIBPROCESS_IP = x` instead.
Actually, I'll pull it out to a variable and leave a comment in line with what I described
here.


- Michael


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


On Oct. 17, 2015, 11: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, 11: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