From 185456e69a428fc3afd4135b5ca3832d987f9394 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2024 22:44:19 +0000 Subject: [PATCH 1/8] avoid duplicate query --- diff --git a/kojihub/repos.py b/kojihub/repos.py index 9a7fd03..265a7cf 100644 --- a/kojihub/repos.py +++ b/kojihub/repos.py @@ -390,7 +390,7 @@ def update_end_events(): n_cached = 0 tag_last = {} updates = [] - for repo in query.execute(): + for repo in repos: tag_id = repo['tag_id'] # use cache to avoid redundant calls if tag_id in tag_last and tag_last[tag_id] <= repo['create_event']: From 507eb924c9cad00dd20d80f3da246566fad06570 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2024 22:44:19 +0000 Subject: [PATCH 2/8] print task info if repo request fails --- diff --git a/koji/util.py b/koji/util.py index 750bbda..d74320d 100644 --- a/koji/util.py +++ b/koji/util.py @@ -295,6 +295,9 @@ class RepoWatcher(object): raise koji.GenericError("Unsuccessfully waited %s for a new %s repo" % (koji.util.duration(self.start), self.taginfo['name'])) if not check['request']['active']: + if check['request']['task_id']: + tstate = koji.TASK_STATES[check['request']['task_state']] + self.logger.error('Task %s state is %s', check['request']['task_id'], tstate) raise koji.GenericError("Repo request no longer active") self.pause() From 02e0edbc1108eca87581bb595fe3a3e9019b04b8 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2024 22:44:19 +0000 Subject: [PATCH 3/8] partial: split autoregen --- diff --git a/util/kojira b/util/kojira index 5e98f39..8785b7d 100755 --- a/util/kojira +++ b/util/kojira @@ -480,7 +480,7 @@ class RepoManager(object): self.logger.info('regenLoop starting') try: while True: - self.regenRepos() + self.session.repo.checkQueue() time.sleep(self.options.sleeptime) except Exception: self.logger.exception('Error in regen thread') @@ -488,6 +488,21 @@ class RepoManager(object): finally: session.logout() + def autoregenLoop(self, session): + """Triggers automatic regens as needed/possible. Runs in a separate thread""" + self.session = session + self.logger = logging.getLogger("koji.repo.autoregen") + self.logger.info('autoregenLoop starting') + try: + while True: + self.session.repo.autoRequests() + time.sleep(self.options.sleeptime) + except Exception: + self.logger.exception('Error in auto regen thread') + raise + finally: + session.logout() + def rmtreeLoop(self, session): self.session = session logger = logging.getLogger("koji.repo.rmtree") @@ -648,11 +663,6 @@ class RepoManager(object): elif repo.state == koji.REPO_PROBLEM: repo.handle_problem() - def regenRepos(self): - """Trigger repo requests as needed""" - self.session.repo.autoRequests() - self.session.repo.checkQueue() - def start_currency_checker(session, repomgr): subsession = session.subsession() From 99def390798d9a12878538cebd3a36f947ec6077 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2024 22:44:19 +0000 Subject: [PATCH 4/8] unify thread init and include autoregen --- diff --git a/util/kojira b/util/kojira index 8785b7d..1ef3ad3 100755 --- a/util/kojira +++ b/util/kojira @@ -664,37 +664,10 @@ class RepoManager(object): repo.handle_problem() -def start_currency_checker(session, repomgr): +def start_thread(session, repomgr, name): + handler = getattr(repomgr, name) subsession = session.subsession() - thread = threading.Thread(name='currencyChecker', - target=repomgr.currencyChecker, args=(subsession,)) - thread.daemon = True - thread.start() - return thread - - -def start_external_currency_checker(session, repomgr): - subsession = session.subsession() - thread = threading.Thread(name='currencyExternalChecker', - target=repomgr.currencyExternalChecker, args=(subsession,)) - thread.daemon = True - thread.start() - return thread - - -def start_regen_loop(session, repomgr): - subsession = session.subsession() - thread = threading.Thread(name='regenLoop', - target=repomgr.regenLoop, args=(subsession,)) - thread.daemon = True - thread.start() - return thread - - -def start_rmtree_loop(session, repomgr): - subsession = session.subsession() - thread = threading.Thread(name='rmtreeLoop', - target=repomgr.rmtreeLoop, args=(subsession,)) + thread = threading.Thread(name=name, target=handler, args=(subsession,)) thread.daemon = True thread.start() return thread @@ -707,29 +680,20 @@ def main(options, session): def shutdown(*args): raise SystemExit signal.signal(signal.SIGTERM, shutdown) - curr_chk_thread = start_currency_checker(session, repomgr) + tnames = ['currencyChecker', 'regenLoop', 'autoregenLoop', 'rmtreeLoop'] if options.check_external_repos: - curr_ext_chk_thread = start_external_currency_checker(session, repomgr) - regen_thread = start_regen_loop(session, repomgr) - rmtree_thread = start_rmtree_loop(session, repomgr) + tnames.append('currencyExternalChecker') + threads = {name: start_thread(session, repomgr, name) for name in tnames} logger.info("Entering main loop") while True: try: repomgr.updateRepos() repomgr.printState() repomgr.pruneLocalRepos() - if not curr_chk_thread.is_alive(): - logger.error("Currency checker thread died. Restarting it.") - curr_chk_thread = start_currency_checker(session, repomgr) - if options.check_external_repos and not curr_ext_chk_thread.is_alive(): - logger.error("External currency checker thread died. Restarting it.") - curr_ext_chk_thread = start_external_currency_checker(session, repomgr) - if not regen_thread.is_alive(): - logger.error("Regeneration thread died. Restarting it.") - regen_thread = start_regen_loop(session, repomgr) - if not rmtree_thread.is_alive(): - logger.error("rmtree thread died. Restarting it.") - rmtree_thread = start_rmtree_loop(session, repomgr) + for name in tnames: + if not threads[name].is_alive(): + logger.error(f'{name} thread died. Restarting it.') + threads[name] = start_thread(session, repomgr, name) except KeyboardInterrupt: logger.warning("User exit") break From 3d1217b6ae5e1f9e407815eafd53b53c50c48290 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2024 22:44:19 +0000 Subject: [PATCH 5/8] unify thread handler loops --- diff --git a/util/kojira b/util/kojira index 1ef3ad3..1d76ffd 100755 --- a/util/kojira +++ b/util/kojira @@ -442,80 +442,43 @@ class RepoManager(object): data['id'] = erepo_id self.checkExternalRepo(data, arches, ts_cache) - def currencyChecker(self, session): - """Continually checks repos for currency. Runs as a separate thread""" + def threadLoop(self, session, name): + """Wrapper for running thread handlers in a loop""" + # we should be passed a subsession of main self.session = session - self.logger = logging.getLogger("koji.repo.currency") - self.logger.info('currencyChecker starting') + handler = getattr(self, f'do_{name}') + self.logger = logging.getLogger('koji.repo.{name}') + self.logger.info('{name} thread starting') try: while True: - self.session.repo.updateEndEvents() - # TODO does this still need to be its own thread? + handler() time.sleep(self.options.sleeptime) except Exception: - self.logger.exception('Error in currency checker thread') + self.logger.exception(f'Error in {name} thread') raise finally: session.logout() - def currencyExternalChecker(self, session): - """Continually checks repos for external repo currency. Runs as a separate thread""" - self.session = session - self.logger = logging.getLogger("koji.repo.currency_external") - self.logger.info('currencyExternalChecker starting') - try: - while True: - self.checkExternalRepos() - time.sleep(self.options.sleeptime) - except Exception: - self.logger.exception('Error in external currency checker thread') - raise - finally: - session.logout() + def do_currency(self): + """Checks repos for currency""" + # this call can take a while + self.session.repo.updateEndEvents() - def regenLoop(self, session): - """Triggers regens as needed/possible. Runs in a separate thread""" - self.session = session - self.logger = logging.getLogger("koji.repo.regen") - self.logger.info('regenLoop starting') - try: - while True: - self.session.repo.checkQueue() - time.sleep(self.options.sleeptime) - except Exception: - self.logger.exception('Error in regen thread') - raise - finally: - session.logout() + def do_check_external(self): + """Check external repos""" + self.checkExternalRepos() - def autoregenLoop(self, session): - """Triggers automatic regens as needed/possible. Runs in a separate thread""" - self.session = session - self.logger = logging.getLogger("koji.repo.autoregen") - self.logger.info('autoregenLoop starting') - try: - while True: - self.session.repo.autoRequests() - time.sleep(self.options.sleeptime) - except Exception: - self.logger.exception('Error in auto regen thread') - raise - finally: - session.logout() + def do_regen(self, session): + """Triggers regens as needed/possible""" + self.session.repo.checkQueue() - def rmtreeLoop(self, session): - self.session = session - logger = logging.getLogger("koji.repo.rmtree") - try: - while True: - logger.debug('queue length: %d', len(self.delete_queue)) - self.checkQueue() - time.sleep(self.options.sleeptime) - except Exception: - logger.exception('Error in rmtree thread') - raise - finally: - session.logout() + def do_autoregen(self, session): + """Triggers automatic regens as needed/possible""" + self.session.repo.autoRequests() + + def do_rmtree(self, session): + logger.debug('queue length: %d', len(self.delete_queue)) + self.checkQueue() def pruneLocalRepos(self): # non-dist repos are always on the default volume @@ -680,10 +643,10 @@ def main(options, session): def shutdown(*args): raise SystemExit signal.signal(signal.SIGTERM, shutdown) - tnames = ['currencyChecker', 'regenLoop', 'autoregenLoop', 'rmtreeLoop'] + tnames = ['currency', 'regen', 'autoregen', 'rmtree'] if options.check_external_repos: - tnames.append('currencyExternalChecker') - threads = {name: start_thread(session, repomgr, name) for name in tnames} + tnames.append('check_external') + threads = {name: repomgr.threadLoop(session, name) for name in tnames} logger.info("Entering main loop") while True: try: From 083a4c0c49b6b4c3105d50f306dbbf3d6f8c87ca Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 22 2024 22:44:19 +0000 Subject: [PATCH 6/8] adjust unit test --- diff --git a/tests/test_kojira/test_repo_manager.py b/tests/test_kojira/test_repo_manager.py index 85ec47e..8192f98 100644 --- a/tests/test_kojira/test_repo_manager.py +++ b/tests/test_kojira/test_repo_manager.py @@ -42,54 +42,16 @@ class RepoManagerTest(unittest.TestCase): return pid, 0 @mock.patch('time.sleep') - def test_regen_loop(self, sleep): + def test_thread_loop(self, sleep): subsession = mock.MagicMock() - self.mgr.regenRepos = mock.MagicMock() - self.mgr.regenRepos.side_effect = [None] * 10 + [OurException()] + self.mgr.do_regen = mock.MagicMock() + self.mgr.do_regen.side_effect = [None] * 10 + [OurException()] # we need the exception to terminate the infinite loop with self.assertRaises(OurException): - self.mgr.regenLoop(subsession) + self.mgr.threadLoop(subsession, 'regen') - self.assertEqual(self.mgr.regenRepos.call_count, 11) - subsession.logout.assert_called_once() - - @mock.patch('time.sleep') - def test_rmtree_loop(self, sleep): - subsession = mock.MagicMock() - self.mgr.checkQueue = mock.MagicMock() - self.mgr.checkQueue.side_effect = [None] * 10 + [OurException()] - # we need the exception to terminate the infinite loop - - with self.assertRaises(OurException): - self.mgr.rmtreeLoop(subsession) - - self.assertEqual(self.mgr.checkQueue.call_count, 11) - subsession.logout.assert_called_once() - - @mock.patch('time.sleep') - def test_currency_loop(self, sleep): - subsession = mock.MagicMock() - subsession.repo.updateEndEvents.side_effect = [None] * 10 + [OurException()] - # we need the exception to terminate the infinite loop - - with self.assertRaises(OurException): - self.mgr.currencyChecker(subsession) - - self.assertEqual(subsession.repo.updateEndEvents.call_count, 11) - subsession.logout.assert_called_once() - - @mock.patch('time.sleep') - def test_external_loop(self, sleep): - subsession = mock.MagicMock() - self.mgr.checkExternalRepos = mock.MagicMock() - self.mgr.checkExternalRepos.side_effect = [None] * 10 + [OurException()] - # we need the exception to terminate the infinite loop - - with self.assertRaises(OurException): - self.mgr.currencyExternalChecker(subsession) - - self.assertEqual(self.mgr.checkExternalRepos.call_count, 11) + self.assertEqual(self.mgr.do_regen.call_count, 11) subsession.logout.assert_called_once() def test_rmtree(self): diff --git a/util/kojira b/util/kojira index 1d76ffd..77064bc 100755 --- a/util/kojira +++ b/util/kojira @@ -468,15 +468,15 @@ class RepoManager(object): """Check external repos""" self.checkExternalRepos() - def do_regen(self, session): + def do_regen(self): """Triggers regens as needed/possible""" self.session.repo.checkQueue() - def do_autoregen(self, session): + def do_autoregen(self): """Triggers automatic regens as needed/possible""" self.session.repo.autoRequests() - def do_rmtree(self, session): + def do_rmtree(self): logger.debug('queue length: %d', len(self.delete_queue)) self.checkQueue() From 75e9117ce3a67af8a2ec66cb96c9877122579cc7 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 25 2024 02:36:18 +0000 Subject: [PATCH 7/8] fixes --- diff --git a/util/kojira b/util/kojira index 77064bc..e01e49f 100755 --- a/util/kojira +++ b/util/kojira @@ -447,8 +447,8 @@ class RepoManager(object): # we should be passed a subsession of main self.session = session handler = getattr(self, f'do_{name}') - self.logger = logging.getLogger('koji.repo.{name}') - self.logger.info('{name} thread starting') + self.logger = logging.getLogger(f'koji.repo.{name}') + self.logger.info(f'{name} thread starting') try: while True: handler() @@ -628,9 +628,9 @@ class RepoManager(object): def start_thread(session, repomgr, name): - handler = getattr(repomgr, name) + handler = getattr(repomgr, 'threadLoop') subsession = session.subsession() - thread = threading.Thread(name=name, target=handler, args=(subsession,)) + thread = threading.Thread(name=name, target=handler, args=(subsession, name)) thread.daemon = True thread.start() return thread @@ -646,7 +646,7 @@ def main(options, session): tnames = ['currency', 'regen', 'autoregen', 'rmtree'] if options.check_external_repos: tnames.append('check_external') - threads = {name: repomgr.threadLoop(session, name) for name in tnames} + threads = {name: start_thread(session, repomgr, name) for name in tnames} logger.info("Entering main loop") while True: try: From 019d8f67b4d9521f20c52ed8cb261e046c4582d9 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 25 2024 02:36:36 +0000 Subject: [PATCH 8/8] more tests --- diff --git a/tests/test_kojira/test_main.py b/tests/test_kojira/test_main.py new file mode 100644 index 0000000..0d5daa8 --- /dev/null +++ b/tests/test_kojira/test_main.py @@ -0,0 +1,84 @@ +from __future__ import absolute_import + +import os.path +import shutil +import signal +import tempfile +import time +import unittest + +import mock +import pytest + +import koji + +from . import loadkojira + +kojira = loadkojira.kojira + + +class MyError(Exception): + """sentinel exception""" + pass + + +class MainTest(unittest.TestCase): + + def setUp(self): + self.start_thread = mock.patch.object(kojira, 'start_thread').start() + self.repomgr = mock.MagicMock() + self.RepoManager = mock.patch.object(kojira, 'RepoManager', return_value=self.repomgr).start() + self.session = mock.MagicMock() + self.options = mock.MagicMock() + self.options.sleeptime = 1 + self.workdir = tempfile.mkdtemp() + self.topdir = self.workdir + '/koji' + self.pathinfo = koji.PathInfo(self.topdir) + mock.patch.object(kojira, 'pathinfo', create=True, new=self.pathinfo).start() + self.logger = mock.patch.object(kojira, 'logger', create=True).start() + self.sleep = mock.patch('time.sleep').start() + self.signal = mock.patch('signal.signal').start() + + def tearDown(self): + mock.patch.stopall() + shutil.rmtree(self.workdir) + + def test_userkill1(self): + self.sleep.side_effect = [None] * 10 + [KeyboardInterrupt()] + kojira.main(self.options, self.session) + + def test_terminal_errors(self): + for cls in KeyboardInterrupt, koji.AuthExpired, koji.AuthError, SystemExit: + err = cls() + self.sleep.side_effect = [None] * 10 + [Exception()] + self.repomgr.updateRepos.side_effect = [None] * 5 + [err] + kojira.main(self.options, self.session) + self.assertEqual(len(self.repomgr.pruneLocalRepos.mock_calls), 5) + self.repomgr.reset_mock() + + def test_nonterminal_error(self): + err = MyError() + self.sleep.side_effect = [None] * 10 + [KeyboardInterrupt()] + self.repomgr.updateRepos.side_effect = [None] * 5 + [err] * 6 + kojira.main(self.options, self.session) + self.assertEqual(len(self.repomgr.updateRepos.mock_calls), 11) + self.assertEqual(len(self.repomgr.pruneLocalRepos.mock_calls), 5) + self.repomgr.reset_mock() + + def test_shutdown_handler(self): + self.signal.side_effect = [Exception('stop here')] + with self.assertRaises(Exception): + kojira.main(self.options, self.session) + + # grab the handler + self.assertEqual(self.signal.mock_calls[0][1][0], signal.SIGTERM) + handler = self.signal.mock_calls[0][1][1] + + self.signal.side_effect = None + + # make sure the handler does what it should + with self.assertRaises(SystemExit): + handler() + + +# the end