Merge "Dequeue changes via github checks API"

releasenotes/notes/github-checks-dequeue-643b7f757ca4d49b.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Changes can now be dequeued via the Github checks API. If a github
+    reporter is configured to use the checks API, all running checks will
+    provide a custom "Abort" action.

tests/base.py

@@ -2215,6 +2215,31 @@ class FakeGithubPullRequest(object):
         }
         return (name, data)
 
+    def getCheckRunAbortEvent(self, check_run):
+        # A check run aborted event can only be created from a FakeCheckRun as
+        # we need some information like external_id which is "calculated"
+        # during the creation of the check run.
+        name = "check_run"
+        data = {
+            "action": "requested_action",
+            "requested_action": {
+                "identifier": "abort",
+            },
+            "check_run": {
+                "head_sha": self.head_sha,
+                "name": check_run["name"],
+                "app": {
+                    "slug": check_run["app"]
+                },
+                "external_id": check_run["external_id"],
+            },
+            "repository": {
+                "full_name": self.project,
+            },
+        }
+
+        return (name, data)
+
     def setMerged(self, commit_message):
         self.is_merged = True
         self.merge_message = commit_message

tests/fakegithub.py

@@ -84,12 +84,17 @@ class FakeStatus(object):
 
 class FakeCheckRun(object):
     def __init__(self, name, details_url, output, status, conclusion,
-                 completed_at, app):
+                 completed_at, external_id, actions, app):
+        if actions is None:
+            actions = []
+
         self.name = name
         self.details_url = details_url
         self.output = output
         self.conclusion = conclusion
         self.completed_at = completed_at
+        self.external_id = external_id
+        self.actions = actions
         self.app = app
 
         # Github automatically sets the status to "completed" if a conclusion
@@ -107,16 +112,21 @@ class FakeCheckRun(object):
             "details_url": self.details_url,
             "conclusion": self.conclusion,
             "completed_at": self.completed_at,
+            "external_id": self.external_id,
+            "actions": self.actions,
             "app": {
                 "slug": self.app,
             },
         }
 
-    def update(self, conclusion, completed_at, output, details_url):
+    def update(self, conclusion, completed_at, output, details_url,
+               external_id, actions):
         self.conclusion = conclusion
         self.completed_at = completed_at
         self.output = output
         self.details_url = details_url
+        self.external_id = external_id
+        self.actions = actions
 
         # As we are only calling the update method when a build is completed,
         # we can always set the status to "completed".
@@ -152,9 +162,17 @@ class FakeCommit(object):
         self._statuses.insert(0, status)
 
     def set_check_run(self, name, details_url, output, status, conclusion,
-                      completed_at, app):
+                      completed_at, external_id, actions, app):
         check_run = FakeCheckRun(
-            name, details_url, output, status, conclusion, completed_at, app
+            name,
+            details_url,
+            output,
+            status,
+            conclusion,
+            completed_at,
+            external_id,
+            actions,
+            app,
         )
         # Always insert a check_run to the front of the list to represent the
         # last check_run provided for a commit.
@@ -264,7 +282,7 @@ class FakeRepository(object):
 
     def create_check_run(self, head_sha, name, details_url=None, output=None,
                          status=None, conclusion=None, completed_at=None,
-                         app="zuul"):
+                         external_id=None, actions=None, app="zuul"):
 
         # Raise the appropriate github3 exception in case we don't have
         # permission to access the checks API
@@ -280,7 +298,16 @@ class FakeRepository(object):
             commit = FakeCommit(head_sha)
             self._commits[head_sha] = commit
         commit.set_check_run(
-            name, details_url, output, status, conclusion, completed_at, app)
+            name,
+            details_url,
+            output,
+            status,
+            conclusion,
+            completed_at,
+            external_id,
+            actions,
+            app,
+        )
 
     def commit(self, sha):
 

tests/unit/test_github_driver.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import json
 import os
 import re
 from testtools.matchers import MatchesRegex, Not, StartsWith
@@ -1796,6 +1797,28 @@ class TestGithubAppDriver(ZuulGithubAppTestCase):
             check_run["output"]["summary"],
             MatchesRegex(r'.*Starting checks-api-reporting jobs.*', re.DOTALL)
         )
+        # The external id should be a json-string containing all relevant
+        # information to uniquely identify this change.
+        self.assertEqual(
+            json.dumps(
+                {
+                    "tenant": "tenant-one",
+                    "pipeline": "checks-api-reporting",
+                    "change": 1
+                }
+            ),
+            check_run["external_id"],
+        )
+        # A running check run should provide a custom abort action
+        self.assertEqual(1, len(check_run["actions"]))
+        self.assertEqual(
+            {
+                "identifier": "abort",
+                "description": "Abort this check run",
+                "label": "Abort",
+            },
+            check_run["actions"][0],
+        )
 
         # TODO (felix): How can we test if the details_url was set correctly?
         # How can the details_url be configured on the test case?
@@ -1817,6 +1840,8 @@ class TestGithubAppDriver(ZuulGithubAppTestCase):
             MatchesRegex(r'.*Build succeeded.*', re.DOTALL)
         )
         self.assertIsNotNone(check_run["completed_at"])
+        # A completed check run should not provide any custom actions
+        self.assertEqual(0, len(check_run["actions"]))
 
         # Tell gate to merge to test checks requirements
         self.fake_github.emitEvent(A.getCommentAddedEvent('merge me'))
@@ -1915,6 +1940,8 @@ class TestGithubAppDriver(ZuulGithubAppTestCase):
             MatchesRegex(r'.*Build succeeded.*', re.DOTALL)
         )
         self.assertIsNotNone(check_run["completed_at"])
+        # A completed check run should not provide any custom actions
+        self.assertEqual(0, len(check_run["actions"]))
 
     @simple_layout("layouts/reporting-github.yaml", driver="github")
     def test_update_check_run_missing_permissions(self):
@@ -1950,6 +1977,65 @@ class TestGithubAppDriver(ZuulGithubAppTestCase):
         self.assertIn(expected_warning, A.comments[0])
         self.assertIn(expected_warning, A.comments[1])
 
+    @simple_layout("layouts/reporting-github.yaml", driver="github")
+    def test_abort_check_run(self):
+        "Test that we can dequeue a change by aborting the related check run"
+        project = "org/project3"
+
+        self.executor_server.hold_jobs_in_build = True
+        A = self.fake_github.openFakePullRequest(project, "master", "A")
+        self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+        self.waitUntilSettled()
+
+        # We should have a pending check for the head sha that provides an
+        # abort action.
+        check_runs = self.fake_github.getCommitChecks(project, A.head_sha)
+
+        self.assertEqual(1, len(check_runs))
+        check_run = check_runs[0]
+        self.assertEqual("tenant-one/checks-api-reporting", check_run["name"])
+        self.assertEqual("in_progress", check_run["status"])
+        self.assertEqual(1, len(check_run["actions"]))
+        self.assertEqual("abort", check_run["actions"][0]["identifier"])
+        self.assertEqual(
+            {
+                "tenant": "tenant-one",
+                "pipeline": "checks-api-reporting",
+                "change": 1
+            },
+            json.loads(check_run["external_id"])
+        )
+
+        # Simulate a click on the "Abort" button in Github by faking a webhook
+        # event with our custom abort action.
+        # Handling this event should dequeue the related change
+        self.fake_github.emitEvent(A.getCheckRunAbortEvent(check_run))
+        self.waitUntilSettled()
+
+        tenant = self.scheds.first.sched.abide.tenants.get("tenant-one")
+        check_pipeline = tenant.layout.pipelines["check"]
+        self.assertEqual(0, len(check_pipeline.getAllItems()))
+        self.assertEqual(1, self.countJobResults(self.history, "ABORTED"))
+
+        # The buildset was already dequeued, so there shouldn't be anything to
+        # release.
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
+        # Since the change/buildset was dequeued, the check run should be
+        # reported as cancelled and don't provide any further action.
+        check_runs = self.fake_github.getCommitChecks(project, A.head_sha)
+        self.assertEqual(1, len(check_runs))
+        aborted_check_run = check_runs[0]
+
+        self.assertEqual(
+            "tenant-one/checks-api-reporting", aborted_check_run["name"]
+        )
+        self.assertEqual("completed", aborted_check_run["status"])
+        self.assertEqual("cancelled", aborted_check_run["conclusion"])
+        self.assertEqual(0, len(aborted_check_run["actions"]))
+
 
 class TestCheckRunAnnotations(ZuulGithubAppTestCase, AnsibleZuulTestCase):
     """We need Github app authentication and be able to run Ansible jobs"""

zuul/driver/github/githubconnection.py

@@ -46,6 +46,7 @@ from zuul.lib.logutil import get_annotated_logger
 from zuul.model import Ref, Branch, Tag, Project
 from zuul.exceptions import MergeFailure
 from zuul.driver.github.githubmodel import PullRequest, GithubTriggerEvent
+from zuul.scheduler import DequeueEvent
 
 GITHUB_BASE_URL = 'https://api.github.com'
 PREVIEW_JSON_ACCEPT = 'application/vnd.github.machine-man-preview+json'
@@ -361,6 +362,8 @@ class GithubEventProcessor(object):
                        self.connection.getEventQueueSize())
         try:
             self._process_event()
+        except Exception:
+            self.log.exception("Exception when processing event:")
         finally:
             self.log.debug("Finished event processing")
         return self.event
@@ -426,7 +429,7 @@ class GithubEventProcessor(object):
                 # If this event references a branch and we're excluding
                 # unprotected branches, we might need to check whether the
                 # branch is now protected.
-                if event.branch:
+                if hasattr(event, "branch") and event.branch:
                     b = self.connection.getBranch(project.name, event.branch)
                     if b is not None:
                         branch_protected = b.get('protected')
@@ -591,7 +594,7 @@ class GithubEventProcessor(object):
         # sent by Github whenever a change is pushed. But as we are already
         # listening to push events, this would result in two trigger events
         # for the same Github event.
-        if action not in ["rerequested", "completed"]:
+        if action not in ["rerequested", "completed", "requested_action"]:
             return
 
         # The head_sha identifies the commit the check_run is requested for
@@ -617,16 +620,55 @@ class GithubEventProcessor(object):
             )
             return
 
-        # Build a trigger event for the check_run request
+        # In case a "requested_action" event was triggered, we must first
+        # evaluate the contained action (identifier), to build the right
+        # event that will e.g. abort/dequeue a buildset.
+        if action == "requested_action":
+            # Look up the action's identifier from the payload
+            identifier = self.body.get("requested_action", {}).get(
+                "identifier"
+            )
+
+            # currently we only support "abort" identifier
+            if identifier != "abort":
+                return
+
+            # In case of an abort (which is currently the only supported
+            # action), we will build a dequeue event and return this rather
+            # than a trigger event.
+            return self._check_run_action_to_event(check_run, project)
+
+        # If no requested_action was supplied, we build a trigger event for the
+        # check run request
         event = self._pull_request_to_event(pr_body)
         event.type = "check_run"
+
         # Simplify rerequested action to requested
         if action == "rerequested":
             action = "requested"
         event.action = action
 
-        check_run = "%s:%s:%s" % _check_as_tuple(self.body["check_run"])
-        event.check_run = check_run
+        check_run_tuple = "%s:%s:%s" % _check_as_tuple(check_run)
+        event.check_run = check_run_tuple
+        return event
+
+    def _check_run_action_to_event(self, check_run, project):
+        # Extract necessary values from the check's external id to dequeue
+        # the corresponding change in Zuul
+        dequeue_attrs = json.loads(check_run["external_id"])
+        # The dequeue operations needs the change in format
+        # <pr_number>,<commit_sha>
+        change = "{},{}".format(dequeue_attrs["change"], check_run["head_sha"])
+
+        # Instead of a trigger event, we directly dequeue the change by calling
+        # the appropriate method on the scheduler.
+        event = DequeueEvent(
+            dequeue_attrs["tenant"],
+            dequeue_attrs["pipeline"],
+            project,
+            change,
+            ref=None
+        )
         return event
 
     def _issue_to_pull_request(self, body):
@@ -1880,12 +1922,16 @@ class GithubConnection(BaseConnection):
         log.debug("Removed label %s from %s#%s", label, proj, pr_number)
 
     def updateCheck(self, project, pr_number, sha, status, completed, context,
-                    details_url, message, file_comments, zuul_event_id=None):
+                    details_url, message, file_comments, external_id,
+                    zuul_event_id=None):
         log = get_annotated_logger(self.log, zuul_event_id)
         github = self.getGithubClient(project, zuul_event_id=zuul_event_id)
         owner, proj = project.split("/")
         repository = github.repository(owner, proj)
 
+        # Always provide an empty list of actions by default
+        actions = []
+
         # Track a list of failed check run operations to report back to Github
         errors = []
 
@@ -1959,6 +2005,8 @@ class GithubConnection(BaseConnection):
                         completed_at=completed_at,
                         output=output,
                         details_url=details_url,
+                        external_id=external_id,
+                        actions=actions,
                     )
                 except github3.exceptions.GitHubException as exc:
                     # TODO (felix): Should we retry the check_run creation?
@@ -1986,6 +2034,8 @@ class GithubConnection(BaseConnection):
                         completed_at=completed_at,
                         output=output,
                         details_url=details_url,
+                        external_id=external_id,
+                        actions=actions,
                     )
                 except github3.exceptions.GitHubException as exc:
                     log.error(
@@ -2000,6 +2050,19 @@ class GithubConnection(BaseConnection):
                     )
 
         else:
+            # Add an abort/dequeue action to running check runs
+            actions.append(
+                {
+                    "label": "Abort",
+                    "description": "Abort this check run",
+                    # Usually Github wants us to provide an identifier for our
+                    # system here, so we can identify this action. But as zuul
+                    # is already identifying this event based on the check run
+                    # this shouldn't be necessary.
+                    "identifier": "abort",
+                }
+            )
+
             # Report the start of a check run
             try:
                 check_run = repository.create_check_run(
@@ -2008,6 +2071,8 @@ class GithubConnection(BaseConnection):
                     status=status,
                     output=output,
                     details_url=details_url,
+                    external_id=external_id,
+                    actions=actions,
                 )
             except github3.exceptions.GitHubException as exc:
                 # TODO (felix): Should we retry the check run creation?

zuul/driver/github/githubreporter.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import json
 import logging
 import voluptuous as v
 import time
@@ -234,6 +235,21 @@ class GithubReporter(BaseReporter):
         # Check for inline comments that can be reported via checks API
         file_comments = self.getFileComments(item)
 
+        # Github allows an external id to be added to a check run. We can use
+        # this to identify the check run in any custom actions we define.
+        # To uniquely identify the corresponding buildset in zuul, we need
+        # tenant, pipeline and change. The buildset's uuid cannot be used
+        # safely, as it might change e.g. during a gate reset. Fore more
+        # information, please see Jim's comment on
+        # https://review.opendev.org/#/c/666258/7
+        external_id = json.dumps(
+            {
+                "tenant": item.pipeline.tenant.name,
+                "pipeline": item.pipeline.name,
+                "change": item.change.number,
+            }
+        )
+
         return self.connection.updateCheck(
             project,
             pr_number,
@@ -244,6 +260,7 @@ class GithubReporter(BaseReporter):
             details_url,
             message,
             file_comments,
+            external_id,
             zuul_event_id=item.event,
         )
 

zuul/scheduler.py

@@ -40,7 +40,7 @@ from zuul.lib.logutil import get_annotated_logger
 from zuul.lib.statsd import get_statsd
 import zuul.lib.queue
 import zuul.lib.repl
-from zuul.model import Build, HoldRequest, Tenant
+from zuul.model import Build, HoldRequest, Tenant, TriggerEvent
 
 COMMANDS = ['full-reconfigure', 'smart-reconfigure', 'stop', 'repl', 'norepl']
 
@@ -456,9 +456,33 @@ class Scheduler(threading.Thread):
         self.statsd.gauge('zuul.executors.jobs_queued', execute_queue)
 
     def addEvent(self, event):
+        # Check the event type and put it in the corresponding queue
+        if isinstance(event, TriggerEvent):
+            return self._addTriggerEvent(event)
+
+        if isinstance(event, ManagementEvent):
+            return self._addManagementEvent(event)
+
+        if isinstance(event, ResultEvent):
+            return self._addResultEvent(event)
+
+        self.log.warning(
+            "Unable to found appropriate queue for event %s", event
+        )
+
+    def _addTriggerEvent(self, event):
         self.trigger_event_queue.put(event)
         self.wake_event.set()
 
+    def _addManagementEvent(self, event):
+        self.management_event_queue.put(event)
+        self.wake_event.set()
+        event.wait()
+
+    def _addResultEvent(self, event):
+        self.result_event_queue.put(event)
+        self.wake_event.set()
+
     def onBuildStarted(self, build):
         build.start_time = time.time()
         event = BuildStartedEvent(build)