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
This commit is contained in:
@@ -518,9 +518,11 @@ itself. Status name, description, and context is taken from the pipeline.
|
||||
Report status via the Github `status API
|
||||
<https://docs.github.com/v3/repos/statuses/>`__. 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.<reporter>.<github source>.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
|
||||
<https://docs.github.com/v3/checks/>`__. 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.<reporter>.<github source>.status`, since this
|
||||
|
||||
@@ -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:
|
||||
|
||||
7
releasenotes/notes/github-queued-be80ca78aceb74ed.yaml
Normal file
7
releasenotes/notes/github-queued-be80ca78aceb74ed.yaml
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
The :attr:`pipeline.<reporter>.<github source>.check` attribute
|
||||
now accepts the value of :value:`pipeline.<reporter>.<github
|
||||
source>.check.queued` to indicate that a pull request has been
|
||||
enqueued into a pipeline.
|
||||
@@ -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'],
|
||||
|
||||
49
tests/fixtures/layouts/reporting-github-enqueue-dequeue.yaml
vendored
Normal file
49
tests/fixtures/layouts/reporting-github-enqueue-dequeue.yaml
vendored
Normal file
@@ -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: []
|
||||
@@ -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"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user