zuul/zuul
Code Issues Proposed changes

Remove "sql connection" backwards compatability for database

Browse Source 
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
This commit is contained in:
James E. Blair 2022-01-17 13:15:58 -08:00 committed by Clark Boylan
parent 07b0b5c5fa
commit 41d8e478a5
16 changed files with 16 additions and 188 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
doc/source/drivers/index.rst
View File

@ -26,6 +26,5 @@ Zuul includes the following drivers:
mqtt
elasticsearch
smtp
sql
timer
zuul

93
doc/source/drivers/sql.rst
View File

@ -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:: <sql connection>
.. 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
<https://docs.sqlalchemy.org/en/latest/core/engines.html#database-urls>`_
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
<http://docs.sqlalchemy.org/en/latest/core/pooling.html#setting-pool-recycle>`_
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<connections>` 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.<reporter>.<sql source>
To report to a database, add a key with the connection name and an
empty value to the desired pipeline :ref:`reporter<reporters>`
attributes.

7
etc/zuul.conf-sample
View File

@ -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

2
releasenotes/notes/last3x-8682ada4910fa750.yaml
View File

@ -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<zk-encrypted-connections>`, and :ref:`configure a SQL
connection<sql-driver>`. Even though only some Zuul components
connection<database>`. 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

2
releasenotes/notes/postgres-ae4f8594d0f4b256.yaml
View File

@ -2,4 +2,4 @@
features:
- |
PostgreSQL is now officially supported as database backend.
See :attr:`<sql connection>` on how to configure database connections.
See ``sql connection`` on how to configure database connections.

2
releasenotes/notes/report-build-page-2-49088c2b0a36e1b7.yaml
View File

@ -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<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.

2
releasenotes/notes/required-sql-7dd38b76d9f4273f.yaml
View File

@ -8,5 +8,5 @@ upgrade:
deprecations:
- |
Defining database connections using :attr:`<sql connection>` is now
Defining database connections using sql connection is now
deprecated. Refer to :ref:`database` how to configure the database now.

7
releasenotes/notes/sql-connection-removal-8579399ab165abf9.yaml Normal file
View File

@ -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.

2
releasenotes/notes/version-table-prefix-c6a5e84851268f4d.yaml
View File

@ -2,7 +2,7 @@
upgrade:
- |
The alembic version table is fixed to being prefixed too. This is necessary
when using :attr:`<sql connection>.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 ``<prefix>alembic_version`` before starting Zuul.
Otherwise zuul will try to create the tables again and fail. If you're not

4
tests/fixtures/config/sql-driver/git/common-config/zuul.yaml vendored
View File

@ -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

4
tests/fixtures/zuul-sql-driver-mysql.conf vendored
View File

@ -18,7 +18,3 @@ sshkey=fake_id_rsa1
[database]
dburi=$MYSQL_FIXTURE_DBURI$
[connection resultsdb_failures]
driver=sql
dburi=$MYSQL_FIXTURE_DBURI$

4
tests/fixtures/zuul-sql-driver-postgres.conf vendored
View File

@ -18,7 +18,3 @@ sshkey=fake_id_rsa1
[database]
dburi=$POSTGRESQL_FIXTURE_DBURI$
[connection resultsdb_failures]
driver=sql
dburi=$POSTGRESQL_FIXTURE_DBURI$

4
tests/fixtures/zuul-sql-driver-prefix-mysql.conf vendored
View File

@ -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$

4
tests/fixtures/zuul-sql-driver-prefix-postgres.conf vendored
View File

@ -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$

58
tests/unit/test_connection.py
View File

@ -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'

8
zuul/lib/connections.py
View File

@ -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