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/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/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): 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 0dcac906..bca9e649 100644 --- a/pungi/wrappers/kojiwrapper.py +++ b/pungi/wrappers/kojiwrapper.py @@ -191,6 +191,9 @@ class KojiWrapper(object): if 'release' in options: cmd.append('--release=%s' % options['release']) + if 'can_fail' in options: + 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() 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__":