From a8ac9316d460f175dfb582d13f47bace5c122d1f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Dec 19 2019 14:03:31 +0000 Subject: [PATCH 1/2] limit distRepo tasks per tag Introduces 'distrepo.cancel_others` extra flag for tags. If enabled, new distRepo task will cancel previous non-finished ones leaving only new one. Fixes: https://pagure.io/koji/issue/1630 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index faca3a9..77fe083 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -6952,8 +6952,14 @@ def handle_regen_repo(options, session, args): def handle_dist_repo(options, session, args): """Create a yum repo with distribution options""" - usage = _("usage: %prog dist-repo [options] [ ...]") - parser = OptionParser(usage=get_usage_str(usage)) + usage = _("usage: %prog dist-repo [options] [ ...]\n\n" + "In normal mode, dist-repo behaves like any other koji task.\n" + "Sometimes you want to limit running distRepo tasks per tag to only\n" + "one. For such behaviour admin (with 'tag' permission) needs to\n" + "modify given tag's extra field 'distrepo.cancel_others' to True'\n" + "via 'koji edit-tag -x distrepo.cancel_others=True'\n") + usage += _("\n(Specify the --help option for a list of other options)") + parser = OptionParser(usage=usage) parser.add_option('--allow-missing-signatures', action='store_true', default=False, help=_('For RPMs not signed with a desired key, fall back to the ' diff --git a/hub/kojihub.py b/hub/kojihub.py index 5f8c3cd..31d4a76 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -11436,6 +11436,20 @@ class RootExports(object): context.session.assertPerm('dist-repo') repo_id, event_id = dist_repo_init(tag, keys, task_opts) task_opts['event'] = event_id + # cancel potentially running distRepos + tinfo = get_tag(tag, strict=True) + if tinfo['extra'].get('distrepo.cancel_others', False): + tasks = self.listTasks(opts={ + 'state': [koji.TASK_STATES['FREE'], + koji.TASK_STATES['OPEN'], + koji.TASK_STATES['ASSIGNED']], + 'method': 'distRepo', + 'decode': True}) + # filter only for this tag + task_ids = [t['id'] for t in tasks if t['request'][0] == tag] + for task_id in task_ids: + logger.debug("Cancelling distRepo task %d" % task_id) + Task(task_id).cancel(recurse=True) return make_task('distRepo', [tag, repo_id, keys, task_opts], priority=15, channel='createrepo') def newRepo(self, tag, event=None, src=False, debuginfo=False, separate_src=False): diff --git a/tests/test_cli/test_dist_repo.py b/tests/test_cli/test_dist_repo.py index f625529..37cac0b 100644 --- a/tests/test_cli/test_dist_repo.py +++ b/tests/test_cli/test_dist_repo.py @@ -45,7 +45,14 @@ class TestDistRepo(utils.CliTestCase): self.session.distRepo.return_value = self.task_id self.error_format = """Usage: %s dist-repo [options] [ ...] -(Specify the --help global option for a list of other help options) + +In normal mode, dist-repo behaves like any other koji task. +Sometimes you want to limit running distRepo tasks per tag to only +one. For such behaviour admin (with 'tag' permission) needs to +modify given tag's extra field 'distrepo.cancel_others' to True' +via 'koji edit-tag -x distrepo.cancel_others=True' + +(Specify the --help option for a list of other options) %s: error: {message} """ % (self.progname, self.progname) @@ -249,7 +256,14 @@ class TestDistRepo(utils.CliTestCase): self.assert_help( handle_dist_repo, """Usage: %s dist-repo [options] [ ...] -(Specify the --help global option for a list of other help options) + +In normal mode, dist-repo behaves like any other koji task. +Sometimes you want to limit running distRepo tasks per tag to only +one. For such behaviour admin (with 'tag' permission) needs to +modify given tag's extra field 'distrepo.cancel_others' to True' +via 'koji edit-tag -x distrepo.cancel_others=True' + +(Specify the --help option for a list of other options) Options: -h, --help show this help message and exit diff --git a/tests/test_hub/test_dist_repo.py b/tests/test_hub/test_dist_repo.py index 0d25bc0..58130f8 100644 --- a/tests/test_hub/test_dist_repo.py +++ b/tests/test_hub/test_dist_repo.py @@ -85,15 +85,17 @@ class TestDistRepoInit(unittest.TestCase): class TestDistRepo(unittest.TestCase): + @mock.patch('kojihub.get_tag') @mock.patch('kojihub.dist_repo_init') @mock.patch('kojihub.make_task') - def test_DistRepo(self, make_task, dist_repo_init): + def test_DistRepo(self, make_task, dist_repo_init, get_tag): session = kojihub.context.session = mock.MagicMock() # It seems MagicMock will not automatically handle attributes that # start with "assert" session.assertPerm = mock.MagicMock() dist_repo_init.return_value = ('repo_id', 'event_id') make_task.return_value = 'task_id' + get_tag.return_value = {'extra': {}} exports = kojihub.RootExports() ret = exports.distRepo('tag', 'keys') From decb1badd2f16871b59c4a631a5af6c965ca20a6 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Dec 20 2019 10:13:29 +0000 Subject: [PATCH 2/2] use getBuildConfig --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 31d4a76..454b992 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -11437,8 +11437,8 @@ class RootExports(object): repo_id, event_id = dist_repo_init(tag, keys, task_opts) task_opts['event'] = event_id # cancel potentially running distRepos - tinfo = get_tag(tag, strict=True) - if tinfo['extra'].get('distrepo.cancel_others', False): + build_config = self.getBuildConfig(get_tag(tag, strict=True)) + if build_config['extra'].get('distrepo.cancel_others', False): tasks = self.listTasks(opts={ 'state': [koji.TASK_STATES['FREE'], koji.TASK_STATES['OPEN'], diff --git a/tests/test_hub/test_dist_repo.py b/tests/test_hub/test_dist_repo.py index 58130f8..b9f9ca2 100644 --- a/tests/test_hub/test_dist_repo.py +++ b/tests/test_hub/test_dist_repo.py @@ -95,14 +95,18 @@ class TestDistRepo(unittest.TestCase): session.assertPerm = mock.MagicMock() dist_repo_init.return_value = ('repo_id', 'event_id') make_task.return_value = 'task_id' - get_tag.return_value = {'extra': {}} - + get_tag.return_value = {'id': 1, 'extra': {}} exports = kojihub.RootExports() + exports.getBuildConfig = mock.MagicMock() + exports.getBuildConfig.return_value = {'extra': {}} + ret = exports.distRepo('tag', 'keys') + session.assertPerm.assert_called_once_with('dist-repo') dist_repo_init.assert_called_once() make_task.assert_called_once() self.assertEquals(ret, make_task.return_value) + exports.getBuildConfig.assert_called_once_with(get_tag.return_value) class TestDistRepoMove(unittest.TestCase): @@ -208,6 +212,7 @@ class TestDistRepoMove(unittest.TestCase): def test_distRepoMove(self): + kojihub.context.session = mock.MagicMock() exports = kojihub.HostExports() exports.distRepoMove(self.rinfo['id'], self.uploadpath, self.arch) # check result