createiso: Allow reusing old images

This patch allows Pungi to reuse ISO image created in previous compose
if a list of assumptions proves to hold:

 * If image is bootable, buildinstall phase must have been reused too.
 * Compose configuration must have not changed (except for a few
   whitelisted options).
 * Volume ID of the ISO much not have changed.
 * No RPM on the ISO must have changed.

The ISO also contains other files. Changes in extra files and product ID
certificates should be visible in configuration (the SHA will differ).
Similarly any repodata configuration would be reflected in
configuration.

JIRA: RHELCMP-5969
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

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": {

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)

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)

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(),
+            ],
+        )