Remove cfg option default value and check if missing

Currently, several plugins already check config options at __init__()
for validity and will exit, if the settings are incorrect. However,
most (all?) config option definitions have default values, so if the
option is missing, a valid, but maybe unexpected value will be used.
This is what occurred in the bug.

The proposed fix is to take a config option, sql_connection, which is
used by many plugins, and remove the default value. Then, at init
time, when the config option is used in configure_db(), a check is
made for the value. If the value is not set, a warning is logged and
the value is set to the default, for db/api.py. It is expected that
this will be the only module to consume this config option.

Added UT to check that log warning is issued. Also, changed the timing
so that the test takes 0.25 secs vs 12 secs. Removed UTs in two plugin
tests that checked the default value for sql_connection.

Other alternatives explored in previous patches, were to either
raise an exception, or mark this config option as "required". This
resulted in a large number of changes to tests, and required config
overrides in plugins that imported quantum.db.api, but did not use
sql_connection.

In order to keep this solution (of this log-hanging fruit) fix, the
proposed, simpler change is being made.

Some cleanup to the Cisco plugin test case was also made, so that
the mock was more in line with what production code does.

bug 1059923

Change-Id: I8c2a4e05231ac4e172d0dccece067e6fdb354341

quantum/db/api.py

@@ -36,10 +36,11 @@ from quantum.openstack.common import cfg
 from quantum.openstack.common import log as logging
 
 LOG = logging.getLogger(__name__)
+SQL_CONNECTION_DEFAULT = 'sqlite://'
 
 
 database_opts = [
-    cfg.StrOpt('sql_connection', default='sqlite://',
+    cfg.StrOpt('sql_connection',
                help=_('The SQLAlchemy connection string used to connect to '
                       'the database')),
     cfg.IntOpt('sql_max_retries', default=-1,
@@ -111,6 +112,11 @@ def configure_db():
     global _ENGINE
     if not _ENGINE:
         sql_connection = cfg.CONF.DATABASE.sql_connection
+        if not sql_connection:
+            LOG.warn(_("Option 'sql_connection' not specified "
+                       "in any config file - using default "
+                       "value '%s'" % SQL_CONNECTION_DEFAULT))
+            sql_connection = SQL_CONNECTION_DEFAULT
         connection_dict = sql.engine.url.make_url(sql_connection)
         engine_args = {
             'pool_recycle': 3600,

quantum/tests/unit/cisco/test_nexus_plugin.py

@@ -19,7 +19,6 @@ import unittest
 from quantum.db import api as db
 from quantum.openstack.common import importutils
 from quantum.plugins.cisco.common import cisco_constants as const
-from quantum.plugins.cisco.db import network_db_v2 as cdb
 from quantum.plugins.cisco.db import network_models_v2
 from quantum.plugins.cisco.nexus import cisco_nexus_plugin_v2
 
@@ -62,9 +61,6 @@ class TestCiscoNexusPlugin(unittest.TestCase):
         }
         self._hostname = HOSTNAME
 
-        def new_cdb_init():
-            db.configure_db()
-
         def new_nexus_init(self):
             self._client = importutils.import_object(NEXUS_DRIVER)
             self._nexus_ip = NEXUS_IP_ADDRESS
@@ -78,13 +74,12 @@ class TestCiscoNexusPlugin(unittest.TestCase):
                     'password': self._nexus_password
                 }
             }
+            db.configure_db()
 
-        with mock.patch.object(cdb, 'initialize', new=new_cdb_init):
-            cdb.initialize()
-            with mock.patch.object(cisco_nexus_plugin_v2.NexusPlugin,
-                                   '__init__', new=new_nexus_init):
-                self._cisco_nexus_plugin = cisco_nexus_plugin_v2.NexusPlugin()
-                self._cisco_nexus_plugin._nexus_switches = self._nexus_switches
+        with mock.patch.object(cisco_nexus_plugin_v2.NexusPlugin,
+                               '__init__', new=new_nexus_init):
+            self._cisco_nexus_plugin = cisco_nexus_plugin_v2.NexusPlugin()
+            self._cisco_nexus_plugin._nexus_switches = self._nexus_switches
 
     def test_a_create_network(self):
         """

quantum/tests/unit/linuxbridge/test_defaults.py

@@ -23,8 +23,6 @@ from quantum.plugins.linuxbridge.common import config
 class ConfigurationTest(unittest.TestCase):
 
     def test_defaults(self):
-        self.assertEqual('sqlite://',
-                         cfg.CONF.DATABASE.sql_connection)
         self.assertEqual(-1,
                          cfg.CONF.DATABASE.sql_max_retries)
         self.assertEqual(2,

quantum/tests/unit/nec/test_config.py

@@ -23,7 +23,6 @@ from quantum.plugins.nec.common import config
 class ConfigurationTest(unittest.TestCase):
 
     def test_defaults(self):
-        self.assertEqual('sqlite://', config.CONF.DATABASE.sql_connection)
         self.assertEqual(-1, config.CONF.DATABASE.sql_max_retries)
         self.assertEqual(2, config.CONF.DATABASE.reconnect_interval)
         self.assertEqual('br-int', config.CONF.OVS.integration_bridge)

quantum/tests/unit/nicira/test_defaults.py

@@ -22,7 +22,6 @@ from quantum.plugins.nicira.nicira_nvp_plugin.common import config
 class ConfigurationTest(unittest.TestCase):
 
     def test_defaults(self):
-        self.assertEqual('sqlite://', cfg.CONF.DATABASE.sql_connection)
         self.assertEqual(-1, cfg.CONF.DATABASE.sql_max_retries)
         self.assertEqual(2, cfg.CONF.DATABASE.reconnect_interval)
         self.assertEqual(64, cfg.CONF.NVP.max_lp_per_bridged_ls)

quantum/tests/unit/openvswitch/test_ovs_defaults.py

@@ -26,7 +26,6 @@ class ConfigurationTest(unittest.TestCase):
         self.assertEqual('br-int', cfg.CONF.OVS.integration_bridge)
         self.assertFalse(cfg.CONF.OVS.enable_tunneling)
         self.assertEqual('br-tun', cfg.CONF.OVS.tunnel_bridge)
-        self.assertEqual('sqlite://', cfg.CONF.DATABASE.sql_connection)
         self.assertEqual(-1, cfg.CONF.DATABASE.sql_max_retries)
         self.assertEqual(2, cfg.CONF.DATABASE.reconnect_interval)
         self.assertEqual(2, cfg.CONF.AGENT.polling_interval)

quantum/tests/unit/ryu/test_defaults.py

@@ -26,7 +26,6 @@ class ConfigurationTest(unittest2.TestCase):
     """Configuration file Tests"""
     def test_defaults(self):
         self.assertEqual('br-int', cfg.CONF.OVS.integration_bridge)
-        self.assertEqual('sqlite://', cfg.CONF.DATABASE.sql_connection)
         self.assertEqual(-1, cfg.CONF.DATABASE.sql_max_retries)
         self.assertEqual(2, cfg.CONF.DATABASE.reconnect_interval)
         self.assertEqual(2, cfg.CONF.AGENT.polling_interval)

quantum/tests/unit/test_db.py

@@ -24,8 +24,25 @@ from quantum.openstack.common import cfg
 
 
 class DBTestCase(unittest.TestCase):
+    def setUp(self):
+        cfg.CONF.set_override('sql_max_retries', 1, 'DATABASE')
+        cfg.CONF.set_override('reconnect_interval', 0, 'DATABASE')
+
+    def tearDown(self):
+        db._ENGINE = None
+        cfg.CONF.reset()
+
     def test_db_reconnect(self):
-        cfg.CONF.set_override('sql_max_retries', 3, 'DATABASE')
         with mock.patch.object(db, 'register_models') as mock_register:
             mock_register.return_value = False
             db.configure_db()
+
+    def test_warn_when_no_connection(self):
+        with mock.patch.object(db, 'register_models') as mock_register:
+            mock_register.return_value = False
+            with mock.patch.object(db.LOG, 'warn') as mock_log:
+                mock_log.return_value = False
+                db.configure_db()
+                self.assertEquals(mock_log.call_count, 1)
+                args = mock_log.call_args
+                self.assertNotEqual(args.find('sql_connection'), -1)