From cdfa3cb45feab460b18abfd0440858b1e5618367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 30 Oct 2017 13:23:49 +0100 Subject: [PATCH] image-build: Drop suffixes from configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file extension in configuration is only used to tell Pungi which files from the task results should be downloaded. The user has to get it right or the phase will fail. Each format has a single valid suffix. Pungi should not require users to specify the suffix, since it can just as well just know the right value. The old configuration will continue working, only the extension will be ignored. Fixes: https://pagure.io/pungi/issue/753 Signed-off-by: Lubomír Sedlář --- doc/configuration.rst | 10 ++-- pungi/checks.py | 8 +++- pungi/phases/image_build.py | 57 +++++++++++++++-------- tests/test_imagebuildphase.py | 88 +++++++++++++++-------------------- 4 files changed, 87 insertions(+), 76 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index 081bd2f2..fd5b67b1 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -1109,8 +1109,8 @@ Image Build Settings 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. + You can set either a single format, or a list of formats. For available + values see help output for ``koji image-build`` command. If ``ksurl`` ends with ``#HEAD``, Pungi will figure out the SHA1 hash of current HEAD and use that instead. @@ -1126,7 +1126,7 @@ Example '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.gz'), ('qcow2', 'qcow2')] + 'format': ['docker', 'qcow2'] 'name': 'fedora-qcow-and-docker-base', 'target': 'koji-target-name', 'ksversion': 'F23', # value from pykickstart @@ -1149,7 +1149,7 @@ Example }, { 'image-build': { - 'format': [('docker', 'tar.gz'), ('qcow2', 'qcow2')] + 'format': ['docker', 'qcow2'] 'name': 'fedora-qcow-and-docker-base', 'target': 'koji-target-name', 'ksversion': 'F23', # value from pykickstart @@ -1167,7 +1167,7 @@ Example }, { 'image-build': { - 'format': [('qcow2','qcow2')] + 'format': 'qcow2', 'name': 'fedora-qcow-base', 'target': 'koji-target-name', 'ksversion': 'F23', # value from pykickstart diff --git a/pungi/checks.py b/pungi/checks.py index 307e87e7..1f31fd90 100644 --- a/pungi/checks.py +++ b/pungi/checks.py @@ -980,7 +980,13 @@ def make_schema(): }, "install_tree_from": {"type": "string"}, "subvariant": {"type": "string"}, - "format": {"$ref": "#/definitions/string_tuples"}, + "format": { + "anyOf": [ + # The variant with explicit extension is deprecated. + {"$ref": "#/definitions/string_tuples"}, + {"$ref": "#/definitions/strings"} + ] + }, }, }, "factory-parameters": { diff --git a/pungi/phases/image_build.py b/pungi/phases/image_build.py index 0fdcf3f7..ef4242c7 100644 --- a/pungi/phases/image_build.py +++ b/pungi/phases/image_build.py @@ -11,9 +11,34 @@ from pungi.phases import base from pungi.linker import Linker from pungi.wrappers.kojiwrapper import KojiWrapper from kobo.threads import ThreadPool, WorkerThread +from kobo.shortcuts import force_list from productmd.images import Image +# This is a mapping from formats to file extensions. The format is what koji +# image-build command expects as argument, and the extension is what the file +# name will be ending with. The extensions are used to filter out which task +# results will be pulled into the compose. +EXTENSIONS = { + 'docker': 'tar.gz', + 'liveimg-squashfs': 'liveimg.squashfs', + 'qcow': 'qcow', + 'qcow2': 'qcow2', + 'raw': 'raw', + 'raw-xz': 'raw.xz', + 'rhevm-ova': 'rhevm.ova', + 'tar-gz': 'tar.gz', + 'vagrant-hyperv': 'vagrant-hyperv.box', + 'vagrant-libvirt': 'vagrant-libvirt.box', + 'vagrant-virtualbox': 'vagrant-virtualbox.box', + 'vagrant-vmware-fusion': 'vagrant-vmware-fusion.box', + 'vdi': 'vdi', + 'vmdk': 'vdmk', + 'vpc': 'vhd', + 'vsphere-ova': 'vsphere.ova', +} + + class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigGuardedPhase): """class for wrapping up koji image-build""" name = "image_build" @@ -102,10 +127,12 @@ class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG image_conf['image-build']['version'] = self.get_version(image_conf['image-build']) image_conf['image-build']['target'] = self.get_config(image_conf['image-build'], 'target') - # transform format into right 'format' for image-build - # e.g. 'docker,qcow2' - format = image_conf["image-build"]["format"] - image_conf["image-build"]["format"] = ",".join([x[0] for x in image_conf["image-build"]["format"]]) + # Pungi config can either contain old [(format, suffix)], or + # just list of formats, or a single format. + formats = [] + for format in force_list(image_conf["image-build"]["format"]): + formats.append(format[0] if isinstance(format, tuple) else format) + image_conf["image-build"]["format"] = formats image_conf["image-build"]['repo'] = self._get_repo(image_conf['image-build'], variant) can_fail = image_conf['image-build'].pop('failable', []) @@ -115,12 +142,11 @@ class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG image_conf['image-build']['can_fail'] = sorted(can_fail) cmd = { - "format": format, "image_conf": image_conf, "conf_file": self.compose.paths.work.image_build_conf( image_conf["image-build"]['variant'], image_name=image_conf["image-build"]['name'], - image_type=image_conf["image-build"]['format'].replace(",", "-"), + image_type='-'.join(formats), arches=image_conf["image-build"]['arches'], ), "image_dir": self.compose.paths.compose.image_dir(variant), @@ -152,15 +178,14 @@ class CreateImageBuildThread(WorkerThread): def worker(self, num, compose, variant, subvariant, cmd): arches = cmd["image_conf"]["image-build"]['arches'] + formats = '-'.join(cmd['image_conf']['image-build']['format']) dash_arches = '-'.join(arches) log_file = compose.paths.log.log_file( dash_arches, - "imagebuild-%s-%s-%s" % (variant.uid, subvariant, - cmd["image_conf"]["image-build"]['format'].replace(",", "-")) + "imagebuild-%s-%s-%s" % (variant.uid, subvariant, formats) ) - msg = ("Creating %s image (arches: %s, variant: %s, subvariant: %s)" - % (cmd["image_conf"]["image-build"]["format"].replace(",", "-"), - dash_arches, variant, subvariant)) + msg = ("Creating image (formats: %s, arches: %s, variant: %s, subvariant: %s)" + % (formats, dash_arches, variant, subvariant)) self.pool.log_info("[BEGIN] %s" % msg) koji_wrapper = KojiWrapper(compose.conf["koji_profile"]) @@ -169,12 +194,6 @@ class CreateImageBuildThread(WorkerThread): self.pool.log_info("Writing image-build config for %s.%s into %s" % ( variant, dash_arches, cmd["conf_file"])) - # Join the arches into a single string. This is the value expected by - # koji config file. - cmd["image_conf"]["image-build"]['arches'] = ','.join(cmd["image_conf"]["image-build"]['arches']) - if 'can_fail' in cmd["image_conf"]["image-build"]: - cmd["image_conf"]["image-build"]['can_fail'] = ','.join(cmd["image_conf"]["image-build"]['can_fail']) - koji_cmd = koji_wrapper.get_image_build_cmd(cmd["image_conf"], conf_file_dest=cmd["conf_file"], scratch=cmd['scratch']) @@ -196,8 +215,8 @@ class CreateImageBuildThread(WorkerThread): for arch, paths in paths.items(): for path in paths: - # format is list of tuples [('qcow2', '.qcow2'), ('raw-xz', 'raw.xz'),] - for format, suffix in cmd['format']: + for format in cmd['image_conf']['image-build']['format']: + suffix = EXTENSIONS[format] if path.endswith(suffix): image_infos.append({'path': path, 'suffix': suffix, 'type': format, 'arch': arch}) break diff --git a/tests/test_imagebuildphase.py b/tests/test_imagebuildphase.py index ef6b130f..019935c1 100644 --- a/tests/test_imagebuildphase.py +++ b/tests/test_imagebuildphase.py @@ -55,12 +55,11 @@ class TestImageBuildPhase(PungiTestCase): # assert at least one thread was started self.assertTrue(phase.pool.add.called) client_args = { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Client/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': self.topdir + '/compose/Client/$arch/os', 'variant': compose.variants['Client'], 'target': 'f24', @@ -80,12 +79,11 @@ class TestImageBuildPhase(PungiTestCase): "scratch": False, } server_args = { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Server/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': self.topdir + '/compose/Server/$arch/os', 'variant': compose.variants['Server'], 'target': 'f24', @@ -119,7 +117,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'kickstart': "fedora-docker-base.ks", 'distro': 'Fedora-20', @@ -140,12 +138,11 @@ class TestImageBuildPhase(PungiTestCase): # assert at least one thread was started self.assertTrue(phase.pool.add.called) server_args = { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Server/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': self.topdir + '/compose/Server/$arch/os', 'variant': compose.variants['Server'], 'target': 'f24', @@ -177,7 +174,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': 'docker', 'name': 'Fedora-Docker-Base', 'kickstart': "fedora-docker-base.ks", 'distro': 'Fedora-20', @@ -196,12 +193,11 @@ class TestImageBuildPhase(PungiTestCase): # assert at least one thread was started self.assertTrue(phase.pool.add.called) server_args = { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Server/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': self.topdir + '/compose/Server/$arch/os', 'variant': compose.variants['Server'], 'target': 'f24', @@ -230,7 +226,7 @@ class TestImageBuildPhase(PungiTestCase): '^Client|Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -263,7 +259,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -294,12 +290,11 @@ class TestImageBuildPhase(PungiTestCase): args, kwargs = phase.pool.queue_put.call_args self.assertEqual(args[0][0], compose) self.assertDictEqual(args[0][1], { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Server-optional/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': self.topdir + '/compose/Server/$arch/os', 'variant': compose.variants['Server'], 'target': 'f24', @@ -325,7 +320,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -356,12 +351,11 @@ class TestImageBuildPhase(PungiTestCase): args, kwargs = phase.pool.queue_put.call_args self.assertEqual(args[0][0], compose) self.assertDictEqual(args[0][1], { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Server/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': ','.join([self.topdir + '/compose/Everything/$arch/os', self.topdir + '/compose/Server-optional/$arch/os', self.topdir + '/compose/Server/$arch/os']), @@ -389,7 +383,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -419,12 +413,11 @@ class TestImageBuildPhase(PungiTestCase): args, kwargs = phase.pool.queue_put.call_args self.assertEqual(args[0][0], compose) self.assertDictEqual(args[0][1], { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': 'http://example.com/install-tree/', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': ','.join([self.topdir + '/compose/Server/$arch/os']), 'variant': compose.variants['Server'], 'target': 'f24', @@ -450,7 +443,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -488,7 +481,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -526,7 +519,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -564,7 +557,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -604,7 +597,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server-optional$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -630,12 +623,11 @@ class TestImageBuildPhase(PungiTestCase): # assert at least one thread was started self.assertTrue(phase.pool.add.called) server_args = { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Server/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': self.topdir + '/compose/Server-optional/$arch/os', 'variant': compose.all_variants['Server-optional'], 'target': 'f24', @@ -664,7 +656,7 @@ class TestImageBuildPhase(PungiTestCase): '^Server$': [ { 'image-build': { - 'format': [('docker', 'tar.xz')], + 'format': ['docker'], 'name': 'Fedora-Docker-Base', 'target': 'f24', 'version': 'Rawhide', @@ -690,12 +682,11 @@ class TestImageBuildPhase(PungiTestCase): # assert at least one thread was started self.assertTrue(phase.pool.add.called) server_args = { - "format": [('docker', 'tar.xz')], "image_conf": { 'image-build': { 'install_tree': self.topdir + '/compose/Server/$arch/os', 'kickstart': 'fedora-docker-base.ks', - 'format': 'docker', + 'format': ['docker'], 'repo': self.topdir + '/compose/Server/$arch/os', 'variant': compose.all_variants['Server'], 'target': 'f24', @@ -730,12 +721,11 @@ class TestCreateImageBuildThread(PungiTestCase): }) 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', + 'format': ['docker', 'qcow2'], 'repo': '/ostree/$arch/Client', 'variant': compose.variants['Client'], 'target': 'f24', @@ -764,12 +754,12 @@ class TestCreateImageBuildThread(PungiTestCase): 'amd64': [ '/koji/task/1235/tdl-amd64.xml', '/koji/task/1235/Fedora-Docker-Base-20160103.amd64.qcow2', - '/koji/task/1235/Fedora-Docker-Base-20160103.amd64.tar.xz' + '/koji/task/1235/Fedora-Docker-Base-20160103.amd64.tar.gz' ], 'x86_64': [ '/koji/task/1235/tdl-x86_64.xml', '/koji/task/1235/Fedora-Docker-Base-20160103.x86_64.qcow2', - '/koji/task/1235/Fedora-Docker-Base-20160103.x86_64.tar.xz' + '/koji/task/1235/Fedora-Docker-Base-20160103.x86_64.tar.gz' ] } @@ -781,7 +771,6 @@ class TestCreateImageBuildThread(PungiTestCase): with mock.patch('time.sleep'): t.process((compose, cmd), 1) - self.assertEqual(cmd['image_conf']['image-build']['arches'], 'amd64,x86_64') self.assertItemsEqual( koji_wrapper.get_image_build_cmd.call_args_list, [mock.call(cmd['image_conf'], @@ -792,7 +781,7 @@ class TestCreateImageBuildThread(PungiTestCase): self.assertItemsEqual( koji_wrapper.run_blocking_cmd.call_args_list, [mock.call(koji_wrapper.get_image_build_cmd.return_value, - log_file=self.topdir + '/logs/amd64-x86_64/imagebuild-Client-KDE-docker.amd64-x86_64.log')] + log_file=self.topdir + '/logs/amd64-x86_64/imagebuild-Client-KDE-docker-qcow2.amd64-x86_64.log')] ) self.assertItemsEqual( @@ -802,16 +791,16 @@ class TestCreateImageBuildThread(PungiTestCase): self.topdir + '/compose/Client/amd64/images/Fedora-Docker-Base-20160103.amd64.qcow2', link_type='hardlink-or-copy'), mock.call.link( - '/koji/task/1235/Fedora-Docker-Base-20160103.amd64.tar.xz', - self.topdir + '/compose/Client/amd64/images/Fedora-Docker-Base-20160103.amd64.tar.xz', + '/koji/task/1235/Fedora-Docker-Base-20160103.amd64.tar.gz', + self.topdir + '/compose/Client/amd64/images/Fedora-Docker-Base-20160103.amd64.tar.gz', link_type='hardlink-or-copy'), mock.call.link( '/koji/task/1235/Fedora-Docker-Base-20160103.x86_64.qcow2', self.topdir + '/compose/Client/x86_64/images/Fedora-Docker-Base-20160103.x86_64.qcow2', link_type='hardlink-or-copy'), mock.call.link( - '/koji/task/1235/Fedora-Docker-Base-20160103.x86_64.tar.xz', - self.topdir + '/compose/Client/x86_64/images/Fedora-Docker-Base-20160103.x86_64.tar.xz', + '/koji/task/1235/Fedora-Docker-Base-20160103.x86_64.tar.gz', + self.topdir + '/compose/Client/x86_64/images/Fedora-Docker-Base-20160103.x86_64.tar.gz', link_type='hardlink-or-copy')]) image_relative_paths = { @@ -820,8 +809,8 @@ class TestCreateImageBuildThread(PungiTestCase): 'type': 'qcow2', 'arch': 'amd64', }, - 'image_dir/Client/amd64/Fedora-Docker-Base-20160103.amd64.tar.xz': { - 'format': 'tar.xz', + 'image_dir/Client/amd64/Fedora-Docker-Base-20160103.amd64.tar.gz': { + 'format': 'tar.gz', 'type': 'docker', 'arch': 'amd64', }, @@ -830,8 +819,8 @@ class TestCreateImageBuildThread(PungiTestCase): 'type': 'qcow2', 'arch': 'x86_64', }, - 'image_dir/Client/x86_64/Fedora-Docker-Base-20160103.x86_64.tar.xz': { - 'format': 'tar.xz', + 'image_dir/Client/x86_64/Fedora-Docker-Base-20160103.x86_64.tar.gz': { + 'format': 'tar.gz', 'type': 'docker', 'arch': 'x86_64', }, @@ -860,12 +849,11 @@ class TestCreateImageBuildThread(PungiTestCase): 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', + 'format': ['docker', 'qcow2'], 'repo': '/ostree/$arch/Client', 'variant': compose.variants['Client'], 'target': 'f24', @@ -899,7 +887,7 @@ class TestCreateImageBuildThread(PungiTestCase): mock.call('[FAIL] Image build (variant Client, arch *, subvariant Client) failed, but going on anyway.'), mock.call('ImageBuild task failed: 1234. See %s for more details.' % (os.path.join(self.topdir, - 'logs/amd64-x86_64/imagebuild-Client-Client-docker.amd64-x86_64.log'))), + 'logs/amd64-x86_64/imagebuild-Client-Client-docker-qcow2.amd64-x86_64.log'))), ]) @mock.patch('pungi.phases.image_build.KojiWrapper') @@ -908,12 +896,11 @@ class TestCreateImageBuildThread(PungiTestCase): 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', + 'format': ['docker', 'qcow2'], 'repo': '/ostree/$arch/Client', 'variant': compose.variants['Client'], 'target': 'f24', @@ -951,12 +938,11 @@ class TestCreateImageBuildThread(PungiTestCase): 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', + 'format': ['docker', 'qcow2'], 'repo': '/ostree/$arch/Client', 'variant': compose.variants['Client'], 'target': 'f24',