From 41d8e478a5298699cef1e8364667945070e620d7 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 17 Jan 2022 13:15:58 -0800 Subject: [PATCH] Remove "sql connection" backwards compatability for database In 4.0 we deprecated connections using the "sql" driver in favor of using the new "database" config file section. Remove the backwards compatible handling of that so that "sql" connections or lack of "database" section report an error. Change-Id: I7e592cf5ff63f73f487e41bb6e3e4a4ae523e3fc --- doc/source/drivers/index.rst | 1 - doc/source/drivers/sql.rst | 93 ------------------- etc/zuul.conf-sample | 7 +- .../notes/last3x-8682ada4910fa750.yaml | 2 +- .../notes/postgres-ae4f8594d0f4b256.yaml | 2 +- .../report-build-page-2-49088c2b0a36e1b7.yaml | 2 +- .../notes/required-sql-7dd38b76d9f4273f.yaml | 2 +- ...l-connection-removal-8579399ab165abf9.yaml | 7 ++ ...version-table-prefix-c6a5e84851268f4d.yaml | 2 +- .../sql-driver/git/common-config/zuul.yaml | 4 - tests/fixtures/zuul-sql-driver-mysql.conf | 4 - tests/fixtures/zuul-sql-driver-postgres.conf | 4 - .../zuul-sql-driver-prefix-mysql.conf | 4 - .../zuul-sql-driver-prefix-postgres.conf | 4 - tests/unit/test_connection.py | 58 ------------ zuul/lib/connections.py | 8 +- 16 files changed, 16 insertions(+), 188 deletions(-) delete mode 100644 doc/source/drivers/sql.rst create mode 100644 releasenotes/notes/sql-connection-removal-8579399ab165abf9.yaml diff --git a/doc/source/drivers/index.rst b/doc/source/drivers/index.rst index 64eedbae04..0c3c7f901b 100644 --- a/doc/source/drivers/index.rst +++ b/doc/source/drivers/index.rst @@ -26,6 +26,5 @@ Zuul includes the following drivers: mqtt elasticsearch smtp - sql timer zuul diff --git a/doc/source/drivers/sql.rst b/doc/source/drivers/sql.rst deleted file mode 100644 index 8a9e0c067e..0000000000 --- a/doc/source/drivers/sql.rst +++ /dev/null @@ -1,93 +0,0 @@ -:title: SQL Driver - -.. _sql-driver: - -SQL -=== - -.. warning:: - - This driver is deprecated, use :attr:`database` configuration instead. - -The SQL driver supports reporters only. Only one connection per -database is permitted. - -Connection Configuration ------------------------- - -The connection options for the SQL driver are: - -.. attr:: - - .. attr:: driver - :required: - - .. value:: sql - - The connection must set ``driver=sql`` for SQL connections. - - .. attr:: dburi - :required: - - Database connection information in the form of a URI understood - by SQLAlchemy. See `The SQLAlchemy manual - `_ - for more information. - - The driver will automatically set up the database creating and managing - the necessary tables. Therefore the provided user should have sufficient - permissions to manage the database. For example: - - .. code-block:: sql - - GRANT ALL ON my_database TO 'my_user'@'%'; - - .. attr:: pool_recycle - :default: 1 - - Tune the pool_recycle value. See `The SQLAlchemy manual on pooling - `_ - for more information. - - .. attr:: table_prefix - :default: '' - - The string to prefix the table names. This makes it possible to run - several zuul deployments against the same database. This can be useful - if you rely on external databases which you don't have under control. - The default is to have no prefix. - -.. _sql_reporter: - -Reporter Configuration ----------------------- - -This reporter is used to store results in a database. - -A :ref:`connection` that uses the sql driver must be -supplied to the reporter. - -``zuul.conf`` contains the database connection and credentials. To -store different reports in different databases you'll need to create a -new connection per database. - -The SQL reporter does nothing on :attr:`pipeline.start` or -:attr:`pipeline.merge-failure`; it only acts on -:attr:`pipeline.success` or :attr:`pipeline.failure` reporting stages. - -For example: - -.. code-block:: yaml - - - pipeline: - name: post-merge - success: - mydb_conn: - failure: - mydb_conn: - -.. attr:: pipeline.. - - To report to a database, add a key with the connection name and an - empty value to the desired pipeline :ref:`reporter` - attributes. diff --git a/etc/zuul.conf-sample b/etc/zuul.conf-sample index 4da0445150..01967078ef 100644 --- a/etc/zuul.conf-sample +++ b/etc/zuul.conf-sample @@ -1,3 +1,6 @@ +[database] +dburi=mysql+pymysql://user@localhost/zuul + [statsd] server=127.0.0.1 @@ -69,7 +72,3 @@ server=localhost user=zuul password=zuul ;keepalive=60 - -[connection mydatabase] -driver=sql -dburi=mysql+pymysql://user@localhost/zuul diff --git a/releasenotes/notes/last3x-8682ada4910fa750.yaml b/releasenotes/notes/last3x-8682ada4910fa750.yaml index 215b328926..7adde1d630 100644 --- a/releasenotes/notes/last3x-8682ada4910fa750.yaml +++ b/releasenotes/notes/last3x-8682ada4910fa750.yaml @@ -13,7 +13,7 @@ prelude: | These features are supported now, so it is important that you take this time to :ref:`switch your ZooKeeper connection to use TLS`, and :ref:`configure a SQL - connection`. Even though only some Zuul components + connection`. Even though only some Zuul components connect to ZooKeeper today, you should ensure that the ``[zookeeper]`` section is present in ``zuul.conf`` for all components, and that they have network connectivity to the ZooKeeper diff --git a/releasenotes/notes/postgres-ae4f8594d0f4b256.yaml b/releasenotes/notes/postgres-ae4f8594d0f4b256.yaml index 6bb31acd8e..c1ef88a0d9 100644 --- a/releasenotes/notes/postgres-ae4f8594d0f4b256.yaml +++ b/releasenotes/notes/postgres-ae4f8594d0f4b256.yaml @@ -2,4 +2,4 @@ features: - | PostgreSQL is now officially supported as database backend. - See :attr:`` on how to configure database connections. + See ``sql connection`` on how to configure database connections. diff --git a/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml b/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml index 8f662a5345..84455aaabc 100644 --- a/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml +++ b/releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml @@ -3,7 +3,7 @@ features: - | An option to use the URL of the Zuul build page when reporting has been added. This feature requires that all the pipelines in the - tenant have a :ref:`SQL reporter` configured, and at + tenant have a SQL reporter configured, and at least one of :attr:`tenant.web-root` or :attr:`web.root` must be defined. diff --git a/releasenotes/notes/required-sql-7dd38b76d9f4273f.yaml b/releasenotes/notes/required-sql-7dd38b76d9f4273f.yaml index d2dffde726..42cb7ce76f 100644 --- a/releasenotes/notes/required-sql-7dd38b76d9f4273f.yaml +++ b/releasenotes/notes/required-sql-7dd38b76d9f4273f.yaml @@ -8,5 +8,5 @@ upgrade: deprecations: - | - Defining database connections using :attr:`` is now + Defining database connections using sql connection is now deprecated. Refer to :ref:`database` how to configure the database now. diff --git a/releasenotes/notes/sql-connection-removal-8579399ab165abf9.yaml b/releasenotes/notes/sql-connection-removal-8579399ab165abf9.yaml new file mode 100644 index 0000000000..316915cef9 --- /dev/null +++ b/releasenotes/notes/sql-connection-removal-8579399ab165abf9.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - | + Support for configuring the database as a `connection` in + ``zuul.conf`` was deprecated in Zuul version 4.0 and has now been + removed. Refer to :ref:`database` how to configure the database + now. diff --git a/releasenotes/notes/version-table-prefix-c6a5e84851268f4d.yaml b/releasenotes/notes/version-table-prefix-c6a5e84851268f4d.yaml index bae8817dce..afe6270fff 100644 --- a/releasenotes/notes/version-table-prefix-c6a5e84851268f4d.yaml +++ b/releasenotes/notes/version-table-prefix-c6a5e84851268f4d.yaml @@ -2,7 +2,7 @@ upgrade: - | The alembic version table is fixed to being prefixed too. This is necessary - when using :attr:`.table_prefix`. However if you are + when using ``table_prefix``. However if you are already using ``table_prefix`` you will need to rename the table ``alembic_version`` to ``alembic_version`` before starting Zuul. Otherwise zuul will try to create the tables again and fail. If you're not diff --git a/tests/fixtures/config/sql-driver/git/common-config/zuul.yaml b/tests/fixtures/config/sql-driver/git/common-config/zuul.yaml index ff0f9d685f..57d50ca43d 100644 --- a/tests/fixtures/config/sql-driver/git/common-config/zuul.yaml +++ b/tests/fixtures/config/sql-driver/git/common-config/zuul.yaml @@ -10,7 +10,6 @@ failure: gerrit: Verified: -1 - resultsdb_failures: null - pipeline: name: gate @@ -28,7 +27,6 @@ failure: gerrit: Verified: -2 - resultsdb_failures: null start: gerrit: Verified: 0 @@ -41,8 +39,6 @@ gerrit: - event: ref-updated ref: ^refs/tags/.*$ - failure: - resultsdb_failures: null - nodeset: name: test-nodeset diff --git a/tests/fixtures/zuul-sql-driver-mysql.conf b/tests/fixtures/zuul-sql-driver-mysql.conf index ab34cb4c09..094efdd63e 100644 --- a/tests/fixtures/zuul-sql-driver-mysql.conf +++ b/tests/fixtures/zuul-sql-driver-mysql.conf @@ -18,7 +18,3 @@ sshkey=fake_id_rsa1 [database] dburi=$MYSQL_FIXTURE_DBURI$ - -[connection resultsdb_failures] -driver=sql -dburi=$MYSQL_FIXTURE_DBURI$ diff --git a/tests/fixtures/zuul-sql-driver-postgres.conf b/tests/fixtures/zuul-sql-driver-postgres.conf index 13e8523c5d..436d16de48 100644 --- a/tests/fixtures/zuul-sql-driver-postgres.conf +++ b/tests/fixtures/zuul-sql-driver-postgres.conf @@ -18,7 +18,3 @@ sshkey=fake_id_rsa1 [database] dburi=$POSTGRESQL_FIXTURE_DBURI$ - -[connection resultsdb_failures] -driver=sql -dburi=$POSTGRESQL_FIXTURE_DBURI$ diff --git a/tests/fixtures/zuul-sql-driver-prefix-mysql.conf b/tests/fixtures/zuul-sql-driver-prefix-mysql.conf index c16620474b..bbe2993b2d 100644 --- a/tests/fixtures/zuul-sql-driver-prefix-mysql.conf +++ b/tests/fixtures/zuul-sql-driver-prefix-mysql.conf @@ -19,7 +19,3 @@ sshkey=fake_id_rsa1 [database] dburi=$MYSQL_FIXTURE_DBURI$ table_prefix=prefix_ - -[connection resultsdb_failures] -driver=sql -dburi=$MYSQL_FIXTURE_DBURI$ diff --git a/tests/fixtures/zuul-sql-driver-prefix-postgres.conf b/tests/fixtures/zuul-sql-driver-prefix-postgres.conf index 85745b6e4c..8cf5530e0f 100644 --- a/tests/fixtures/zuul-sql-driver-prefix-postgres.conf +++ b/tests/fixtures/zuul-sql-driver-prefix-postgres.conf @@ -20,7 +20,3 @@ sshkey=fake_id_rsa1 driver=sql dburi=$POSTGRESQL_FIXTURE_DBURI$ table_prefix=prefix_ - -[connection resultsdb_failures] -driver=sql -dburi=$POSTGRESQL_FIXTURE_DBURI$ diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py index d357d3cd27..46e7ab9035 100644 --- a/tests/unit/test_connection.py +++ b/tests/unit/test_connection.py @@ -300,64 +300,6 @@ class TestSQLConnectionMysql(ZuulTestCase): check_results() - def test_multiple_sql_connections(self): - "Test putting results in different databases" - # Add a successful result - 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_gerrit.addFakeChange('org/project', 'master', 'B') - self.executor_server.failJob('project-test1', B) - self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) - self.waitUntilSettled() - - def check_results(connection_name_1, connection_name_2): - # Grab the sa tables for resultsdb - tenant = self.scheds.first.sched.abide.tenants.get("tenant-one") - pipeline = tenant.layout.pipelines['check'] - reporter1 = self.scheds.first.connections.getSqlReporter( - pipeline) - - conn = self.scheds.first.connections.getSqlConnection().\ - 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 and failure report) - self.assertEqual(2, len(buildsets_resultsdb)) - - # The first one should have passed - self.assertEqual('check', buildsets_resultsdb[0]['pipeline']) - self.assertEqual( - 'org/project', buildsets_resultsdb[0]['project']) - self.assertEqual(1, buildsets_resultsdb[0]['change']) - self.assertEqual('1', buildsets_resultsdb[0]['patchset']) - self.assertEqual('SUCCESS', buildsets_resultsdb[0]['result']) - self.assertEqual( - 'Build succeeded.', buildsets_resultsdb[0]['message']) - - buildsets_resultsdb_failures = conn.execute(sa.sql.select( - [reporter1.connection.zuul_buildset_table])).fetchall() - # The failure db should only have 1 buildset failed - self.assertEqual(2, len(buildsets_resultsdb_failures)) - - self.assertEqual( - 'check', buildsets_resultsdb_failures[1]['pipeline']) - self.assertEqual('org/project', - buildsets_resultsdb_failures[1]['project']) - self.assertEqual(2, - buildsets_resultsdb_failures[1]['change']) - self.assertEqual( - '1', buildsets_resultsdb_failures[1]['patchset']) - self.assertEqual( - 'FAILURE', buildsets_resultsdb_failures[1]['result']) - self.assertEqual('Build failed.', - buildsets_resultsdb_failures[1]['message']) - - check_results('database', 'resultsdb_failures') - class TestSQLConnectionPostgres(TestSQLConnectionMysql): config_file = 'zuul-sql-driver-postgres.conf' diff --git a/zuul/lib/connections.py b/zuul/lib/connections.py index cb57313edd..a00868091d 100644 --- a/zuul/lib/connections.py +++ b/zuul/lib/connections.py @@ -114,7 +114,7 @@ class ConnectionRegistry(object): % con_name) con_driver = con_config['driver'] - if con_driver not in self.drivers: + if (con_driver not in self.drivers) or con_driver == 'sql': raise Exception("Unknown driver, %s, for connection %s" % (con_config['driver'], con_name)) @@ -128,12 +128,6 @@ class ConnectionRegistry(object): connection = driver.getConnection(con_name, con_config) connections[con_name] = connection - if con_driver == 'sql' and 'database' not in connections: - # The [database] section was missing. To stay backwards - # compatible duplicate the first database connection to the - # connection named 'database' - connections['database'] = driver.getConnection( - 'database', con_config) # If the [gerrit] or [smtp] sections still exist, load them in as a # connection named 'gerrit' or 'smtp' respectfully