Merge "Implement Depends-On for github" into feature/zuulv3

tests/base.py

@@ -554,7 +554,7 @@ class FakeGithubPullRequest(object):
 
     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 @@ class FakeGithubPullRequest(object):
         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 @@ class FakeGithubPullRequest(object):
     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 @@ class FakeGithubPullRequest(object):
         }
         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 @@ class FakeGithubPullRequest(object):
                     'repo': {
                         'full_name': self.project
                     }
-                }
+                },
+                'body': self.body
             },
             'sender': {
                 'login': 'ghuser'
@@ -870,11 +879,12 @@ class FakeGithubConnection(githubconnection.GithubConnection):
         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
 
@@ -935,7 +945,9 @@ class FakeGithubConnection(githubconnection.GithubConnection):
                 }
             },
             'files': pr.files,
-            'labels': pr.labels
+            'labels': pr.labels,
+            'merged': pr.is_merged,
+            'body': pr.body
         }
         return data
 
@@ -1033,6 +1045,19 @@ class FakeGithubConnection(githubconnection.GithubConnection):
         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):

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

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)

zuul/driver/github/githubconnection.py

@@ -18,6 +18,7 @@ import logging
 import hmac
 import hashlib
 import time
+import re
 
 import cachecontrol
 from cachecontrol.cache import DictCache
@@ -72,7 +73,10 @@ class GithubWebhookListener():
 
         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 @@ class GithubWebhookListener():
         elif action == 'unlabeled':
             event.action = 'unlabeled'
             event.label = body['label']['name']
+        elif action == 'edited':
+            event.action = 'edited'
         else:
             return None
 
@@ -353,6 +359,12 @@ class GithubConnection(BaseConnection):
             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 @@ class GithubConnection(BaseConnection):
         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 @@ class GithubConnection(BaseConnection):
             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 @@ class GithubConnection(BaseConnection):
             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
 

zuul/driver/github/githubreporter.py

@@ -145,7 +145,7 @@ class GithubReporter(BaseReporter):
         if change.title:
             message += change.title
 
-        account = change.source_event.account
+        account = getattr(change.source_event, 'account', None)
         if not account:
             return message