From 3531b96f594bc3b9533a46779ee902753f1d7a98 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Oct 08 2020 14:22:03 +0000
Subject: [PATCH 1/4] kojira: cache external repo timestamps by arch_url


Fixes: https://pagure.io/koji/issue/2532

---

diff --git a/util/kojira b/util/kojira
index 040fb60..438af29 100755
--- a/util/kojira
+++ b/util/kojira
@@ -275,7 +275,7 @@ class RepoManager(object):
         self._local = threading.local()
         self._local.session = session
         self.repos = {}
-        self.external_repos = {}
+        self.external_repo_ts = {}
         self.tasks = {}
         self.recent_tasks = {}
         self.other_tasks = {}
@@ -398,32 +398,44 @@ class RepoManager(object):
     def checkExternalRepo(self, ts, repodata, tag):
         """Determine which external repos are current, return True if remote repo is newer"""
         url = repodata['url']
-        if url not in self.external_repos:
-            self.external_repos[url] = 0
-            arches = []  # placeholder for repos without $arch bit
+
+        # expand the arch urls if needed
+        expanded_urls = [url]
+        if '$arch' in url:
+            taginfo = getTag(self.session, tag)
+            if not taginfo:
+                self.logger.error('Invalid tag for external repo: %s', tag)
+                return False
+            arches = taginfo.get('arches', '').split()
+            if not arches:
+                self.logger.error('Tag with external repo lacks arches: %(name)s', taginfo)
+                return False
+            expanded_urls = [url.replace('$arch', a) for a in arches]
+
+        # find latest timestamp across expanded urls
+        max_ts = 0
+        for arch_url in expanded_urls:
+            if arch_url in self.external_repo_ts:
+                # just use the cache
+                max_ts = max(max_ts, self.external_repo_ts[arch_url])
+                continue
+            arch_url = os.path.join(arch_url, 'repodata/repomd.xml')
+            self.logger.debug('Checking external url: %s' % arch_url)
             try:
-                arches = getTag(self.session, tag)['arches'].split()
-            except AttributeError:
+                r = requests.get(arch_url, timeout=5)
+                root = ElementTree.fromstring(r.text)
+                ts_elements = root.iter('{http://linux.duke.edu/metadata/repo}timestamp')
+                arch_ts = max([int(child.text) for child in ts_elements])
+                self.external_repo_ts[arch_url] = arch_ts
+                max_ts = max(max_ts, arch_ts)
+            except Exception:
+                # inaccessible or without timestamps
+                # treat repo as unchanged (ts = 0)
+                self.logger.warning('Unable to read timestamp for external repo: %s', arch_url)
+                self.external_repo_ts[arch_url] = 0
                 pass
-            for arch in arches:
-                if '$arch' in url:
-                    arch_url = url.replace('$arch', arch)
-                else:
-                    arch_url = url
-                arch_url = os.path.join(arch_url, 'repodata/repomd.xml')
-                self.logger.debug('Checking external url: %s' % arch_url)
-                try:
-                    r = requests.get(arch_url, timeout=5)
-                    root = ElementTree.fromstring(r.text)
-                    for child in root.iter('{http://linux.duke.edu/metadata/repo}timestamp'):
-                        remote_ts = int(child.text)
-                        if remote_ts > self.external_repos[url]:
-                            self.external_repos[url] = remote_ts
-                except Exception:
-                    # inaccessible or without timestamps
-                    # treat repo as unchanged (ts = 0)
-                    pass
-        return ts < self.external_repos[url]
+
+        return ts < max_ts
 
     def reposToCheck(self):
         to_check = []
@@ -451,7 +463,7 @@ class RepoManager(object):
     def checkExternalRepos(self):
         """Determine which external repos changed"""
         # clean external repo cache
-        self.external_repos = {}
+        self.external_repo_ts = {}
         for repo in self.reposToCheck():
             changed = False
             for tag in repo.taglist:

From 93a77fefa6319097c7f07e375779fb95d7d40173 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Oct 08 2020 14:29:53 +0000
Subject: [PATCH 2/4] minor logging adjustment


---

diff --git a/util/kojira b/util/kojira
index 438af29..a5de98d 100755
--- a/util/kojira
+++ b/util/kojira
@@ -408,7 +408,7 @@ class RepoManager(object):
                 return False
             arches = taginfo.get('arches', '').split()
             if not arches:
-                self.logger.error('Tag with external repo lacks arches: %(name)s', taginfo)
+                self.logger.warning('Tag with external repo lacks arches: %(name)s', taginfo)
                 return False
             expanded_urls = [url.replace('$arch', a) for a in arches]
 

From 12995e5bbee849f75f517c487a1de2e73bd7ae7b Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Oct 08 2020 16:13:06 +0000
Subject: [PATCH 3/4] make cache key consistent


---

diff --git a/util/kojira b/util/kojira
index a5de98d..54619ef 100755
--- a/util/kojira
+++ b/util/kojira
@@ -415,11 +415,11 @@ class RepoManager(object):
         # find latest timestamp across expanded urls
         max_ts = 0
         for arch_url in expanded_urls:
+            arch_url = os.path.join(arch_url, 'repodata/repomd.xml')
             if arch_url in self.external_repo_ts:
                 # just use the cache
                 max_ts = max(max_ts, self.external_repo_ts[arch_url])
                 continue
-            arch_url = os.path.join(arch_url, 'repodata/repomd.xml')
             self.logger.debug('Checking external url: %s' % arch_url)
             try:
                 r = requests.get(arch_url, timeout=5)

From 916e281a2abc3908d8cc055ddb48654f2c4a8520 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Oct 23 2020 20:31:05 +0000
Subject: [PATCH 4/4] handle arches=None case


Should handle all these "empty" cases: missing, None, blank string, only whitespace

---

diff --git a/util/kojira b/util/kojira
index 54619ef..6dbe076 100755
--- a/util/kojira
+++ b/util/kojira
@@ -406,7 +406,7 @@ class RepoManager(object):
             if not taginfo:
                 self.logger.error('Invalid tag for external repo: %s', tag)
                 return False
-            arches = taginfo.get('arches', '').split()
+            arches = (taginfo.get('arches', '') or '').split()
             if not arches:
                 self.logger.warning('Tag with external repo lacks arches: %(name)s', taginfo)
                 return False

