From e03d8c887c092367b9ff2b00b48f244d4f6584f9 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 24 Feb 2022 15:38:45 -0800 Subject: [PATCH] Rename MERGER_FAILURE to MERGE_CONFLICT This is a prelude to a change which will report a distinct buildset result to the database if the upstream code review system is unable to merge a change. Currently it is reported as MERGER_FAILURE which makes it difficult to distinguish from merge conflicts. Essentially, the two states we're interested in are when Zuul's merger is unable to prepare a git checkout of the change (99% of the time, this is a merge conflict). This will be known as MERGE_CONFLICT now. The second state is when Zuul asks Gerrit/Github/etc to submit/merge a change and the remote system is unable (or refuses) to do so. In a future change, that will be reported as MERGE_FAILURE. To avoid confusion and use names which better reflect the situation, this change performs the rename to MERGE_CONFLICT. Because there are pipeline configuration options tied to the MERGER_FAILURE status (which start with 'merge-failure') they are also renamed to 'merge-conflict'. The old names are supported for backwards compatibility. A SQL migration takes care of updating values in the database. The upgrade procedure is noted as being special because of the db value updates. If an operator doesn't follow the recommended procedure, however, the consequences are minimal (builds which won't be easily queried in the web ui; that can be manually corrected if desired). A model API change is not needed since the only place where we receive this value from ZK can be updated to accept both values. Change-Id: I3050409ed68805c748efe7a176b9755fa281536f --- doc/source/config/pipeline.rst | 8 +- doc/source/drivers/elasticsearch.rst | 2 +- doc/source/drivers/mqtt.rst | 2 +- doc/source/monitoring.rst | 2 +- ...erge-conflict-rename-da402e1c8685949b.yaml | 30 +++++++ tests/base.py | 4 +- .../git/common-config/zuul.yaml | 2 +- tests/fixtures/layouts/merge-conflict.yaml | 82 +++++++++++++++++++ tests/fixtures/layouts/merging-github.yaml | 2 +- tests/unit/test_database.py | 24 ++++++ tests/unit/test_scheduler.py | 67 ++++++++++++--- web/src/containers/build/Misc.jsx | 2 +- web/src/containers/charts/Misc.jsx | 4 +- web/src/pages/Builds.jsx | 2 +- web/src/pages/Buildsets.jsx | 2 +- zuul/configloader.py | 28 +++++-- zuul/driver/gitlab/gitlabreporter.py | 2 +- zuul/driver/pagure/pagurereporter.py | 2 +- .../4647def24b32_merge_conflict_rename.py | 42 ++++++++++ zuul/driver/sql/sqlconnection.py | 6 +- zuul/executor/server.py | 2 +- zuul/manager/__init__.py | 12 +-- zuul/model.py | 6 +- zuul/reporter/__init__.py | 8 +- zuul/scheduler.py | 5 +- 25 files changed, 291 insertions(+), 57 deletions(-) create mode 100644 releasenotes/notes/merge-conflict-rename-da402e1c8685949b.yaml create mode 100644 tests/fixtures/layouts/merge-conflict.yaml create mode 100644 zuul/driver/sql/alembic/versions/4647def24b32_merge_conflict_rename.py 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