Allow specifying the current version in 'glance-manage version_control'

Fixes bug #966242

The version_control command is a bit useless since you can't actually
specify what version to  use and it defaults to version=0.

Allow the user to specify a version:

  $> glance-manage version_control 9 # set the diablo version

and default to the latest version if none is specified.

Also, allow db_sync to be supplied a version for the case where we're
upgrading an unversioned DB.

Finally, re-work the argument handling in glance-manage to more easily
handle optional args.

The tests are extended to test using db_sync for upgrades and to test
placing an existing database under version control.

Change-Id: I231dc710554198bfd1fdcb82c3c3768963f64bd8

bin/glance-manage

@@ -54,42 +54,36 @@ def do_db_version(conf, args):
 
 def do_upgrade(conf, args):
     """Upgrade the database's migration level"""
-    try:
-        db_version = args[1]
-    except IndexError:
-        db_version = None
-
-    glance.registry.db.migration.upgrade(conf, version=db_version)
+    version = args.pop(0) if args else None
+    glance.registry.db.migration.upgrade(conf, version)
 
 
 def do_downgrade(conf, args):
     """Downgrade the database's migration level"""
-    try:
-        db_version = args[1]
-    except IndexError:
+    if not args:
         raise exception.MissingArgumentError(
             "downgrade requires a version argument")
 
-    glance.registry.db.migration.downgrade(conf, version=db_version)
+    version = args.pop(0)
+    glance.registry.db.migration.downgrade(conf, version)
 
 
 def do_version_control(conf, args):
     """Place a database under migration control"""
-    glance.registry.db.migration.version_control(conf)
+    version = args.pop(0) if args else None
+    glance.registry.db.migration.version_control(conf, version)
 
 
 def do_db_sync(conf, args):
     """Place a database under migration control and upgrade"""
-    try:
-        db_version = args[1]
-    except IndexError:
-        db_version = None
-    glance.registry.db.migration.db_sync(conf, version=db_version)
+    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)
 
 
 def dispatch_cmd(conf, args):
     """Search for do_* cmd in this module and then run it"""
-    cmd = args[0]
+    cmd = args.pop(0)
     try:
         cmd_func = globals()['do_%s' % cmd]
     except KeyError:

glance/registry/db/migration.py

@@ -25,6 +25,7 @@ try:
     from migrate.versioning import exceptions as versioning_exceptions
 except ImportError:
     from migrate import exceptions as versioning_exceptions
+from migrate.versioning import repository as versioning_repository
 
 from glance.common import exception
 
@@ -82,7 +83,7 @@ def downgrade(conf, version):
     return versioning_api.downgrade(sql_connection, repo_path, version)
 
 
-def version_control(conf):
+def version_control(conf, version=None):
     """
     Place a database under migration control
 
@@ -90,14 +91,14 @@ def version_control(conf):
     """
     sql_connection = conf.sql_connection
     try:
-        _version_control(conf)
+        _version_control(conf, version)
     except versioning_exceptions.DatabaseAlreadyControlledError, e:
         msg = (_("database '%(sql_connection)s' is already under migration "
                "control") % locals())
         raise exception.DatabaseMigrationError(msg)
 
 
-def _version_control(conf):
+def _version_control(conf, version):
     """
     Place a database under migration control
 
@@ -105,20 +106,26 @@ def _version_control(conf):
     """
     repo_path = get_migrate_repo_path()
     sql_connection = conf.sql_connection
-    return versioning_api.version_control(sql_connection, repo_path)
+    if version is None:
+        version = versioning_repository.Repository(repo_path).latest
+    return versioning_api.version_control(sql_connection, repo_path, version)
 
 
-def db_sync(conf, version=None):
+def db_sync(conf, version=None, current_version=None):
     """
     Place a database under migration control and perform an upgrade
 
     :param conf: conf dict
     :retval version number
     """
+    sql_connection = conf.sql_connection
     try:
-        _version_control(conf)
+        _version_control(conf, current_version)
     except versioning_exceptions.DatabaseAlreadyControlledError, e:
-        pass
+        if current_version is not None:
+            msg = (_("database '%(sql_connection)s' is already under "
+                     "migration control") % locals())
+            raise exception.DatabaseMigrationError(msg)
 
     upgrade(conf, version=version)
 

glance/tests/unit/test_migrations.py

@@ -37,6 +37,7 @@ from sqlalchemy.pool import NullPool
 from glance.common import cfg
 from glance.common import exception
 import glance.registry.db.migration as migration_api
+from glance.registry.db import models
 from glance.tests import utils
 
 
@@ -130,7 +131,38 @@ class TestMigrations(unittest.TestCase):
             conf.register_opt(cfg.StrOpt('sql_connection'))
             self._walk_versions(conf)
 
-    def _walk_versions(self, conf):
+    def test_version_control_existing_db(self):
+        """
+        Creates a DB without version control information, places it
+        under version control and checks that it can be upgraded
+        without errors.
+        """
+        for key, engine in self.engines.items():
+            conf = utils.TestConfigOpts({
+                    'sql_connection': TestMigrations.TEST_DATABASES[key]})
+            conf.register_opt(cfg.StrOpt('sql_connection'))
+            self._create_unversioned_001_db(engine)
+            self._walk_versions(conf, initial_version=1)
+
+    def _create_unversioned_001_db(self, engine):
+        # Create the initial version of the images table
+        meta = MetaData()
+        meta.bind = engine
+        images_001 = Table('images', meta,
+            Column('id', models.Integer, primary_key=True),
+            Column('name', String(255)),
+            Column('type', String(30)),
+            Column('size', Integer),
+            Column('status', String(30)),
+            Column('is_public', Boolean, default=False),
+            Column('location', Text),
+            Column('created_at', DateTime(), nullable=False),
+            Column('updated_at', DateTime()),
+            Column('deleted_at', DateTime()),
+            Column('deleted', Boolean(), nullable=False, default=False))
+        images_001.create()
+
+    def _walk_versions(self, conf, initial_version=0):
         # Determine latest version script from the repo, then
         # upgrade from 1 through to the latest, with no data
         # in the databases. This just checks that the schema itself
@@ -141,13 +173,14 @@ class TestMigrations(unittest.TestCase):
                           migration_api.db_version,
                           conf)
         # Place the database under version control
-        migration_api.version_control(conf)
+        migration_api.version_control(conf, version=initial_version)
 
         cur_version = migration_api.db_version(conf)
-        self.assertEqual(0, cur_version)
+        self.assertEqual(initial_version, cur_version)
 
-        for version in xrange(1, TestMigrations.REPOSITORY.latest + 1):
-            migration_api.upgrade(conf, version)
+        for version in xrange(initial_version + 1,
+                              TestMigrations.REPOSITORY.latest + 1):
+            migration_api.db_sync(conf, version)
             cur_version = migration_api.db_version(conf)
             self.assertEqual(cur_version, version)
 
@@ -174,7 +207,7 @@ class TestMigrations(unittest.TestCase):
             self._no_data_loss_2_to_3_to_2(engine, conf)
 
     def _no_data_loss_2_to_3_to_2(self, engine, conf):
-        migration_api.version_control(conf)
+        migration_api.version_control(conf, version=0)
         migration_api.upgrade(conf, 2)
 
         cur_version = migration_api.db_version(conf)