From a36cb8baa67dc55044ec758815aa8fdd3379f665 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 23 2021 16:04:43 +0000 Subject: [PATCH 1/5] hub: priority policy Fixes: https://projects.engineering.redhat.com/browse/RHELBLD-4275 --- diff --git a/docs/source/defining_hub_policies.rst b/docs/source/defining_hub_policies.rst index 0d4e0ec..784a4c0 100644 --- a/docs/source/defining_hub_policies.rst +++ b/docs/source/defining_hub_policies.rst @@ -10,6 +10,7 @@ in the system. At present, policy allows you to control: * allowing builds from expired repos * managing the package list for a tag * managing which channel a task goes to +* altering task priority In the future, we expect to add more policy hooks for controlling more aspects of the system. @@ -161,6 +162,28 @@ the following actions: * only valid for child tasks * recommend using the ``is_child_task`` test to be sure +The priority policy is used to alter task's priority. In most cases you should +manage priorities by different channels and builders assigned to them. There is +nevertheless few corner-cases which can benefit from altering task's priority. +Note, that you can easily get to deadlock situation if this is not handled with +caution (lower priority tasks will get assigned only if there is no higher +priority task for given channel). + +Technically it is very similar to ``channel`` policy. Only actions are +different: + +``stay`` + * don't touch the default priority of the task + +``set `` + * set priority to this value + +``increment by `` + * increment default priority + +``decrement by `` + * decrement default priority + Available tests =============== ``true`` diff --git a/hub/kojihub.py b/hub/kojihub.py index 921e8c9..c0c119a 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -593,6 +593,31 @@ def make_task(method, arglist, **opts): logger.error("Invalid result from channel policy: %s", ruleset.last_rule()) raise koji.GenericError("invalid channel policy") + ruleset = context.policy.get('priority') + result = ruleset.apply(policy_data) + if result is None: + logger.warning('Priority policy returned no result, using default value: %s' % opts['priority']) + else: + try: + parts = result.split() + if parts[0] == 'stay': + # dont' change priority + pass + elif parts[0] == 'set': + # fixed value + opts['priority'] = int(parts[1]) + elif parts[0] == 'increment' and parts[1] == 'by': + opts['priority'] += int(parts[2]) + elif parts[0] == 'decrement' and parts[1] == 'by': + opts['priority'] += int(parts[2]) + else: + logger.error("Invalid result from priority policy: %s", ruleset.last_rule()) + raise koji.GenericError("invalid priority policy") + except IndexError: + logger.error("Invalid result from priority policy: %s", ruleset.last_rule()) + raise koji.GenericError("invalid priority policy") + + # encode xmlrpc request opts['request'] = koji.xmlrpcplus.dumps(tuple(arglist), methodname=method) opts['state'] = koji.TASK_STATES['FREE'] diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index 5578a65..25ad98a 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -543,6 +543,9 @@ _default_policies = { 'volume': ''' all :: DEFAULT ''', + 'priority': ''' + all :: stay + ''', } From d41d65cf34e8a5984c5063460c4985d4e0a7d157 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 23 2021 16:04:43 +0000 Subject: [PATCH 2/5] fix typo --- diff --git a/hub/kojihub.py b/hub/kojihub.py index c0c119a..c9a3778 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -609,7 +609,7 @@ def make_task(method, arglist, **opts): elif parts[0] == 'increment' and parts[1] == 'by': opts['priority'] += int(parts[2]) elif parts[0] == 'decrement' and parts[1] == 'by': - opts['priority'] += int(parts[2]) + opts['priority'] -= int(parts[2]) else: logger.error("Invalid result from priority policy: %s", ruleset.last_rule()) raise koji.GenericError("invalid priority policy") From 57b48c207a980323d8e429ad508054d05a4fc2ba Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Feb 23 2021 16:04:44 +0000 Subject: [PATCH 3/5] alter priority policy result syntax, catch ValueError --- diff --git a/docs/source/defining_hub_policies.rst b/docs/source/defining_hub_policies.rst index 784a4c0..054cde5 100644 --- a/docs/source/defining_hub_policies.rst +++ b/docs/source/defining_hub_policies.rst @@ -178,10 +178,10 @@ different: ``set `` * set priority to this value -``increment by `` +``adjust +`` * increment default priority -``decrement by `` +``adjust -`` * decrement default priority Available tests diff --git a/hub/kojihub.py b/hub/kojihub.py index c9a3778..0cc5d1a 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -606,14 +606,13 @@ def make_task(method, arglist, **opts): elif parts[0] == 'set': # fixed value opts['priority'] = int(parts[1]) - elif parts[0] == 'increment' and parts[1] == 'by': - opts['priority'] += int(parts[2]) - elif parts[0] == 'decrement' and parts[1] == 'by': - opts['priority'] -= int(parts[2]) + elif parts[0] == 'adjust': + # note: int() will accept our preferred "+1" representation for increments + opts['priority'] += int(parts[1]) else: logger.error("Invalid result from priority policy: %s", ruleset.last_rule()) raise koji.GenericError("invalid priority policy") - except IndexError: + except (IndexError, ValueError): logger.error("Invalid result from priority policy: %s", ruleset.last_rule()) raise koji.GenericError("invalid priority policy") From 4234bf953373113fca7b8aec34c73ccb38890878 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Feb 23 2021 16:04:44 +0000 Subject: [PATCH 4/5] appease flake8 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 0cc5d1a..345c3b8 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -596,7 +596,8 @@ def make_task(method, arglist, **opts): ruleset = context.policy.get('priority') result = ruleset.apply(policy_data) if result is None: - logger.warning('Priority policy returned no result, using default value: %s' % opts['priority']) + logger.warning('Priority policy returned no result, using default value: %s' + % opts['priority']) else: try: parts = result.split() @@ -616,7 +617,6 @@ def make_task(method, arglist, **opts): logger.error("Invalid result from priority policy: %s", ruleset.last_rule()) raise koji.GenericError("invalid priority policy") - # encode xmlrpc request opts['request'] = koji.xmlrpcplus.dumps(tuple(arglist), methodname=method) opts['state'] = koji.TASK_STATES['FREE'] From 385607d1a351044866ba238a2e5fbbe4bfc310e2 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Feb 23 2021 16:04:44 +0000 Subject: [PATCH 5/5] doc: expanding doc on priority policy --- diff --git a/docs/source/defining_hub_policies.rst b/docs/source/defining_hub_policies.rst index 054cde5..d0ccc0f 100644 --- a/docs/source/defining_hub_policies.rst +++ b/docs/source/defining_hub_policies.rst @@ -146,7 +146,7 @@ Actions Most of the policies are simply allow/deny policies. They have two possible actions: ``allow`` or ``deny``. -The channel policy is used to determine the channel for a task. It supports +The **channel** policy is used to determine the channel for a task. It supports the following actions: ``use `` @@ -162,14 +162,23 @@ the following actions: * only valid for child tasks * recommend using the ``is_child_task`` test to be sure -The priority policy is used to alter task's priority. In most cases you should -manage priorities by different channels and builders assigned to them. There is -nevertheless few corner-cases which can benefit from altering task's priority. +The **priority** policy is used to alter task's priority. In most cases you +should manage priorities by different channels and builders assigned to them. +There is nevertheless few corner-cases which can benefit from altering task's +priority. + Note, that you can easily get to deadlock situation if this is not handled with caution (lower priority tasks will get assigned only if there is no higher priority task for given channel). -Technically it is very similar to ``channel`` policy. Only actions are +.. note:: + For example OSBS use this mechanism to propagate higher priority tasks to + its plugin. Deadlock problem is here mitigated by limiting policy to + ``buildContainer`` tasks only. These tasks are consumed only by dedicated + builders/channel, so they will not take priority over other types of tasks + (e.g. ``newRepo`` or ``tagBuild`` tasks which could be blocked otherwise. + +Technically it is very similar to **channel** policy. Only actions are different: ``stay``