Separate reporters from triggers

Allows multiple reports per a patchset to be sent to pluggable
destinations. These are configurable per pipeline and, if not
specified, defaults to the legacy behaviour of reporting back only
to gerrit.

Having multiple reporting methods means only certain success/failure
/start parameters will apply to certain reporters. Reporters are
listed as keys under each of those actions.

This means that each key under success/failure/start is a reporter and the
dictionaries under those are sent to the reporter to deal with.

Change-Id: I80d7539772e1485d5880132f22e55751b25ec198

20 files changed, 288 insertions, 112 deletions

diff --git a/NEWS.rst b/NEWS.rst
index f6500f06a..db269a45d 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 039cffa75..5a0c7b956 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 000000000..d64d4f76f
--- /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 f5e2226ba..91ac24afc 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 b49f5d596..30a335216 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 edf104439..cd4ba67fa 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 e0613f1d1..6caf6222a 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 f477a1250..e261a884b 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 e24fb131d..9e0f66b44 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 675d3516b..1cb86880e 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 15be6ef7e..4bd5e70fe 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
+    start:
+      gerrit:
+        verified: 0
     success:
-      verified: 2
-      code-review: 1
-      submit: true
+      gerrit:
+        verified: 2
+        code-review: 1
+        submit: true
     failure:
-      verified: -2
-      workinprogress: true
-    start:
-      verified: 0
+      gerrit:
+        verified: -2
+        workinprogress: true
 
 jobs:
   - name: ^.*-merge$
diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py
index c7f730d03..a473ccb24 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 4b49be2e5..404764fd7 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 9ddef1156..64058543f 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 a582ead24..d68ac915d 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 '<Pipeline %s>' % 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 '<ActionReporter %s, %s>' % (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 000000000..e69de29bb
--- /dev/null
+++ b/zuul/reporter/__init__.py
diff --git a/zuul/reporter/gerrit.py b/zuul/reporter/gerrit.py
new file mode 100644
index 000000000..cceaccaa1
--- /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 8f909ebf1..8a4d94285 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 beecf3922..976849c1f 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 e67af01a7..b75a165ca 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.")