Wait on startup if SQL not available

Now that the SQL database is required, fail to start if the dburi has
an error (like an incorrect module specification), and wait forever
for a connection to the database before proceeding.

This can be especially helpful in container environments where starting
Zuul may race starting a SQL database.

A test which verified that Zuul would start despite problems with the
SQL connection is removed since that is no longer the desired behavior.

Change-Id: Iae8ea420297f6264ae1d265b22b96d81f1df9a12

tests/base.py

@@ -75,7 +75,7 @@ from zuul.driver.git import GitDriver
 from zuul.driver.smtp import SMTPDriver
 from zuul.driver.github import GithubDriver
 from zuul.driver.timer import TimerDriver
-from zuul.driver.sql import SQLDriver, sqlconnection
+from zuul.driver.sql import SQLDriver, sqlconnection, sqlreporter
 from zuul.driver.bubblewrap import BubblewrapDriver
 from zuul.driver.nullwrap import NullwrapDriver
 from zuul.driver.mqtt import MQTTDriver
@@ -319,6 +319,9 @@ class SQLDriverMock(SQLDriver):
     def getConnection(self, name, config):
         return FakeSqlConnection(self, name, config)
 
+    def getReporter(self, connection, pipeline, config=None):
+        return FakeSqlReporter(self, connection, config)
+
 
 class TestConnectionRegistry(ConnectionRegistry):
     def __init__(self, changes: Dict[str, Dict[str, Change]],
@@ -2996,6 +2999,12 @@ class FakeSqlConnection(sqlconnection.SQLConnection):
         return FakeZuulDatabaseSession(self)
 
 
+class FakeSqlReporter(sqlreporter.SQLReporter):
+
+    def report(self, item):
+        pass
+
+
 class RecordingAnsibleJob(zuul.executor.server.AnsibleJob):
     result = None
 

tests/fixtures/zuul-sql-driver-bad.conf

@@ -1,34 +0,0 @@
-[gearman]
-server=127.0.0.1
-
-[zuul]
-layout_config=layout-connections-multiple-voters.yaml
-
-[merger]
-git_dir=/tmp/zuul-test/merger-git
-git_user_email=zuul@example.com
-git_user_name=zuul
-
-[executor]
-git_dir=/tmp/zuul-test/executor-git
-
-[connection gerrit]
-driver=gerrit
-server=review.example.com
-user=jenkins
-sshkey=fake_id_rsa1
-
-[database]
-dburi=mysql+pymysql://bad:creds@host/db
-
-[connection resultsdb_mysql_failures]
-driver=sql
-dburi=mysql+pymysql://bad:creds@host/db
-
-[connection resultsdb_postgresql]
-driver=sql
-dburi=postgresql://bad:creds@host/db
-
-[connection resultsdb_postgresql_failures]
-driver=sql
-dburi=postgresql://bad:creds@host/db

tests/unit/test_connection.py

@@ -428,23 +428,6 @@ class TestRequiredSQLConnection(BaseTestCase):
         self.connections.stop()
 
 
-class TestConnectionsBadSQL(ZuulDBTestCase):
-    config_file = 'zuul-sql-driver-bad.conf'
-    tenant_config_file = 'config/sql-driver/main.yaml'
-
-    def test_unable_to_connect(self):
-        "Test the SQL reporter fails gracefully when unable to connect"
-        self.config.set('zuul', 'layout_config',
-                        'tests/fixtures/layout-sql-reporter.yaml')
-        self.scheds.execute(lambda app: app.sched.reconfigure(app.config))
-
-        # Trigger a reporter. If no errors are raised, the reporter has been
-        # disabled correctly
-        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
-        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
-        self.waitUntilSettled()
-
-
 class TestMultipleGerrits(ZuulTestCase):
     config_file = 'zuul-connections-multiple-gerrits.conf'
     tenant_config_file = 'config/zuul-connections-multiple-gerrits/main.yaml'

zuul/driver/sql/sqlconnection.py

@@ -13,6 +13,7 @@
 # under the License.
 
 import logging
+import time
 
 import alembic
 import alembic.command
@@ -188,7 +189,6 @@ class SQLConnection(BaseConnection):
         self.dburi = None
         self.engine = None
         self.connection = None
-        self.tables_established = False
         self.table_prefix = self.connection_config.get('table_prefix', '')
         self.log.info("Initializing SQL connection {} (prefix: {})".format(
             connection_name, self.table_prefix))
@@ -215,15 +215,10 @@ class SQLConnection(BaseConnection):
                                                     expire_on_commit=False,
                                                     autoflush=False)
             self.session = orm.scoped_session(self.session_factory)
-
         except sa.exc.NoSuchModuleError:
-            self.log.exception(
-                "The required module for the dburi dialect isn't available. "
-                "SQL connection %s will be unavailable." % connection_name)
-        except sa.exc.OperationalError:
-            self.log.exception(
-                "Unable to connect to the database or establish the required "
-                "tables. Reporter %s is disabled" % self)
+            self.log.error(
+                "The required module for the dburi dialect isn't available.")
+            raise
 
     def getSession(self):
         return DatabaseSession(self)
@@ -247,18 +242,15 @@ class SQLConnection(BaseConnection):
             alembic.command.upgrade(config, 'head', tag=tag)
 
     def onLoad(self):
-        try:
-            self._migrate()
-            self.tables_established = True
-        except sa.exc.NoSuchModuleError:
-            self.log.exception(
-                "The required module for the dburi dialect isn't available. "
-                "SQL connection %s will be unavailable." %
-                self.connection_name)
-        except sa.exc.OperationalError:
-            self.log.exception(
-                "Unable to connect to the database or establish the required "
-                "tables. Connection %s is disabled" % self)
+        while True:
+            try:
+                self._migrate()
+                break
+            except sa.exc.OperationalError:
+                self.log.error(
+                    "Unable to connect to the database or establish the "
+                    "required tables.")
+            time.sleep(10)
 
     def _setup_models(self):
         Base = declarative_base(metadata=sa.MetaData())

zuul/driver/sql/sqlreporter.py

@@ -18,7 +18,6 @@ import logging
 import time
 import voluptuous as v
 
-from zuul.lib.logutil import get_annotated_logger
 from zuul.lib.result_data import get_artifacts_from_result_data
 from zuul.reporter import BaseReporter
 
@@ -66,12 +65,6 @@ class SQLReporter(BaseReporter):
 
     def report(self, item):
         """Create an entry into a database."""
-        log = get_annotated_logger(self.log, item.event)
-
-        if not self.connection.tables_established:
-            log.warning("SQL reporter (%s) is disabled ", self)
-            return
-
         event_id = None
         if item.event is not None:
             event_id = getattr(item.event, "zuul_event_id", None)