]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Dropped release name from migration branch labels
authorIhar Hrachyshka <ihrachys@redhat.com>
Thu, 20 Aug 2015 11:01:46 +0000 (13:01 +0200)
committerIhar Hrachyshka <ihrachys@redhat.com>
Thu, 20 Aug 2015 11:40:02 +0000 (13:40 +0200)
Since the plan is to attach first Mitaka scripts to Liberty branches
with down_revision, and since labels are inherited from all other
revisions in the chain, using release names in branch labels would mean
that the following commands would be valid:

neutron-db-manage upgrade liberty_expand@<some_revision_from_mitaka>
neutron-db-manage upgrade mitaka_expand@<some_revision_from_liberty>

which may be confusing to users.

So let's drop release names from branch labels and use expand@head and
contract@head to access latest migrations.

Change-Id: Id524d7673ad248c831f6dbb3a6f2f3c50094acae
Partially-Implements: blueprint online-schema-migrations

doc/source/devref/alembic_migrations.rst
neutron/db/migration/alembic_migrations/versions/liberty/contract/30018084ec99_initial.py
neutron/db/migration/alembic_migrations/versions/liberty/expand/354db87e3225_nsxv_vdr_metadata.py
neutron/db/migration/cli.py
neutron/tests/unit/db/test_migration.py

index 245bf2fe932dbdaecb90b58e0d30b6b0060ee4f2..725bc46f6488f410813d53bf9d4ae448044b12ab 100644 (file)
@@ -294,12 +294,12 @@ Applying database migration rules
 
 To apply just expansion rules, execute::
 
- neutron-db-manage upgrade liberty_expand@head
+ neutron-db-manage upgrade expand@head
 
 After the first step is done, you can stop neutron-server, apply remaining
 non-expansive migration rules, if any::
 
- neutron-db-manage upgrade liberty_contract@head
+ neutron-db-manage upgrade contract@head
 
 and finally, start your neutron-server again.
 
index a4a26704cd918aba38dbcf1af127073a2204e8ff..0e6358ffb7eae8de81614b5f9a5cdd7292d9054c 100644 (file)
@@ -19,10 +19,13 @@ Create Date: 2015-06-22 00:00:00.000000
 
 """
 
+from neutron.db.migration import cli
+
+
 # revision identifiers, used by Alembic.
 revision = '30018084ec99'
 down_revision = 'kilo'
-branch_labels = ('liberty_contract',)
+branch_labels = (cli.CONTRACT_BRANCH,)
 
 
 def upgrade():
index 33d521abbacaf5e146828fc01c6f5efddd721fcf..e63b3f5d09b96a953b893f3b98b60aca3904a2ef 100644 (file)
@@ -21,13 +21,16 @@ Create Date: 2015-04-19 14:59:15.102609
 
 """
 
+from alembic import op
+import sqlalchemy as sa
+
+from neutron.db.migration import cli
+
+
 # revision identifiers, used by Alembic.
 revision = '354db87e3225'
 down_revision = 'kilo'
-branch_labels = ('liberty_expand',)
-
-from alembic import op
-import sqlalchemy as sa
+branch_labels = (cli.EXPAND_BRANCH,)
 
 
 def upgrade():
index 7b1cee435a7b577a5ab9521b10c7c498d086bc4f..d33baa84df72a49f1dafdb2a1fb915f19575c828 100644 (file)
@@ -31,8 +31,10 @@ from neutron.common import utils
 HEAD_FILENAME = 'HEAD'
 HEADS_FILENAME = 'HEADS'
 CURRENT_RELEASE = "liberty"
-RELEASES = (CURRENT_RELEASE,)
-MIGRATION_BRANCHES = ('expand', 'contract')
+
+EXPAND_BRANCH = 'expand'
+CONTRACT_BRANCH = 'contract'
+MIGRATION_BRANCHES = (EXPAND_BRANCH, CONTRACT_BRANCH)
 
 MIGRATION_ENTRYPOINTS = 'neutron.db.alembic_migrations'
 migration_entrypoints = {
@@ -160,14 +162,9 @@ def do_stamp(config, cmd):
                        sql=CONF.command.sql)
 
 
-def _get_branch_label(branch, release=None):
-    '''Get the latest branch label corresponding to release cycle.'''
-    return '%s_%s' % (release or CURRENT_RELEASE, branch)
-
-
 def _get_branch_head(branch):
     '''Get the latest @head specification for a branch.'''
-    return '%s@head' % _get_branch_label(branch)
+    return '%s@head' % branch
 
 
 def do_revision(config, cmd):
@@ -182,20 +179,13 @@ def do_revision(config, cmd):
         for branch in MIGRATION_BRANCHES:
             version_path = _get_version_branch_path(config, branch)
             addn_kwargs['version_path'] = version_path
+            addn_kwargs['head'] = _get_branch_head(branch)
 
             if not os.path.exists(version_path):
                 # Bootstrap initial directory structure
                 utils.ensure_dir(version_path)
-                # Each new release stream of migrations is detached from
-                # previous migration chains
-                addn_kwargs['head'] = 'base'
                 # Mark the very first revision in the new branch with its label
-                addn_kwargs['branch_label'] = _get_branch_label(branch)
-                # TODO(ihrachyshka): ideally, we would also add depends_on here
-                # to refer to the head of the previous release stream. But
-                # alembic API does not support it yet.
-            else:
-                addn_kwargs['head'] = _get_branch_head(branch)
+                addn_kwargs['branch_label'] = branch
 
             do_alembic_command(config, cmd, **addn_kwargs)
     else:
@@ -203,10 +193,30 @@ def do_revision(config, cmd):
     update_heads_file(config)
 
 
+def _get_release_labels(labels):
+    result = set()
+    for label in labels:
+        result.add('%s_%s' % (CURRENT_RELEASE, label))
+    return result
+
+
 def _compare_labels(revision, expected_labels):
-    # validate that the script has the only label that corresponds to path
+    # validate that the script has expected labels only
     bad_labels = revision.branch_labels - expected_labels
     if bad_labels:
+        # NOTE(ihrachyshka): this hack is temporary to accomodate those
+        # projects that already initialized their branches with liberty_*
+        # labels. Let's notify them about the deprecation for now and drop it
+        # later.
+        bad_labels_with_release = (revision.branch_labels -
+                                   _get_release_labels(expected_labels))
+        if not bad_labels_with_release:
+            alembic_util.warn(
+                _('Release aware branch labels (%s) are deprecated. '
+                  'Please switch to expand@ and contract@ '
+                  'labels.') % bad_labels)
+            return
+
         script_name = os.path.basename(revision.path)
         alembic_util.err(
             _('Unexpected label for script %(script_name)s: %(labels)s') %
@@ -215,13 +225,10 @@ def _compare_labels(revision, expected_labels):
         )
 
 
-def _validate_single_revision_labels(script_dir, revision,
-                                     release=None, branch=None):
-    if branch is not None:
-        branch_label = _get_branch_label(branch, release=release)
-        expected_labels = set([branch_label])
-    else:
-        expected_labels = set()
+def _validate_single_revision_labels(script_dir, revision, label=None):
+    expected_labels = set()
+    if label is not None:
+        expected_labels.add(label)
 
     _compare_labels(revision, expected_labels)
 
@@ -234,12 +241,10 @@ def _validate_single_revision_labels(script_dir, revision,
 
 def _validate_revision(script_dir, revision):
     for branch in MIGRATION_BRANCHES:
-        for release in RELEASES:
-            marker = os.path.join(release, branch)
-            if marker in revision.path:
-                _validate_single_revision_labels(
-                    script_dir, revision, release=release, branch=branch)
-                return
+        if branch in revision.path:
+            _validate_single_revision_labels(
+                script_dir, revision, label=branch)
+            return
 
     # validate script from branchless part of migration rules
     _validate_single_revision_labels(script_dir, revision)
index 35e882e8240febe2895c9f82cc6c6bebd2351c93..3de29cb7bbc8afeb372985a485bf2d0dc6655b2a 100644 (file)
@@ -376,15 +376,6 @@ class TestCli(base.BaseTestCase):
         self.assertRaises(
             SystemExit, cli._get_subproject_base, 'not-installed')
 
-    def test__get_branch_label_current(self):
-        self.assertEqual('%s_fakebranch' % cli.CURRENT_RELEASE,
-                         cli._get_branch_label('fakebranch'))
-
-    def test__get_branch_label_other_release(self):
-        self.assertEqual('fakerelease_fakebranch',
-                         cli._get_branch_label('fakebranch',
-                                               release='fakerelease'))
-
     def test__compare_labels_ok(self):
         labels = {'label1', 'label2'}
         fake_revision = FakeRevision(labels)
@@ -407,7 +398,7 @@ class TestCli(base.BaseTestCase):
         script_dir = mock.Mock()
         script_dir.get_revision.return_value = fake_down_revision
         cli._validate_single_revision_labels(script_dir, fake_revision,
-                                             branch=None)
+                                             label=None)
 
         expected_labels = set()
         compare_mock.assert_has_calls(
@@ -425,10 +416,9 @@ class TestCli(base.BaseTestCase):
         script_dir = mock.Mock()
         script_dir.get_revision.return_value = fake_down_revision
         cli._validate_single_revision_labels(
-            script_dir, fake_revision,
-            release='fakerelease', branch='fakebranch')
+            script_dir, fake_revision, label='fakebranch')
 
-        expected_labels = {'fakerelease_fakebranch'}
+        expected_labels = {'fakebranch'}
         compare_mock.assert_has_calls(
             [mock.call(revision, expected_labels)
              for revision in (fake_revision, fake_down_revision)]
@@ -438,12 +428,11 @@ class TestCli(base.BaseTestCase):
     def test__validate_revision_validates_branches(self, validate_mock):
         script_dir = mock.Mock()
         fake_revision = FakeRevision()
-        release = cli.RELEASES[0]
         branch = cli.MIGRATION_BRANCHES[0]
-        fake_revision.path = os.path.join('/fake/path', release, branch)
+        fake_revision.path = os.path.join('/fake/path', branch)
         cli._validate_revision(script_dir, fake_revision)
         validate_mock.assert_called_with(
-            script_dir, fake_revision, release=release, branch=branch)
+            script_dir, fake_revision, label=branch)
 
     @mock.patch.object(cli, '_validate_single_revision_labels')
     def test__validate_revision_validates_branchless_migrations(