You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Till Toenshoff <to...@me.com> on 2014/04/30 17:36:31 UTC
Review Request 20893: Fixed test_containerizer.py to deliver proper
Termination when getting destroyed.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20893/
-----------------------------------------------------------
Review request for mesos and Ian Downes.
Repository: mesos-git
Description
-------
A destroy on the container caused the wait-lock within test_containerizer.py to result into a value-error when attempting to parse the exit-status. That in turn caused the Wait result Future<Termination> to get failed, triggering a crash within the new tear-down extensions within cluster.hpp.
Diffs
-----
src/examples/python/test_containerizer.py 7bebfee
Diff: https://reviews.apache.org/r/20893/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 20893: Fixed test_containerizer.py to deliver proper
Termination when getting destroyed.
Posted by Till Toenshoff <to...@me.com>.
> On April 30, 2014, 4:46 p.m., Ian Downes wrote:
> > src/examples/python/test_containerizer.py, line 337
> > <https://reviews.apache.org/r/20893/diff/2/?file=571608#file571608line337>
> >
> > Can you be more descriptive in this message?
> >
> > Would ValueError be raised from line 327 with the int(status)? if so, you could say something like "Could not parse termination status"
Yes, makes sense.
> On April 30, 2014, 4:46 p.m., Ian Downes wrote:
> > src/examples/python/test_containerizer.py, line 316
> > <https://reviews.apache.org/r/20893/diff/2/?file=571608#file571608line316>
> >
> > termination status is optional - it should be set to none
Ok
> On April 30, 2014, 4:46 p.m., Ian Downes wrote:
> > src/examples/python/test_containerizer.py, line 315
> > <https://reviews.apache.org/r/20893/diff/2/?file=571608#file571608line315>
> >
> > how about "Container killed"
Ok
> On April 30, 2014, 4:46 p.m., Ian Downes wrote:
> > src/examples/python/test_containerizer.py, line 127
> > <https://reviews.apache.org/r/20893/diff/2/?file=571608#file571608line127>
> >
> > Can you write the appropriate termination status when you destroy? then, wait will read that it's been destroyed, rather than having to infer it.
Yes, you are right, that is a bit of a hack. I will have to revise this - already got some ideas here;
One major problem of my implementation is the fact that I am serializing the wrong PID - so I am killing the wrong process within this script.
> On April 30, 2014, 4:46 p.m., Ian Downes wrote:
> > src/examples/python/test_containerizer.py, line 100
> > <https://reviews.apache.org/r/20893/diff/2/?file=571608#file571608line100>
> >
> > Not related to this commit, but where is the Environment from CommandInfo set for the command?
Omg, yes, I think that got lost in some refactorings of the ExternalContainerizer. Will definitely come up with a fix for that.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20893/#review41847
-----------------------------------------------------------
On April 30, 2014, 3:36 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20893/
> -----------------------------------------------------------
>
> (Updated April 30, 2014, 3:36 p.m.)
>
>
> Review request for mesos and Ian Downes.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> A destroy on the container caused the wait-lock within test_containerizer.py to result into a value-error when attempting to parse the exit-status. That in turn caused the Wait result Future<Termination> to get failed, triggering a crash within the new tear-down extensions within cluster.hpp.
>
>
> Diffs
> -----
>
> src/examples/python/test_containerizer.py 7bebfee
>
> Diff: https://reviews.apache.org/r/20893/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 20893: Fixed test_containerizer.py to deliver proper
Termination when getting destroyed.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20893/#review41847
-----------------------------------------------------------
Ship it!
src/examples/python/test_containerizer.py
<https://reviews.apache.org/r/20893/#comment75485>
Not related to this commit, but where is the Environment from CommandInfo set for the command?
src/examples/python/test_containerizer.py
<https://reviews.apache.org/r/20893/#comment75486>
Can you write the appropriate termination status when you destroy? then, wait will read that it's been destroyed, rather than having to infer it.
src/examples/python/test_containerizer.py
<https://reviews.apache.org/r/20893/#comment75477>
how about "Container killed"
src/examples/python/test_containerizer.py
<https://reviews.apache.org/r/20893/#comment75478>
termination status is optional - it should be set to none
src/examples/python/test_containerizer.py
<https://reviews.apache.org/r/20893/#comment75482>
Can you be more descriptive in this message?
Would ValueError be raised from line 327 with the int(status)? if so, you could say something like "Could not parse termination status"
- Ian Downes
On April 30, 2014, 8:36 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20893/
> -----------------------------------------------------------
>
> (Updated April 30, 2014, 8:36 a.m.)
>
>
> Review request for mesos and Ian Downes.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> A destroy on the container caused the wait-lock within test_containerizer.py to result into a value-error when attempting to parse the exit-status. That in turn caused the Wait result Future<Termination> to get failed, triggering a crash within the new tear-down extensions within cluster.hpp.
>
>
> Diffs
> -----
>
> src/examples/python/test_containerizer.py 7bebfee
>
> Diff: https://reviews.apache.org/r/20893/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>