diff --git a/doc/source/reference/drivers/github.rst b/doc/source/reference/drivers/github.rst index 925b06d300..e5811cfab1 100644 --- a/doc/source/reference/drivers/github.rst +++ b/doc/source/reference/drivers/github.rst @@ -59,6 +59,7 @@ To create a `GitHub application * Set permissions: * Repository administration: Read + * Checks: Read & Write * Repository contents: Read & Write (write to let zuul merge change) * Issues: Read & Write * Pull requests: Read & Write @@ -66,6 +67,7 @@ To create a `GitHub application * Set events subscription: + * Check run * Commit comment * Create * Push @@ -203,6 +205,8 @@ the following options. .. value:: push + .. value:: check_run + .. attr:: action A :value:`pipeline.trigger..event.pull_request` @@ -254,6 +258,18 @@ the following options. Pull request review removed. + A :value:`pipeline.trigger..event.check_run` + event will have associated action(s) to trigger from. The + supported actions are: + + .. value:: requested + + A check run is requested. + + .. value:: completed + + A check run completed. + .. attr:: branch The branch associated with the event. Example: ``master``. This @@ -295,6 +311,23 @@ the following options. format of ``user:context:status``. For example, ``zuul_github_ci_bot:check_pipeline:success``. + .. attr: check + + This is only used for ``check_run`` events. It works similar to + the ``status`` attribute and accepts a list of strings each of + which matches the app requesting or updating the check run, the + check run's name and the conclusion in the format of + ``app:name::conclusion``. + To make Zuul properly interact with Github's checks API, each + pipeline that is using the checks API should have at least one + trigger that matches the pipeline's name regardless of the result, + e.g. ``zuul:cool-pipeline:.*``. This will enable the cool-pipeline + to trigger whenever a user requests the ``cool-pipeline`` check + run as part of the ``zuul`` check suite. + Additionally, one could use ``.*:success`` to trigger a pipeline + whenever a successful check run is reported (e.g. useful for + gating). + .. attr:: ref This is only used for ``push`` events. This field is treated as @@ -330,6 +363,12 @@ itself. Status name, description, and context is taken from the pipeline. status. Defaults to the zuul server status_url, or the empty string if that is unset. + .. attr:: check + + If the reporter should utilize github's checks API to set the commit + status, this must be set to ``in_progress``, ``success`` or ``failure`` + (depending on which status the reporter should report). + .. attr:: comment :default: true diff --git a/releasenotes/notes/github-checks-api-a990a473c37b4db4.yaml b/releasenotes/notes/github-checks-api-a990a473c37b4db4.yaml new file mode 100644 index 0000000000..de7c686166 --- /dev/null +++ b/releasenotes/notes/github-checks-api-a990a473c37b4db4.yaml @@ -0,0 +1,12 @@ +--- +features: + - | + The Github driver now has a basic support for the Github checks API. + To enable reporting build results via the checks API one can configure the + the new :attr:`pipeline...check` attribute on the + Github reporter. It's also possible to trigger on a requested or completed + :value:`pipeline.trigger..event.check_run`. + + To be able to use the checks API, zuul must be authenticated as Github + app. For more information about the necessary requirements, please see + the :ref:`github_driver` driver documentation. diff --git a/tests/base.py b/tests/base.py index b86a60d5a0..19bae11118 100644 --- a/tests/base.py +++ b/tests/base.py @@ -2089,6 +2089,23 @@ class FakeGithubPullRequest(object): } return (name, data) + def getCheckRunRequestedEvent(self, cr_name, app="zuul"): + name = "check_run" + data = { + "action": "rerequested", + "check_run": { + "head_sha": self.head_sha, + "name": cr_name, + "app": { + "slug": app, + }, + }, + "repository": { + "full_name": self.project, + }, + } + return (name, data) + def setMerged(self, commit_message): self.is_merged = True self.merge_message = commit_message diff --git a/tests/fakegithub.py b/tests/fakegithub.py index b085169bc9..f3ab7e69bc 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -64,6 +64,47 @@ class FakeStatus(object): } +class FakeCheckRun(object): + def __init__(self, name, details_url, output, status, conclusion, + completed_at, app): + self.name = name + self.details_url = details_url + self.output = output + self.conclusion = conclusion + self.completed_at = completed_at + self.app = app + + # Github automatically sets the status to "completed" if a conclusion + # is provided. + if conclusion is not None: + self.status = "completed" + else: + self.status = status + + def as_dict(self): + return { + "name": self.name, + "status": self.status, + "output": self.output, + "details_url": self.details_url, + "conclusion": self.conclusion, + "completed_at": self.completed_at, + "app": { + "slug": self.app, + }, + } + + def update(self, conclusion, completed_at, output, details_url): + self.conclusion = conclusion + self.completed_at = completed_at + self.output = output + self.details_url = details_url + + # As we are only calling the update method when a build is completed, + # we can always set the status to "completed". + self.status = "completed" + + class FakeGHReview(object): def __init__(self, data): @@ -83,6 +124,7 @@ class FakeCommit(object): def __init__(self, sha): self._statuses = [] self.sha = sha + self._check_runs = [] def set_status(self, state, url, description, context, user): status = FakeStatus( @@ -91,14 +133,29 @@ class FakeCommit(object): # the last status provided for a commit. self._statuses.insert(0, status) + def set_check_run(self, name, details_url, output, status, conclusion, + completed_at, app): + check_run = FakeCheckRun( + name, details_url, output, status, conclusion, completed_at, app) + # Always insert a check_run to the front of the list to represent the + # last check_run provided for a commit. + self._check_runs.insert(0, check_run) + def get_url(self, path, params=None): if path == 'statuses': statuses = [s.as_dict() for s in self._statuses] return FakeResponse(statuses) + if path == "check-runs": + check_runs = [c.as_dict() for c in self._check_runs] + resp = {"total_count": len(check_runs), "check_runs": check_runs} + return FakeResponse(resp) def statuses(self): return self._statuses + def check_runs(self): + return self._check_runs + def status(self): ''' Returns the combined status wich only contains the latest statuses of @@ -121,6 +178,15 @@ class FakeRepository(object): self.data = data self.name = name + # Simple dictionary to store permission values per feature (e.g. + # checks, Repository contents, Pull requests, Commit statuses, ...). + # Could be used to just enable/deable a permission (True, False) or + # provide more specific values like "read" or "read&write". The mocked + # functionality in the FakeRepository class should then check for this + # value and raise an appropriate exception like a production Github + # would do in case the permission is not sufficient or missing at all. + self._permissions = {} + # fail the next commit requests with 404 self.fail_not_found = 0 @@ -136,6 +202,13 @@ class FakeRepository(object): branch.protected = protected return + def _set_permission(self, key, value): + # NOTE (felix): Currently, this is only used to mock a repo with + # missing checks API permissions. But we could also use it to test + # arbitrary permission values like missing write, but only read + # permissions for a specific functionality. + self._permissions[key] = value + def _build_url(self, *args, **kwargs): path_args = ['repos', self.name] path_args.extend(args) @@ -162,6 +235,26 @@ class FakeRepository(object): self._commits[sha] = commit commit.set_status(state, url, description, context, user) + def create_check_run(self, head_sha, name, details_url=None, output=None, + status=None, conclusion=None, completed_at=None, + app="zuul"): + + # Raise the appropriate github3 exception in case we don't have + # permission to access the checks API + if self._permissions.get("checks") is False: + # To create a proper github3 exception, we need to mock a response + # object + raise github3.exceptions.ForbiddenError( + FakeResponse("Resource not accessible by integration", 403) + ) + + commit = self._commits.get(head_sha, None) + if commit is None: + commit = FakeCommit(head_sha) + self._commits[head_sha] = commit + commit.set_check_run( + name, details_url, output, status, conclusion, completed_at, app) + def commit(self, sha): if self.fail_not_found > 0: @@ -410,6 +503,12 @@ class FakeResponse(object): self.data = data self.links = {} + @property + def content(self): + # Building github3 exceptions requires a Response object with the + # content attribute set. + return self.data + def json(self): return self.data diff --git a/tests/fixtures/layouts/reporting-github.yaml b/tests/fixtures/layouts/reporting-github.yaml index c909cf44b4..0333d037a9 100644 --- a/tests/fixtures/layouts/reporting-github.yaml +++ b/tests/fixtures/layouts/reporting-github.yaml @@ -78,6 +78,25 @@ comment: true status: failure +- pipeline: + name: checks-api-reporting + description: Reporting via Githubs Checks API + manager: independent + trigger: + github: + - event: push + - event: pull_request + action: opened + start: + github: + check: in_progress + success: + github: + check: success + failure: + github: + check: failure + - job: name: base parent: null @@ -104,3 +123,9 @@ push-reporting: jobs: - project-test1 + +- project: + name: org/project3 + checks-api-reporting: + jobs: + - project-test1 diff --git a/tests/fixtures/layouts/requirements-github.yaml b/tests/fixtures/layouts/requirements-github.yaml index 30ed44476f..f5fa0f5de1 100644 --- a/tests/fixtures/layouts/requirements-github.yaml +++ b/tests/fixtures/layouts/requirements-github.yaml @@ -75,6 +75,19 @@ github: comment: true +- pipeline: + name: trigger_check_run + manager: independent + trigger: + github: + - event: check_run + action: requested + check: zuul:tenant-one/check:.* + success: + github: + comment: true + check: success + - pipeline: name: trigger manager: independent @@ -350,6 +363,10 @@ name: project14-current run: playbooks/project14-current.yaml +- job: + name: project15-check-run + run: playbooks/project15-check-run.yaml + - project: name: org/project1 pipeline: @@ -442,3 +459,9 @@ reject_current: jobs: - project14-current + +- project: + name: org/project15 + trigger_check_run: + jobs: + - project15-check-run diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 1748b665c8..3a9cbd0cac 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -26,7 +26,8 @@ import github3.exceptions from zuul.driver.github.githubconnection import GithubShaCache import zuul.rpcclient -from tests.base import BaseTestCase, ZuulTestCase, simple_layout, random_sha1 +from tests.base import (BaseTestCase, ZuulGithubAppTestCase, ZuulTestCase, + simple_layout, random_sha1) from tests.base import ZuulWebFixture @@ -576,6 +577,37 @@ class TestGithubDriver(ZuulTestCase): self.executor_server.release() self.waitUntilSettled() + @simple_layout("layouts/reporting-github.yaml", driver="github") + def test_reporting_checks_api_unauthorized(self): + # Using the checks API only works with app authentication. As all tests + # within the TestGithubDriver class are executed without app + # authentication, the checks API won't work here. + + project = "org/project3" + github = self.fake_github.getGithubClient(None) + + # The pipeline reports pull request status both on start and success. + # As we are not authenticated as app, this won't create or update any + # check runs, but should post two comments (start, success) informing + # the user about the missing authentication. + A = self.fake_github.openFakePullRequest(project, "master", "A") + self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) + self.waitUntilSettled() + + self.assertIn( + A.head_sha, github.repo_from_project(project)._commits.keys() + ) + check_runs = self.fake_github.getCommitChecks(project, A.head_sha) + self.assertEqual(0, len(check_runs)) + + expected_warning = ( + "Unable to create or update check tenant-one/checks-api-reporting." + " Must be authenticated as app integration." + ) + self.assertEqual(2, len(A.comments)) + self.assertIn(expected_warning, A.comments[0]) + self.assertIn(expected_warning, A.comments[1]) + @simple_layout('layouts/merging-github.yaml', driver='github') def test_report_pull_merge(self): # pipeline merges the pull request on success @@ -1554,3 +1586,128 @@ class TestGithubShaCache(BaseTestCase): } cache.update('foo/bar', pr_dict) self.assertEqual(cache.get('foo/bar', '123456'), set({1})) + + +class TestGithubAppDriver(ZuulGithubAppTestCase): + """Inheriting from ZuulGithubAppTestCase will enable app authentication""" + config_file = 'zuul-github-driver.conf' + + @simple_layout("layouts/reporting-github.yaml", driver="github") + def test_reporting_checks_api(self): + """Using the checks API only works with app authentication""" + project = "org/project3" + github = self.fake_github.getGithubClient(None) + + # pipeline reports pull request 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()) + self.waitUntilSettled() + + # We should have a pending check for the head sha + self.assertIn( + A.head_sha, github.repo_from_project(project)._commits.keys()) + 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.assertThat( + check_run["output"]["summary"], + MatchesRegex(r'.*Starting checks-api-reporting jobs.*', re.DOTALL) + ) + + # TODO (felix): How can we test if the details_url was set correctly? + # How can the details_url be configured on the test case? + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + # We should now have an updated status for the head sha + 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("completed", check_run["status"]) + self.assertEqual("success", check_run["conclusion"]) + self.assertThat( + check_run["output"]["summary"], + MatchesRegex(r'.*Build succeeded.*', re.DOTALL) + ) + self.assertIsNotNone(check_run["completed_at"]) + + @simple_layout("layouts/reporting-github.yaml", driver="github") + def test_update_non_existing_check_run(self): + project = "org/project3" + github = self.fake_github.getGithubClient(None) + + # pipeline reports pull request 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()) + self.waitUntilSettled() + + # We should have a pending check for the head sha + commit = github.repo_from_project(project)._commits.get(A.head_sha) + check_runs = commit.check_runs() + self.assertEqual(1, len(check_runs)) + + # Delete this check_run to simulate a failed check_run creation + commit._check_runs = [] + + # Now run the build and check if the update of the check_run could + # still be accomplished. + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + 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("completed", check_run["status"]) + self.assertEqual("success", check_run["conclusion"]) + self.assertThat( + check_run["output"]["summary"], + MatchesRegex(r'.*Build succeeded.*', re.DOTALL) + ) + self.assertIsNotNone(check_run["completed_at"]) + + @simple_layout("layouts/reporting-github.yaml", driver="github") + def test_update_check_run_missing_permissions(self): + project = "org/project3" + github = self.fake_github.getGithubClient(None) + + repo = github.repo_from_project(project) + repo._set_permission("checks", False) + + A = self.fake_github.openFakePullRequest(project, "master", "A") + self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) + self.waitUntilSettled() + + # Alghough we are authenticated as github app, we are lacking the + # necessary "checks" permissions for the test repository. Thus, the + # check run creation/update should fail and we end up in two comments + # being posted to the PR with appropriate warnings. + commit = github.repo_from_project(project)._commits.get(A.head_sha) + check_runs = commit.check_runs() + self.assertEqual(0, len(check_runs)) + + self.assertIn( + A.head_sha, github.repo_from_project(project)._commits.keys() + ) + check_runs = self.fake_github.getCommitChecks(project, A.head_sha) + self.assertEqual(0, len(check_runs)) + + expected_warning = ( + "Failed to update check run tenant-one/checks-api-reporting: " + "403 Resource not accessible by integration" + ) + self.assertEqual(2, len(A.comments)) + self.assertIn(expected_warning, A.comments[0]) + self.assertIn(expected_warning, A.comments[1]) diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py index c833f6dd76..461df55dac 100644 --- a/tests/unit/test_github_requirements.py +++ b/tests/unit/test_github_requirements.py @@ -135,6 +135,37 @@ class TestGithubRequirements(ZuulTestCase): self.assertEqual(len(self.history), 2) self.assertEqual(self.history[1].name, 'project2-trigger') + @simple_layout("layouts/requirements-github.yaml", driver="github") + def test_trigger_on_check_run(self): + """Test trigger on: check_run""" + project = "org/project15" + A = self.fake_github.openFakePullRequest(project, "master", "A") + + # A check_run request with a different name should not cause it to be + # enqueued. + self.fake_github.emitEvent( + A.getCheckRunRequestedEvent("tenant-one/different-check") + ) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # A check_run request with the correct name, but for a different app + # should not cause it to be enqueued. + self.fake_github.emitEvent( + A.getCheckRunRequestedEvent("tenant-one/check", app="other-ci") + ) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # A check_run request with the correct name for the correct app should + # cause it to be enqueued. + self.fake_github.emitEvent( + A.getCheckRunRequestedEvent("tenant-one/check")) + + self.waitUntilSettled() + self.assertEqual(len(self.history), 1) + self.assertEqual(self.history[0].name, "project15-check-run") + @simple_layout('layouts/requirements-github.yaml', driver='github') def test_pipeline_require_review_username(self): "Test pipeline requirement: review username" diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index ffa8519910..0bb1808d79 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -552,6 +552,58 @@ class GithubEventProcessor(object): event.status = "%s:%s:%s" % _status_as_tuple(self.body) return event + def _event_check_run(self): + """Handles check_run requests. + + This maps to the "Re-run" action on a check run and the "Re-run failed + checks" on a check suite in Github. + + This event should be handled similar to a PR commnent or a push. + """ + action = self.body.get("action") + + # NOTE (felix): We could also handle "requested" events here, which are + # 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"]: + return + + # The head_sha identifies the commit the check_run is requested for + # (similar to Github's status API). + check_run = self.body.get("check_run") + if not check_run: + # This shouldn't happen but in case something went wrong it should + # also not cause an exception in the event handling + return + + project = self.body.get("repository", {}).get("full_name") + head_sha = check_run.get("head_sha") + + # Zuul will only accept Github changes that are part of a PR, thus we + # must look up the PR first. + pr_body = self.connection.getPullBySha( + head_sha, project, self.zuul_event_id) + if pr_body is None: + self.log.debug( + "Could not find appropriate PR for SHA %s. " + "Skipping check_run event", + head_sha + ) + return + + # 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 + return event + def _issue_to_pull_request(self, body): number = body.get('issue').get('number') project_name = body.get('repository').get('full_name') @@ -1628,6 +1680,17 @@ class GithubConnection(BaseConnection): successful = set([s.context for s in commit.status().statuses if s.state == 'success']) + if self.app_id: + try: + # Required contexts can be fulfilled by statuses or check runs. + successful.update([cr.name for cr in commit.check_runs() + if cr.conclusion == 'success']) + except github3.exceptions.GitHubException as exc: + self.log.error( + "Unable to retrieve check runs for commit %s: %s", + commit.sha, str(exc) + ) + # Required contexts must be a subset of the successful contexts as # we allow additional successful status contexts we don't care about. return required_contexts.issubset(successful) @@ -1723,6 +1786,29 @@ class GithubConnection(BaseConnection): log.debug("Set commit status to %s for sha %s on %s", state, sha, project) + def getCommitChecks(self, project_name, sha, zuul_event_id=None): + log = get_annotated_logger(self.log, zuul_event_id) + if not self.app_id: + log.debug( + "Not authenticated as Github app. Unable to retrieve commit " + "checks for sha %s on %s", + sha, project_name + ) + return [] + + github = self.getGithubClient( + project_name, zuul_event_id=zuul_event_id + ) + url = github.session.build_url( + "repos", project_name, "commits", sha, "check-runs") + headers = {'Accept': 'application/vnd.github.antiope-preview+json'} + params = {"per_page": 100} + resp = github.session.get(url, params=params, headers=headers) + resp.raise_for_status() + + log.debug("Got commit checks for sha %s on %s", sha, project_name) + return resp.json().get("check_runs", []) + def reviewPull(self, project, pr_number, sha, review, body, zuul_event_id=None): github = self.getGithubClient(project, zuul_event_id=zuul_event_id) @@ -1748,6 +1834,144 @@ class GithubConnection(BaseConnection): pull_request.remove_label(label) 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, 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) + + # Track a list of failed check run operations to report back to Github + errors = [] + + if not self.app_id: + # We don't try to update check runs, if we aren't authenticated as + # Github app at all. If we are, we still have to ensure that we + # don't crash on missing permissions. + log.debug( + "Not authenticated as Github app. Unable to create or update " + "check run '%s' for sha %s on %s", + context, sha, project + ) + + errors.append( + "Unable to create or update check {}. Must be authenticated " + "as app integration.".format( + context + ) + ) + return errors + + output = {"title": "Summary", "summary": message} + + # Currently, the GithubReporter only supports start and end reporting. + # During the build no further update will be reported. + if completed: + # As the buildset itself does not provide a proper end time, we + # use the current time instead. Otherwise, we would have to query + # all builds contained in the buildset and search for the latest + # build.end_time available. + completed_at = datetime.datetime.now(utc).isoformat() + + # When reporting the completion of a check_run, we must set the + # conclusion, as the status will always be "completed". + conclusion = status + + # Unless something went wrong during the start reporting of this + # change (e.g. the check_run creation failed), there should already + # be a check_run available. If not we will create one. + check_runs = [] + try: + check_runs = [ + c for c in repository.commit(sha).check_runs() + if c.name == context + ] + except github3.exceptions.GitHubException as exc: + log.error( + "Could not retrieve existing check runs for %s#%s on " + "sha %s: %s", + project, pr_number, sha, str(exc), + ) + + if not check_runs: + log.debug( + "Could not find check run %s for %s#%s on sha %s. " + "Creating a new one", + context, project, pr_number, sha, + ) + + try: + check_run = repository.create_check_run( + name=context, + head_sha=sha, + conclusion=conclusion, + completed_at=completed_at, + output=output, + details_url=details_url, + ) + except github3.exceptions.GitHubException as exc: + # TODO (felix): Should we retry the check_run creation? + log.error( + "Failed to create check run %s for %s#%s on sha %s: " + "%s", + context, project, pr_number, sha, str(exc) + ) + errors.append( + "Failed to create check run {}: {}".format( + context, str(exc) + ) + ) + else: + check_run = check_runs[0] + log.debug( + "Updating existing check run %s for %s#%s on sha %s " + "with status %s", + context, project, pr_number, sha, status, + ) + + try: + check_run.update( + conclusion=conclusion, + completed_at=completed_at, + output=output, + details_url=details_url, + ) + except github3.exceptions.GitHubException as exc: + log.error( + "Failed to update check run %s for %s#%s on sha %s: " + "%s", + context, project, pr_number, sha, str(exc), + ) + errors.append( + "Failed to update check run {}: {}".format( + context, str(exc) + ) + ) + + else: + # Report the start of a check run + try: + check_run = repository.create_check_run( + name=context, + head_sha=sha, + status=status, + output=output, + details_url=details_url, + ) + except github3.exceptions.GitHubException as exc: + # TODO (felix): Should we retry the check run creation? + log.error( + "Failed to create check run %s for %s#%s on sha %s: %s", + context, project, pr_number, sha, str(exc), + ) + errors.append( + "Failed to update check run {}: {}".format( + context, str(exc) + ) + ) + + return errors + def getPushedFileNames(self, event): files = set() for c in event.commits: @@ -1768,8 +1992,7 @@ class GithubConnection(BaseConnection): # by user, so that we can require/trigger by user too. seen = [] statuses = [] - for status in self.getCommitStatuses( - project.name, sha, event): + for status in self.getCommitStatuses(project.name, sha, event): stuple = _status_as_tuple(status) if "%s:%s" % (stuple[0], stuple[1]) not in seen: statuses.append("%s:%s:%s" % stuple) @@ -1898,3 +2121,18 @@ def _status_as_tuple(status): context = status.get('context') state = status.get('state') return (user, context, state) + + +def _check_as_tuple(check): + """Translate a check into a tuple of app, name, conclusion""" + + # A check_run does not contain any "creator" information like a status, but + # only the app for/by which it was created. + app = check.get("app") + if app: + slug = app.get("slug") + else: + slug = "Unknown" + name = check.get("name") + conclusion = check.get("conclusion") + return (slug, name, conclusion) diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index cc2e7141df..9183a5df13 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -56,6 +56,7 @@ class GithubTriggerEvent(TriggerEvent): self.unlabel = None self.action = None self.delivery = None + self.check_runs = None def isPatchsetCreated(self): if self.type == 'pull_request': @@ -76,6 +77,8 @@ class GithubTriggerEvent(TriggerEvent): r.append('%s,%s' % (self.change_number, self.patch_number)) if self.delivery: r.append('delivery: %s' % self.delivery) + if self.check_runs: + r.append('check_runs: %s' % self.check_runs) return ' '.join(r) @@ -217,7 +220,7 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): def __init__(self, trigger, types=[], branches=[], refs=[], comments=[], actions=[], labels=[], unlabels=[], states=[], statuses=[], required_statuses=[], - ignore_deletes=True): + check_runs=[], ignore_deletes=True): EventFilter.__init__(self, trigger) @@ -237,6 +240,7 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): self.states = states self.statuses = statuses self.required_statuses = required_statuses + self.check_runs = check_runs self.ignore_deletes = ignore_deletes def __repr__(self): @@ -254,6 +258,8 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): ret += ' comments: %s' % ', '.join(self._comments) if self.actions: ret += ' actions: %s' % ', '.join(self.actions) + if self.check_runs: + ret += ' check_runs: %s' % ','.join(self.check_runs) if self.labels: ret += ' labels: %s' % ', '.join(self.labels) if self.unlabels: @@ -320,6 +326,17 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): return FalseWithReason("Actions %s doesn't match %s" % ( self.actions, event.action)) + # check_runs are ORed + if self.check_runs: + check_run_found = False + for check_run in self.check_runs: + if re2.fullmatch(check_run, event.check_run): + check_run_found = True + break + if not check_run_found: + return FalseWithReason("Check_runs %s doesn't match %s" % ( + self.check_runs, event.check_run)) + # labels are ORed if self.labels and event.label not in self.labels: return FalseWithReason("Labels %s doesn't match %s" % ( diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index f63dfceafe..93b209242a 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -42,6 +42,7 @@ class GithubReporter(BaseReporter): super(GithubReporter, self).__init__(driver, connection, config) self._commit_status = self.config.get('status', None) self._create_comment = self.config.get('comment', True) + self._check = self.config.get('check', False) self._merge = self.config.get('merge', False) self._labels = self.config.get('label', []) if not isinstance(self._labels, list): @@ -77,12 +78,22 @@ class GithubReporter(BaseReporter): # 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'): - if self._create_comment: - self.addPullComment(item) + errors_received = False if self._labels or self._unlabels: self.setLabels(item) if self._review: self.addReview(item) + if self._check: + check_errors = self.updateCheck(item) + # TODO (felix): We could use this mechanism to also report back + # errors from label and review actions + if check_errors: + item.current_build_set.warning_messages.extend( + check_errors + ) + errors_received = True + if self._create_comment or errors_received: + self.addPullComment(item) if (self._merge): self.mergePull(item) if not item.change.is_merged: @@ -194,6 +205,38 @@ class GithubReporter(BaseReporter): self.connection.unlabelPull(project, pr_number, label, zuul_event_id=item.event) + def updateCheck(self, item): + log = get_annotated_logger(self.log, item.event) + message = self._formatItemReport(item) + project = item.change.project.name + pr_number = item.change.number + sha = item.change.patchset + + # Check if the buildset is finished or not. In case it's finished, we + # must provide additional parameters when updating the check_run via + # the Github API later on. + completed = item.current_build_set.result is not None + status = self._check + + log.debug( + "Updating check for change %s, params %s, context %s, message: %s", + item.change, self.config, self.context, message + ) + + details_url = item.formatStatusUrl() + + return self.connection.updateCheck( + project, + pr_number, + sha, + status, + completed, + self.context, + details_url, + message, + zuul_event_id=item.event, + ) + def setLabels(self, item): log = get_annotated_logger(self.log, item.event) project = item.change.project.name @@ -244,9 +287,11 @@ class GithubReporter(BaseReporter): this reporter itself is likely to set before submitting. """ - # check if we report a status, if not we can return an empty list + # check if we report a status or a check, if not we can return an + # empty list status = self.config.get('status') - if not status: + check = self.config.get("check") + if not any([status, check]): return [] # we return a status so return the status we report to github @@ -262,6 +307,7 @@ def getSchema(): 'label': scalar_or_list(str), 'unlabel': scalar_or_list(str), 'review': v.Any('approve', 'request-changes', 'comment'), - 'review-body': str + 'review-body': str, + 'check': v.Any("in_progress", "success", "failure"), }) return github_reporter diff --git a/zuul/driver/github/githubtrigger.py b/zuul/driver/github/githubtrigger.py index 328879d9b5..267302b4c2 100644 --- a/zuul/driver/github/githubtrigger.py +++ b/zuul/driver/github/githubtrigger.py @@ -33,6 +33,7 @@ class GithubTrigger(BaseTrigger): branches=to_list(trigger.get('branch')), refs=to_list(trigger.get('ref')), comments=to_list(trigger.get('comment')), + check_runs=to_list(trigger.get('check')), labels=to_list(trigger.get('label')), unlabels=to_list(trigger.get('unlabel')), states=to_list(trigger.get('state')), @@ -52,7 +53,8 @@ def getSchema(): v.Required('event'): scalar_or_list(v.Any('pull_request', 'pull_request_review', - 'push')), + 'push', + 'check_run')), 'action': scalar_or_list(str), 'branch': scalar_or_list(str), 'ref': scalar_or_list(str), @@ -61,7 +63,8 @@ def getSchema(): 'unlabel': scalar_or_list(str), 'state': scalar_or_list(str), 'require-status': scalar_or_list(str), - 'status': scalar_or_list(str) + 'status': scalar_or_list(str), + 'check': scalar_or_list(str), } return github_trigger