From c03d23c0f275cf4169b8b20064a07e514da9a47a Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Nov 20 2024 20:18:14 +0000
Subject: [PATCH 1/7] kojira: adjust expire_check


---

diff --git a/util/kojira b/util/kojira
index 5e98f39..ac60546 100755
--- a/util/kojira
+++ b/util/kojira
@@ -150,9 +150,30 @@ class ManagedRepo(object):
         if self.data['end_event'] is None and not self.data['custom_opts']:
             # repo is current and has default options. keep it
             return
-        # otherwise repo is either obsolete or custom
-        if self.get_age() > self.options.repo_lifetime:
-            self.expire()
+
+        # keep repos for configured lifetime
+        if self.get_age() <= self.options.repo_lifetime:
+            return
+
+        # keep latest default repo for build tags
+        if not self.data['custom_opts']:
+            targets = self.session.get_build_targets(buildTagID=self.data['tag_id'])
+            if targets and self.is_latest():
+                return
+
+        self.expire()
+
+    def is_latest(self):
+        """Check if repo is latest for its tag (not necessarily current)"""
+        # similar query to symlink_if_latest on hub
+        clauses = [
+            ['tag_id', '=', self.data['tag_id']],
+            ['state', '=', koji.REPO_READY],
+            ['custom_opts', '=', '{}'],
+            ['create_event', '>', self.data['create_event']],
+        ]
+        newer = self.session.repo.query(clauses, ['id'])
+        return not newer  # True if no newer matching repo
 
     def dist_expire_check(self):
         """Check to see if a dist repo should be expired"""

From 81433e588369e5adcfdaeea1b91626039d29a967 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Nov 20 2024 20:26:32 +0000
Subject: [PATCH 2/7] tweak unit test


---

diff --git a/tests/test_kojira/test_repo_manager.py b/tests/test_kojira/test_repo_manager.py
index 85ec47e..21e4116 100644
--- a/tests/test_kojira/test_repo_manager.py
+++ b/tests/test_kojira/test_repo_manager.py
@@ -183,8 +183,9 @@ class RepoManagerTest(unittest.TestCase):
         self.assertEqual(set(self.mgr.repos), set([r['id'] for r in repos]))
 
     # using autospec so we can grab self from mock_calls
+    @mock.patch.object(kojira.ManagedRepo, 'is_latest', autospec=True)
     @mock.patch.object(kojira.ManagedRepo, 'delete_check', autospec=True)
-    def test_update_repos(self, delete_check):
+    def test_update_repos(self, delete_check, is_latest):
         self.options.init_timeout = 3600
         self.options.repo_lifetime = 3600 * 24
         self.options.dist_repo_lifetime = 3600 * 24
@@ -206,6 +207,7 @@ class RepoManagerTest(unittest.TestCase):
         repos[2]['state'] = koji.REPO_EXPIRED
 
         # do the run
+        is_latest.return_value = False
         self.session.repo.query.return_value = repos
         with mock.patch('time.time') as _time:
             _time.return_value = base_ts + 100  # shorter than all timeouts

From 99f001e1807e404f9642d0237cc2e8385a65e6f9 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Nov 21 2024 18:13:33 +0000
Subject: [PATCH 3/7] recheck_period for expire check


---

diff --git a/util/kojira b/util/kojira
index ac60546..3a571df 100755
--- a/util/kojira
+++ b/util/kojira
@@ -155,9 +155,15 @@ class ManagedRepo(object):
         if self.get_age() <= self.options.repo_lifetime:
             return
 
-        # keep latest default repo for build tags
+        # remaining checks are more expensive, don't recheck every cycle
+        last_check = getattr(self, 'expire_check_ts', None)
+        if last_check and time.time() - last_check < self.options.recheck_period:
+            return
+        self.expire_check_ts = time.time()
+
+        # keep latest default repo for build tags, even if not current
         if not self.data['custom_opts']:
-            targets = self.session.get_build_targets(buildTagID=self.data['tag_id'])
+            targets = self.session.getBuildTargets(buildTagID=self.data['tag_id'])
             if targets and self.is_latest():
                 return
 
@@ -835,7 +841,8 @@ def get_options():
                 'expired_repo_lifetime': None,  # default handled below
                 'deleted_repo_lifetime': None,  # compat alias for expired_repo_lifetime
                 'init_timeout': 7200,
-                'reference_recheck_period': 3600,
+                'recheck_period': 3600,
+                'reference_recheck_period': None,  # defaults to recheck_period
                 'no_repo_effective_age': 2 * 24 * 3600,
                 'check_external_repos': True,
                 'sleeptime': 15,
@@ -849,7 +856,7 @@ def get_options():
                     'retry_interval', 'max_retries', 'offline_retry_interval',
                     'max_delete_processes', 'dist_repo_lifetime',
                     'sleeptime', 'expired_repo_lifetime',
-                    'repo_lifetime', 'reference_recheck_period')
+                    'repo_lifetime', 'recheck_period', 'reference_recheck_period')
         str_opts = ('topdir', 'server', 'user', 'password', 'logfile', 'principal', 'keytab',
                     'cert', 'serverca')
         bool_opts = ('verbose', 'debug', 'ignore_stray_repos', 'offline_retry',
@@ -889,6 +896,8 @@ def get_options():
             options.expired_repo_lifetime = options.deleted_repo_lifetime
     elif options.expired_repo_lifetime is None:
         options.expired_repo_lifetime = 7 * 24 * 3600
+    if options.reference_recheck_period is None:
+        options.reference_recheck_period = options.recheck_period
     if options.logfile in ('', 'None', 'none'):
         options.logfile = None
     # special handling for cert defaults

From b2a7f90143fd872439e834a2731a01cbf1f1b561 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Nov 21 2024 18:13:33 +0000
Subject: [PATCH 4/7] unit tests


---

diff --git a/tests/test_kojira/test_managed_repo.py b/tests/test_kojira/test_managed_repo.py
index 56bda2a..9d19b6a 100644
--- a/tests/test_kojira/test_managed_repo.py
+++ b/tests/test_kojira/test_managed_repo.py
@@ -40,6 +40,7 @@ class ManagedRepoTest(unittest.TestCase):
             'end_event': None,
             'id': 2385,
             'opts': {'debuginfo': False, 'separate_src': False, 'src': False},
+            'custom_opts': {},
             'state': 1,
             'state_ts': 1710705227.166751,
             'tag_id': 50,
@@ -126,4 +127,86 @@ class ManagedRepoTest(unittest.TestCase):
         self.session.repo.setState.assert_called_once_with(self.repo.id, koji.REPO_DELETED)
         self.mgr.rmtree.assert_called_once_with(path)
 
+    def test_expire_check_recent(self):
+        self.options.repo_lifetime = 3600 * 24
+        self.options.recheck_period = 3600
+        base_ts = 444888888
+        now = base_ts + 100
+
+        self.repo.data['state'] = koji.REPO_READY
+        self.repo.data['state_ts'] = base_ts
+        self.repo.data['end_event'] = 999
+
+        with mock.patch('time.time') as _time:
+            _time.return_value = now
+            self.repo.expire_check()
+
+        # we should have stopped at the age check
+        self.session.getBuildTargets.assert_not_called()
+        self.session.repoExpire.assert_not_called()
+
+    def test_expire_check_recheck(self):
+        self.options.repo_lifetime = 3600 * 24
+        self.options.recheck_period = 3600
+        base_ts = 444888888
+        now = base_ts + self.options.repo_lifetime + 100
+
+        # recheck period still in effect
+        self.repo.expire_check_ts = now - 3500
+        # otherwise eligible to expire
+        self.repo.data['state'] = koji.REPO_READY
+        self.repo.data['state_ts'] = base_ts
+        self.repo.data['end_event'] = 999
+
+        with mock.patch('time.time') as _time:
+            _time.return_value = now
+            self.repo.expire_check()
+
+        self.session.getBuildTargets.assert_not_called()
+        self.session.repo.query.assert_not_called()
+        self.session.repoExpire.assert_not_called()
+
+    def test_expire_check_latest(self):
+        self.options.repo_lifetime = 3600 * 24
+        self.options.recheck_period = 3600
+        base_ts = 444888888
+        now = base_ts + self.options.repo_lifetime + 100
+
+        self.repo.data['state'] = koji.REPO_READY
+        self.repo.data['state_ts'] = base_ts
+        self.repo.data['end_event'] = 999
+        # latest for a target, should not get expired
+        self.session.getBuildTargets.return_value = ['TARGET']
+        self.session.repo.query.return_value = []
+
+        with mock.patch('time.time') as _time:
+            _time.return_value = now
+            self.repo.expire_check()
+
+        self.session.getBuildTargets.assert_called_once()
+        self.session.repo.query.assert_called_once()
+        self.session.repoExpire.assert_not_called()
+
+    def test_expire_check_expire(self):
+        self.options.repo_lifetime = 3600 * 24
+        self.options.recheck_period = 3600
+        base_ts = 444888888
+        now = base_ts + self.options.repo_lifetime + 100
+
+        self.repo.data['state'] = koji.REPO_READY
+        self.repo.data['state_ts'] = base_ts
+        self.repo.data['end_event'] = 999
+        # not latest
+        self.session.getBuildTargets.return_value = ['TARGET']
+        self.session.repo.query.return_value = ['NEWER_REPO']
+
+        with mock.patch('time.time') as _time:
+            _time.return_value = now
+            self.repo.expire_check()
+
+        self.session.getBuildTargets.assert_called_once()
+        self.session.repo.query.assert_called_once()
+        self.session.repoExpire.assert_called_once()
+
+
 # the end

From 55d0dbef06bf38041ead2911f8e23a9f0be37c6a Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Jan 17 2025 15:33:26 +0000
Subject: [PATCH 5/7] avoid expiring latest dist repos also


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

---

diff --git a/util/kojira b/util/kojira
index 3a571df..416fa57 100755
--- a/util/kojira
+++ b/util/kojira
@@ -144,15 +144,20 @@ class ManagedRepo(object):
             return time.time() - self.first_seen
 
     def expire_check(self):
-        if self.state != koji.REPO_READY or self.dist:
+        if self.state != koji.REPO_READY:
             return
 
         if self.data['end_event'] is None and not self.data['custom_opts']:
             # repo is current and has default options. keep it
+            # this covers dist repos, where custom_opts=None
             return
 
         # keep repos for configured lifetime
-        if self.get_age() <= self.options.repo_lifetime:
+        if self.dist:
+            lifetime = self.options.dist_repo_lifetime
+        else:
+            lifetime = self.options.repo_lifetime
+        if self.get_age() <= lifetime:
             return
 
         # remaining checks are more expensive, don't recheck every cycle
@@ -163,6 +168,7 @@ class ManagedRepo(object):
 
         # keep latest default repo for build tags, even if not current
         if not self.data['custom_opts']:
+            # this covers dist repos, where custom_opts=None
             targets = self.session.getBuildTargets(buildTagID=self.data['tag_id'])
             if targets and self.is_latest():
                 return
@@ -175,21 +181,17 @@ class ManagedRepo(object):
         clauses = [
             ['tag_id', '=', self.data['tag_id']],
             ['state', '=', koji.REPO_READY],
-            ['custom_opts', '=', '{}'],
             ['create_event', '>', self.data['create_event']],
         ]
+        if self.dist:
+            clauses.append(['dist', '=', True])
+        else:
+            clauses.append(['dist', '=', False])
+            clauses.append(['custom_opts', '=', '{}'])
+            # ^this clause is only for normal repos, dist repos have custom_opts=None
         newer = self.session.repo.query(clauses, ['id'])
         return not newer  # True if no newer matching repo
 
-    def dist_expire_check(self):
-        """Check to see if a dist repo should be expired"""
-        if not self.dist or self.state != koji.REPO_READY:
-            return
-
-        if self.get_age() > self.options.dist_repo_lifetime:
-            self.logger.info("Expiring dist repo %(id)s for tag %(tag_name)s", self.data)
-            self.expire()
-
     def delete_check(self):
         """Delete the repo if appropriate"""
 
@@ -666,10 +668,7 @@ class RepoManager(object):
             if repo.state == koji.REPO_INIT:
                 repo.check_init()
             elif repo.state == koji.REPO_READY:
-                if repo.dist:
-                    repo.dist_expire_check()
-                else:
-                    repo.expire_check()
+                repo.expire_check()
             elif repo.state == koji.REPO_EXPIRED:
                 repo.delete_check()
             elif repo.state == koji.REPO_PROBLEM:

From c56a9c84ef552e4c2cac2ab962f82e861484b482 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Jan 17 2025 16:57:18 +0000
Subject: [PATCH 6/7] target check makes no sense for dist repos


---

diff --git a/util/kojira b/util/kojira
index 416fa57..4cb55d1 100755
--- a/util/kojira
+++ b/util/kojira
@@ -149,7 +149,7 @@ class ManagedRepo(object):
 
         if self.data['end_event'] is None and not self.data['custom_opts']:
             # repo is current and has default options. keep it
-            # this covers dist repos, where custom_opts=None
+            # this covers current dist repos, where custom_opts=None
             return
 
         # keep repos for configured lifetime
@@ -166,9 +166,13 @@ class ManagedRepo(object):
             return
         self.expire_check_ts = time.time()
 
-        # keep latest default repo for build tags, even if not current
-        if not self.data['custom_opts']:
-            # this covers dist repos, where custom_opts=None
+        # keep latest default repo in some cases, even if not current
+        if self.dist:
+            # no target check -- they are irrelevant for dist repos
+            if self.is_latest():
+                return
+        elif not self.data['custom_opts']:
+            # normal repo, default options
             targets = self.session.getBuildTargets(buildTagID=self.data['tag_id'])
             if targets and self.is_latest():
                 return

From 547342e41e38485877d7c862424898e446acdfd5 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Jan 17 2025 17:08:48 +0000
Subject: [PATCH 7/7] unit test


---

diff --git a/tests/test_kojira/test_managed_repo.py b/tests/test_kojira/test_managed_repo.py
index 9d19b6a..df8a932 100644
--- a/tests/test_kojira/test_managed_repo.py
+++ b/tests/test_kojira/test_managed_repo.py
@@ -187,6 +187,30 @@ class ManagedRepoTest(unittest.TestCase):
         self.session.repo.query.assert_called_once()
         self.session.repoExpire.assert_not_called()
 
+    def test_expire_check_latest_dist(self):
+        self.options.dist_repo_lifetime = 3600 * 24
+        self.options.recheck_period = 3600
+        base_ts = 444888888
+        now = base_ts + self.options.dist_repo_lifetime + 100
+
+        self.repo.data['dist'] = True
+        self.repo.dist = True
+        self.repo.data['state'] = koji.REPO_READY
+        self.repo.data['state_ts'] = base_ts
+        self.repo.data['end_event'] = 999
+        # latest for a target, should not get expired
+        self.session.getBuildTargets.return_value = ['TARGET']
+        self.session.repo.query.return_value = []
+
+        with mock.patch('time.time') as _time:
+            _time.return_value = now
+            self.repo.expire_check()
+
+        self.session.getBuildTargets.assert_not_called()
+        # no target check for dist repos
+        self.session.repo.query.assert_called_once()
+        self.session.repoExpire.assert_not_called()
+
     def test_expire_check_expire(self):
         self.options.repo_lifetime = 3600 * 24
         self.options.recheck_period = 3600
@@ -208,5 +232,29 @@ class ManagedRepoTest(unittest.TestCase):
         self.session.repo.query.assert_called_once()
         self.session.repoExpire.assert_called_once()
 
+    def test_expire_check_expire_dist(self):
+        self.options.dist_repo_lifetime = 3600 * 24
+        self.options.recheck_period = 3600
+        base_ts = 444888888
+        now = base_ts + self.options.dist_repo_lifetime + 100
+
+        self.repo.data['dist'] = True
+        self.repo.dist = True
+        self.repo.data['state'] = koji.REPO_READY
+        self.repo.data['state_ts'] = base_ts
+        self.repo.data['end_event'] = 999
+        # not latest
+        self.session.getBuildTargets.return_value = ['TARGET']
+        self.session.repo.query.return_value = ['NEWER_REPO']
+
+        with mock.patch('time.time') as _time:
+            _time.return_value = now
+            self.repo.expire_check()
+
+        self.session.getBuildTargets.assert_not_called()
+        # no target check for dist repos
+        self.session.repo.query.assert_called_once()
+        self.session.repoExpire.assert_called_once()
+
 
 # the end

