From af7fb6c9da0b7e4ae75e02c138f1542b6db02301 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Thu, 20 Aug 2015 13:01:46 +0200 Subject: [PATCH] Dropped release name from migration branch labels 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@ neutron-db-manage upgrade mitaka_expand@ 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 | 4 +- .../liberty/contract/30018084ec99_initial.py | 5 +- .../expand/354db87e3225_nsxv_vdr_metadata.py | 11 +-- neutron/db/migration/cli.py | 67 ++++++++++--------- neutron/tests/unit/db/test_migration.py | 21 ++---- 5 files changed, 54 insertions(+), 54 deletions(-) diff --git a/doc/source/devref/alembic_migrations.rst b/doc/source/devref/alembic_migrations.rst index 245bf2fe932..725bc46f648 100644 --- a/doc/source/devref/alembic_migrations.rst +++ b/doc/source/devref/alembic_migrations.rst @@ -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. diff --git a/neutron/db/migration/alembic_migrations/versions/liberty/contract/30018084ec99_initial.py b/neutron/db/migration/alembic_migrations/versions/liberty/contract/30018084ec99_initial.py index a4a26704cd9..0e6358ffb7e 100644 --- a/neutron/db/migration/alembic_migrations/versions/liberty/contract/30018084ec99_initial.py +++ b/neutron/db/migration/alembic_migrations/versions/liberty/contract/30018084ec99_initial.py @@ -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(): diff --git a/neutron/db/migration/alembic_migrations/versions/liberty/expand/354db87e3225_nsxv_vdr_metadata.py b/neutron/db/migration/alembic_migrations/versions/liberty/expand/354db87e3225_nsxv_vdr_metadata.py index 33d521abbac..e63b3f5d09b 100644 --- a/neutron/db/migration/alembic_migrations/versions/liberty/expand/354db87e3225_nsxv_vdr_metadata.py +++ b/neutron/db/migration/alembic_migrations/versions/liberty/expand/354db87e3225_nsxv_vdr_metadata.py @@ -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(): diff --git a/neutron/db/migration/cli.py b/neutron/db/migration/cli.py index 7b1cee435a7..d33baa84df7 100644 --- a/neutron/db/migration/cli.py +++ b/neutron/db/migration/cli.py @@ -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) diff --git a/neutron/tests/unit/db/test_migration.py b/neutron/tests/unit/db/test_migration.py index 35e882e8240..3de29cb7bbc 100644 --- a/neutron/tests/unit/db/test_migration.py +++ b/neutron/tests/unit/db/test_migration.py @@ -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(