From 35956c5ac7285ac248d4cac37c7453b724233704 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 17 Sep 2012 22:13:36 +0200 Subject: [PATCH 1/8] Switch to smart HTTP mode. Gerrit actually only speaks smart http, but we were assuming dumb HTTP (which is provided in OpenStack's configuration). Change-Id: Id299f53a6be6fc1670edf1da4b3353115bf1e31e Reviewed-on: https://review.openstack.org/13146 Reviewed-by: Clark Boylan Reviewed-by: Monty Taylor Approved: James E. Blair Tested-by: Jenkins --- tests/test_scheduler.py | 9 +++++++-- zuul/trigger/gerrit.py | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index cacf4a91e8..41848f6d34 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -580,11 +580,16 @@ class FakeURLOpener(object): res = urlparse.urlparse(self.url) path = res.path project = '/'.join(path.split('/')[2:-2]) - ret = '' + ret = '001e# service=git-upload-pack\n' + ret += ('000000a31270149696713ba7e06f1beb760f20d359c4abed HEAD\x00' + 'multi_ack thin-pack side-band side-band-64k ofs-delta ' + 'shallow no-progress include-tag multi_ack_detailed no-done\n') path = os.path.join(UPSTREAM_ROOT, project) repo = git.Repo(path) for ref in repo.refs: - ret += ref.object.hexsha + '\t' + ref.path + '\n' + r = ref.object.hexsha + ' ' + ref.path + '\n' + ret += '%04x%s' % (len(r) + 4, r) + ret += '0000' return ret diff --git a/zuul/trigger/gerrit.py b/zuul/trigger/gerrit.py index 1c4b74c774..5f0064e478 100644 --- a/zuul/trigger/gerrit.py +++ b/zuul/trigger/gerrit.py @@ -117,12 +117,41 @@ class Gerrit(object): message, action) def _getInfoRefs(self, project): - url = "https://%s/p/%s/info/refs" % (self.server, project) + url = "https://%s/p/%s/info/refs?service=git-upload-pack" % ( + self.server, project) data = urllib2.urlopen(url).read() ret = {} - for line in data.split('\n'): - if not line: + read_headers = False + read_advertisement = False + if data[4] != '#': + raise Exception("Gerrit repository does not support " + "git-upload-pack") + i = 0 + while i < len(data): + if len(data) - i < 4: + raise Exception("Invalid length in info/refs") + plen = int(data[i:i + 4], 16) + i += 4 + # It's the length of the packet, including the 4 bytes of the + # length itself, unless it's null, in which case the length is + # not included. + if plen > 0: + plen -= 4 + if len(data) - i < plen: + raise Exception("Invalid data in info/refs") + line = data[i:i + plen] + i += plen + if not read_headers: + if plen == 0: + read_headers = True continue + if not read_advertisement: + read_advertisement = True + continue + if plen == 0: + # The terminating null + continue + line = line.strip() revision, ref = line.split() ret[ref] = revision return ret From fec5c7ac183fb8aff2ff64700f67a5112b9b146c Mon Sep 17 00:00:00 2001 From: Antoine Musso Date: Sat, 22 Sep 2012 12:32:37 +0200 Subject: [PATCH 2/8] update github URLs in documentation Some puppet modules have been renamed in openstack-ci-puppet: * `jenkins_slave` puppet module has been integrated in `jenkins` module * `openstack-ci-config` has been renamed `openstack_project` Change-Id: Ie4a35022301c156f74e1fdef859c87319efb60da Reviewed-on: https://review.openstack.org/13519 Approved: Monty Taylor Reviewed-by: Monty Taylor Tested-by: Jenkins --- doc/source/launchers.rst | 2 +- doc/source/zuul.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/source/launchers.rst b/doc/source/launchers.rst index d936d4ef98..ff818be5d7 100644 --- a/doc/source/launchers.rst +++ b/doc/source/launchers.rst @@ -86,7 +86,7 @@ And multiple changes are separated by a carat ("^"). E.g.:: The OpenStack project uses the following script to update the repository in a workspace and merge appropriate changes: - https://github.com/openstack/openstack-ci-puppet/blob/master/modules/jenkins_slave/files/slave_scripts/gerrit-git-prep.sh + https://github.com/openstack/openstack-ci-puppet/blob/master/modules/jenkins/files/slave_scripts/gerrit-git-prep.sh Gerrit events that do not include a change (e.g., ref-updated events which are emitted after a git ref is updated (i.e., a commit is merged diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst index 8d7bbeff99..b1df2013dd 100644 --- a/doc/source/zuul.rst +++ b/doc/source/zuul.rst @@ -344,7 +344,7 @@ succeeds. In the above example, project-unittest, project-pep8, and project-pyflakes are only executed if project-merge succeeds. This can help avoid running unnecessary jobs. -.. seealso:: The OpenStack Zuul configuration for a comprehensive example: https://github.com/openstack/openstack-ci-puppet/blob/master/modules/openstack-ci-config/files/zuul/layout.yaml +.. seealso:: The OpenStack Zuul configuration for a comprehensive example: https://github.com/openstack/openstack-ci-puppet/blob/master/modules/openstack_project/files/zuul/layout.yaml logging.conf From a969e820300d811b73811041d3b14272338c7bb9 Mon Sep 17 00:00:00 2001 From: Antoine Musso Date: Sat, 22 Sep 2012 21:37:02 +0200 Subject: [PATCH 3/8] update pip dependencies zuul also need the 'lockfile' and 'python-daemon' python packages. Change-Id: I16ed4fba9705796f83aa980687de8b9d262eb715 Reviewed-on: https://review.openstack.org/13523 Reviewed-by: Monty Taylor Reviewed-by: James E. Blair Approved: James E. Blair Tested-by: Jenkins --- tools/pip-requires | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/pip-requires b/tools/pip-requires index aa1debe4c1..703734f799 100644 --- a/tools/pip-requires +++ b/tools/pip-requires @@ -4,3 +4,5 @@ Paste webob paramiko GitPython>=0.3.2.RC1 +lockfile +python-daemon From 80925f513125caf627758a5363bebb9782b30c57 Mon Sep 17 00:00:00 2001 From: Antoine Musso Date: Sat, 22 Sep 2012 21:37:45 +0200 Subject: [PATCH 4/8] fix back compat issues with python modules When using python-daemon 1.6, the interface has changed in an uncompatible way. Clark pointed me to Gerritbot which solves that issue with a simple try / catch block implemented with: https://github.com/openstack-ci/gerritbot/commit/b2be72e69d08c774989b2cc8c39748ed06ffb1bf So this patch is merely a copy/paste from David "davido" Ostrovsky with a small workaround for pyflakes issue #13 (we have to prented we are using the variable holding the module). Change-Id: Iffdf7fca067734fa9c09b5bddfb13f122e6251a7 Reviewed-on: https://review.openstack.org/13524 Reviewed-by: Monty Taylor Reviewed-by: James E. Blair Approved: James E. Blair Tested-by: Jenkins --- zuul-server | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/zuul-server b/zuul-server index 0f89cd2672..3e06e06652 100755 --- a/zuul-server +++ b/zuul-server @@ -16,7 +16,15 @@ import argparse import ConfigParser import daemon -import daemon.pidlockfile + +try: + import daemon.pidlockfile as pid_file_module + pid_file_module # workaround for pyflakes issue #13 +except: + # as of python-daemon 1.6 it doesn't bundle pidlockfile anymore + # instead it depends on lockfile-0.9.1 which uses pidfile. + import daemon.pidfile as pid_file_module + import logging.config import os import signal @@ -120,7 +128,7 @@ if __name__ == '__main__': pid_fn = os.path.expanduser(server.config.get('zuul', 'pidfile')) else: pid_fn = '/var/run/zuul/zuul.pid' - pid = daemon.pidlockfile.TimeoutPIDLockFile(pid_fn, 10) + pid = pid_file_module.TimeoutPIDLockFile(pid_fn, 10) if server.args.nodaemon: server.setup_logging() From 40b9907cd30a1ed121fe448a1c4dfa40c84bbea6 Mon Sep 17 00:00:00 2001 From: Antoine Musso Date: Sat, 22 Sep 2012 23:12:53 +0200 Subject: [PATCH 5/8] `git_dir` parameter in zuul.conf sample Change-Id: I95ef3dde919f4ab5ce471295fb7cc1568e42402a Reviewed-on: https://review.openstack.org/13526 Reviewed-by: Monty Taylor Reviewed-by: Clark Boylan Approved: James E. Blair Reviewed-by: James E. Blair Tested-by: Jenkins --- etc/zuul.conf-sample | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/zuul.conf-sample b/etc/zuul.conf-sample index 41f238684c..6c91c8fd01 100644 --- a/etc/zuul.conf-sample +++ b/etc/zuul.conf-sample @@ -13,3 +13,4 @@ layout_config=/etc/zuul/layout.yaml log_config=/etc/zuul/logging.yaml pidfile=/var/run/zuul/zuul.pid state_dir=/var/lib/zuul +git_dir=/var/lib/zuul/git From 6ad5d5b1afd342bbdab6186d8c8c6677b721a459 Mon Sep 17 00:00:00 2001 From: Antoine Musso Date: Mon, 24 Sep 2012 13:25:28 +0200 Subject: [PATCH 6/8] handle KeyboardInterrupt When running zuul in non-daemon mode (with -d), it is often useful to abort it using Control + C. Change-Id: I385a41625633f3120b95c07283bef015824e5853 Reviewed-on: https://review.openstack.org/13571 Reviewed-by: Monty Taylor Reviewed-by: James E. Blair Approved: James E. Blair Tested-by: Jenkins --- zuul-server | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/zuul-server b/zuul-server index 3e06e06652..d57c564071 100755 --- a/zuul-server +++ b/zuul-server @@ -101,8 +101,11 @@ class Server(object): signal.signal(signal.SIGHUP, self.reconfigure_handler) signal.signal(signal.SIGUSR1, self.exit_handler) while True: - signal.pause() - + try: + signal.pause() + except KeyboardInterrupt: + print "Ctrl + C: asking scheduler to exit nicely...\n" + self.exit_handler( signal.SIGINT, None ) if __name__ == '__main__': server = Server() From 5d55607e5becb89b2055e8f5730d288952444d31 Mon Sep 17 00:00:00 2001 From: Zhongyue Luo Date: Fri, 21 Sep 2012 02:00:47 +0900 Subject: [PATCH 7/8] Fix pep8 E127 violations Updated pep8 version requirement to 1.3.3 Fixed E127 errors All ignores are to be removed in the next sequence of patches Change-Id: Ia9e922b8873686a0f905f2548cc43d534ee1c912 Reviewed-on: https://review.openstack.org/13642 Reviewed-by: James E. Blair Reviewed-by: Zhongyue Luo Approved: James E. Blair Tested-by: Jenkins --- .mailmap | 4 ++++ AUTHORS | 2 +- tests/test_scheduler.py | 40 ++++++++++++++++++++++++---------------- tox.ini | 4 ++-- zuul/launcher/jenkins.py | 2 +- zuul/model.py | 2 +- 6 files changed, 33 insertions(+), 21 deletions(-) create mode 100644 .mailmap diff --git a/.mailmap b/.mailmap new file mode 100644 index 0000000000..18221d4058 --- /dev/null +++ b/.mailmap @@ -0,0 +1,4 @@ +# Format is: +# +# +Zhongyue Luo diff --git a/AUTHORS b/AUTHORS index 61b563367e..09d24373a1 100644 --- a/AUTHORS +++ b/AUTHORS @@ -1,3 +1,3 @@ James E. Blair Clark Boylan -Zhongyue Luo +Zhongyue Luo diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 41848f6d34..3493c985fd 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -362,14 +362,17 @@ class FakeGerrit(object): class FakeJenkinsEvent(object): def __init__(self, name, number, parameters, phase, status=None): - data = {'build': - {'full_url': 'https://server/job/%s/%s/' % (name, number), - 'number': number, - 'parameters': parameters, - 'phase': phase, - 'url': 'job/%s/%s/' % (name, number)}, - 'name': name, - 'url': 'job/%s/' % name} + data = { + 'build': { + 'full_url': 'https://server/job/%s/%s/' % (name, number), + 'number': number, + 'parameters': parameters, + 'phase': phase, + 'url': 'job/%s/%s/' % (name, number), + }, + 'name': name, + 'url': 'job/%s/' % name, + } if status: data['build']['status'] = status self.body = json.dumps(data) @@ -815,7 +818,7 @@ class testScheduler(unittest.TestCase): def test_independent_queues(self): "Test that changes end up in the right queues" self.fake_jenkins.hold_jobs_in_build = True - A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') B = self.fake_gerrit.addFakeChange('org/project1', 'master', 'B') C = self.fake_gerrit.addFakeChange('org/project2', 'master', 'C') A.addApproval('CRVW', 2) @@ -1122,13 +1125,18 @@ class testScheduler(unittest.TestCase): def test_post(self): "Test that post jobs run" - e = {"type": "ref-updated", - "submitter": {"name": "User Name"}, - "refUpdate": {"oldRev": - "90f173846e3af9154517b88543ffbd1691f31366", - "newRev": - "d479a0bfcb34da57a31adb2a595c0cf687812543", - "refName": "master", "project": "org/project"}} + e = { + "type": "ref-updated", + "submitter": { + "name": "User Name", + }, + "refUpdate": { + "oldRev": "90f173846e3af9154517b88543ffbd1691f31366", + "newRev": "d479a0bfcb34da57a31adb2a595c0cf687812543", + "refName": "master", + "project": "org/project", + } + } self.fake_gerrit.addEvent(e) self.waitUntilSettled() diff --git a/tox.ini b/tox.ini index d4a0074529..34075cb706 100644 --- a/tox.ini +++ b/tox.ini @@ -10,8 +10,8 @@ commands = nosetests {posargs} downloadcache = ~/cache/pip [testenv:pep8] -deps = pep8==1.2 -commands = pep8 --repeat --show-source --exclude=.venv,.tox,dist,doc,build . +deps = pep8==1.3.3 +commands = pep8 --ignore=E122,E125,E126,E128 --repeat --show-source --exclude=.venv,.tox,dist,doc,build . [testenv:cover] setenv = NOSE_WITH_COVERAGE=1 diff --git a/zuul/launcher/jenkins.py b/zuul/launcher/jenkins.py index 6fd4ca4109..be659aed33 100644 --- a/zuul/launcher/jenkins.py +++ b/zuul/launcher/jenkins.py @@ -155,7 +155,7 @@ class ExtendedJenkins(jenkins.Jenkins): # Jenkins returns a 302 from this URL, unless Referer is not set, # then you get a 404. request = urllib2.Request(self.server + CANCEL_QUEUE % locals(), - headers={'Referer': self.server}) + headers={'Referer': self.server}) self.jenkins_open(request) def get_build_info(self, name, number): diff --git a/zuul/model.py b/zuul/model.py index 7deef068da..96859c48f2 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -565,7 +565,7 @@ class TriggerEvent(object): class EventFilter(object): def __init__(self, types=[], branches=[], refs=[], approvals={}, - comment_filters=[]): + comment_filters=[]): self._types = types self._branches = branches self._refs = refs From aa85ebfe92759b88692ba82eb7460ce78ae330ae Mon Sep 17 00:00:00 2001 From: Zhongyue Luo Date: Fri, 21 Sep 2012 16:38:33 +0800 Subject: [PATCH 8/8] Remove pep8 ignores Fixed all pep8 errors. E125 is ignored because of false alarms. Change-Id: I4da60409e0095c0896230cd01bda548ed2e3f741 Reviewed-on: https://review.openstack.org/13740 Reviewed-by: James E. Blair Approved: James E. Blair Tested-by: Jenkins --- tests/test_scheduler.py | 49 ++++++++++++++++++++++------------------ tox.ini | 2 +- zuul/launcher/jenkins.py | 5 ++-- zuul/lib/gerrit.py | 4 ++-- zuul/model.py | 6 ++--- zuul/scheduler.py | 8 +++---- 6 files changed, 39 insertions(+), 35 deletions(-) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 3493c985fd..12f2ea27ff 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -206,12 +206,12 @@ class FakeChange(object): def getPatchsetCreatedEvent(self, patchset): event = {"type": "patchset-created", "change": {"project": self.project, - "branch": self.branch, - "id": "I5459869c07352a31bfb1e7a8cac379cabfcb25af", - "number": str(self.number), - "subject": self.subject, - "owner": {"name": "User Name"}, - "url": "https://hostname/3"}, + "branch": self.branch, + "id": "I5459869c07352a31bfb1e7a8cac379cabfcb25af", + "number": str(self.number), + "subject": self.subject, + "owner": {"name": "User Name"}, + "url": "https://hostname/3"}, "patchSet": self.patchsets[patchset - 1], "uploader": {"name": "User Name"}} return event @@ -305,8 +305,8 @@ class FakeChange(object): return json.loads(json.dumps(self.data)) def setMerged(self): - if (self.depends_on_change - and self.depends_on_change.data['status'] != 'MERGED'): + if (self.depends_on_change and + self.depends_on_change.data['status'] != 'MERGED'): return if self.fail_merge: return @@ -425,29 +425,34 @@ class FakeJenkinsJob(threading.Thread): if self.canceled: self.jenkins.all_jobs.remove(self) return - self.callback.jenkins_endpoint(FakeJenkinsEvent( - self.name, self.number, self.parameters, - 'STARTED')) + self.callback.jenkins_endpoint(FakeJenkinsEvent(self.name, + self.number, + self.parameters, + 'STARTED')) if self.jenkins.hold_jobs_in_build: self._wait() self.log.debug("Job %s continuing" % (self.parameters['UUID'])) result = 'SUCCESS' - if ('ZUUL_REF' in self.parameters) and self.jenkins.fakeShouldFailTest( - self.name, - self.parameters['ZUUL_REF']): + if (('ZUUL_REF' in self.parameters) and + self.jenkins.fakeShouldFailTest(self.name, + self.parameters['ZUUL_REF'])): result = 'FAILURE' if self.aborted: result = 'ABORTED' self.jenkins.fakeAddHistory(name=self.name, number=self.number, result=result) - self.callback.jenkins_endpoint(FakeJenkinsEvent( - self.name, self.number, self.parameters, - 'COMPLETED', result)) - self.callback.jenkins_endpoint(FakeJenkinsEvent( - self.name, self.number, self.parameters, - 'FINISHED', result)) + self.callback.jenkins_endpoint(FakeJenkinsEvent(self.name, + self.number, + self.parameters, + 'COMPLETED', + result)) + self.callback.jenkins_endpoint(FakeJenkinsEvent(self.name, + self.number, + self.parameters, + 'FINISHED', + result)) self.jenkins.all_jobs.remove(self) @@ -482,8 +487,8 @@ class FakeJenkins(object): self.log.debug("releasing job %s" % (job.parameters['UUID'])) job.release() else: - self.log.debug("not releasing job %s" % ( - job.parameters['UUID'])) + self.log.debug("not releasing job %s" % + (job.parameters['UUID'])) self.log.debug("done releasing jobs %s (%s)" % (regex, len(self.all_jobs))) diff --git a/tox.ini b/tox.ini index 34075cb706..334cc94ec1 100644 --- a/tox.ini +++ b/tox.ini @@ -11,7 +11,7 @@ downloadcache = ~/cache/pip [testenv:pep8] deps = pep8==1.3.3 -commands = pep8 --ignore=E122,E125,E126,E128 --repeat --show-source --exclude=.venv,.tox,dist,doc,build . +commands = pep8 --ignore=E125 --repeat --show-source --exclude=.venv,.tox,dist,doc,build . [testenv:cover] setenv = NOSE_WITH_COVERAGE=1 diff --git a/zuul/launcher/jenkins.py b/zuul/launcher/jenkins.py index be659aed33..c53e6f64ff 100644 --- a/zuul/launcher/jenkins.py +++ b/zuul/launcher/jenkins.py @@ -227,8 +227,9 @@ class Jenkins(object): params['ZUUL_BRANCH'] = change.branch params['GERRIT_CHANGES'] = changes_str params['ZUUL_CHANGES'] = changes_str - params['ZUUL_REF'] = 'refs/zuul/%s/%s' % (change.branch, - change.current_build_set.ref) + params['ZUUL_REF'] = ('refs/zuul/%s/%s' % + (change.branch, + change.current_build_set.ref)) zuul_changes = ' '.join(['%s,%s' % (c.number, c.patchset) for c in dependent_changes + [change]]) diff --git a/zuul/lib/gerrit.py b/zuul/lib/gerrit.py index ca626f0f8a..0cefa22e92 100644 --- a/zuul/lib/gerrit.py +++ b/zuul/lib/gerrit.py @@ -139,8 +139,8 @@ class Gerrit(object): data = json.loads(lines[0]) if not data: return False - self.log.debug("Received data from Gerrit query: \n%s" % ( - pprint.pformat(data))) + self.log.debug("Received data from Gerrit query: \n%s" % + (pprint.pformat(data))) return data def _open(self): diff --git a/zuul/model.py b/zuul/model.py index 96859c48f2..c6841c57c9 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -486,8 +486,7 @@ class Change(Changeish): return '' % (id(self), self._id()) def equals(self, other): - if (self.number == other.number and - self.patchset == other.patchset): + if self.number == other.number and self.patchset == other.patchset: return True return False @@ -508,8 +507,7 @@ class Ref(Changeish): return self.newrev def equals(self, other): - if (self.ref == other.ref and - self.newrev == other.newrev): + if self.ref == other.ref and self.newrev == other.newrev: return True return False diff --git a/zuul/scheduler.py b/zuul/scheduler.py index 329d0c42ec..66c18e8258 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -98,8 +98,8 @@ class Scheduler(threading.Thread): branches=toList(trigger.get('branch')), refs=toList(trigger.get('ref')), approvals=approvals, - comment_filters=toList( - trigger.get('comment_filter'))) + comment_filters= + toList(trigger.get('comment_filter'))) manager.event_filters.append(f) for config_job in data['jobs']: @@ -533,8 +533,8 @@ class BasePipelineManager(object): if hasattr(change, 'refspec') and not ref: change.current_build_set.setConfiguration() ref = change.current_build_set.getRef() - merged = self.sched.merger.mergeChanges([change], ref, - mode=model.MERGE_IF_NECESSARY) + mode = model.MERGE_IF_NECESSARY + merged = self.sched.merger.mergeChanges([change], ref, mode=mode) if not merged: self.log.info("Unable to merge change %s" % change) self.pipeline.setUnableToMerge(change)