zuul/zuul
Code Issues Proposed changes

Add 'comment' option to Gerrit reporter

Browse Source 
Mirror the GitHub reporter's "comment" option to allow for supressing
review messages when reporting to Gerrit.  This is needed for use with
the checks api since the purpose is to avoid leaving job results in
comments.  This removes the ability for 'comment' to be a review label,
which seems unlikely, but plausible, so a release note is added.

Rename the "checks_api" Gerrit reporter option to "checks-api" for
hyphen consistency with the rest of the configuration.  This is an
experimental feature and not generally useful yet, so no release note
is included.  This change supports both forms and updates the
documentation; a later change will remove "checks_api".

While adding documentation for the "comment" option, the description
of the Gerrit reporter is updated.

A small refactoring of the Gerrit reporter is undertaken to make
dealing with additional non-label options easier.

Change-Id: I44c0ba93601f3be3eb0f094bd782cecc4a0372f7
This commit is contained in:
James E. Blair 2019-10-23 08:28:03 -07:00
parent 0735b7b838
commit c6db86ad38
9 changed files with 118 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
doc/source/admin/drivers/gerrit.rst
View File

@ -224,16 +224,30 @@ be able to invoke the ``gerrit stream-events`` command over SSH.
Reporter Configuration
----------------------
Zuul works with standard versions of Gerrit by invoking the
``gerrit`` command over an SSH connection. It reports back to
Gerrit using SSH.
Zuul works with standard versions of Gerrit by invoking the ``gerrit``
command over an SSH connection, unless the connection is configured
with an HTTP password, in which case the HTTP API is used.
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``.
.. attr:: pipeline.reporter.<gerrit reporter>
The dictionary passed to the Gerrit reporter is used to provide label
values to Gerrit. To set the `Verified` label to `1`, add ``verified:
1`` to the dictionary.
The following additional keys are recognized:
.. attr:: submit
:default: False
Set this to ``True`` to submit (merge) the change.
.. attr:: comment
:default: True
If this is true (the default), Zuul will leave review messages
on the change (including job results). Set this to false to
disable this behavior (file and line commands will still be sent
if present).
A :ref:`connection<connections>` that uses the gerrit driver must be
supplied to the trigger.
@ -412,31 +426,31 @@ configure a pipeline like this:
uuid: 'zuul:check'
enqueue:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: SCHEDULED
message: 'Change has been enqueued in check'
start:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: RUNNING
message: 'Jobs have started running'
no-jobs:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: NOT_RELEVANT
message: 'Change has no jobs configured'
success:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: SUCCESSFUL
message: 'Change passed all voting jobs'
failure:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: FAILED
message: 'Change failed'
@ -459,31 +473,31 @@ names such as ``zuul_check:project1``, ``zuul_gate:project1``,
scheme: 'zuul_check'
enqueue:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: SCHEDULED
message: 'Change has been enqueued in check'
start:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: RUNNING
message: 'Jobs have started running'
no-jobs:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: NOT_RELEVANT
message: 'Change has no jobs configured'
success:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: SUCCESSFUL
message: 'Change passed all voting jobs'
failure:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: FAILED
message: 'Change failed'

7
releasenotes/notes/gerrit-comment-de803cf3890b709d.yaml Normal file
View File

@ -0,0 +1,7 @@
upgrade:
- |
The Gerrit driver now has an additional option,
:attr:`pipeline.reporter.<gerrit reporter>.comment` which may be
used to suppress reporting job results as review comments. Due to
the configuration syntax for Gerrit reporters, the word "comment"
may no longer be used as a review label.

29
tests/base.py
View File

@ -727,12 +727,12 @@ class GerritWebServer(object):
return self._404()
message = data['message']
action = data.get('labels', {})
labels = data.get('labels', {})
comments = data.get('robot_comments', data.get('comments', {}))
tag = data.get('tag', None)
fake_gerrit._test_handle_review(
int(change.data['number']), message, action, comments,
tag=tag)
int(change.data['number']), message, False, labels,
comments, tag=tag)
self.send_response(200)
self.end_headers()
@ -742,9 +742,9 @@ class GerritWebServer(object):
return self._404()
message = None
action = {'submit': True}
labels = {}
fake_gerrit._test_handle_review(
int(change.data['number']), message, action)
int(change.data['number']), message, True, labels)
self.send_response(200)
self.end_headers()
@ -973,14 +973,16 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
}
return event
def review(self, item, message, action={}, file_comments={},
def review(self, item, message, submit, labels, checks_api, file_comments,
zuul_event_id=None):
if self.web_server:
return super(FakeGerritConnection, self).review(
item, message, action, file_comments)
self._test_handle_review(int(item.change.number), message, action)
item, message, submit, labels, checks_api, file_comments,
zuul_event_id)
self._test_handle_review(int(item.change.number), message, submit,
labels)
def _test_handle_review(self, change_number, message, action,
def _test_handle_review(self, change_number, message, submit, labels,
file_comments=None, tag=None):
# Handle a review action from a test
change = self.changes[change_number]
@ -995,10 +997,9 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
# happens they can add their own verified event into the queue.
# Nevertheless, we can update change with the new review in gerrit.
for cat in action:
if cat != 'submit':
change.addApproval(cat, action[cat], username=self.user,
tag=tag)
for cat in labels:
change.addApproval(cat, labels[cat], username=self.user,
tag=tag)
if message:
change.messages.append(message)
@ -1010,7 +1011,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection):
comment['message'], 'Zuul',
'zuul@example.com', self.user,
comment.get('range'))
if 'submit' in action:
if submit:
change.setMerged()
if message:
change.setReported()

20
tests/fixtures/layouts/gerrit-checks-nojobs.yaml vendored
View File

@ -7,31 +7,31 @@
uuid: 'zuul:check'
enqueue:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: SCHEDULED
message: 'Change has been enqueued in check'
start:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: RUNNING
message: 'Jobs have started running'
no-jobs:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: NOT_RELEVANT
message: 'Change has no jobs configured'
success:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: SUCCESSFUL
message: 'Change passed all voting jobs'
failure:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:check'
state: FAILED
message: 'Change failed'
@ -45,20 +45,20 @@
uuid: 'zuul:gate'
enqueue:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:gate'
state: SCHEDULED
message: 'Change has been enqueued in gate'
start:
gerrit:
Verified: 0
checks_api:
checks-api:
uuid: 'zuul:gate'
state: RUNNING
message: 'Jobs have started running'
no-jobs:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:gate'
state: NOT_RELEVANT
message: 'Change has no jobs configured'
@ -66,14 +66,14 @@
gerrit:
Verified: 2
submit: true
checks_api:
checks-api:
uuid: 'zuul:gate'
state: SUCCESSFUL
message: 'Change passed all voting jobs'
failure:
gerrit:
Verified: -2
checks_api:
checks-api:
uuid: 'zuul:gate'
state: FAILED
message: 'Change failed'

10
tests/fixtures/layouts/gerrit-checks-scheme.yaml vendored
View File

@ -7,31 +7,31 @@
scheme: 'zuul_check'
enqueue:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: SCHEDULED
message: 'Change has been enqueued in check'
start:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: RUNNING
message: 'Jobs have started running'
no-jobs:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: NOT_RELEVANT
message: 'Change has no jobs configured'
success:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: SUCCESSFUL
message: 'Change passed all voting jobs'
failure:
gerrit:
checks_api:
checks-api:
scheme: 'zuul_check'
state: FAILED
message: 'Change failed'

25
tests/fixtures/layouts/gerrit-checks.yaml vendored
View File

@ -7,31 +7,36 @@
uuid: 'zuul:check'
enqueue:
gerrit:
checks_api:
comment: False
checks-api:
uuid: 'zuul:check'
state: SCHEDULED
message: 'Change has been enqueued in check'
start:
gerrit:
checks_api:
comment: False
checks-api:
uuid: 'zuul:check'
state: RUNNING
message: 'Jobs have started running'
no-jobs:
gerrit:
checks_api:
comment: False
checks-api:
uuid: 'zuul:check'
state: NOT_RELEVANT
message: 'Change has no jobs configured'
success:
gerrit:
checks_api:
comment: False
checks-api:
uuid: 'zuul:check'
state: SUCCESSFUL
message: 'Change passed all voting jobs'
failure:
gerrit:
checks_api:
comment: False
checks-api:
uuid: 'zuul:check'
state: FAILED
message: 'Change failed'
@ -45,20 +50,20 @@
uuid: 'zuul:gate'
enqueue:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:gate'
state: SCHEDULED
message: 'Change has been enqueued in gate'
start:
gerrit:
Verified: 0
checks_api:
checks-api:
uuid: 'zuul:gate'
state: RUNNING
message: 'Jobs have started running'
no-jobs:
gerrit:
checks_api:
checks-api:
uuid: 'zuul:gate'
state: NOT_RELEVANT
message: 'Change has no jobs configured'
@ -66,14 +71,14 @@
gerrit:
Verified: 2
submit: true
checks_api:
checks-api:
uuid: 'zuul:gate'
state: SUCCESSFUL
message: 'Change passed all voting jobs'
failure:
gerrit:
Verified: -2
checks_api:
checks-api:
uuid: 'zuul:gate'
state: FAILED
message: 'Change failed'

4
tests/unit/test_gerrit.py
View File

@ -309,6 +309,8 @@ class TestChecksApi(ZuulTestCase):
'Change passed all voting jobs')
self.assertHistory([
dict(name='test-job', result='SUCCESS', changes='1,1')])
self.assertEqual(A.reported, 0, "no messages should be reported")
self.assertEqual(A.messages, [], "no messages should be reported")
@simple_layout('layouts/gerrit-checks.yaml')
def test_gate_pipeline(self):
@ -331,6 +333,8 @@ class TestChecksApi(ZuulTestCase):
self.assertHistory([
dict(name='test-job', result='SUCCESS', changes='1,1')])
self.assertEqual(A.data['status'], 'MERGED')
self.assertEqual(A.reported, 2,
"start and success messages should be reported")
@simple_layout('layouts/gerrit-checks-scheme.yaml')
def test_check_pipeline_scheme(self):

34
zuul/driver/gerrit/gerritconnection.py
View File

@ -935,23 +935,25 @@ class GerritConnection(BaseConnection):
def eventDone(self):
self.event_queue.task_done()
def review(self, item, message, action={},
file_comments={}, zuul_event_id=None):
def review(self, item, message, submit, labels, checks_api,
file_comments, zuul_event_id=None):
if self.session:
meth = self.review_http
else:
meth = self.review_ssh
return meth(item, message, action=action,
file_comments=file_comments, zuul_event_id=zuul_event_id)
return meth(item, message, submit, labels, checks_api,
file_comments, zuul_event_id=zuul_event_id)
def review_ssh(self, item, message, action={},
file_comments={}, zuul_event_id=None):
def review_ssh(self, item, message, submit, labels, checks_api,
file_comments, zuul_event_id=None):
change = item.change
project = change.project.name
cmd = 'gerrit review --project %s' % project
if message:
cmd += ' --message %s' % shlex.quote(message)
for key, val in action.items():
if submit:
cmd += ' --submit'
for key, val in labels.items():
if val is True:
cmd += ' --%s' % key
else:
@ -1000,22 +1002,12 @@ class GerritConnection(BaseConnection):
"attempt %s", x)
time.sleep(x * 10)
def review_http(self, item, message, action={},
file_comments={}, zuul_event_id=None):
def review_http(self, item, message, submit, labels,
checks_api, file_comments, zuul_event_id=None):
change = item.change
log = get_annotated_logger(self.log, zuul_event_id)
data = dict(message=message,
strict_labels=False)
submit = False
labels = {}
for key, val in action.items():
if key == 'checks_api':
continue
if val is True:
if key == 'submit':
submit = True
else:
labels[key] = val
if change.is_current_patchset:
if labels:
data['labels'] = labels
@ -1038,8 +1030,8 @@ class GerritConnection(BaseConnection):
urllib.parse.quote(str(change.project), safe=''),
urllib.parse.quote(str(change.branch), safe=''),
change.id)
if 'checks_api' in action:
self.report_checks(log, item, changeid, action['checks_api'])
if checks_api:
self.report_checks(log, item, changeid, checks_api)
if (message or data.get('labels') or data.get('comments')):
for x in range(1, 4):
try:

19
zuul/driver/gerrit/gerritreporter.py
View File

@ -26,6 +26,15 @@ class GerritReporter(BaseReporter):
name = 'gerrit'
log = logging.getLogger("zuul.GerritReporter")
def __init__(self, driver, connection, config=None):
super(GerritReporter, self).__init__(driver, connection, config)
action = self.config.copy()
self._create_comment = action.pop('comment', True)
self._submit = action.pop('submit', False)
self._checks_api = action.pop('checks-api',
action.pop('checks_api', None))
self._labels = action
def _getFileComments(self, item):
ret = {}
for build in item.current_build_set.getBuilds():
@ -54,14 +63,18 @@ class GerritReporter(BaseReporter):
comments = self._getFileComments(item)
self.filterComments(item, comments)
message = self._formatItemReport(item)
if self._create_comment:
message = self._formatItemReport(item)
else:
message = ''
log.debug("Report change %s, params %s, message: %s, comments: %s",
item.change, self.config, message, comments)
item.change._ref_sha = item.change.project.source.getRefSha(
item.change.project, 'refs/heads/' + item.change.branch)
return self.connection.review(item, message, self.config,
return self.connection.review(item, message, self._submit,
self._labels, self._checks_api,
comments, zuul_event_id=item.event)
def getSubmitAllowNeeds(self):
@ -70,7 +83,7 @@ class GerritReporter(BaseReporter):
to this queue. In other words, the list of review labels
this reporter itself is likely to set before submitting.
"""
return self.config
return self._labels
def getSchema():