| kparal | |
| tflink |
Fixes https://taskotron.fedoraproject.org/taskmaster/builders/x86_64/builds/63826 that was caused by http://koji.fedoraproject.org/koji/buildinfo?buildID=632072 ('el7' not in release).
Also it removes blacklisted_releases, as @kparal suggested, we don't have to add a new tag everytime there's new epel release (although it's not often).
Tests pass.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
I'm hesitant to hardcode blacklisting of everything epel but I suppose that running on epel isn't exactly on the roadmap for the near future and we can always back out the hardcoded changes.
If my rawhide concern is invalid, you have my ack.
| jobtriggers/koji_build_msg.py | ||
|---|---|---|
| 39–40 | Since you removed the blacklist feature, this message might be slightly confusing. What about
? | |
| jobtriggers/koji_tag_msg.py | ||
| 56–57 | What do you think about checking for f\d\d.* regular expressions? Should be a bit safer. This will exclude rawhide koji tag, is that intended? I don't see it in the unit tests, so I don't know. But we do trigger some checks for rawhide builds at the moment (rpmlint), I believe. | |
| 57–59 | Ditto. | |
| jobtriggers/koji_tag_msg.py | ||
|---|---|---|
| 56–57 | Yes, but rawhide builds (rpmlint) are triggered by koji_build consumer. This is koji_tag consumer. Even so, doesn't rawhide koji tag start with "f23"? | |
| jobtriggers/koji_tag_msg.py | ||
|---|---|---|
| 56–57 | Ah, in that case it's fine. If you look here: | |
| Path | Packages | |||
|---|---|---|---|---|
| M | conf/trigger.cfg.example (1 line) | |||
| M | jobtriggers/config.py (2 lines) | |||
| M | jobtriggers/koji_build_msg.py (4 lines) | |||
| M | jobtriggers/koji_tag_msg.py (7 lines) | |||
| M | jobtriggers/utils.py (3 lines) | |||
| M | testing/test_koji_build_trigger.py (2 lines) | |||
| M | testing/test_koji_tag_trigger.py (15 lines) | |||
| D | M | testing/test_utils.py (20 lines) |
| Commit | Tree | Parents | Author | Summary | Date |
|---|---|---|---|---|---|
| 153cd839c74e | d27448d74b05 | 8f38164a25ef | Martin Krizek | Use regex to be a bit safer about koji tag recognition | May 6 2015, 2:58 PM |
| 8f38164a25ef | dcde309e2b65 | 4c22c4b41d28 | Martin Krizek | Blacklist anything that is not Fedora (Show More…) | Apr 30 2015, 11:35 AM |
| 1 | [buildbot] | 1 | [buildbot] | ||
|---|---|---|---|---|---|
| 2 | url = http://localhost:8080/buildbot/change_hook | 2 | url = http://localhost:8080/buildbot/change_hook | ||
| 3 | 3 | | |||
| 4 | [trigger] | 4 | [trigger] | ||
| 5 | koji_build_completed_tasks = rpmlint | 5 | koji_build_completed_tasks = rpmlint | ||
| 6 | koji_tag_changed_tasks = upgradepath,depcheck | 6 | koji_tag_changed_tasks = upgradepath,depcheck | ||
| 7 | compose_completed_tasks = | 7 | compose_completed_tasks = | ||
| 8 | valid_arches = x86_64 | 8 | valid_arches = x86_64 | ||
| 9 | koji_url = http://koji.fedoraproject.org/kojihub | 9 | koji_url = http://koji.fedoraproject.org/kojihub | ||
| 10 | execdb_server = http://localhost:5000 | 10 | execdb_server = http://localhost:5000 | ||
| 11 | type = StreamTrigger | 11 | type = StreamTrigger | ||
| 12 | blacklist_releases = el5,el6,el7 | | |||
| 13 | job_logging = False | 12 | job_logging = False | ||
| 14 | joblog_file = /var/log/taskotron-trigger/jobs.csv | 13 | joblog_file = /var/log/taskotron-trigger/jobs.csv | ||
| 15 | fuse_delay = 1000 ; seconds | 14 | fuse_delay = 1000 ; seconds | ||
| 1 | import ConfigParser | 1 | import ConfigParser | ||
|---|---|---|---|---|---|
| 2 | import io | 2 | import io | ||
| 3 | 3 | | |||
| 4 | defaults = """ | 4 | defaults = """ | ||
| 5 | [buildbot] | 5 | [buildbot] | ||
| 6 | url = http://localhost:8080/buildbot/change_hook | 6 | url = http://localhost:8080/buildbot/change_hook | ||
| 7 | 7 | | |||
| 8 | [trigger] | 8 | [trigger] | ||
| 9 | koji_build_completed_tasks = rpmlint | 9 | koji_build_completed_tasks = rpmlint | ||
| 10 | koji_tag_changed_tasks = upgradepath,depcheck | 10 | koji_tag_changed_tasks = upgradepath,depcheck | ||
| 11 | compose_completed_tasks = | 11 | compose_completed_tasks = | ||
| 12 | valid_arches = x86_64 | 12 | valid_arches = x86_64 | ||
| 13 | koji_url = http://koji.fedoraproject.org/kojihub | 13 | koji_url = http://koji.fedoraproject.org/kojihub | ||
| 14 | execdb_server = http://localhost:5000 | 14 | execdb_server = http://localhost:5000 | ||
| 15 | type = StreamTrigger | 15 | type = StreamTrigger | ||
| 16 | blacklist_releases = el5,el6,el7 | | |||
| 17 | job_logging = False | 16 | job_logging = False | ||
| 18 | joblog_file = /var/log/taskotron-trigger/jobs.cvs | 17 | joblog_file = /var/log/taskotron-trigger/jobs.cvs | ||
| 19 | fuse_delay = 1000 ; seconds | 18 | fuse_delay = 1000 ; seconds | ||
| 20 | """ | 19 | """ | ||
| 21 | 20 | | |||
| 22 | config = ConfigParser.ConfigParser() | 21 | config = ConfigParser.ConfigParser() | ||
| 23 | config.readfp(io.BytesIO(defaults)) | 22 | config.readfp(io.BytesIO(defaults)) | ||
| 24 | config.read(['/etc/taskotron/trigger.cfg', './conf/trigger.cfg']) | 23 | config.read(['/etc/taskotron/trigger.cfg', './conf/trigger.cfg']) | ||
| 25 | 24 | | |||
| 26 | 25 | | |||
| 27 | buildbot_url = config.get('buildbot', 'url') | 26 | buildbot_url = config.get('buildbot', 'url') | ||
| 28 | 27 | | |||
| 29 | koji_build_completed_tasks = [e.strip() for e in config.get('trigger', 'koji_build_completed_tasks').split(',') if e] | 28 | koji_build_completed_tasks = [e.strip() for e in config.get('trigger', 'koji_build_completed_tasks').split(',') if e] | ||
| 30 | koji_tag_changed_tasks = [e.strip() for e in config.get('trigger', 'koji_tag_changed_tasks').split(',') if e] | 29 | koji_tag_changed_tasks = [e.strip() for e in config.get('trigger', 'koji_tag_changed_tasks').split(',') if e] | ||
| 31 | compose_completed_tasks = [e.strip() for e in config.get('trigger', 'compose_completed_tasks').split(',') if e] | 30 | compose_completed_tasks = [e.strip() for e in config.get('trigger', 'compose_completed_tasks').split(',') if e] | ||
| 32 | valid_arches = [e.strip() for e in config.get('trigger', 'valid_arches').split(',') if e] | 31 | valid_arches = [e.strip() for e in config.get('trigger', 'valid_arches').split(',') if e] | ||
| 33 | koji_url = config.get('trigger', 'koji_url') | 32 | koji_url = config.get('trigger', 'koji_url') | ||
| 34 | execdb_server = config.get('trigger', 'execdb_server') | 33 | execdb_server = config.get('trigger', 'execdb_server') | ||
| 35 | trigger_type = config.get('trigger', 'type') | 34 | trigger_type = config.get('trigger', 'type') | ||
| 36 | blacklist_releases = [e.strip() for e in config.get('trigger', 'blacklist_releases').split(',') if e] | | |||
| 37 | job_logging = True if config.get('trigger', 'job_logging') == 'True' else False | 35 | job_logging = True if config.get('trigger', 'job_logging') == 'True' else False | ||
| 38 | joblog_file = config.get('trigger', 'joblog_file') | 36 | joblog_file = config.get('trigger', 'joblog_file') | ||
| 39 | fuse_delay = int(config.get('trigger', 'fuse_delay')) # seconds | 37 | fuse_delay = int(config.get('trigger', 'fuse_delay')) # seconds | ||
| Show All 30 Lines | 18 | def consume(self, message): | |||
|---|---|---|---|---|---|
| 31 | if not instance == 'primary': | 31 | if not instance == 'primary': | ||
| 32 | self.log.debug('rejecting message not from primary koji instance (%s from %s)' % (envr, instance)) | 32 | self.log.debug('rejecting message not from primary koji instance (%s from %s)' % (envr, instance)) | ||
| 33 | return | 33 | return | ||
| 34 | 34 | | |||
| 35 | if state != koji.BUILD_STATES['COMPLETE']: | 35 | if state != koji.BUILD_STATES['COMPLETE']: | ||
| 36 | self.log.debug('rejecting message for not being a build complete message (%s, status %s)' % (envr, state)) | 36 | self.log.debug('rejecting message for not being a build complete message (%s, status %s)' % (envr, state)) | ||
| 37 | return | 37 | return | ||
| 38 | 38 | | |||
| 39 | if utils.blacklist_releases(release): | 39 | if 'fc' not in release: | ||
| 40 | self.log.debug('rejecting message from blacklisted release (%s)' % envr) | 40 | self.log.debug('rejecting message for not containing "fc" in release (%s)' % envr) | ||
| 41 | return | 41 | return | ||
| 42 | 42 | | |||
| 43 | self.log.debug('should be scheduling job for %s' % envr) | 43 | self.log.debug('should be scheduling job for %s' % envr) | ||
| 44 | 44 | | |||
| 45 | self.trigger_tasks(envr, ITEM_TYPE, config.koji_build_completed_tasks, config.valid_arches) | 45 | self.trigger_tasks(envr, ITEM_TYPE, config.koji_build_completed_tasks, config.valid_arches) | ||
| 1 | import re | ||||
|---|---|---|---|---|---|
| 1 | from copy import deepcopy | 2 | from copy import deepcopy | ||
| 2 | 3 | | |||
| 3 | import fedmsg | 4 | import fedmsg | ||
| 4 | import fedmsg.encoding | 5 | import fedmsg.encoding | ||
| 5 | import moksha.hub.reactor | 6 | import moksha.hub.reactor | ||
| 6 | 7 | | |||
| 7 | from twisted.internet.task import LoopingCall | 8 | from twisted.internet.task import LoopingCall | ||
| 8 | 9 | | |||
| Show All 29 Lines | 38 | finally: | |||
| 38 | self.queued_tags = set() | 39 | self.queued_tags = set() | ||
| 39 | else: | 40 | else: | ||
| 40 | self.log.debug("Woke up, but there were no messages.") | 41 | self.log.debug("Woke up, but there were no messages.") | ||
| 41 | 42 | | |||
| 42 | def consume(self, message): | 43 | def consume(self, message): | ||
| 43 | usable_message = fedmsg.encoding.loads(message.body) | 44 | usable_message = fedmsg.encoding.loads(message.body) | ||
| 44 | 45 | | |||
| 45 | koji_tag = usable_message['msg']['tag'] | 46 | koji_tag = usable_message['msg']['tag'] | ||
| 46 | release = usable_message['msg']['release'] | | |||
| 47 | instance = usable_message['msg']['instance'] | 47 | instance = usable_message['msg']['instance'] | ||
| 48 | 48 | | |||
| 49 | if not instance == 'primary': | 49 | if not instance == 'primary': | ||
| 50 | self.log.debug('rejecting message not from primary koji instance (%s from %s)' % (koji_tag, instance)) | 50 | self.log.debug('rejecting message not from primary koji instance (%s from %s)' % (koji_tag, instance)) | ||
| 51 | return | 51 | return | ||
| 52 | 52 | | |||
| 53 | if not koji_tag.endswith('pending'): | 53 | if not koji_tag.endswith('pending'): | ||
| 54 | self.log.debug('rejecting message for not being a pending tag (%s)' % koji_tag) | 54 | self.log.debug('rejecting message for not being a pending tag (%s)' % koji_tag) | ||
| 55 | return | 55 | return | ||
| 56 | 56 | | |||
| 57 | if utils.blacklist_releases(release): | 57 | m = re.match(r'^f\d\d-.*', koji_tag) | ||
What do you think about checking for f\d\d.* regular expressions? Should be a bit safer. This will exclude rawhide koji tag, is that intended? I don't see it in the unit tests, so I don't know. But we do trigger some checks for rawhide builds at the moment (rpmlint), I believe. | |||||
Ah, in that case it's fine. If you look here: | |||||
| 58 | self.log.debug('rejecting message from blacklisted release (%s)' % koji_tag) | 58 | if m is None: | ||
| 59 | self.log.debug('rejecting message for not starting with fedora version (e.g. f22) (%s)' % koji_tag) | ||||
| 59 | return | 60 | return | ||
| 60 | 61 | | |||
| 61 | if not len(self.queued_tags): | 62 | if not len(self.queued_tags): | ||
| 62 | self.log.info('setting fuse (%d seconds) for koji_tag %s', config.fuse_delay, koji_tag) | 63 | self.log.info('setting fuse (%d seconds) for koji_tag %s', config.fuse_delay, koji_tag) | ||
| 63 | elif koji_tag not in self.queued_tags: | 64 | elif koji_tag not in self.queued_tags: | ||
| 64 | self.log.info('adding koji_tag %s to the fuse queue', koji_tag) | 65 | self.log.info('adding koji_tag %s to the fuse queue', koji_tag) | ||
| 65 | 66 | | |||
| 66 | self.queued_tags.add(koji_tag) | 67 | self.queued_tags.add(koji_tag) | ||
| 1 | import csv | 1 | import csv | ||
|---|---|---|---|---|---|
| 2 | import datetime | 2 | import datetime | ||
| 3 | 3 | | |||
| 4 | from . import config | 4 | from . import config | ||
| 5 | 5 | | |||
| 6 | def log_job(logname, item, item_type, task, arch): | 6 | def log_job(logname, item, item_type, task, arch): | ||
| 7 | now = datetime.datetime.now() | 7 | now = datetime.datetime.now() | ||
| 8 | 8 | | |||
| 9 | with open(logname, 'a') as csvfile: | 9 | with open(logname, 'a') as csvfile: | ||
| 10 | writer = csv.writer(csvfile, delimiter=';') | 10 | writer = csv.writer(csvfile, delimiter=';') | ||
| 11 | writer.writerow([now, item, item_type, task, arch]) | 11 | writer.writerow([now, item, item_type, task, arch]) | ||
| 12 | | ||||
| 13 | def blacklist_releases(release): | | |||
| 14 | return any(b in release for b in config.blacklist_releases) | | |||
| Show All 15 Lines | 12 | def prepare(self, monkeypatch): | |||
|---|---|---|---|---|---|
| 16 | self.ref_version = '4.1.1' | 16 | self.ref_version = '4.1.1' | ||
| 17 | self.ref_release = '3.fc18' | 17 | self.ref_release = '3.fc18' | ||
| 18 | self.ref_nvr = '%s-%s-%s' % (self.ref_name, self.ref_version, self.ref_release) | 18 | self.ref_nvr = '%s-%s-%s' % (self.ref_name, self.ref_version, self.ref_release) | ||
| 19 | 19 | | |||
| 20 | self._create_msg(self.ref_instance, self.ref_new, self.ref_name, self.ref_release, self.ref_version) | 20 | self._create_msg(self.ref_instance, self.ref_new, self.ref_name, self.ref_release, self.ref_version) | ||
| 21 | 21 | | |||
| 22 | self.ref_tasks = ['koji_build_task1', 'koji_build_task2'] | 22 | self.ref_tasks = ['koji_build_task1', 'koji_build_task2'] | ||
| 23 | self.ref_validarches = ['i386', 'x86_64'] | 23 | self.ref_validarches = ['i386', 'x86_64'] | ||
| 24 | self.ref_blacklist_releases= ['el5', 'el6'] | | |||
| 25 | 24 | | |||
| 26 | stub_hub = Bunch(config=Bunch(get=0)) | 25 | stub_hub = Bunch(config=Bunch(get=0)) | ||
| 27 | 26 | | |||
| 28 | self.helper = koji_build_msg.KojiBuildCompletedJobTrigger(stub_hub) | 27 | self.helper = koji_build_msg.KojiBuildCompletedJobTrigger(stub_hub) | ||
| 29 | self.helper.trigger = Dingus() | 28 | self.helper.trigger = Dingus() | ||
| 30 | 29 | | |||
| 31 | koji_build_msg.config.koji_build_completed_tasks = self.ref_tasks | 30 | koji_build_msg.config.koji_build_completed_tasks = self.ref_tasks | ||
| 32 | koji_build_msg.config.valid_arches = self.ref_validarches | 31 | koji_build_msg.config.valid_arches = self.ref_validarches | ||
| 33 | koji_build_msg.config.blacklist_releases = self.ref_blacklist_releases | | |||
| 34 | koji_build_msg.config.job_logging = False | 32 | koji_build_msg.config.job_logging = False | ||
| 35 | 33 | | |||
| 36 | def _create_msg(self, ref_instance, ref_new, ref_name, ref_release, ref_version): | 34 | def _create_msg(self, ref_instance, ref_new, ref_name, ref_release, ref_version): | ||
| 37 | self.ref_message = Bunch(body='{"i": 1,\ | 35 | self.ref_message = Bunch(body='{"i": 1,\ | ||
| 38 | "msg": {"build_id": 221146,\ | 36 | "msg": {"build_id": 221146,\ | ||
| 39 | "instance": "%s",\ | 37 | "instance": "%s",\ | ||
| 40 | "new": %d,\ | 38 | "new": %d,\ | ||
| 41 | "name": "%s",\ | 39 | "name": "%s",\ | ||
| ▲ Show 20 Lines • Show All 48 Lines • Show Last 20 Lines | |||||
| Show All 12 Lines | 11 | def prepare(self, monkeypatch): | |||
|---|---|---|---|---|---|
| 13 | self.ref_name = 'stage' | 13 | self.ref_name = 'stage' | ||
| 14 | self.ref_version = '4.1.1' | 14 | self.ref_version = '4.1.1' | ||
| 15 | self.ref_release = '3.fc18' | 15 | self.ref_release = '3.fc18' | ||
| 16 | self.ref_tag = 'f18-updates-pending' | 16 | self.ref_tag = 'f18-updates-pending' | ||
| 17 | self.ref_nvr = '%s-%s-%s' % (self.ref_name, self.ref_version, self.ref_release) | 17 | self.ref_nvr = '%s-%s-%s' % (self.ref_name, self.ref_version, self.ref_release) | ||
| 18 | 18 | | |||
| 19 | self.ref_tasks = ['depcheck', 'upgradepath'] | 19 | self.ref_tasks = ['depcheck', 'upgradepath'] | ||
| 20 | self.ref_validarches = ['i386', 'x86_64'] | 20 | self.ref_validarches = ['i386', 'x86_64'] | ||
| 21 | self.ref_blacklist_releases= ['el5', 'el6'] | | |||
| 22 | 21 | | |||
| 23 | self._create_msg(self.ref_instance, self.ref_name, self.ref_release, self.ref_tag, self.ref_version) | 22 | self._create_msg(self.ref_instance, self.ref_name, self.ref_release, self.ref_tag, self.ref_version) | ||
| 24 | 23 | | |||
| 25 | stub_hub = Bunch(config=Bunch(get=0)) | 24 | stub_hub = Bunch(config=Bunch(get=0)) | ||
| 26 | self.helper = koji_tag_msg.KojiTagChangedJobTrigger(stub_hub) | 25 | self.helper = koji_tag_msg.KojiTagChangedJobTrigger(stub_hub) | ||
| 27 | 26 | | |||
| 28 | self.helper.trigger = Dingus() | 27 | self.helper.trigger = Dingus() | ||
| 29 | 28 | | |||
| ▲ Show 20 Lines • Show All 66 Lines • ▼ Show 20 Line(s) | 87 | def test_delayed_consume_upgradepath(self): | |||
| 96 | assert len(trigger_calls) == 2 | 95 | assert len(trigger_calls) == 2 | ||
| 97 | 96 | | |||
| 98 | assert trigger_calls[0][1] == (ref_tag, koji_tag_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[0]) | 97 | assert trigger_calls[0][1] == (ref_tag, koji_tag_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[0]) | ||
| 99 | assert trigger_calls[1][1] == (ref_tag, koji_tag_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[1]) | 98 | assert trigger_calls[1][1] == (ref_tag, koji_tag_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[1]) | ||
| 100 | 99 | | |||
| 101 | assert self.helper.queued_tags == set() | 100 | assert self.helper.queued_tags == set() | ||
| 102 | 101 | | |||
| 103 | def test_consume_blacklisted_release(self): | 102 | def test_consume_blacklisted_release(self): | ||
| 104 | ref_release = '3.el5.1' | 103 | ref_release = '3.el7.1' | ||
| 105 | self._create_msg(self.ref_instance, self.ref_name, ref_release, self.ref_tag, self.ref_version) | 104 | ref_tag = 'epel7-testing-pending' | ||
| 105 | self._create_msg(self.ref_instance, self.ref_name, ref_release, ref_tag, self.ref_version) | ||||
| 106 | 106 | | |||
| 107 | self.helper.consume(self.ref_message) | 107 | self.helper.consume(self.ref_message) | ||
| 108 | 108 | | |||
| 109 | assert self.helper.queued_tags == set([]) | 109 | assert self.helper.queued_tags == set([]) | ||
| 110 | | ||||
| 111 | def test_consume_allowed_release(self): | ||||
| 112 | ref_release = '3.fc20.1' | ||||
| 113 | ref_tag = 'f20-testing-pending' | ||||
| 114 | self._create_msg(self.ref_instance, self.ref_name, ref_release, ref_tag, self.ref_version) | ||||
| 115 | | ||||
| 116 | self.helper.consume(self.ref_message) | ||||
| 117 | | ||||
| 118 | assert self.helper.queued_tags == set([ref_tag]) | ||||
Since you removed the blacklist feature, this message might be slightly confusing. What about
?