Configurable Trigger
ClosedPublic

Authored by jskladan on Aug 9 2016, 10:56 AM.

Details

Summary

This started as simple bike-shedding to make more sense in naming (so
everything is not named "Trigger"), but it went further :D

The main change here is, what I call "configurable trigger" - at the moment,
every time we want to add support for even the most basic new task (like the
package-specific task for docker), changes are needed in the trigger's source
code.

These changes add a concept of a "rules engine", that decides what tasks to
schedule based on data extacted from the received FedMessage, and a set of rules.

The rules-engine is YAML, in a format like this::

- do:
  - {tasks: [depcheck, upgradepath]}
  when: {message_type: KojiTagChanged}
- do:
  - {tasks: [dockerautotest]}
  when: {message_type: KojiBuildCompleted, name: docker}
- do:
  - {tasks: [abicheck]}
  when:
    message_type: KojiBuildCompleted
    name:
      $in: ${critpath_pkgs}
      $nin: ['docker'] # critpath excludes

The rules are split in two parts when and do, the when clause is
a mongo query that will get evaluated against the dataset provided by
the FedMsg consumer. For example, the KojiBuildCompletedJobTrigger now
publishes this (values are fake, to make it more descriptive::

message_data = {
        "_msg": {...snipped...},
        "message_type": "KojiBuildCompleted",
        "item": "docker-1.9.1-6.git6ec29ef.fc23",
        "item_type": "koji_build",
        "name": "docker",
        "version": "1.9.1-6.git6ec29ef",
        "release": "fc23",
        "critpath_pkgs": [..., "docker", ...]
        "distgit_branch": "f23",
        }

So taking the rules, and the data, going from the top:

  1. First rule's when is False as message_type is not KojiTagChanged
  2. Second rule is True because both the message_type and name in the when clause match the data
  3. Third rule does _not_ schedule anything, because even though docker is in critpath_pkgs, it also is part of the critpath excludes list, and so the rule is ignored

The when clauses are in fact mongo queries https://docs.mongodb.com/manual/tutorial/query-documents/,
evaluated using a Python library that implements it for querying Python objects.

The rules engine then takes the do clauses of the 'passed' rules, and
produces arguments for the trigger_tasks() calls. By default, item, and
item_type are taken from the message_data, arches is set to
config.valid_arches, and then all the key/values from the do's body are
added on top. This means, that we can have a task, that for example forces
an architecture different than default::

- do:
  - {tasks: [awesome_arm_check], arches: [armhfp]}
  when: {message_type: KojiBuildCompleted}

The do clause can have multiple items in it, so something like this is
possible::

- do:
  - {tasks: [rpmlint]}
  - {tasks: [awesome_arm_check], arches: [armhfp]}
  when: {message_type: KojiBuildCompleted}

Triggering rpmlint on the default architectures, and awesome_arm_check
on armhfp for each package built in Koji.

This means, that when we want to trigger new (somewhat specific) tasks,
no changes are needed in the trigger's code, but just in the configuration,
to alter the rules. If we come to the point where more functionality is
needed, than it obviously calls for changes in the underlying code, in order
to add more key/values to the data provided by the Fedmsg consumer, or
adding more general functionality overall.

A good example of this is the dist-git style tasks problem. To solve it
I have added a new command ($discover)to the do section, that crawls the
provided git repo/branch, and schedules jobs for all runtask.yml's found::

- do:
  - {$discover: {repo: 'http://pkgs.fedoraproject.org/git/rpms-checks/${name}.git', branch: '${distgit_branch}'}}
  when: {message_type: KojiBuildCompleted}

In the bigger picture, this 'rules engine' functionality can be used to
make (for example) a web interface, that allows creating/altering the rules,
instead of changing the config file (the rules can as easily be taken from
a database, as from the config file), or even to provide a per-user triggering
capability - we could add a piece of code, that checks (selected) users'
Fedorapeople profile for a file, that contains rules in this format, and
then could simply run the engine on those rules+data from Fedmsg to decide
whether the user-defined tasks should be run.

Test Plan

unittests changed/added

Diff Detail

Repository
rTRGR taskotron-trigger
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jskladan retitled this revision from to Configurable Trigger.Aug 9 2016, 10:56 AM
jskladan updated this object.
jskladan edited the test plan for this revision. (Show Details)
jskladan added a reviewer: taskotron-trigger.
jskladan updated this revision to Diff 2459.Aug 9 2016, 11:07 AM
  • Decouple trigger rules from the source code
jskladan updated this revision to Diff 2460.Aug 9 2016, 11:11 AM
  • Decouple trigger rules from the source code

I don't have any big problems with this.

I assume that the lint errors would be fixed before the change was actually committed?

jskladan updated this revision to Diff 2470.Aug 11 2016, 11:44 AM
  • Decouple trigger rules from the source code
  • Smarter string.Template with known mongo keywords
kparal added a subscriber: kparal.Aug 11 2016, 12:00 PM

TLDR. The concept sounds good, though. Kudos for new unit tests.

jobtriggers/mongoquery_string_template.py
2

This file should probably get the same header as mongoquery.py?

jskladan updated this revision to Diff 2531.Sep 6 2016, 6:48 AM
  • Decouple trigger rules from the source code
  • Smarter string.Template with known mongo keywords
  • Addded rpmgrill to trigger rules
  • some config cleaning
  • Fix regexp in trigger rules
  • Add local configs to gitignore
  • Updated Dockerfile
tflink accepted this revision.Sep 12 2016, 8:16 PM
tflink added a reviewer: tflink.

Overall, looks good to me. I like the new syntax and the fact that we don't have to patch every time a rule changes :)

A few things with documentation and headers. The "making lists generic" bit should be an enhancement request, not blocking this change.

Are you planning to write up docs for this so that we can point people at stuff other than the intro to this diff?

Dockerfile
1

I'm not seeing this used anywhere. I assume that it's meant to run the tests in relative isolation but I'd like to see some usage mentioned in the README or docs

conf/trigger_rules.yml.example
13

While I like the syntax, I'm wondering if it would be better to make this a bit more generic. I'd be surprised if we didn't run into any "run task on everything but X" requests in the not-too-distant future.

That being said, this is probably better left todo as an enhancement - the method could be generic if we re-jiggered the config and added some conventions.

jobtriggers/mongoquery_string_template.py
3

Actually, isn't this custom code which extends mongoquery so that the templates work like we want them to?

If so, I think that would need the same header as the rest of the code in the project

This revision is now accepted and ready to land.Sep 12 2016, 8:16 PM
jskladan added inline comments.Sep 16 2016, 10:36 AM
jobtriggers/mongoquery_string_template.py
3

Well, it is the string.template code, with some changes. I don't really know how to do it properly. What I did:

I took code from Python's std library (string.template), and modified it, to do what I want. Python 2.7 is under PSF license https://www.python.org/download/releases/2.7/license/ (GPL compatible), our code is GPL.

What should I do here?

Too long, went through it quickly but overall looks good to me. There are some TODOs but we can get that done later. I'd like to have this deployed on dev ASAP.

jobtriggers/compose_complete_msg.py
3–4

Is this consumer relevant? We haven't started using it and I guess with pungi4 it's not uptodate anymore? We might as well get rid of it. Unless I am missing something.

Closed by commit rTRGR14cf50770140: Configurable Trigger (authored by Josef Skladanka <jskladan@redhat.com>). · Explain WhyOct 4 2016, 1:10 PM
This revision was automatically updated to reflect the committed changes.