From 9eadaed9812cc02e97602cb3344e0c69a03e0f81 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 02 2024 18:27:28 +0000 Subject: [PATCH 1/7] auto_arch_refuse() --- diff --git a/kojihub/scheduler.py b/kojihub/scheduler.py index 8aa7386..0ba2fb3 100644 --- a/kojihub/scheduler.py +++ b/kojihub/scheduler.py @@ -104,6 +104,55 @@ def set_refusal(hostID, taskID, soft=True, by_host=False, msg=''): log_both(f'Host refused task: {msg}', task_id=taskID, host_id=hostID) +def auto_arch_refuse(task_id): + """Set refusals for hosts based on task parameters""" + # HACK + task = kojihub.Task(task_id) + info = task.getInfo(request=True) + if info['arch'] != 'noarch': + return + if info['method'] not in {'buildArch', 'buildMaven', 'wrapperRPM', 'rebuildSRPM', + 'buildSRPMFromSCM', 'rebuildSRPM'}: + return + + try: + task_params = koji.tasks.parse_task_params(info['method'], info['request']) + except Exception: + logger.warning('Invalid params for task %i', task_id) + return + + try: + # figure out build tag + if info['method'] in {'buildMaven', 'buildSRPMFromSCM', 'rebuildSRPM'}: + tag = task_params['build_tag'] + elif info['method'] == 'buildArch': + tag = task_params['root'] + elif info['method'] == 'wrapperRPM': + target = kojihub.get_build_target(task_params['build_target']) + if not target: + logger.warning('Invalid target for task %i', task_id) + return + tag = target['build_tag'] + taginfo = kojihub.get_tag(tag) + if not taginfo: + logger.warning('Invalid build tag for task %i', task_id) + return + + # from here, we're basically doing checkHostArch() for all hosts in the channel + tag_arches = set([koji.canonArch(a) for a in taginfo['arches'].split()]) + + hosts = context.handlers.call('listHosts', channelID=info['channel_id'], enabled=True) + for host in hosts: + host_arches = host['arches'].split() + logger.debug('%r vs %r', tag_arches, host_arches) + if not tag_arches.intersection(host_arches): + set_refusal(host['id'], task_id, soft=False, msg='automatic arch refusal') + + except Exception: + logger.exception('Error generating auto refusals for task %i', task_id) + return + + class TaskRefusalsQuery(QueryView): tables = ['scheduler_task_refusals'] From f507263932924679ce10e76928dcc87474f6419e Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 02 2024 18:27:28 +0000 Subject: [PATCH 2/7] trigger auto refusals in make_task --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 7d8ddd1..b5b30fa 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -747,6 +747,7 @@ def make_task(method, arglist, **opts): opts['id'] = task_id koji.plugin.run_callbacks( 'postTaskStateChange', attribute='state', old=None, new='FREE', info=opts) + scheduler.auto_arch_refuse(task_id) # temporary hack return task_id diff --git a/kojihub/scheduler.py b/kojihub/scheduler.py index 0ba2fb3..c3b8287 100644 --- a/kojihub/scheduler.py +++ b/kojihub/scheduler.py @@ -114,6 +114,10 @@ def auto_arch_refuse(task_id): if info['method'] not in {'buildArch', 'buildMaven', 'wrapperRPM', 'rebuildSRPM', 'buildSRPMFromSCM', 'rebuildSRPM'}: return + if task.isFinished(): + # shouldn't happen + logger.warning('Skipping auto refusal for closed task %i', task_id) + return try: task_params = koji.tasks.parse_task_params(info['method'], info['request']) @@ -141,7 +145,8 @@ def auto_arch_refuse(task_id): # from here, we're basically doing checkHostArch() for all hosts in the channel tag_arches = set([koji.canonArch(a) for a in taginfo['arches'].split()]) - hosts = context.handlers.call('listHosts', channelID=info['channel_id'], enabled=True) + hosts = context.handlers.call('listHosts', channelID=info['channel_id'], enabled=True, + queryOpts={'order':'id'}) for host in hosts: host_arches = host['arches'].split() logger.debug('%r vs %r', tag_arches, host_arches) From 260f559cbd77c4d375f3016f77483bc3d0bde6b3 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 02 2024 18:27:28 +0000 Subject: [PATCH 3/7] typo --- diff --git a/kojihub/scheduler.py b/kojihub/scheduler.py index c3b8287..0fd8c9c 100644 --- a/kojihub/scheduler.py +++ b/kojihub/scheduler.py @@ -112,7 +112,7 @@ def auto_arch_refuse(task_id): if info['arch'] != 'noarch': return if info['method'] not in {'buildArch', 'buildMaven', 'wrapperRPM', 'rebuildSRPM', - 'buildSRPMFromSCM', 'rebuildSRPM'}: + 'buildSRPMFromSCM'}: return if task.isFinished(): # shouldn't happen From d1490f71363066c54490746db209d745fc784195 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 02 2024 18:27:28 +0000 Subject: [PATCH 4/7] add unit test and adjust error handling (no errors were found with the code, but needed to avoid masking them in tests) --- diff --git a/kojihub/scheduler.py b/kojihub/scheduler.py index 0fd8c9c..adc6aff 100644 --- a/kojihub/scheduler.py +++ b/kojihub/scheduler.py @@ -107,6 +107,15 @@ def set_refusal(hostID, taskID, soft=True, by_host=False, msg=''): def auto_arch_refuse(task_id): """Set refusals for hosts based on task parameters""" # HACK + try: + _auto_arch_refuse(task_id) + except Exception: + # better to not fail make_task() + logger.exception('Error generating auto refusals for task %i', task_id) + return + + +def _auto_arch_refuse(task_id): task = kojihub.Task(task_id) info = task.getInfo(request=True) if info['arch'] != 'noarch': @@ -125,37 +134,32 @@ def auto_arch_refuse(task_id): logger.warning('Invalid params for task %i', task_id) return - try: - # figure out build tag - if info['method'] in {'buildMaven', 'buildSRPMFromSCM', 'rebuildSRPM'}: - tag = task_params['build_tag'] - elif info['method'] == 'buildArch': - tag = task_params['root'] - elif info['method'] == 'wrapperRPM': - target = kojihub.get_build_target(task_params['build_target']) - if not target: - logger.warning('Invalid target for task %i', task_id) - return - tag = target['build_tag'] - taginfo = kojihub.get_tag(tag) - if not taginfo: - logger.warning('Invalid build tag for task %i', task_id) + # figure out build tag + if info['method'] in {'buildMaven', 'buildSRPMFromSCM', 'rebuildSRPM'}: + tag = task_params['build_tag'] + elif info['method'] == 'buildArch': + tag = task_params['root'] + elif info['method'] == 'wrapperRPM': + target = kojihub.get_build_target(task_params['build_target']) + if not target: + logger.warning('Invalid target for task %i', task_id) return + tag = target['build_tag'] + taginfo = kojihub.get_tag(tag) + if not taginfo: + logger.warning('Invalid build tag for task %i', task_id) + return - # from here, we're basically doing checkHostArch() for all hosts in the channel - tag_arches = set([koji.canonArch(a) for a in taginfo['arches'].split()]) - - hosts = context.handlers.call('listHosts', channelID=info['channel_id'], enabled=True, - queryOpts={'order':'id'}) - for host in hosts: - host_arches = host['arches'].split() - logger.debug('%r vs %r', tag_arches, host_arches) - if not tag_arches.intersection(host_arches): - set_refusal(host['id'], task_id, soft=False, msg='automatic arch refusal') + # from here, we're basically doing checkHostArch() for all hosts in the channel + tag_arches = set([koji.canonArch(a) for a in taginfo['arches'].split()]) - except Exception: - logger.exception('Error generating auto refusals for task %i', task_id) - return + hosts = context.handlers.call('listHosts', channelID=info['channel_id'], enabled=True, + queryOpts={'order': 'id'}) + for host in hosts: + host_arches = host['arches'].split() + logger.debug('%r vs %r', tag_arches, host_arches) + if not tag_arches.intersection(host_arches): + set_refusal(host['id'], task_id, soft=False, msg='automatic arch refusal') class TaskRefusalsQuery(QueryView): diff --git a/tests/test_hub/test_auto_arch_refuse.py b/tests/test_hub/test_auto_arch_refuse.py new file mode 100644 index 0000000..96c6f3a --- /dev/null +++ b/tests/test_hub/test_auto_arch_refuse.py @@ -0,0 +1,182 @@ +import datetime +import mock +import unittest + +import koji +import kojihub +import kojihub.db +from kojihub import scheduler + + +QP = scheduler.QueryProcessor +IP = scheduler.InsertProcessor +UP = scheduler.UpdateProcessor +TASK = kojihub.Task + + +class MyError(Exception): + pass + + +class AutoRefuseTest(unittest.TestCase): + + def setUp(self): + self._dml = mock.patch('kojihub.db._dml').start() + # self.exports = kojihub.RootExports() + self.task = mock.MagicMock() + self.Task = mock.patch('kojihub.kojihub.Task', return_value=self.task).start() + self.get_build_target = mock.patch('kojihub.kojihub.get_build_target').start() + self.get_tag = mock.patch('kojihub.kojihub.get_tag').start() + self.context = mock.patch('kojihub.scheduler.context').start() + self.set_refusal = mock.patch('kojihub.scheduler.set_refusal').start() + self.set_base_data() + + def tearDown(self): + mock.patch.stopall() + + def set_base_data(self): + request = [ + 'tasks/8755/59888755/release-e2e-test-1.0.4474-1.el9.src.rpm', + 'TAG_ID', + 'x86_64', + True, + {'repo_id': 8075973}] + self.taskinfo = { + 'arch': 'noarch', + 'channel_id': 35, + 'id': 59888794, + 'method': 'buildArch', + 'request': request, + 'state': 1, + } + self.task.getInfo.return_value = self.taskinfo + self.task.isFinished.return_value = False + self.get_tag.return_value = {'arches': 'x86_64 s390x ppc64le aarch64'} + self.context.handlers.call.return_value = [{'id': 'HOST', 'arches': 'x86_64 i686'}] + + def test_arch_overlap(self): + # we mostly test the underlying function to avoid masking errors + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_tag.assert_called_once_with('TAG_ID') + self.set_refusal.assert_not_called() + + def test_arch_disjoint(self): + self.context.handlers.call.return_value = [{'id': 'HOST', 'arches': 'riscv128'}] + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_tag.assert_called_once_with('TAG_ID') + self.set_refusal.assert_called_once() + + def test_not_noarch(self): + self.taskinfo['arch'] = 'x86_64' + + scheduler._auto_arch_refuse(100) + + self.task.isFinished.assert_not_called() + + def test_other_method(self): + self.taskinfo['method'] = 'build' + + scheduler._auto_arch_refuse(100) + + self.task.isFinished.assert_not_called() + + def test_task_finished(self): + self.task.isFinished.return_value = True + + scheduler._auto_arch_refuse(100) + + self.get_tag.assert_not_called() + + def test_bad_tag(self): + self.get_tag.return_value = None + + scheduler._auto_arch_refuse(100) + + self.context.handlers.call.assert_not_called() + + def test_bad_params(self): + self.taskinfo['request'] = [] + + scheduler._auto_arch_refuse(100) + + self.get_tag.assert_not_called() + + def test_unexpected_error(self): + self.get_tag.side_effect = MyError('should be caught') + + # the wrapper should catch this + scheduler.auto_arch_refuse(100) + + self.context.handlers.call.assert_not_called() + + def test_unexpected_error2(self): + self.get_tag.side_effect = MyError('should not be caught') + + # the underlying call should not + with self.assertRaises(MyError): + scheduler._auto_arch_refuse(100) + + self.context.handlers.call.assert_not_called() + + def test_from_scm(self): + self.taskinfo['method'] = 'buildSRPMFromSCM' + self.taskinfo['request'] = [ + 'git+https://HOST/PATH', + 'TAG_ID', + {'repo_id': 8075973, 'scratch': None}] + + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_tag.assert_called_once_with('TAG_ID') + self.set_refusal.assert_not_called() + + def test_from_srpm(self): + self.taskinfo['method'] = 'rebuildSRPM' + self.taskinfo['request'] = [ + 'cli-build/1709137799.6498768.BFGhzghk/fake-1.1-35.src.rpm', + 'TAG_ID', + {'repo_id': 2330, 'scratch': True}] + + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_tag.assert_called_once_with('TAG_ID') + self.set_refusal.assert_not_called() + + def test_wrapper(self): + self.taskinfo['method'] = 'wrapperRPM' + self.taskinfo['request'] = [ + 'git://HOST/PATH', + 'TARGET', + {'build_id': 421}, + None, + {'repo_id': 958, 'scratch': True}] + self.get_build_target.return_value = {'build_tag': 'TAG_ID'} + + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_build_target.assert_called_once_with('TARGET') + self.get_tag.assert_called_once_with('TAG_ID') + self.set_refusal.assert_not_called() + + def test_bad_target(self): + self.taskinfo['method'] = 'wrapperRPM' + self.taskinfo['request'] = [ + 'git://HOST/PATH', + 'TARGET', + {'build_id': 421}, + None, + {'repo_id': 958, 'scratch': True}] + self.get_build_target.return_value = None + + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_build_target.assert_called_once_with('TARGET') + self.get_tag.assert_not_called() From e0d76076d4da94aaafa0f68493b290ce1f5f74ac Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 02 2024 18:27:28 +0000 Subject: [PATCH 5/7] one more test --- diff --git a/tests/test_hub/test_auto_arch_refuse.py b/tests/test_hub/test_auto_arch_refuse.py index 96c6f3a..6e320f5 100644 --- a/tests/test_hub/test_auto_arch_refuse.py +++ b/tests/test_hub/test_auto_arch_refuse.py @@ -70,6 +70,22 @@ class AutoRefuseTest(unittest.TestCase): self.get_tag.assert_called_once_with('TAG_ID') self.set_refusal.assert_called_once() + def test_mixed_hosts(self): + good1 = [{'id': n, 'arches': 'x86_64 i686'} for n in range(0,5)] + bad1 = [{'id': n, 'arches': 'ia64'} for n in range(5,10)] + good2 = [{'id': n, 'arches': 'aarch64'} for n in range(10,15)] + bad2 = [{'id': n, 'arches': 'sparc64'} for n in range(15,20)] + hosts = good1 + bad1 + good2 + bad2 + self.context.handlers.call.return_value = hosts + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_tag.assert_called_once_with('TAG_ID') + + # should only refuse the bad ones + expect = [mock.call(h['id'], 100, soft=False, msg='automatic arch refusal') for h in bad1 + bad2] + self.assertListEqual(self.set_refusal.mock_calls, expect) + def test_not_noarch(self): self.taskinfo['arch'] = 'x86_64' From 641a3bb9e9cfde6bdd78a5b00365ca9f5f4c2a9a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 02 2024 18:27:28 +0000 Subject: [PATCH 6/7] adjust comments --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index b5b30fa..569665b 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -747,7 +747,7 @@ def make_task(method, arglist, **opts): opts['id'] = task_id koji.plugin.run_callbacks( 'postTaskStateChange', attribute='state', old=None, new='FREE', info=opts) - scheduler.auto_arch_refuse(task_id) # temporary hack + scheduler.auto_arch_refuse(task_id) # temporary workaround return task_id diff --git a/kojihub/scheduler.py b/kojihub/scheduler.py index adc6aff..e83996d 100644 --- a/kojihub/scheduler.py +++ b/kojihub/scheduler.py @@ -106,7 +106,7 @@ def set_refusal(hostID, taskID, soft=True, by_host=False, msg=''): def auto_arch_refuse(task_id): """Set refusals for hosts based on task parameters""" - # HACK + # This is a temporary workaround try: _auto_arch_refuse(task_id) except Exception: From 177f45b09215734699f53046705e5baefc5eed69 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 02 2024 18:27:28 +0000 Subject: [PATCH 7/7] adjust handling for tags with no arches --- diff --git a/kojihub/scheduler.py b/kojihub/scheduler.py index e83996d..c649827 100644 --- a/kojihub/scheduler.py +++ b/kojihub/scheduler.py @@ -151,7 +151,13 @@ def _auto_arch_refuse(task_id): return # from here, we're basically doing checkHostArch() for all hosts in the channel - tag_arches = set([koji.canonArch(a) for a in taginfo['arches'].split()]) + buildconfig = context.handlers.call('getBuildConfig', taginfo['id']) + # getBuildConfig traverses inheritance to find arches if tag does not have them + tag_arches = set([koji.canonArch(a) for a in buildconfig['arches'].split()]) + if not tag_arches: + logger.warning("No arches for tag %(name)s [%(id)s]", taginfo) + # we don't error here, allowing the task itself to fail + return hosts = context.handlers.call('listHosts', channelID=info['channel_id'], enabled=True, queryOpts={'order': 'id'}) diff --git a/tests/test_hub/test_auto_arch_refuse.py b/tests/test_hub/test_auto_arch_refuse.py index 6e320f5..674e393 100644 --- a/tests/test_hub/test_auto_arch_refuse.py +++ b/tests/test_hub/test_auto_arch_refuse.py @@ -29,6 +29,11 @@ class AutoRefuseTest(unittest.TestCase): self.get_tag = mock.patch('kojihub.kojihub.get_tag').start() self.context = mock.patch('kojihub.scheduler.context').start() self.set_refusal = mock.patch('kojihub.scheduler.set_refusal').start() + self.handlers = { + 'getBuildConfig': mock.MagicMock(), + 'listHosts': mock.MagicMock(), + } + self.context.handlers.call.side_effect = self._my_handler_call self.set_base_data() def tearDown(self): @@ -51,8 +56,13 @@ class AutoRefuseTest(unittest.TestCase): } self.task.getInfo.return_value = self.taskinfo self.task.isFinished.return_value = False - self.get_tag.return_value = {'arches': 'x86_64 s390x ppc64le aarch64'} - self.context.handlers.call.return_value = [{'id': 'HOST', 'arches': 'x86_64 i686'}] + self.get_tag.return_value = {'id': 'TAGID', 'name': 'MYTAG'} + self.handlers['listHosts'].return_value = [{'id': 'HOST', 'arches': 'x86_64 i686'}] + self.handlers['getBuildConfig'].return_value = {'arches': 'x86_64 s390x ppc64le aarch64'} + + def _my_handler_call(self, method, *a, **kw): + handler = self.handlers[method] + return handler(*a, **kw) def test_arch_overlap(self): # we mostly test the underlying function to avoid masking errors @@ -63,20 +73,29 @@ class AutoRefuseTest(unittest.TestCase): self.set_refusal.assert_not_called() def test_arch_disjoint(self): - self.context.handlers.call.return_value = [{'id': 'HOST', 'arches': 'riscv128'}] + self.handlers['listHosts'].return_value = [{'id': 'HOST', 'arches': 'riscv128'}] scheduler._auto_arch_refuse(100) self.Task.assert_called_once_with(100) self.get_tag.assert_called_once_with('TAG_ID') self.set_refusal.assert_called_once() + def test_no_tag_arches(self): + self.handlers['getBuildConfig'].return_value = {'arches': ''} + scheduler._auto_arch_refuse(100) + + self.Task.assert_called_once_with(100) + self.get_tag.assert_called_once_with('TAG_ID') + self.handlers['listHosts'].assert_not_called() + self.set_refusal.assert_not_called() + def test_mixed_hosts(self): good1 = [{'id': n, 'arches': 'x86_64 i686'} for n in range(0,5)] bad1 = [{'id': n, 'arches': 'ia64'} for n in range(5,10)] good2 = [{'id': n, 'arches': 'aarch64'} for n in range(10,15)] bad2 = [{'id': n, 'arches': 'sparc64'} for n in range(15,20)] hosts = good1 + bad1 + good2 + bad2 - self.context.handlers.call.return_value = hosts + self.handlers['listHosts'].return_value = hosts scheduler._auto_arch_refuse(100) self.Task.assert_called_once_with(100)