diff --git a/doc/source/drivers/gerrit.rst b/doc/source/drivers/gerrit.rst index ecdfd6247a..0a30a222c3 100644 --- a/doc/source/drivers/gerrit.rst +++ b/doc/source/drivers/gerrit.rst @@ -530,6 +530,16 @@ Reporter Configuration disable this behavior (file and line commands will still be sent if present). + .. attr:: notify + + If this is set to a notify handling value then send + notifications at the specified level. If not, use the default + specified by the gerrit api. Some possible values include + ``ALL`` and ``NONE``. See the gerrit api for available options + and default value: + + ``_ + A :ref:`connection` that uses the gerrit driver must be supplied to the trigger. diff --git a/releasenotes/notes/gerrit-reporter-notify-e64c72c49978ded3.yaml b/releasenotes/notes/gerrit-reporter-notify-e64c72c49978ded3.yaml new file mode 100644 index 0000000000..f1be9ef740 --- /dev/null +++ b/releasenotes/notes/gerrit-reporter-notify-e64c72c49978ded3.yaml @@ -0,0 +1,10 @@ +--- +upgrade: + - | + The Gerrit driver now has an additional option, + :attr:`pipeline.reporter..notify` which + configures the notification handling of reviews as noted in the + `Gerrit changes API Docs `_. + used to suppress reporting job results as review comments. Due to + the configuration syntax for Gerrit reporters, the word "notify" + may no longer be used as a review label. diff --git a/tests/fakegerrit.py b/tests/fakegerrit.py index ebe79c9a88..cb850382e0 100644 --- a/tests/fakegerrit.py +++ b/tests/fakegerrit.py @@ -53,6 +53,7 @@ class FakeGerritChange(object): self.source_hostname = gerrit.canonical_hostname self.gerrit_baseurl = gerrit.baseurl self.reported = 0 + self.notify = None self.queried = 0 self.patchsets = [] self.number = number @@ -829,7 +830,7 @@ class GerritWebServer(object): tag = data.get('tag', None) fake_gerrit._test_handle_review( int(change.data['number']), message, False, labels, - True, False, comments, tag=tag) + None, True, False, comments, tag=tag) self.send_response(200) self.end_headers() @@ -863,7 +864,7 @@ class GerritWebServer(object): for change_number in changes_to_merge: fake_gerrit._test_handle_review( int(change_number), message, True, labels, - False, True) + None, False, True) self.send_response(200) self.end_headers() @@ -1162,14 +1163,15 @@ class FakeGerritConnection(gerritconnection.GerritConnection): return event def review(self, item, change, message, submit, labels, - checks_api, file_comments, phase1, phase2, + checks_api, notify, file_comments, phase1, phase2, zuul_event_id=None): if self.web_server: return super(FakeGerritConnection, self).review( item, change, message, submit, labels, checks_api, - file_comments, phase1, phase2, zuul_event_id) + notify, file_comments, phase1, phase2, + zuul_event_id) self._test_handle_review(int(change.number), message, submit, - labels, phase1, phase2) + labels, notify, phase1, phase2) def _test_get_submitted_together(self, change): topic = change.data.get('topic') @@ -1194,10 +1196,13 @@ class FakeGerritConnection(gerritconnection.GerritConnection): return results def _test_handle_review(self, change_number, message, submit, labels, + notify, phase1, phase2, file_comments=None, tag=None): # Handle a review action from a test change = self.changes[change_number] + change.notify = notify + # Add the approval back onto the change (ie simulate what gerrit would # do). # Usually when zuul leaves a review it'll create a feedback loop where diff --git a/tests/fixtures/layouts/gerrit-notify.yaml b/tests/fixtures/layouts/gerrit-notify.yaml new file mode 100644 index 0000000000..76864d48cc --- /dev/null +++ b/tests/fixtures/layouts/gerrit-notify.yaml @@ -0,0 +1,28 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + notify: NONE + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: check-job + parent: null + run: playbooks/base.yaml + nodeset: + nodes: + - label: ubuntu-xenial + name: controller + +- project: + name: org/project + check: + jobs: + - check-job diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py index c29c4c8edc..a5dcc38220 100644 --- a/tests/unit/test_gerrit.py +++ b/tests/unit/test_gerrit.py @@ -1423,3 +1423,15 @@ class TestGerritDriver(ZuulTestCase): self.assertHistory([ dict(name='check-job', result='SUCCESS', changes='1,1'), ]) + + @simple_layout('layouts/gerrit-notify.yaml') + def test_notify(self): + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertHistory([ + dict(name='check-job', result='SUCCESS'), + ]) + + self.assertEqual(A.notify, 'NONE') diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index df9c20635a..8c317ac967 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -1119,17 +1119,17 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): self.event_queue.put(event) def review(self, item, change, message, submit, labels, checks_api, - file_comments, phase1, phase2, zuul_event_id=None): + notify, file_comments, phase1, phase2, zuul_event_id=None): if self.session: meth = self.review_http else: meth = self.review_ssh - return meth(item, change, message, submit, labels, checks_api, + return meth(item, change, message, submit, labels, checks_api, notify, file_comments, phase1, phase2, zuul_event_id=zuul_event_id) def review_ssh(self, item, change, message, submit, labels, checks_api, - file_comments, phase1, phase2, zuul_event_id=None): + notify, file_comments, phase1, phase2, zuul_event_id=None): log = get_annotated_logger(self.log, zuul_event_id) if checks_api: log.error("Zuul is configured to report to the checks API, " @@ -1137,6 +1137,8 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): "in the configuration file.") project = change.project.name cmd = 'gerrit review --project %s' % project + if notify: + cmd += ' --notify %s' % shlex.quote(notify) if phase1: if message: b_len = len(message.encode('utf-8')) @@ -1206,7 +1208,7 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): time.sleep(x * self.submit_retry_backoff) def review_http(self, item, change, message, submit, labels, - checks_api, file_comments, phase1, phase2, + checks_api, notify, file_comments, phase1, phase2, zuul_event_id=None): changeid = "%s~%s~%s" % ( urllib.parse.quote(str(change.project), safe=''), @@ -1220,6 +1222,8 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): message = ("%s... (truncated)" % message[:GERRIT_HUMAN_MESSAGE_LIMIT - 20]) data = dict(strict_labels=False) + if notify: + data['notify'] = notify if phase1: data['message'] = message if change.is_current_patchset: diff --git a/zuul/driver/gerrit/gerritreporter.py b/zuul/driver/gerrit/gerritreporter.py index 3d32bfc4e3..b55b7bc9ec 100644 --- a/zuul/driver/gerrit/gerritreporter.py +++ b/zuul/driver/gerrit/gerritreporter.py @@ -35,6 +35,7 @@ class GerritReporter(BaseReporter): self._create_comment = action.pop('comment', True) self._submit = action.pop('submit', False) self._checks_api = action.pop('checks-api', None) + self._notify = action.pop('notify', None) self._labels = {str(k): v for k, v in action.items()} def __repr__(self): @@ -95,7 +96,8 @@ class GerritReporter(BaseReporter): return self.connection.review(item, change, message, self._submit, self._labels, - self._checks_api, comments, + self._checks_api, self._notify, + comments, phase1, phase2, zuul_event_id=item.event)