Re: Review Request 13034: balloon_framework_test.sh: Tweak so the test runs properly

[Eric Biederman <eb...@xmission.com>]

> On July 30, 2013, 7:22 p.m., Vinod Kone wrote:
> > src/tests/balloon_framework_test.sh, lines 57-60
> > <https://reviews.apache.org/r/13034/diff/1/?file=330349#file330349line57>
> >
> >     Can you add a comment for why are doing this. You can pluck this from description in this RB. Also note that, this behavior of mesos failing on unrecognized flags might be changing in near future.
> 
> Vinod Kone wrote:
>     by unrecognized flags, i meant unrecognized MESOS_* variables in the environment.
> 
> Eric Biederman wrote:
>     This will continue to work if mesos stops failing on unrecognized MESOS_* environment variables so as a future change that is irrelevant here.
>     
>     Adding an extra comment seems reasonable as a follow on change.  I don't see it worth holding up a necessary bug fix, or invalidating the testing I have already done.
>
> 
> Vinod Kone wrote:
>     Hey Eric. Just to be clear I'm not suggesting to drop this change/fix. We need this to fix the test. I was suggesting you add a comment on why you were doing unsets. This would help other people understand the context when they look at the code at a later point in time.

I will be happy to add a comment just not in this patch/reviewboard.  As respinning the patch when there are no bugs and invalidating all of the testing I have done just to add a comment that makes the code marginally more readable is silly.

I have seen way to many pieces of code destroyed by people making suggested changes and not bothering to rerun all of their tests because they know the changes won't affect anything.

I will send a follow on change to add the missing comment.  Does that sound good?


- Eric


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


On July 29, 2013, 9:57 p.m., Eric Biederman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13034/
> -----------------------------------------------------------
> 
> (Updated July 29, 2013, 9:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> balloon_framework_test.sh: Tweak so the test runs properly
> 
> Change the location of the mesos hierarchy and the mesos root directory
> to match other tests.
> 
> Stop exporting variables that begin with MESOS_ into the mesos environment.
> mesos-master and mesos-slave will not start if their are unrecognized variables
> that begin with MESOS_ in the environment.
> 
> Set the LD_LIBRARY_PATH so that the balloon binaries built with libtool
> will be able to find all of their pieces and execute proper without
> needing to be installed.
> 
> Cleanup the cleanup code so we don't unmount a cgroup hierarchy unless we have
> fully cleaned it up.  This is necessary to prevent a problem where the hierarchy
> can never be mounted again.
> 
> Update the command line options to mesos-master to pass the hierarchy and
> the mesos root directory in that hierarchy.
> 
> Clearly report when the balloon hierarchy returns with an unexpected return
> code.  Indicating the test failed because of a problem in the baloon framework
> or within mesos.
> 
> With this set of changes the test is running properly. And when not tripping
> over kernel bugs the test appears to pass.
> 
> 
> Diffs
> -----
> 
>   src/tests/balloon_framework_test.sh 4309f40 
> 
> Diff: https://reviews.apache.org/r/13034/diff/
> 
> 
> Testing
> -------
> 
> su -c 'make check' and watched the kernel crash!
> 
> 
> Thanks,
> 
> Eric Biederman
> 
>