From a738d00fb98a386e79cb2ecd149d1570d66360b8 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 17 Nov 2025 13:30:55 -0800 Subject: [PATCH] GitHub: Add "queued" check status This adds support for the "queued" status in github checks. This allows users to configure an "enqueue" reporter to set the "queued" status on a check for a github PR. Since the "start" reporter doesn't change the status of a check until Zuul starts to run jobs, this can be used to provide near-immediate feedback to users that a change has been enqueued into a pipeline. If the change remains outside the active window for some time, the "queued" status can be useful to let users know that future work is pending. The reference gate pipeline for github is updated to include an "enqueue" reporter, as well as a reporter for both "dequeue" and "no-jobs". These two, along with "success" and "failure", cover all of the ways a change may exit a pipeline and will therefore finalize the check with a completed result (so that it is not left in a pending state in the github web UI). Some minor changes are made to the reference pipelines for consistent formatting. The docs are updated not only to add the new status keyword, but also to switch to using the "value" tag in the "zuul" sphinx domain. This allows us to link to the individual values. To ensure that we don't report "dequeue" in cases where we have not reported anything yet, we add an additional flag to the queue item to storte whether we have reported "enqueue". This is initialized in the constructor, so it will be set for currently existing objects without the need for an explicit upgrade path. The code that sets the reported_start flag is updated slightly so that both reported_start and reported_enqueue are used the same way. Change-Id: I99653a0b31d26724e1728e3e8368015a11f73bff --- doc/source/drivers/github.rst | 27 +- .../pipelines/github-reference-pipelines.yaml | 32 ++- .../notes/github-queued-be80ca78aceb74ed.yaml | 7 + tests/fakegithub.py | 21 +- .../reporting-github-enqueue-dequeue.yaml | 49 ++++ tests/unit/test_github_driver.py | 231 ++++++++++++++++++ zuul/driver/github/githubreporter.py | 2 +- zuul/manager/__init__.py | 14 +- zuul/model.py | 2 + 9 files changed, 352 insertions(+), 33 deletions(-) create mode 100644 releasenotes/notes/github-queued-be80ca78aceb74ed.yaml create mode 100644 tests/fixtures/layouts/reporting-github-enqueue-dequeue.yaml diff --git a/doc/source/drivers/github.rst b/doc/source/drivers/github.rst index a304d0ffe8..9ffa1e4877 100644 --- a/doc/source/drivers/github.rst +++ b/doc/source/drivers/github.rst @@ -518,9 +518,11 @@ itself. Status name, description, and context is taken from the pipeline. Report status via the Github `status API `__. Set to one of - * ``pending`` - * ``success`` - * ``failure`` + .. value:: pending + + .. value:: success + + .. value:: failure This is usually mutually exclusive with a value set in :attr:`pipeline...check`, since this @@ -535,12 +537,19 @@ itself. Status name, description, and context is taken from the pipeline. Report status via the Github `checks API `__. Set to one of - * ``cancelled`` - * ``failure`` - * ``in_progress`` - * ``neutral`` - * ``skipped`` - * ``success`` + .. value:: queued + + .. value:: cancelled + + .. value:: failure + + .. value:: in_progress + + .. value:: neutral + + .. value:: skipped + + .. value:: success This is usually mutually exclusive with a value set in :attr:`pipeline...status`, since this diff --git a/doc/source/examples/pipelines/github-reference-pipelines.yaml b/doc/source/examples/pipelines/github-reference-pipelines.yaml index 453d9556d5..d1be9252e5 100644 --- a/doc/source/examples/pipelines/github-reference-pipelines.yaml +++ b/doc/source/examples/pipelines/github-reference-pipelines.yaml @@ -23,7 +23,7 @@ check: .*/check:.* start: github: - check: 'in_progress' + check: in_progress comment: false # It is recommended to use the checks API for consistency with # other common CI tools that integrate with Github. Results @@ -36,18 +36,18 @@ # sections below. You should choose one or the other # depending on project preferences. # - #status: 'pending' + #status: pending #comment: false success: github: - check: 'success' + check: success comment: false - #status: 'success' + #status: success failure: github: - check: 'failure' + check: failure comment: false - #status: 'failure' + #status: failure dequeue: github: check: cancelled @@ -93,21 +93,29 @@ action: labeled label: - gate + enqueue: + github: + check: queued + comment: false start: github: - check: 'in_progress' + check: in_progress comment: false - #status: 'pending' + #status: pending success: github: - check: 'success' + check: success comment: false - #status: 'success' + #status: success merge: true failure: github: - check: 'failure' - #status: 'failure' + check: failure + #status: failure + comment: false + no-jobs: + github: + check: skipped comment: false dequeue: github: diff --git a/releasenotes/notes/github-queued-be80ca78aceb74ed.yaml b/releasenotes/notes/github-queued-be80ca78aceb74ed.yaml new file mode 100644 index 0000000000..55374d77bc --- /dev/null +++ b/releasenotes/notes/github-queued-be80ca78aceb74ed.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The :attr:`pipeline...check` attribute + now accepts the value of :value:`pipeline...check.queued` to indicate that a pull request has been + enqueued into a pipeline. diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 25047b8f55..9702593b22 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -586,8 +586,9 @@ class FakeCheckRun: }, } - def update(self, conclusion, completed_at, output, details_url, + def update(self, status, conclusion, completed_at, output, details_url, external_id, actions): + self.status = status self.conclusion = conclusion self.completed_at = completed_at self.output = output @@ -595,10 +596,6 @@ class FakeCheckRun: 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". - self.status = "completed" - class FakeGithubReview(object): @@ -1217,8 +1214,18 @@ class FakeGithubSession(object): ] check_run = check_runs[0] - check_run.update(json['conclusion'], - json['completed_at'], + conclusion = json.get('conclusion') + if conclusion: + status = 'completed' + else: + status = json['status'] + completed_at = json.get('completed_at') + if status == 'completed' and not completed_at: + raise Exception( + "Provide completed_at for completed check runs") + check_run.update(status, + conclusion, + completed_at, json['output'], json['details_url'], json['external_id'], diff --git a/tests/fixtures/layouts/reporting-github-enqueue-dequeue.yaml b/tests/fixtures/layouts/reporting-github-enqueue-dequeue.yaml new file mode 100644 index 0000000000..c66343bc56 --- /dev/null +++ b/tests/fixtures/layouts/reporting-github-enqueue-dequeue.yaml @@ -0,0 +1,49 @@ +- pipeline: + name: reporting + description: Reporting via Githubs Checks API + manager: dependent + trigger: + github: + - event: push + - event: pull_request + action: opened + enqueue: + github: + check: queued + start: + github: + check: in_progress + success: + github: + check: success + failure: + github: + check: failure + no-jobs: + github: + check: skipped + dequeue: + github: + check: cancelled + window: 1 + window-floor: 1 + window-ceiling: 1 + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: + name: project-test1 + +- project: + name: org/project + reporting: + jobs: + - project-test1 + +- project: + name: org/project1 + reporting: + jobs: [] diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 6d4ca360e3..d2a5ab5d0f 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -2500,6 +2500,237 @@ class TestGithubAppDriver(ZuulGithubAppTestCase): ) self.assertIsNotNone(check_run["completed_at"]) + @simple_layout("layouts/reporting-github-enqueue-dequeue.yaml", + driver="github") + def test_reporting_checks_api_enqueue(self): + # Test enqueue reporting with normal completion + project = "org/project" + github = self.fake_github.getGithubClient(None) + 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 an in-progress 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/reporting", check_run["name"]) + self.assertEqual("in_progress", check_run["status"]) + self.assertThat( + check_run["output"]["summary"], + MatchesRegex( + r'.*Starting reporting jobs.*', re.DOTALL) + ) + self.assertIsNone(check_run["completed_at"]) + + B = self.fake_github.openFakePullRequest(project, "master", "B") + self.fake_github.emitEvent(B.getPullRequestOpenedEvent()) + self.waitUntilSettled() + + # We should have a queued check for the second change + self.assertIn( + B.head_sha, github.repo_from_project(project)._commits.keys()) + check_runs = self.fake_github.getCommitChecks(project, B.head_sha) + + self.assertEqual(1, len(check_runs)) + check_run = check_runs[0] + + self.assertEqual("tenant-one/reporting", check_run["name"]) + self.assertEqual("queued", check_run["status"]) + self.assertEqual("", check_run["output"]["summary"]) + self.assertIsNone(check_run["completed_at"]) + + # Release the first build + self.release(self.builds[0]) + self.waitUntilSettled() + + # We should now have a completed check run for the first change + 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/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"]) + + # Check that the second build is now in_progress + check_runs = self.fake_github.getCommitChecks(project, B.head_sha) + self.assertEqual(1, len(check_runs)) + check_run = check_runs[0] + + self.assertEqual("tenant-one/reporting", check_run["name"]) + self.assertEqual("in_progress", check_run["status"]) + self.assertThat( + check_run["output"]["summary"], + MatchesRegex( + r'.*Starting reporting jobs.*', re.DOTALL) + ) + self.assertIsNone(check_run["completed_at"]) + + # Release the second build + self.release(self.builds[0]) + self.waitUntilSettled() + + # We should now have a completed check run for the second change + check_runs = self.fake_github.getCommitChecks(project, B.head_sha) + self.assertEqual(1, len(check_runs)) + check_run = check_runs[0] + + self.assertEqual("tenant-one/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-enqueue-dequeue.yaml", + driver="github") + def test_reporting_checks_api_enqueue_dequeue(self): + # Test an enqueue/dequeue reporting pair + project = "org/project" + github = self.fake_github.getGithubClient(None) + 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 an in-progress 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/reporting", check_run["name"]) + self.assertEqual("in_progress", check_run["status"]) + self.assertThat( + check_run["output"]["summary"], + MatchesRegex( + r'.*Starting reporting jobs.*', re.DOTALL) + ) + self.assertIsNone(check_run["completed_at"]) + + B = self.fake_github.openFakePullRequest(project, "master", "B") + self.fake_github.emitEvent(B.getPullRequestOpenedEvent()) + self.waitUntilSettled() + + # We should have a queued check for the second change + self.assertIn( + B.head_sha, github.repo_from_project(project)._commits.keys()) + check_runs = self.fake_github.getCommitChecks(project, B.head_sha) + + self.assertEqual(1, len(check_runs)) + check_run = check_runs[0] + + self.assertEqual("tenant-one/reporting", check_run["name"]) + self.assertEqual("queued", check_run["status"]) + self.assertEqual("", check_run["output"]["summary"]) + self.assertIsNone(check_run["completed_at"]) + + # Dequeue the second change + event = DequeueEvent('tenant-one', 'reporting', + 'github.com', 'org/project', + change='{},{}'.format(B.number, B.head_sha), + ref=None, oldrev=None, newrev=None) + self.scheds.first.sched.pipeline_management_events['tenant-one'][ + 'reporting'].put(event) + self.waitUntilSettled() + + # We should now have a cancelled check run for the head sha + check_runs = self.fake_github.getCommitChecks(project, B.head_sha) + self.assertEqual(1, len(check_runs)) + check_run = check_runs[0] + + self.assertEqual("tenant-one/reporting", check_run["name"]) + self.assertEqual("completed", check_run["status"]) + self.assertEqual("cancelled", check_run["conclusion"]) + self.assertThat( + check_run["output"]["summary"], + MatchesRegex(r'.*Build canceled.*', re.DOTALL) + ) + self.assertIsNotNone(check_run["completed_at"]) + + # Dequeue the first change + event = DequeueEvent('tenant-one', 'reporting', + 'github.com', 'org/project', + change='{},{}'.format(A.number, A.head_sha), + ref=None, oldrev=None, newrev=None) + self.scheds.first.sched.pipeline_management_events['tenant-one'][ + 'reporting'].put(event) + self.waitUntilSettled() + + # We should now have a cancelled check run 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/reporting", check_run["name"]) + self.assertEqual("completed", check_run["status"]) + self.assertEqual("cancelled", check_run["conclusion"]) + self.assertThat( + check_run["output"]["summary"], + MatchesRegex(r'.*Build canceled.*', re.DOTALL) + ) + self.assertIsNotNone(check_run["completed_at"]) + + @simple_layout("layouts/reporting-github-enqueue-dequeue.yaml", + driver="github") + def test_reporting_checks_api_enqueue_without_jobs(self): + # Test an enqueue/dequeue reporting pair if no jobs run + # NOTE: this test does not use the "no-jobs" reporter + project = "org/project1" + github = self.fake_github.getGithubClient(None) + # Hold the merge job so we can observe the queued state + self.hold_merge_jobs_in_queue = True + + A = self.fake_github.openFakePullRequest(project, "master", "A") + self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) + self.waitUntilSettled() + + # We should have an in-progress 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/reporting", check_run["name"]) + self.assertEqual("queued", check_run["status"]) + self.assertEqual("", check_run["output"]["summary"]) + self.assertIsNone(check_run["completed_at"]) + + # Release the merge job + self.hold_merge_jobs_in_queue = False + self.merger_api.release() + self.waitUntilSettled() + + # We should now have a cancelled check run 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/reporting", check_run["name"]) + self.assertEqual("completed", check_run["status"]) + self.assertEqual("skipped", check_run["conclusion"]) + self.assertEqual("", check_run["output"]["summary"]) + 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" diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index 2a3137af53..f18bcf6002 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -420,7 +420,7 @@ def getSchema(): 'review': v.Any('approve', 'request-changes', 'comment'), 'review-body': str, 'check': v.Any( - "in_progress", "success", "failure", "cancelled", + "queued", "in_progress", "success", "failure", "cancelled", "skipped", "neutral"), }) return github_reporter diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index 3cbd95b779..696b203855 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -384,6 +384,9 @@ class PipelineManager(metaclass=ABCMeta): if ret: log.error("Reporting item enqueued %s received: %s" % (item, ret)) + else: + item.updateAttributes(self.current_context, + reported_enqueue=True) def reportStart(self, item): if not self.state.disabled: @@ -393,6 +396,9 @@ class PipelineManager(metaclass=ABCMeta): ret = self.sendReport(self.pipeline.start_actions, item) if ret: log.error("Reporting item start %s received: %s" % (item, ret)) + else: + item.updateAttributes(self.current_context, + reported_start=True) def reportNormalBuildsetEnd(self, build_set, action, final, result=None): # Report a buildset end if there are jobs or errors @@ -1034,10 +1040,12 @@ class PipelineManager(metaclass=ABCMeta): # twice. if not item.current_build_set.result and item.live: item.setReportedResult('DEQUEUED') - if not item.reported_start: + if not (item.reported_enqueue or item.reported_start): # If we haven't reported start, we don't know if Zuul # was supposed to act on the item at all, so keep it - # quiet. + # quiet; but if we reported enqueue, then we're + # probably in a pipeline that has an enqueue/dequeue + # pair, so we should make sure we clean that up. quiet = True self.reportDequeue(item, quiet) item.queue.dequeueItem(item) @@ -1950,8 +1958,6 @@ class PipelineManager(metaclass=ABCMeta): and not item.quiet ): self.reportStart(item) - item.updateAttributes(self.current_context, - reported_start=True) if item.current_build_set.unable_to_merge: failing_reasons.append("it has a merge conflict") if item.current_build_set.has_blocking_errors: diff --git a/zuul/model.py b/zuul/model.py index 0a674d7961..0144721e8e 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -6815,6 +6815,7 @@ class QueueItem(zkobject.ZKObject): dequeue_time=None, first_job_start_time=None, reported=False, + reported_enqueue=False, reported_start=False, quiet=False, active=False, # Whether an item is within an active window @@ -6892,6 +6893,7 @@ class QueueItem(zkobject.ZKObject): "report_time": self.report_time, "dequeue_time": self.dequeue_time, "reported": self.reported, + "reported_enqueue": self.reported_enqueue, "reported_start": self.reported_start, "quiet": self.quiet, "active": self.active,