Merge "Fix some inconsistent indentation in docs" into feature/zuulv3
diff --git a/tests/base.py b/tests/base.py
index 785c136..617169a 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -554,7 +554,7 @@
def __init__(self, github, number, project, branch,
subject, upstream_root, files=[], number_of_commits=1,
- writers=[]):
+ writers=[], body=''):
"""Creates a new PR with several commits.
Sends an event about opened PR."""
self.github = github
@@ -563,6 +563,7 @@
self.project = project
self.branch = branch
self.subject = subject
+ self.body = body
self.number_of_commits = 0
self.upstream_root = upstream_root
self.files = []
@@ -602,6 +603,9 @@
def getPullRequestClosedEvent(self):
return self._getPullRequestEvent('closed')
+ def getPullRequestEditedEvent(self):
+ return self._getPullRequestEvent('edited')
+
def addComment(self, message):
self.comments.append(message)
self._updateTimeStamp()
@@ -723,6 +727,10 @@
}
return (name, data)
+ def editBody(self, body):
+ self.body = body
+ self._updateTimeStamp()
+
def _getRepo(self):
repo_path = os.path.join(self.upstream_root, self.project)
return git.Repo(repo_path)
@@ -830,7 +838,8 @@
'repo': {
'full_name': self.project
}
- }
+ },
+ 'body': self.body
},
'sender': {
'login': 'ghuser'
@@ -868,12 +877,14 @@
self.upstream_root = upstream_root
self.merge_failure = False
self.merge_not_allowed_count = 0
+ self.reports = []
- def openFakePullRequest(self, project, branch, subject, files=[]):
+ def openFakePullRequest(self, project, branch, subject, files=[],
+ body=''):
self.pr_number += 1
pull_request = FakeGithubPullRequest(
self, self.pr_number, project, branch, subject, self.upstream_root,
- files=files)
+ files=files, body=body)
self.pull_requests.append(pull_request)
return pull_request
@@ -934,7 +945,9 @@
}
},
'files': pr.files,
- 'labels': pr.labels
+ 'labels': pr.labels,
+ 'merged': pr.is_merged,
+ 'body': pr.body
}
return data
@@ -980,10 +993,14 @@
return ['master']
def commentPull(self, project, pr_number, message):
+ # record that this got reported
+ self.reports.append((project, pr_number, 'comment'))
pull_request = self.pull_requests[pr_number - 1]
pull_request.addComment(message)
def mergePull(self, project, pr_number, commit_message='', sha=None):
+ # record that this got reported
+ self.reports.append((project, pr_number, 'merge'))
pull_request = self.pull_requests[pr_number - 1]
if self.merge_failure:
raise Exception('Pull request was not merged')
@@ -999,6 +1016,8 @@
def setCommitStatus(self, project, sha, state, url='', description='',
context='default', user='zuul'):
+ # record that this got reported
+ self.reports.append((project, sha, 'status', (user, context, state)))
# always insert a status to the front of the list, to represent
# the last status provided for a commit.
# Since we're bypassing github API, which would require a user, we
@@ -1015,13 +1034,30 @@
})
def labelPull(self, project, pr_number, label):
+ # record that this got reported
+ self.reports.append((project, pr_number, 'label', label))
pull_request = self.pull_requests[pr_number - 1]
pull_request.addLabel(label)
def unlabelPull(self, project, pr_number, label):
+ # record that this got reported
+ self.reports.append((project, pr_number, 'unlabel', label))
pull_request = self.pull_requests[pr_number - 1]
pull_request.removeLabel(label)
+ def _getNeededByFromPR(self, change):
+ prs = []
+ pattern = re.compile(r"Depends-On.*https://%s/%s/pull/%s" %
+ (self.git_host, change.project.name,
+ change.number))
+ for pr in self.pull_requests:
+ if pattern.search(pr.body):
+ # Get our version of a pull so that it's a dict
+ pull = self.getPull(pr.project, pr.number)
+ prs.append(pull)
+
+ return prs
+
class BuildHistory(object):
def __init__(self, **kw):
diff --git a/tests/fixtures/layouts/crd-github.yaml b/tests/fixtures/layouts/crd-github.yaml
new file mode 100644
index 0000000..11bdf76
--- /dev/null
+++ b/tests/fixtures/layouts/crd-github.yaml
@@ -0,0 +1,79 @@
+- pipeline:
+ name: check
+ manager: independent
+ trigger:
+ github:
+ - event: pull_request
+ action: edited
+ start:
+ github: {}
+ success:
+ github: {}
+ failure:
+ github: {}
+
+- pipeline:
+ name: gate
+ manager: dependent
+ trigger:
+ github:
+ - event: pull_request
+ action: edited
+ start:
+ github: {}
+ success:
+ github:
+ merge: true
+ failure:
+ github: {}
+
+- job:
+ name: project1-test
+- job:
+ name: project2-test
+- job:
+ name: project3-test
+- job:
+ name: project4-test
+- job:
+ name: project5-test
+- job:
+ name: project6-test
+
+- project:
+ name: org/project1
+ check:
+ jobs:
+ - project1-test
+
+- project:
+ name: org/project2
+ check:
+ jobs:
+ - project2-test
+
+- project:
+ name: org/project3
+ gate:
+ queue: cogated
+ jobs:
+ - project3-test
+
+- project:
+ name: org/project4
+ gate:
+ queue: cogated
+ jobs:
+ - project4-test
+
+- project:
+ name: org/project5
+ gate:
+ jobs:
+ - project5-test
+
+- project:
+ name: org/project6
+ gate:
+ jobs:
+ - project6-test
diff --git a/tests/fixtures/layouts/reporting-github.yaml b/tests/fixtures/layouts/reporting-github.yaml
index d054df7..0fdec85 100644
--- a/tests/fixtures/layouts/reporting-github.yaml
+++ b/tests/fixtures/layouts/reporting-github.yaml
@@ -34,6 +34,29 @@
github:
comment: false
+- pipeline:
+ name: push-reporting
+ description: Uncommon reporting
+ manager: independent
+ trigger:
+ github:
+ - event: push
+ - event: pull_request
+ action: opened
+ start:
+ github:
+ comment: true
+ status: 'pending'
+ success:
+ github:
+ comment: true
+ status: 'success'
+ merge: true
+ failure:
+ github:
+ comment: true
+ status: 'failure'
+
- job:
name: project-test1
@@ -45,3 +68,9 @@
reporting:
jobs:
- project-test1
+
+- project:
+ name: org/project2
+ push-reporting:
+ jobs:
+ - project-test1
diff --git a/tests/unit/test_github_crd.py b/tests/unit/test_github_crd.py
new file mode 100644
index 0000000..bc7d499
--- /dev/null
+++ b/tests/unit/test_github_crd.py
@@ -0,0 +1,172 @@
+#!/usr/bin/env python
+# Copyright (c) 2017 IBM Corp.
+#
+# 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.
+
+from tests.base import ZuulTestCase, simple_layout
+
+
+class TestGithubCrossRepoDeps(ZuulTestCase):
+ """Test Github cross-repo dependencies"""
+ config_file = 'zuul-github-driver.conf'
+
+ @simple_layout('layouts/crd-github.yaml', driver='github')
+ def test_crd_independent(self):
+ "Test cross-repo dependences on an independent pipeline"
+
+ # Create a change in project1 that a project2 change will depend on
+ A = self.fake_github.openFakePullRequest('org/project1', 'master', 'A')
+
+ # Create a commit in B that sets the dependency on A
+ msg = "Depends-On: https://github.com/org/project1/pull/%s" % A.number
+ B = self.fake_github.openFakePullRequest('org/project2', 'master', 'B',
+ body=msg)
+
+ # Make an event to re-use
+ event = B.getPullRequestEditedEvent()
+
+ self.fake_github.emitEvent(event)
+ self.waitUntilSettled()
+
+ # The changes for the job from project2 should include the project1
+ # PR contet
+ changes = self.getJobFromHistory(
+ 'project2-test', 'org/project2').changes
+
+ self.assertEqual(changes, "%s,%s %s,%s" % (A.number,
+ A.head_sha,
+ B.number,
+ B.head_sha))
+
+ # There should be no more changes in the queue
+ tenant = self.sched.abide.tenants.get('tenant-one')
+ self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0)
+
+ @simple_layout('layouts/crd-github.yaml', driver='github')
+ def test_crd_dependent(self):
+ "Test cross-repo dependences on a dependent pipeline"
+
+ # Create a change in project3 that a project4 change will depend on
+ A = self.fake_github.openFakePullRequest('org/project3', 'master', 'A')
+
+ # Create a commit in B that sets the dependency on A
+ msg = "Depends-On: https://github.com/org/project3/pull/%s" % A.number
+ B = self.fake_github.openFakePullRequest('org/project4', 'master', 'B',
+ body=msg)
+
+ # Make an event to re-use
+ event = B.getPullRequestEditedEvent()
+
+ self.fake_github.emitEvent(event)
+ self.waitUntilSettled()
+
+ # The changes for the job from project4 should include the project3
+ # PR contet
+ changes = self.getJobFromHistory(
+ 'project4-test', 'org/project4').changes
+
+ self.assertEqual(changes, "%s,%s %s,%s" % (A.number,
+ A.head_sha,
+ B.number,
+ B.head_sha))
+
+ self.assertTrue(A.is_merged)
+ self.assertTrue(B.is_merged)
+
+ @simple_layout('layouts/crd-github.yaml', driver='github')
+ def test_crd_unshared_dependent(self):
+ "Test cross-repo dependences on unshared dependent pipeline"
+
+ # Create a change in project1 that a project2 change will depend on
+ A = self.fake_github.openFakePullRequest('org/project5', 'master', 'A')
+
+ # Create a commit in B that sets the dependency on A
+ msg = "Depends-On: https://github.com/org/project5/pull/%s" % A.number
+ B = self.fake_github.openFakePullRequest('org/project6', 'master', 'B',
+ body=msg)
+
+ # Make an event for B
+ event = B.getPullRequestEditedEvent()
+
+ # Emit for B, which should not enqueue A because they do not share
+ # A queue. Since B depends on A, and A isn't enqueue, B will not run
+ self.fake_github.emitEvent(event)
+ self.waitUntilSettled()
+
+ self.assertEqual(0, len(self.history))
+
+ # Enqueue A alone, let it finish
+ self.fake_github.emitEvent(A.getPullRequestEditedEvent())
+ self.waitUntilSettled()
+
+ self.assertTrue(A.is_merged)
+ self.assertFalse(B.is_merged)
+ self.assertEqual(1, len(self.history))
+
+ # With A merged, B should go through
+ self.fake_github.emitEvent(event)
+ self.waitUntilSettled()
+
+ self.assertTrue(B.is_merged)
+ self.assertEqual(2, len(self.history))
+
+ @simple_layout('layouts/crd-github.yaml', driver='github')
+ def test_crd_cycle(self):
+ "Test cross-repo dependency cycles"
+
+ # A -> B -> A
+ msg = "Depends-On: https://github.com/org/project6/pull/2"
+ A = self.fake_github.openFakePullRequest('org/project5', 'master', 'A',
+ body=msg)
+ msg = "Depends-On: https://github.com/org/project5/pull/1"
+ B = self.fake_github.openFakePullRequest('org/project6', 'master', 'B',
+ body=msg)
+
+ self.fake_github.emitEvent(A.getPullRequestEditedEvent())
+ self.waitUntilSettled()
+
+ self.assertFalse(A.is_merged)
+ self.assertFalse(B.is_merged)
+ self.assertEqual(0, len(self.history))
+
+ @simple_layout('layouts/crd-github.yaml', driver='github')
+ def test_crd_needed_changes(self):
+ "Test cross-repo needed changes discovery"
+
+ # Given change A and B, where B depends on A, when A
+ # completes B should be enqueued (using a shared queue)
+
+ # Create a change in project3 that a project4 change will depend on
+ A = self.fake_github.openFakePullRequest('org/project3', 'master', 'A')
+
+ # Set B to depend on A
+ msg = "Depends-On: https://github.com/org/project3/pull/%s" % A.number
+ B = self.fake_github.openFakePullRequest('org/project4', 'master', 'B',
+ body=msg)
+
+ # Enqueue A, which when finished should enqueue B
+ self.fake_github.emitEvent(A.getPullRequestEditedEvent())
+ self.waitUntilSettled()
+
+ # The changes for the job from project4 should include the project3
+ # PR contet
+ changes = self.getJobFromHistory(
+ 'project4-test', 'org/project4').changes
+
+ self.assertEqual(changes, "%s,%s %s,%s" % (A.number,
+ A.head_sha,
+ B.number,
+ B.head_sha))
+
+ self.assertTrue(A.is_merged)
+ self.assertTrue(B.is_merged)
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index ba8e497..a19073c 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -320,6 +320,46 @@
self.assertThat(report_status['url'][len(base):],
MatchesRegex('^[a-fA-F0-9]{32}\/$'))
+ @simple_layout('layouts/reporting-github.yaml', driver='github')
+ def test_push_reporting(self):
+ project = 'org/project2'
+ # pipeline reports pull status both on start and success
+ self.executor_server.hold_jobs_in_build = True
+ pevent = self.fake_github.getPushEvent(project=project,
+ ref='refs/heads/master')
+
+ self.fake_github.emitEvent(pevent)
+ self.waitUntilSettled()
+
+ # there should only be one report, a status
+ self.assertEqual(1, len(self.fake_github.reports))
+ # Verify the user/context/state of the status
+ status = ('zuul', 'tenant-one/push-reporting', 'pending')
+ self.assertEqual(status, self.fake_github.reports[0][-1])
+
+ # free the executor, allow the build to finish
+ self.executor_server.hold_jobs_in_build = False
+ self.executor_server.release()
+ self.waitUntilSettled()
+
+ # Now there should be a second report, the success of the build
+ self.assertEqual(2, len(self.fake_github.reports))
+ # Verify the user/context/state of the status
+ status = ('zuul', 'tenant-one/push-reporting', 'success')
+ self.assertEqual(status, self.fake_github.reports[-1][-1])
+
+ # now make a PR which should also comment
+ self.executor_server.hold_jobs_in_build = True
+ A = self.fake_github.openFakePullRequest(project, 'master', 'A')
+ self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+ self.waitUntilSettled()
+
+ # Now there should be a four reports, a new comment
+ # and status
+ self.assertEqual(4, len(self.fake_github.reports))
+ self.executor_server.release()
+ self.waitUntilSettled()
+
@simple_layout('layouts/merging-github.yaml', driver='github')
def test_report_pull_merge(self):
# pipeline merges the pull request on success
diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py
index 2e5d49a..838cba5 100644
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -18,6 +18,7 @@
import hmac
import hashlib
import time
+import re
import cachecontrol
from cachecontrol.cache import DictCache
@@ -72,7 +73,10 @@
self._validate_signature(request)
- self.__dispatch_event(request)
+ try:
+ self.__dispatch_event(request)
+ except:
+ self.log.exception("Exception handling Github event:")
def __dispatch_event(self, request):
try:
@@ -172,6 +176,8 @@
elif action == 'unlabeled':
event.action = 'unlabeled'
event.label = body['label']['name']
+ elif action == 'edited':
+ event.action = 'edited'
else:
return None
@@ -353,6 +359,12 @@
DictCache(),
cache_etags=True)
+ # The regex is based on the connection host. We do not yet support
+ # cross-connection dependency gathering
+ self.depends_on_re = re.compile(
+ r"^Depends-On: https://%s/.+/.+/pull/[0-9]+$" % self.git_host,
+ re.MULTILINE | re.IGNORECASE)
+
def onLoad(self):
webhook_listener = GithubWebhookListener(self)
self.registerHttpHandler(self.payload_path,
@@ -476,8 +488,6 @@
if event.change_number:
change = self._getChange(project, event.change_number,
event.patch_number, refresh=refresh)
- change.refspec = event.refspec
- change.branch = event.branch
change.url = event.change_url
change.updated_at = self._ghTimestampToDate(event.updated_at)
change.source_event = event
@@ -495,8 +505,9 @@
change = Ref(project)
return change
- def _getChange(self, project, number, patchset, refresh=False):
- key = '%s/%s/%s' % (project.name, number, patchset)
+ def _getChange(self, project, number, patchset=None, refresh=False,
+ history=None):
+ key = (project.name, number, patchset)
change = self._change_cache.get(key)
if change and not refresh:
return change
@@ -507,24 +518,122 @@
change.patchset = patchset
self._change_cache[key] = change
try:
- self._updateChange(change)
+ self._updateChange(change, history)
except Exception:
if key in self._change_cache:
del self._change_cache[key]
raise
return change
- def _updateChange(self, change):
+ def _getDependsOnFromPR(self, body):
+ prs = []
+ seen = set()
+
+ for match in self.depends_on_re.findall(body):
+ if match in seen:
+ self.log.debug("Ignoring duplicate Depends-On: %s" % (match,))
+ continue
+ seen.add(match)
+ # Get the github url
+ url = match.rsplit()[-1]
+ # break it into the parts we need
+ _, org, proj, _, num = url.rsplit('/', 4)
+ # Get a pull object so we can get the head sha
+ pull = self.getPull('%s/%s' % (org, proj), int(num))
+ prs.append(pull)
+
+ return prs
+
+ def _getNeededByFromPR(self, change):
+ prs = []
+ seen = set()
+ # This shouldn't return duplicate issues, but code as if it could
+
+ # This leaves off the protocol, but looks for the specific GitHub
+ # hostname, the org/project, and the pull request number.
+ pattern = 'Depends-On %s/%s/pull/%s' % (self.git_host,
+ change.project.name,
+ change.number)
+ query = '%s type:pr is:open in:body' % pattern
+ github = self.getGithubClient()
+ for issue in github.search_issues(query=query):
+ pr = issue.issue.pull_request().as_dict()
+ if not pr.get('url'):
+ continue
+ if issue in seen:
+ continue
+ # the issue provides no good description of the project :\
+ org, proj, _, num = pr.get('url').split('/')[-4:]
+ self.log.debug("Found PR %s/%s/%s needs %s/%s" %
+ (org, proj, num, change.project.name,
+ change.number))
+ prs.append(pr)
+ seen.add(issue)
+
+ log_rate_limit(self.log, github)
+ return prs
+
+ def _updateChange(self, change, history=None):
+
+ # If this change is already in the history, we have a cyclic
+ # dependency loop and we do not need to update again, since it
+ # was done in a previous frame.
+ if history and (change.project.name, change.number) in history:
+ return change
+
self.log.info("Updating %s" % (change,))
change.pr = self.getPull(change.project.name, change.number)
+ change.refspec = "refs/pull/%s/head" % change.number
+ change.branch = change.pr.get('base').get('ref')
change.files = change.pr.get('files')
change.title = change.pr.get('title')
change.open = change.pr.get('state') == 'open'
+ change.is_merged = change.pr.get('merged')
change.status = self._get_statuses(change.project,
change.patchset)
change.reviews = self.getPullReviews(change.project,
change.number)
change.labels = change.pr.get('labels')
+ change.body = change.pr.get('body')
+
+ if history is None:
+ history = []
+ else:
+ history = history[:]
+ history.append((change.project.name, change.number))
+
+ needs_changes = []
+
+ # Get all the PRs this may depend on
+ for pr in self._getDependsOnFromPR(change.body):
+ proj = pr.get('base').get('repo').get('full_name')
+ pull = pr.get('number')
+ self.log.debug("Updating %s: Getting dependent "
+ "pull request %s/%s" %
+ (change, proj, pull))
+ project = self.source.getProject(proj)
+ dep = self._getChange(project, pull,
+ patchset=pr.get('head').get('sha'),
+ history=history)
+ if (not dep.is_merged) and dep not in needs_changes:
+ needs_changes.append(dep)
+
+ change.needs_changes = needs_changes
+
+ needed_by_changes = []
+ for pr in self._getNeededByFromPR(change):
+ proj = pr.get('base').get('repo').get('full_name')
+ pull = pr.get('number')
+ self.log.debug("Updating %s: Getting needed "
+ "pull request %s/%s" %
+ (change, proj, pull))
+ project = self.source.getProject(proj)
+ dep = self._getChange(project, pull,
+ patchset=pr.get('head').get('sha'),
+ history=history)
+ if not dep.is_merged:
+ needed_by_changes.append(dep)
+ change.needed_by_changes = needed_by_changes
return change
diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py
index 37cbe61..72087bf 100644
--- a/zuul/driver/github/githubreporter.py
+++ b/zuul/driver/github/githubreporter.py
@@ -43,10 +43,13 @@
"""Report on an event."""
# order is important for github branch protection.
# A status should be set before a merge attempt
- if (self._commit_status is not None and
- hasattr(item.change, 'patchset') and
- item.change.patchset is not None):
- self.setCommitStatus(item)
+ if self._commit_status is not None:
+ if (hasattr(item.change, 'patchset') and
+ item.change.patchset is not None):
+ self.setCommitStatus(item)
+ elif (hasattr(item.change, 'newrev') and
+ item.change.newrev is not None):
+ self.setCommitStatus(item)
# Comments, labels, and merges can only be performed on pull requests.
# If the change is not a pull request (e.g. a push) skip them.
if hasattr(item.change, 'number'):
@@ -71,7 +74,10 @@
def setCommitStatus(self, item):
project = item.change.project.name
- sha = item.change.patchset
+ if hasattr(item.change, 'patchset'):
+ sha = item.change.patchset
+ elif hasattr(item.change, 'newrev'):
+ sha = item.change.newrev
context = '%s/%s' % (item.pipeline.layout.tenant.name,
item.pipeline.name)
state = self._commit_status
@@ -139,7 +145,7 @@
if change.title:
message += change.title
- account = change.source_event.account
+ account = getattr(change.source_event, 'account', None)
if not account:
return message