murdock: implement numeric priorities

[kaspar030]

This PR drops the fasttrack queue and turns the "regular" queue into a priority queue.
It adds configurable priority modifiers (per-branch, per-label) that get applied to queued or (re-)labeled jobs.

Previously, it was possible to "fasttrack" jobs, moving them ahead of "regular" jobs, essentially supporting two priorities.
With this PR, the job priority becomes numeric. This is limiting.

As an actual use-case, RIOT will move to bors for builds that should get merged, but non-bors quickbuilds for PRs.
The bors builds are expected to take >2hrs, whereas the quickbuilds are below 5min.
PR builds should be prioritized over the long bors builds.
Up to this point, two priorities work fine (fasttrack all PR builds).

Now it would be great if some PR builds could be set to a lower priority (for contributors that want to use CI for build testing without hampering workflow), or even higher priority (for important backports or CI fixes).
Also, it would be great if e.g., adding the "CI: high priority") label would update the job's queue position.
And, if "can_fast_ci_build.py" determines a subset build, the priority can be dynamically adapted.

So this PR implements the following changes:

• replace the fasttrack and regular queues with one priority queue
• adds a "priority" field to MurdockJob
• add an __lt() implementation to MurdockJob (making it sort by priority)
• make MurdockJob calculate it's priority based on priorities: repo-level (.murdock.yml) configuration
• adds priorities: .murdock.yml configuration
• implements requeuing on label change (if the priority changes)

[codecov]

Codecov Report

Base: 96.35% // Head: 95.14% // Decreases project coverage by -1.21% ⚠️

Coverage data is based on head (2a879d7) compared to base (3ad1537).
Patch coverage: 68.18% of modified lines in pull request are covered.

❗ Current head 2a879d7 differs from pull request most recent head 429950f. Consider uploading reports for the commit 429950f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #18      +/-   ##
==========================================
- Coverage   96.35%   95.14%   -1.21%     
==========================================
  Files          26       21       -5     
  Lines        3097     2596     -501     
  Branches      362      311      -51     
==========================================
- Hits         2984     2470     -514     
- Misses         81       88       +7     
- Partials       32       38       +6     

Impacted Files	Coverage Δ
murdock/job.py	86.86% <50.00%> (-4.24%)	⬇️
murdock/murdock.py	84.42% <72.72%> (ø)
murdock/config.py	100.00% <100.00%> (ø)
murdock/models.py	100.00% <100.00%> (ø)
murdock/task.py	74.64% <0.00%> (-1.03%)	⬇️
murdock/job_containers.py	99.02% <0.00%> (-0.98%)	⬇️
murdock/github.py	94.91% <0.00%> (-0.37%)	⬇️
murdock/tests/conftest.py	100.00% <0.00%> (ø)
... and 16 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[kaspar030]

Ok, I've added a priority field to MurdockJob.
I've left the logic on how to set the priority value a bit open, there's now a calculate_priority() static_method on MurdockJob, which currently only takes the fasttrack label into account.

We can discuss whether fasttracking is needed at all, but maybe it's nice to have it as "build that next" hammer.

@aabadie how does the added JobModel field interact with the existing database?

[kaspar030]

As for how to set priorities, I'd suggest (as discussed before with @aabadie) configuration in .murdock.yml:

priorities:
  labels:
    "CI: high priority": 10
    "CI: low priority": -10
  branches:
    trying: -1
    staging: -1

[kaspar030]

adding the priority configuration was straight-forward.
I might need some help with adding tests, I don't understand the test suite (yet).

[aabadie]

all these function parameters are already instance attributes. Thus there's no need for calculate_priority to be a staticmethod. A regular instance method, private, should be enough def _priority(self): and use self.config, self.pr, self.ref and self.fastracked inside.

[aabadie]

Note that an @property method could also be used but it would recompute the priority each time it's accessed. Maybe that's better I don't know

[kaspar030]

I wasn't sure, can safely use a class method within __init__()?
(Also it needs to be public so it can react to label changes.)

[aabadie]

can safely use a class method within init()?

yes, if the method is accessing already defined/initialized attributes.

it needs to be public so it can react to label changes

then I think a property method is more appropriate.

[kaspar030]

a property method would make e.g., "job.priority" actually call the method?

[kaspar030]

assuming the pr field of a job is a reference to updated PR data.
if that's not the case, it's easier to have this staticmethod and supply all needed data at the call site.

[aabadie]

How do you requeue ? In the current state, a new MurdockJob instance is created and the previous matching ones are marked as cancelled.

I also thought that the display order must also be taken into account. This is something that is in another data structure, a simple list. So a sort on priority must be added there.

https://github.com/murdock-ng/murdock/blob/e62397d29e8ffe1ded2bbb2dd0a7f7a0c49ee853/murdock/murdock.py#L561-569

[kaspar030]

I would add re-queueing here:

murdock/murdock/murdock.py

Line 478 in e62397d

if action == "labeled":

(if the job is queued, re-calc its priority and compare to the queued one. if it has changed, don't "return" but fall through to "schedule_job()", which would cancel the old job and create a new, sorted into the queue by priority.)

[aabadie]

I'm a bit lost. What is the use case exactly ?

[kaspar030]

The PR blew up from just replacing two unprioritized queues with one prioritized, to making full use of that change. :( I'll update the description.

[aabadie]

I might need some help with adding tests, I don't understand the test suite (yet).

The test suite is becoming huge I admit. Maybe start by adding a test function in test_job.py to check the job.priority attribute depending on certain cases. Using pytest.mark.parametrize will be useful I think.

To run the tests, just install tox (pip install tox) and run tox. You can give a parameter which the name of the test to skip the others, like tox murdock/tests/test_job.py to only the tests that related to the MurdockJob class.

[kaspar030]

• changed the "priorities" settings to always have sane defaults (eases accessing them instead of checking for optionals)
• implemented priority label changes (thanks @aabadie for the discussion, much simpler now than my first iteration)
• changed get_queued_jobs() to also sort by job.priority

[aabadie]

This needs a rebase!

[aabadie]

Tiny typo:

Suggested change

	# skip re-queing if there's no queued job
	# skip re-queuing if there's no queued job

[aabadie]

This also needs another rebase, sorry. There are also some failures reported by the CI