From 9eadaed9812cc02e97602cb3344e0c69a03e0f81 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
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 <mikem@redhat.com>
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 <mikem@redhat.com>
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 <mikem@redhat.com>
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 <mikem@redhat.com>
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 <mikem@redhat.com>
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 <mikem@redhat.com>
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)

