From 7f815d80f57dc95fbdbd28a5bc648f686a141dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Fri, 8 Jan 2016 13:37:09 +0100 Subject: [PATCH 1/4] [image-build] Take install tree from another variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a configuration option to use install tree from another variant. Signed-off-by: Lubomír Sedlář --- doc/configuration.rst | 8 +++- pungi/phases/image_build.py | 24 ++++++++++-- tests/test_imagebuildphase.py | 72 +++++++++++++++++++++++++++++++---- 3 files changed, 91 insertions(+), 13 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index 149bf6b1..d4b07191 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -613,7 +613,10 @@ Image Build Settings automatically transformed into format suitable for ``koji``. A repo for the currently built variant will be added as well. - Please don't set ``install_tree`` as it would get overriden by pungi. + Please don't set ``install_tree``. This gets automatically set by *pungi* + based on current variant. You can use ``install_tree_from`` key to use + install tree from another variant. + The ``format`` attr is [('image_type', 'image_suffix'), ...]. See productmd documentation for list of supported types and suffixes. @@ -655,6 +658,9 @@ Example # only build this type of image on x86_64 'arches': ['x86_64'] + + # Use install tree from Everything variant. + 'install_tree_from': 'Everything', } ] } diff --git a/pungi/phases/image_build.py b/pungi/phases/image_build.py index 436ccbc5..db80e80f 100644 --- a/pungi/phases/image_build.py +++ b/pungi/phases/image_build.py @@ -29,6 +29,23 @@ class ImageBuildPhase(PhaseBase): return True return False + def _get_install_tree(self, image_conf, variant): + """ + Get a path to os tree for a variant specified in `install_tree_from` or + current variant. If the config is set, it will be removed from the + dict. + """ + install_tree_from = image_conf.pop('install_tree_from', variant.uid) + install_tree_source = self.compose.variants.get(install_tree_from) + if not install_tree_source: + raise RuntimeError( + 'There is no variant %s to get install tree from when building image for %s.' + % (install_tree_from, variant.uid)) + return translate_path( + self.compose, + self.compose.paths.compose.os_tree('$arch', install_tree_source) + ) + def run(self): for variant in self.compose.get_variants(): arches = set([x for x in variant.arches if x != 'src']) @@ -54,10 +71,9 @@ class ImageBuildPhase(PhaseBase): continue image_conf["variant"] = variant - image_conf["install_tree"] = translate_path( - self.compose, - self.compose.paths.compose.os_tree('$arch', variant) - ) + + image_conf["install_tree"] = self._get_install_tree(image_conf, variant) + # transform format into right 'format' for image-build # e.g. 'docker,qcow2' format = image_conf["format"] diff --git a/tests/test_imagebuildphase.py b/tests/test_imagebuildphase.py index 448d783d..e1867df0 100755 --- a/tests/test_imagebuildphase.py +++ b/tests/test_imagebuildphase.py @@ -49,17 +49,18 @@ class _DummyCompose(object): ) ) self._logger = mock.Mock() - self.variants = [ - mock.Mock(uid='Server', arches=['x86_64', 'amd64']), - mock.Mock(uid='Client', arches=['amd64']), - ] + self.variants = { + 'Server': mock.Mock(uid='Server', arches=['x86_64', 'amd64']), + 'Client': mock.Mock(uid='Client', arches=['amd64']), + 'Everything': mock.Mock(uid='Everything', arches=['x86_64', 'amd64']), + } self.im = mock.Mock() def get_arches(self): return ['x86_64', 'amd64'] def get_variants(self, arch=None, types=None): - return [v for v in self.variants if not arch or arch in v.arches] + return [v for v in self.variants.values() if not arch or arch in v.arches] class TestImageBuildPhase(unittest.TestCase): @@ -97,7 +98,7 @@ class TestImageBuildPhase(unittest.TestCase): 'kickstart': 'fedora-docker-base.ks', 'format': 'docker', 'repo': '/ostree/$arch/Client', - 'variant': compose.variants[1], + 'variant': compose.variants['Client'], 'target': 'f24', 'disk_size': 3, 'name': 'Fedora-Docker-Base', @@ -118,7 +119,7 @@ class TestImageBuildPhase(unittest.TestCase): 'kickstart': 'fedora-docker-base.ks', 'format': 'docker', 'repo': '/ostree/$arch/Server', - 'variant': compose.variants[0], + 'variant': compose.variants['Server'], 'target': 'f24', 'disk_size': 3, 'name': 'Fedora-Docker-Base', @@ -165,6 +166,61 @@ class TestImageBuildPhase(unittest.TestCase): self.assertFalse(phase.pool.add.called) self.assertFalse(phase.pool.queue_put.called) + @mock.patch('pungi.phases.image_build.ThreadPool') + def test_image_build_set_install_tree(self, ThreadPool): + compose = _DummyCompose({ + 'image_build': { + '^Server$': [ + { + 'format': [('docker', 'tar.xz')], + 'name': 'Fedora-Docker-Base', + 'target': 'f24', + 'version': 'Rawhide', + 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', + 'kickstart': "fedora-docker-base.ks", + 'distro': 'Fedora-20', + 'disk_size': 3, + 'arches': ['x86_64'], + 'install_tree_from': 'Everything', + } + ] + }, + 'koji_profile': 'koji', + }) + + phase = ImageBuildPhase(compose) + + phase.run() + + # assert at least one thread was started + self.assertTrue(phase.pool.add.called) + + self.assertTrue(phase.pool.queue_put.called_once) + args, kwargs = phase.pool.queue_put.call_args + self.assertEqual(args[0][0], compose) + self.maxDiff = None + self.assertDictEqual(args[0][1], { + "format": [('docker', 'tar.xz')], + "image_conf": { + 'install_tree': '/ostree/$arch/Everything', + 'kickstart': 'fedora-docker-base.ks', + 'format': 'docker', + 'repo': '/ostree/$arch/Server', + 'variant': compose.variants['Server'], + 'target': 'f24', + 'disk_size': 3, + 'name': 'Fedora-Docker-Base', + 'arches': 'x86_64', + 'version': 'Rawhide', + 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', + 'distro': 'Fedora-20', + }, + "conf_file": 'Server-Fedora-Docker-Base-docker', + "image_dir": '/image_dir/Server/%(arch)s', + "relative_image_dir": 'image_dir/Server/%(arch)s', + "link_type": 'hardlink-or-copy', + }) + class TestCreateImageBuildThread(unittest.TestCase): @@ -182,7 +238,7 @@ class TestCreateImageBuildThread(unittest.TestCase): 'kickstart': 'fedora-docker-base.ks', 'format': 'docker', 'repo': '/ostree/$arch/Client', - 'variant': compose.variants[1], + 'variant': compose.variants['Client'], 'target': 'f24', 'disk_size': 3, 'name': 'Fedora-Docker-Base', From d2ea1dd2884af65fc69a1796525e3a81239cdab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 11 Jan 2016 09:03:04 +0100 Subject: [PATCH 2/4] [image-build] Use repo from another variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The config can now additionally specify other variants whose repos will be passed on to koji. The previous way of specifying extra repos is still available, as is the automatic adding of repo for current variant. Signed-off-by: Lubomír Sedlář --- doc/configuration.rst | 6 +++- pungi/phases/image_build.py | 32 ++++++++++++++++---- tests/test_imagebuildphase.py | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index d4b07191..8a980bba 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -613,6 +613,9 @@ Image Build Settings automatically transformed into format suitable for ``koji``. A repo for the currently built variant will be added as well. + You can also add extra variants to get repos from with key ``repo_from``. + The value should be a list of variant names. + Please don't set ``install_tree``. This gets automatically set by *pungi* based on current variant. You can use ``install_tree_from`` key to use install tree from another variant. @@ -659,8 +662,9 @@ Example # only build this type of image on x86_64 'arches': ['x86_64'] - # Use install tree from Everything variant. + # Use install tree and repo from Everything variant. 'install_tree_from': 'Everything', + 'repo_from': ['Everything'], } ] } diff --git a/pungi/phases/image_build.py b/pungi/phases/image_build.py index db80e80f..07970756 100644 --- a/pungi/phases/image_build.py +++ b/pungi/phases/image_build.py @@ -3,6 +3,7 @@ import copy import os import time +from kobo import shortcuts from pungi.util import get_variant_data, resolve_git_url from pungi.phases.base import PhaseBase @@ -46,6 +47,30 @@ class ImageBuildPhase(PhaseBase): self.compose.paths.compose.os_tree('$arch', install_tree_source) ) + def _get_repo(self, image_conf, variant): + """ + Get a comma separated list of repos. First included are those + explicitly listed in config, followed by repos from other variants, + finally followed by repo for current variant. + + The `repo_from` key is removed from the dict (if present). + """ + repo = shortcuts.force_list(image_conf.get('repo', [])) + + extras = shortcuts.force_list(image_conf.pop('repo_from', [])) + extras.append(variant.uid) + + for extra in extras: + v = self.compose.variants.get(extra) + if not v: + raise RuntimeError( + 'There is no variant %s to get repo from when building image for %s.' + % (extra, variant.uid)) + repo.append(translate_path(self.compose, + self.compose.paths.compose.os_tree('$arch', v))) + + return ",".join(repo) + def run(self): for variant in self.compose.get_variants(): arches = set([x for x in variant.arches if x != 'src']) @@ -79,12 +104,7 @@ class ImageBuildPhase(PhaseBase): format = image_conf["format"] image_conf["format"] = ",".join([x[0] for x in image_conf["format"]]) - repo = image_conf.get('repo', []) - if isinstance(repo, str): - repo = [repo] - repo.append(translate_path(self.compose, self.compose.paths.compose.os_tree('$arch', variant))) - # supply repo as str separated by , instead of list - image_conf['repo'] = ",".join(repo) + image_conf['repo'] = self._get_repo(image_conf, variant) cmd = { "format": format, diff --git a/tests/test_imagebuildphase.py b/tests/test_imagebuildphase.py index e1867df0..2e59d267 100755 --- a/tests/test_imagebuildphase.py +++ b/tests/test_imagebuildphase.py @@ -221,6 +221,61 @@ class TestImageBuildPhase(unittest.TestCase): "link_type": 'hardlink-or-copy', }) + @mock.patch('pungi.phases.image_build.ThreadPool') + def test_image_build_set_extra_repos(self, ThreadPool): + compose = _DummyCompose({ + 'image_build': { + '^Server$': [ + { + 'format': [('docker', 'tar.xz')], + 'name': 'Fedora-Docker-Base', + 'target': 'f24', + 'version': 'Rawhide', + 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', + 'kickstart': "fedora-docker-base.ks", + 'distro': 'Fedora-20', + 'disk_size': 3, + 'arches': ['x86_64'], + 'repo_from': 'Everything', + } + ] + }, + 'koji_profile': 'koji', + }) + + phase = ImageBuildPhase(compose) + + phase.run() + + # assert at least one thread was started + self.assertTrue(phase.pool.add.called) + + self.assertTrue(phase.pool.queue_put.called_once) + args, kwargs = phase.pool.queue_put.call_args + self.assertEqual(args[0][0], compose) + self.maxDiff = None + self.assertDictEqual(args[0][1], { + "format": [('docker', 'tar.xz')], + "image_conf": { + 'install_tree': '/ostree/$arch/Server', + 'kickstart': 'fedora-docker-base.ks', + 'format': 'docker', + 'repo': '/ostree/$arch/Everything,/ostree/$arch/Server', + 'variant': compose.variants['Server'], + 'target': 'f24', + 'disk_size': 3, + 'name': 'Fedora-Docker-Base', + 'arches': 'x86_64', + 'version': 'Rawhide', + 'ksurl': 'git://git.fedorahosted.org/git/spin-kickstarts.git', + 'distro': 'Fedora-20', + }, + "conf_file": 'Server-Fedora-Docker-Base-docker', + "image_dir": '/image_dir/Server/%(arch)s', + "relative_image_dir": 'image_dir/Server/%(arch)s', + "link_type": 'hardlink-or-copy', + }) + class TestCreateImageBuildThread(unittest.TestCase): From af11bebf1bd4d6cd20c94cb40d1dc496bbafe124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 11 Jan 2016 09:06:13 +0100 Subject: [PATCH 3/4] [image-build] Refactoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Move getting arches to separate method. * Modify git url only if there is going to be a build. This is not very likely to happen, but could save a bit of time if it really does happen. Signed-off-by: Lubomír Sedlář --- pungi/phases/image_build.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pungi/phases/image_build.py b/pungi/phases/image_build.py index 07970756..7c503d28 100644 --- a/pungi/phases/image_build.py +++ b/pungi/phases/image_build.py @@ -71,6 +71,11 @@ class ImageBuildPhase(PhaseBase): return ",".join(repo) + def _get_arches(self, image_conf, arches): + if 'arches' in image_conf: + arches = set(image_conf.get('arches', [])) & arches + return ','.join(sorted(arches)) + def run(self): for variant in self.compose.get_variants(): arches = set([x for x in variant.arches if x != 'src']) @@ -81,20 +86,16 @@ class ImageBuildPhase(PhaseBase): # value is needed. image_conf = copy.deepcopy(image_conf) + # image_conf is passed to get_image_build_cmd as dict + + image_conf['arches'] = self._get_arches(image_conf, arches) + if not image_conf['arches']: + continue + # Replace possible ambiguous ref name with explicit hash. if 'ksurl' in image_conf: image_conf['ksurl'] = resolve_git_url(image_conf['ksurl']) - # image_conf is passed to get_image_build_cmd as dict - - if 'arches' in image_conf: - image_conf["arches"] = ','.join(sorted(set(image_conf.get('arches', [])) & arches)) - else: - image_conf['arches'] = ','.join(sorted(arches)) - - if not image_conf['arches']: - continue - image_conf["variant"] = variant image_conf["install_tree"] = self._get_install_tree(image_conf, variant) From 5688f1cbaee014b4c678170094cacae435bdeccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 11 Jan 2016 17:14:51 +0100 Subject: [PATCH 4/4] [image-build] Optionally do not break whole compose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The configuration can now specify image-build as a deliverable that can fail but not abort the whole compose. Signed-off-by: Lubomír Sedlář --- doc/configuration.rst | 6 ++ pungi/phases/image_build.py | 11 ++++ tests/test_compose.py | 3 + tests/test_imagebuildphase.py | 102 ++++++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+) diff --git a/doc/configuration.rst b/doc/configuration.rst index 8a980bba..14169358 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -135,6 +135,12 @@ Options * buildinstall * iso * live + * image-build + + .. note:: + + Image building is not run per-architecture. If you want to mark it + as failable, specify it in a block with arch set as ``*``. Please note that ``*`` as a wildcard matches all architectures but ``src``. diff --git a/pungi/phases/image_build.py b/pungi/phases/image_build.py index 7c503d28..0d85ae15 100644 --- a/pungi/phases/image_build.py +++ b/pungi/phases/image_build.py @@ -133,6 +133,17 @@ class CreateImageBuildThread(WorkerThread): def process(self, item, num): compose, cmd = item + try: + self.worker(num, compose, cmd) + except: + if not compose.can_fail(cmd['image_conf']['variant'], '*', 'image-build'): + raise + else: + msg = ('[FAIL] image-build for variant %s failed, but going on anyway.' + % cmd['image_conf']['variant']) + self.pool.log_info(msg) + + def worker(self, num, compose, cmd): arches = cmd['image_conf']['arches'].split(',') mounts = [compose.paths.compose.topdir()] diff --git a/tests/test_compose.py b/tests/test_compose.py index a23bf31f..556a49a8 100755 --- a/tests/test_compose.py +++ b/tests/test_compose.py @@ -40,6 +40,9 @@ class ComposeTestCase(unittest.TestCase): self.assertFalse(compose.can_fail(None, 'x86_64', 'live')) self.assertTrue(compose.can_fail(None, 'i386', 'live')) + self.assertTrue(compose.can_fail(variant, '*', 'buildinstall')) + self.assertFalse(compose.can_fail(variant, '*', 'live')) + @mock.patch('pungi.compose.ComposeInfo') def test_get_image_name(self, ci): conf = {} diff --git a/tests/test_imagebuildphase.py b/tests/test_imagebuildphase.py index 2e59d267..91026eac 100755 --- a/tests/test_imagebuildphase.py +++ b/tests/test_imagebuildphase.py @@ -11,6 +11,7 @@ import sys sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from pungi.phases.image_build import ImageBuildPhase, CreateImageBuildThread +from pungi.util import get_arch_variant_data class _DummyCompose(object): @@ -55,6 +56,7 @@ class _DummyCompose(object): 'Everything': mock.Mock(uid='Everything', arches=['x86_64', 'amd64']), } self.im = mock.Mock() + self.log_error = mock.Mock() def get_arches(self): return ['x86_64', 'amd64'] @@ -62,6 +64,10 @@ class _DummyCompose(object): def get_variants(self, arch=None, types=None): return [v for v in self.variants.values() if not arch or arch in v.arches] + def can_fail(self, variant, arch, deliverable): + failable = get_arch_variant_data(self.conf, 'failable_deliverables', arch, variant) + return deliverable in failable + class TestImageBuildPhase(unittest.TestCase): @@ -387,6 +393,102 @@ class TestCreateImageBuildThread(unittest.TestCase): self.assertEqual(data['format'], image.format) self.assertEqual(data['type'], image.type) + @mock.patch('pungi.phases.image_build.KojiWrapper') + @mock.patch('pungi.phases.image_build.Linker') + def test_process_handle_fail(self, Linker, KojiWrapper): + compose = _DummyCompose({ + 'koji_profile': 'koji', + 'failable_deliverables': [ + ('^.*$', { + '*': ['image-build'] + }) + ] + }) + pool = mock.Mock() + cmd = { + "format": [('docker', 'tar.xz'), ('qcow2', 'qcow2')], + "image_conf": { + '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', + }, + "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', + } + koji_wrapper = KojiWrapper.return_value + koji_wrapper.run_create_image_cmd.return_value = { + "retcode": 1, + "output": None, + "task_id": 1234, + } + + t = CreateImageBuildThread(pool) + with mock.patch('os.stat') as stat: + with mock.patch('os.path.getsize') as getsize: + with mock.patch('time.sleep'): + getsize.return_value = 1024 + stat.return_value.st_mtime = 13579 + t.process((compose, cmd), 1) + + @mock.patch('pungi.phases.image_build.KojiWrapper') + @mock.patch('pungi.phases.image_build.Linker') + def test_process_handle_exception(self, Linker, KojiWrapper): + compose = _DummyCompose({ + 'koji_profile': 'koji', + 'failable_deliverables': [ + ('^.*$', { + '*': ['image-build'] + }) + ] + }) + pool = mock.Mock() + cmd = { + "format": [('docker', 'tar.xz'), ('qcow2', 'qcow2')], + "image_conf": { + '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', + }, + "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', + } + + def boom(*args, **kwargs): + raise RuntimeError('BOOM') + + koji_wrapper = KojiWrapper.return_value + koji_wrapper.run_create_image_cmd.side_effect = boom + + t = CreateImageBuildThread(pool) + with mock.patch('os.stat') as stat: + with mock.patch('os.path.getsize') as getsize: + with mock.patch('time.sleep'): + getsize.return_value = 1024 + stat.return_value.st_mtime = 13579 + t.process((compose, cmd), 1) + if __name__ == "__main__": unittest.main()