Forbid more than one branch point in alembic dependency chains

This change will allow us to get rid of HEADS file that currently serves as a
caution guard against excessive branching due to git conflicts it invokes on
any new migration merged.

Change-Id: Iab88e4e11cea319bb7cfa364cb5cdefe9dcb004b

diff --git a/neutron/db/migration/cli.py b/neutron/db/migration/cli.py
index 4c7ea5b1be35da5c579dc917763b2b660a129d4f..cdd3d909373f84f61a420830a4db266e5805df19 100644 (file)
--- a/neutron/db/migration/cli.py
+++ b/neutron/db/migration/cli.py
@@ -118,7 +118,7 @@ def _get_alembic_entrypoint(project):
 
 def do_check_migration(config, cmd):
     do_alembic_command(config, 'branches')
-    validate_labels(config)
+    validate_revisions(config)
     validate_heads_file(config)
 
 
@@ -261,13 +261,25 @@ def _validate_revision(script_dir, revision):
     _validate_single_revision_labels(script_dir, revision)
 
 
-def validate_labels(config):
+def validate_revisions(config):
     script_dir = alembic_script.ScriptDirectory.from_config(config)
     revisions = [v for v in script_dir.walk_revisions(base='base',
                                                       head='heads')]
+
+    branchpoints = []
     for revision in revisions:
+        if revision.is_branch_point:
+            branchpoints.append(revision)
+
         _validate_revision(script_dir, revision)
 
+    if len(branchpoints) > 1:
+        branchpoints = ', '.join(p.revision for p in branchpoints)
+        alembic_util.err(
+            _('Unexpected number of alembic branch points: %(branchpoints)s') %
+            {'branchpoints': branchpoints}
+        )
+
 
 def _get_sorted_heads(script):
     '''Get the list of heads for all branches, sorted.'''

diff --git a/neutron/tests/tools.py b/neutron/tests/tools.py
index 63f730f8c213db699ad7dfd8e7c78cbfe5b7f964..5339f23322850b3119857035beee072cfd886b2d 100644 (file)
--- a/neutron/tests/tools.py
+++ b/neutron/tests/tools.py
@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import random
+import string
 import warnings
 
 import fixtures
@@ -121,3 +123,7 @@ class UnorderedList(list):
 
     def __neq__(self, other):
         return not self == other
+
+
+def get_random_string(n=10):
+    return ''.join(random.choice(string.ascii_lowercase) for _ in range(n))

diff --git a/neutron/tests/unit/db/test_migration.py b/neutron/tests/unit/db/test_migration.py
index 399fc5070f3cb68cda59746ebbc4faf76c665a6b..0bfde44621cd4e84b24279028607ba2ce7e79966 100644 (file)
--- a/neutron/tests/unit/db/test_migration.py
+++ b/neutron/tests/unit/db/test_migration.py
@@ -30,6 +30,7 @@ from neutron.db import migration
 from neutron.db.migration import autogen
 from neutron.db.migration import cli
 from neutron.tests import base
+from neutron.tests import tools
 from neutron.tests.unit import testlib_api
 
 
@@ -40,11 +41,13 @@ class FakeConfig(object):
 class FakeRevision(object):
     path = 'fakepath'
 
-    def __init__(self, labels=None, down_revision=None):
+    def __init__(self, labels=None, down_revision=None, is_branch_point=False):
         if not labels:
             labels = set()
         self.branch_labels = labels
         self.down_revision = down_revision
+        self.is_branch_point = is_branch_point
+        self.revision = tools.get_random_string()
 
 
 class MigrationEntrypointsMemento(fixtures.Fixture):
@@ -144,7 +147,7 @@ class TestCli(base.BaseTestCase):
     def _main_test_helper(self, argv, func_name, exp_kwargs=[{}]):
         with mock.patch.object(sys, 'argv', argv),\
             mock.patch.object(cli, 'run_sanity_checks'),\
-            mock.patch.object(cli, 'validate_labels'):
+            mock.patch.object(cli, 'validate_revisions'):
 
             cli.main()
             self.do_alembic_cmd.assert_has_calls(
@@ -485,16 +488,26 @@ class TestCli(base.BaseTestCase):
 
     @mock.patch.object(cli, '_validate_revision')
     @mock.patch('alembic.script.ScriptDirectory.walk_revisions')
-    def test_validate_labels_walks_thru_all_revisions(
+    def test_validate_revisions_walks_thru_all_revisions(
         self, walk_mock, validate_mock):
 
-        revisions = [mock.Mock() for i in range(10)]
+        revisions = [FakeRevision() for i in range(10)]
         walk_mock.return_value = revisions
-        cli.validate_labels(self.configs[0])
+        cli.validate_revisions(self.configs[0])
         validate_mock.assert_has_calls(
             [mock.call(mock.ANY, revision) for revision in revisions]
         )
 
+    @mock.patch.object(cli, '_validate_revision')
+    @mock.patch('alembic.script.ScriptDirectory.walk_revisions')
+    def test_validate_revisions_fails_on_multiple_branch_points(
+        self, walk_mock, validate_mock):
+
+        revisions = [FakeRevision(is_branch_point=True) for i in range(2)]
+        walk_mock.return_value = revisions
+        self.assertRaises(
+            SystemExit, cli.validate_revisions, self.configs[0])
+
     @mock.patch.object(cli, '_use_separate_migration_branches')
     @mock.patch.object(cli, '_get_version_branch_path')
     def test_autogen_process_directives(
@@ -604,5 +617,5 @@ class TestCli(base.BaseTestCase):
 
 class TestSafetyChecks(base.BaseTestCase):
 
-    def test_validate_labels(self, *mocks):
-        cli.validate_labels(cli.get_neutron_config())
+    def test_validate_revisions(self, *mocks):
+        cli.validate_revisions(cli.get_neutron_config())

diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py
index 34208d251a7a6fcbb8885b1ccbc69fec0b5d52e0..22126e43b86c881e82932ca65d3a28156a20a526 100644 (file)
--- a/neutron/tests/unit/objects/test_base.py
+++ b/neutron/tests/unit/objects/test_base.py
@@ -12,7 +12,6 @@
 
 import copy
 import random
-import string
 
 import mock
 from oslo_db import exception as obj_exc
@@ -25,6 +24,7 @@ from neutron import context
 from neutron.db import api as db_api
 from neutron.objects import base
 from neutron.tests import base as test_base
+from neutron.tests import tools
 
 
 SQLALCHEMY_COMMIT = 'sqlalchemy.engine.Connection._commit_impl'
@@ -53,10 +53,6 @@ class FakeNeutronObject(base.NeutronDbObject):
     synthetic_fields = ['field2']
 
 
-def _random_string(n=10):
-    return ''.join(random.choice(string.ascii_lowercase) for _ in range(n))
-
-
 def _random_boolean():
     return bool(random.getrandbits(1))
 
@@ -68,8 +64,8 @@ def _random_integer():
 FIELD_TYPE_VALUE_GENERATOR_MAP = {
     obj_fields.BooleanField: _random_boolean,
     obj_fields.IntegerField: _random_integer,
-    obj_fields.StringField: _random_string,
-    obj_fields.UUIDField: _random_string,
+    obj_fields.StringField: tools.get_random_string,
+    obj_fields.UUIDField: tools.get_random_string,
     obj_fields.ListOfObjectsField: lambda: []
 }