From 8284489e095cf2be26fc09361f71d20db0edc92d Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 6 Mar 2017 10:55:26 -0800 Subject: [PATCH] Port SQLAlchemy reporter to v3 driver structure This completes the merge from master and re-enables the sqlalchemy driver tests. Change-Id: I8e6effb3f0052343d5c6c80e9edaa2a9ff360b4d --- tests/base.py | 5 +- .../playbooks/project-merge.yaml | 2 + .../playbooks/project-test1.yaml | 2 + .../playbooks/project-test2.yaml | 2 + .../sql-driver/git/common-config/zuul.yaml | 38 +++++++++ .../config/sql-driver/git/org_project/README | 1 + tests/fixtures/config/sql-driver/main.yaml | 8 ++ tests/fixtures/layout-sql-reporter.yaml | 27 ------ .../zuul-connections-same-gerrit.conf | 9 -- ...-bad-sql.conf => zuul-sql-driver-bad.conf} | 24 ++---- tests/fixtures/zuul-sql-driver.conf | 40 +++++++++ tests/unit/test_connection.py | 83 ++++++++----------- zuul/driver/sql/__init__.py | 33 ++++++++ .../sql.py => driver/sql/sqlconnection.py} | 5 +- .../sql.py => driver/sql/sqlreporter.py} | 15 ++-- zuul/lib/connections.py | 2 + 16 files changed, 183 insertions(+), 113 deletions(-) create mode 100644 tests/fixtures/config/sql-driver/git/common-config/playbooks/project-merge.yaml create mode 100644 tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test1.yaml create mode 100644 tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test2.yaml create mode 100644 tests/fixtures/config/sql-driver/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/sql-driver/git/org_project/README create mode 100644 tests/fixtures/config/sql-driver/main.yaml delete mode 100644 tests/fixtures/layout-sql-reporter.yaml rename tests/fixtures/{zuul-connections-bad-sql.conf => zuul-sql-driver-bad.conf} (58%) create mode 100644 tests/fixtures/zuul-sql-driver.conf create mode 100644 zuul/driver/sql/__init__.py rename zuul/{connection/sql.py => driver/sql/sqlconnection.py} (94%) rename zuul/{reporter/sql.py => driver/sql/sqlreporter.py} (86%) diff --git a/tests/base.py b/tests/base.py index 2729233063..29981acff4 100755 --- a/tests/base.py +++ b/tests/base.py @@ -53,7 +53,6 @@ from git.exc import NoSuchPathError import zuul.driver.gerrit.gerritsource as gerritsource import zuul.driver.gerrit.gerritconnection as gerritconnection -import zuul.connection.sql import zuul.scheduler import zuul.webapp import zuul.rpclistener @@ -1918,8 +1917,8 @@ class AnsibleZuulTestCase(ZuulTestCase): class ZuulDBTestCase(ZuulTestCase): - def setup_config(self, config_file='zuul-connections-same-gerrit.conf'): - super(ZuulDBTestCase, self).setup_config(config_file) + def setup_config(self): + super(ZuulDBTestCase, self).setup_config() for section_name in self.config.sections(): con_match = re.match(r'^connection ([\'\"]?)(.*)(\1)$', section_name, re.I) diff --git a/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-merge.yaml b/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-merge.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-merge.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test1.yaml b/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test1.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test1.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test2.yaml b/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test2.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/sql-driver/git/common-config/playbooks/project-test2.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/sql-driver/git/common-config/zuul.yaml b/tests/fixtures/config/sql-driver/git/common-config/zuul.yaml new file mode 100644 index 0000000000..36c7602cbf --- /dev/null +++ b/tests/fixtures/config/sql-driver/git/common-config/zuul.yaml @@ -0,0 +1,38 @@ +- pipeline: + name: check + manager: independent + source: gerrit + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + verified: 1 + resultsdb: + score: 1 + failure: + gerrit: + verified: -1 + resultsdb: + score: -1 + resultsdb_failures: + score: -1 + +- job: + name: project-merge + +- job: + name: project-test1 + +- job: + name: project-test2 + +- project: + name: org/project + check: + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge diff --git a/tests/fixtures/config/sql-driver/git/org_project/README b/tests/fixtures/config/sql-driver/git/org_project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/sql-driver/git/org_project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/sql-driver/main.yaml b/tests/fixtures/config/sql-driver/main.yaml new file mode 100644 index 0000000000..d9868fad0f --- /dev/null +++ b/tests/fixtures/config/sql-driver/main.yaml @@ -0,0 +1,8 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-repos: + - common-config + project-repos: + - org/project diff --git a/tests/fixtures/layout-sql-reporter.yaml b/tests/fixtures/layout-sql-reporter.yaml deleted file mode 100644 index c79a4328d9..0000000000 --- a/tests/fixtures/layout-sql-reporter.yaml +++ /dev/null @@ -1,27 +0,0 @@ -pipelines: - - name: check - manager: IndependentPipelineManager - source: - review_gerrit - trigger: - review_gerrit: - - event: patchset-created - success: - review_gerrit: - verified: 1 - resultsdb: - score: 1 - failure: - review_gerrit: - verified: -1 - resultsdb: - score: -1 - resultsdb_failures: - score: -1 - -projects: - - name: org/project - check: - - project-merge: - - project-test1 - - project-test2 diff --git a/tests/fixtures/zuul-connections-same-gerrit.conf b/tests/fixtures/zuul-connections-same-gerrit.conf index 69f5239826..6156df44c0 100644 --- a/tests/fixtures/zuul-connections-same-gerrit.conf +++ b/tests/fixtures/zuul-connections-same-gerrit.conf @@ -33,12 +33,3 @@ server=localhost port=25 default_from=zuul@example.com default_to=you@example.com - -# TODOv3(jeblair): commented out until sqlalchemy conenction ported to -# v3 driver syntax -#[connection resultsdb] driver=sql -#dburi=$MYSQL_FIXTURE_DBURI$ - -#[connection resultsdb_failures] -#driver=sql -#dburi=$MYSQL_FIXTURE_DBURI$ diff --git a/tests/fixtures/zuul-connections-bad-sql.conf b/tests/fixtures/zuul-sql-driver-bad.conf similarity index 58% rename from tests/fixtures/zuul-connections-bad-sql.conf rename to tests/fixtures/zuul-sql-driver-bad.conf index 2d1e804b19..d91e2f6a07 100644 --- a/tests/fixtures/zuul-connections-bad-sql.conf +++ b/tests/fixtures/zuul-sql-driver-bad.conf @@ -2,34 +2,24 @@ server=127.0.0.1 [zuul] -layout_config=layout-connections-multiple-voters.yaml +tenant_config=main.yaml url_pattern=http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number} job_name_in_report=true [merger] -git_dir=/tmp/zuul-test/git +git_dir=/tmp/zuul-test/merger-git git_user_email=zuul@example.com git_user_name=zuul zuul_url=http://zuul.example.com/p -[connection review_gerrit] +[executor] +git_dir=/tmp/zuul-test/executor-git + +[connection gerrit] driver=gerrit server=review.example.com user=jenkins -sshkey=none - -[connection alt_voting_gerrit] -driver=gerrit -server=alt_review.example.com -user=civoter -sshkey=none - -[connection outgoing_smtp] -driver=smtp -server=localhost -port=25 -default_from=zuul@example.com -default_to=you@example.com +sshkey=fake_id_rsa1 [connection resultsdb] driver=sql diff --git a/tests/fixtures/zuul-sql-driver.conf b/tests/fixtures/zuul-sql-driver.conf new file mode 100644 index 0000000000..b3f7f9eb08 --- /dev/null +++ b/tests/fixtures/zuul-sql-driver.conf @@ -0,0 +1,40 @@ +[gearman] +server=127.0.0.1 + +[zuul] +tenant_config=main.yaml +url_pattern=http://logs.example.com/{change.number}/{change.patchset}/{pipeline.name}/{job.name}/{build.number} +job_name_in_report=true + +[merger] +git_dir=/tmp/zuul-test/merger-git +git_user_email=zuul@example.com +git_user_name=zuul +zuul_url=http://zuul.example.com/p + +[executor] +git_dir=/tmp/zuul-test/executor-git + +[swift] +authurl=https://identity.api.example.org/v2.0/ +user=username +key=password +tenant_name=" " + +default_container=logs +region_name=EXP +logserver_prefix=http://logs.example.org/server.app/ + +[connection gerrit] +driver=gerrit +server=review.example.com +user=jenkins +sshkey=fake_id_rsa1 + +[connection resultsdb] +driver=sql +dburi=$MYSQL_FIXTURE_DBURI$ + +[connection resultsdb_failures] +driver=sql +dburi=$MYSQL_FIXTURE_DBURI$ diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py index 22cf331039..ee9a0b01cc 100644 --- a/tests/unit/test_connection.py +++ b/tests/unit/test_connection.py @@ -13,7 +13,6 @@ # under the License. import sqlalchemy as sa -from unittest import skip from tests.base import ZuulTestCase, ZuulDBTestCase @@ -57,45 +56,44 @@ class TestConnections(ZuulTestCase): self.assertEqual(B.patchsets[-1]['approvals'][0]['by']['username'], 'civoter') - def _test_sql_tables_created(self, metadata_table=None): + +class TestSQLConnection(ZuulDBTestCase): + config_file = 'zuul-sql-driver.conf' + tenant_config_file = 'config/sql-driver/main.yaml' + + def test_sql_tables_created(self, metadata_table=None): "Test the tables for storing results are created properly" buildset_table = 'zuul_buildset' build_table = 'zuul_build' insp = sa.engine.reflection.Inspector( - self.connections['resultsdb'].engine) + self.connections.connections['resultsdb'].engine) self.assertEqual(9, len(insp.get_columns(buildset_table))) self.assertEqual(10, len(insp.get_columns(build_table))) - @skip("Disabled for early v3 development") - def test_sql_tables_created(self): - "Test the default table is created" - self.config.set('zuul', 'layout_config', - 'tests/fixtures/layout-sql-reporter.yaml') - self.sched.reconfigure(self.config) - self._test_sql_tables_created() - - def _test_sql_results(self): + def test_sql_results(self): "Test results are entered into an sql table" # Grab the sa tables + tenant = self.sched.abide.tenants.get('tenant-one') reporter = _get_reporter_from_connection_name( - self.sched.layout.pipelines['check'].success_actions, + tenant.layout.pipelines['check'].success_actions, 'resultsdb' ) # Add a success result - A = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'A') - self.fake_review_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) self.waitUntilSettled() # Add a failed result for a negative score - B = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'B') - self.worker.addFailTest('project-test1', B) - self.fake_review_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') + + self.executor_server.failJob('project-test1', B) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) self.waitUntilSettled() - conn = self.connections['resultsdb'].engine.connect() + conn = self.connections.connections['resultsdb'].engine.connect() result = conn.execute( sa.sql.select([reporter.connection.zuul_buildset_table])) @@ -122,7 +120,7 @@ class TestConnections(ZuulTestCase): # Check the first result, which should be the project-merge job self.assertEqual('project-merge', buildset0_builds[0]['job_name']) self.assertEqual("SUCCESS", buildset0_builds[0]['result']) - self.assertEqual('http://logs.example.com/1/1/check/project-merge/0', + self.assertEqual('https://server/job/project-merge/0/', buildset0_builds[0]['log_url']) self.assertEqual('check', buildset1['pipeline']) @@ -144,42 +142,33 @@ class TestConnections(ZuulTestCase): # which failed self.assertEqual('project-test1', buildset1_builds[-2]['job_name']) self.assertEqual("FAILURE", buildset1_builds[-2]['result']) - self.assertEqual('http://logs.example.com/2/1/check/project-test1/4', + self.assertEqual('https://server/job/project-test1/0/', buildset1_builds[-2]['log_url']) - @skip("Disabled for early v3 development") - def test_sql_results(self): - "Test results are entered into the default sql table" - self.config.set('zuul', 'layout_config', - 'tests/fixtures/layout-sql-reporter.yaml') - self.sched.reconfigure(self.config) - self._test_sql_results() - - @skip("Disabled for early v3 development") def test_multiple_sql_connections(self): "Test putting results in different databases" - self.config.set('zuul', 'layout_config', - 'tests/fixtures/layout-sql-reporter.yaml') - self.sched.reconfigure(self.config) + self.updateConfigLayout( + 'tests/fixtures/layout-sql-reporter.yaml') # Add a successful result - A = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'A') - self.fake_review_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) self.waitUntilSettled() # Add a failed result - B = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'B') - self.worker.addFailTest('project-test1', B) - self.fake_review_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') + self.executor_server.failJob('project-test1', B) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) self.waitUntilSettled() # Grab the sa tables for resultsdb + tenant = self.sched.abide.tenants.get('tenant-one') reporter1 = _get_reporter_from_connection_name( - self.sched.layout.pipelines['check'].success_actions, + tenant.layout.pipelines['check'].success_actions, 'resultsdb' ) - conn = self.connections['resultsdb'].engine.connect() + conn = self.connections.connections['resultsdb'].engine.connect() buildsets_resultsdb = conn.execute(sa.sql.select( [reporter1.connection.zuul_buildset_table])).fetchall() # Should have been 2 buildset reported to the resultsdb (both success @@ -196,11 +185,12 @@ class TestConnections(ZuulTestCase): # Grab the sa tables for resultsdb_failures reporter2 = _get_reporter_from_connection_name( - self.sched.layout.pipelines['check'].failure_actions, + tenant.layout.pipelines['check'].failure_actions, 'resultsdb_failures' ) - conn = self.connections['resultsdb_failures'].engine.connect() + conn = self.connections.connections['resultsdb_failures'].\ + engine.connect() buildsets_resultsdb_failures = conn.execute(sa.sql.select( [reporter2.connection.zuul_buildset_table])).fetchall() # The failure db should only have 1 buildset failed @@ -217,10 +207,9 @@ class TestConnections(ZuulTestCase): class TestConnectionsBadSQL(ZuulDBTestCase): - def setup_config(self, config_file='zuul-connections-bad-sql.conf'): - super(TestConnectionsBadSQL, self).setup_config(config_file) + config_file = 'zuul-sql-driver-bad.conf' + tenant_config_file = 'config/sql-driver/main.yaml' - @skip("Disabled for early v3 development") def test_unable_to_connect(self): "Test the SQL reporter fails gracefully when unable to connect" self.config.set('zuul', 'layout_config', @@ -229,8 +218,8 @@ class TestConnectionsBadSQL(ZuulDBTestCase): # Trigger a reporter. If no errors are raised, the reporter has been # disabled correctly - A = self.fake_review_gerrit.addFakeChange('org/project', 'master', 'A') - self.fake_review_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) self.waitUntilSettled() diff --git a/zuul/driver/sql/__init__.py b/zuul/driver/sql/__init__.py new file mode 100644 index 0000000000..a5f8923773 --- /dev/null +++ b/zuul/driver/sql/__init__.py @@ -0,0 +1,33 @@ +# Copyright 2016 Red Hat, Inc. +# +# 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. + +from zuul.driver import Driver, ConnectionInterface, ReporterInterface +import sqlconnection +import sqlreporter + + +class SQLDriver(Driver, ConnectionInterface, ReporterInterface): + name = 'sql' + + def registerScheduler(self, scheduler): + self.sched = scheduler + + def getConnection(self, name, config): + return sqlconnection.SQLConnection(self, name, config) + + def getReporter(self, connection, config=None): + return sqlreporter.SQLReporter(self, connection, config) + + def getReporterSchema(self): + return sqlreporter.getSchema() diff --git a/zuul/connection/sql.py b/zuul/driver/sql/sqlconnection.py similarity index 94% rename from zuul/connection/sql.py rename to zuul/driver/sql/sqlconnection.py index 479ee443c9..bedfaf3e52 100644 --- a/zuul/connection/sql.py +++ b/zuul/driver/sql/sqlconnection.py @@ -29,9 +29,10 @@ class SQLConnection(BaseConnection): driver_name = 'sql' log = logging.getLogger("connection.sql") - def __init__(self, connection_name, connection_config): + def __init__(self, driver, connection_name, connection_config): - super(SQLConnection, self).__init__(connection_name, connection_config) + super(SQLConnection, self).__init__(driver, connection_name, + connection_config) self.dburi = None self.engine = None diff --git a/zuul/reporter/sql.py b/zuul/driver/sql/sqlreporter.py similarity index 86% rename from zuul/reporter/sql.py rename to zuul/driver/sql/sqlreporter.py index b663a59757..2129f53619 100644 --- a/zuul/reporter/sql.py +++ b/zuul/driver/sql/sqlreporter.py @@ -25,10 +25,10 @@ class SQLReporter(BaseReporter): name = 'sql' log = logging.getLogger("zuul.reporter.mysql.SQLReporter") - def __init__(self, reporter_config={}, sched=None, connection=None): + def __init__(self, driver, connection, config={}): super(SQLReporter, self).__init__( - reporter_config, sched, connection) - self.result_score = reporter_config.get('score', None) + driver, connection, config) + self.result_score = config.get('score', None) def report(self, source, pipeline, item): """Create an entry into a database.""" @@ -37,13 +37,12 @@ class SQLReporter(BaseReporter): self.log.warn("SQL reporter (%s) is disabled " % self) return - if self.sched.config.has_option('zuul', 'url_pattern'): - url_pattern = self.sched.config.get('zuul', 'url_pattern') + if self.driver.sched.config.has_option('zuul', 'url_pattern'): + url_pattern = self.driver.sched.config.get('zuul', 'url_pattern') else: url_pattern = None - score = self.reporter_config['score']\ - if 'score' in self.reporter_config else 0 + score = self.config.get('score', 0) with self.connection.engine.begin() as conn: buildset_ins = self.connection.zuul_buildset_table.insert().values( @@ -60,7 +59,7 @@ class SQLReporter(BaseReporter): buildset_ins_result = conn.execute(buildset_ins) build_inserts = [] - for job in pipeline.getJobs(item): + for job in item.getJobs(): build = item.current_build_set.getBuild(job.name) if not build: # build hasn't began. The sql reporter can only send back diff --git a/zuul/lib/connections.py b/zuul/lib/connections.py index 27d8a1bedd..9964ba9694 100644 --- a/zuul/lib/connections.py +++ b/zuul/lib/connections.py @@ -20,6 +20,7 @@ import zuul.driver.gerrit import zuul.driver.git import zuul.driver.smtp import zuul.driver.timer +import zuul.driver.sql from zuul.connection import BaseConnection @@ -41,6 +42,7 @@ class ConnectionRegistry(object): self.registerDriver(zuul.driver.git.GitDriver()) self.registerDriver(zuul.driver.smtp.SMTPDriver()) self.registerDriver(zuul.driver.timer.TimerDriver()) + self.registerDriver(zuul.driver.sql.SQLDriver()) def registerDriver(self, driver): if driver.name in self.drivers: