zuul/zuul
Code Issues Proposed changes

Include skipped builds in database and web ui

Browse Source 
We have had an on-and-off relationship with skipped builds in the
database.  Generally we have attempted to exclude them from the db,
but we have occasionally (accidentally?) included them.  The status
quo is that builds with a result of SKIPPED (as well as several
other results which don't come from the executor) are not recorded
in the database.

With a greater interest in being able to determine which jobs ran
or did not run for a change after the fact, this job deliberately
adds all builds (whether they touch an executor or not, whether
real or not) to the database.  This means than anything that could
potentially show up on the status page or in a code-review report
will be in the database, and can therefore be seen in the web UI.

It is still the case that we are not actually interested in seeing
a page full of SKIPPED builds when we visit the "Builds" tab in
the web ui (which is the principal reason we have not included them
in the database so far).  To address this, we set the default query
in the builds tab to exclude skipped builds (it is easy to add other
types of builds to exclude in the future if we wish).  If a user
then specifies a query filter to *include* specific results, we drop
the exclusion from the query string.  This allows for the expected
behavior of not showing SKIPPED by default, then as specific results
are added to the filter, we show only those, and if the user selects
that they want to see SKIPPED, they will then be included.

On the buildset page, we add a switch similar to the current "show
retried jobs" switch that selects whether skipped builds in a buildset
should be displayed (again, it hides them by default).

Change-Id: I1835965101299bc7a95c952e99f6b0b095def085
This commit is contained in:
James E. Blair
2022-10-05 15:49:58 -07:00
parent b7c25e0f51
commit 0738d31b08
7 changed files with 112 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
tests/unit/test_web.py
View File

@@ -1783,6 +1783,45 @@ class TestBuildInfo(BaseTestWeb):
"idx_min=%i" % idx_max).json()
self.assertEqual(len(builds_query), 1, builds_query)
def test_web_list_skipped_builds(self):
# Test the exclude_result filter
# Generate some build records in the db.
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
self.executor_server.failJob('project-merge', A)
A.addApproval('Code-Review', 2)
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
builds = self.get_url("api/tenant/tenant-one/builds").json()
builds.sort(key=lambda x: x['job_name'])
self.assertEqual(len(builds), 3)
self.assertEqual(builds[0]['job_name'], 'project-merge')
self.assertEqual(builds[1]['job_name'], 'project-test1')
self.assertEqual(builds[2]['job_name'], 'project-test2')
self.assertEqual(builds[0]['result'], 'FAILURE')
self.assertEqual(builds[1]['result'], 'SKIPPED')
self.assertEqual(builds[2]['result'], 'SKIPPED')
builds = self.get_url("api/tenant/tenant-one/builds?"
"exclude_result=SKIPPED").json()
self.assertEqual(len(builds), 1)
self.assertEqual(builds[0]['job_name'], 'project-merge')
self.assertEqual(builds[0]['result'], 'FAILURE')
builds = self.get_url("api/tenant/tenant-one/builds?"
"result=SKIPPED&result=FAILURE").json()
builds.sort(key=lambda x: x['job_name'])
self.assertEqual(len(builds), 3)
self.assertEqual(builds[0]['job_name'], 'project-merge')
self.assertEqual(builds[1]['job_name'], 'project-test1')
self.assertEqual(builds[2]['job_name'], 'project-test2')
self.assertEqual(builds[0]['result'], 'FAILURE')
self.assertEqual(builds[1]['result'], 'SKIPPED')
self.assertEqual(builds[2]['result'], 'SKIPPED')
def test_web_badge(self):
# Generate some build records in the db.
self.add_base_changes()

7
web/src/containers/FilterToolbar.jsx
View File

@@ -319,14 +319,21 @@ function writeFiltersToUrl(filters, location, history) {
function buildQueryString(filters) {
let queryString = '&complete=true'
let resultFilter = false
if (filters) {
Object.keys(filters).map((key) => {
filters[key].forEach((value) => {
if (value === 'result') {
resultFilter = true
}
queryString += '&' + key + '=' + value
})
return queryString
})
}
if (!resultFilter) {
queryString += '&exclude_result=SKIPPED'
}
return queryString
}

32
web/src/containers/build/BuildList.jsx
View File

@@ -57,15 +57,24 @@ class BuildList extends React.Component {
return self.indexOf(build) === idx
})
let skippedJobs = builds.filter((build) => {
return build.result === 'SKIPPED'
}).map((build) => (build.job_name)
).filter((build, idx, self) => {
return self.indexOf(build) === idx
})
this.state = {
visibleNonFinalBuilds: retriedJobs,
retriedJobs: retriedJobs,
skippedJobs: skippedJobs,
showSkipped: false,
}
}
sortedBuilds = () => {
const { builds } = this.props
const { visibleNonFinalBuilds } = this.state
const { visibleNonFinalBuilds, showSkipped } = this.state
return builds.sort((a, b) => {
// Group builds by job name, then order by decreasing start time; this will ensure retries are together
@@ -85,6 +94,9 @@ class BuildList extends React.Component {
}
}).filter((build) => {
if (build.final || visibleNonFinalBuilds.indexOf(build.job_name) >= 0) {
if (build.result === 'SKIPPED' && !showSkipped) {
return false
}
return true
}
else {
@@ -98,6 +110,10 @@ class BuildList extends React.Component {
this.setState({ visibleNonFinalBuilds: (isChecked ? retriedJobs : []) })
}
handleSkippedSwitch = isChecked => {
this.setState({ showSkipped: isChecked })
}
handleToggleVisibleNonFinalBuilds = (jobName) => {
const { visibleNonFinalBuilds } = this.state
const index = visibleNonFinalBuilds.indexOf(jobName)
@@ -138,7 +154,7 @@ class BuildList extends React.Component {
render() {
const { tenant } = this.props
const { visibleNonFinalBuilds, retriedJobs } = this.state
const { visibleNonFinalBuilds, retriedJobs, skippedJobs, showSkipped } = this.state
let retrySwitch = retriedJobs.length > 0 ?
<FlexItem align={{ default: 'alignRight' }}>
@@ -151,10 +167,22 @@ class BuildList extends React.Component {
</FlexItem> :
<></>
let skippedSwitch = skippedJobs.length > 0 ?
<FlexItem align={{ default: 'alignRight' }}>
<span>Show skipped jobs &nbsp;</span>
<Switch
isChecked={showSkipped}
onChange={this.handleSkippedSwitch}
isReversed
/>
</FlexItem> :
<></>
const sortedBuilds = this.sortedBuilds()
return (
<Flex direction={{ default: 'column' }}>
{skippedSwitch}
{retrySwitch}
<FlexItem>
<DataList

12
zuul/driver/sql/sqlconnection.py
View File

@@ -59,6 +59,14 @@ class DatabaseSession(object):
return query.filter(column.in_(value))
return query.filter(column == value)
def exListFilter(self, query, column, value):
# Exclude values in list
if value is None:
return query
if isinstance(value, list) or isinstance(value, tuple):
return query.filter(column.not_in(value))
return query.filter(column != value)
def getBuilds(self, tenant=None, project=None, pipeline=None,
change=None, branch=None, patchset=None, ref=None,
newrev=None, event_id=None, event_timestamp=None,
@@ -66,7 +74,8 @@ class DatabaseSession(object):
uuid=None, job_name=None, voting=None, nodeset=None,
result=None, provides=None, final=None, held=None,
complete=None, sort_by_buildset=False, limit=50,
offset=0, idx_min=None, idx_max=None):
offset=0, idx_min=None, idx_max=None,
exclude_result=None):
build_table = self.connection.zuul_build_table
buildset_table = self.connection.zuul_buildset_table
@@ -114,6 +123,7 @@ class DatabaseSession(object):
q = self.listFilter(q, build_table.c.voting, voting)
q = self.listFilter(q, build_table.c.nodeset, nodeset)
q = self.listFilter(q, build_table.c.result, result)
q = self.exListFilter(q, build_table.c.result, exclude_result)
q = self.listFilter(q, build_table.c.final, final)
if complete is True:
q = q.filter(build_table.c.result != None) # noqa

8
zuul/manager/__init__.py
View File

@@ -1965,13 +1965,7 @@ class PipelineManager(metaclass=ABCMeta):
build_set.jobNodeRequestComplete(request.job_name, nodeset)
# Put a fake build through the cycle to clean it up.
if not request.fulfilled:
fakebuild = build_set.item.setNodeRequestFailure(job)
try:
self.sql.reportBuildEnd(
fakebuild, tenant=build_set.item.pipeline.tenant.name,
final=True)
except Exception:
log.exception("Error reporting build completion to DB:")
build_set.item.setNodeRequestFailure(job)
self._resumeBuilds(build_set)
tenant = build_set.item.pipeline.tenant
tenant.semaphore_handler.release(

21
zuul/model.py
View File

@@ -4921,6 +4921,10 @@ class QueueItem(zkobject.ZKObject):
job=job, build_set=self.current_build_set,
result='FAILURE')
self.addBuild(fakebuild)
self.pipeline.manager.sql.reportBuildEnd(
fakebuild,
tenant=self.pipeline.tenant.name,
final=True)
self.setResult(fakebuild)
ret = False
return ret
@@ -5254,6 +5258,10 @@ class QueueItem(zkobject.ZKObject):
build_set=self.current_build_set,
result='SKIPPED')
self.addBuild(fakebuild)
self.pipeline.manager.sql.reportBuildEnd(
fakebuild,
tenant=self.pipeline.tenant.name,
final=True)
def setNodeRequestFailure(self, job):
fakebuild = Build.new(
@@ -5265,8 +5273,11 @@ class QueueItem(zkobject.ZKObject):
result='NODE_FAILURE',
)
self.addBuild(fakebuild)
self.pipeline.manager.sql.reportBuildEnd(
fakebuild,
tenant=self.pipeline.tenant.name,
final=True)
self.setResult(fakebuild)
return fakebuild
def setDequeuedNeedingChange(self, msg):
self.updateAttributes(
@@ -5322,6 +5333,10 @@ class QueueItem(zkobject.ZKObject):
job=job, build_set=self.current_build_set,
result='SKIPPED')
self.addBuild(fakebuild)
self.pipeline.manager.sql.reportBuildEnd(
fakebuild,
tenant=self.pipeline.tenant.name,
final=True)
def _setMissingJobsSkipped(self):
for job in self.getJobs():
@@ -5332,6 +5347,10 @@ class QueueItem(zkobject.ZKObject):
job=job, build_set=self.current_build_set,
result='SKIPPED')
self.addBuild(fakebuild)
self.pipeline.manager.sql.reportBuildEnd(
fakebuild,
tenant=self.pipeline.tenant.name,
final=True)
def getNodePriority(self):
return self.pipeline.manager.getNodePriority(self)

6
zuul/web/__init__.py
View File

@@ -1399,7 +1399,8 @@ class ZuulWebAPI(object):
branch=None, patchset=None, ref=None, newrev=None,
uuid=None, job_name=None, voting=None, nodeset=None,
result=None, final=None, held=None, complete=None,
limit=50, skip=0, idx_min=None, idx_max=None):
limit=50, skip=0, idx_min=None, idx_max=None,
exclude_result=None):
connection = self._get_connection()
if tenant not in self.zuulweb.abide.tenants.keys():
@@ -1423,7 +1424,8 @@ class ZuulWebAPI(object):
branch=branch, patchset=patchset, ref=ref, newrev=newrev,
uuid=uuid, job_name=job_name, voting=voting, nodeset=nodeset,
result=result, final=final, held=held, complete=complete,
limit=limit, offset=skip, idx_min=_idx_min, idx_max=_idx_max)
limit=limit, offset=skip, idx_min=_idx_min, idx_max=_idx_max,
exclude_result=exclude_result)
resp = cherrypy.response
resp.headers['Access-Control-Allow-Origin'] = '*'