Add MERGE_FAILURE buildset result

We now report a distinct buildset result to the database if the upstream code
review system is unable to merge a change.  Previously it was reported as
MERGE_CONFLICT 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 is the MERGE_CONFLICT result.

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.  This is MERGE_FAILURE.

Change-Id: I47af242aeb19ae4b4aedde353fcc76ff82d90fbe
This commit is contained in:
James E. Blair 2022-02-24 18:32:20 -08:00
parent e03d8c887c
commit 370d36c025
9 changed files with 64 additions and 7 deletions

View File

@ -0,0 +1,6 @@
---
features:
- |
A new buildset result ``MERGE_FAILURE`` is stored in the build
database in the case where a buildset is unable to be merged by
the upstream code review system.

View File

@ -667,6 +667,12 @@ class TestGerritCircularDependencies(ZuulTestCase):
self.assertEqual(A.data["status"], "NEW")
self.assertEqual(B.data["status"], "NEW")
buildsets = {bs.change: bs for bs in
self.scheds.first.connections.connections[
'database'].getBuildsets()}
self.assertEqual(buildsets[2].result, 'MERGE_FAILURE')
self.assertEqual(buildsets[1].result, 'FAILURE')
def test_cycle_reporting_partial_failure(self):
A = self.fake_gerrit.addFakeChange("org/project", "master", "A")
B = self.fake_gerrit.addFakeChange("org/project1", "master", "B")

View File

@ -5229,6 +5229,19 @@ For CI problems and help debugging, contact ci@example.org"""
self.assertNotIn('logs.example.com', B.messages[0])
self.assertNotIn('SKIPPED', B.messages[0])
def test_submit_failure(self):
A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A')
A.fail_merge = True
A.addApproval('Code-Review', 2)
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
buildsets = list(
self.scheds.first.connections.connections[
'database'].getBuildsets())
self.assertEqual(buildsets[0].result, 'MERGE_FAILURE')
@simple_layout('layouts/nonvoting-pipeline.yaml')
def test_nonvoting_pipeline(self):
"Test that a nonvoting pipeline (experimental) can still report"

View File

@ -63,6 +63,11 @@ const RESULT_ICON_CONFIGS = {
color: 'var(--pf-global--warning-color--100)',
badgeColor: 'orange',
},
MERGE_FAILURE: {
icon: ExclamationIcon,
color: 'var(--pf-global--warning-color--100)',
badgeColor: 'orange',
},
NODE_FAILURE: {
icon: ExclamationIcon,
color: 'var(--pf-global--warning-color--100)',

View File

@ -45,6 +45,11 @@ const buildResultLegendData = [
// PF orange-200
symbol: { fill: '#EF9234' },
},
{
name: 'MERGE_FAILURE',
// PF orange-200
symbol: { fill: '#EF9234' },
},
{
name: 'NODE_FAILURE',
// PF orange-300

View File

@ -83,6 +83,7 @@ class BuildsPage extends React.Component {
'SKIPPED',
'NODE_FAILURE',
'MERGE_CONFLICT',
'MERGE_FAILURE',
'CONFIG_ERROR',
'TIMED_OUT',
'CANCELED',

View File

@ -71,6 +71,7 @@ class BuildsetsPage extends React.Component {
'SUCCESS',
'FAILURE',
'MERGE_CONFLICT',
'MERGE_FAILURE',
'DEQUEUED',
]
},

View File

@ -1546,10 +1546,13 @@ class PipelineManager(metaclass=ABCMeta):
actions = self.pipeline.failure_actions
for ri in reported_items:
ri.setReportedResult('FAILURE')
self.sendReport(actions, ri)
self.sql.reportBuildsetEnd(ri.current_build_set,
'failure', final=True)
if ri is not item:
# Don't override the reported sql result for the item
# that "really" failed.
ri.setReportedResult('FAILURE')
self.sql.reportBuildsetEnd(ri.current_build_set,
'failure', final=True)
def processQueue(self):
# Do whatever needs to be done for each change in the queue
@ -1787,16 +1790,25 @@ class PipelineManager(metaclass=ABCMeta):
def reportItem(self, item):
log = get_annotated_logger(self.log, item.event)
action = None
if not item.reported:
# _reportItem() returns True if it failed to report.
action, reported = self._reportItem(item)
item.updateAttributes(self.current_context,
reported=not self._reportItem(item))
reported=reported)
if self.changes_merge:
succeeded = item.didAllJobsSucceed() and not item.isBundleFailing()
merged = item.reported
source = item.change.project.source
if merged:
merged = source.isMerged(item.change, item.change.branch)
if action:
if action == 'success' and not merged:
log.debug("Overriding result for %s to merge failure",
item.change)
action = 'merge-failure'
item.setReportedResult('MERGE_FAILURE')
self.sql.reportBuildsetEnd(item.current_build_set,
action, final=True)
change_queue = item.queue
if not (succeeded and merged):
if (not item.current_build_set.job_graph or
@ -1816,6 +1828,8 @@ class PipelineManager(metaclass=ABCMeta):
raise exceptions.MergeFailure(
"Change %s failed to merge" % item.change)
else:
self.sql.reportBuildsetEnd(item.current_build_set,
action, final=True)
log.info("Reported change %s status: all-succeeded: %s, "
"merged: %s", item.change, succeeded, merged)
change_queue.increaseWindowSize()
@ -1825,6 +1839,9 @@ class PipelineManager(metaclass=ABCMeta):
zuul_driver = self.sched.connections.drivers['zuul']
tenant = self.pipeline.tenant
zuul_driver.onChangeMerged(tenant, item.change, source)
elif action:
self.sql.reportBuildsetEnd(item.current_build_set,
action, final=True)
def _reportItem(self, item):
log = get_annotated_logger(self.log, item.event)
@ -1922,8 +1939,7 @@ class PipelineManager(metaclass=ABCMeta):
ret = self.sendReport(actions, item)
if ret:
log.error("Reporting item %s received: %s", item, ret)
self.sql.reportBuildsetEnd(item.current_build_set, action, final=True)
return ret
return action, (not ret)
def reportStats(self, item, added=False):
if not self.sched.statsd:

View File

@ -123,6 +123,7 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
'success': self._formatItemReportSuccess,
'failure': self._formatItemReportFailure,
'merge-conflict': self._formatItemReportMergeConflict,
'merge-failure': self._formatItemReportMergeFailure,
'no-jobs': self._formatItemReportNoJobs,
'disabled': self._formatItemReportDisabled,
'dequeue': self._formatItemReportDequeue,
@ -198,6 +199,9 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
def _formatItemReportMergeConflict(self, item, with_jobs=True):
return item.pipeline.merge_conflict_message
def _formatItemReportMergeFailure(self, item, with_jobs=True):
return 'This change was not merged by the code review system.\n'
def _formatItemReportNoJobs(self, item, with_jobs=True):
status_url = get_default(self.connection.sched.config,
'web', 'status_url', '')