Support DB auto-create suppression.

Adds a new boolean config option, db_auto_create, to allow the
DB auto-creation be suppressed on demand. This defaults to True
for now to maintain the pre-existing behaviour, but should be
changed to False before the Folsom release.

The 'glance-manage db_sync' command will now create the image*
tables if the DB did not previously exist. The db_auto_create
flag is irrelevant in that case.

The @glance.tests.function.runs_sql annotation is now obsolete
as the glance-api/registry services launched by functional tests
must now all run against an on-disk sqlite instance (as opposed
to in-memory, as this makes no sense when the DB tables are
created in advance).

Change-Id: I05fc6b3ca7691dfaf00bc75a0743c921c93b9694

Conflicts:

	glance/tests/functional/__init__.py
	glance/tests/functional/test_sqlite.py
	glance/tests/functional/v1/test_api.py
	glance/tests/functional/v2/test_images.py

Author: Eoghan Glynn

bin/glance-manage

@@ -44,6 +44,7 @@ from glance.common import cfg
 from glance.common import config
 from glance.common import exception
 import glance.registry.db
+import glance.registry.db.api
 import glance.registry.db.migration
 
 
@@ -75,7 +76,14 @@ def do_version_control(conf, args):
 
 
 def do_db_sync(conf, args):
-    """Place a database under migration control and upgrade"""
+    """
+    Place a database under migration control and upgrade,
+    creating first if necessary.
+    """
+    # override auto-create flag, as complete DB should always
+    # be created on sync if not already existing
+    conf.db_auto_create = True
+    glance.registry.db.api.configure_db(conf)
     version = args.pop(0) if args else None
     current_version = args.pop(0) if args else None
     glance.registry.db.migration.db_sync(conf, version, current_version)

etc/glance-registry.conf

@@ -23,6 +23,10 @@ backlog = 4096
 # See: http://www.sqlalchemy.org/docs/05/reference/sqlalchemy/connections.html#sqlalchemy.create_engine
 sql_connection = sqlite:///glance.sqlite
 
+# Whether the glance service creates the database tables
+# automatically at startup, or explicitly with db_sync
+db_auto_create = True
+
 # Period in seconds after which SQLAlchemy should reestablish its connection
 # to the database.
 #

glance/registry/db/api.py

@@ -68,7 +68,8 @@
     cfg.IntOpt('sql_idle_timeout', default=3600),
     cfg.StrOpt('sql_connection', default='sqlite:///glance.sqlite'),
     cfg.IntOpt('sql_max_retries', default=10),
-    cfg.IntOpt('sql_retry_interval', default=1)
+    cfg.IntOpt('sql_retry_interval', default=1),
+    cfg.BoolOpt('db_auto_create', default=True),
     ]
 
 
@@ -102,7 +103,12 @@ def configure_db(conf):
     """
     global _ENGINE, sa_logger, logger, _MAX_RETRIES, _RETRY_INTERVAL
     if not _ENGINE:
-        conf.register_opts(db_opts)
+        for opt in db_opts:
+            # avoid duplicate registration
+            try:
+                getattr(conf, opt.name)
+            except cfg.NoSuchOptError:
+                conf.register_opt(opt)
         sql_connection = conf.sql_connection
         _MAX_RETRIES = conf.sql_max_retries
         _RETRY_INTERVAL = conf.sql_retry_interval
@@ -131,12 +137,16 @@ def configure_db(conf):
         elif conf.verbose:
             sa_logger.setLevel(logging.INFO)
 
-        models.register_models(_ENGINE)
-        try:
-            migration.version_control(conf)
-        except exception.DatabaseMigrationError:
-            # only arises when the DB exists and is under version control
-            pass
+        if conf.db_auto_create:
+            logger.info('auto-creating glance registry DB')
+            models.register_models(_ENGINE)
+            try:
+                migration.version_control(conf)
+            except exception.DatabaseMigrationError:
+                # only arises when the DB exists and is under version control
+                pass
+        else:
+            logger.info('not auto-creating glance registry DB')
 
 
 def check_mutate_authorization(context, image_ref):

glance/tests/functional/__init__.py

@@ -43,27 +43,6 @@
 execute, get_unused_port = test_utils.execute, test_utils.get_unused_port
 
 
-def runs_sql(func):
-    """
-    Decorator for a test case method that ensures that the
-    sql_connection setting is overridden to ensure a disk-based
-    SQLite database so that arbitrary SQL statements can be
-    executed out-of-process against the datastore...
-    """
-    @functools.wraps(func)
-    def wrapped(*a, **kwargs):
-        test_obj = a[0]
-        orig_sql_connection = test_obj.registry_server.sql_connection
-        try:
-            if orig_sql_connection.startswith('sqlite'):
-                test_obj.registry_server.sql_connection =\
-                        "sqlite:///tests.sqlite"
-            func(*a, **kwargs)
-        finally:
-            test_obj.registry_server.sql_connection = orig_sql_connection
-    return wrapped
-
-
 class Server(object):
     """
     Class used to easily manage starting and stopping
@@ -89,6 +68,7 @@ def __init__(self, test_dir, port):
         self.exec_env = None
         self.deployment_flavor = ''
         self.server_control_options = ''
+        self.needs_database = False
 
     def write_conf(self, **kwargs):
         """
@@ -145,6 +125,8 @@ def start(self, expect_exit=True, expected_exitcode=0, **kwargs):
         # Ensure the configuration file is written
         overridden = self.write_conf(**kwargs)[1]
 
+        self.create_database()
+
         cmd = ("%(server_control)s %(server_name)s start "
                "%(conf_file_name)s --pid-file=%(pid_file)s "
                "%(server_control_options)s"
@@ -156,6 +138,23 @@ def start(self, expect_exit=True, expected_exitcode=0, **kwargs):
                        expected_exitcode=expected_exitcode,
                        context=overridden)
 
+    def create_database(self):
+        """Create database if required for this server"""
+        if self.needs_database:
+            conf_dir = os.path.join(self.test_dir, 'etc')
+            utils.safe_mkdirs(conf_dir)
+            conf_filepath = os.path.join(conf_dir, 'glance-manage.conf')
+
+            with open(conf_filepath, 'wb') as conf_file:
+                conf_file.write('[DEFAULT]\n')
+                conf_file.write('sql_connection = %s' % self.sql_connection)
+                conf_file.flush()
+
+            cmd = ('bin/glance-manage db_sync --config-file %s'
+                   % conf_filepath)
+            execute(cmd, no_venv=self.no_venv, exec_env=self.exec_env,
+                    expect_exit=True)
+
     def stop(self):
         """
         Spin down the server.
@@ -212,6 +211,12 @@ def __init__(self, test_dir, port, registry_port, policy_file,
         self.policy_file = policy_file
         self.policy_default_rule = 'default'
         self.server_control_options = '--capture-output'
+
+        self.needs_database = True
+        default_sql_connection = 'sqlite:///tests.sqlite'
+        self.sql_connection = os.environ.get('GLANCE_TEST_SQL_CONNECTION',
+                                             default_sql_connection)
+
         self.conf_base = """[DEFAULT]
 verbose = %(verbose)s
 debug = %(debug)s
@@ -248,6 +253,8 @@ def __init__(self, test_dir, port, registry_port, policy_file,
 image_cache_driver = %(image_cache_driver)s
 policy_file = %(policy_file)s
 policy_default_rule = %(policy_default_rule)s
+db_auto_create = False
+sql_connection = %(sql_connection)s
 [paste_deploy]
 flavor = %(deployment_flavor)s
 """
@@ -300,7 +307,8 @@ def __init__(self, test_dir, port):
         super(RegistryServer, self).__init__(test_dir, port)
         self.server_name = 'registry'
 
-        default_sql_connection = 'sqlite:///'
+        self.needs_database = True
+        default_sql_connection = 'sqlite:///tests.sqlite'
         self.sql_connection = os.environ.get('GLANCE_TEST_SQL_CONNECTION',
                                              default_sql_connection)
 
@@ -315,6 +323,7 @@ def __init__(self, test_dir, port):
 bind_host = 0.0.0.0
 bind_port = %(bind_port)s
 log_file = %(log_file)s
+db_auto_create = False
 sql_connection = %(sql_connection)s
 sql_idle_timeout = 3600
 api_limit_max = 1000
@@ -625,11 +634,6 @@ def stop_servers(self):
         if os.path.exists(self.test_dir):
             shutil.rmtree(self.test_dir)
 
-        # We do this here because the @runs_sql decorator above
-        # actually resets the registry server's sql_connection
-        # to the original (usually memory-based SQLite connection)
-        # and this block of code is run *before* the finally:
-        # block in that decorator...
         self._reset_database(self.registry_server.sql_connection)
 
     def run_sql_cmd(self, sql):

glance/tests/functional/test_bin_glance.py

@@ -647,7 +647,6 @@ def test_killed_image_not_in_index(self):
 
         self.stop_servers()
 
-    @functional.runs_sql
     def test_add_location_with_checksum(self):
         """
         We test the following:
@@ -679,7 +678,6 @@ def test_add_location_with_checksum(self):
 
         self.stop_servers()
 
-    @functional.runs_sql
     def test_add_location_without_checksum(self):
         """
         We test the following:
@@ -711,7 +709,6 @@ def test_add_location_without_checksum(self):
 
         self.stop_servers()
 
-    @functional.runs_sql
     def test_add_clear(self):
         """
         We test the following:

glance/tests/functional/test_glance_manage.py

@@ -0,0 +1,77 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2012 Red Hat, Inc
+# All Rights Reserved.
+#
+#    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.
+
+"""Functional test cases for glance-manage"""
+
+import os
+
+from glance.common import utils
+from glance.tests import functional
+from glance.tests.utils import execute, depends_on_exe, skip_if_disabled
+
+
+class TestGlanceManage(functional.FunctionalTest):
+    """Functional tests for glance-manage"""
+
+    def setUp(self):
+        super(TestGlanceManage, self).setUp()
+        conf_dir = os.path.join(self.test_dir, 'etc')
+        utils.safe_mkdirs(conf_dir)
+        self.conf_filepath = os.path.join(conf_dir, 'glance-manage.conf')
+        self.db_filepath = os.path.join(conf_dir, 'test.sqlite')
+        self.connection = ('sql_connection = sqlite:///%s' %
+                           self.db_filepath)
+
+    def _sync_db(self, auto_create):
+        with open(self.conf_filepath, 'wb') as conf_file:
+            conf_file.write('[DEFAULT]\n')
+            conf_file.write('db_auto_create = %r\n' % auto_create)
+            conf_file.write(self.connection)
+            conf_file.flush()
+
+        cmd = ('bin/glance-manage db_sync --config-file %s' %
+               self.conf_filepath)
+        execute(cmd, raise_error=True)
+
+    def _assert_tables(self):
+        cmd = "sqlite3 %s '.schema'" % self.db_filepath
+        exitcode, out, err = execute(cmd, raise_error=True)
+
+        self.assertTrue('CREATE TABLE images' in out)
+        self.assertTrue('CREATE TABLE image_tags' in out)
+        self.assertTrue('CREATE TABLE image_members' in out)
+        self.assertTrue('CREATE TABLE image_properties' in out)
+
+    @depends_on_exe('sqlite3')
+    @skip_if_disabled
+    def test_db_creation(self):
+        """Test DB creation by db_sync on a fresh DB"""
+        self._sync_db(True)
+
+        self._assert_tables()
+
+        self.stop_servers()
+
+    @depends_on_exe('sqlite3')
+    @skip_if_disabled
+    def test_db_creation_auto_create_overridden(self):
+        """Test DB creation with db_auto_create False"""
+        self._sync_db(False)
+
+        self._assert_tables()
+
+        self.stop_servers()

glance/tests/functional/test_sqlite.py

@@ -25,7 +25,6 @@
 class TestSqlite(functional.FunctionalTest):
     """Functional tests for sqlite-specific logic"""
 
-    @functional.runs_sql
     def test_big_int_mapping(self):
         """Ensure BigInteger not mapped to BIGINT"""
         self.cleanup()