From f80c97e3ec9885a7fcd8d2e5214625df36460b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Mon, 22 Feb 2016 10:13:04 +0100 Subject: [PATCH] [live-images] Fix path processing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add option to keep filenames generated by Koji. * Put results of spin-appliance into image dir * On failure images are no longer deleted. Signed-off-by: Lubomír Sedlář --- doc/configuration.rst | 6 ++ pungi/phases/live_images.py | 118 ++++++++++++++-------------- tests/test_liveimagesphase.py | 142 ++++++++++++++++++++++++++++++---- 3 files changed, 192 insertions(+), 74 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index f1b4d0c8..03ab2275 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -689,6 +689,12 @@ Live Images Settings is ``appliance`` corresponding to ``koji spin-appliance``. * ``sign`` (*bool*) -- only RPM-wrapped images can be signed +**live_images_no_rename** + (*bool*) -- When set to ``True``, filenames generated by Koji will be used. + When ``False``, filenames will be generated based on ``image_name_format`` + configuration option. + + Live Media Settings =================== diff --git a/pungi/phases/live_images.py b/pungi/phases/live_images.py index 8b038c35..a4433af2 100644 --- a/pungi/phases/live_images.py +++ b/pungi/phases/live_images.py @@ -66,6 +66,11 @@ class LiveImagesPhase(PhaseBase): "expected_types": [str], "optional": True, }, + { + "name": "live_images_no_rename", + "expected_types": [bool], + "optional": True, + } ) def __init__(self, compose): @@ -116,21 +121,35 @@ class LiveImagesPhase(PhaseBase): for variant in self.compose.variants.values(): for arch in variant.arches + ["src"]: for data in get_arch_variant_data(self.compose.conf, "live_images", arch, variant): - iso_dir = self.compose.paths.compose.iso_dir(arch, variant, symlink_to=symlink_isos_to) - if not iso_dir: + type = data.get('type', 'live') + + if type == 'live': + dest_dir = self.compose.paths.compose.iso_dir(arch, variant, symlink_to=symlink_isos_to) + elif type == 'appliance': + dest_dir = self.compose.paths.compose.image_dir(variant, symlink_to=symlink_isos_to) + dest_dir = dest_dir % {'arch': arch} + else: + raise RuntimeError('Unknown live image type %s' % type) + if not dest_dir: continue cmd = { - "name": None, - "version": None, - "iso_path": None, - "wrapped_rpms_path": iso_dir, + "name": data.get("name", None), + "version": data.get("version", None), + "release": self._get_release(data), + "dest_dir": dest_dir, "build_arch": arch, "ks_file": data['kickstart'], "ksurl": None, - "specfile": None, - "scratch": False, + # Used for images wrapped in RPM + "specfile": data.get("specfile", None), + # Scratch (only taken in consideration if specfile + # specified) For images wrapped in rpm is scratch + # disabled by default For other images is scratch + # always on + "scratch": data.get("scratch", False), "sign": False, + "type": type, "label": "", # currently not used } @@ -139,42 +158,11 @@ class LiveImagesPhase(PhaseBase): cmd["repos"] = self._get_repos(arch, variant, data) - # Explicit name and version - cmd["name"] = data.get("name", None) - cmd["version"] = data.get("version", None) - - cmd['type'] = data.get('type', 'live') - cmd['release'] = self._get_release(data) - - # Specfile (for images wrapped in rpm) - cmd["specfile"] = data.get("specfile", None) - - # Scratch (only taken in consideration if specfile specified) - # For images wrapped in rpm is scratch disabled by default - # For other images is scratch always on - cmd["scratch"] = data.get("scratch", False) - # Signing of the rpm wrapped image if not cmd["scratch"] and data.get("sign"): cmd["sign"] = True - format = "%(compose_id)s-%(variant)s-%(arch)s-%(disc_type)s%(disc_num)s%(suffix)s" - # Custom name (prefix) - if cmd["name"]: - custom_iso_name = cmd["name"] - if cmd["version"]: - custom_iso_name += "-%s" % cmd["version"] - format = custom_iso_name + "-%(variant)s-%(arch)s-%(disc_type)s%(disc_num)s%(suffix)s" - - # XXX: hardcoded disc_type and disc_num - filename = self.compose.get_image_name(arch, variant, disc_type="live", - disc_num=None, format=format) - iso_path = self.compose.paths.compose.iso_path(arch, variant, filename, - symlink_to=symlink_isos_to) - if os.path.isfile(iso_path): - self.compose.log_warning("Skipping creating live image, it already exists: %s" % iso_path) - continue - cmd["iso_path"] = iso_path + cmd['filename'] = self._get_file_name(arch, variant, cmd['name'], cmd['version']) commands.append((cmd, variant, arch)) @@ -184,6 +172,22 @@ class LiveImagesPhase(PhaseBase): self.pool.start() + def _get_file_name(self, arch, variant, name=None, version=None): + if self.compose.conf.get('live_images_no_rename', False): + return None + + format = "%(compose_id)s-%(variant)s-%(arch)s-%(disc_type)s%(disc_num)s%(suffix)s" + # Custom name (prefix) + if name: + custom_iso_name = name + if version: + custom_iso_name += "-%s" % version + format = custom_iso_name + "-%(variant)s-%(arch)s-%(disc_type)s%(disc_num)s%(suffix)s" + + # XXX: hardcoded disc_type and disc_num + return self.compose.get_image_name(arch, variant, disc_type="live", + disc_num=None, format=format) + def stop(self, *args, **kwargs): PhaseBase.stop(self, *args, **kwargs) if self.skip(): @@ -191,14 +195,6 @@ class LiveImagesPhase(PhaseBase): class CreateLiveImageThread(WorkerThread): - def fail(self, compose, cmd): - compose.log_error("LiveImage failed, removing ISO: %s" % cmd["iso_path"]) - try: - # remove (possibly?) incomplete ISO - os.unlink(cmd["iso_path"]) - except OSError: - pass - def process(self, item, num): compose, cmd, variant, arch = item try: @@ -212,9 +208,10 @@ class CreateLiveImageThread(WorkerThread): self.pool.log_info(msg) def worker(self, compose, cmd, variant, arch, num): - log_file = compose.paths.log.log_file(arch, "createiso-%s" % os.path.basename(cmd["iso_path"])) + self.basename = '%(name)s-%(version)s-%(release)s' % cmd + log_file = compose.paths.log.log_file(arch, "createiso-%s" % self.basename) - msg = "Creating ISO (arch: %s, variant: %s): %s" % (arch, variant, os.path.basename(cmd["iso_path"])) + msg = "Creating ISO (arch: %s, variant: %s): %s" % (arch, variant, self.basename) self.pool.log_info("[BEGIN] %s" % msg) koji_wrapper = KojiWrapper(compose.conf["koji_profile"]) @@ -243,7 +240,6 @@ class CreateLiveImageThread(WorkerThread): output = koji_wrapper.run_blocking_cmd(koji_cmd, log_file=log_file) if output["retcode"] != 0: - self.fail(compose, cmd) raise RuntimeError("LiveImage task failed: %s. See %s for more details." % (output["task_id"], log_file)) # copy finished image to isos/ @@ -253,7 +249,9 @@ class CreateLiveImageThread(WorkerThread): raise RuntimeError('Got %d images from task %d, expected 1.' % (len(image_path), output['task_id'])) image_path = image_path[0] - shutil.copy2(image_path, cmd["iso_path"]) + filename = cmd.get('filename') or os.path.basename(image_path) + destination = os.path.join(cmd['dest_dir'], filename) + shutil.copy2(image_path, destination) # copy finished rpm to isos/ (if rpm wrapped ISO was built) if cmd["specfile"]: @@ -261,15 +259,16 @@ class CreateLiveImageThread(WorkerThread): if cmd["sign"]: # Sign the rpm wrapped images and get their paths - compose.log_info("Signing rpm wrapped images in task_id: %s (expected key ID: %s)" % (output["task_id"], compose.conf.get("signing_key_id"))) + compose.log_info("Signing rpm wrapped images in task_id: %s (expected key ID: %s)" + % (output["task_id"], compose.conf.get("signing_key_id"))) signed_rpm_paths = self._sign_image(koji_wrapper, compose, cmd, output["task_id"]) if signed_rpm_paths: rpm_paths = signed_rpm_paths for rpm_path in rpm_paths: - shutil.copy2(rpm_path, cmd["wrapped_rpms_path"]) + shutil.copy2(rpm_path, cmd["dest_dir"]) - self._write_manifest(cmd['iso_path']) + self._write_manifest(destination) self.pool.log_info("[DONE ] %s" % msg) @@ -302,11 +301,14 @@ class CreateLiveImageThread(WorkerThread): return None # Prepare signing log file - signing_log_file = compose.paths.log.log_file(cmd["build_arch"], "live_images-signing-%s" % os.path.basename(cmd["iso_path"])) + signing_log_file = compose.paths.log.log_file(cmd["build_arch"], + "live_images-signing-%s" % self.basename) # Sign the rpm wrapped images try: - sign_builds_in_task(koji_wrapper, koji_task_id, signing_command, log_file=signing_log_file, signing_key_password=compose.conf.get("signing_key_password")) + sign_builds_in_task(koji_wrapper, koji_task_id, signing_command, + log_file=signing_log_file, + signing_key_password=compose.conf.get("signing_key_password")) except RuntimeError: compose.log_error("Error while signing rpm wrapped images. See log: %s" % signing_log_file) raise @@ -316,7 +318,7 @@ class CreateLiveImageThread(WorkerThread): rpm_paths = koji_wrapper.get_signed_wrapped_rpms_paths(koji_task_id, signing_key_id) # Wait untill files are available - if wait_paths(rpm_paths, 60*15): + if wait_paths(rpm_paths, 60 * 15): # Files are ready return rpm_paths diff --git a/tests/test_liveimagesphase.py b/tests/test_liveimagesphase.py index de255e9b..8eb69a33 100755 --- a/tests/test_liveimagesphase.py +++ b/tests/test_liveimagesphase.py @@ -30,6 +30,11 @@ class _DummyCompose(object): '/iso_dir', arch, variant.uid ) ), + image_dir=mock.Mock( + side_effect=lambda variant, symlink_to: os.path.join( + '/image_dir/%(arch)s', variant.uid + ) + ), iso_path=mock.Mock( side_effect=lambda arch, variant, filename, symlink_to: os.path.join( '/iso_dir', arch, variant.uid, filename @@ -88,14 +93,58 @@ class TestLiveImagesPhase(unittest.TestCase): [mock.call((compose, {'ks_file': 'test.ks', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/iso_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/', '/repo/amd64/Everything'], 'label': '', 'name': None, - 'iso_path': '/iso_dir/amd64/Client/image-name', + 'filename': 'image-name', + 'version': None, + 'specfile': None, + 'sign': False, + 'type': 'live', + 'release': '20151203.0', + 'ksurl': None}, + compose.variants['Client'], + 'amd64'))]) + + @mock.patch('pungi.phases.live_images.ThreadPool') + def test_live_image_build_without_rename(self, ThreadPool): + compose = _DummyCompose({ + 'live_images_no_rename': True, + 'live_images': [ + ('^Client$', { + 'amd64': { + 'kickstart': 'test.ks', + 'additional_repos': ['http://example.com/repo/'], + 'repo_from': ['Everything'], + 'release': None, + } + }) + ], + }) + + phase = LiveImagesPhase(compose) + + phase.run() + + # assert at least one thread was started + self.assertTrue(phase.pool.add.called) + self.maxDiff = None + self.assertItemsEqual(phase.pool.queue_put.mock_calls, + [mock.call((compose, + {'ks_file': 'test.ks', + 'build_arch': 'amd64', + 'dest_dir': '/iso_dir/amd64/Client', + 'scratch': False, + 'repos': ['/repo/amd64/Client', + 'http://example.com/repo/', + '/repo/amd64/Everything'], + 'label': '', + 'name': None, + 'filename': None, 'version': None, 'specfile': None, 'sign': False, @@ -134,14 +183,14 @@ class TestLiveImagesPhase(unittest.TestCase): [mock.call((compose, {'ks_file': 'test.ks', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/iso_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/', '/repo/amd64/Everything'], 'label': '', 'name': None, - 'iso_path': '/iso_dir/amd64/Client/image-name', + 'filename': 'image-name', 'version': None, 'specfile': None, 'sign': False, @@ -153,14 +202,14 @@ class TestLiveImagesPhase(unittest.TestCase): mock.call((compose, {'ks_file': 'another.ks', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/iso_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/', '/repo/amd64/Everything'], 'label': '', 'name': None, - 'iso_path': '/iso_dir/amd64/Client/image-name', + 'filename': 'image-name', 'version': None, 'specfile': None, 'sign': False, @@ -200,14 +249,14 @@ class TestLiveImagesPhase(unittest.TestCase): [mock.call((compose, {'ks_file': 'test.ks', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/image_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/', '/repo/amd64/Everything'], 'label': '', 'name': None, - 'iso_path': '/iso_dir/amd64/Client/image-name', + 'filename': 'image-name', 'version': None, 'specfile': None, 'sign': False, @@ -231,14 +280,14 @@ class TestCreateLiveImageThread(unittest.TestCase): cmd = { 'ks_file': '/path/to/ks_file', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/iso_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/', '/repo/amd64/Everything'], 'label': '', 'name': None, - 'iso_path': '/iso_dir/amd64/Client/image-name', + 'filename': 'image-name', 'version': None, 'specfile': None, 'type': 'live', @@ -286,20 +335,81 @@ class TestCreateLiveImageThread(unittest.TestCase): @mock.patch('shutil.copy2') @mock.patch('pungi.phases.live_images.run') @mock.patch('pungi.phases.live_images.KojiWrapper') - def test_process_applicance(self, KojiWrapper, run, copy2): + def test_process_no_rename(self, KojiWrapper, run, copy2): compose = _DummyCompose({'koji_profile': 'koji'}) pool = mock.Mock() cmd = { 'ks_file': '/path/to/ks_file', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/iso_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/', '/repo/amd64/Everything'], 'label': '', 'name': None, - 'iso_path': '/iso_dir/amd64/Client/image-name', + 'filename': None, + 'version': None, + 'specfile': None, + 'type': 'live', + 'ksurl': 'https://git.example.com/kickstarts.git?#CAFEBABE', + 'release': None, + } + + koji_wrapper = KojiWrapper.return_value + koji_wrapper.get_create_image_cmd.return_value = 'koji spin-livecd ...' + koji_wrapper.run_blocking_cmd.return_value = { + 'retcode': 0, + 'output': 'some output', + 'task_id': 123 + } + koji_wrapper.get_image_path.return_value = ['/path/to/image.iso'] + + t = CreateLiveImageThread(pool) + with mock.patch('time.sleep'): + t.process((compose, cmd, compose.variants['Client'], 'amd64'), 1) + + self.assertEqual(koji_wrapper.run_blocking_cmd.mock_calls, + [mock.call('koji spin-livecd ...', log_file='/a/b/log/log_file')]) + self.assertEqual(koji_wrapper.get_image_path.mock_calls, [mock.call(123)]) + self.assertEqual(copy2.mock_calls, + [mock.call('/path/to/image.iso', '/iso_dir/amd64/Client/image.iso')]) + + write_manifest_cmd = ' && '.join([ + 'cd /iso_dir/amd64/Client', + 'isoinfo -R -f -i image.iso | grep -v \'/TRANS.TBL$\' | sort >> image.iso.manifest' + ]) + self.assertEqual(run.mock_calls, [mock.call(write_manifest_cmd)]) + self.assertEqual(koji_wrapper.get_create_image_cmd.mock_calls, + [mock.call('Test', '20151203.0.t', 'rhel-7.0-candidate', + 'amd64', '/path/to/ks_file', + ['/repo/amd64/Client', + 'http://example.com/repo/', + '/repo/amd64/Everything'], + image_type='live', + archive=False, + specfile=None, + wait=True, + release=None, + ksurl='https://git.example.com/kickstarts.git?#CAFEBABE')]) + + @mock.patch('shutil.copy2') + @mock.patch('pungi.phases.live_images.run') + @mock.patch('pungi.phases.live_images.KojiWrapper') + def test_process_applicance(self, KojiWrapper, run, copy2): + compose = _DummyCompose({'koji_profile': 'koji'}) + pool = mock.Mock() + cmd = { + 'ks_file': '/path/to/ks_file', + 'build_arch': 'amd64', + 'dest_dir': '/iso_dir/amd64/Client', + 'scratch': False, + 'repos': ['/repo/amd64/Client', + 'http://example.com/repo/', + '/repo/amd64/Everything'], + 'label': '', + 'name': None, + 'filename': 'image-name', 'version': None, 'specfile': None, 'type': 'appliance', @@ -356,14 +466,14 @@ class TestCreateLiveImageThread(unittest.TestCase): cmd = { 'ks_file': '/path/to/ks_file', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/iso_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/', '/repo/amd64/Everything'], 'label': '', 'name': None, - 'iso_path': '/iso_dir/amd64/Client/image-name', + 'filename': 'image-name', 'version': None, 'specfile': None, 'ksurl': None, @@ -393,7 +503,7 @@ class TestCreateLiveImageThread(unittest.TestCase): cmd = { 'ks_file': '/path/to/ks_file', 'build_arch': 'amd64', - 'wrapped_rpms_path': '/iso_dir/amd64/Client', + 'dest_dir': '/iso_dir/amd64/Client', 'scratch': False, 'repos': ['/repo/amd64/Client', 'http://example.com/repo/',