Revert "Support unique labels for alembic branches"

A problem was found with the --expand (and --contract) options, see the bug report. I am reverting this patch so that I can redo it and have one single patch that can be back-ported.

Closes-Bug: #1599574

This reverts commit 88359e3fe9.

Change-Id: Iebca469c6fec32bac0480be6627ad5e9dedaad9d
This commit is contained in:
Henry Gessau 2016-07-06 16:51:04 +00:00
parent 88359e3fe9
commit e8ac4b402a
7 changed files with 97 additions and 205 deletions

View File

@ -418,20 +418,6 @@ following directive should be added in the contraction script::
depends_on = ('<expansion-revision>',)
Expand and Contract Branch Labeling
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Before the Newton release, neutron and each sub-project had its own alembic
environment and each repo could re-use the labels ``contract`` and ``expand``
for their alembic branches. With the Newton release, all neutron sub-projects
use neutron's alembic environment, and this requires globally unique branch
labels.
To be compatible with the Newton release of neutron, all projects must use
unique alembic branch labels of the form ``<project>-contract`` and
``<project>-expand``.
HEAD files for conflict management
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

View File

@ -25,7 +25,7 @@ from neutron.db.migration import cli
# revision identifiers, used by Alembic.
revision = '30018084ec99'
down_revision = 'kilo'
branch_labels = ('neutron-' + cli.CONTRACT_BRANCH,)
branch_labels = (cli.CONTRACT_BRANCH,)
def upgrade():

View File

@ -30,7 +30,7 @@ from neutron.db.migration import cli
# revision identifiers, used by Alembic.
revision = '354db87e3225'
down_revision = 'kilo'
branch_labels = ('neutron-' + cli.EXPAND_BRANCH,)
branch_labels = (cli.EXPAND_BRANCH,)
def upgrade():

View File

@ -54,8 +54,7 @@ def _migration_script_ops(context, directive, phase):
version_path = cli._get_version_branch_path(
context.config, release=cli.CURRENT_RELEASE, branch=phase)
autogen_kwargs = {}
cli._check_bootstrap_new_branch(context.config, phase, version_path,
autogen_kwargs)
cli._check_bootstrap_new_branch(phase, version_path, autogen_kwargs)
op = ops.MigrationScript(
new_rev_id(),

View File

@ -93,14 +93,11 @@ CONF.register_cli_opts(_db_opts, 'database')
def do_alembic_command(config, cmd, revision=None, desc=None, **kwargs):
project = config.get_main_option('neutron_project')
args = []
if revision:
# We use unique branch labels from Newton onwards.
if revision.split('@')[0] in MIGRATION_BRANCHES:
revision = '-'.join([project, revision])
args.append(revision)
project = config.get_main_option('neutron_project')
if desc:
alembic_util.msg(_('Running %(cmd)s (%(desc)s) for %(project)s ...') %
{'cmd': cmd, 'desc': desc, 'project': project})
@ -145,13 +142,13 @@ def add_branch_options(parser):
def _find_milestone_revisions(config, milestone, branch=None):
"""Return the revision(s) for a given milestone."""
script = alembic_script.ScriptDirectory.from_config(config)
milestone_revisions = []
for m in _get_revisions(script):
for branch_label in (m.branch_labels or [None]):
if milestone in getattr(m.module, 'neutron_milestone', []):
if branch is None or branch_label.endswith(branch):
milestone_revisions.append((m.revision, branch))
return milestone_revisions
return [
(m.revision, label)
for m in _get_revisions(script)
for label in (m.branch_labels or [None])
if milestone in getattr(m.module, 'neutron_milestone', []) and
(branch is None or branch in m.branch_labels)
]
def do_upgrade(config, cmd):
@ -164,11 +161,11 @@ def do_upgrade(config, cmd):
if CONF.command.expand:
branch = EXPAND_BRANCH
revision = _get_branch_head(config, EXPAND_BRANCH)
revision = _get_branch_head(EXPAND_BRANCH)
elif CONF.command.contract:
branch = CONTRACT_BRANCH
revision = _get_branch_head(config, CONTRACT_BRANCH)
revision = _get_branch_head(CONTRACT_BRANCH)
elif not CONF.command.revision and not CONF.command.delta:
raise SystemExit(_('You must provide a revision or relative delta'))
@ -220,26 +217,14 @@ def do_stamp(config, cmd):
sql=CONF.command.sql)
def _get_branch_head(config, branch):
def _get_branch_head(branch):
'''Get the latest @head specification for a branch.'''
script_dir = alembic_script.ScriptDirectory.from_config(config)
revs = script_dir.revision_map.get_revisions('heads')
for rev in revs:
for branch_label in rev.branch_labels:
# For forwards and backwards compatibility we handle branch
# names of either
# 'contract/expand'
# or
# 'subproject-contract/subproject-expand'.
if branch_label.endswith(branch):
branch = branch_label
break
return '%s@head' % branch
def _check_bootstrap_new_branch(config, branch, version_path, addn_kwargs):
def _check_bootstrap_new_branch(branch, version_path, addn_kwargs):
addn_kwargs['version_path'] = version_path
addn_kwargs['head'] = _get_branch_head(config, branch)
addn_kwargs['head'] = _get_branch_head(branch)
if not os.path.exists(version_path):
# Bootstrap initial directory structure
utils.ensure_dir(version_path)
@ -266,7 +251,7 @@ def do_revision(config, cmd):
args = copy.copy(kwargs)
version_path = _get_version_branch_path(
config, release=CURRENT_RELEASE, branch=branch)
_check_bootstrap_new_branch(config, branch, version_path, args)
_check_bootstrap_new_branch(branch, version_path, args)
do_alembic_command(config, cmd, **args)
else:
# autogeneration code will take care of enforcing proper directories
@ -275,15 +260,32 @@ def do_revision(config, cmd):
update_head_files(config)
def _get_release_labels(labels):
result = set()
for label in labels:
# release labels were introduced Liberty for a short time and dropped
# in that same release cycle
result.add('%s_%s' % (migration.LIBERTY, label))
return result
def _compare_labels(revision, expected_labels):
# validate that the script has expected labels only
expected_branch_labels = set()
for label in revision.branch_labels:
for expected_label in expected_labels:
if label.endswith(expected_label):
expected_branch_labels.add(label)
bad_labels = revision.branch_labels - expected_branch_labels
bad_labels = revision.branch_labels - expected_labels
if bad_labels:
# NOTE(ihrachyshka): this hack is temporary to accommodate 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') %
@ -337,14 +339,6 @@ def _get_revisions(script):
return list(script.walk_revisions(base='base', head='heads'))
def _get_branch_type(revision):
for branch_label in revision.branch_labels:
if branch_label.endswith(CONTRACT_BRANCH):
return CONTRACT_BRANCH
if branch_label.endswith(EXPAND_BRANCH):
return EXPAND_BRANCH
def _get_branch_points(script):
branchpoints = []
for revision in _get_revisions(script):
@ -358,7 +352,10 @@ def _get_heads_map(config):
heads = script.get_heads()
head_map = {}
for head in heads:
head_map[_get_branch_type(script.get_revision(head))] = head
if CONTRACT_BRANCH in script.get_revision(head).branch_labels:
head_map[CONTRACT_BRANCH] = head
else:
head_map[EXPAND_BRANCH] = head
return head_map

View File

@ -237,12 +237,7 @@ class TestModelsMigrationsMysql(_TestModelsMigrations,
script = alembic_script.ScriptDirectory.from_config(
self.alembic_config)
for m in list(script.walk_revisions(base='base', head='heads')):
branches = set()
for bl in m.branch_labels:
if bl.endswith(migration.CONTRACT_BRANCH):
branches.add(migration.CONTRACT_BRANCH)
elif bl.endswith(migration.EXPAND_BRANCH):
branches.add(migration.EXPAND_BRANCH)
branches = m.branch_labels or []
if migration.CONTRACT_BRANCH in branches:
method_name = 'contract_creation_exceptions'
exceptions_dict = creation_exceptions

View File

@ -148,28 +148,7 @@ class TestCli(base.BaseTestCase):
attrs=attrs)
cli.migration_entrypoints[project] = entrypoint
self.old_labels_revs = {
'a': FakeRevision(labels={cli.CONTRACT_BRANCH}),
'b': FakeRevision(labels={cli.EXPAND_BRANCH})
}
self.new_labels_revs = {
'x': FakeRevision(labels={'foo-' + cli.CONTRACT_BRANCH}),
'y': FakeRevision(labels={'foo-' + cli.EXPAND_BRANCH})
}
@staticmethod
def _old_and_new_label_combination_helper(test_method):
old_cb = cli.CONTRACT_BRANCH
new_cb = 'foo-' + cli.CONTRACT_BRANCH
old_eb = cli.EXPAND_BRANCH
new_eb = 'foo-' + cli.EXPAND_BRANCH
# Test all combinations of old and new branch labels.
test_method(old_cb, old_eb)
test_method(old_cb, new_eb)
test_method(new_cb, old_eb)
test_method(new_cb, new_eb)
def _main_test_helper(self, argv, func_name, exp_kwargs=None):
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_revisions'):
@ -184,7 +163,7 @@ class TestCli(base.BaseTestCase):
self.do_alembic_cmd.assert_has_calls(
[mock.call(mock.ANY, func_name, **_append_version_path(kwargs))
for kwargs in (exp_kwargs or [{}])]
for kwargs in exp_kwargs]
)
def test_stamp(self):
@ -228,11 +207,10 @@ class TestCli(base.BaseTestCase):
self._main_test_helper(['prog', 'check_migration'], 'branches')
self.assertEqual(len(self.projects), validate.call_count)
def _test_database_sync_revision(self, revs):
with mock.patch.object(cli, 'update_head_files') as update,\
mock.patch('alembic.script.ScriptDirectory.from_config') as fc:
fc.return_value.get_revisions.side_effect = revs
mock.patch('os.path.exists').start()
def _test_database_sync_revision(self, separate_branches=True):
with mock.patch.object(cli, 'update_head_files') as update:
if separate_branches:
mock.patch('os.path.exists').start()
expected_kwargs = [{
'message': 'message', 'sql': False, 'autogenerate': True,
}]
@ -248,7 +226,7 @@ class TestCli(base.BaseTestCase):
'message': 'message',
'sql': True,
'autogenerate': False,
'head': cli._get_branch_head(self.configs[0], branch)
'head': cli._get_branch_head(branch)
} for branch in cli.MIGRATION_BRANCHES]
for kwarg in expected_kwargs:
kwarg['autogenerate'] = False
@ -286,11 +264,12 @@ class TestCli(base.BaseTestCase):
)
self.assertEqual(len(self.projects), update.call_count)
def test_database_sync_revision_old_branch_labels(self):
self._test_database_sync_revision(self.old_labels_revs)
def test_database_sync_revision(self):
self._test_database_sync_revision()
def test_database_sync_revision_new_branch_labels(self):
self._test_database_sync_revision(self.new_labels_revs)
def test_database_sync_revision_no_branches(self):
# Test that old branchless approach is still supported
self._test_database_sync_revision(separate_branches=False)
def test_upgrade_revision(self):
self._main_test_helper(
@ -313,22 +292,14 @@ class TestCli(base.BaseTestCase):
[{'desc': None, 'revision': 'kilo+3', 'sql': False}]
)
def _test_upgrade_expand(self, revs):
with mock.patch('alembic.script.ScriptDirectory.from_config') as fc:
fc.return_value.get_revisions.side_effect = revs
self._main_test_helper(
['prog', 'upgrade', '--expand'],
'upgrade',
[{'desc': cli.EXPAND_BRANCH,
'revision': 'expand@head',
'sql': False}]
)
def test_upgrade_expand_old_labels(self):
self._test_upgrade_expand(self.old_labels_revs)
def test_upgrade_expand_new_labels(self):
self._test_upgrade_expand(self.new_labels_revs)
def test_upgrade_expand(self):
self._main_test_helper(
['prog', 'upgrade', '--expand'],
'upgrade',
[{'desc': cli.EXPAND_BRANCH,
'revision': 'expand@head',
'sql': False}]
)
def test_upgrade_expand_contract_are_mutually_exclusive(self):
with testlib_api.ExpectedException(SystemExit):
@ -371,31 +342,20 @@ class TestCli(base.BaseTestCase):
def test_upgrade_contract_conflicts_with_delta(self):
self._test_upgrade_conflicts_with_delta('contract')
def _test_upgrade_contract(self, revs):
with mock.patch('alembic.script.ScriptDirectory.from_config') as fc:
fc.return_value.get_revisions.side_effect = revs
self._main_test_helper(
['prog', 'upgrade', '--contract'],
'upgrade',
[{'desc': cli.CONTRACT_BRANCH,
'revision': 'contract@head',
'sql': False}]
)
def test_upgrade_contract_old_labels(self):
self._test_upgrade_contract(self.old_labels_revs)
def test_upgrade_contract_new_labels(self):
self._test_upgrade_contract(self.new_labels_revs)
def test_upgrade_contract(self):
self._main_test_helper(
['prog', 'upgrade', '--contract'],
'upgrade',
[{'desc': cli.CONTRACT_BRANCH,
'revision': 'contract@head',
'sql': False}]
)
@mock.patch('alembic.script.ScriptDirectory.walk_revisions')
def _test_upgrade_milestone_expand_before_contract(self,
contract_label,
expand_label,
walk_mock):
c_revs = [FakeRevision(labels={contract_label}) for r in range(5)]
def test_upgrade_milestone_expand_before_contract(self, walk_mock):
c_revs = [FakeRevision(labels={cli.CONTRACT_BRANCH}) for r in range(5)]
c_revs[1].module.neutron_milestone = [migration.LIBERTY]
e_revs = [FakeRevision(labels={expand_label}) for r in range(5)]
e_revs = [FakeRevision(labels={cli.EXPAND_BRANCH}) for r in range(5)]
e_revs[3].module.neutron_milestone = [migration.LIBERTY]
walk_mock.return_value = c_revs + e_revs
self._main_test_helper(
@ -409,10 +369,6 @@ class TestCli(base.BaseTestCase):
'sql': False}]
)
def test_upgrade_milestone_expand_before_contract(self):
self._old_and_new_label_combination_helper(
self._test_upgrade_milestone_expand_before_contract)
def assert_command_fails(self, command):
# Avoid cluttering stdout with argparse error messages
mock.patch('argparse.ArgumentParser._print_message').start()
@ -432,8 +388,8 @@ class TestCli(base.BaseTestCase):
def test_upgrade_rejects_delta_with_relative_revision(self):
self.assert_command_fails(['prog', 'upgrade', '+2', '--delta', '3'])
def _test_validate_head_files_common(self, heads, revs,
contract_head='', expand_head=''):
def _test_validate_head_files_helper(self, heads, contract_head='',
expand_head=''):
fake_config = self.configs[0]
head_files_not_exist = (contract_head == expand_head == '')
with mock.patch('alembic.script.ScriptDirectory.from_config') as fc,\
@ -444,6 +400,9 @@ class TestCli(base.BaseTestCase):
os_mock.return_value = True
fc.return_value.get_heads.return_value = heads
revs = {heads[0]: FakeRevision(labels=cli.CONTRACT_BRANCH),
heads[1]: FakeRevision(labels=cli.EXPAND_BRANCH)}
fc.return_value.get_revision.side_effect = revs.__getitem__
mock_open_con = self.useFixture(
tools.OpenFixture(cli._get_contract_head_file_path(
@ -474,23 +433,6 @@ class TestCli(base.BaseTestCase):
if not head_files_not_exist:
fc.assert_called_once_with(fake_config)
def _test_validate_head_files_helper(self, heads,
contract_head='', expand_head=''):
old_labels_revs = {
heads[0]: FakeRevision(labels={cli.CONTRACT_BRANCH}),
heads[1]: FakeRevision(labels={cli.EXPAND_BRANCH})
}
new_labels_revs = {
heads[0]: FakeRevision(labels={'foo-' + cli.CONTRACT_BRANCH}),
heads[1]: FakeRevision(labels={'foo-' + cli.EXPAND_BRANCH})
}
self._test_validate_head_files_common(heads, old_labels_revs,
contract_head=contract_head,
expand_head=expand_head)
self._test_validate_head_files_common(heads, new_labels_revs,
contract_head=contract_head,
expand_head=expand_head)
def test_validate_head_files_success(self):
self._test_validate_head_files_helper(['a', 'b'], contract_head='a',
expand_head='b')
@ -503,14 +445,8 @@ class TestCli(base.BaseTestCase):
expand_head='d')
@mock.patch.object(fileutils, 'delete_if_exists')
def _test_update_head_files_success(self, revs, *mocks):
contract_head = expand_head = None
for rev, revision in revs.items():
for branch_label in revision.branch_labels:
if branch_label.endswith(cli.CONTRACT_BRANCH):
contract_head = rev
if branch_label.endswith(cli.EXPAND_BRANCH):
expand_head = rev
def test_update_head_files_success(self, *mocks):
heads = ['a', 'b']
mock_open_con = self.useFixture(
tools.OpenFixture(cli._get_contract_head_file_path(
self.configs[0]))).mock_open
@ -518,13 +454,14 @@ class TestCli(base.BaseTestCase):
tools.OpenFixture(cli._get_expand_head_file_path(
self.configs[0]))).mock_open
with mock.patch('alembic.script.ScriptDirectory.from_config') as fc:
fc.return_value.get_heads.return_value = list(revs)
fc.return_value.get_heads.return_value = heads
revs = {heads[0]: FakeRevision(labels=cli.CONTRACT_BRANCH),
heads[1]: FakeRevision(labels=cli.EXPAND_BRANCH)}
fc.return_value.get_revision.side_effect = revs.__getitem__
cli.update_head_files(self.configs[0])
mock_open_con.return_value.write.assert_called_with(
contract_head + '\n')
mock_open_ex.return_value.write.assert_called_with(
expand_head + '\n')
heads[0] + '\n')
mock_open_ex.return_value.write.assert_called_with(heads[1] + '\n')
old_head_file = cli._get_head_file_path(
self.configs[0])
@ -536,12 +473,6 @@ class TestCli(base.BaseTestCase):
self.assertIn(mock.call(old_heads_file),
delete_if_exists.call_args_list)
def test_update_head_files_success_old_labels(self):
self._test_update_head_files_success(self.old_labels_revs)
def test_update_head_files_success_new_labels(self):
self._test_update_head_files_success(self.new_labels_revs)
def test_get_project_base(self):
config = alembic_config.Config()
config.set_main_option('script_location', 'a.b.c:d')
@ -658,7 +589,7 @@ class TestCli(base.BaseTestCase):
@mock.patch('alembic.script.ScriptDirectory.walk_revisions')
def test__get_branch_points(self, walk_mock):
revisions = [FakeRevision(is_branch_point=tools.get_random_boolean())
revisions = [FakeRevision(is_branch_point=tools.get_random_boolean)
for i in range(50)]
walk_mock.return_value = revisions
script_dir = alembic_script.ScriptDirectory.from_config(
@ -732,10 +663,8 @@ class TestCli(base.BaseTestCase):
)
directives = [migration_script]
context = mock.Mock()
context.config = self.configs[0]
autogen.process_revision_directives(
context, mock.Mock(), directives
mock.Mock(), mock.Mock(), directives
)
expand = directives[0]
@ -770,33 +699,23 @@ class TestCli(base.BaseTestCase):
)
@mock.patch('alembic.script.ScriptDirectory.walk_revisions')
def _test__find_milestone_revisions_one_branch(self,
contract_label,
expand_label,
walk_mock):
c_revs = [FakeRevision(labels={contract_label}) for r in range(5)]
def test__find_milestone_revisions_one_branch(self, walk_mock):
c_revs = [FakeRevision(labels={cli.CONTRACT_BRANCH}) for r in range(5)]
c_revs[1].module.neutron_milestone = [migration.LIBERTY]
walk_mock.return_value = c_revs
m = cli._find_milestone_revisions(self.configs[0], 'liberty',
contract_label)
cli.CONTRACT_BRANCH)
self.assertEqual(1, len(m))
m = cli._find_milestone_revisions(self.configs[0], 'liberty',
expand_label)
cli.EXPAND_BRANCH)
self.assertEqual(0, len(m))
def test__find_milestone_revisions_one_branch(self):
self._old_and_new_label_combination_helper(
self._test__find_milestone_revisions_one_branch)
@mock.patch('alembic.script.ScriptDirectory.walk_revisions')
def _test__find_milestone_revisions_two_branches(self,
contract_label,
expand_label,
walk_mock):
c_revs = [FakeRevision(labels={contract_label}) for r in range(5)]
def test__find_milestone_revisions_two_branches(self, walk_mock):
c_revs = [FakeRevision(labels={cli.CONTRACT_BRANCH}) for r in range(5)]
c_revs[1].module.neutron_milestone = [migration.LIBERTY]
e_revs = [FakeRevision(labels={expand_label}) for r in range(5)]
e_revs = [FakeRevision(labels={cli.EXPAND_BRANCH}) for r in range(5)]
e_revs[3].module.neutron_milestone = [migration.LIBERTY]
walk_mock.return_value = c_revs + e_revs
@ -806,10 +725,6 @@ class TestCli(base.BaseTestCase):
m = cli._find_milestone_revisions(self.configs[0], 'mitaka')
self.assertEqual(0, len(m))
def test__find_milestone_revisions_two_branches(self):
self._old_and_new_label_combination_helper(
self._test__find_milestone_revisions_two_branches)
@mock.patch('alembic.script.ScriptDirectory.walk_revisions')
def test__find_milestone_revisions_branchless(self, walk_mock):
revisions = [FakeRevision() for r in range(5)]