ACK/APPLIED/Cmnt: [ACT][PATCH] UBUNTU: SAUCE: ubuntu_lxc: make sure to cleanup containers on exit

[Andrea Righi]

On Fri, Feb 25, 2022 at 11:24:21AM +0800, Po-Hsu Lin wrote:
> 
> On Fri, Feb 25, 2022 at 12:19 AM Andrea Righi <andrea.righi at canonical.com> wrote:
> >
> > Some containers may still exist even when the test completes. This can
> > introduce some false positive failures on systems that are not
> > reprovisioned between a run and another (e.g., s390x).
> >
> > Introduce a cleanup routine to explicitly remove some containers that
> > may be not cleaned up properly ('device_add_remove_test' and
> > 'mount_injection_test').
> >
> > Signed-off-by: Andrea Righi <andrea.righi at canonical.com>
> > ---
> >  ubuntu_lxc/ubuntu_lxc.py | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/ubuntu_lxc/ubuntu_lxc.py b/ubuntu_lxc/ubuntu_lxc.py
> > index 2e5d589b..e5f21da8 100644
> > --- a/ubuntu_lxc/ubuntu_lxc.py
> > +++ b/ubuntu_lxc/ubuntu_lxc.py
> > @@ -105,4 +105,15 @@ class ubuntu_lxc(test.test):
> >          cmd = fpath + test_name
> >          utils.system_output(cmd, retain_output=True)
> >
> > +    def cleanup(self, test_name):
> > +        if test_name == 'setup':
> > +            return
> > +
> > +        # Make sure to properly cleanup containers that may still exist if
> > +        # sub-tests are failing
> > +        leftover_containers = ('device_add_remove_test', 'mount_injection_test', )
> > +        for name in leftover_containers:
> > +            cmd = "lxc-destroy -f -n {0} 2>/dev/null || true".format(name)
> > +            utils.system(cmd)
> 
> Since we know what to cleanup, and this cleanup() will be triggered
> after each sub-test. I think this for loop can be changed to an if
> statement so it won't have to run this repeatedly:
> 
>     if test_name in leftover_containers:
>         cmd = "lxc-destroy -f -n {0} 2>/dev/null || true".format(test_name)
>         utils.system(cmd)

Oh this is way better! I didn't pay attention to the fact that cleanup
has test_name in the arguments. :)

Then we can definitely declare leftover_containers as global and do the
check that you suggested.

Thanks!
-Andrea

> 
> This patch itself is looking good. I will follow this up later.
> 
> Acked-by: Po-Hsu Lin <po-hsu.lin at canonical.com>
> Applied and pushed.
> 
> Thanks
> Sam
> 
> > +
> >  # vi:set ts=4 sw=4 expandtab syntax=python:
> > --
> > 2.34.1
> >
> >
> > --
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team