mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Hindman <b...@berkeley.edu>
Subject Re: Review Request 52473: Added nested MesosContainerizer tests.
Date Tue, 04 Oct 2016 01:26:09 GMT


> On Oct. 3, 2016, 9:30 p.m., Jie Yu wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp, line 426
> > <https://reviews.apache.org/r/52473/diff/1/?file=1518369#file1518369line426>
> >
> >     dash (default shell on ubuntu) does not support double digit file descriptor.
This is test may be flaky on some linux distributions. We should force to use 'bash' which
does not have this issue.
> >     https://bugs.launchpad.net/ubuntu/+source/dash/+bug/249620
> >     
> >     (BTW, we ran into this issue during debugging and I spent on like 4 hours debugging
this issue)
> >     
> >     To force to use 'bash', you need to use the non shell version of CommandInfo:
> >     ```
> >     shell=false
> >     value=bash
> >     arguments=['bash', '-c', 'read key <&x']
> >     ```

Let's create a `createCommandInfo` in another review as suggested in the next comment.


> On Oct. 3, 2016, 9:30 p.m., Jie Yu wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp, line 662
> > <https://reviews.apache.org/r/52473/diff/1/?file=1518369#file1518369line662>
> >
> >     Why reset here given you'll reset below? Please fix all other occurances.

I added a comment here for why we need to do two resets.


> On Oct. 3, 2016, 9:30 p.m., Jie Yu wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp, lines 750-752
> > <https://reviews.apache.org/r/52473/diff/1/?file=1518369#file1518369line750>
> >
> >     I don't like the current way of simulating a launcher orphan. First, it's linux
launcher specific. It uses some internal impl. details in the linux launcher (painful to update
when that layout changes). In the future, I think we should still parameterize the tests in
this file to use both posix launcher or linux launcher.
> >     
> >     To simulate a launcher orphan, i think we should launch container, and then
wipe the runtime dir.

Agreed, but I'm going to postpone this to a subsequent review.


- Benjamin


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


On Oct. 3, 2016, 3:28 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52473/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2016, 3:28 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added nested MesosContainerizer tests.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am c897d863bde284de41c99ed50ccbbdfd2dcad23b 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 8b5a19e121ef74eaf99b39682f8fd170605bf56d

>   src/tests/containerizer/nested_container_tests.cpp 8430823d54d132bc3e9c3e0781f027072999683e

>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


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