From 25ccee91afe60b2fec4dd9f66048080bf04e28c1 Mon Sep 17 00:00:00 2001 From: Benedikt Loeffler Date: Mon, 28 Jan 2019 14:38:15 +0100 Subject: [PATCH] Report retried builds via sql reporter. Since we added those to the MQTT reporter, we should also store them in the SQL database. They are stored in the zuul_build table and can be identified via the new "final" column which is set to False for those builds (and True for all others). The final flag can also be used in the API to specifically filter for those builds or remove them from the result. By default, no builds are filtered out. The buildset API includes these retried builds under a dedicated 'retry_builds' key to not mix them up with the final builds. Thus, the JSON format is equally to the one the MQTT reporter uses. For the migration of the SQL database, all existing builds will be set to final. We could also provide a filter mechanism via the web UI, but that should be part of a separate change (e.g. add a checkbox next to the search filter saying "Show retried builds"). Change-Id: I5960df8a997c7ab81a07b9bd8631c14dbe22b8ab --- .../non-final-builds-d9024df868c7e036.yaml | 9 ++ tests/unit/test_connection.py | 83 ++++++++++++++++++- .../269691d2220e_add_build_final_column.py | 43 ++++++++++ zuul/driver/sql/sqlconnection.py | 4 +- zuul/driver/sql/sqlreporter.py | 72 ++++++++++------ zuul/web/__init__.py | 16 +++- 6 files changed, 195 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/non-final-builds-d9024df868c7e036.yaml create mode 100644 zuul/driver/sql/alembic/versions/269691d2220e_add_build_final_column.py diff --git a/releasenotes/notes/non-final-builds-d9024df868c7e036.yaml b/releasenotes/notes/non-final-builds-d9024df868c7e036.yaml new file mode 100644 index 0000000000..30b83b81e7 --- /dev/null +++ b/releasenotes/notes/non-final-builds-d9024df868c7e036.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + The builds/ and buildset/ API endpoints now include information about + retried builds. They are called non-final as those are all builds that + were retried at least once and thus weren't visible to the user so far. + + The builds/ API can filter those retried builds and you might exclude + them from API result by setting ``final=false`` in the API request. diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py index d9b84fa6dd..4f15d888db 100644 --- a/tests/unit/test_connection.py +++ b/tests/unit/test_connection.py @@ -75,7 +75,7 @@ class TestSQLConnection(ZuulDBTestCase): build_table = table_prefix + 'zuul_build' self.assertEqual(16, len(insp.get_columns(buildset_table))) - self.assertEqual(11, len(insp.get_columns(build_table))) + self.assertEqual(12, len(insp.get_columns(build_table))) def test_sql_tables_created(self): "Test the tables for storing results are created properly" @@ -226,6 +226,87 @@ class TestSQLConnection(ZuulDBTestCase): check_results('resultsdb_mysql') check_results('resultsdb_postgresql') + def test_sql_results_retry_builds(self): + "Test that retry results are entered into an sql table correctly" + + # Check the results + def check_results(connection_name): + # Grab the sa tables + tenant = self.scheds.first.sched.abide.tenants.get("tenant-one") + reporter = _get_reporter_from_connection_name( + tenant.layout.pipelines["check"].success_actions, + connection_name + ) + + with self.connections.connections[ + connection_name].engine.connect() as conn: + + result = conn.execute( + sa.sql.select([reporter.connection.zuul_buildset_table]) + ) + + buildsets = result.fetchall() + self.assertEqual(1, len(buildsets)) + buildset0 = buildsets[0] + + self.assertEqual('check', buildset0['pipeline']) + self.assertEqual('org/project', buildset0['project']) + self.assertEqual(1, buildset0['change']) + self.assertEqual('1', buildset0['patchset']) + self.assertEqual('SUCCESS', buildset0['result']) + self.assertEqual('Build succeeded.', buildset0['message']) + self.assertEqual('tenant-one', buildset0['tenant']) + self.assertEqual( + 'https://review.example.com/%d' % buildset0['change'], + buildset0['ref_url']) + + buildset0_builds = conn.execute( + sa.sql.select( + [reporter.connection.zuul_build_table] + ).where( + reporter.connection.zuul_build_table.c.buildset_id == + buildset0['id'] + ) + ).fetchall() + + # Check the retry results + self.assertEqual('project-merge', buildset0_builds[0]['job_name']) + self.assertEqual('SUCCESS', buildset0_builds[0]['result']) + self.assertTrue(buildset0_builds[0]['final']) + + self.assertEqual('project-test1', buildset0_builds[1]['job_name']) + self.assertEqual('RETRY', buildset0_builds[1]['result']) + self.assertFalse(buildset0_builds[1]['final']) + self.assertEqual('project-test1', buildset0_builds[2]['job_name']) + self.assertEqual('SUCCESS', buildset0_builds[2]['result']) + self.assertTrue(buildset0_builds[2]['final']) + + self.assertEqual('project-test2', buildset0_builds[3]['job_name']) + self.assertEqual('RETRY', buildset0_builds[3]['result']) + self.assertFalse(buildset0_builds[3]['final']) + self.assertEqual('project-test2', buildset0_builds[4]['job_name']) + self.assertEqual('SUCCESS', buildset0_builds[4]['result']) + self.assertTrue(buildset0_builds[4]['final']) + + self.executor_server.hold_jobs_in_build = True + + # Add a retry result + self.log.debug("Adding retry FakeChange") + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + # Release the merge job (which is the dependency for the other jobs) + self.executor_server.release('.*-merge') + self.waitUntilSettled() + # Let both test jobs fail on the first run, so they are both run again. + self.builds[0].requeue = True + self.builds[1].requeue = True + self.orderedRelease() + self.waitUntilSettled() + + check_results('resultsdb_mysql') + check_results('resultsdb_postgresql') + def test_multiple_sql_connections(self): "Test putting results in different databases" # Add a successful result diff --git a/zuul/driver/sql/alembic/versions/269691d2220e_add_build_final_column.py b/zuul/driver/sql/alembic/versions/269691d2220e_add_build_final_column.py new file mode 100644 index 0000000000..7711ccfd74 --- /dev/null +++ b/zuul/driver/sql/alembic/versions/269691d2220e_add_build_final_column.py @@ -0,0 +1,43 @@ +# 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. + +"""add build final column + +Revision ID: 269691d2220e +Revises: e0eda5d09eae +Create Date: 2020-01-03 07:53:15.962739 + +""" + +# revision identifiers, used by Alembic. +revision = '269691d2220e' +down_revision = '16c1dc9054d0' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + +BUILD_TABLE = 'zuul_build' + + +def upgrade(table_prefix=''): + op.add_column(table_prefix + BUILD_TABLE, sa.Column('final', sa.Boolean)) + + # Set all existing build entries to final (otherwise they will vanish from + # the UI) + new_column = sa.table(table_prefix + BUILD_TABLE, sa.column('final')) + op.execute(new_column.update().values(**{'final': True})) + + +def downgrade(): + raise Exception("Downgrades not supported") diff --git a/zuul/driver/sql/sqlconnection.py b/zuul/driver/sql/sqlconnection.py index 985fd5329e..cd820004f7 100644 --- a/zuul/driver/sql/sqlconnection.py +++ b/zuul/driver/sql/sqlconnection.py @@ -60,7 +60,7 @@ class DatabaseSession(object): change=None, branch=None, patchset=None, ref=None, newrev=None, event_id=None, uuid=None, job_name=None, voting=None, node_name=None, result=None, provides=None, - limit=50, offset=0): + final=None, limit=50, offset=0): build_table = self.connection.zuul_build_table buildset_table = self.connection.zuul_buildset_table @@ -102,6 +102,7 @@ class DatabaseSession(object): q = self.listFilter(q, build_table.c.voting, voting) q = self.listFilter(q, build_table.c.node_name, node_name) q = self.listFilter(q, build_table.c.result, result) + q = self.listFilter(q, build_table.c.final, final) q = self.listFilter(q, provides_table.c.name, provides) q = q.order_by(build_table.c.id.desc()).\ @@ -301,6 +302,7 @@ class SQLConnection(BaseConnection): log_url = sa.Column(sa.String(255)) node_name = sa.Column(sa.String(255)) error_detail = sa.Column(sa.TEXT()) + final = sa.Column(sa.Boolean) buildset = orm.relationship(BuildSetModel, backref="builds") def createArtifact(self, *args, **kw): diff --git a/zuul/driver/sql/sqlreporter.py b/zuul/driver/sql/sqlreporter.py index 951861cdc0..1c4caa891e 100644 --- a/zuul/driver/sql/sqlreporter.py +++ b/zuul/driver/sql/sqlreporter.py @@ -29,6 +29,43 @@ class SQLReporter(BaseReporter): name = 'sql' log = logging.getLogger("zuul.SQLReporter") + def _getBuildData(self, item, job, build): + (result, url) = item.formatJobResult(job, build) + log_url = build.result_data.get('zuul', {}).get('log_url') + if log_url and log_url[-1] != '/': + log_url = log_url + '/' + start = end = None + if build.start_time: + start = datetime.datetime.fromtimestamp( + build.start_time, + tz=datetime.timezone.utc) + if build.end_time: + end = datetime.datetime.fromtimestamp( + build.end_time, + tz=datetime.timezone.utc) + return result, log_url, start, end + + def createBuildEntry(self, item, job, db_buildset, build, final=True): + # Ensure end_time is defined + if not build.end_time: + build.end_time = time.time() + + result, log_url, start, end = self._getBuildData(item, job, build) + db_build = db_buildset.createBuild( + uuid=build.uuid, + job_name=build.job.name, + result=result, + start_time=start, + end_time=end, + voting=build.job.voting, + log_url=log_url, + node_name=build.node_name, + error_detail=build.error_detail, + final=final, + ) + + return db_build + def report(self, item): """Create an entry into a database.""" log = get_annotated_logger(self.log, item.event) @@ -66,35 +103,16 @@ class SQLReporter(BaseReporter): # stats about builds. It doesn't understand how to store # information about the change. continue - # Ensure end_time is defined - if not build.end_time: - build.end_time = time.time() - (result, url) = item.formatJobResult(job) - log_url = build.result_data.get('zuul', {}).get('log_url') - if log_url and log_url[-1] != '/': - log_url = log_url + '/' - start = end = None - if build.start_time: - start = datetime.datetime.fromtimestamp( - build.start_time, - tz=datetime.timezone.utc) - if build.end_time: - end = datetime.datetime.fromtimestamp( - build.end_time, - tz=datetime.timezone.utc) - - db_build = db_buildset.createBuild( - uuid=build.uuid, - job_name=build.job.name, - result=result, - start_time=start, - end_time=end, - voting=build.job.voting, - log_url=log_url, - node_name=build.node_name, - error_detail=build.error_detail, + retry_builds = item.current_build_set.getRetryBuildsForJob( + job.name ) + for retry_build in retry_builds: + self.createBuildEntry( + item, job, db_buildset, retry_build, final=False + ) + + db_build = self.createBuildEntry(item, job, db_buildset, build) for provides in job.provides: db_build.createProvides(name=provides) diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py index 6c3b6a9339..89ca16928c 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -825,6 +825,7 @@ class ZuulWebAPI(object): 'log_url': build.log_url, 'node_name': build.node_name, 'error_detail': build.error_detail, + 'final': build.final, 'artifacts': [], 'provides': [], } @@ -876,14 +877,18 @@ class ZuulWebAPI(object): def builds(self, tenant, project=None, pipeline=None, change=None, branch=None, patchset=None, ref=None, newrev=None, uuid=None, job_name=None, voting=None, node_name=None, - result=None, limit=50, skip=0): + result=None, final=None, limit=50, skip=0): connection = self._get_connection(tenant) + # If final is None, we return all builds, both final and non-final + if final is not None: + final = final.lower() == "true" + builds = connection.getBuilds( tenant=tenant, project=project, pipeline=pipeline, change=change, branch=branch, patchset=patchset, ref=ref, newrev=newrev, uuid=uuid, job_name=job_name, voting=voting, node_name=node_name, - result=result, limit=limit, offset=skip) + result=result, final=final, limit=limit, offset=skip) resp = cherrypy.response resp.headers['Access-Control-Allow-Origin'] = '*' @@ -920,8 +925,13 @@ class ZuulWebAPI(object): } if builds: ret['builds'] = [] + ret['retry_builds'] = [] for build in builds: - ret['builds'].append(self.buildToDict(build)) + # Put all non-final (retry) builds under a different key + if not build.final: + ret['retry_builds'].append(self.buildToDict(build)) + else: + ret['builds'].append(self.buildToDict(build)) return ret @cherrypy.expose