diff mbox series

[ACT] UBUNTU: SAUCE: ubuntu_lxc: destroy leftover container before tests

Message ID 20210712105936.1937440-1-kleber.souza@canonical.com
State New
Headers show
Series [ACT] UBUNTU: SAUCE: ubuntu_lxc: destroy leftover container before tests | expand

Commit Message

Kleber Sacilotto de Souza July 12, 2021, 10:59 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1788574

Commits 1a50e77ca803 ("UBUNTU: SAUCE: ubuntu_lxc: remove the reboot
container after test") and 298cd01e ("UBUNTU: SAUCE: ubuntu_lxc: destroy
the leftover container before exit") fixed the issue with the leftover
"reboot" contained on the 'exercise' script. However, this script is run
only for series older than artful. For newer series, the testcase is run
from the source package via 'autopkgtest'. Fix it by destroying the
container before running any test.

Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
---
 ubuntu_lxc/ubuntu_lxc.py | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Krzysztof Kozlowski July 12, 2021, 3:37 p.m. UTC | #1
On 12/07/2021 12:59, Kleber Sacilotto de Souza wrote:
> BugLink: https://bugs.launchpad.net/bugs/1788574
> 
> Commits 1a50e77ca803 ("UBUNTU: SAUCE: ubuntu_lxc: remove the reboot
> container after test") and 298cd01e ("UBUNTU: SAUCE: ubuntu_lxc: destroy
> the leftover container before exit") fixed the issue with the leftover
> "reboot" contained on the 'exercise' script. However, this script is run
> only for series older than artful. For newer series, the testcase is run
> from the source package via 'autopkgtest'. Fix it by destroying the
> container before running any test.
> 
> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
> ---
>  ubuntu_lxc/ubuntu_lxc.py | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/ubuntu_lxc/ubuntu_lxc.py b/ubuntu_lxc/ubuntu_lxc.py
> index e201b0d4..cc8466ba 100644
> --- a/ubuntu_lxc/ubuntu_lxc.py
> +++ b/ubuntu_lxc/ubuntu_lxc.py
> @@ -44,6 +44,12 @@ class ubuntu_lxc(test.test):
>      def run_once(self, test_name):
>          if test_name == 'setup':
>              return
> +
> +        # Destroy the "reboot" container which might have been left
> +        # behind (LP#1788574)
> +        cmd = 'lxc-destroy reboot'
> +        utils.system(cmd, ignore_status=True)
> +
>          if self.series in ['precise', 'trusty', 'xenial', 'artful']:
>              cmd = '/bin/sh %s/exercise' % self.bindir
>          else:
> 

Maybe it should be rather at the end of test in try-finally block to
leave the machine/container in a clean state? But I don't have any
strong preference or arguments, so:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof
Kleber Sacilotto de Souza July 12, 2021, 4:30 p.m. UTC | #2
On 12.07.21 17:37, Krzysztof Kozlowski wrote:
> On 12/07/2021 12:59, Kleber Sacilotto de Souza wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1788574
>>
>> Commits 1a50e77ca803 ("UBUNTU: SAUCE: ubuntu_lxc: remove the reboot
>> container after test") and 298cd01e ("UBUNTU: SAUCE: ubuntu_lxc: destroy
>> the leftover container before exit") fixed the issue with the leftover
>> "reboot" contained on the 'exercise' script. However, this script is run
>> only for series older than artful. For newer series, the testcase is run
>> from the source package via 'autopkgtest'. Fix it by destroying the
>> container before running any test.
>>
>> Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
>> ---
>>   ubuntu_lxc/ubuntu_lxc.py | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/ubuntu_lxc/ubuntu_lxc.py b/ubuntu_lxc/ubuntu_lxc.py
>> index e201b0d4..cc8466ba 100644
>> --- a/ubuntu_lxc/ubuntu_lxc.py
>> +++ b/ubuntu_lxc/ubuntu_lxc.py
>> @@ -44,6 +44,12 @@ class ubuntu_lxc(test.test):
>>       def run_once(self, test_name):
>>           if test_name == 'setup':
>>               return
>> +
>> +        # Destroy the "reboot" container which might have been left
>> +        # behind (LP#1788574)
>> +        cmd = 'lxc-destroy reboot'
>> +        utils.system(cmd, ignore_status=True)
>> +
>>           if self.series in ['precise', 'trusty', 'xenial', 'artful']:
>>               cmd = '/bin/sh %s/exercise' % self.bindir
>>           else:
>>
> 
> Maybe it should be rather at the end of test in try-finally block to
> leave the machine/container in a clean state? But I don't have any
> strong preference or arguments, so:

I thought of doing it before to make sure that if any previous run that
might have left the container behind would not impact the current run.
But if we can guarantee that doing it after the test is run would
*always* clean it up then it sounds like a better approach. The only
issue would be that the first time we run this script with the fix it
will still fail, but that wouldn't be a big deal.

> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 
> 
> Best regards,
> Krzysztof
>
Po-Hsu Lin July 13, 2021, 11:31 a.m. UTC | #3
Nice catch.
Maybe we can remove the code piece in the exercise script in the
future. With ignore_status set to True it will be fine even if
the reboot container does not exist.

Acked-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Applied and pushed.


Thanks
Sam
diff mbox series

Patch

diff --git a/ubuntu_lxc/ubuntu_lxc.py b/ubuntu_lxc/ubuntu_lxc.py
index e201b0d4..cc8466ba 100644
--- a/ubuntu_lxc/ubuntu_lxc.py
+++ b/ubuntu_lxc/ubuntu_lxc.py
@@ -44,6 +44,12 @@  class ubuntu_lxc(test.test):
     def run_once(self, test_name):
         if test_name == 'setup':
             return
+
+        # Destroy the "reboot" container which might have been left
+        # behind (LP#1788574)
+        cmd = 'lxc-destroy reboot'
+        utils.system(cmd, ignore_status=True)
+
         if self.series in ['precise', 'trusty', 'xenial', 'artful']:
             cmd = '/bin/sh %s/exercise' % self.bindir
         else: