diff --git a/0001-Ensure-all-phases-are-stopped.patch b/0001-Ensure-all-phases-are-stopped.patch new file mode 100644 index 00000000..edeae546 --- /dev/null +++ b/0001-Ensure-all-phases-are-stopped.patch @@ -0,0 +1,190 @@ +From 860124c7d3581a6f268d313420aed27e7218b205 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= +Date: Mon, 29 May 2017 09:56:23 +0200 +Subject: [PATCH] Ensure all phases are stopped +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +If a phase is started successfully, it needs to be stopped as well. In +most cases when `stop` is called immediately after `start`, this is not +a problem. + +Only when something else happens while a phase is runnning and this +something fails, Pungi will deadlock and never exit. This something +could be another phase or just main thread raising an exception. + +Fixes: #625 +Signed-off-by: Lubomír Sedlář +--- + bin/pungi-koji | 48 ++++++++++++++++++++--------------------------- + pungi/phases/__init__.py | 18 ++++++++++++++++++ + tests/test_phase_base.py | 49 ++++++++++++++++++++++++++++++++++++++++++++++-- + 3 files changed, 85 insertions(+), 30 deletions(-) + +diff --git a/bin/pungi-koji b/bin/pungi-koji +index c9a3654..480618d 100755 +--- a/bin/pungi-koji ++++ b/bin/pungi-koji +@@ -354,25 +354,25 @@ def run_compose(compose, create_latest_link=True, latest_link_status=None): + pkgset_phase.start() + pkgset_phase.stop() + +- # BUILDINSTALL phase - start ++ # BUILDINSTALL phase - start, we can run gathering, extra files and ++ # createrepo while buildinstall is in progress. + buildinstall_phase.start() + +- # GATHER phase +- gather_phase.start() +- gather_phase.stop() ++ # If any of the following three phases fail, we must ensure that ++ # buildinstall is stopped. Otherwise the whole process will hang. ++ try: ++ gather_phase.start() ++ gather_phase.stop() ++ ++ extrafiles_phase.start() ++ extrafiles_phase.stop() + +- # EXTRA_FILES phase +- extrafiles_phase.start() +- extrafiles_phase.stop() ++ createrepo_phase.start() ++ createrepo_phase.stop() + +- # CREATEREPO phase +- createrepo_phase.start() +- createrepo_phase.stop() ++ finally: ++ buildinstall_phase.stop() + +- # BUILDINSTALL phase +- # must finish before PRODUCTIMG +- # must finish before CREATEISO +- buildinstall_phase.stop() + if not buildinstall_phase.skip(): + buildinstall_phase.copy_files() + +@@ -396,20 +396,12 @@ def run_compose(compose, create_latest_link=True, latest_link_status=None): + timestamp = pungi.metadata.write_discinfo(compose, arch, variant) + pungi.metadata.write_media_repo(compose, arch, variant, timestamp) + +- # CREATEISO and LIVEIMAGES phases +- createiso_phase.start() +- liveimages_phase.start() +- image_build_phase.start() +- livemedia_phase.start() +- ostree_installer_phase.start() +- osbs_phase.start() +- +- createiso_phase.stop() +- liveimages_phase.stop() +- image_build_phase.stop() +- livemedia_phase.stop() +- ostree_installer_phase.stop() +- osbs_phase.stop() ++ # Start all phases for image artifacts ++ pungi.phases.run_all([createiso_phase, ++ liveimages_phase, ++ image_build_phase, ++ ostree_installer_phase, ++ osbs_phase]) + + image_checksum_phase.start() + image_checksum_phase.stop() +diff --git a/pungi/phases/__init__.py b/pungi/phases/__init__.py +index e0db601..044aa46 100644 +--- a/pungi/phases/__init__.py ++++ b/pungi/phases/__init__.py +@@ -31,3 +31,21 @@ from .livemedia_phase import LiveMediaPhase # noqa + from .ostree import OSTreePhase # noqa + from .ostree_installer import OstreeInstallerPhase # noqa + from .osbs import OSBSPhase # noqa ++ ++ ++def run_all(phases): ++ """Start and stop all given phases and make them run in parallel. ++ ++ This function makes sure that even if one of the phases fails and raises an ++ exception, all the phases will still be correctly stopped. ++ ++ If multiple phases fail, a exception from one of them will be raised, but ++ there are no guarantees which one it will be. ++ """ ++ if not phases: ++ return ++ phases[0].start() ++ try: ++ run_all(phases[1:]) ++ finally: ++ phases[0].stop() +diff --git a/tests/test_phase_base.py b/tests/test_phase_base.py +index ea2992d..d0588fe 100644 +--- a/tests/test_phase_base.py ++++ b/tests/test_phase_base.py +@@ -8,8 +8,8 @@ import sys + + sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +-from pungi.phases import base +-from tests.helpers import DummyCompose, PungiTestCase ++from pungi.phases import run_all, base ++from tests.helpers import DummyCompose, PungiTestCase, boom + + + class Phase1(base.ImageConfigMixin, base.PhaseBase): +@@ -69,5 +69,50 @@ class ImageConfigMixinTestCase(PungiTestCase): + self.assertEqual(resolve_git_url.num, 3, 'Resolver was not called three times') + + ++class TestRunAll(unittest.TestCase): ++ def assertFinalized(self, p): ++ self.assertEqual(p.mock_calls, [mock.call.start(), mock.call.stop()]) ++ ++ def test_calls_stop(self): ++ p1 = mock.Mock() ++ p2 = mock.Mock() ++ ++ run_all([p1, p2]) ++ ++ self.assertFinalized(p1) ++ self.assertFinalized(p2) ++ ++ def test_calls_stop_on_failure(self): ++ p1 = mock.Mock() ++ p2 = mock.Mock() ++ p3 = mock.Mock() ++ ++ p2.stop.side_effect = boom ++ ++ with self.assertRaises(Exception) as ctx: ++ run_all([p1, p2, p3]) ++ ++ self.assertEqual('BOOM', str(ctx.exception)) ++ self.assertFinalized(p1) ++ self.assertFinalized(p2) ++ self.assertFinalized(p3) ++ ++ def test_multiple_fail(self): ++ p1 = mock.Mock(name='p1') ++ p2 = mock.Mock(name='p2') ++ p3 = mock.Mock(name='p3') ++ ++ p2.stop.side_effect = boom ++ p3.stop.side_effect = boom ++ ++ with self.assertRaises(Exception) as ctx: ++ run_all([p1, p2, p3]) ++ ++ self.assertEqual('BOOM', str(ctx.exception)) ++ self.assertFinalized(p1) ++ self.assertFinalized(p2) ++ self.assertFinalized(p3) ++ ++ + if __name__ == "__main__": + unittest.main() +-- +2.9.4 + diff --git a/pungi.spec b/pungi.spec index ce6b7d5e..2786fd2a 100644 --- a/pungi.spec +++ b/pungi.spec @@ -1,12 +1,13 @@ Name: pungi Version: 4.1.15 -Release: 1%{?dist} +Release: 2%{?dist} Summary: Distribution compose tool Group: Development/Tools License: GPLv2 URL: https://pagure.io/pungi Source0: https://pagure.io/releases/%{name}/%{name}-%{version}.tar.bz2 +Patch0: 0001-Ensure-all-phases-are-stopped.patch BuildRequires: python-nose, python-mock BuildRequires: python-devel, python-setuptools, python2-productmd >= 1.3 @@ -76,6 +77,7 @@ notification to Fedora Message Bus. %prep %setup -q +%patch0 -p1 %build %{__python} setup.py build @@ -120,6 +122,9 @@ cd tests && ./test_compose.sh %{_bindir}/%{name}-compare-depsolving %changelog +* Mon Jun 05 2017 Lubomír Sedlář - 4.1.15-2 +- Ensure proper exit on failure + * Fri May 05 2017 Lubomír Sedlář - 4.1.15-1 - pkgset: Remove use of undefined variable (lsedlar) - Store RPM artifacts in resulting repository in modulemd metadata. (jkaluza)