diff --git a/doc/source/config/pipeline.rst b/doc/source/config/pipeline.rst index c4539c2e37..f1c294775e 100644 --- a/doc/source/config/pipeline.rst +++ b/doc/source/config/pipeline.rst @@ -207,7 +207,7 @@ success, the pipeline reports back to Gerrit with ``Verified`` vote of The introductory text in reports when an item is enqueued. Empty by default. - .. attr:: merge-failure-message + .. attr:: merge-conflict-message :default: Merge failed. The introductory text in the message reported when a change @@ -329,10 +329,10 @@ success, the pipeline reports back to Gerrit with ``Verified`` vote of These reporters describe what Zuul should do if at least one job fails. - .. attr:: merge-failure + .. attr:: merge-conflict These reporters describe what Zuul should do if it is unable to - merge in the patchset. If no merge-failure reporters are listed + merge in the patchset. If no merge-conflict reporters are listed then the ``failure`` reporters will be used to notify of unsuccessful merges. @@ -377,7 +377,7 @@ success, the pipeline reports back to Gerrit with ``Verified`` vote of If set, a pipeline can enter a *disabled* state if too many changes in a row fail. When this value is exceeded the pipeline will stop reporting to any of the **success**, **failure** or - **merge-failure** reporters and instead only report to the + **merge-conflict** reporters and instead only report to the **disabled** reporters. (No **start** reports are made when a pipeline is disabled). diff --git a/doc/source/drivers/elasticsearch.rst b/doc/source/drivers/elasticsearch.rst index 4bf103a6f2..03ee9de873 100644 --- a/doc/source/drivers/elasticsearch.rst +++ b/doc/source/drivers/elasticsearch.rst @@ -105,7 +105,7 @@ Reporter Configuration This reporter is used to store build results in an Elasticsearch index. The Elasticsearch reporter does nothing on :attr:`pipeline.start` or -:attr:`pipeline.merge-failure`; it only acts on +:attr:`pipeline.merge-conflict`; it only acts on :attr:`pipeline.success` or :attr:`pipeline.failure` reporting stages. .. attr:: pipeline.. diff --git a/doc/source/drivers/mqtt.rst b/doc/source/drivers/mqtt.rst index 168785f63d..10cd4c8a2d 100644 --- a/doc/source/drivers/mqtt.rst +++ b/doc/source/drivers/mqtt.rst @@ -22,7 +22,7 @@ An MQTT report uses this schema: .. attr:: action The reporter action name, e.g.: 'start', 'success', 'failure', - 'merge-failure', ... + 'merge-conflict', ... .. attr:: tenant diff --git a/doc/source/monitoring.rst b/doc/source/monitoring.rst index 3c0b2a1ff0..5db81b8e70 100644 --- a/doc/source/monitoring.rst +++ b/doc/source/monitoring.rst @@ -285,7 +285,7 @@ These metrics are emitted by the Zuul :ref:`scheduler`: Incremented to represent the status of a Zuul executor's merger operations. ```` can be either ``SUCCESS`` or ``FAILURE``. A failed merge operation which would be accounted for as a ``FAILURE`` - is what ends up being returned by Zuul as a ``MERGER_FAILURE``. + is what ends up being returned by Zuul as a ``MERGE_CONFLICT``. .. stat:: builds :type: counter diff --git a/releasenotes/notes/merge-conflict-rename-da402e1c8685949b.yaml b/releasenotes/notes/merge-conflict-rename-da402e1c8685949b.yaml new file mode 100644 index 0000000000..5942f33023 --- /dev/null +++ b/releasenotes/notes/merge-conflict-rename-da402e1c8685949b.yaml @@ -0,0 +1,30 @@ +--- +upgrade: + - | + The buildset result ``MERGER_FAILURE`` has been renamed to + ``MERGE_CONFLICT``, and the pipeline reporter configuration + ``merge-failure`` has been renamed to ``merge-conflict``. + + These are more descriptive of the most common errors actually + reported, and so are expected to be less confusing to users. This + is also in service of a future change to support a new buildset + result ``MERGE_FAILURE`` which will indicate that the change was + unable to be merged in the upstream repository. + + When upgrading, it is recommended to stop all schedulers briefly + (i.e, when the first scheduler of the new version starts, there + should be no schedulers running the old version). The new + scheduler will perform a database migration when it starts and + update all existing ``MERGER_FAILURE`` buildset results to + ``MERGE_CONFLICT``. If old schedulers are running, they may + continue to add ``MERGER_FAILURE`` entries which will need to be + manually updated in order to be visible in the web UI or rest API. + +deprecations: + - | + The ``merge-failure`` and ``merge-failure-message`` pipeline + configuration options have been renamed to ``merge-conflict`` and + ``merge-conflict-message`` respectively. The old settings are + retained for backwards compatibility, but will be removed in a + later version. Please update your usage of them as soon as + possible. diff --git a/tests/base.py b/tests/base.py index e2eec9508d..564aa54954 100644 --- a/tests/base.py +++ b/tests/base.py @@ -3087,8 +3087,8 @@ class RecordingAnsibleJob(zuul.executor.server.AnsibleJob): # Get a merger in order to update the repos involved in this job. commit = super(RecordingAnsibleJob, self).doMergeChanges( *args, **kw) - if not commit: # merge conflict - self.recordResult('MERGER_FAILURE') + if not commit: + self.recordResult('MERGE_CONFLICT') return commit diff --git a/tests/fixtures/config/zuultrigger/project-change-merged/git/common-config/zuul.yaml b/tests/fixtures/config/zuultrigger/project-change-merged/git/common-config/zuul.yaml index 77a96c25b8..ab0e1967c5 100644 --- a/tests/fixtures/config/zuultrigger/project-change-merged/git/common-config/zuul.yaml +++ b/tests/fixtures/config/zuultrigger/project-change-merged/git/common-config/zuul.yaml @@ -43,7 +43,7 @@ trigger: zuul: - event: project-change-merged - merge-failure: + merge-conflict: gerrit: Verified: -1 diff --git a/tests/fixtures/layouts/merge-conflict.yaml b/tests/fixtures/layouts/merge-conflict.yaml new file mode 100644 index 0000000000..2efcdced32 --- /dev/null +++ b/tests/fixtures/layouts/merge-conflict.yaml @@ -0,0 +1,82 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- pipeline: + name: post + manager: independent + trigger: + gerrit: + - event: ref-updated + ref: ^(?!refs/).*$ + +- pipeline: + name: gate + manager: dependent + failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures + merge-conflict-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-conflict: + gerrit: + Verified: -1 + smtp: + to: you@example.com + start: + gerrit: + Verified: 0 + precedence: high + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: + name: project-merge + hold-following-changes: true + run: playbooks/project-merge.yaml + +- job: + name: project-test1 + run: playbooks/project-test1.yaml + +- job: + name: project-test2 + run: playbooks/project-test2.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/merging-github.yaml b/tests/fixtures/layouts/merging-github.yaml index d7bd7212cb..e887fb36d7 100644 --- a/tests/fixtures/layouts/merging-github.yaml +++ b/tests/fixtures/layouts/merging-github.yaml @@ -2,7 +2,7 @@ name: merge description: Pipeline for merging the pull request manager: dependent - merge-failure-message: Merge failed + merge-conflict-message: Merge failed trigger: github: - event: pull_request diff --git a/tests/unit/test_database.py b/tests/unit/test_database.py index 689576ec35..60f7eea37c 100644 --- a/tests/unit/test_database.py +++ b/tests/unit/test_database.py @@ -98,6 +98,30 @@ class TestMysqlDatabase(DBBaseTestCase): f"show create table {table}").one()[1] self.compareMysql(create, sqlalchemy_tables[table]) + def test_migration_4647def24b32(self): + with self.connection.engine.begin() as connection: + connection.exec_driver_sql("set foreign_key_checks=0") + for table in connection.exec_driver_sql("show tables"): + table = table[0] + connection.exec_driver_sql(f"drop table {table}") + connection.exec_driver_sql("set foreign_key_checks=1") + + self.connection._migrate('c57e9e76b812') + with self.connection.engine.begin() as connection: + connection.exec_driver_sql( + "insert into zuul_buildset (result) values ('SUCCESS')") + connection.exec_driver_sql( + "insert into zuul_buildset (result) values ('MERGER_FAILURE')") + results = [r[0] for r in connection.exec_driver_sql( + "select result from zuul_buildset")] + self.assertEqual(results, ['SUCCESS', 'MERGER_FAILURE']) + + self.connection._migrate() + with self.connection.engine.begin() as connection: + results = [r[0] for r in connection.exec_driver_sql( + "select result from zuul_buildset")] + self.assertEqual(results, ['SUCCESS', 'MERGE_CONFLICT']) + def test_buildsets(self): tenant = 'tenant1', buildset_uuid = 'deadbeef' diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 69597401b8..035d602df2 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -5082,8 +5082,8 @@ For CI problems and help debugging, contact ci@example.org""" self.assertEqual(0, len(A.messages)) - @simple_layout('layouts/merge-failure.yaml') - def test_merge_failure_reporters(self): + @simple_layout('layouts/merge-conflict.yaml') + def test_merge_conflict_reporters(self): """Check that the config is set up correctly""" tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') @@ -5092,34 +5092,77 @@ For CI problems and help debugging, contact ci@example.org""" "dependencies was unable to be automatically merged with the " "current state of its repository. Please rebase the change and " "upload a new patchset.", - tenant.layout.pipelines['check'].merge_failure_message) + tenant.layout.pipelines['check'].merge_conflict_message) self.assertEqual( "The merge failed! For more information...", - tenant.layout.pipelines['gate'].merge_failure_message) + tenant.layout.pipelines['gate'].merge_conflict_message) self.assertEqual( - len(tenant.layout.pipelines['check'].merge_failure_actions), 1) + len(tenant.layout.pipelines['check'].merge_conflict_actions), 1) self.assertEqual( - len(tenant.layout.pipelines['gate'].merge_failure_actions), 2) + len(tenant.layout.pipelines['gate'].merge_conflict_actions), 2) self.assertTrue(isinstance( - tenant.layout.pipelines['check'].merge_failure_actions[0], + tenant.layout.pipelines['check'].merge_conflict_actions[0], gerritreporter.GerritReporter)) self.assertTrue( ( isinstance(tenant.layout.pipelines['gate']. - merge_failure_actions[0], + merge_conflict_actions[0], zuul.driver.smtp.smtpreporter.SMTPReporter) and isinstance(tenant.layout.pipelines['gate']. - merge_failure_actions[1], + merge_conflict_actions[1], gerritreporter.GerritReporter) ) or ( isinstance(tenant.layout.pipelines['gate']. - merge_failure_actions[0], + merge_conflict_actions[0], gerritreporter.GerritReporter) and isinstance(tenant.layout.pipelines['gate']. - merge_failure_actions[1], + merge_conflict_actions[1], + zuul.driver.smtp.smtpreporter.SMTPReporter) + ) + ) + + @simple_layout('layouts/merge-failure.yaml') + def test_merge_failure_reporters(self): + """Check that the config is set up correctly""" + # TODO: Remove this backwards compat test in v6.0 + + tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') + self.assertEqual( + "Merge Failed.\n\nThis change or one of its cross-repo " + "dependencies was unable to be automatically merged with the " + "current state of its repository. Please rebase the change and " + "upload a new patchset.", + tenant.layout.pipelines['check'].merge_conflict_message) + self.assertEqual( + "The merge failed! For more information...", + tenant.layout.pipelines['gate'].merge_conflict_message) + + self.assertEqual( + len(tenant.layout.pipelines['check'].merge_conflict_actions), 1) + self.assertEqual( + len(tenant.layout.pipelines['gate'].merge_conflict_actions), 2) + + self.assertTrue(isinstance( + tenant.layout.pipelines['check'].merge_conflict_actions[0], + gerritreporter.GerritReporter)) + + self.assertTrue( + ( + isinstance(tenant.layout.pipelines['gate']. + merge_conflict_actions[0], + zuul.driver.smtp.smtpreporter.SMTPReporter) and + isinstance(tenant.layout.pipelines['gate']. + merge_conflict_actions[1], + gerritreporter.GerritReporter) + ) or ( + isinstance(tenant.layout.pipelines['gate']. + merge_conflict_actions[0], + gerritreporter.GerritReporter) and + isinstance(tenant.layout.pipelines['gate']. + merge_conflict_actions[1], zuul.driver.smtp.smtpreporter.SMTPReporter) ) ) @@ -5128,7 +5171,7 @@ For CI problems and help debugging, contact ci@example.org""" """Check that when a change fails to merge the correct message is sent to the correct reporter""" self.commitConfigUpdate('common-config', - 'layouts/merge-failure.yaml') + 'layouts/merge-conflict.yaml') self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) # Check a test failure isn't reported to SMTP diff --git a/web/src/containers/build/Misc.jsx b/web/src/containers/build/Misc.jsx index 53270d83b9..f52fbcd963 100644 --- a/web/src/containers/build/Misc.jsx +++ b/web/src/containers/build/Misc.jsx @@ -58,7 +58,7 @@ const RESULT_ICON_CONFIGS = { color: 'var(--pf-global--info-color--100)', badgeColor: 'yellow', }, - MERGER_FAILURE: { + MERGE_CONFLICT: { icon: ExclamationIcon, color: 'var(--pf-global--warning-color--100)', badgeColor: 'orange', diff --git a/web/src/containers/charts/Misc.jsx b/web/src/containers/charts/Misc.jsx index 9ee90e5e66..d55c459b55 100644 --- a/web/src/containers/charts/Misc.jsx +++ b/web/src/containers/charts/Misc.jsx @@ -41,7 +41,7 @@ const buildResultLegendData = [ symbol: { fill: '#F6D173' }, }, { - name: 'MERGER_FAILURE', + name: 'MERGE_CONFLICT', // PF orange-200 symbol: { fill: '#EF9234' }, }, @@ -81,4 +81,4 @@ const buildsBarStyle = { } } -export { buildResultLegendData, buildsBarStyleMap, buildsBarStyle } \ No newline at end of file +export { buildResultLegendData, buildsBarStyleMap, buildsBarStyle } diff --git a/web/src/pages/Builds.jsx b/web/src/pages/Builds.jsx index 3517e14a7f..0e76a73be5 100644 --- a/web/src/pages/Builds.jsx +++ b/web/src/pages/Builds.jsx @@ -82,7 +82,7 @@ class BuildsPage extends React.Component { 'POST_FAILURE', 'SKIPPED', 'NODE_FAILURE', - 'MERGER_FAILURE', + 'MERGE_CONFLICT', 'CONFIG_ERROR', 'TIMED_OUT', 'CANCELED', diff --git a/web/src/pages/Buildsets.jsx b/web/src/pages/Buildsets.jsx index f0d0e52772..382df5eb2c 100644 --- a/web/src/pages/Buildsets.jsx +++ b/web/src/pages/Buildsets.jsx @@ -70,7 +70,7 @@ class BuildsetsPage extends React.Component { options: [ 'SUCCESS', 'FAILURE', - 'MERGER_FAILURE', + 'MERGE_CONFLICT', 'DEQUEUED', ] }, diff --git a/zuul/configloader.py b/zuul/configloader.py index 2ee732c13f..e0cf88d129 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1148,7 +1148,7 @@ class PipelineParser(object): 'start': 'start_actions', 'success': 'success_actions', 'failure': 'failure_actions', - 'merge-failure': 'merge_failure_actions', + 'merge-conflict': 'merge_conflict_actions', 'no-jobs': 'no_jobs_actions', 'disabled': 'disabled_actions', 'dequeue': 'dequeue_actions', @@ -1198,6 +1198,7 @@ class PipelineParser(object): 'success-message': str, 'failure-message': str, 'start-message': str, + 'merge-conflict-message': str, 'merge-failure-message': str, 'no-jobs-message': str, 'footer-message': str, @@ -1220,7 +1221,8 @@ 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', 'dequeue']: + 'merge-conflict', 'merge-failure', 'no-jobs', + 'disabled', 'dequeue']: pipeline[action] = self.getDriverSchema('reporter') return vs.Schema(pipeline) @@ -1236,12 +1238,16 @@ class PipelineParser(object): pipeline.precedence = precedence pipeline.failure_message = conf.get('failure-message', "Build failed.") - pipeline.merge_failure_message = conf.get( + # TODO: Remove in Zuul v6.0 + backwards_compat_merge_message = conf.get( 'merge-failure-message', "Merge Failed.\n\nThis change or one " "of its cross-repo dependencies was unable to be " "automatically merged with the current state of its " "repository. Please rebase the change and upload a new " "patchset.") + pipeline.merge_conflict_message = conf.get( + 'merge-conflict-message', backwards_compat_merge_message) + pipeline.success_message = conf.get('success-message', "Build succeeded.") pipeline.footer_message = conf.get('footer-message', "") @@ -1259,12 +1265,18 @@ class PipelineParser(object): pipeline.post_review = conf.get( 'post-review', False) + # TODO: Remove in Zuul v6.0 + # Make a copy to manipulate for backwards compat. + conf_copy = conf.copy() + if 'merge-failure' in conf_copy and 'merge-conflict' not in conf_copy: + conf_copy['merge-conflict'] = conf_copy['merge-failure'] + for conf_key, action in self.reporter_actions.items(): reporter_set = [] allowed_reporters = self.pcontext.tenant.allowed_reporters - if conf.get(conf_key): + if conf_copy.get(conf_key): for reporter_name, params \ - in conf.get(conf_key).items(): + in conf_copy.get(conf_key).items(): if allowed_reporters is not None and \ reporter_name not in allowed_reporters: raise UnknownConnection(reporter_name) @@ -1274,9 +1286,9 @@ class PipelineParser(object): reporter_set.append(reporter) setattr(pipeline, action, reporter_set) - # If merge-failure actions aren't explicit, use the failure actions - if not pipeline.merge_failure_actions: - pipeline.merge_failure_actions = pipeline.failure_actions + # If merge-conflict actions aren't explicit, use the failure actions + if not pipeline.merge_conflict_actions: + pipeline.merge_conflict_actions = pipeline.failure_actions pipeline.disable_at = conf.get( 'disable-after-consecutive-failures', None) diff --git a/zuul/driver/gitlab/gitlabreporter.py b/zuul/driver/gitlab/gitlabreporter.py index c3abf3dd89..f364521f5c 100644 --- a/zuul/driver/gitlab/gitlabreporter.py +++ b/zuul/driver/gitlab/gitlabreporter.py @@ -69,7 +69,7 @@ class GitlabReporter(BaseReporter): if self._merge: self.mergeMR(item) if not item.change.is_merged: - msg = self._formatItemReportMergeFailure(item) + msg = self._formatItemReportMergeConflict(item) self.addMRComment(item, msg) def addMRComment(self, item, comment=None): diff --git a/zuul/driver/pagure/pagurereporter.py b/zuul/driver/pagure/pagurereporter.py index e6f33ef473..918a31b6b7 100644 --- a/zuul/driver/pagure/pagurereporter.py +++ b/zuul/driver/pagure/pagurereporter.py @@ -60,7 +60,7 @@ class PagureReporter(BaseReporter): if self._merge: self.mergePull(item) if not item.change.is_merged: - msg = self._formatItemReportMergeFailure(item) + msg = self._formatItemReportMergeConflict(item) self.addPullComment(item, msg) def _formatItemReportJobs(self, item): diff --git a/zuul/driver/sql/alembic/versions/4647def24b32_merge_conflict_rename.py b/zuul/driver/sql/alembic/versions/4647def24b32_merge_conflict_rename.py new file mode 100644 index 0000000000..64be6fe8f2 --- /dev/null +++ b/zuul/driver/sql/alembic/versions/4647def24b32_merge_conflict_rename.py @@ -0,0 +1,42 @@ +# 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. + +"""merge_conflict_rename + +Revision ID: 4647def24b32 +Revises: c57e9e76b812 +Create Date: 2022-02-24 14:56:52.597907 + +""" + +# revision identifiers, used by Alembic. +revision = '4647def24b32' +down_revision = 'c57e9e76b812' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + +BUILDSET_TABLE = 'zuul_buildset' + + +def upgrade(table_prefix=''): + buildset = sa.table(table_prefix + BUILDSET_TABLE, sa.column('result')) + op.execute(buildset.update(). + where(buildset.c.result == + op.inline_literal('MERGER_FAILURE')). + values({'result': op.inline_literal('MERGE_CONFLICT')})) + + +def downgrade(): + raise Exception("Downgrades not supported") diff --git a/zuul/driver/sql/sqlconnection.py b/zuul/driver/sql/sqlconnection.py index 2f9195988b..8dd5f9d447 100644 --- a/zuul/driver/sql/sqlconnection.py +++ b/zuul/driver/sql/sqlconnection.py @@ -284,7 +284,7 @@ class SQLConnection(BaseConnection): def getSession(self): return DatabaseSession(self) - def _migrate(self): + def _migrate(self, revision='head'): """Perform the alembic migrations for this connection""" with self.engine.begin() as conn: context = alembic.migration.MigrationContext.configure(conn) @@ -304,9 +304,9 @@ class SQLConnection(BaseConnection): if current_rev is None and not self.force_migrations: self.metadata.create_all(self.engine) - alembic.command.stamp(config, "head", tag=tag) + alembic.command.stamp(config, revision, tag=tag) else: - alembic.command.upgrade(config, 'head', tag=tag) + alembic.command.upgrade(config, revision, tag=tag) def onLoad(self, zk_client, component_registry=None): safe_connection = quote_plus(self.connection_name) diff --git a/zuul/executor/server.py b/zuul/executor/server.py index acb2e3cc71..5a267432b2 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -1616,7 +1616,7 @@ class AnsibleJob(object): self.executor_server.completeBuild(self.build_request, result) return None if not ret: # merge conflict - result = dict(result='MERGER_FAILURE') + result = dict(result='MERGE_CONFLICT') if self.executor_server.statsd: base_key = "zuul.executor.{hostname}.merger" self.executor_server.statsd.incr(base_key + ".FAILURE") diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index afaec4e25d..d82d0d4a0f 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -1862,14 +1862,14 @@ class PipelineManager(metaclass=ABCMeta): elif item.getConfigErrors(): log.debug("Invalid config for change %s", item.change) # TODOv3(jeblair): consider a new reporter action for this - action = 'merge-failure' - actions = self.pipeline.merge_failure_actions + action = 'merge-conflict' + actions = self.pipeline.merge_conflict_actions item.setReportedResult('CONFIG_ERROR') elif item.didMergerFail(): - log.debug("Merger failure") - action = 'merge-failure' - actions = self.pipeline.merge_failure_actions - item.setReportedResult('MERGER_FAILURE') + log.debug("Merge conflict") + action = 'merge-conflict' + actions = self.pipeline.merge_conflict_actions + item.setReportedResult('MERGE_CONFLICT') elif item.wasDequeuedNeedingChange(): log.debug("Dequeued needing change") action = 'failure' diff --git a/zuul/model.py b/zuul/model.py index 130ed2dd5f..0e0b0de592 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -430,7 +430,7 @@ class Pipeline(object): self.start_mark = None self.description = None self.failure_message = None - self.merge_failure_message = None + self.merge_conflict_message = None self.success_message = None self.footer_message = None self.enqueue_message = None @@ -448,7 +448,7 @@ class Pipeline(object): self.start_actions = [] self.success_actions = [] self.failure_actions = [] - self.merge_failure_actions = [] + self.merge_conflict_actions = [] self.no_jobs_actions = [] self.disabled_actions = [] self.dequeue_actions = [] @@ -473,7 +473,7 @@ class Pipeline(object): self.start_actions + self.success_actions + self.failure_actions + - self.merge_failure_actions + + self.merge_conflict_actions + self.no_jobs_actions + self.disabled_actions + self.dequeue_actions diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py index 1749f8a63e..3372c8ae59 100644 --- a/zuul/reporter/__init__.py +++ b/zuul/reporter/__init__.py @@ -122,7 +122,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta): 'start': self._formatItemReportStart, 'success': self._formatItemReportSuccess, 'failure': self._formatItemReportFailure, - 'merge-failure': self._formatItemReportMergeFailure, + 'merge-conflict': self._formatItemReportMergeConflict, 'no-jobs': self._formatItemReportNoJobs, 'disabled': self._formatItemReportDisabled, 'dequeue': self._formatItemReportDequeue, @@ -186,7 +186,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta): msg = "{}\n\n{}".format( msg, self._formatItemReportOtherBundleItems(item)) elif item.didMergerFail(): - msg = item.pipeline.merge_failure_message + msg = item.pipeline.merge_conflict_message elif item.getConfigErrors(): msg = str(item.getConfigErrors()[0].error) else: @@ -195,8 +195,8 @@ class BaseReporter(object, metaclass=abc.ABCMeta): msg += '\n\n' + self._formatItemReportJobs(item) return msg - def _formatItemReportMergeFailure(self, item, with_jobs=True): - return item.pipeline.merge_failure_message + def _formatItemReportMergeConflict(self, item, with_jobs=True): + return item.pipeline.merge_conflict_message def _formatItemReportNoJobs(self, item, with_jobs=True): status_url = get_default(self.connection.sched.config, diff --git a/zuul/scheduler.py b/zuul/scheduler.py index a74d0f9461..a30ebb0a7d 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -2447,8 +2447,9 @@ class Scheduler(threading.Thread): if result == "ABORTED": # Always retry if the executor just went away build.retry = True - if result == "MERGER_FAILURE": - # The build result MERGER_FAILURE is a bit misleading here + # TODO: Remove merger_failure in v6.0 + if result in ["MERGE_CONFLICT", "MERGER_FAILURE"]: + # The build result MERGE_CONFLICT is a bit misleading here # because when we got here we know that there are no merge # conflicts. Instead this is most likely caused by some # infrastructure failure. This can be anything like connection