Drop support for SQL Schema Downgrades

Remove downgrade from the --autogenerate option of neutron-db-manage.
Add tests to check that downgrade options cannot be used.

Related cross-project spec: https://review.openstack.org/152337
Partial-Bug: 1434103

Change-Id: Id2f7f521644828ab7fbc027c6037f76f0337e121

neutron/db/migration/README

@@ -14,11 +14,11 @@
 
 The migrations in the alembic/versions contain the changes needed to migrate
 from older Neutron releases to newer versions. A migration occurs by executing
-a script that details the changes needed to upgrade/downgrade the database. The
-migration scripts are ordered so that multiple scripts can run sequentially to
-update the database. The scripts are executed by Neutron's migration wrapper
-which uses the Alembic library to manage the migration.  Neutron supports
-migration from Folsom or later.
+a script that details the changes needed to upgrade the database. The migration
+scripts are ordered so that multiple scripts can run sequentially to update the
+database. The scripts are executed by Neutron's migration wrapper which uses
+the Alembic library to manage the migration.  Neutron supports migration from
+Havana or later.
 
 
 If you are a deployer or developer and want to migrate from Folsom to Grizzly
@@ -48,19 +48,18 @@ Upgrade the database incrementally:
 $ neutron-db-manage --config-file /path/to/neutron.conf \
 --config-file /path/to/plugin/config.ini upgrade --delta <# of revs>
 
-Downgrade the database by a certain number of revisions:
-$ neutron-db-manage --config-file /path/to/neutron.conf \
---config-file /path/to/plugin/config.ini downgrade --delta <# of revs>
+NOTE: Database downgrade is not supported.
 
 
 DEVELOPERS:
+
 A database migration script is required when you submit a change to Neutron
 that alters the database model definition.  The migration script is a special
-python file that includes code to update/downgrade the database to match the
-changes in the model definition. Alembic will execute these scripts in order to
-provide a linear migration path between revision. The neutron-db-manage command
-can be used to generate migration template for you to complete.  The operations
-in the template are those supported by the Alembic migration library.
+python file that includes code to upgrade the database to match the changes in
+the model definition. Alembic will execute these scripts in order to provide a
+linear migration path between revision. The neutron-db-manage command can be
+used to generate migration template for you to complete.  The operations in the
+template are those supported by the Alembic migration library.
 
 $ neutron-db-manage --config-file /path/to/neutron.conf \
 --config-file /path/to/plugin/config.ini revision \
@@ -72,16 +71,15 @@ database state with the models.  You should inspect the autogenerated template
 to ensure that the proper models have been altered.
 
 In rare circumstances, you may want to start with an empty migration template
-and manually author the changes necessary for an upgrade/downgrade.  You can
-create a blank file via:
+and manually author the changes necessary for an upgrade.  You can create a
+blank file via:
 
 $ neutron-db-manage --config-file /path/to/neutron.conf \
 --config-file /path/to/plugin/config.ini revision \
 -m "description of revision"
 
 The migration timeline should remain linear so that there is a clear path when
-upgrading/downgrading.  To verify that the timeline does branch, you can run
-this command:
+upgrading.  To verify that the timeline does branch, you can run this command:
 $ neutron-db-manage --config-file /path/to/neutron.conf \
 --config-file /path/to/plugin/config.ini check_migration
 

neutron/db/migration/alembic_migrations/script.py.mako

@@ -32,7 +32,3 @@ ${imports if imports else ""}
 
 def upgrade():
     ${upgrades if upgrades else "pass"}
-
-
-def downgrade():
-    ${downgrades if downgrades else "pass"}

neutron/db/migration/cli.py

@@ -79,22 +79,37 @@ def do_check_migration(config, cmd):
     validate_head_file(config)
 
 
-def do_upgrade_downgrade(config, cmd):
+def add_alembic_subparser(sub, cmd):
+    return sub.add_parser(cmd, help=getattr(alembic_command, cmd).__doc__)
+
+
+def do_upgrade(config, cmd):
     if not CONF.command.revision and not CONF.command.delta:
         raise SystemExit(_('You must provide a revision or relative delta'))
 
-    revision = CONF.command.revision
+    revision = CONF.command.revision or ''
+    if '-' in revision:
+        raise SystemExit(_('Negative relative revision (downgrade) not '
+                           'supported'))
 
-    if CONF.command.delta:
-        sign = '+' if CONF.command.name == 'upgrade' else '-'
-        revision = sign + str(CONF.command.delta)
-    else:
-        revision = CONF.command.revision
-    if CONF.command.name == 'upgrade' and not CONF.command.sql:
+    delta = CONF.command.delta
+    if delta:
+        if '+' in revision:
+            raise SystemExit(_('Use either --delta or relative revision, '
+                               'not both'))
+        if delta < 0:
+            raise SystemExit(_('Negative delta (downgrade) not supported'))
+        revision = '%s+%d' % (revision, delta)
+
+    if not CONF.command.sql:
         run_sanity_checks(config, revision)
     do_alembic_command(config, cmd, revision, sql=CONF.command.sql)
 
 
+def no_downgrade(config, cmd):
+    raise SystemExit(_("Downgrade no longer supported"))
+
+
 def do_stamp(config, cmd):
     do_alembic_command(config, cmd,
                        CONF.command.revision,
@@ -134,29 +149,34 @@ def update_head_file(config):
 
 def add_command_parsers(subparsers):
     for name in ['current', 'history', 'branches']:
-        parser = subparsers.add_parser(name)
+        parser = add_alembic_subparser(subparsers, name)
         parser.set_defaults(func=do_alembic_command)
 
-    parser = subparsers.add_parser('check_migration')
+    help_text = (getattr(alembic_command, 'branches').__doc__ +
+                 ' and validate head file')
+    parser = subparsers.add_parser('check_migration', help=help_text)
     parser.set_defaults(func=do_check_migration)
 
-    for name in ['upgrade', 'downgrade']:
-        parser = subparsers.add_parser(name)
-        parser.add_argument('--delta', type=int)
-        parser.add_argument('--sql', action='store_true')
-        parser.add_argument('revision', nargs='?')
-        parser.add_argument('--mysql-engine',
-                            default='',
-                            help='Change MySQL storage engine of current '
-                                 'existing tables')
-        parser.set_defaults(func=do_upgrade_downgrade)
+    parser = add_alembic_subparser(subparsers, 'upgrade')
+    parser.add_argument('--delta', type=int)
+    parser.add_argument('--sql', action='store_true')
+    parser.add_argument('revision', nargs='?')
+    parser.add_argument('--mysql-engine',
+                        default='',
+                        help='Change MySQL storage engine of current '
+                             'existing tables')
+    parser.set_defaults(func=do_upgrade)
 
-    parser = subparsers.add_parser('stamp')
+    parser = subparsers.add_parser('downgrade', help="(No longer supported)")
+    parser.add_argument('None', nargs='?', help="Downgrade not supported")
+    parser.set_defaults(func=no_downgrade)
+
+    parser = add_alembic_subparser(subparsers, 'stamp')
     parser.add_argument('--sql', action='store_true')
     parser.add_argument('revision')
     parser.set_defaults(func=do_stamp)
 
-    parser = subparsers.add_parser('revision')
+    parser = add_alembic_subparser(subparsers, 'revision')
     parser.add_argument('-m', '--message')
     parser.add_argument('--autogenerate', action='store_true')
     parser.add_argument('--sql', action='store_true')

neutron/tests/unit/test_db_migration.py

@@ -143,21 +143,32 @@ class TestCli(base.BaseTestCase):
             {'sql': False}
         )
 
-    def test_downgrade(self):
         self._main_test_helper(
-            ['prog', 'downgrade', '--sql', 'folsom'],
-            'downgrade',
-            ('folsom',),
-            {'sql': True}
-        )
-
-        self._main_test_helper(
-            ['prog', 'downgrade', '--delta', '2'],
-            'downgrade',
-            ('-2',),
+            ['prog', 'upgrade', 'kilo', '--delta', '3'],
+            'upgrade',
+            ('kilo+3',),
             {'sql': False}
         )
 
+    def assert_command_fails(self, command):
+        # Avoid cluttering stdout with argparse error messages
+        mock.patch('argparse.ArgumentParser._print_message').start()
+        with mock.patch.object(sys, 'argv', command), mock.patch.object(
+                cli, 'run_sanity_checks'):
+            self.assertRaises(SystemExit, cli.main)
+
+    def test_downgrade_fails(self):
+        self.assert_command_fails(['prog', 'downgrade', '--sql', 'juno'])
+
+    def test_upgrade_negative_relative_revision_fails(self):
+        self.assert_command_fails(['prog', 'upgrade', '-2'])
+
+    def test_upgrade_negative_delta_fails(self):
+        self.assert_command_fails(['prog', 'upgrade', '--delta', '-2'])
+
+    def test_upgrade_rejects_delta_with_relative_revision(self):
+        self.assert_command_fails(['prog', 'upgrade', '+2', '--delta', '3'])
+
     def _test_validate_head_file_helper(self, heads, file_content=None):
         with mock.patch('alembic.script.ScriptDirectory.from_config') as fc:
             fc.return_value.get_heads.return_value = heads