PR#4270: keep latest default repo for build tags - koji

		`@@ -40,6 +40,7 @@`
		`'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,134 @@`
		`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_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`
		`+ 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()`
		`+`
		`+ 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`

tests/test_kojira/test_repo_manager.py

file modified

+3 -1

		`@@ -183,8 +183,9 @@`
		`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 @@`
		`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`

util/kojira

file modified

+49 -16

		`@@ -144,24 +144,57 @@`
		`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 current dist repos, where custom_opts=None`
		`return`
		`- # otherwise repo is either obsolete or custom`
		`- if self.get_age() > self.options.repo_lifetime:`
		`- self.expire()`

		`- 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:`
		`+ # keep repos for configured lifetime`
		`+ if self.dist:`
		`+ lifetime = self.options.dist_repo_lifetime`
		`+ else:`
		`+ lifetime = self.options.repo_lifetime`
		`+ if self.get_age() <= lifetime:`
		`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()`
		`+ # 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 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`
		`+`
		`+ 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],`
		`+ ['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 delete_check(self):`
		`"""Delete the repo if appropriate"""`
		`@@ -639,10 +672,7 @@`
		`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:`
		`@@ -814,7 +844,8 @@`
		`'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,`
		`@@ -828,7 +859,7 @@`
		`'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',`
		`@@ -868,6 +899,8 @@`
		`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`

mikem commented 4 months ago

If a build target is not used for building for longer than the total repo lifetime, then we could end up with no repo. This change causes kojira to keep the latest repo for build tags, even if they are no longer current.

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