diff --git a/doc/source/reference/drivers/github.rst b/doc/source/reference/drivers/github.rst index 474c8ac827..17a745a019 100644 --- a/doc/source/reference/drivers/github.rst +++ b/doc/source/reference/drivers/github.rst @@ -366,8 +366,8 @@ itself. Status name, description, and context is taken from the pipeline. .. 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). + status, this must be set to ``in_progress``, ``success``, ``failure`` + or ``cancelled`` (depending on which status the reporter should report). .. attr:: comment :default: true diff --git a/doc/source/reference/pipeline_def.rst b/doc/source/reference/pipeline_def.rst index e2c2d95874..da13570f74 100644 --- a/doc/source/reference/pipeline_def.rst +++ b/doc/source/reference/pipeline_def.rst @@ -219,6 +219,13 @@ success, the pipeline reports back to Gerrit with ``Verified`` vote of The introductory text in reports when an item is dequeued without running any jobs. Empty by default. + .. attr:: dequeue-message + :default: Build canceled. + + The introductory text in reports when an item is dequeued. + The dequeue message only applies if the item was dequeued without + a result. + .. attr:: footer-message Supplies additional information after test results. Useful for @@ -354,6 +361,12 @@ success, the pipeline reports back to Gerrit with ``Verified`` vote of These reporters describe what Zuul should do when a pipeline is disabled. See ``disable-after-consecutive-failures``. + .. attr:: dequeue + + These reporters describe what Zuul should do if an item is + dequeued. The dequeue reporters will only apply, if the item + was dequeued without a result. + The following options can be used to alter Zuul's behavior to mitigate situations in which jobs are failing frequently (perhaps due to a problem with an external dependency, or unusually high diff --git a/releasenotes/notes/dequeue-reporting-620f364309587304.yaml b/releasenotes/notes/dequeue-reporting-620f364309587304.yaml new file mode 100644 index 0000000000..9cd28ad520 --- /dev/null +++ b/releasenotes/notes/dequeue-reporting-620f364309587304.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Pipelines now provide a :attr:`pipeline.dequeue` reporter action so that + reporters may run whenever an item is dequeued. The dequeue reporters will + only apply if the item wasn't a success or failure. diff --git a/tests/fixtures/layouts/dequeue-reporting.yaml b/tests/fixtures/layouts/dequeue-reporting.yaml new file mode 100644 index 0000000000..52afe9a5ad --- /dev/null +++ b/tests/fixtures/layouts/dequeue-reporting.yaml @@ -0,0 +1,85 @@ +- pipeline: + name: check + manager: independent + failure-message: Build failed (check) + success-message: Build succeeded (check) + dequeue-message: Build canceled (check) + start-message: Build started (check) + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + start: + gerrit: + Verified: 0 + dequeue: + gerrit: + Verified: 0 + +- pipeline: + name: gate + manager: dependent + supercedes: check + failure-message: Build failed (gate) + success-message: Build succeeded (gate) + dequeue-message: Build canceled (gate) + start-message: Build started (gate) + trigger: + gerrit: + - event: comment-added + approval: + - Approved: 1 + success: + gerrit: + Verified: 2 + submit: true + failure: + gerrit: + Verified: -2 + start: + gerrit: + Verified: 0 + dequeue: + gerrit: + Verified: 0 + precedence: high + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: + name: project-test1 + run: playbooks/project-test1.yaml + +- job: + name: project-test2 + run: playbooks/project-test2.yaml + +- job: + name: project-merge + hold-following-changes: true + run: playbooks/project-merge.yaml + +- project: + name: org/project + check: + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge + gate: + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge diff --git a/tests/fixtures/layouts/reporting-github.yaml b/tests/fixtures/layouts/reporting-github.yaml index c35207af12..a9cbf8fcb1 100644 --- a/tests/fixtures/layouts/reporting-github.yaml +++ b/tests/fixtures/layouts/reporting-github.yaml @@ -96,6 +96,9 @@ failure: github: check: failure + dequeue: + github: + check: cancelled - pipeline: name: gate diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 0558c9994d..39c33440a1 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1684,6 +1684,61 @@ class TestGithubAppDriver(ZuulGithubAppTestCase): self.waitUntilSettled() self.assertTrue(A.is_merged) + @simple_layout("layouts/reporting-github.yaml", driver="github") + def test_reporting_checks_api_dequeue(self): + "Test that a dequeued change will be reported back to the check run" + project = "org/project3" + github = self.fake_github.getGithubClient(None) + + client = zuul.rpcclient.RPCClient( + "127.0.0.1", self.gearman_server.port + ) + self.addCleanup(client.shutdown) + + 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) + ) + + # Use the client to dequeue the pending change + client.dequeue( + tenant="tenant-one", + pipeline="checks-api-reporting", + project="org/project3", + change="{},{}".format(A.number, A.head_sha), + ref=None, + ) + 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/checks-api-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.yaml", driver="github") def test_update_non_existing_check_run(self): project = "org/project3" diff --git a/tests/unit/test_reporting.py b/tests/unit/test_reporting.py new file mode 100644 index 0000000000..dab0412440 --- /dev/null +++ b/tests/unit/test_reporting.py @@ -0,0 +1,137 @@ +# Copyright 2020 BMW Group +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import zuul.rpcclient + +from tests.base import ZuulTestCase, simple_layout + + +class TestReporting(ZuulTestCase): + tenant_config_file = "config/single-tenant/main.yaml" + + @simple_layout("layouts/dequeue-reporting.yaml") + def test_dequeue_reporting(self): + """Check that explicitly dequeued items are reported as dequeued""" + + # We use the rpcclient to explicitly dequeue the item + client = zuul.rpcclient.RPCClient( + "127.0.0.1", self.gearman_server.port + ) + self.addCleanup(client.shutdown) + + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange("org/project", "master", "A") + A.addApproval("Code-Review", 2) + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + client.dequeue( + tenant="tenant-one", + pipeline="check", + project="org/project", + change="1,1", + ref=None, + ) + self.waitUntilSettled() + + tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') + check_pipeline = tenant.layout.pipelines['check'] + + # A should have been reported two times: start, cancel + self.assertEqual(2, A.reported) + self.assertEqual(2, len(A.messages)) + self.assertIn("Build started (check)", A.messages[0]) + self.assertIn("Build canceled (check)", A.messages[1]) + # There shouldn't be any successful items + self.assertEqual(len(check_pipeline.getAllItems()), 0) + # But one canceled + self.assertEqual(self.countJobResults(self.history, "ABORTED"), 1) + + @simple_layout("layouts/dequeue-reporting.yaml") + def test_dequeue_reporting_gate_reset(self): + """Check that a gate reset is not reported as dequeued""" + + A = self.fake_gerrit.addFakeChange("org/project", "master", "A") + B = self.fake_gerrit.addFakeChange("org/project", "master", "B") + A.addApproval("Code-Review", 2) + B.addApproval("Code-Review", 2) + + self.executor_server.failJob("project-test1", A) + + self.fake_gerrit.addEvent(A.addApproval("Approved", 1)) + self.fake_gerrit.addEvent(B.addApproval("Approved", 1)) + self.waitUntilSettled() + + # None of the items should be reported as dequeued, only success or + # failure + self.assertEqual(A.data["status"], "NEW") + self.assertEqual(B.data["status"], "MERGED") + self.assertEqual(A.reported, 2) + self.assertEqual(B.reported, 2) + + self.assertIn("Build started (gate)", A.messages[0]) + self.assertIn("Build failed (gate)", A.messages[1]) + self.assertIn("Build started (gate)", B.messages[0]) + self.assertIn("Build succeeded (gate)", B.messages[1]) + + @simple_layout("layouts/dequeue-reporting.yaml") + def test_dequeue_reporting_supercedes(self): + """Test that a superceeded change is reported as dequeued""" + + self.executor_server.hold_jobs_in_build = True + + A = self.fake_gerrit.addFakeChange("org/project", "master", "A") + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + A.addApproval("Code-Review", 2) + self.fake_gerrit.addEvent(A.addApproval("Approved", 1)) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(4, A.reported) + + self.assertIn("Build started (check)", A.messages[0]) + self.assertIn("Build canceled (check)", A.messages[1]) + self.assertIn("Build started (gate)", A.messages[2]) + self.assertIn("Build succeeded (gate)", A.messages[3]) + + @simple_layout("layouts/dequeue-reporting.yaml") + def test_dequeue_reporting_new_patchset(self): + "Test that change superceeded by a new patchset is reported as deqeued" + + self.executor_server.hold_jobs_in_build = True + + A = self.fake_gerrit.addFakeChange("org/project", "master", "A") + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(1, len(self.builds)) + + A.addPatchset() + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(2)) + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(4, A.reported) + + self.assertIn("Build started (check)", A.messages[0]) + self.assertIn("Build canceled (check)", A.messages[1]) + self.assertIn("Build started (check)", A.messages[2]) + self.assertIn("Build succeeded (check)", A.messages[3]) diff --git a/zuul/configloader.py b/zuul/configloader.py index f985804c6e..b999bd0c4f 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1151,6 +1151,7 @@ class PipelineParser(object): 'merge-failure': 'merge_failure_actions', 'no-jobs': 'no_jobs_actions', 'disabled': 'disabled_actions', + 'dequeue': 'dequeue_actions', } def __init__(self, pcontext): @@ -1200,6 +1201,7 @@ class PipelineParser(object): 'merge-failure-message': str, 'no-jobs-message': str, 'footer-message': str, + 'dequeue-message': str, 'dequeue-on-new-patchset': bool, 'ignore-dependencies': bool, 'post-review': bool, @@ -1218,7 +1220,7 @@ class PipelineParser(object): pipeline['reject'] = self.getDriverSchema('reject') pipeline['trigger'] = vs.Required(self.getDriverSchema('trigger')) for action in ['enqueue', 'start', 'success', 'failure', - 'merge-failure', 'no-jobs', 'disabled']: + 'merge-failure', 'no-jobs', 'disabled', 'dequeue']: pipeline[action] = self.getDriverSchema('reporter') return vs.Schema(pipeline) @@ -1247,6 +1249,9 @@ class PipelineParser(object): "Starting {pipeline.name} jobs.") pipeline.enqueue_message = conf.get('enqueue-message', "") pipeline.no_jobs_message = conf.get('no-jobs-message', "") + pipeline.dequeue_message = conf.get( + "dequeue-message", "Build canceled." + ) pipeline.dequeue_on_new_patchset = conf.get( 'dequeue-on-new-patchset', True) pipeline.ignore_dependencies = conf.get( diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index 2c9ccec348..a5a509623e 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -213,11 +213,16 @@ class GithubReporter(BaseReporter): 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 + # We declare a item as completed if it either has a result + # (success|failure) or a dequeue reporter is called (cancelled in case + # of Github checks API). For the latter one, the item might or might + # not have a result, but we still must set a conclusion on the check + # run. Thus, we cannot rely on the buildset's result only, but also + # check the state the reporter is going to report. + completed = ( + item.current_build_set.result is not None or status == "cancelled" + ) log.debug( "Updating check for change %s, params %s, context %s, message: %s", @@ -313,6 +318,6 @@ def getSchema(): 'unlabel': scalar_or_list(str), 'review': v.Any('approve', 'request-changes', 'comment'), 'review-body': str, - 'check': v.Any("in_progress", "success", "failure"), + 'check': v.Any("in_progress", "success", "failure", "cancelled"), }) return github_reporter diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index 469d2c1b38..2c643cf3c4 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -169,6 +169,19 @@ class PipelineManager(metaclass=ABCMeta): self.log.error("Reporting item start %s received: %s" % (item, ret)) + def reportDequeue(self, item): + if not self.pipeline._disabled: + self.log.info( + "Reporting dequeue, action %s item%s", + self.pipeline.dequeue_actions, + item, + ) + ret = self.sendReport(self.pipeline.dequeue_actions, item) + if ret: + self.log.error( + "Reporting item dequeue %s received: %s", item, ret + ) + def sendReport(self, action_reporters, item, message=None): """Sends the built message off to configured reporters. @@ -371,6 +384,12 @@ class PipelineManager(metaclass=ABCMeta): log = get_annotated_logger(self.log, item.event) log.debug("Removing change %s from queue", item.change) item.queue.dequeueItem(item) + # In case a item is dequeued that doesn't have a result yet + # (success/failed/...) we report it as dequeued. + # Without this check, all items with a valid result would be reported + # twice. + if not item.current_build_set.result and item.live: + self.reportDequeue(item) def removeItem(self, item): log = get_annotated_logger(self.log, item.event) diff --git a/zuul/model.py b/zuul/model.py index a5dd8827db..a1e3d3dc4e 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -261,6 +261,7 @@ class Pipeline(object): self.footer_message = None self.enqueue_message = None self.start_message = None + self.dequeue_message = None self.post_review = False self.dequeue_on_new_patchset = True self.ignore_dependencies = False @@ -276,6 +277,7 @@ class Pipeline(object): self.merge_failure_actions = [] self.no_jobs_actions = [] self.disabled_actions = [] + self.dequeue_actions = [] self.disable_at = None self._consecutive_failures = 0 self._disabled = False @@ -295,7 +297,8 @@ class Pipeline(object): self.failure_actions + self.merge_failure_actions + self.no_jobs_actions + - self.disabled_actions + self.disabled_actions + + self.dequeue_actions ) def __repr__(self): diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py index ac5f371d6b..da3c606b41 100644 --- a/zuul/reporter/__init__.py +++ b/zuul/reporter/__init__.py @@ -123,7 +123,8 @@ class BaseReporter(object, metaclass=abc.ABCMeta): 'failure': self._formatItemReportFailure, 'merge-failure': self._formatItemReportMergeFailure, 'no-jobs': self._formatItemReportNoJobs, - 'disabled': self._formatItemReportDisabled + 'disabled': self._formatItemReportDisabled, + 'dequeue': self._formatItemReportDequeue, } return format_methods[self._action] @@ -208,6 +209,12 @@ class BaseReporter(object, metaclass=abc.ABCMeta): else: return self._formatItemReport(item) + def _formatItemReportDequeue(self, item, with_jobs=True): + msg = item.pipeline.dequeue_message + if with_jobs: + msg += '\n\n' + self._formatItemReportJobs(item) + return msg + def _getItemReportJobsFields(self, item): # Extract the report elements from an item config = self.connection.sched.config