Detect Gerrit gate pipelines with the wrong connection
If multiple Gerrit connections are configured and the wrong one is used in a gate pipeline, then the Gerrit reporter will correctly detect that it should not act on a change from a different connection, however, the isMerged check which is associated with the (correct) source connection performs an extra check immediately after reporting. Since the reporter didn't (couldn't) act, the ._ref_sha attribute (pointing to the previous sha before reporting) won't be set, and the isMerged check throws an exception. To correct this, have isMerged detect this case, log an error, and return False (which will fail the item in the pipeline). Also, have the reporter log when it's skipping reporting for a different connection. This is normal behavior when everything is configured correctly, but could be a helpful clue when something is misconfigured, so that is set at debug level. Change-Id: Ic3b715273e08ed2df7096c79add1b319066c46ec Story: 2007761
This commit is contained in:
parent
ec18f479e5
commit
7b6134ca81
2
tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/playbooks/test-job.yaml
vendored
Normal file
2
tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/playbooks/test-job.yaml
vendored
Normal file
|
@ -0,0 +1,2 @@
|
|||
- hosts: all
|
||||
tasks: []
|
33
tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/zuul.yaml
vendored
Normal file
33
tests/fixtures/config/wrong-connection-in-pipeline/git/common-config/zuul.yaml
vendored
Normal file
|
@ -0,0 +1,33 @@
|
|||
- pipeline:
|
||||
name: gate
|
||||
manager: dependent
|
||||
trigger:
|
||||
review_gerrit:
|
||||
- event: comment-added
|
||||
approval:
|
||||
- Approved: 1
|
||||
success:
|
||||
another_gerrit:
|
||||
Verified: 2
|
||||
submit: true
|
||||
failure:
|
||||
another_gerrit:
|
||||
Verified: -2
|
||||
start:
|
||||
another_gerrit:
|
||||
Verified: 0
|
||||
precedence: high
|
||||
|
||||
- job:
|
||||
name: base
|
||||
parent: null
|
||||
|
||||
- job:
|
||||
name: test-job
|
||||
run: playbooks/test-job.yaml
|
||||
|
||||
- project:
|
||||
name: review.example.com/org/project
|
||||
gate:
|
||||
jobs:
|
||||
- test-job
|
|
@ -0,0 +1 @@
|
|||
test
|
|
@ -0,0 +1,11 @@
|
|||
- tenant:
|
||||
name: tenant-one
|
||||
source:
|
||||
review_gerrit:
|
||||
config-projects:
|
||||
- common-config
|
||||
untrusted-projects:
|
||||
- org/project
|
||||
another_gerrit:
|
||||
untrusted-projects:
|
||||
- org/project
|
|
@ -552,3 +552,35 @@ class TestPolling(ZuulTestCase):
|
|||
self.assertHistory([
|
||||
dict(name='tag-job', result='SUCCESS'),
|
||||
])
|
||||
|
||||
|
||||
class TestWrongConnection(ZuulTestCase):
|
||||
config_file = 'zuul-connections-multiple-gerrits.conf'
|
||||
tenant_config_file = 'config/wrong-connection-in-pipeline/main.yaml'
|
||||
|
||||
def test_wrong_connection(self):
|
||||
# Test if the wrong connection is configured in a gate pipeline
|
||||
|
||||
# Our system has two gerrits, and we have configured a gate
|
||||
# pipeline to trigger on the "review_gerrit" connection, but
|
||||
# report (and merge) via "another_gerrit".
|
||||
A = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'A')
|
||||
A.addApproval('Code-Review', 2)
|
||||
self.fake_review_gerrit.addEvent(A.addApproval('Approved', 1))
|
||||
|
||||
self.waitUntilSettled()
|
||||
|
||||
B = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'B')
|
||||
# Let's try this as if the change was merged (say, via another tenant).
|
||||
B.setMerged()
|
||||
B.addApproval('Code-Review', 2)
|
||||
self.fake_review_gerrit.addEvent(B.addApproval('Approved', 1))
|
||||
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.assertEqual(A.reported, 0)
|
||||
self.assertEqual(B.reported, 0)
|
||||
self.assertHistory([
|
||||
dict(name='test-job', result='SUCCESS', changes='1,1'),
|
||||
dict(name='test-job', result='SUCCESS', changes='2,1'),
|
||||
], ordered=False)
|
||||
|
|
|
@ -917,6 +917,14 @@ class GerritConnection(BaseConnection):
|
|||
|
||||
ref = 'refs/heads/' + change.branch
|
||||
self.log.debug("Waiting for %s to appear in git repo" % (change))
|
||||
if not hasattr(change, '_ref_sha'):
|
||||
self.log.error("Unable to confirm change %s in git repo: "
|
||||
"the change has not been reported; "
|
||||
"this pipeline may be misconfigured "
|
||||
"(check for multiple Gerrit connections)." %
|
||||
(change,))
|
||||
return False
|
||||
|
||||
if self._waitForRefSha(change.project, ref, change._ref_sha):
|
||||
self.log.debug("Change %s is in the git repo" %
|
||||
(change))
|
||||
|
|
|
@ -51,6 +51,10 @@ class GerritReporter(BaseReporter):
|
|||
# the canonical hostname.
|
||||
if item.change.project.source.connection.canonical_hostname != \
|
||||
self.connection.canonical_hostname:
|
||||
log.debug("Not reporting %s as this Gerrit reporter "
|
||||
"is for %s and the change is from %s",
|
||||
item, self.connection.canonical_hostname,
|
||||
item.change.project.source.connection.canonical_hostname)
|
||||
return
|
||||
|
||||
comments = self.getFileComments(item)
|
||||
|
|
Loading…
Reference in New Issue