pungi/checks.py

@@ -654,6 +654,7 @@ def make_schema():
             "gather_allow_reuse": {"type": "boolean", "default": False},
             "pkgset_allow_reuse": {"type": "boolean", "default": True},
             "createiso_allow_reuse": {"type": "boolean", "default": True},
+            "extraiso_allow_reuse": {"type": "boolean", "default": True},
             "pkgset_source": {"type": "string", "enum": ["koji", "repos"]},
             "createrepo_c": {"type": "boolean", "default": True},
             "createrepo_checksum": {

pungi/phases/extra_isos.py

@@ -14,6 +14,8 @@
 # along with this program; if not, see <https://gnu.org/licenses/>.
 
 import os
+import hashlib
+import json
 
 from kobo.shortcuts import force_list
 from kobo.threads import ThreadPool, WorkerThread
@@ -28,8 +30,16 @@ from pungi.phases.createiso import (
     copy_boot_images,
     run_createiso_command,
     load_and_tweak_treeinfo,
+    compare_packages,
+    OldFileLinker,
+)
+from pungi.util import (
+    failable,
+    get_format_substs,
+    get_variant_data,
+    get_volid,
+    read_json_file,
 )
-from pungi.util import failable, get_format_substs, get_variant_data, get_volid
 from pungi.wrappers import iso
 from pungi.wrappers.scm import get_dir_from_scm, get_file_from_scm
 
@@ -37,9 +47,10 @@ from pungi.wrappers.scm import get_dir_from_scm, get_file_from_scm
 class ExtraIsosPhase(PhaseLoggerMixin, ConfigGuardedPhase, PhaseBase):
     name = "extra_isos"
 
-    def __init__(self, compose):
+    def __init__(self, compose, buildinstall_phase):
         super(ExtraIsosPhase, self).__init__(compose)
         self.pool = ThreadPool(logger=self.logger)
+        self.bi = buildinstall_phase
 
     def validate(self):
         for variant in self.compose.get_variants(types=["variant"]):
@@ -65,13 +76,17 @@ class ExtraIsosPhase(PhaseLoggerMixin, ConfigGuardedPhase, PhaseBase):
                     commands.append((config, variant, arch))
 
         for (config, variant, arch) in commands:
-            self.pool.add(ExtraIsosThread(self.pool))
+            self.pool.add(ExtraIsosThread(self.pool, self.bi))
             self.pool.queue_put((self.compose, config, variant, arch))
 
         self.pool.start()
 
 
 class ExtraIsosThread(WorkerThread):
+    def __init__(self, pool, buildinstall_phase):
+        super(ExtraIsosThread, self).__init__(pool)
+        self.bi = buildinstall_phase
+
     def process(self, item, num):
         self.num = num
         compose, config, variant, arch = item
@@ -127,6 +142,12 @@ class ExtraIsosThread(WorkerThread):
                 buildinstall_method=compose.conf["buildinstall_method"]
             )
 
+        # Check if it can be reused.
+        hash = hashlib.sha256()
+        hash.update(json.dumps(config, sort_keys=True).encode("utf-8"))
+        config_hash = hash.hexdigest()
+
+        if not self.try_reuse(compose, variant, arch, config_hash, opts):
             script_file = os.path.join(
                 compose.paths.work.tmp_dir(arch, variant), "extraiso-%s.sh" % filename
             )
@@ -156,8 +177,153 @@ class ExtraIsosThread(WorkerThread):
         )
         img._max_size = config.get("max_size")
 
+        save_reuse_metadata(compose, variant, arch, config_hash, opts, iso_path)
+
         self.pool.log_info("[DONE ] %s" % msg)
 
+    def try_reuse(self, compose, variant, arch, config_hash, opts):
+        # Check explicit config
+        if not compose.conf["extraiso_allow_reuse"]:
+            return
+
+        log_msg = "Cannot reuse ISO for %s.%s" % (variant, arch)
+
+        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.pool.log_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 = compose.load_old_compose_config()
+        if not old_config:
+            self.pool.log_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(compose.conf))
+        for opt in 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.pool.log_info("%s - option %s differs", log_msg, opt)
+                return False
+
+        old_metadata = load_old_metadata(compose, variant, arch, config_hash)
+        if not old_metadata:
+            self.pool.log_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 opts.volid != old_metadata["opts"]["volid"]:
+            self.pool.log_info("%s - volume ID differs", log_msg)
+            return False
+
+        # Compare packages on the ISO.
+        if compare_packages(
+            old_metadata["opts"]["graft_points"],
+            opts.graft_points,
+        ):
+            self.pool.log_info("%s - packages differ", log_msg)
+            return False
+
+        try:
+            self.perform_reuse(
+                compose,
+                variant,
+                arch,
+                opts,
+                old_metadata["opts"]["output_dir"],
+                old_metadata["opts"]["iso_name"],
+            )
+            return True
+        except Exception as exc:
+            self.pool.log_error(
+                "Error while reusing ISO for %s.%s: %s", variant, arch, exc
+            )
+            compose.traceback("extraiso-reuse-%s-%s-%s" % (variant, arch, config_hash))
+            return False
+
+    def perform_reuse(self, compose, variant, arch, opts, old_iso_dir, old_file_name):
+        """
+        Copy all related files from old compose to the new one. As a last step
+        add the new image to metadata.
+        """
+        linker = OldFileLinker(self.pool._logger)
+        old_iso_path = os.path.join(old_iso_dir, old_file_name)
+        iso_path = os.path.join(opts.output_dir, opts.iso_name)
+        try:
+            # Hardlink ISO and manifest
+            for suffix in ("", ".manifest"):
+                linker.link(old_iso_path + suffix, 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 = compose.paths.log.log_file(arch, "extraiso-%s" % opts.iso_name)
+            old_log_file = compose.paths.old_compose_path(
+                compose.paths.log.log_file(arch, "extraiso-%s" % old_file_name)
+            )
+            linker.link(old_log_file, log_file)
+            # Copy jigdo files
+            if opts.jigdo_dir:
+                old_jigdo_dir = 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, opts.iso_name) + suffix,
+                    )
+        except Exception:
+            # A problem happened while linking some file, let's clean up
+            # everything.
+            linker.abort()
+            raise
+
+
+def save_reuse_metadata(compose, variant, arch, config_hash, opts, iso_path):
+    """
+    Save metadata for possible reuse of this image. The file name is determined
+    from the hash of a configuration snippet for this image. Any change in that
+    configuration in next compose will change the hash and thus reuse will be
+    blocked.
+    """
+    metadata = {"opts": opts._asdict()}
+    metadata_path = compose.paths.log.log_file(
+        arch,
+        "extraiso-reuse-%s-%s-%s" % (variant.uid, arch, config_hash),
+        ext="json",
+    )
+    with open(metadata_path, "w") as f:
+        json.dump(metadata, f, indent=2)
+
+
+def load_old_metadata(compose, variant, arch, config_hash):
+    metadata_path = compose.paths.log.log_file(
+        arch,
+        "extraiso-reuse-%s-%s-%s" % (variant.uid, arch, config_hash),
+        ext="json",
+    )
+    old_path = compose.paths.old_compose_path(metadata_path)
+    try:
+        return read_json_file(old_path)
+    except Exception:
+        return None
+
 
 def get_extra_files(compose, variant, arch, extra_files):
     """Clone the configured files into a directory from where they can be

pungi/scripts/config_validate.py

@@ -127,7 +127,7 @@ def run(config, topdir, has_old, offline, defined_variables, schema_overrides):
         pungi.phases.OstreeInstallerPhase(compose, buildinstall_phase),
         pungi.phases.OSTreePhase(compose),
         pungi.phases.CreateisoPhase(compose, buildinstall_phase),
-        pungi.phases.ExtraIsosPhase(compose),
+        pungi.phases.ExtraIsosPhase(compose, buildinstall_phase),
         pungi.phases.LiveImagesPhase(compose),
         pungi.phases.LiveMediaPhase(compose),
         pungi.phases.ImageBuildPhase(compose),

pungi/scripts/pungi_koji.py

@@ -403,7 +403,7 @@ def run_compose(
     )
     ostree_phase = pungi.phases.OSTreePhase(compose, pkgset_phase)
     createiso_phase = pungi.phases.CreateisoPhase(compose, buildinstall_phase)
-    extra_isos_phase = pungi.phases.ExtraIsosPhase(compose)
+    extra_isos_phase = pungi.phases.ExtraIsosPhase(compose, buildinstall_phase)
     liveimages_phase = pungi.phases.LiveImagesPhase(compose)
     livemedia_phase = pungi.phases.LiveMediaPhase(compose)
     image_build_phase = pungi.phases.ImageBuildPhase(compose, buildinstall_phase)

tests/test_extra_isos_phase.py

@@ -1,12 +1,13 @@
 # -*- coding: utf-8 -*-
 
-
+import logging
 import mock
 import six
 
 import os
 
 from tests import helpers
+from pungi.createiso import CreateIsoOpts
 from pungi.phases import extra_isos
 
 
@@ -19,7 +20,7 @@ class ExtraIsosPhaseTest(helpers.PungiTestCase):
         }
         compose = helpers.DummyCompose(self.topdir, {"extra_isos": {"^Server$": [cfg]}})
 
-        phase = extra_isos.ExtraIsosPhase(compose)
+        phase = extra_isos.ExtraIsosPhase(compose, mock.Mock())
         phase.validate()
 
         self.assertEqual(len(compose.log_warning.call_args_list), 1)
@@ -30,7 +31,7 @@ class ExtraIsosPhaseTest(helpers.PungiTestCase):
         }
         compose = helpers.DummyCompose(self.topdir, {"extra_isos": {"^Server$": [cfg]}})
 
-        phase = extra_isos.ExtraIsosPhase(compose)
+        phase = extra_isos.ExtraIsosPhase(compose, mock.Mock())
         phase.run()
 
         self.assertEqual(len(ThreadPool.return_value.add.call_args_list), 3)
@@ -51,7 +52,7 @@ class ExtraIsosPhaseTest(helpers.PungiTestCase):
         }
         compose = helpers.DummyCompose(self.topdir, {"extra_isos": {"^Server$": [cfg]}})
 
-        phase = extra_isos.ExtraIsosPhase(compose)
+        phase = extra_isos.ExtraIsosPhase(compose, mock.Mock())
         phase.run()
 
         self.assertEqual(len(ThreadPool.return_value.add.call_args_list), 2)
@@ -71,7 +72,7 @@ class ExtraIsosPhaseTest(helpers.PungiTestCase):
         }
         compose = helpers.DummyCompose(self.topdir, {"extra_isos": {"^Server$": [cfg]}})
 
-        phase = extra_isos.ExtraIsosPhase(compose)
+        phase = extra_isos.ExtraIsosPhase(compose, mock.Mock())
         phase.run()
 
         self.assertEqual(len(ThreadPool.return_value.add.call_args_list), 2)
@@ -106,7 +107,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
         gvi.return_value = "my volume id"
         gic.return_value = "/tmp/iso-graft-points"
 
-        t = extra_isos.ExtraIsosThread(mock.Mock())
+        t = extra_isos.ExtraIsosThread(mock.Mock(), mock.Mock())
         with mock.patch("time.sleep"):
             t.process((compose, cfg, server, "x86_64"), 1)
 
@@ -182,7 +183,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
         gvi.return_value = "my volume id"
         gic.return_value = "/tmp/iso-graft-points"
 
-        t = extra_isos.ExtraIsosThread(mock.Mock())
+        t = extra_isos.ExtraIsosThread(mock.Mock(), mock.Mock())
         with mock.patch("time.sleep"):
             t.process((compose, cfg, server, "x86_64"), 1)
 
@@ -256,7 +257,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
         gvi.return_value = "my volume id"
         gic.return_value = "/tmp/iso-graft-points"
 
-        t = extra_isos.ExtraIsosThread(mock.Mock())
+        t = extra_isos.ExtraIsosThread(mock.Mock(), mock.Mock())
         with mock.patch("time.sleep"):
             t.process((compose, cfg, server, "x86_64"), 1)
 
@@ -330,7 +331,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
         gvi.return_value = "my volume id"
         gic.return_value = "/tmp/iso-graft-points"
 
-        t = extra_isos.ExtraIsosThread(mock.Mock())
+        t = extra_isos.ExtraIsosThread(mock.Mock(), mock.Mock())
         with mock.patch("time.sleep"):
             t.process((compose, cfg, server, "x86_64"), 1)
 
@@ -405,7 +406,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
         gvi.return_value = "my volume id"
         gic.return_value = "/tmp/iso-graft-points"
 
-        t = extra_isos.ExtraIsosThread(mock.Mock())
+        t = extra_isos.ExtraIsosThread(mock.Mock(), mock.Mock())
         with mock.patch("time.sleep"):
             t.process((compose, cfg, server, "src"), 1)
 
@@ -476,7 +477,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
         gic.return_value = "/tmp/iso-graft-points"
         rcc.side_effect = helpers.mk_boom()
 
-        t = extra_isos.ExtraIsosThread(mock.Mock())
+        t = extra_isos.ExtraIsosThread(mock.Mock(), mock.Mock())
         with mock.patch("time.sleep"):
             t.process((compose, cfg, server, "x86_64"), 1)
 
@@ -494,7 +495,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase):
         gic.return_value = "/tmp/iso-graft-points"
         rcc.side_effect = helpers.mk_boom(RuntimeError)
 
-        t = extra_isos.ExtraIsosThread(mock.Mock())
+        t = extra_isos.ExtraIsosThread(mock.Mock(), mock.Mock())
         with self.assertRaises(RuntimeError):
             with mock.patch("time.sleep"):
                 t.process((compose, cfg, server, "x86_64"), 1)
@@ -1061,3 +1062,215 @@ class PrepareMetadataTest(helpers.PungiTestCase):
                 ),
             ],
         )
+
+
+class ExtraisoTryReusePhaseTest(helpers.PungiTestCase):
+    def setUp(self):
+        super(ExtraisoTryReusePhaseTest, 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, {"extraiso_allow_reuse": False})
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        opts = CreateIsoOpts()
+
+        self.assertFalse(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+
+    def test_buildinstall_changed(self):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        thread.logger = self.logger
+        thread.bi = mock.Mock()
+        thread.bi.reused.return_value = False
+        opts = CreateIsoOpts(buildinstall_method="lorax")
+
+        self.assertFalse(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+
+    def test_no_old_config(self):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        thread.logger = self.logger
+        opts = CreateIsoOpts()
+
+        self.assertFalse(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+
+    def test_old_config_changed(self):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        old_config = compose.conf.copy()
+        old_config["release_version"] = "2"
+        compose.load_old_compose_config.return_value = old_config
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        thread.logger = self.logger
+        opts = CreateIsoOpts()
+
+        self.assertFalse(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+
+    def test_no_old_metadata(self):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        compose.load_old_compose_config.return_value = compose.conf.copy()
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        thread.logger = self.logger
+        opts = CreateIsoOpts()
+
+        self.assertFalse(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+
+    @mock.patch("pungi.phases.extra_isos.read_json_file")
+    def test_volume_id_differs(self, read_json_file):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        compose.load_old_compose_config.return_value = compose.conf.copy()
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        thread.logger = self.logger
+
+        opts = CreateIsoOpts(volid="new-volid")
+
+        read_json_file.return_value = {"opts": {"volid": "old-volid"}}
+
+        self.assertFalse(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+
+    @mock.patch("pungi.phases.extra_isos.read_json_file")
+    def test_packages_differ(self, read_json_file):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        compose.load_old_compose_config.return_value = compose.conf.copy()
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        thread.logger = self.logger
+
+        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(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+
+    @mock.patch("pungi.phases.extra_isos.read_json_file")
+    def test_runs_perform_reuse(self, read_json_file):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        compose.load_old_compose_config.return_value = compose.conf.copy()
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        thread.logger = self.logger
+        thread.perform_reuse = mock.Mock()
+
+        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/dummy.iso"
+        read_json_file.return_value = {
+            "opts": {
+                "graft_points": old_graft_points,
+                "volid": "volid",
+                "output_dir": os.path.dirname(dummy_iso_path),
+                "iso_name": os.path.basename(dummy_iso_path),
+            },
+        }
+
+        self.assertTrue(
+            thread.try_reuse(
+                compose, compose.variants["Server"], "x86_64", "abcdef", opts
+            )
+        )
+        self.assertEqual(
+            thread.perform_reuse.call_args_list,
+            [
+                mock.call(
+                    compose,
+                    compose.variants["Server"],
+                    "x86_64",
+                    opts,
+                    "dummy-iso-path",
+                    "dummy.iso",
+                )
+            ],
+        )
+
+
+@mock.patch("pungi.phases.extra_isos.OldFileLinker")
+class ExtraIsoPerformReusePhaseTest(helpers.PungiTestCase):
+    def test_success(self, OldFileLinker):
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        opts = CreateIsoOpts(output_dir="new/path", iso_name="new.iso")
+
+        thread.perform_reuse(
+            compose,
+            compose.variants["Server"],
+            "x86_64",
+            opts,
+            "old",
+            "image.iso",
+        )
+
+        self.assertEqual(
+            OldFileLinker.return_value.mock_calls,
+            [
+                mock.call.link("old/image.iso", "new/path/new.iso"),
+                mock.call.link("old/image.iso.manifest", "new/path/new.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/extraiso-new.iso.x86_64.log"
+                    ),
+                ),
+            ],
+        )
+
+    def test_failure(self, OldFileLinker):
+        OldFileLinker.return_value.link.side_effect = helpers.mk_boom()
+        compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True})
+        thread = extra_isos.ExtraIsosThread(compose, mock.Mock())
+        opts = CreateIsoOpts(output_dir="new/path", iso_name="new.iso")
+
+        with self.assertRaises(Exception):
+            thread.perform_reuse(
+                compose,
+                compose.variants["Server"],
+                "x86_64",
+                opts,
+                "old",
+                "image.iso",
+            )
+
+        self.assertEqual(
+            OldFileLinker.return_value.mock_calls,
+            [
+                mock.call.link("old/image.iso", "new/path/new.iso"),
+                mock.call.abort(),
+            ],
+        )