Differential D40

Make trigger configuration more flexible
ClosedPublic
Actions

Authored by mkrizek on Apr 2 2014, 2:59 PM.

Details

Reviewers
kparal
ralph
tflink
Maniphest Tasks
T115: Make trigger configuration more flexible
Commits
rTRGRd97c2a33ad2d: Make trigger configuration more flexible
rTRGR8b29118bf5d2: Make trigger configuration more flexible
Summary

Fixes T115

Test Plan

run fedmsg-hub according to instructions in readme.rst

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
kparal added inline comments.Apr 3 2014, 9:21 AM
jobtriggers/config.py
20

I wonder, maybe it would be better to consolidate our configuration directories?
For libtaskotron:
/etc/taskotron/(lib)taskotron.yaml
For taskotron-trigger:
/etc/taskotron/trigger.cfg

mkrizek added inline comments.Apr 3 2014, 10:28 AM
jobtriggers/config.py
20

Which rpm would own /etc/taskotron/ directory though? taskotron-trigger or libtaskotron?

kparal added inline comments.Apr 3 2014, 12:07 PM
jobtriggers/config.py
20

Both, at least that's the correct approach, I believe.

ralph added a comment.Apr 3 2014, 1:35 PM

Are there no tests for this?

tflink added a comment.Apr 3 2014, 2:42 PM

Are there no tests for this?

Can you be more specific on what you're looking for - tests in general or a specific part of the config? There are some tests in the code: https://bitbucket.org/fedoraqa/taskotron-trigger/src/6835b51d797441d864fd2d06a536e62e0acff5a2/testing/?at=master

I wonder, maybe it would be better to consolidate our configuration directories?

That's something that I'm hitting with most of the components as well - resultsdb, resultsdb_frontend each have their own config directory. So does fake_fedorainfra, but that's less directly related.

While it would be nice to see config files consolidated into the same directory, would it be worth the potential coupling and dependencies between the different components?

I'm OK with the code as is for now but I'm holding off on accepting the changes in case @kparal or @ralph have stronger objections.

jobtriggers/config.py
20

Can you do that with rpm without having file conflicts?

kparal added inline comments.Apr 3 2014, 3:08 PM
jobtriggers/config.py
20

I'm almost sure that file conflicts do not apply for directories. Quite the opposite, all packages are expected to own all directories they use (except for basic directories inside filesystem package), and it happens quite often that multiple packages own the same directory. But I need to go AFK now, so I can verify tomorrow.

ralph accepted this revision.Apr 3 2014, 4:18 PM
mkrizek added inline comments.Apr 4 2014, 8:28 AM
jobtriggers/config.py
20

According to [1], owning a directory by both packages is indeed the correct approach (note: we must ensure that the ownership and permissions match in both packages). I'll send the change shortly...

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function

mkrizek updated this revision.Apr 4 2014, 8:40 AM

Put conf file into /etc/taskotron

kparal added a comment.Apr 4 2014, 1:36 PM

Apart from that proposal below, the code seems fine, even though I'm not very familiar with taskotron-trigger.

jobtriggers/config.py
20

Maybe it would be better to use the same name for the config file regardless of location? (Therefore ./conf/trigger.cfg).

mkrizek updated this revision.Apr 7 2014, 11:29 AM

Make naming of config files consistent

tflink accepted this revision.Apr 8 2014, 2:45 PM

Looks good to me

mkrizek closed this revision.Apr 8 2014, 2:55 PM

Closed by commit rTRGRd97c2a33ad2d (authored by @mkrizek).

Revision Contents

Diff 125

View Options

.gitignore

1config.ini 1​config.ini
2.*.swp 2​.*.swp
3*.pyc 3​*.pyc
4*.pyo 4​*.pyo
5env_trigger/ 5​env_trigger/
6jobtriggers.egg-info/ 6​jobtriggers.egg-info/
7​conf/trigger.cfg
View Options

conf/trigger.cfg.example

  • This file was added.
1​[buildbot]
2​username = username
3​password = password
4​url = http://localhost/buildbot
5​
6​[trigger]
7​koji_tasks = rpmlint
8​bodhi_tasks = examplebodhi
9​valid_arches = i386,i686,x86_64,noarch
10​koji_url = http://koji.fedoraproject.org/kojihub
View Options

jobtriggers/bodhi_msg.py

1import logging 1​import logging
2import fedmsg 2​import fedmsg
3import fedmsg.encoding 3​import fedmsg.encoding
4import fedmsg.consumers 4​import fedmsg.consumers
5 5​
6from . import triggers, utils, settings 6​from . import triggers, utils, config
7 7​
8 8​
9UNIT_TYPE = 'bodhi_update' 9​UNIT_TYPE = 'bodhi_update'
10 10​
11logging.basicConfig() 11​logging.basicConfig()
12log = logging.getLogger(__name__) 12​log = logging.getLogger(__name__)
13 13​
14 14​
Show All 15 Lines29 if not topic.endswith('stable') and not topic.endswith('testing'):
30 return 30​ return
31 31​
32 updatetitle = usable_message['msg']['update']['title'] 32​ updatetitle = usable_message['msg']['update']['title']
33 builds = usable_message['msg']['update']['builds'] 33​ builds = usable_message['msg']['update']['builds']
34 nvrs = [build['nvr'] for build in builds] 34​ nvrs = [build['nvr'] for build in builds]
35 35​
36 arches = utils.get_arches(nvr=nvrs[0]) 36​ arches = utils.get_arches(nvr=nvrs[0])
37 37​
38 for task in settings.bodhi_tasks: 38​ for task in config.bodhi_tasks:
39 for arch in arches: 39​ for arch in arches:
40 log.info('triggering %s for %s on %s', task, updatetitle, arch) 40​ log.info('triggering %s for %s on %s', task, updatetitle, arch)
41 self.trigger.trigger_job(updatetitle, UNIT_TYPE, task, arch) 41​ self.trigger.trigger_job(updatetitle, UNIT_TYPE, task, arch)
View Options

jobtriggers/config.py

  • This file was added.
1​import ConfigParser
2​import io
3​
4​defaults = """
5​[buildbot]
6​username = username
7​password = password
8​url = http://localhost/buildbot
9​
10​[trigger]
11​koji_tasks = rpmlint
12​bodhi_tasks = examplebodhi
13​valid_arches = i386,i686,x86_64,noarch
14​koji_url = http://koji.fedoraproject.org/kojihub
15​"""
16​
17​config = ConfigParser.ConfigParser()
18​config.readfp(io.BytesIO(defaults))
19​config.read(['/etc/taskotron/trigger.cfg', './conf/trigger.cfg'])
20​
kparalUnsubmitted
Not Done

I wonder, maybe it would be better to consolidate our configuration directories?
For libtaskotron:
/etc/taskotron/(lib)taskotron.yaml
For taskotron-trigger:
/etc/taskotron/trigger.cfg

mkrizekAuthorUnsubmitted
Not Done

Which rpm would own /etc/taskotron/ directory though? taskotron-trigger or libtaskotron?

kparalUnsubmitted
Not Done

Both, at least that's the correct approach, I believe.

tflinkUnsubmitted
Not Done

Can you do that with rpm without having file conflicts?

kparalUnsubmitted
Not Done

I'm almost sure that file conflicts do not apply for directories. Quite the opposite, all packages are expected to own all directories they use (except for basic directories inside filesystem package), and it happens quite often that multiple packages own the same directory. But I need to go AFK now, so I can verify tomorrow.

mkrizekAuthorUnsubmitted
Not Done

According to [1], owning a directory by both packages is indeed the correct approach (note: we must ensure that the ownership and permissions match in both packages). I'll send the change shortly...

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function

kparalUnsubmitted
Not Done

Maybe it would be better to use the same name for the config file regardless of location? (Therefore ./conf/trigger.cfg).

21​
22​buildbot_username = config.get('buildbot', 'username')
23​buildbot_password = config.get('buildbot', 'password')
24​buildbot_url = config.get('buildbot', 'url')
25​
26​koji_tasks = config.get('trigger', 'koji_tasks').split(',')
27​bodhi_tasks = config.get('trigger', 'bodhi_tasks').split(',')
28​valid_arches = config.get('trigger', 'valid_arches').split(',')
29​koji_url = config.get('trigger', 'koji_url')
View Options

jobtriggers/koji_msg.py

1import logging 1​import logging
2 2​
3import fedmsg 3​import fedmsg
4import fedmsg.encoding 4​import fedmsg.encoding
5import fedmsg.consumers 5​import fedmsg.consumers
6 6​
7from . import triggers, utils, settings 7​from . import triggers, utils, config
8 8​
9 9​
10UNIT_TYPE = 'koji_build' 10​UNIT_TYPE = 'koji_build'
11 11​
12logging.basicConfig() 12​logging.basicConfig()
13log = logging.getLogger(__name__) 13​log = logging.getLogger(__name__)
14 14​
15 15​
Show All 18 Lines25 def consume(self, message):
34 buildversion = usable_message['msg']['version'] 34​ buildversion = usable_message['msg']['version']
35 buildrelease = usable_message['msg']['release'] 35​ buildrelease = usable_message['msg']['release']
36 buildid = usable_message['msg']['build_id'] 36​ buildid = usable_message['msg']['build_id']
37 37​
38 envr = "%s-%s-%s" % (buildname, buildversion, buildrelease) 38​ envr = "%s-%s-%s" % (buildname, buildversion, buildrelease)
39 39​
40 arches = utils.get_arches(buildid=buildid) 40​ arches = utils.get_arches(buildid=buildid)
41 41​
42 for task in settings.koji_tasks: 42​ for task in config.koji_tasks:
43 for arch in arches: 43​ for arch in arches:
44 log.info('triggering %s for %s on %s', task, envr, arch) 44​ log.info('triggering %s for %s on %s', task, envr, arch)
45 self.trigger.trigger_job(envr, UNIT_TYPE, task, arch) 45​ self.trigger.trigger_job(envr, UNIT_TYPE, task, arch)
View Options

jobtriggers/settings.py

  • This file was deleted.
This file was completely deleted. Show File Contents
View Options

jobtriggers/triggers.py

1import pprint 1​import pprint
2import requests 2​import requests
3 3​
4from . import settings 4​from . import config
5 5​
6class BaseTrigger(object): 6​class BaseTrigger(object):
7 """ 7​ """
8 Format: 8​ Format:
9 { 9​ {
10 'item': <name of item - envr, update id, koji tag, etc.>, 10​ 'item': <name of item - envr, update id, koji tag, etc.>,
11 'item_type': <koji_build, koji_tag, bodhi_update>, 11​ 'item_type': <koji_build, koji_tag, bodhi_update>,
12 'taskname': <task to be scheduled>, 12​ 'taskname': <task to be scheduled>,
Show All 10 Lines21 def trigger_job(self, item, item_type, taskname, arch):
23 pprint.pprint(job) 23​ pprint.pprint(job)
24 print 24​ print
25 25​
26 26​
27class BuildbotTrigger(BaseTrigger): 27​class BuildbotTrigger(BaseTrigger):
28 def trigger_job(self, item, item_type, taskname, arch): 28​ def trigger_job(self, item, item_type, taskname, arch):
29 s = requests.session() 29​ s = requests.session()
30 30​
31 s.post('%s/login' % settings.buildbot['url'], 31​ s.post('%s/login' % config.buildbot_url,
32 data={'username': settings.buildbot['username'], 32​ data={'username': config.buildbot_username,
33 'passwd': settings.buildbot['password']}) 33​ 'passwd': config.buildbot_password})
34 34​
35 s.post('%s/builders/statictasks/force' % settings.buildbot['url'], 35​ s.post('%s/builders/statictasks/force' % config.buildbot_url,
36 data={'forcescheduler': 'rpmcheck', 36​ data={'forcescheduler': 'rpmcheck',
37 'reason': 'triggered by fedmsg', 37​ 'reason': 'triggered by fedmsg',
38 'item': item, 38​ 'item': item,
39 'item_type': item_type, 39​ 'item_type': item_type,
40 'taskname': taskname, 40​ 'taskname': taskname,
41 'arch': arch}) 41​ 'arch': arch})
42 42​
43 s.get('%s/logout' % settings.buildbot['url']) 43​ s.get('%s/logout' % config.buildbot_url)
View Options

jobtriggers/utils.py

1import koji 1​import koji
2 2​
3from . import settings 3​from . import config
4 4​
5koji_client = koji.ClientSession(settings.koji_url) 5​koji_client = koji.ClientSession(config.koji_url)
6 6​
7def get_arches(buildid=None, nvr=None): 7​def get_arches(buildid=None, nvr=None):
8 if buildid is None: 8​ if buildid is None:
9 info = koji_client.getBuild(nvr) 9​ info = koji_client.getBuild(nvr)
10 buildid = info['id'] 10​ buildid = info['id']
11 11​
12 arches = set([rpm['arch'] for rpm in koji_client.listRPMs(buildid) if rpm['arch'] in settings.valid_arches]) 12​ arches = set([rpm['arch'] for rpm in koji_client.listRPMs(buildid) if rpm['arch'] in config.valid_arches])
13 13​
14 if 'i686' in arches: 14​ if 'i686' in arches:
15 arches.remove('i686') 15​ arches.remove('i686')
16 arches.add('i386') 16​ arches.add('i386')
17 17​
18 return arches 18​ return arches
View Options

taskotron-trigger.spec

Show All 23 Lines
24%build 24​%build
25%{__python} setup.py build 25​%{__python} setup.py build
26 26​
27 27​
28%install 28​%install
29rm -rf $RPM_BUILD_ROOT 29​rm -rf $RPM_BUILD_ROOT
30%{__python} setup.py install -O1 --skip-build --root %{buildroot} 30​%{__python} setup.py install -O1 --skip-build --root %{buildroot}
31 31​
32# configuration file 32​# configuration files
33​mkdir -p %{buildroot}%{_sysconfdir}/%{name}/
34​install conf/trigger.cfg.example %{buildroot}%{_sysconfdir}/taskotron/trigger.cfg
33mkdir -p %{buildroot}%{_sysconfdir}/fedmsg.d/ 35​mkdir -p %{buildroot}%{_sysconfdir}/fedmsg.d/
34install fedmsg.d/taskotron-trigger.py %{buildroot}%{_sysconfdir}/fedmsg.d/taskotron-trigger.py 36​install fedmsg.d/taskotron-trigger.py %{buildroot}%{_sysconfdir}/fedmsg.d/taskotron-trigger.py
35 37​
36 38​
37%files 39​%files
38%doc readme.rst 40​%doc readme.rst
39%{python_sitelib}/* 41​%{python_sitelib}/*
42​%dir %{_sysconfdir}/taskotron
43​%{_sysconfdir}/taskotron/trigger.cfg
40%{_sysconfdir}/fedmsg.d/taskotron-trigger.py* 44​%{_sysconfdir}/fedmsg.d/taskotron-trigger.py*
41 45​
42 46​
43%changelog 47​%changelog
44* Mon Mar 10 2014 Martin Krizek <mkrizek@fedoraproject.org> - 0.0.9-1 48​* Mon Mar 10 2014 Martin Krizek <mkrizek@fedoraproject.org> - 0.0.9-1
45- Initial packaging 49​- Initial packaging
View Options

testing/test_bodhi_trigger.py

Show All 20 Lines20 self.ref_message = Bunch(body='{"msg": {"agent": "lmacken",\
21 "update": {"submitter": "lmacken",\ 21​ "update": {"submitter": "lmacken",\
22 "title": "%s",\ 22​ "title": "%s",\
23 "builds": [{"nvr": "gnome-settings-daemon-3.6.1-1.fc18"}]}},\ 23​ "builds": [{"nvr": "gnome-settings-daemon-3.6.1-1.fc18"}]}},\
24 "topic": "%s"}' % 24​ "topic": "%s"}' %
25 (ref_updatetitle, ref_topic)) 25​ (ref_updatetitle, ref_topic))
26 26​
27 def test_consume(self, monkeypatch): 27​ def test_consume(self, monkeypatch):
28 monkeypatch.setattr(self.helper, 'trigger', Dingus()) 28​ monkeypatch.setattr(self.helper, 'trigger', Dingus())
29 monkeypatch.setattr(bodhi_msg.settings, 'bodhi_tasks', self.ref_tasks) 29​ monkeypatch.setattr(bodhi_msg.config, 'bodhi_tasks', self.ref_tasks)
30 monkeypatch.setattr(bodhi_msg.settings, 'valid_arches', self.ref_validarches) 30​ monkeypatch.setattr(bodhi_msg.config, 'valid_arches', self.ref_validarches)
31 monkeypatch.setattr(bodhi_msg, 'utils', Dingus(get_arches__returns=self.ref_validarches)) 31​ monkeypatch.setattr(bodhi_msg, 'utils', Dingus(get_arches__returns=self.ref_validarches))
32 32​
33 self.helper.consume(self.ref_message) 33​ self.helper.consume(self.ref_message)
34 34​
35 trigger_calls = self.helper.trigger.trigger_job.calls() 35​ trigger_calls = self.helper.trigger.trigger_job.calls()
36 36​
37 assert len(trigger_calls) == 4 37​ assert len(trigger_calls) == 4
38 38​
Show All 16 Lines
View Options

testing/test_koji_trigger.py

Show All 30 Lines24 "msg": {"build_id": 221146,\
31 "version": "%s"},\ 31​ "version": "%s"},\
32 "timestamp": 1359603469.21164,\ 32​ "timestamp": 1359603469.21164,\
33 "topic": "org.fedoraproject.prod.buildsys.tag",\ 33​ "topic": "org.fedoraproject.prod.buildsys.tag",\
34 "username": "apache"}' % 34​ "username": "apache"}' %
35 (ref_name, ref_release, ref_tag, ref_version)) 35​ (ref_name, ref_release, ref_tag, ref_version))
36 36​
37 def test_consume(self, monkeypatch): 37​ def test_consume(self, monkeypatch):
38 monkeypatch.setattr(self.helper, 'trigger', Dingus()) 38​ monkeypatch.setattr(self.helper, 'trigger', Dingus())
39 monkeypatch.setattr(koji_msg.settings, 'koji_tasks', self.ref_tasks) 39​ monkeypatch.setattr(koji_msg.config, 'koji_tasks', self.ref_tasks)
40 monkeypatch.setattr(koji_msg.settings, 'valid_arches', self.ref_validarches) 40​ monkeypatch.setattr(koji_msg.config, 'valid_arches', self.ref_validarches)
41 monkeypatch.setattr(koji_msg, 'utils', Dingus(get_arches__returns=self.ref_validarches)) 41​ monkeypatch.setattr(koji_msg, 'utils', Dingus(get_arches__returns=self.ref_validarches))
42 42​
43 self.helper.consume(self.ref_message) 43​ self.helper.consume(self.ref_message)
44 44​
45 trigger_calls = self.helper.trigger.trigger_job.calls() 45​ trigger_calls = self.helper.trigger.trigger_job.calls()
46 46​
47 assert len(trigger_calls) == 4 47​ assert len(trigger_calls) == 4
48 48​
Show All 15 Lines