diff --git a/NEWS.rst b/NEWS.rst index f6500f06a7..db269a45d5 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -37,3 +37,8 @@ Since 1.3.0: zuul.pipeline.{pipeline-name}.job.{job-name}.{result} * Job names in statsd keys now have the '_' character substituted for the '.' character. + +* The layout.yaml structure has changed to introduce configurable + reporters. This requires restructuring the start/success/failure + actions to include a dictionary of reporters and their parameters. + See reporters in the docs and layout.yaml-sample. diff --git a/doc/source/index.rst b/doc/source/index.rst index 039cffa75e..5a0c7b956e 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -21,6 +21,7 @@ Contents: gating triggers launchers + reporters zuul Indices and tables diff --git a/doc/source/reporters.rst b/doc/source/reporters.rst new file mode 100644 index 0000000000..d64d4f76fc --- /dev/null +++ b/doc/source/reporters.rst @@ -0,0 +1,32 @@ +:title: Reporters + +Reporters +========= + +Zuul can communicate results and progress back to configurable +protocols. For example, after succeeding in a build a pipeline can be +configured to post a positive review back to gerrit. + +There are three stages when a report can be handled. That is on: +Start, Success or Failure. Each stage can have multiple reports. +For example, you can set verified on gerrit and send an email. + +Gerrit +------ + +Zuul works with standard versions of Gerrit by invoking the +``gerrit`` command over an SSH connection. It reports back to +Gerrit using SSH. + +The dictionary passed to the gerrit reporter is used for ``gerrit +review`` arguments, with the boolean value of ``true`` simply +indicating that the argument should be present without following it +with a value. For example, ``verified: 1`` becomes ``gerrit review +--verified 1`` and ``submit: true`` becomes ``gerrit review +--submit``. + +Gerrit Configuration +~~~~~~~~~~~~~~~~~~~~ + +The configuration for posting back to gerrit is shared with the gerrit +trigger in zuul.conf. Please consult the gerrit trigger documentation. diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst index f5e2226ba3..91ac24afc8 100644 --- a/doc/source/zuul.rst +++ b/doc/source/zuul.rst @@ -338,16 +338,14 @@ explanation of each of the parameters:: the change, set this to ``false``. Default: ``true``. **success** - Describes what Zuul should do if all the jobs complete successfully. + Describes where Zuul should report to if all the jobs complete + successfully. This section is optional; if it is omitted, Zuul will run jobs and do nothing on success; it will not even report a message to Gerrit. - If the section is present, it will leave a message on the Gerrit - review. Each additional argument is assumed to be an argument to - ``gerrit review``, with the boolean value of ``true`` simply - indicating that the argument should be present without following it - with a value. For example, ``verified: 1`` becomes ``gerrit - review --verified 1`` and ``submit: true`` becomes ``gerrit review - --submit``. + If the section is present, the listed reporter plugins will be + asked to report on the jobs. + Each reporter's value dictionary is handled by the reporter. See + reporters for more details. **failure** Uses the same syntax as **success**, but describes what Zuul should @@ -373,9 +371,11 @@ file. The first is called a *check* pipeline:: trigger: - event: patchset-created success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 This will trigger jobs each time a new patchset (or change) is uploaded to Gerrit, and report +/-1 values to Gerrit in the @@ -388,10 +388,12 @@ uploaded to Gerrit, and report +/-1 values to Gerrit in the approval: - approved: 1 success: - verified: 2 - submit: true + gerrit: + verified: 2 + submit: true failure: - verified: -2 + gerrit: + verified: -2 This will trigger jobs whenever a reviewer leaves a vote of ``1`` in the ``approved`` review category in Gerrit (a non-standard category). @@ -425,9 +427,11 @@ development and not yet ready to be presented to developers. :: trigger: - event: change-merged success: - force-message: True + gerrit: + force-message: True failure: - force-message: True + gerrit: + force-message: True The ``change-merged`` events happen when a change has been merged in the git repository. The change is thus closed and Gerrit will not accept modifications diff --git a/etc/layout.yaml-sample b/etc/layout.yaml-sample index b49f5d5968..30a335216c 100644 --- a/etc/layout.yaml-sample +++ b/etc/layout.yaml-sample @@ -5,9 +5,11 @@ pipelines: gerrit: - event: patchset-created success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: tests manager: IndependentPipelineManager @@ -16,9 +18,11 @@ pipelines: - event: patchset-created email_filter: ^.*@example.org$ success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: post manager: IndependentPipelineManager @@ -35,12 +39,14 @@ pipelines: approval: - approved: 1 start: - verified: 0 + gerrit: + verified: 0 success: - verified: 2 - submit: true + gerrit: + verified: 1 failure: - verified: -2 + gerrit: + verified: -1 jobs: - name: ^.*-merge$ diff --git a/etc/zuul.conf-sample b/etc/zuul.conf-sample index edf1044393..cd4ba67faf 100644 --- a/etc/zuul.conf-sample +++ b/etc/zuul.conf-sample @@ -18,4 +18,4 @@ state_dir=/var/lib/zuul git_dir=/var/lib/zuul/git ;git_user_email=zuul@example.com ;git_user_name=zuul -status_url=https://jenkins.example.com/zuul/status +status_url=https://jenkins.example.com/zuul/status \ No newline at end of file diff --git a/tests/fixtures/layout-delayed-repo-init.yaml b/tests/fixtures/layout-delayed-repo-init.yaml index e0613f1d1b..6caf6222a2 100644 --- a/tests/fixtures/layout-delayed-repo-init.yaml +++ b/tests/fixtures/layout-delayed-repo-init.yaml @@ -5,9 +5,11 @@ pipelines: gerrit: - event: patchset-created success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: post manager: IndependentPipelineManager @@ -25,12 +27,15 @@ pipelines: approval: - approved: 1 success: - verified: 2 - submit: true + gerrit: + verified: 2 + submit: true failure: - verified: -2 + gerrit: + verified: -2 start: - verified: 0 + gerrit: + verified: 0 precedence: high projects: diff --git a/tests/fixtures/layout-live-reconfiguration-functions.yaml b/tests/fixtures/layout-live-reconfiguration-functions.yaml index f477a1250e..e261a884b4 100644 --- a/tests/fixtures/layout-live-reconfiguration-functions.yaml +++ b/tests/fixtures/layout-live-reconfiguration-functions.yaml @@ -11,12 +11,15 @@ pipelines: approval: - approved: 1 success: - verified: 2 - submit: true + gerrit: + verified: 2 + submit: true failure: - verified: -2 + gerrit: + verified: -2 start: - verified: 0 + gerrit: + verified: 0 precedence: high jobs: diff --git a/tests/fixtures/layout-timer.yaml b/tests/fixtures/layout-timer.yaml index e24fb131d5..9e0f66b44b 100644 --- a/tests/fixtures/layout-timer.yaml +++ b/tests/fixtures/layout-timer.yaml @@ -5,9 +5,11 @@ pipelines: gerrit: - event: patchset-created success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: periodic manager: IndependentPipelineManager diff --git a/tests/fixtures/layout.yaml b/tests/fixtures/layout.yaml index 675d3516b7..1cb86880e2 100644 --- a/tests/fixtures/layout.yaml +++ b/tests/fixtures/layout.yaml @@ -8,9 +8,11 @@ pipelines: gerrit: - event: patchset-created success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: post manager: IndependentPipelineManager @@ -28,12 +30,15 @@ pipelines: approval: - approved: 1 success: - verified: 2 - submit: true + gerrit: + verified: 2 + submit: true failure: - verified: -2 + gerrit: + verified: -2 start: - verified: 0 + gerrit: + verified: 0 precedence: high - name: unused @@ -51,9 +56,11 @@ pipelines: gerrit: - event: change-restored success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: dup2 manager: IndependentPipelineManager @@ -61,9 +68,11 @@ pipelines: gerrit: - event: change-restored success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: conflict dequeue-on-conflict: false @@ -75,12 +84,15 @@ pipelines: approval: - approved: 1 success: - verified: 2 - submit: true + gerrit: + verified: 2 + submit: true failure: - verified: -2 + gerrit: + verified: -2 start: - verified: 0 + gerrit: + verified: 0 jobs: - name: ^.*-merge$ diff --git a/tests/fixtures/layouts/good_layout.yaml b/tests/fixtures/layouts/good_layout.yaml index 15be6ef7e5..4bd5e70fe4 100644 --- a/tests/fixtures/layouts/good_layout.yaml +++ b/tests/fixtures/layouts/good_layout.yaml @@ -8,9 +8,11 @@ pipelines: gerrit: - event: patchset-created success: - verified: 1 + gerrit: + verified: 1 failure: - verified: -1 + gerrit: + verified: -1 - name: post manager: IndependentPipelineManager @@ -28,15 +30,18 @@ pipelines: - event: comment-added approval: - approved: 1 - success: - verified: 2 - code-review: 1 - submit: true - failure: - verified: -2 - workinprogress: true start: - verified: 0 + gerrit: + verified: 0 + success: + gerrit: + verified: 2 + code-review: 1 + submit: true + failure: + gerrit: + verified: -2 + workinprogress: true jobs: - name: ^.*-merge$ diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index c7f730d033..a473ccb244 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -44,6 +44,7 @@ import testtools import zuul.scheduler import zuul.webapp import zuul.launcher.gearman +import zuul.reporter.gerrit import zuul.trigger.gerrit import zuul.trigger.timer @@ -388,6 +389,8 @@ class FakeURLOpener(object): class FakeGerritTrigger(zuul.trigger.gerrit.Gerrit): + name = 'gerrit' + def __init__(self, upstream_root, *args): super(FakeGerritTrigger, self).__init__(*args) self.upstream_root = upstream_root @@ -777,6 +780,9 @@ class TestScheduler(testtools.TestCase): self.timer = zuul.trigger.timer.Timer(self.config, self.sched) self.sched.registerTrigger(self.timer) + self.sched.registerReporter( + zuul.reporter.gerrit.Reporter(self.gerrit)) + self.sched.start() self.sched.reconfigure(self.config) self.sched.resume() diff --git a/zuul/cmd/server.py b/zuul/cmd/server.py index 4b49be2e58..404764fd7a 100755 --- a/zuul/cmd/server.py +++ b/zuul/cmd/server.py @@ -165,6 +165,7 @@ class Server(object): # See comment at top of file about zuul imports import zuul.scheduler import zuul.launcher.gearman + import zuul.reporter.gerrit import zuul.trigger.gerrit import zuul.trigger.timer import zuul.webapp @@ -181,10 +182,12 @@ class Server(object): gerrit = zuul.trigger.gerrit.Gerrit(self.config, self.sched) timer = zuul.trigger.timer.Timer(self.config, self.sched) webapp = zuul.webapp.WebApp(self.sched) + gerrit_reporter = zuul.reporter.gerrit.Reporter(gerrit) self.sched.setLauncher(gearman) self.sched.registerTrigger(gerrit) self.sched.registerTrigger(timer) + self.sched.registerReporter(gerrit_reporter) self.sched.start() self.sched.reconfigure(self.config) diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py index 9ddef11562..64058543f2 100644 --- a/zuul/layoutvalidator.py +++ b/zuul/layoutvalidator.py @@ -54,6 +54,8 @@ class LayoutSchema(object): trigger = v.Required(v.Any({'gerrit': toList(gerrit_trigger)}, {'timer': toList(timer_trigger)})) + report_actions = {'gerrit': variable_dict} + pipeline = {v.Required('name'): str, v.Required('manager'): manager, 'precedence': precedence, @@ -63,9 +65,9 @@ class LayoutSchema(object): 'dequeue-on-new-patchset': bool, 'dequeue-on-conflict': bool, 'trigger': trigger, - 'success': variable_dict, - 'failure': variable_dict, - 'start': variable_dict, + 'success': report_actions, + 'failure': report_actions, + 'start': report_actions, } pipelines = [pipeline] diff --git a/zuul/model.py b/zuul/model.py index a582ead248..d68ac915dd 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -57,6 +57,9 @@ class Pipeline(object): self.queues = [] self.precedence = PRECEDENCE_NORMAL self.trigger = None + self.start_actions = None + self.success_actions = None + self.failure_actions = None def __repr__(self): return '' % self.name @@ -348,6 +351,28 @@ class Pipeline(object): return ret +class ActionReporter(object): + """An ActionReporter has a reporter and its configured paramaters""" + + def __repr__(self): + return '' % (self.reporter, self.params) + + def __init__(self, reporter, params): + self.reporter = reporter + self.params = params + + def report(self, change, message): + """Sends the built message off to the configured reporter. + Takes the change and message and adds the configured parameters. + """ + return self.reporter.report(change, message, self.params) + + def getSubmitAllowNeeds(self): + """Gets the submit allow needs from the reporter based off the + parameters.""" + return self.reporter.getSubmitAllowNeeds(self.params) + + class ChangeQueue(object): """DependentPipelines have multiple parallel queues shared by different projects; this is one of them. For instance, there may diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/zuul/reporter/gerrit.py b/zuul/reporter/gerrit.py new file mode 100644 index 0000000000..cceaccaa1d --- /dev/null +++ b/zuul/reporter/gerrit.py @@ -0,0 +1,49 @@ +# Copyright 2013 Rackspace Australia +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import logging + + +class Reporter(object): + """Sends off reports to Gerrit.""" + + name = 'gerrit' + log = logging.getLogger("zuul.reporter.gerrit.Reporter") + + def __init__(self, trigger): + """Set up the reporter.""" + self.gerrit = trigger.gerrit + self.trigger = trigger + + def report(self, change, message, params): + """Send a message to gerrit.""" + self.log.debug("Report change %s, params %s, message: %s" % + (change, params, message)) + if not params: + self.log.debug("Not reporting change %s: No params specified." % + change) + return + changeid = '%s,%s' % (change.number, change.patchset) + change._ref_sha = self.trigger.getRefSha(change.project.name, + 'refs/heads/' + change.branch) + return self.gerrit.review(change.project.name, changeid, message, + params) + + def getSubmitAllowNeeds(self, params): + """Get a list of code review labels that are allowed to be + "needed" in the submit records for a change, with respect + to this queue. In other words, the list of review labels + this reporter itself is likely to set before submitting. + """ + return params diff --git a/zuul/scheduler.py b/zuul/scheduler.py index 8f909ebf1f..8a4d942857 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -28,7 +28,7 @@ import yaml import layoutvalidator import model -from model import Pipeline, Project, ChangeQueue, EventFilter +from model import ActionReporter, Pipeline, Project, ChangeQueue, EventFilter import merger statsd = extras.try_import('statsd.statsd') @@ -75,6 +75,7 @@ class Scheduler(threading.Thread): self._stopped = False self.launcher = None self.triggers = dict() + self.reporters = dict() self.config = None self._maintain_trigger_cache = False @@ -134,13 +135,27 @@ class Scheduler(threading.Thread): 'dequeue-on-new-patchset', True) pipeline.dequeue_on_conflict = conf_pipeline.get( 'dequeue-on-conflict', True) + + action_reporters = {} + for action in ['start', 'success', 'failure']: + action_reporters[action] = [] + if conf_pipeline.get(action): + for reporter_name, params \ + in conf_pipeline.get(action).items(): + if reporter_name in self.reporters.keys(): + action_reporters[action].append(ActionReporter( + self.reporters[reporter_name], params)) + else: + self.log.error('Invalid reporter name %s' % + reporter_name) + pipeline.start_actions = action_reporters['start'] + pipeline.success_actions = action_reporters['success'] + pipeline.failure_actions = action_reporters['failure'] + manager = globals()[conf_pipeline['manager']](self, pipeline) pipeline.setManager(manager) - layout.pipelines[conf_pipeline['name']] = pipeline - manager.success_action = conf_pipeline.get('success') - manager.failure_action = conf_pipeline.get('failure') - manager.start_action = conf_pipeline.get('start') + # TODO: move this into triggers (may require pluggable # configuration) if 'gerrit' in conf_pipeline['trigger']: @@ -299,6 +314,11 @@ class Scheduler(threading.Thread): name = trigger.name self.triggers[name] = trigger + def registerReporter(self, reporter, name=None): + if name is None: + name = reporter.name + self.reporters[name] = reporter + def getProject(self, name): self.layout_lock.acquire() p = None @@ -655,9 +675,6 @@ class BasePipelineManager(object): self.pipeline = pipeline self.building_jobs = {} self.event_filters = [] - self.success_action = {} - self.failure_action = {} - self.start_action = {} if self.sched.config and self.sched.config.has_option( 'zuul', 'report_times'): self.report_times = self.sched.config.getboolean( @@ -701,25 +718,22 @@ class BasePipelineManager(object): if tree: self.log.info(" %s" % p) log_jobs(tree) - if self.start_action: - self.log.info(" On start:") - self.log.info(" %s" % self.start_action) - if self.success_action: - self.log.info(" On success:") - self.log.info(" %s" % self.success_action) - if self.failure_action: - self.log.info(" On failure:") - self.log.info(" %s" % self.failure_action) + self.log.info(" On start:") + self.log.info(" %s" % self.pipeline.start_actions) + self.log.info(" On success:") + self.log.info(" %s" % self.pipeline.success_actions) + self.log.info(" On failure:") + self.log.info(" %s" % self.pipeline.failure_actions) def getSubmitAllowNeeds(self): # Get a list of code review labels that are allowed to be # "needed" in the submit records for a change, with respect # to this queue. In other words, the list of review labels # this queue itself is likely to set before submitting. - if self.success_action: - return self.success_action.keys() - else: - return {} + allow_needs = set() + for action_reporter in self.pipeline.success_actions: + allow_needs.update(action_reporter.getSubmitAllowNeeds()) + return allow_needs def eventMatches(self, event): for ef in self.event_filters: @@ -736,17 +750,38 @@ class BasePipelineManager(object): def reportStart(self, change): try: self.log.info("Reporting start, action %s change %s" % - (self.start_action, change)) + (self.pipeline.start_actions, change)) msg = "Starting %s jobs." % self.pipeline.name if self.sched.config.has_option('zuul', 'status_url'): msg += "\n" + self.sched.config.get('zuul', 'status_url') - ret = self.pipeline.trigger.report(change, msg, self.start_action) + ret = self.sendReport(self.pipeline.start_actions, + change, msg) if ret: self.log.error("Reporting change start %s received: %s" % (change, ret)) except: self.log.exception("Exception while reporting start:") + def sendReport(self, action_reporters, change, message): + """Sends the built message off to configured reporters. + + Takes the action_reporters, change, message and extra options and + sends them to the pluggable reporters. + """ + report_errors = [] + if len(action_reporters) > 0: + if not change.number: + self.log.debug("Not reporting change %s: No number present." + % change) + return + for action_reporter in action_reporters: + ret = action_reporter.report(change, message) + if ret: + report_errors.append(ret) + if len(report_errors) == 0: + return + return report_errors + def isChangeReadyToBeEnqueued(self, change): return True @@ -824,7 +859,7 @@ class BasePipelineManager(object): if change_queue: self.log.debug("Adding change %s to queue %s" % (change, change_queue)) - if self.start_action: + if len(self.pipeline.start_actions) > 0: self.reportStart(change) item = change_queue.enqueueChange(change) self.reportStats(item) @@ -1076,19 +1111,19 @@ class BasePipelineManager(object): self.log.debug("Reporting change %s" % item.change) ret = True # Means error as returned by trigger.report if self.pipeline.didAllJobsSucceed(item): - self.log.debug("success %s %s" % (self.success_action, - self.failure_action)) - action = self.success_action + self.log.debug("success %s %s" % (self.pipeline.success_actions, + self.pipeline.failure_actions)) + actions = self.pipeline.success_actions item.setReportedResult('SUCCESS') else: - action = self.failure_action + actions = self.pipeline.failure_actions item.setReportedResult('FAILURE') report = self.formatReport(item) item.reported = True try: - self.log.info("Reporting change %s, action: %s" % - (item.change, action)) - ret = self.pipeline.trigger.report(item.change, report, action) + self.log.info("Reporting change %s, actions: %s" % + (item.change, actions)) + ret = self.sendReport(actions, item.change, report) if ret: self.log.error("Reporting change %s received: %s" % (item.change, ret)) diff --git a/zuul/trigger/gerrit.py b/zuul/trigger/gerrit.py index beecf39220..976849c1fd 100644 --- a/zuul/trigger/gerrit.py +++ b/zuul/trigger/gerrit.py @@ -138,22 +138,6 @@ class Gerrit(object): self.gerrit_connector.stop() self.gerrit_connector.join() - def report(self, change, message, action): - self.log.debug("Report change %s, action %s, message: %s" % - (change, action, message)) - if not change.number: - self.log.debug("Change has no number; not reporting") - return - if not action: - self.log.debug("No action specified; not reporting") - return - changeid = '%s,%s' % (change.number, change.patchset) - ref = 'refs/heads/' + change.branch - change._ref_sha = self.getRefSha(change.project.name, - ref) - return self.gerrit.review(change.project.name, changeid, - message, action) - def _getInfoRefs(self, project): url = "%s/p/%s/info/refs?service=git-upload-pack" % ( self.baseurl, project) diff --git a/zuul/trigger/timer.py b/zuul/trigger/timer.py index e67af01a73..b75a165ca5 100644 --- a/zuul/trigger/timer.py +++ b/zuul/trigger/timer.py @@ -40,9 +40,6 @@ class Timer(object): def stop(self): self.apsched.shutdown() - def report(self, change, message, action): - raise Exception("Timer trigger does not support reporting.") - def isMerged(self, change, head=None): raise Exception("Timer trigger does not support checking if " "a change is merged.")