diff --git a/doc/source/zuul.rst b/doc/source/zuul.rst index c711394b31..72743421f0 100644 --- a/doc/source/zuul.rst +++ b/doc/source/zuul.rst @@ -238,6 +238,12 @@ explanation of each of the parameters:: reported back to Gerrit when at least one voting build fails. Defaults to "Build failed." +**merge-failure-message** + An optional field that supplies the introductory text in message + reported back to Gerrit when a change fails to merge with the + current state of the repository. + Defaults to "Merge failed." + **footer-message** An optional field to supply additional information after test results. Useful for adding information about the CI system such as debugging @@ -413,6 +419,12 @@ explanation of each of the parameters:: Uses the same syntax as **success**, but describes what Zuul should do if at least one job fails. +**merge-failure** + Uses the same syntax as **success**, but describes what Zuul should + do if it is unable to merge in the patchset. If no merge-failure + reporters are listed then the ``failure`` reporters will be used to + notify of unsuccessful merges. + **start** Uses the same syntax as **success**, but describes what Zuul should do when a change is added to the pipeline manager. This can be used, diff --git a/tests/fixtures/layout-merge-failure.yaml b/tests/fixtures/layout-merge-failure.yaml new file mode 100644 index 0000000000..72bc9c9c09 --- /dev/null +++ b/tests/fixtures/layout-merge-failure.yaml @@ -0,0 +1,56 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + failure: + gerrit: + verified: -1 + + - name: post + manager: IndependentPipelineManager + trigger: + gerrit: + - event: ref-updated + ref: ^(?!refs/).*$ + + - name: gate + manager: DependentPipelineManager + failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures + merge-failure-message: "The merge failed! For more information..." + trigger: + gerrit: + - event: comment-added + approval: + - approved: 1 + success: + gerrit: + verified: 2 + submit: true + failure: + gerrit: + verified: -2 + merge-failure: + gerrit: + verified: -1 + smtp: + to: you@example.com + start: + gerrit: + verified: 0 + precedence: high + +projects: + - name: org/project + check: + - project-merge: + - project-test1 + - project-test2 + gate: + - project-merge: + - project-test1 + - project-test2 diff --git a/tests/fixtures/layouts/bad_merge_failure.yaml b/tests/fixtures/layouts/bad_merge_failure.yaml new file mode 100644 index 0000000000..313d23b8a3 --- /dev/null +++ b/tests/fixtures/layouts/bad_merge_failure.yaml @@ -0,0 +1,39 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + failure: + gerrit: + verified: -1 + merge-failure-message: + + - name: gate + manager: DependentPipelineManager + failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures + trigger: + gerrit: + - event: comment-added + approval: + - approved: 1 + success: + gerrit: + verified: 2 + submit: true + failure: + gerrit: + verified: -2 + merge-failure: + start: + gerrit: + verified: 0 + precedence: high + +projects: + - name: org/project + check: + - project-check diff --git a/tests/fixtures/layouts/good_merge_failure.yaml b/tests/fixtures/layouts/good_merge_failure.yaml new file mode 100644 index 0000000000..f69b764932 --- /dev/null +++ b/tests/fixtures/layouts/good_merge_failure.yaml @@ -0,0 +1,53 @@ +pipelines: + - name: check + manager: IndependentPipelineManager + merge-failure-message: "Could not merge the change. Please rebase..." + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + failure: + gerrit: + verified: -1 + + - name: post + manager: IndependentPipelineManager + trigger: + gerrit: + - event: ref-updated + ref: ^(?!refs/).*$ + merge-failure: + gerrit: + verified: -1 + + - name: gate + manager: DependentPipelineManager + failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures + trigger: + gerrit: + - event: comment-added + approval: + - approved: 1 + success: + gerrit: + verified: 2 + submit: true + failure: + gerrit: + verified: -2 + merge-failure: + gerrit: + verified: -1 + smtp: + to: you@example.com + start: + gerrit: + verified: 0 + precedence: high + +projects: + - name: org/project + check: + - project-check diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 2d8d6bb28f..351854db5f 100755 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -3731,3 +3731,82 @@ For CI problems and help debugging, contact ci@example.org""" self.assertEqual(failure_body, self.smtp_messages[0]['body']) self.assertEqual(success_body, self.smtp_messages[1]['body']) + + def test_merge_failure_reporters(self): + """Check that the config is set up correctly""" + + self.config.set('zuul', 'layout_config', + 'tests/fixtures/layout-merge-failure.yaml') + self.sched.reconfigure(self.config) + self.registerJobs() + + self.assertEqual( + "Merge Failed.\n\nThis change was unable to be automatically " + "merged with the current state of the repository. Please rebase " + "your change and upload a new patchset.", + self.sched.layout.pipelines['check'].merge_failure_message) + self.assertEqual( + "The merge failed! For more information...", + self.sched.layout.pipelines['gate'].merge_failure_message) + + self.assertEqual( + len(self.sched.layout.pipelines['check'].merge_failure_actions), 1) + self.assertEqual( + len(self.sched.layout.pipelines['gate'].merge_failure_actions), 2) + + self.assertTrue(isinstance( + self.sched.layout.pipelines['check'].merge_failure_actions[0]. + reporter, zuul.reporter.gerrit.Reporter)) + + self.assertTrue( + ( + isinstance(self.sched.layout.pipelines['gate']. + merge_failure_actions[0].reporter, + zuul.reporter.smtp.Reporter) and + isinstance(self.sched.layout.pipelines['gate']. + merge_failure_actions[1].reporter, + zuul.reporter.gerrit.Reporter) + ) or ( + isinstance(self.sched.layout.pipelines['gate']. + merge_failure_actions[0].reporter, + zuul.reporter.gerrit.Reporter) and + isinstance(self.sched.layout.pipelines['gate']. + merge_failure_actions[1].reporter, + zuul.reporter.smtp.Reporter) + ) + ) + + def test_merge_failure_reports(self): + """Check that when a change fails to merge the correct message is sent + to the correct reporter""" + self.config.set('zuul', 'layout_config', + 'tests/fixtures/layout-merge-failure.yaml') + self.sched.reconfigure(self.config) + self.registerJobs() + + # Check a test failure isn't reported to SMTP + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + A.addApproval('CRVW', 2) + self.worker.addFailTest('project-test1', A) + self.fake_gerrit.addEvent(A.addApproval('APRV', 1)) + self.waitUntilSettled() + + self.assertEqual(3, len(self.history)) # 3 jobs + self.assertEqual(0, len(self.smtp_messages)) + + # Check a merge failure is reported to SMTP + # B should be merged, but C will conflict with B + B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') + B.addPatchset(['conflict']) + C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C') + C.addPatchset(['conflict']) + B.addApproval('CRVW', 2) + C.addApproval('CRVW', 2) + self.fake_gerrit.addEvent(B.addApproval('APRV', 1)) + self.fake_gerrit.addEvent(C.addApproval('APRV', 1)) + self.waitUntilSettled() + + self.assertEqual(6, len(self.history)) # A and B jobs + self.assertEqual(1, len(self.smtp_messages)) + self.assertEqual('The merge failed! For more information...', + self.smtp_messages[0]['body']) diff --git a/zuul/layoutvalidator.py b/zuul/layoutvalidator.py index de58c251e0..18b532d88a 100644 --- a/zuul/layoutvalidator.py +++ b/zuul/layoutvalidator.py @@ -79,11 +79,13 @@ class LayoutSchema(object): 'description': str, 'success-message': str, 'failure-message': str, + 'merge-failure-message': str, 'footer-message': str, 'dequeue-on-new-patchset': bool, 'trigger': trigger, 'success': report_actions, 'failure': report_actions, + 'merge-failure': report_actions, 'start': report_actions, 'window': window, 'window-floor': window_floor, diff --git a/zuul/model.py b/zuul/model.py index b20c08ea29..1a7c421c20 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -63,6 +63,7 @@ class Pipeline(object): self.name = name self.description = None self.failure_message = None + self.merge_failure_message = None self.success_message = None self.footer_message = None self.dequeue_on_new_patchset = True @@ -171,6 +172,11 @@ class Pipeline(object): return False return True + def didMergerSucceed(self, item): + if item.current_build_set.unable_to_merge: + return False + return True + def didAnyJobFail(self, item): for job in self.getJobs(item.change): if not job.voting: @@ -206,9 +212,8 @@ class Pipeline(object): fakebuild.result = 'SKIPPED' item.addBuild(fakebuild) - def setUnableToMerge(self, item, msg): + def setUnableToMerge(self, item): item.current_build_set.unable_to_merge = True - item.current_build_set.unable_to_merge_message = msg root = self.getJobTree(item.change.project) for job in root.getJobs(): fakebuild = Build(job, None) @@ -677,7 +682,6 @@ class BuildSet(object): self.commit = None self.zuul_url = None self.unable_to_merge = False - self.unable_to_merge_message = None self.failing_reasons = [] self.merge_state = self.NEW diff --git a/zuul/scheduler.py b/zuul/scheduler.py index c941a98eea..1a4474c233 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -226,6 +226,11 @@ class Scheduler(threading.Thread): pipeline.precedence = precedence pipeline.failure_message = conf_pipeline.get('failure-message', "Build failed.") + pipeline.merge_failure_message = conf_pipeline.get( + 'merge-failure-message', "Merge Failed.\n\nThis change was " + "unable to be automatically merged with the current state of " + "the repository. Please rebase your change and upload a new " + "patchset.") pipeline.success_message = conf_pipeline.get('success-message', "Build succeeded.") pipeline.footer_message = conf_pipeline.get('footer-message', "") @@ -233,7 +238,7 @@ class Scheduler(threading.Thread): 'dequeue-on-new-patchset', True) action_reporters = {} - for action in ['start', 'success', 'failure']: + for action in ['start', 'success', 'failure', 'merge-failure']: action_reporters[action] = [] if conf_pipeline.get(action): for reporter_name, params \ @@ -247,6 +252,11 @@ class Scheduler(threading.Thread): pipeline.start_actions = action_reporters['start'] pipeline.success_actions = action_reporters['success'] pipeline.failure_actions = action_reporters['failure'] + if len(action_reporters['merge-failure']) > 0: + pipeline.merge_failure_actions = \ + action_reporters['merge-failure'] + else: + pipeline.merge_failure_actions = action_reporters['failure'] pipeline.window = conf_pipeline.get('window', 20) pipeline.window_floor = conf_pipeline.get('window-floor', 3) @@ -936,6 +946,8 @@ class BasePipelineManager(object): self.log.info(" %s" % self.pipeline.success_actions) self.log.info(" On failure:") self.log.info(" %s" % self.pipeline.failure_actions) + self.log.info(" On merge-failure:") + self.log.info(" %s" % self.pipeline.merge_failure_actions) def getSubmitAllowNeeds(self): # Get a list of code review labels that are allowed to be @@ -1334,10 +1346,7 @@ class BasePipelineManager(object): build_set.commit = item.change.newrev if not build_set.commit: self.log.info("Unable to merge change %s" % item.change) - msg = ("This change was unable to be automatically merged " - "with the current state of the repository. Please " - "rebase your change and upload a new patchset.") - self.pipeline.setUnableToMerge(item, msg) + self.pipeline.setUnableToMerge(item) def reportItem(self, item): if item.reported: @@ -1370,10 +1379,12 @@ class BasePipelineManager(object): self.log.debug("Reporting change %s" % item.change) ret = True # Means error as returned by trigger.report if self.pipeline.didAllJobsSucceed(item): - self.log.debug("success %s %s" % (self.pipeline.success_actions, - self.pipeline.failure_actions)) + self.log.debug("success %s" % (self.pipeline.success_actions)) actions = self.pipeline.success_actions item.setReportedResult('SUCCESS') + elif not self.pipeline.didMergerSucceed(item): + actions = self.pipeline.merge_failure_actions + item.setReportedResult('MERGER_FAILURE') else: actions = self.pipeline.failure_actions item.setReportedResult('FAILURE') @@ -1395,66 +1406,72 @@ class BasePipelineManager(object): def formatReport(self, item): ret = '' + + if not self.pipeline.didMergerSucceed(item): + ret += self.pipeline.merge_failure_message + if item.dequeued_needing_change: + ret += ('\n\nThis change depends on a change that failed to ' + 'merge.') + if self.pipeline.footer_message: + ret += '\n\n' + self.pipeline.footer_message + return ret + if self.pipeline.didAllJobsSucceed(item): ret += self.pipeline.success_message + '\n\n' else: ret += self.pipeline.failure_message + '\n\n' - if item.dequeued_needing_change: - ret += "This change depends on a change that failed to merge." - elif item.current_build_set.unable_to_merge_message: - ret += item.current_build_set.unable_to_merge_message + if self.sched.config.has_option('zuul', 'url_pattern'): + url_pattern = self.sched.config.get('zuul', 'url_pattern') else: - if self.sched.config.has_option('zuul', 'url_pattern'): - url_pattern = self.sched.config.get('zuul', 'url_pattern') + url_pattern = None + + for job in self.pipeline.getJobs(item.change): + build = item.current_build_set.getBuild(job.name) + result = build.result + pattern = url_pattern + if result == 'SUCCESS': + if job.success_message: + result = job.success_message + if job.success_pattern: + pattern = job.success_pattern + elif result == 'FAILURE': + if job.failure_message: + result = job.failure_message + if job.failure_pattern: + pattern = job.failure_pattern + if pattern: + url = pattern.format(change=item.change, + pipeline=self.pipeline, + job=job, + build=build) else: - url_pattern = None - for job in self.pipeline.getJobs(item.change): - build = item.current_build_set.getBuild(job.name) - result = build.result - pattern = url_pattern - if result == 'SUCCESS': - if job.success_message: - result = job.success_message - if job.success_pattern: - pattern = job.success_pattern - elif result == 'FAILURE': - if job.failure_message: - result = job.failure_message - if job.failure_pattern: - pattern = job.failure_pattern - if pattern: - url = pattern.format(change=item.change, - pipeline=self.pipeline, - job=job, - build=build) + url = build.url or job.name + if not job.voting: + voting = ' (non-voting)' + else: + voting = '' + if self.report_times and build.end_time and build.start_time: + dt = int(build.end_time - build.start_time) + m, s = divmod(dt, 60) + h, m = divmod(m, 60) + if h: + elapsed = ' in %dh %02dm %02ds' % (h, m, s) + elif m: + elapsed = ' in %dm %02ds' % (m, s) else: - url = build.url or job.name - if not job.voting: - voting = ' (non-voting)' - else: - voting = '' - if self.report_times and build.end_time and build.start_time: - dt = int(build.end_time - build.start_time) - m, s = divmod(dt, 60) - h, m = divmod(m, 60) - if h: - elapsed = ' in %dh %02dm %02ds' % (h, m, s) - elif m: - elapsed = ' in %dm %02ds' % (m, s) - else: - elapsed = ' in %ds' % (s) - else: - elapsed = '' - name = '' - if self.sched.config.has_option('zuul', 'job_name_in_report'): - if self.sched.config.getboolean('zuul', - 'job_name_in_report'): - name = job.name + ' ' - ret += '- %s%s : %s%s%s\n' % (name, url, result, elapsed, - voting) - ret += '\n' - ret += self.pipeline.footer_message + elapsed = ' in %ds' % (s) + else: + elapsed = '' + name = '' + if self.sched.config.has_option('zuul', 'job_name_in_report'): + if self.sched.config.getboolean('zuul', + 'job_name_in_report'): + name = job.name + ' ' + ret += '- %s%s : %s%s%s\n' % (name, url, result, elapsed, + voting) + if self.pipeline.footer_message: + ret += '\n' + self.pipeline.footer_message return ret def formatDescription(self, build):