From b862fc5a50b8094336241c5e0341cae1fcac67f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Wed, 17 Aug 2016 12:30:42 +0200 Subject: [PATCH] [buildinstall] Fix cleaning output dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ář --- pungi/phases/buildinstall.py | 48 +++++++++++++++++---------------- tests/test_buildinstall.py | 52 ++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 23 deletions(-) diff --git a/pungi/phases/buildinstall.py b/pungi/phases/buildinstall.py index ff5a05f9..766c73da 100644 --- a/pungi/phases/buildinstall.py +++ b/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"], - self.compose.conf["release_version"], - self.compose.conf["release_version"], - repo_baseurl, - os.path.join(output_dir, variant.uid), - variant=variant.uid, - buildinstallpackages=variant.buildinstallpackages, - is_final=self.compose.supported, - buildarch=buildarch, - volid=volid, - nomacboot=nomacboot, - bugurl=bugurl, - noupgrade=noupgrade) + lorax_cmd = lorax.get_lorax_cmd(self.compose.conf["release_name"], + self.compose.conf["release_version"], + self.compose.conf["release_version"], + repo_baseurl, + output_dir, + variant=variant.uid, + buildinstallpackages=variant.buildinstallpackages, + is_final=self.compose.supported, + buildarch=buildarch, + volid=volid, + nomacboot=nomacboot, + bugurl=bugurl, + 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 os.listdir(output_dir): - # output dir is *not* empty -> SKIP - self.pool.log_warning("[SKIP ] %s" % msg) - return - else: - # output dir is empty -> remove it and run buildinstall - self.pool.log_debug("Removing existing (but empty) buildinstall dir: %s" % output_dir) - os.rmdir(output_dir) + if variant: + output_dir = os.path.join(output_dir, variant.uid) + + if os.path.isdir(output_dir) and os.listdir(output_dir): + # output dir is *not* empty -> SKIP + self.pool.log_warning( + '[SKIP ] Buildinstall for arch %s, variant %s' % (arch, variant)) + return self.pool.log_info("[BEGIN] %s" % msg) diff --git a/tests/test_buildinstall.py b/tests/test_buildinstall.py index a92119bd..2279d670 100755 --- a/tests/test_buildinstall.py +++ b/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):