gather: Adjust reusing with lookaside

- Do not reuse when there is any external lookaside repo
- Do not reuse when lookaside variant is not reused

JIRA: RHELCMP-4596
Signed-off-by: Haibo Lin <hlin@redhat.com>

pungi/phases/gather/__init__.py

@@ -19,7 +19,6 @@ import glob
 import os
 import shutil
 import threading
-import six
 from six.moves import cPickle as pickle
 
 from kobo.rpmlib import parse_nvra
@@ -219,33 +218,33 @@ def reuse_old_gather_packages(compose, arch, variant, package_sets, methods):
         compose.log_info(log_msg % "no old compose config dump.")
         return
 
+    # Do not reuse when required variant is not reused.
+    if not hasattr(compose, "_gather_reused_variant_arch"):
+        setattr(compose, "_gather_reused_variant_arch", [])
+    variant_as_lookaside = compose.conf.get("variant_as_lookaside", [])
+    for (requiring, required) in variant_as_lookaside:
+        if (
+            requiring == variant.uid
+            and (required, arch) not in compose._gather_reused_variant_arch
+        ):
+            compose.log_info(
+                log_msg % "variant %s as lookaside is not reused." % required
+            )
+            return
+
+    # Do not reuse if there's external lookaside repo.
+    with open(compose.paths.log.log_file("global", "config-dump"), "r") as f:
+        config_dump = json.load(f)
+    if config_dump.get("gather_lookaside_repos") or old_config.get(
+        "gather_lookaside_repos"
+    ):
+        compose.log_info(log_msg % "there's external lookaside repo.")
+        return
+
     # The dumps/loads is needed to convert all unicode strings to non-unicode ones.
     config = json.loads(json.dumps(compose.conf))
     for opt, value in old_config.items():
-        # Gather lookaside repos are updated during the gather phase. Check that
-        # the gather_lookaside_repos except the ones added are the same.
-        if opt == "gather_lookaside_repos" and opt in config:
-            value_to_compare = []
-            # Filter out repourls which starts with `compose.topdir` and also remove
-            # their parent list in case it would be empty.
-            for variant, per_arch_repos in config[opt]:
-                per_arch_repos_to_compare = {}
-                for arch, repourl in per_arch_repos.items():
-                    # The gather_lookaside_repos config allows setting multiple repourls
-                    # using list, but `_update_config` always uses strings. Therefore we
-                    # only try to filter out string_types.
-                    if not isinstance(repourl, six.string_types):
-                        continue
-                    if not repourl.startswith(compose.topdir):
-                        per_arch_repos_to_compare[arch] = repourl
-                if per_arch_repos_to_compare:
-                    value_to_compare.append([variant, per_arch_repos_to_compare])
-            if value != value_to_compare:
-                compose.log_info(
-                    log_msg
-                    % ("compose configuration option gather_lookaside_repos changed.")
-                )
-                return
+        if opt == "gather_lookaside_repos":
             continue
 
         # Skip checking for frequently changing configuration options which do *not*
@@ -374,6 +373,8 @@ def reuse_old_gather_packages(compose, arch, variant, package_sets, methods):
                 compose.log_info(log_msg % "some RPMs have been removed.")
                 return
 
+    compose._gather_reused_variant_arch.append((variant.uid, arch))
+
     # Copy old gather log for debugging
     try:
         if methods == "hybrid":

tests/test_gather_phase.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 
 import copy
+import json
 import mock
 import os
 
@@ -1080,11 +1081,17 @@ class TestGatherPackages(helpers.PungiTestCase):
 
 
 class TestReuseOldGatherPackages(helpers.PungiTestCase):
+    def _save_config_dump(self, compose):
+        config_dump_full = compose.paths.log.log_file("global", "config-dump")
+        with open(config_dump_full, "w") as f:
+            json.dump(compose.conf, f, sort_keys=True, indent=4)
+
     @mock.patch("pungi.phases.gather.load_old_gather_result")
     def test_reuse_no_old_gather_result(self, load_old_gather_result):
         load_old_gather_result.return_value = None
 
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         result = gather.reuse_old_gather_packages(
             compose, "x86_64", compose.variants["Server"], [], "deps"
         )
@@ -1102,6 +1109,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
         }
 
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = None
 
         result = gather.reuse_old_gather_packages(
@@ -1121,6 +1129,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
         }
 
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         compose_conf_copy = dict(compose.conf)
         compose_conf_copy["gather_method"] = "nodeps"
         load_old_compose_config.return_value = compose_conf_copy
@@ -1143,6 +1152,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             }
 
             compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+            self._save_config_dump(compose)
             compose_conf_copy = dict(compose.conf)
             compose_conf_copy[whitelist_opt] = "different"
             load_old_compose_config.return_value = compose_conf_copy
@@ -1179,6 +1189,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             load_old_gather_result, requires=[], provides=[]
         )
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = compose.conf
 
         result = gather.reuse_old_gather_packages(
@@ -1202,6 +1213,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             load_old_gather_result, requires=[], provides=[]
         )
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = copy.deepcopy(compose.conf)
 
         gather._update_config(compose, "Server", "x86_64", compose.topdir)
@@ -1226,6 +1238,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             load_old_gather_result, requires=[], provides=[]
         )
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         lookasides = compose.conf["gather_lookaside_repos"]
         lookasides.append(("^Server$", {"x86_64": "http://localhost/real.repo"}))
         load_old_compose_config.return_value = copy.deepcopy(compose.conf)
@@ -1245,6 +1258,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             load_old_gather_result, requires=[], provides=[]
         )
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         lookasides = compose.conf["gather_lookaside_repos"]
         repos = ["http://localhost/real1.repo", "http://localhost/real2.repo"]
         lookasides.append(("^Server$", {"x86_64": repos}))
@@ -1268,6 +1282,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             "/build/foo-1-1.x86_64.rpm": MockPkg("foo-1-1.x86_64.rpm", sourcerpm="foo")
         }
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = compose.conf
 
         result = gather.reuse_old_gather_packages(
@@ -1290,6 +1305,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
         pkg_set.old_file_cache["/build/bash-1-2.x86_64.rpm"] = bash_pkg
         pkg_set.file_cache["/build/bash-1-2.x86_64.rpm"] = bash_pkg
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = compose.conf
 
         result = gather.reuse_old_gather_packages(
@@ -1315,6 +1331,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
         pkg_set.old_file_cache["/build/file-1-1.x86_64.rpm"] = file_pkg
         pkg_set.file_cache["/build/foo-1-1.x86_64.rpm"] = foo_pkg
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = compose.conf
 
         result = gather.reuse_old_gather_packages(
@@ -1332,6 +1349,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
         )
         package_sets[0]["global"].old_file_cache = None
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = compose.conf
 
         result = gather.reuse_old_gather_packages(
@@ -1348,6 +1366,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             load_old_gather_result, requires=["foo"], provides=[]
         )
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = compose.conf
 
         result = gather.reuse_old_gather_packages(
@@ -1364,6 +1383,7 @@ class TestReuseOldGatherPackages(helpers.PungiTestCase):
             load_old_gather_result, requires=[], provides=["foo"]
         )
         compose = helpers.DummyCompose(self.topdir, {"gather_allow_reuse": True})
+        self._save_config_dump(compose)
         load_old_compose_config.return_value = compose.conf
 
         result = gather.reuse_old_gather_packages(