> On July 30, 2013, 7:22 p.m., Vinod Kone wrote: > > src/tests/balloon_framework_test.sh, lines 54-56 > > <https://reviews.apache.org/r/13034/diff/1/?file=330349#file330349line54> > > > > Any reason why you extracted these into variables? Also, if extracting > > is necessary, we prefer to declare them close to where they are used. > > Eric Biederman wrote: > It is necessary to create variables and to do it here. MESOS_BUILD_DIR > and MESOS_SOURCE_DIR must be unset, and this is the last place we can use > them before they can be unset. >
gotcha. > 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. > 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. - Vinod ----------------------------------------------------------- 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 > >
