From cfda99e6fe8b0ac79ccea4c7862b6fcfa25c71b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Wed, 2 Dec 2015 15:55:53 +0100 Subject: [PATCH 1/4] Add basic tests for buildinstall phase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test only checks that commands are created with correct arguments and that a proper number of threads is started. There is no validation for what the threads are actually doing. Signed-off-by: Lubomír Sedlář --- tests/test_buildinstall.py | 122 +++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100755 tests/test_buildinstall.py diff --git a/tests/test_buildinstall.py b/tests/test_buildinstall.py new file mode 100755 index 00000000..3b850369 --- /dev/null +++ b/tests/test_buildinstall.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python2 +# -*- coding: utf-8 -*- + + +import unittest +import mock + +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from pungi.phases.buildinstall import BuildinstallPhase + + +class _DummyCompose(object): + def __init__(self, config): + self.conf = config + self.paths = mock.Mock( + compose=mock.Mock( + topdir=mock.Mock(return_value='/a/b') + ), + work=mock.Mock( + arch_repo=mock.Mock(return_value='file:///a/b/'), + buildinstall_dir=mock.Mock(side_effect=lambda x: '/buildinstall_dir/' + x), + ) + ) + self._logger = mock.Mock() + self.log_debug = mock.Mock() + self.supported = True + + def get_arches(self): + return ['x86_64', 'amd64'] + + +class TestImageChecksumPhase(unittest.TestCase): + + def test_config_skip_unless_bootable(self): + compose = _DummyCompose({}) + compose.just_phases = None + compose.skip_phases = [] + + phase = BuildinstallPhase(compose) + + self.assertTrue(phase.skip()) + + def test_does_not_skip_on_bootable(self): + compose = _DummyCompose({'bootable': True}) + compose.just_phases = None + compose.skip_phases = [] + + phase = BuildinstallPhase(compose) + + self.assertFalse(phase.skip()) + + @mock.patch('pungi.phases.buildinstall.ThreadPool') + @mock.patch('pungi.phases.buildinstall.LoraxWrapper') + @mock.patch('pungi.phases.buildinstall.get_volid') + def test_starts_threads_for_each_cmd_with_lorax(self, get_volid, loraxCls, poolCls): + compose = _DummyCompose({ + 'bootable': True, + 'release_name': 'Test', + 'release_short': 't', + 'release_version': '1', + 'release_is_layered': False, + 'buildinstall_method': 'lorax' + }) + + get_volid.return_value = 'vol_id' + + phase = BuildinstallPhase(compose) + + phase.run() + + # Two items added for processing in total. + pool = poolCls.return_value + self.assertEqual(2, len(pool.queue_put.mock_calls)) + + # Obtained correct lorax commands. + lorax = loraxCls.return_value + lorax.get_lorax_cmd.assert_has_calls( + [mock.call('Test', '1', '1', 'file:///a/b/', '/buildinstall_dir/x86_64', + buildarch='x86_64', is_final=True, nomacboot=True, noupgrade=True, volid='vol_id'), + mock.call('Test', '1', '1', 'file:///a/b/', '/buildinstall_dir/amd64', + buildarch='amd64', is_final=True, nomacboot=True, noupgrade=True, volid='vol_id')], + any_order=True) + + @mock.patch('pungi.phases.buildinstall.ThreadPool') + @mock.patch('pungi.phases.buildinstall.LoraxWrapper') + @mock.patch('pungi.phases.buildinstall.get_volid') + def test_starts_threads_for_each_cmd_with_buildinstall(self, get_volid, loraxCls, poolCls): + compose = _DummyCompose({ + 'bootable': True, + 'release_name': 'Test', + 'release_short': 't', + 'release_version': '1', + 'release_is_layered': False, + 'buildinstall_method': 'buildinstall' + }) + + get_volid.return_value = 'vol_id' + + phase = BuildinstallPhase(compose) + + phase.run() + + # Two items added for processing in total. + pool = poolCls.return_value + self.assertEqual(2, len(pool.queue_put.mock_calls)) + + # Obtained correct lorax commands. + lorax = loraxCls.return_value + lorax.get_buildinstall_cmd.assert_has_calls( + [mock.call('Test', '1', '1', 'file:///a/b/', '/buildinstall_dir/x86_64', + buildarch='x86_64', is_final=True, volid='vol_id'), + mock.call('Test', '1', '1', 'file:///a/b/', '/buildinstall_dir/amd64', + buildarch='amd64', is_final=True, volid='vol_id')], + any_order=True) + + +if __name__ == "__main__": + unittest.main() From 791448cc3265a0bfa2f34b126a698780e1118720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 3 Dec 2015 13:12:59 +0100 Subject: [PATCH 2/4] Allow specifying which packages to install in variants xml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch updates the variants DTD and parsing to allow specifying multiple build install packages for each variant. Signed-off-by: Lubomír Sedlář --- pungi/wrappers/variants.py | 9 ++++++++- share/variants.dtd | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pungi/wrappers/variants.py b/pungi/wrappers/variants.py index 6355fdea..ba1e0382 100755 --- a/pungi/wrappers/variants.py +++ b/pungi/wrappers/variants.py @@ -70,6 +70,7 @@ class VariantsXmlParser(object): "arches": [str(i) for i in variant_node.xpath("arches/arch/text()")], "groups": [], "environments": [], + "buildinstallpackages": [], } if self.tree_arches: variant_dict["arches"] = [i for i in variant_dict["arches"] if i in self.tree_arches] @@ -106,6 +107,10 @@ class VariantsXmlParser(object): variant_dict["environments"].append(environment) + for buildinstallpackages_node in variant_node.xpath("buildinstallpackages"): + for package_node in buildinstallpackages_node.xpath("package"): + variant_dict["buildinstallpackages"].append(package_node.text) + variant = Variant(**variant_dict) if variant.type == "layered-product": release_node = variant_node.xpath("release")[0] @@ -157,11 +162,12 @@ class VariantsXmlParser(object): class Variant(object): - def __init__(self, id, name, type, arches, groups, environments=None): + def __init__(self, id, name, type, arches, groups, environments=None, buildinstallpackages=None): if not id.isalnum(): raise ValueError("Variant ID must contain only alphanumeric characters: %s" % id) environments = environments or [] + buildinstallpackages = buildinstallpackages or [] self.id = id self.name = name @@ -169,6 +175,7 @@ class Variant(object): self.arches = sorted(copy.deepcopy(arches)) self.groups = sorted(copy.deepcopy(groups), lambda x, y: cmp(x["name"], y["name"])) self.environments = sorted(copy.deepcopy(environments), lambda x, y: cmp(x["name"], y["name"])) + self.buildinstallpackages = sorted(buildinstallpackages) self.variants = {} self.parent = None diff --git a/share/variants.dtd b/share/variants.dtd index 718510d1..1944d022 100644 --- a/share/variants.dtd +++ b/share/variants.dtd @@ -1,6 +1,6 @@ - + + + + From 94f519d01cecd6a5caa3870e8f0c8078695b8438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 3 Dec 2015 14:19:15 +0100 Subject: [PATCH 3/4] Update lorax wrapper to use --installpkgs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lorax wrapper class now understands the --installpkgs argument and can use it to pass multiple package names to the command. There is a simple test to make sure the commands includes all specified options. A bug is fixed where the bug URL would not be correctly included. Signed-off-by: Lubomír Sedlář --- pungi/wrappers/lorax.py | 9 +++++-- tests/test_lorax_wrapper.py | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100755 tests/test_lorax_wrapper.py diff --git a/pungi/wrappers/lorax.py b/pungi/wrappers/lorax.py index 80d7f9f8..133c0e4c 100644 --- a/pungi/wrappers/lorax.py +++ b/pungi/wrappers/lorax.py @@ -21,7 +21,9 @@ from kobo.shortcuts import force_list class LoraxWrapper(object): - def get_lorax_cmd(self, product, version, release, repo_baseurl, output_dir, variant=None, bugurl=None, nomacboot=False, noupgrade=False, is_final=False, buildarch=None, volid=None): + def get_lorax_cmd(self, product, version, release, repo_baseurl, output_dir, + variant=None, bugurl=None, nomacboot=False, noupgrade=False, + is_final=False, buildarch=None, volid=None, buildinstallpackages=None): cmd = ["lorax"] cmd.append("--product=%s" % product) cmd.append("--version=%s" % version) @@ -36,7 +38,7 @@ class LoraxWrapper(object): cmd.append("--variant=%s" % variant) if bugurl is not None: - cmd.append("--bugurl=%s" % variant) + cmd.append("--bugurl=%s" % bugurl) if nomacboot: cmd.append("--nomacboot") @@ -53,6 +55,9 @@ class LoraxWrapper(object): if volid: cmd.append("--volid=%s" % volid) + if buildinstallpackages: + cmd.extend(["--installpkgs=%s" % package for package in buildinstallpackages]) + output_dir = os.path.abspath(output_dir) cmd.append(output_dir) diff --git a/tests/test_lorax_wrapper.py b/tests/test_lorax_wrapper.py new file mode 100755 index 00000000..938ea9d7 --- /dev/null +++ b/tests/test_lorax_wrapper.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python2 +# -*- coding: utf-8 -*- + +import unittest + +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from pungi.wrappers.lorax import LoraxWrapper + + +class LoraxWrapperTest(unittest.TestCase): + + def setUp(self): + self.lorax = LoraxWrapper() + + def test_get_command_with_minimal_arguments(self): + cmd = self.lorax.get_lorax_cmd("product", "version", "release", + "/mnt/repo_baseurl", "/mnt/output_dir") + + self.assertEqual(cmd[0], 'lorax') + self.assertItemsEqual(cmd[1:], + ['--product=product', + '--version=version', + '--release=release', + '--source=file:///mnt/repo_baseurl', + '/mnt/output_dir']) + + def test_get_command_with_all_arguments(self): + cmd = self.lorax.get_lorax_cmd("product", "version", "release", + "/mnt/repo_baseurl", "/mnt/output_dir", + variant="Server", bugurl="http://example.com/", + nomacboot=True, noupgrade=True, is_final=True, + buildarch='x86_64', volid='VOLUME_ID', + buildinstallpackages=['bash', 'vim']) + + self.assertEqual(cmd[0], 'lorax') + self.assertItemsEqual(cmd[1:], + ['--product=product', '--version=version', + '--release=release', '--variant=Server', + '--source=file:///mnt/repo_baseurl', + '--bugurl=http://example.com/', + '--buildarch=x86_64', '--volid=VOLUME_ID', + '--nomacboot', '--noupgrade', '--isfinal', + '--installpkgs=bash', '--installpkgs=vim', + '/mnt/output_dir']) + + +if __name__ == "__main__": + unittest.main() From cdec198b6d766a24fc50755af6afef50e4b3926a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Wed, 2 Dec 2015 14:46:39 +0100 Subject: [PATCH 4/4] Start lorax for each variant separately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The buildinstall phase now starts lorax for each variant separately. Running with buildinstall should not be affected in any way. Only variants of type=variant are considered. The side-effect of this is that if an architecture is only used by variants of other types, lorax will not be called at all. Signed-off-by: Lubomír Sedlář --- pungi/phases/buildinstall.py | 28 ++++++++++++++++++++++++++-- tests/test_buildinstall.py | 22 ++++++++++++++++++---- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/pungi/phases/buildinstall.py b/pungi/phases/buildinstall.py index 1d7ffc54..1bd8f030 100644 --- a/pungi/phases/buildinstall.py +++ b/pungi/phases/buildinstall.py @@ -86,6 +86,8 @@ class BuildinstallPhase(PhaseBase): noupgrade = not self.compose.conf.get("buildinstall_upgrade_image", False) buildinstall_method = self.compose.conf["buildinstall_method"] + commands = [] + for arch in self.compose.get_arches(): repo_baseurl = self.compose.paths.work.arch_repo(arch) output_dir = self.compose.paths.work.buildinstall_dir(arch) @@ -93,11 +95,33 @@ class BuildinstallPhase(PhaseBase): buildarch = get_valid_arches(arch)[0] if buildinstall_method == "lorax": - cmd = lorax.get_lorax_cmd(product, version, release, repo_baseurl, output_dir, is_final=self.compose.supported, buildarch=buildarch, volid=volid, nomacboot=True, noupgrade=noupgrade) + for variant in self.compose.get_variants(arch=arch, types=['variant']): + commands.append( + lorax.get_lorax_cmd(product, + version, + release, + repo_baseurl, + output_dir, + variant=variant.uid, + buildinstallpackages=variant.buildinstallpackages, + is_final=self.compose.supported, + buildarch=buildarch, + volid=volid, + nomacboot=True, + noupgrade=noupgrade)) elif buildinstall_method == "buildinstall": - cmd = lorax.get_buildinstall_cmd(product, version, release, repo_baseurl, output_dir, is_final=self.compose.supported, buildarch=buildarch, volid=volid) + commands.append(lorax.get_buildinstall_cmd(product, + version, + release, + repo_baseurl, + output_dir, + is_final=self.compose.supported, + buildarch=buildarch, + volid=volid)) else: raise ValueError("Unsupported buildinstall method: %s" % buildinstall_method) + + for cmd in commands: self.pool.add(BuildinstallThread(self.pool)) self.pool.queue_put((self.compose, arch, cmd)) diff --git a/tests/test_buildinstall.py b/tests/test_buildinstall.py index 3b850369..b5e8f9e5 100755 --- a/tests/test_buildinstall.py +++ b/tests/test_buildinstall.py @@ -32,6 +32,14 @@ class _DummyCompose(object): def get_arches(self): return ['x86_64', 'amd64'] + def get_variants(self, arch, types): + variants = { + 'x86_64': [mock.Mock(uid='Server', buildinstallpackages=['bash', 'vim'])], + 'amd64': [mock.Mock(uid='Client', buildinstallpackages=[]), + mock.Mock(uid='Server', buildinstallpackages=['bash', 'vim'])], + } + return variants.get(arch, []) + class TestImageChecksumPhase(unittest.TestCase): @@ -72,17 +80,23 @@ class TestImageChecksumPhase(unittest.TestCase): phase.run() - # Two items added for processing in total. + # Three items added for processing in total. + # Server.x86_64, Client.amd64, Server.x86_64 pool = poolCls.return_value - self.assertEqual(2, len(pool.queue_put.mock_calls)) + self.assertEqual(3, len(pool.queue_put.mock_calls)) # Obtained correct lorax commands. lorax = loraxCls.return_value lorax.get_lorax_cmd.assert_has_calls( [mock.call('Test', '1', '1', 'file:///a/b/', '/buildinstall_dir/x86_64', - buildarch='x86_64', is_final=True, nomacboot=True, noupgrade=True, volid='vol_id'), + buildarch='x86_64', is_final=True, nomacboot=True, noupgrade=True, + volid='vol_id', variant='Server', buildinstallpackages=['bash', 'vim']), mock.call('Test', '1', '1', 'file:///a/b/', '/buildinstall_dir/amd64', - buildarch='amd64', is_final=True, nomacboot=True, noupgrade=True, volid='vol_id')], + buildarch='amd64', is_final=True, nomacboot=True, noupgrade=True, + volid='vol_id', variant='Server', buildinstallpackages=['bash', 'vim']), + mock.call('Test', '1', '1', 'file:///a/b/', '/buildinstall_dir/amd64', + buildarch='amd64', is_final=True, nomacboot=True, noupgrade=True, + volid='vol_id', variant='Client', buildinstallpackages=[])], any_order=True) @mock.patch('pungi.phases.buildinstall.ThreadPool')