[buildinstall] Fix cleaning output dir

Before the task is started, the output directory is checked and if it
exists and is not empty, the runroot task will be skipped. This is meant
for debugging when restarting the same compose. Under usual
circumstances, the directory will not be created in the first place.

The runroot task will start by removing the output directory. This way,
if koji restarts the task, lorax will not fail.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

pungi/phases/buildinstall.py

@@ -104,20 +104,23 @@ class BuildinstallPhase(PhaseBase):
                 bugurl = data.get('bugurl')
             if not data.get('nomacboot', True):
                 nomacboot = False
+        output_dir = os.path.join(output_dir, variant.uid)
         lorax = LoraxWrapper()
-        return lorax.get_lorax_cmd(self.compose.conf["release_name"],
+        lorax_cmd = lorax.get_lorax_cmd(self.compose.conf["release_name"],
-                                   self.compose.conf["release_version"],
+                                        self.compose.conf["release_version"],
-                                   self.compose.conf["release_version"],
+                                        self.compose.conf["release_version"],
-                                   repo_baseurl,
+                                        repo_baseurl,
-                                   os.path.join(output_dir, variant.uid),
+                                        output_dir,
-                                   variant=variant.uid,
+                                        variant=variant.uid,
-                                   buildinstallpackages=variant.buildinstallpackages,
+                                        buildinstallpackages=variant.buildinstallpackages,
-                                   is_final=self.compose.supported,
+                                        is_final=self.compose.supported,
-                                   buildarch=buildarch,
+                                        buildarch=buildarch,
-                                   volid=volid,
+                                        volid=volid,
-                                   nomacboot=nomacboot,
+                                        nomacboot=nomacboot,
-                                   bugurl=bugurl,
+                                        bugurl=bugurl,
-                                   noupgrade=noupgrade)
+                                        noupgrade=noupgrade)
+        return 'rm -rf %s && %s' % (pipes.quote(output_dir),
+                                    ' '.join([pipes.quote(x) for x in lorax_cmd]))
 
     def run(self):
         lorax = LoraxWrapper()
@@ -394,18 +397,17 @@ class BuildinstallThread(WorkerThread):
         log_filename = ('buildinstall-%s' % variant.uid) if variant else 'buildinstall'
         log_file = compose.paths.log.log_file(arch, log_filename)
 
-        msg = "Running buildinstall for arch %s" % arch
+        msg = "Running buildinstall for arch %s, variant %s" % (arch, variant)
 
         output_dir = compose.paths.work.buildinstall_dir(arch)
-        if os.path.isdir(output_dir):
+        if variant:
-            if os.listdir(output_dir):
+            output_dir = os.path.join(output_dir, variant.uid)
-                # output dir is *not* empty -> SKIP
+
-                self.pool.log_warning("[SKIP ] %s" % msg)
+        if os.path.isdir(output_dir) and os.listdir(output_dir):
-                return
+            # output dir is *not* empty -> SKIP
-            else:
+            self.pool.log_warning(
-                # output dir is empty -> remove it and run buildinstall
+                '[SKIP ] Buildinstall for arch %s, variant %s' % (arch, variant))
-                self.pool.log_debug("Removing existing (but empty) buildinstall dir: %s" % output_dir)
+            return
-                os.rmdir(output_dir)
 
         self.pool.log_info("[BEGIN] %s" % msg)
 

tests/test_buildinstall.py

@@ -65,6 +65,7 @@ class TestBuildinstallPhase(PungiTestCase):
         })
 
         get_volid.return_value = 'vol_id'
+        loraxCls.return_value.get_lorax_cmd.return_value = ['lorax', '...']
 
         phase = BuildinstallPhase(compose)
 
@@ -74,6 +75,11 @@ class TestBuildinstallPhase(PungiTestCase):
         # Server.x86_64, Client.amd64, Server.x86_64
         pool = poolCls.return_value
         self.assertEqual(3, len(pool.queue_put.mock_calls))
+        self.assertItemsEqual(
+            [call[0][0][3] for call in pool.queue_put.call_args_list],
+            ['rm -rf %s/work/amd64/buildinstall/Client && lorax ...' % self.topdir,
+             'rm -rf %s/work/amd64/buildinstall/Server && lorax ...' % self.topdir,
+             'rm -rf %s/work/x86_64/buildinstall/Server && lorax ...' % self.topdir])
 
         # Obtained correct lorax commands.
         self.assertItemsEqual(
@@ -114,6 +120,7 @@ class TestBuildinstallPhase(PungiTestCase):
 
         get_volid.return_value = 'vol_id'
         compose.variants['Server'].is_empty = True
+        loraxCls.return_value.get_lorax_cmd.return_value = ['lorax', '...']
 
         phase = BuildinstallPhase(compose)
 
@@ -121,6 +128,9 @@ class TestBuildinstallPhase(PungiTestCase):
 
         pool = poolCls.return_value
         self.assertEqual(1, len(pool.queue_put.mock_calls))
+        self.assertItemsEqual(
+            [call[0][0][3] for call in pool.queue_put.call_args_list],
+            ['rm -rf %s/work/amd64/buildinstall/Client && lorax ...' % self.topdir])
 
         # Obtained correct lorax command.
         lorax = loraxCls.return_value
@@ -226,6 +236,7 @@ class TestBuildinstallPhase(PungiTestCase):
         })
 
         get_volid.return_value = 'vol_id'
+        loraxCls.return_value.get_lorax_cmd.return_value = ['lorax', '...']
 
         phase = BuildinstallPhase(compose)
 
@@ -235,6 +246,11 @@ class TestBuildinstallPhase(PungiTestCase):
         # Server.x86_64, Client.amd64, Server.x86_64
         pool = poolCls.return_value
         self.assertEqual(3, len(pool.queue_put.mock_calls))
+        self.assertItemsEqual(
+            [call[0][0][3] for call in pool.queue_put.call_args_list],
+            ['rm -rf %s/work/amd64/buildinstall/Client && lorax ...' % self.topdir,
+             'rm -rf %s/work/amd64/buildinstall/Server && lorax ...' % self.topdir,
+             'rm -rf %s/work/x86_64/buildinstall/Server && lorax ...' % self.topdir])
 
         # Obtained correct lorax commands.
         self.assertItemsEqual(
@@ -280,6 +296,7 @@ class TestBuildinstallPhase(PungiTestCase):
         })
 
         get_volid.return_value = 'vol_id'
+        loraxCls.return_value.get_lorax_cmd.return_value = ['lorax', '...']
 
         phase = BuildinstallPhase(compose)
 
@@ -289,6 +306,11 @@ class TestBuildinstallPhase(PungiTestCase):
         # Server.x86_64, Client.amd64, Server.x86_64
         pool = poolCls.return_value
         self.assertEqual(3, len(pool.queue_put.mock_calls))
+        self.assertItemsEqual(
+            [call[0][0][3] for call in pool.queue_put.call_args_list],
+            ['rm -rf %s/work/amd64/buildinstall/Client && lorax ...' % self.topdir,
+             'rm -rf %s/work/amd64/buildinstall/Server && lorax ...' % self.topdir,
+             'rm -rf %s/work/x86_64/buildinstall/Server && lorax ...' % self.topdir])
 
         # Obtained correct lorax commands.
         self.assertItemsEqual(
@@ -601,6 +623,36 @@ class BuildinstallThreadTestCase(PungiTestCase):
             mock.call('Runroot task failed: 1234. See %s/logs/x86_64/buildinstall-Server.x86_64.log for more details.' % self.topdir)
         ])
 
+    @mock.patch('pungi.phases.buildinstall.KojiWrapper')
+    @mock.patch('pungi.phases.buildinstall.get_buildroot_rpms')
+    @mock.patch('pungi.phases.buildinstall.run')
+    def test_skips_on_existing_output_dir(self, run, get_buildroot_rpms, KojiWrapperMock):
+        compose = BuildInstallCompose(self.topdir, {
+            'buildinstall_method': 'lorax',
+            'runroot': True,
+            'runroot_tag': 'rrt',
+            'koji_profile': 'koji',
+            'failable_deliverables': [
+                ('^.+$', {'*': ['buildinstall']})
+            ],
+        })
+
+        get_buildroot_rpms.return_value = ['bash', 'zsh']
+        pool = mock.Mock()
+        cmd = mock.Mock()
+
+        dummy_file = os.path.join(self.topdir, 'work/x86_64/buildinstall/Server/dummy')
+        touch(os.path.join(dummy_file))
+
+        t = BuildinstallThread(pool)
+
+        with mock.patch('time.sleep'):
+            t.process((compose, 'x86_64', compose.variants['Server'], cmd), 0)
+
+        self.assertEqual(0, len(run.mock_calls))
+
+        self.assertTrue(os.path.exists(dummy_file))
+
 
 class TestSymlinkIso(PungiTestCase):