zuul/zuul
Code Issues Proposed changes

Further no_job reporting fixes

Browse Source 
The previous fix for erroneously reporting NO_JOB results to the
database did not handle the case of superceding queues.  An item
enqueued in check and then superceded into gate would still report
NO_JOBS in gate and DEQUEUED in check, neither of which is desired.

It appears that we were previously relying on not having reported
a buildset start in order to avoid reporting buildset ends when
they are uninteresting.  This changed in
bf2eb71f95257e0dfac259fb74e7a97fe4a53eb8 where we now intentionally
create missing buildset entries.

To return to the original behavior, we now only attempt to report
a buildset end under most circumstances if we would have reported
a start as well (by virtue of having a non-empty job graph).  There
is one exception to this, where we report an item which otherwise
can't be enqueued in order to report an error.  This is maintained.

Change-Id: Ic2322e293a44a2946c6b766cf87d256ed39319ea
This commit is contained in:
James E. Blair 2022-04-22 13:55:13 -07:00
parent 7f0d634b58
commit 5a92b30033
4 changed files with 82 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
releasenotes/notes/no-jobs2-fe2df7e33aa07acd.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
Further instances of reporting NO_JOBS results to the database
when not necessary have been corrected.

24
tests/fixtures/layouts/no-jobs-reporting.yaml vendored
View File

@ -14,7 +14,31 @@
gerrit:
Verified: 0
- pipeline:
name: gate
manager: dependent
success-message: Build succeeded (gate).
supercedes: check
trigger:
gerrit:
- event: comment-added
approval:
- Approved: 1
success:
gerrit:
Verified: 2
submit: true
failure:
gerrit:
Verified: -2
start:
gerrit:
Verified: 0
precedence: high
- project:
name: org/project
check:
jobs: []
gate:
jobs: []

34
tests/unit/test_reporting.py
View File

@ -132,12 +132,11 @@ class TestReporting(ZuulTestCase):
self.assertIn("Build succeeded (check)", A.messages[3])
@simple_layout("layouts/no-jobs-reporting.yaml")
def test_no_jobs_reporting(self):
def test_no_jobs_reporting_check(self):
# Test that we don't report NO_JOBS results
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()
@ -155,4 +154,35 @@ class TestReporting(ZuulTestCase):
sa.sql.select([reporter.connection.zuul_buildset_table]))
buildsets = result.fetchall()
for x in buildsets:
self.log.debug("Buildset %s", x)
self.assertEqual(0, len(buildsets))
@simple_layout("layouts/no-jobs-reporting.yaml")
def test_no_jobs_reporting_check_and_gate(self):
# Test that we don't report NO_JOBS results
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.fake_gerrit.addEvent(A.addApproval("Approved", 1))
self.waitUntilSettled()
self.assertEqual(0, A.reported)
self.assertHistory([])
tenant = self.scheds.first.sched.abide.tenants.get('tenant-one')
pipeline = tenant.layout.pipelines['check']
reporter = self.scheds.first.connections.getSqlReporter(
pipeline)
with self.scheds.first.connections.getSqlConnection().\
engine.connect() as conn:
result = conn.execute(
sa.sql.select([reporter.connection.zuul_buildset_table]))
buildsets = result.fetchall()
for x in buildsets:
self.log.debug("Buildset %s", x)
self.assertEqual(0, len(buildsets))

33
zuul/manager/__init__.py
View File

@ -315,6 +315,13 @@ class PipelineManager(metaclass=ABCMeta):
self.log.error("Reporting item start %s received: %s" %
(item, ret))
def reportNormalBuildsetEnd(self, build_set, action, final, result=None):
# Report a buildset end, but only if there are jobs
if (build_set.job_graph and
len(build_set.job_graph.jobs) > 0):
self.sql.reportBuildsetEnd(build_set, action,
final, result)
def reportDequeue(self, item):
if not self.pipeline.state.disabled:
self.log.info(
@ -329,8 +336,8 @@ class PipelineManager(metaclass=ABCMeta):
)
# This might be called after canceljobs, which also sets a
# non-final 'cancel' result.
self.sql.reportBuildsetEnd(item.current_build_set, 'dequeue',
final=False)
self.reportNormalBuildsetEnd(item.current_build_set, 'dequeue',
final=False)
def sendReport(self, action_reporters, item, message=None):
"""Sends the built message off to configured reporters.
@ -613,6 +620,8 @@ class PipelineManager(metaclass=ABCMeta):
if self.pipeline.tenant.layout.getProjectPipelineConfig(ci):
self.sendReport(actions, ci)
self.dequeueItem(ci)
# We don't use reportNormalBuildsetEnd here because we want to
# report even with no jobs.
self.sql.reportBuildsetEnd(ci.current_build_set,
'failure', final=True)
@ -949,7 +958,7 @@ class PipelineManager(metaclass=ABCMeta):
item.didBundleStartReporting()):
# Force a dequeued result here because we haven't actually
# reported the item, but we are done with this buildset.
self.sql.reportBuildsetEnd(
self.reportNormalBuildsetEnd(
item.current_build_set, 'dequeue', final=False,
result='DEQUEUED')
item.resetAllBuilds()
@ -1610,8 +1619,8 @@ class PipelineManager(metaclass=ABCMeta):
# 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)
self.reportNormalBuildsetEnd(ri.current_build_set,
'failure', final=True)
def processQueue(self):
# Do whatever needs to be done for each change in the queue
@ -1868,8 +1877,8 @@ class PipelineManager(metaclass=ABCMeta):
item.change)
action = 'merge-failure'
item.setReportedResult('MERGE_FAILURE')
self.sql.reportBuildsetEnd(item.current_build_set,
action, final=True)
self.reportNormalBuildsetEnd(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
@ -1889,8 +1898,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)
self.reportNormalBuildsetEnd(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()
@ -1900,9 +1909,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 != 'no-jobs':
self.sql.reportBuildsetEnd(item.current_build_set,
action, final=True)
elif action:
self.reportNormalBuildsetEnd(item.current_build_set,
action, final=True)
def _reportItem(self, item):
log = get_annotated_logger(self.log, item.event)