From 7f56b978cece9264ff1d9d3fa49964e59163b79c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 28 Nov 2016 14:25:35 +0100 Subject: [PATCH 1/3] live-media: Allow some arches to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch uses the `--can-fail` option of koji command line. If only optional arches fail, the task will report as success. Failures on compose box side are ignored if and only if all architectures are optional. Signed-off-by: Lubomír Sedlář --- pungi/phases/livemedia_phase.py | 12 ++- pungi/wrappers/kojiwrapper.py | 3 + tests/test_livemediaphase.py | 141 +++++++++++++++++++++++++++++++- 3 files changed, 148 insertions(+), 8 deletions(-) diff --git a/pungi/phases/livemedia_phase.py b/pungi/phases/livemedia_phase.py index aa0f400b..61b8d704 100644 --- a/pungi/phases/livemedia_phase.py +++ b/pungi/phases/livemedia_phase.py @@ -90,6 +90,8 @@ class LiveMediaPhase(PhaseLoggerMixin, ImageConfigMixin, ConfigGuardedPhase): 'version': self.get_version(image_conf), 'failable_arches': image_conf.get('failable', []), } + if config['failable_arches'] == ['*']: + config['failable_arches'] = config['arches'] self.pool.add(LiveMediaThread(self.pool)) self.pool.queue_put((self.compose, variant, config)) @@ -102,8 +104,8 @@ class LiveMediaThread(WorkerThread): subvariant = config.pop('subvariant') self.failable_arches = config.pop('failable_arches') self.num = num - # TODO handle failure per architecture; currently not possible in single task - with failable(compose, bool(self.failable_arches), variant, '*', 'live-media', subvariant, + can_fail = set(self.failable_arches) == set(config['arches']) + with failable(compose, can_fail, variant, '*', 'live-media', subvariant, logger=self.pool._logger): self.worker(compose, variant, subvariant, config) @@ -126,6 +128,7 @@ class LiveMediaThread(WorkerThread): """Replace `arches` (as list) with `arch` as a comma-separated string.""" copy = dict(config) copy['arch'] = ','.join(copy.pop('arches', [])) + copy['can_fail'] = self.failable_arches return koji_wrapper.get_live_media_cmd(copy) def worker(self, compose, variant, subvariant, config): @@ -149,9 +152,10 @@ class LiveMediaThread(WorkerThread): if path.endswith('.iso'): image_infos.append({'path': path, 'arch': arch}) - if len(image_infos) != len(config['arches']): + if len(image_infos) < len(config['arches']) - len(self.failable_arches): self.pool.log_error( - 'Error in koji task %s. Expected to find one image for each arch (%s). Got %s.' + 'Error in koji task %s. Expected to find at least one image ' + 'for each required arch (%s). Got %s.' % (output['task_id'], len(config['arches']), len(image_infos))) raise RuntimeError('Image count mismatch in task %s.' % output['task_id']) diff --git a/pungi/wrappers/kojiwrapper.py b/pungi/wrappers/kojiwrapper.py index 85a933a1..f75b3f54 100644 --- a/pungi/wrappers/kojiwrapper.py +++ b/pungi/wrappers/kojiwrapper.py @@ -189,6 +189,9 @@ class KojiWrapper(object): if 'release' in options: cmd.append('--release=%s' % options['release']) + if 'can_fail' in options: + cmd.append('--can-fail=%s' % options['can_fail']) + if wait: cmd.append('--wait') diff --git a/tests/test_livemediaphase.py b/tests/test_livemediaphase.py index 61c37ada..99dffd06 100644 --- a/tests/test_livemediaphase.py +++ b/tests/test_livemediaphase.py @@ -59,6 +59,51 @@ class TestLiveMediaPhase(PungiTestCase): 'failable_arches': [], }))]) + @mock.patch('pungi.phases.livemedia_phase.ThreadPool') + def test_expand_failable(self, ThreadPool): + compose = DummyCompose(self.topdir, { + 'live_media': { + '^Server$': [ + { + 'target': 'f24', + 'kickstart': 'file.ks', + 'ksurl': 'git://example.com/repo.git', + 'name': 'Fedora Server Live', + 'version': 'Rawhide', + 'failable': ['*'], + } + ] + }, + 'koji_profile': 'koji', + }) + + self.assertValidConfig(compose.conf) + + phase = LiveMediaPhase(compose) + + phase.run() + self.assertTrue(phase.pool.add.called) + self.assertEqual(phase.pool.queue_put.call_args_list, + [mock.call((compose, + compose.variants['Server'], + { + 'arches': ['amd64', 'x86_64'], + 'ksfile': 'file.ks', + 'ksurl': 'git://example.com/repo.git', + 'ksversion': None, + 'name': 'Fedora Server Live', + 'release': None, + 'repo': [self.topdir + '/compose/Server/$basearch/os'], + 'scratch': False, + 'skip_tag': None, + 'target': 'f24', + 'title': None, + 'install_tree': self.topdir + '/compose/Server/$basearch/os', + 'version': 'Rawhide', + 'subvariant': 'Server', + 'failable_arches': ['amd64', 'x86_64'], + }))]) + @mock.patch('pungi.util.resolve_git_url') @mock.patch('pungi.phases.livemedia_phase.ThreadPool') def test_live_media_with_phase_global_opts(self, ThreadPool, resolve_git_url): @@ -368,7 +413,7 @@ class TestLiveMediaPhase(PungiTestCase): 'install_tree': self.topdir + '/compose/Server-optional/$basearch/os', 'version': '25', 'subvariant': 'Something', - 'failable_arches': ['*'], + 'failable_arches': ['x86_64'], }))]) @@ -446,7 +491,8 @@ class TestLiveMediaThread(PungiTestCase): 'skip_tag': None, 'target': 'f24', 'title': None, - 'version': 'Rawhide'})]) + 'version': 'Rawhide', + 'can_fail': []})]) self.assertEqual(get_image_paths.mock_calls, [mock.call(1234)]) self.assertTrue(os.path.isdir(self.topdir + '/compose/Server/x86_64/iso')) @@ -498,7 +544,7 @@ class TestLiveMediaThread(PungiTestCase): 'title': None, 'version': 'Rawhide', 'subvariant': 'KDE', - 'failable_arches': ['*'], + 'failable_arches': ['amd64', 'x86_64'], } pool = mock.Mock() @@ -520,6 +566,22 @@ class TestLiveMediaThread(PungiTestCase): mock.call('Live media task failed: 1234. See %s for more details.' % (os.path.join(self.topdir, 'logs/amd64-x86_64/livemedia-Server-KDE.amd64-x86_64.log'))) ]) + self.assertEqual(KojiWrapper.return_value.get_live_media_cmd.mock_calls, + [mock.call({ + 'arch': 'amd64,x86_64', + 'ksfile': 'file.ks', + 'ksurl': 'git://example.com/repo.git', + 'ksversion': None, + 'skip_tag': None, + 'target': 'f24', + 'title': None, + 'release': None, + 'version': 'Rawhide', + 'scratch': False, + 'can_fail': ['amd64', 'x86_64'], + 'name': 'Fedora Server Live', + 'repo': ['/repo/$basearch/Server'], + })]) @mock.patch('pungi.phases.livemedia_phase.get_mtime') @mock.patch('pungi.phases.livemedia_phase.get_file_size') @@ -545,7 +607,7 @@ class TestLiveMediaThread(PungiTestCase): 'title': None, 'version': 'Rawhide', 'subvariant': 'KDE', - 'failable_arches': ['*'], + 'failable_arches': ['amd64', 'x86_64'], } pool = mock.Mock() @@ -562,6 +624,77 @@ class TestLiveMediaThread(PungiTestCase): mock.call('[FAIL] Live media (variant Server, arch *, subvariant KDE) failed, but going on anyway.'), mock.call('BOOM') ]) + self.assertEqual(KojiWrapper.return_value.get_live_media_cmd.mock_calls, + [mock.call({ + 'arch': 'amd64,x86_64', + 'ksfile': 'file.ks', + 'ksurl': 'git://example.com/repo.git', + 'ksversion': None, + 'skip_tag': None, + 'target': 'f24', + 'title': None, + 'release': None, + 'version': 'Rawhide', + 'scratch': False, + 'can_fail': ['amd64', 'x86_64'], + 'name': 'Fedora Server Live', + 'repo': ['/repo/$basearch/Server'], + })]) + + @mock.patch('pungi.phases.livemedia_phase.get_mtime') + @mock.patch('pungi.phases.livemedia_phase.get_file_size') + @mock.patch('pungi.phases.livemedia_phase.KojiWrapper') + def test_handle_exception_only_one_arch_optional(self, KojiWrapper, get_file_size, get_mtime): + compose = DummyCompose(self.topdir, { + 'koji_profile': 'koji', + 'failable_deliverables': [ + ('^.+$', {'*': ['live-media']}) + ] + }) + config = { + 'arches': ['amd64', 'x86_64'], + 'ksfile': 'file.ks', + 'ksurl': 'git://example.com/repo.git', + 'ksversion': None, + 'name': 'Fedora Server Live', + 'release': None, + 'repo': ['/repo/$basearch/Server'], + 'scratch': False, + 'skip_tag': None, + 'target': 'f24', + 'title': None, + 'version': 'Rawhide', + 'subvariant': 'KDE', + 'failable_arches': ['amd64'], + } + pool = mock.Mock() + + run_blocking_cmd = KojiWrapper.return_value.run_blocking_cmd + run_blocking_cmd.side_effect = boom + get_file_size.return_value = 1024 + get_mtime.return_value.st_mtime = 13579 + + t = LiveMediaThread(pool) + with self.assertRaises(Exception): + with mock.patch('time.sleep'): + t.process((compose, compose.variants['Server'], config), 1) + + self.assertEqual(KojiWrapper.return_value.get_live_media_cmd.mock_calls, + [mock.call({ + 'arch': 'amd64,x86_64', + 'ksfile': 'file.ks', + 'ksurl': 'git://example.com/repo.git', + 'ksversion': None, + 'skip_tag': None, + 'target': 'f24', + 'title': None, + 'release': None, + 'version': 'Rawhide', + 'scratch': False, + 'can_fail': ['amd64'], + 'name': 'Fedora Server Live', + 'repo': ['/repo/$basearch/Server'], + })]) if __name__ == "__main__": From 356b78d440b317c9151c7b20f3204254e5c4bd16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 28 Nov 2016 15:28:00 +0100 Subject: [PATCH 2/3] image-build: Allow failure only on some arches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses the --can-fail option in koji. Failing an optional image will not abort whole task. If the whole task fails (or there is a problem on the compose side), we abort unless all arches are optional. Signed-off-by: Lubomír Sedlář --- pungi/phases/image_build.py | 15 ++++++--- pungi/wrappers/kojiwrapper.py | 2 +- tests/test_imagebuildphase.py | 58 +++++++++++++++++++++++++++++------ 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/pungi/phases/image_build.py b/pungi/phases/image_build.py index b1d0f878..5cfdbb09 100644 --- a/pungi/phases/image_build.py +++ b/pungi/phases/image_build.py @@ -120,6 +120,12 @@ class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG image_conf["image-build"]["format"] = ",".join([x[0] for x in image_conf["image-build"]["format"]]) image_conf["image-build"]['repo'] = self._get_repo(image_conf['image-build'], variant) + can_fail = image_conf['image-build'].pop('failable', []) + if can_fail == ['*']: + can_fail = image_conf['image-build']['arches'] + if can_fail: + image_conf['image-build']['can_fail'] = ','.join(sorted(can_fail)) + cmd = { "format": format, "image_conf": image_conf, @@ -134,7 +140,6 @@ class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG ), "link_type": self.compose.conf["link_type"], "scratch": image_conf['image-build'].pop('scratch', False), - "failable_arches": image_conf['image-build'].pop('failable', []), } self.pool.add(CreateImageBuildThread(self.pool)) self.pool.queue_put((self.compose, cmd)) @@ -150,15 +155,15 @@ class CreateImageBuildThread(WorkerThread): compose, cmd = item variant = cmd["image_conf"]["image-build"]["variant"] subvariant = cmd["image_conf"]["image-build"].get("subvariant", variant.uid) - failable_arches = cmd.get('failable_arches', []) - self.can_fail = bool(failable_arches) - # TODO handle failure per architecture; currently not possible in single task + self.failable_arches = cmd["image_conf"]['image-build'].get('can_fail', '') + self.can_fail = self.failable_arches == cmd['image_conf']['image-build']['arches'] with failable(compose, self.can_fail, variant, '*', 'image-build', subvariant, logger=self.pool._logger): self.worker(num, compose, variant, subvariant, cmd) def worker(self, num, compose, variant, subvariant, cmd): arches = cmd["image_conf"]["image-build"]['arches'].split(',') + failable_arches = self.failable_arches.split(',') dash_arches = '-'.join(arches) log_file = compose.paths.log.log_file( dash_arches, @@ -202,7 +207,7 @@ class CreateImageBuildThread(WorkerThread): image_infos.append({'path': path, 'suffix': suffix, 'type': format, 'arch': arch}) break - if len(image_infos) != len(cmd['format']) * len(arches): + if len(image_infos) != len(cmd['format']) * (len(arches) - len(failable_arches)): self.pool.log_error( "Error in koji task %s. Expected to find same amount of images " "as in suffixes attr in image-build (%s) for each arch (%s). Got '%s'." % diff --git a/pungi/wrappers/kojiwrapper.py b/pungi/wrappers/kojiwrapper.py index f75b3f54..7f4ff82e 100644 --- a/pungi/wrappers/kojiwrapper.py +++ b/pungi/wrappers/kojiwrapper.py @@ -190,7 +190,7 @@ class KojiWrapper(object): cmd.append('--release=%s' % options['release']) if 'can_fail' in options: - cmd.append('--can-fail=%s' % options['can_fail']) + cmd.append('--can-fail=%s' % ','.join(options['can_fail'])) if wait: cmd.append('--wait') diff --git a/tests/test_imagebuildphase.py b/tests/test_imagebuildphase.py index 5f753cfd..86012acd 100644 --- a/tests/test_imagebuildphase.py +++ b/tests/test_imagebuildphase.py @@ -70,6 +70,7 @@ class TestImageBuildPhase(PungiTestCase): 'version': 'Rawhide', 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', 'distro': 'Fedora-20', + 'can_fail': 'x86_64', } }, "conf_file": self.topdir + '/work/image-build/Client/docker_Fedora-Docker-Base.cfg', @@ -77,7 +78,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Client/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": ['x86_64'], } server_args = { "format": [('docker', 'tar.xz')], @@ -95,6 +95,7 @@ class TestImageBuildPhase(PungiTestCase): 'version': 'Rawhide', 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', 'distro': 'Fedora-20', + 'can_fail': 'x86_64', } }, "conf_file": self.topdir + '/work/image-build/Server/docker_Fedora-Docker-Base.cfg', @@ -102,7 +103,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Server/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": ['x86_64'], } self.assertItemsEqual(phase.pool.queue_put.mock_calls, [mock.call((compose, client_args)), @@ -163,7 +163,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Server/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": [], } self.assertItemsEqual(phase.pool.queue_put.mock_calls, [mock.call((compose, server_args))]) @@ -220,7 +219,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Server/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": [], } self.assertItemsEqual(phase.pool.queue_put.mock_calls, [mock.call((compose, server_args))]) @@ -318,7 +316,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Server/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": [], }) @mock.patch('pungi.phases.image_build.ThreadPool') @@ -383,7 +380,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Server/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": [], }) @mock.patch('pungi.phases.image_build.ThreadPool') @@ -445,7 +441,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Server/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": [], }) @mock.patch('pungi.phases.image_build.ThreadPool') @@ -612,6 +607,7 @@ class TestImageBuildPhase(PungiTestCase): 'version': 'Rawhide', 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', 'distro': 'Fedora-20', + 'can_fail': 'x86_64', } }, "conf_file": self.topdir + '/work/image-build/Server-optional/docker_Fedora-Docker-Base.cfg', @@ -619,7 +615,6 @@ class TestImageBuildPhase(PungiTestCase): "relative_image_dir": 'Server-optional/%(arch)s/images', "link_type": 'hardlink-or-copy', "scratch": False, - "failable_arches": ['x86_64'], } self.assertItemsEqual(phase.pool.queue_put.mock_calls, [mock.call((compose, server_args))]) @@ -781,6 +776,7 @@ class TestCreateImageBuildThread(PungiTestCase): 'version': 'Rawhide', 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', 'distro': 'Fedora-20', + "can_fail": 'amd64,x86_64', } }, "conf_file": 'amd64,x86_64-Client-Fedora-Docker-Base-docker', @@ -788,7 +784,6 @@ class TestCreateImageBuildThread(PungiTestCase): "relative_image_dir": 'image_dir/Client/%(arch)s', "link_type": 'hardlink-or-copy', 'scratch': False, - "failable_arches": ['*'], } koji_wrapper = KojiWrapper.return_value koji_wrapper.run_blocking_cmd.return_value = { @@ -829,6 +824,7 @@ class TestCreateImageBuildThread(PungiTestCase): 'version': 'Rawhide', 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', 'distro': 'Fedora-20', + 'can_fail': 'amd64,x86_64', } }, "conf_file": 'amd64,x86_64-Client-Fedora-Docker-Base-docker', @@ -836,7 +832,6 @@ class TestCreateImageBuildThread(PungiTestCase): "relative_image_dir": 'image_dir/Client/%(arch)s', "link_type": 'hardlink-or-copy', 'scratch': False, - "failable_arches": ['*'], } koji_wrapper = KojiWrapper.return_value @@ -851,6 +846,49 @@ class TestCreateImageBuildThread(PungiTestCase): mock.call('BOOM'), ]) + @mock.patch('pungi.phases.image_build.KojiWrapper') + @mock.patch('pungi.phases.image_build.Linker') + def test_process_handle_fail_only_one_optional(self, Linker, KojiWrapper): + compose = DummyCompose(self.topdir, {'koji_profile': 'koji'}) + pool = mock.Mock() + cmd = { + "format": [('docker', 'tar.xz'), ('qcow2', 'qcow2')], + "image_conf": { + 'image-build': { + 'install_tree': '/ostree/$arch/Client', + 'kickstart': 'fedora-docker-base.ks', + 'format': 'docker', + 'repo': '/ostree/$arch/Client', + 'variant': compose.variants['Client'], + 'target': 'f24', + 'disk_size': 3, + 'name': 'Fedora-Docker-Base', + 'arches': 'amd64,x86_64', + 'version': 'Rawhide', + 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', + 'distro': 'Fedora-20', + 'can_fail': 'amd64', + } + }, + "conf_file": 'amd64,x86_64-Client-Fedora-Docker-Base-docker', + "image_dir": '/image_dir/Client/%(arch)s', + "relative_image_dir": 'image_dir/Client/%(arch)s', + "link_type": 'hardlink-or-copy', + 'scratch': False, + } + + koji_wrapper = KojiWrapper.return_value + koji_wrapper.run_blocking_cmd.return_value = { + "retcode": 1, + "output": None, + "task_id": 1234, + } + + t = CreateImageBuildThread(pool) + with self.assertRaises(RuntimeError): + with mock.patch('time.sleep'): + t.process((compose, cmd), 1) + if __name__ == "__main__": unittest.main() From 17a5f2841cbbecdc8c6b7f0e5ba2859ea1a0fd4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 28 Nov 2016 15:57:54 +0100 Subject: [PATCH 3/3] Update documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also remove the TODO comment from live images phase: the appliances are already submitted one task per single arch, so this change is not necessary. Signed-off-by: Lubomír Sedlář --- doc/configuration.rst | 12 +++++------- pungi/phases/live_images.py | 1 - 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index 62d73612..163a1528 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -864,13 +864,11 @@ provided; date, compose type and respin will be used. * ``image_build_release`` * ``live_images_release`` -Each configuration block can also optionally specify a list of architectures -that are not release blocking with ``failable`` key. If any deliverable fails, -it will not abort the whole compose. Due to limitations in how the tasks are -done in Koji, if any architecture fails, all of them fail. Until this is -resolved, it is not possible to configure failability per architecture. An -empty list means required deliverable, non-empty list means non-blocking -deliverable. +Each configuration block can also optionally specify a ``failable`` key. For +live images it should have a boolean value. For live media and image build it +should be a list of strings containing architectures that are optional. If any +deliverable fails on an optional architecture, it will not abort the whole +compose. If the list contains only ``"*"``, all arches will be substituted. Live Images Settings diff --git a/pungi/phases/live_images.py b/pungi/phases/live_images.py index 3d3a1605..2905dbca 100644 --- a/pungi/phases/live_images.py +++ b/pungi/phases/live_images.py @@ -152,7 +152,6 @@ class CreateLiveImageThread(WorkerThread): def process(self, item, num): compose, cmd, variant, arch = item self.failable_arches = cmd.get('failable_arches', []) - # TODO handle failure per architecture; currently not possible in single task self.can_fail = bool(self.failable_arches) with failable(compose, self.can_fail, variant, arch, 'live', cmd.get('subvariant'), logger=self.pool._logger):