Limit github reporting to github sources
Also limit it to the specific github host that generated the event. This
corrects an issue seen live where the github driver attempts to report
on gerrit events causing a failure, which will eject the change without
reporting it on the gerrit side.
Testing this required adding start reporters, so that a traceback in
reporting a start to the wrong driver would cause the change to get
dequeued before the jobs run, and before the success is reported.
Also a new test needed to be added to make sure multiple connections
using the github driver are able to correctly report to the right github
server.
Change-Id: Ic4630dd46d72fbeb45f691615a99ef1299a8cd05
diff --git a/tests/fixtures/config/multi-driver/git/common-config/zuul.yaml b/tests/fixtures/config/multi-driver/git/common-config/zuul.yaml
index 2dab845..7b5a77c 100644
--- a/tests/fixtures/config/multi-driver/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/multi-driver/git/common-config/zuul.yaml
@@ -1,5 +1,5 @@
- pipeline:
- name: check_github
+ name: check
manager: independent
trigger:
github:
@@ -8,25 +8,23 @@
- opened
- changed
- reopened
- success:
- github:
- status: 'success'
- failure:
- github:
- status: 'failure'
-
-- pipeline:
- name: check_gerrit
- manager: independent
- trigger:
gerrit:
- event: patchset-created
success:
+ github:
+ status: 'success'
gerrit:
- verify: 1
+ Verified: 1
failure:
+ github:
+ status: 'failure'
gerrit:
- verify: 1
+ Verified: 1
+ start:
+ github:
+ comment: true
+ gerrit:
+ Verified: 0
- job:
name: project-gerrit
@@ -35,12 +33,12 @@
- project:
name: org/project
- check_gerrit:
+ check:
jobs:
- project-gerrit
- project:
name: org/project1
- check_github:
+ check:
jobs:
- project1-github
diff --git a/tests/fixtures/layouts/reporting-multiple-github.yaml b/tests/fixtures/layouts/reporting-multiple-github.yaml
new file mode 100644
index 0000000..f14000e
--- /dev/null
+++ b/tests/fixtures/layouts/reporting-multiple-github.yaml
@@ -0,0 +1,40 @@
+- pipeline:
+ name: check
+ description: Standard check
+ manager: independent
+ trigger:
+ github:
+ - event: pull_request
+ action: opened
+ github_ent:
+ - event: pull_request
+ action: opened
+ start:
+ github:
+ status: 'pending'
+ comment: false
+ github_ent:
+ status: 'pending'
+ comment: false
+ success:
+ github:
+ status: 'success'
+ github_ent:
+ status: 'success'
+
+- job:
+ name: project1-test1
+- job:
+ name: project2-test1
+
+- project:
+ name: org/project1
+ check:
+ jobs:
+ - project1-test1
+
+- project:
+ name: org/project2
+ check:
+ jobs:
+ - project2-test2
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index e8dff51..1ae36aa 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -406,6 +406,48 @@
self.assertEqual(len(D.comments), 1)
self.assertEqual(D.comments[0], 'Merge failed')
+ @simple_layout('layouts/reporting-multiple-github.yaml', driver='github')
+ def test_reporting_multiple_github(self):
+ project = 'org/project1'
+ # pipeline reports pull status both on start and success
+ self.executor_server.hold_jobs_in_build = True
+ A = self.fake_github.openFakePullRequest(project, 'master', 'A')
+ self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+ # open one on B as well, which should not effect A reporting
+ B = self.fake_github.openFakePullRequest('org/project2', 'master',
+ 'B')
+ self.fake_github.emitEvent(B.getPullRequestOpenedEvent())
+ self.waitUntilSettled()
+ # We should have a status container for the head sha
+ statuses = self.fake_github.statuses[project][A.head_sha]
+ self.assertIn(A.head_sha, self.fake_github.statuses[project].keys())
+ # We should only have one status for the head sha
+ self.assertEqual(1, len(statuses))
+ check_status = statuses[0]
+ check_url = ('http://zuul.example.com/status/#%s,%s' %
+ (A.number, A.head_sha))
+ self.assertEqual('tenant-one/check', check_status['context'])
+ self.assertEqual('Standard check', check_status['description'])
+ self.assertEqual('pending', check_status['state'])
+ self.assertEqual(check_url, check_status['url'])
+ self.assertEqual(0, len(A.comments))
+
+ self.executor_server.hold_jobs_in_build = False
+ self.executor_server.release()
+ self.waitUntilSettled()
+ # We should only have two statuses for the head sha
+ statuses = self.fake_github.statuses[project][A.head_sha]
+ self.assertEqual(2, len(statuses))
+ check_status = statuses[0]
+ check_url = ('http://zuul.example.com/status/#%s,%s' %
+ (A.number, A.head_sha))
+ self.assertEqual('tenant-one/check', check_status['context'])
+ self.assertEqual('success', check_status['state'])
+ self.assertEqual(check_url, check_status['url'])
+ self.assertEqual(1, len(A.comments))
+ self.assertThat(A.comments[0],
+ MatchesRegex('.*Build succeeded.*', re.DOTALL))
+
@simple_layout('layouts/dependent-github.yaml', driver='github')
def test_parallel_changes(self):
"Test that changes are tested in parallel and merged in series"
diff --git a/tests/unit/test_multi_driver.py b/tests/unit/test_multi_driver.py
index 864bd31..e40591b 100644
--- a/tests/unit/test_multi_driver.py
+++ b/tests/unit/test_multi_driver.py
@@ -43,3 +43,12 @@
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
+
+ # Check on reporting results
+ # github should have a success status (only).
+ statuses = self.fake_github.statuses['org/project1'][B.head_sha]
+ self.assertEqual(1, len(statuses))
+ self.assertEqual('success', statuses[0]['state'])
+
+ # gerrit should have only reported twice, on start and success
+ self.assertEqual(A.reported, 2)
diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py
index ea41ccd..b0791d9 100644
--- a/zuul/driver/github/githubreporter.py
+++ b/zuul/driver/github/githubreporter.py
@@ -19,6 +19,7 @@
from zuul.reporter import BaseReporter
from zuul.exceptions import MergeFailure
from zuul.driver.util import scalar_or_list
+from zuul.driver.github.githubsource import GithubSource
class GithubReporter(BaseReporter):
@@ -41,6 +42,17 @@
def report(self, item):
"""Report on an event."""
+
+ # If the source is not GithubSource we cannot report anything here.
+ if not isinstance(item.change.project.source, GithubSource):
+ return
+
+ # For supporting several Github connections we also must filter by
+ # the canonical hostname.
+ if item.change.project.source.connection.canonical_hostname != \
+ self.connection.canonical_hostname:
+ return
+
# order is important for github branch protection.
# A status should be set before a merge attempt
if self._commit_status is not None: