diff --git a/pungi/checks.py b/pungi/checks.py index e5c2f225..d262f313 100644 --- a/pungi/checks.py +++ b/pungi/checks.py @@ -653,6 +653,7 @@ def make_schema(): "gather_profiler": {"type": "boolean", "default": False}, "gather_allow_reuse": {"type": "boolean", "default": False}, "pkgset_allow_reuse": {"type": "boolean", "default": True}, + "createiso_allow_reuse": {"type": "boolean", "default": True}, "pkgset_source": {"type": "string", "enum": ["koji", "repos"]}, "createrepo_c": {"type": "boolean", "default": True}, "createrepo_checksum": { diff --git a/pungi/phases/createiso.py b/pungi/phases/createiso.py index 529101f5..706ba577 100644 --- a/pungi/phases/createiso.py +++ b/pungi/phases/createiso.py @@ -18,6 +18,7 @@ import os import random import shutil import stat +import json import productmd.treeinfo from productmd.images import Image @@ -36,6 +37,7 @@ from pungi.util import ( failable, get_file_size, get_mtime, + read_json_file, ) from pungi.media_split import MediaSplitter, convert_media_size from pungi.compose_metadata.discinfo import read_discinfo, write_discinfo @@ -73,6 +75,168 @@ class CreateisoPhase(PhaseLoggerMixin, PhaseBase): return False return bool(self.compose.conf.get("buildinstall_method", "")) + def _metadata_path(self, variant, arch, disc_num, disc_count): + return self.compose.paths.log.log_file( + arch, + "createiso-%s-%d-%d" % (variant.uid, disc_num, disc_count), + ext="json", + ) + + def save_reuse_metadata(self, cmd, variant, arch, opts): + """Save metadata for future composes to verify if the compose can be reused.""" + metadata = { + "cmd": cmd, + "opts": opts._asdict(), + } + + metadata_path = self._metadata_path( + variant, arch, cmd["disc_num"], cmd["disc_count"] + ) + with open(metadata_path, "w") as f: + json.dump(metadata, f, indent=2) + return metadata + + def _load_old_metadata(self, cmd, variant, arch): + metadata_path = self._metadata_path( + variant, arch, cmd["disc_num"], cmd["disc_count"] + ) + old_path = self.compose.paths.old_compose_path(metadata_path) + self.logger.info( + "Loading old metadata for %s.%s from: %s", variant, arch, old_path + ) + try: + return read_json_file(old_path) + except Exception: + return None + + def perform_reuse(self, cmd, variant, arch, opts, iso_path): + """ + Copy all related files from old compose to the new one. As a last step + add the new image to metadata. + """ + linker = OldFileLinker(self.logger) + old_file_name = os.path.basename(iso_path) + current_file_name = os.path.basename(cmd["iso_path"]) + try: + # Hardlink ISO and manifest + for suffix in ("", ".manifest"): + linker.link(iso_path + suffix, cmd["iso_path"] + suffix) + # Copy log files + # The log file name includes filename of the image, so we need to + # find old file with the old name, and rename it to the new name. + log_file = self.compose.paths.log.log_file( + arch, "createiso-%s" % current_file_name + ) + old_log_file = self.compose.paths.old_compose_path( + self.compose.paths.log.log_file(arch, "createiso-%s" % old_file_name) + ) + linker.link(old_log_file, log_file) + # Copy jigdo files + if opts.jigdo_dir: + old_jigdo_dir = self.compose.paths.old_compose_path(opts.jigdo_dir) + for suffix in (".template", ".jigdo"): + linker.link( + os.path.join(old_jigdo_dir, old_file_name) + suffix, + os.path.join(opts.jigdo_dir, current_file_name) + suffix, + ) + except Exception: + # A problem happened while linking some file, let's clean up + # everything. + linker.abort() + raise + # Add image to manifest + add_iso_to_metadata( + self.compose, + variant, + arch, + cmd["iso_path"], + bootable=cmd["bootable"], + disc_num=cmd["disc_num"], + disc_count=cmd["disc_count"], + ) + + def try_reuse(self, cmd, variant, arch, opts): + """Try to reuse image from previous compose. + + :returns bool: True if reuse was successful, False otherwise + """ + if not self.compose.conf["createiso_allow_reuse"]: + return + + log_msg = "Cannot reuse ISO for %s.%s" % (variant, arch) + current_metadata = self.save_reuse_metadata(cmd, variant, arch, opts) + + if opts.buildinstall_method and not self.bi.reused(variant, arch): + # If buildinstall phase was not reused for some reason, we can not + # reuse any bootable image. If a package change caused rebuild of + # boot.iso, we would catch it here too, but there could be a + # configuration change in lorax template which would remain + # undetected. + self.logger.info("%s - boot configuration changed", log_msg) + return False + + # Check old compose configuration: extra_files and product_ids can be + # reflected on ISO. + old_config = self.compose.load_old_compose_config() + if not old_config: + self.logger.info("%s - no config for old compose", log_msg) + return False + # Convert current configuration to JSON and back to encode it similarly + # to the old one + config = json.loads(json.dumps(self.compose.conf)) + for opt in self.compose.conf: + # Skip a selection of options: these affect what packages can be + # included, which we explicitly check later on. + config_whitelist = set( + [ + "gather_lookaside_repos", + "pkgset_koji_builds", + "pkgset_koji_scratch_tasks", + "pkgset_koji_module_builds", + ] + ) + if opt in config_whitelist: + continue + + if old_config.get(opt) != config.get(opt): + self.logger.info("%s - option %s differs", log_msg, opt) + return False + + old_metadata = self._load_old_metadata(cmd, variant, arch) + if not old_metadata: + self.logger.info("%s - no old metadata found", log_msg) + return False + + # Test if volume ID matches - volid can be generated dynamically based on + # other values, and could change even if nothing else is different. + if current_metadata["opts"]["volid"] != old_metadata["opts"]["volid"]: + self.logger.info("%s - volume ID differs", log_msg) + return False + + # Compare packages on the ISO. + if compare_packages( + old_metadata["opts"]["graft_points"], + current_metadata["opts"]["graft_points"], + ): + self.logger.info("%s - packages differ", log_msg) + return False + + try: + self.perform_reuse( + cmd, + variant, + arch, + opts, + old_metadata["cmd"]["iso_path"], + ) + return True + except Exception as exc: + self.compose.log_error( + "Error while reusing ISO for %s.%s: %s", variant, arch, exc + ) + self.compose.traceback("createiso-reuse-%s-%s" % (variant, arch)) + return False + def run(self): symlink_isos_to = self.compose.conf.get("symlink_isos_to") disc_type = self.compose.conf["disc_types"].get("dvd", "dvd") @@ -184,6 +348,11 @@ class CreateisoPhase(PhaseLoggerMixin, PhaseBase): jigdo_dir = self.compose.paths.compose.jigdo_dir(arch, variant) opts = opts._replace(jigdo_dir=jigdo_dir, os_tree=os_tree) + # Try to reuse + if self.try_reuse(cmd, variant, arch, opts): + # Reuse was successful, go to next ISO + continue + script_file = os.path.join( self.compose.paths.work.tmp_dir(arch, variant), "createiso-%s.sh" % filename, @@ -203,6 +372,29 @@ class CreateisoPhase(PhaseLoggerMixin, PhaseBase): self.pool.start() +def read_packages(graft_points): + """Read packages that were listed in given graft points file. + + Only files under Packages directory are considered. Particularly this + excludes .discinfo, .treeinfo and media.repo as well as repodata and + any extra files. + + Extra files are easier to check by configuration (same name doesn't + imply same content). Repodata depend entirely on included packages (and + possibly product id certificate), but are affected by current time + which can change checksum despite data being the same. + """ + with open(graft_points) as f: + return set(line.split("=", 1)[0] for line in f if line.startswith("Packages/")) + + +def compare_packages(old_graft_points, new_graft_points): + """Read packages from the two files and compare them.""" + old_files = read_packages(old_graft_points) + new_files = read_packages(new_graft_points) + return old_files != new_files + + class CreateIsoThread(WorkerThread): def fail(self, compose, cmd, variant, arch): self.pool.log_error("CreateISO failed, removing ISO: %s" % cmd["iso_path"]) @@ -599,3 +791,24 @@ def create_hardlinks(staging_dir, log_file): """ cmd = ["/usr/sbin/hardlink", "-c", "-vv", staging_dir] run(cmd, logfile=log_file, show_cmd=True) + + +class OldFileLinker(object): + """ + A wrapper around os.link that remembers which files were linked and can + clean them up. + """ + + def __init__(self, logger): + self.logger = logger + self.linked_files = [] + + def link(self, src, dst): + self.logger.debug("Hardlinking %s to %s", src, dst) + os.link(src, dst) + self.linked_files.append(dst) + + def abort(self): + """Clean up all files created by this instance.""" + for f in self.linked_files: + os.unlink(f) diff --git a/pungi/util.py b/pungi/util.py index c6a40d4d..06b657ad 100644 --- a/pungi/util.py +++ b/pungi/util.py @@ -1130,3 +1130,9 @@ class PartialFuncThreadPool(ThreadPool): @property def results(self): return self._results + + +def read_json_file(file_path): + """A helper function to read a JSON file.""" + with open(file_path) as f: + return json.load(f) diff --git a/tests/test_createiso_phase.py b/tests/test_createiso_phase.py index 48bf37b7..180760a5 100644 --- a/tests/test_createiso_phase.py +++ b/tests/test_createiso_phase.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- +import logging import mock import six @@ -1322,3 +1323,227 @@ class TweakTreeinfo(helpers.PungiTestCase): ti.dump(output) self.assertFilesEqual(output, expected) + + +class CreateisoTryReusePhaseTest(helpers.PungiTestCase): + def setUp(self): + super(CreateisoTryReusePhaseTest, self).setUp() + self.logger = logging.getLogger() + self.logger.setLevel(logging.DEBUG) + self.logger.addHandler(logging.NullHandler()) + + def test_disabled(self): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": False}) + phase = createiso.CreateisoPhase(compose, mock.Mock()) + + self.assertFalse(phase.try_reuse(mock.Mock(), "Server", "x86_64", mock.Mock())) + + def test_buildinstall_changed(self): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + phase.bi = mock.Mock() + phase.bi.reused.return_value = False + cmd = {"disc_num": 1, "disc_count": 1} + opts = CreateIsoOpts(buildinstall_method="lorax") + + self.assertFalse( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + + def test_no_old_config(self): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + cmd = {"disc_num": 1, "disc_count": 1} + opts = CreateIsoOpts() + + self.assertFalse( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + + def test_old_config_changed(self): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + old_config = compose.conf.copy() + old_config["release_version"] = "2" + compose.load_old_compose_config.return_value = old_config + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + cmd = {"disc_num": 1, "disc_count": 1} + opts = CreateIsoOpts() + + self.assertFalse( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + + def test_no_old_metadata(self): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose.load_old_compose_config.return_value = compose.conf.copy() + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + cmd = {"disc_num": 1, "disc_count": 1} + opts = CreateIsoOpts() + + self.assertFalse( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + + @mock.patch("pungi.phases.createiso.read_json_file") + def test_volume_id_differs(self, read_json_file): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose.load_old_compose_config.return_value = compose.conf.copy() + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + cmd = {"disc_num": 1, "disc_count": 1} + + opts = CreateIsoOpts(volid="new-volid") + + read_json_file.return_value = {"opts": {"volid": "old-volid"}} + + self.assertFalse( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + + @mock.patch("pungi.phases.createiso.read_json_file") + def test_packages_differ(self, read_json_file): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose.load_old_compose_config.return_value = compose.conf.copy() + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + cmd = {"disc_num": 1, "disc_count": 1} + + new_graft_points = os.path.join(self.topdir, "new_graft_points") + helpers.touch(new_graft_points, "Packages/f/foo-1-1.x86_64.rpm\n") + opts = CreateIsoOpts(graft_points=new_graft_points, volid="volid") + + old_graft_points = os.path.join(self.topdir, "old_graft_points") + helpers.touch(old_graft_points, "Packages/f/foo-1-2.x86_64.rpm\n") + read_json_file.return_value = { + "opts": {"graft_points": old_graft_points, "volid": "volid"} + } + + self.assertFalse( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + + @mock.patch("pungi.phases.createiso.read_json_file") + def test_runs_perform_reuse(self, read_json_file): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose.load_old_compose_config.return_value = compose.conf.copy() + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + phase.perform_reuse = mock.Mock() + cmd = {"disc_num": 1, "disc_count": 1} + + new_graft_points = os.path.join(self.topdir, "new_graft_points") + helpers.touch(new_graft_points) + opts = CreateIsoOpts(graft_points=new_graft_points, volid="volid") + + old_graft_points = os.path.join(self.topdir, "old_graft_points") + helpers.touch(old_graft_points) + dummy_iso_path = "dummy-iso-path" + read_json_file.return_value = { + "opts": { + "graft_points": old_graft_points, + "volid": "volid", + }, + "cmd": {"iso_path": dummy_iso_path}, + } + + self.assertTrue( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + self.assertEqual( + phase.perform_reuse.call_args_list, + [ + mock.call( + cmd, + compose.variants["Server"], + "x86_64", + opts, + dummy_iso_path, + ) + ], + ) + + +@mock.patch("pungi.phases.createiso.OldFileLinker") +@mock.patch("pungi.phases.createiso.add_iso_to_metadata") +class CreateisoPerformReusePhaseTest(helpers.PungiTestCase): + def test_success(self, add_iso_to_metadata, OldFileLinker): + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + phase = createiso.CreateisoPhase(compose, mock.Mock()) + cmd = { + "iso_path": "target/image.iso", + "bootable": False, + "disc_num": 1, + "disc_count": 2, + } + opts = CreateIsoOpts() + + phase.perform_reuse( + cmd, + compose.variants["Server"], + "x86_64", + opts, + "old/image.iso", + ) + + self.assertEqual( + add_iso_to_metadata.call_args_list, + [ + mock.call( + compose, + compose.variants["Server"], + "x86_64", + cmd["iso_path"], + bootable=False, + disc_count=2, + disc_num=1, + ), + ], + ) + self.assertEqual( + OldFileLinker.return_value.mock_calls, + [ + mock.call.link("old/image.iso", "target/image.iso"), + mock.call.link("old/image.iso.manifest", "target/image.iso.manifest"), + # The old log file doesn't exist in the test scenario. + mock.call.link( + None, + os.path.join( + self.topdir, "logs/x86_64/createiso-image.iso.x86_64.log" + ), + ), + ], + ) + + def test_failure(self, add_iso_to_metadata, OldFileLinker): + OldFileLinker.return_value.link.side_effect = helpers.mk_boom() + compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + phase = createiso.CreateisoPhase(compose, mock.Mock()) + cmd = { + "iso_path": "target/image.iso", + "bootable": False, + "disc_num": 1, + "disc_count": 2, + } + opts = CreateIsoOpts() + + with self.assertRaises(Exception): + phase.perform_reuse( + cmd, + compose.variants["Server"], + "x86_64", + opts, + "old/image.iso", + ) + + self.assertEqual(add_iso_to_metadata.call_args_list, []) + self.assertEqual( + OldFileLinker.return_value.mock_calls, + [ + mock.call.link("old/image.iso", "target/image.iso"), + mock.call.abort(), + ], + )