From 5a5762b71c978312d2b00c1cbc6b382fe35ee30e Mon Sep 17 00:00:00 2001 From: "shilpa.devharakar" Date: Fri, 9 Feb 2018 11:58:07 +0530 Subject: [PATCH] Add validation to check if E-M-C is already in sync If you run expand and migrate commands for the second time, it should return a user friendly message instead of attempting to upgrade db again. Added a check to confirm if expand and migrate are already in sync and return a user friendly message. Closes-Bug: #1745360 Change-Id: Iaf2e8ae2004db03f9b7498a2c498360fec096066 --- glance/cmd/manage.py | 25 +- glance/tests/functional/test_glance_manage.py | 57 +++- glance/tests/unit/test_manage.py | 310 ++++++++++++++++++ 3 files changed, 382 insertions(+), 10 deletions(-) diff --git a/glance/cmd/manage.py b/glance/cmd/manage.py index e13df4ad95..57bd087b13 100644 --- a/glance/cmd/manage.py +++ b/glance/cmd/manage.py @@ -212,14 +212,18 @@ class DbCommands(object): print(_('Database is up to date. No migrations needed.')) sys.exit() - self._sync(version=expand_head) - - curr_heads = alembic_migrations.get_current_alembic_heads() if expand_head not in curr_heads: - sys.exit(_('Database expansion failed. Database expansion should ' - 'have brought the database version up to "%(e_rev)s" ' - 'revision. But, current revisions are: %(curr_revs)s ') - % {'e_rev': expand_head, 'curr_revs': curr_heads}) + self._sync(version=expand_head) + + curr_heads = alembic_migrations.get_current_alembic_heads() + if expand_head not in curr_heads: + sys.exit(_('Database expansion failed. Database expansion ' + 'should have brought the database version up to ' + '"%(e_rev)s" revision. But, current revisions are' + ': %(curr_revs)s ') % {'e_rev': expand_head, + 'curr_revs': curr_heads}) + else: + print(_('Database expansion is up to date. No expansion needed.')) def contract(self): """Run the contraction phase of a rolling upgrade procedure.""" @@ -278,8 +282,11 @@ class DbCommands(object): 'run before database expansion. Run database ' 'expansion first using "glance-manage db expand"')) - rows_migrated = data_migrations.migrate(db_api.get_engine()) - print(_('Migrated %s rows') % rows_migrated) + if data_migrations.has_pending_migrations(db_api.get_engine()): + rows_migrated = data_migrations.migrate(db_api.get_engine()) + print(_('Migrated %s rows') % rows_migrated) + else: + print(_('Database migration is up to date. No migration needed.')) @args('--path', metavar='', help='Path to the directory or file ' 'where json metadata is stored') diff --git a/glance/tests/functional/test_glance_manage.py b/glance/tests/functional/test_glance_manage.py index 8e65644acf..8fb012990f 100644 --- a/glance/tests/functional/test_glance_manage.py +++ b/glance/tests/functional/test_glance_manage.py @@ -56,7 +56,7 @@ class TestGlanceManage(functional.FunctionalTest): cmd = ('%s -m glance.cmd.manage --config-file %s db %s' % (sys.executable, self.conf_filepath, db_method)) - execute(cmd, raise_error=True) + return execute(cmd, raise_error=True) def _check_db(self, expected_exitcode): with open(self.conf_filepath, 'w') as conf_file: @@ -119,3 +119,58 @@ class TestGlanceManage(functional.FunctionalTest): self._db_command(db_method='contract') exitcode, out = self._check_db(0) self.assertEqual(0, exitcode) + + @depends_on_exe('sqlite3') + @skip_if_disabled + def test_expand(self): + """Test DB expand""" + self._db_command(db_method='expand') + expand_head = alembic_migrations.get_alembic_branch_head( + db_migration.EXPAND_BRANCH) + + cmd = ("sqlite3 {0} \"SELECT version_num FROM alembic_version\"" + ).format(self.db_filepath) + exitcode, out, err = execute(cmd, raise_error=True) + self.assertEqual(expand_head, out.rstrip().decode("utf-8")) + exitcode, out, err = self._db_command(db_method='expand') + self.assertIn('Database expansion is up to date. ' + 'No expansion needed.', out) + + @depends_on_exe('sqlite3') + @skip_if_disabled + def test_migrate(self): + """Test DB migrate""" + self._db_command(db_method='expand') + if data_migrations.has_pending_migrations(db_api.get_engine()): + self._db_command(db_method='migrate') + expand_head = alembic_migrations.get_alembic_branch_head( + db_migration.EXPAND_BRANCH) + + cmd = ("sqlite3 {0} \"SELECT version_num FROM alembic_version\"" + ).format(self.db_filepath) + exitcode, out, err = execute(cmd, raise_error=True) + self.assertEqual(expand_head, out.rstrip().decode("utf-8")) + self.assertEqual(False, data_migrations.has_pending_migrations( + db_api.get_engine())) + if data_migrations.has_pending_migrations(db_api.get_engine()): + exitcode, out, err = self._db_command(db_method='migrate') + self.assertIn('Database migration is up to date. No migration ' + 'needed.', out) + + @depends_on_exe('sqlite3') + @skip_if_disabled + def test_contract(self): + """Test DB contract""" + self._db_command(db_method='expand') + if data_migrations.has_pending_migrations(db_api.get_engine()): + self._db_command(db_method='migrate') + self._db_command(db_method='contract') + contract_head = alembic_migrations.get_alembic_branch_head( + db_migration.CONTRACT_BRANCH) + + cmd = ("sqlite3 {0} \"SELECT version_num FROM alembic_version\"" + ).format(self.db_filepath) + exitcode, out, err = execute(cmd, raise_error=True) + self.assertEqual(contract_head, out.rstrip().decode("utf-8")) + exitcode, out, err = self._db_command(db_method='contract') + self.assertIn('Database is up to date. No migrations needed.', out) diff --git a/glance/tests/unit/test_manage.py b/glance/tests/unit/test_manage.py index 244e002c3e..2d28d6fdf8 100644 --- a/glance/tests/unit/test_manage.py +++ b/glance/tests/unit/test_manage.py @@ -20,6 +20,7 @@ import mock from six.moves import StringIO from glance.cmd import manage +from glance.common import exception from glance.db.sqlalchemy import api as db_api from glance.db.sqlalchemy import metadata as db_metadata from glance.tests import utils as test_utils @@ -223,6 +224,315 @@ class TestManage(TestManageBase): self.assertIn('Database is up to date. No upgrades needed.', self.output.getvalue()) + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, 'expand') + @mock.patch.object(manage.DbCommands, 'migrate') + @mock.patch.object(manage.DbCommands, 'contract') + def test_sync(self, mock_contract, mock_migrate, mock_expand, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + mock_get_current_alembic_heads.return_value = ['ocata_contract01'] + mock_get_alembic_branch_head.return_value = ['pike_contract01'] + self.db.sync() + mock_expand.assert_called_once_with() + mock_migrate.assert_called_once_with() + mock_contract.assert_called_once_with() + self.assertIn('Database is synced successfully.', + self.output.getvalue()) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + def test_sync_db_is_already_sync(self, mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + mock_get_current_alembic_heads.return_value = ['pike_contract01'] + mock_get_alembic_branch_head.return_value = ['pike_contract01'] + self.assertRaises(SystemExit, self.db.sync) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + @mock.patch.object(manage.DbCommands, 'expand') + def test_sync_failed_to_sync(self, mock_expand, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['ocata_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', ''] + mock_expand.side_effect = exception.GlanceException + exit = self.assertRaises(SystemExit, self.db.sync) + self.assertIn('Failed to sync database: ERROR:', exit.code) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + @mock.patch.object(manage.DbCommands, '_sync') + def test_expand(self, mock_sync, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.side_effect = ['ocata_contract01', + 'pike_expand01'] + mock_get_alembic_branch_head.side_effect = ['pike_expand01', + 'pike_contract01'] + self.db.expand() + mock_sync.assert_called_once_with(version='pike_expand01') + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_expand_if_not_expand_head(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['ocata_contract01'] + mock_get_alembic_branch_head.return_value = [] + exit = self.assertRaises(SystemExit, self.db.expand) + self.assertIn('Database expansion failed. Couldn\'t find head ' + 'revision of expand branch.', exit.code) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_expand_db_is_already_sync(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['pike_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_expand01', + 'pike_contract01'] + self.assertRaises(SystemExit, self.db.expand) + self.assertIn('Database is up to date. No migrations needed.', + self.output.getvalue()) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_expand_already_sync(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['pike_expand01'] + mock_get_alembic_branch_head.side_effect = ['pike_expand01', + 'pike_contract01'] + self.db.expand() + self.assertIn('Database expansion is up to date. ' + 'No expansion needed.', self.output.getvalue()) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + @mock.patch.object(manage.DbCommands, '_sync') + def test_expand_failed(self, mock_sync, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.side_effect = ['ocata_contract01', + 'test'] + mock_get_alembic_branch_head.side_effect = ['pike_expand01', + 'pike_contract01'] + exit = self.assertRaises(SystemExit, self.db.expand) + mock_sync.assert_called_once_with(version='pike_expand01') + self.assertIn('Database expansion failed. Database expansion should ' + 'have brought the database version up to "pike_expand01"' + ' revision. But, current revisions are: test ', + exit.code) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + @mock.patch.object(manage.DbCommands, '_sync') + def test_contract(self, mock_sync, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.side_effect = ['pike_expand01', + 'pike_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', + 'pike_expand01'] + self.db.contract() + mock_sync.assert_called_once_with(version='pike_contract01') + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_contract_if_not_contract_head(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['ocata_contract01'] + mock_get_alembic_branch_head.return_value = [] + exit = self.assertRaises(SystemExit, self.db.contract) + self.assertIn('Database contraction failed. Couldn\'t find head ' + 'revision of contract branch.', exit.code) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_contract_db_is_already_sync(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['pike_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', + 'pike_expand01'] + self.assertRaises(SystemExit, self.db.contract) + self.assertIn('Database is up to date. No migrations needed.', + self.output.getvalue()) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_contract_before_expand(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['ocata_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_expand01', + 'pike_contract01'] + exit = self.assertRaises(SystemExit, self.db.contract) + self.assertIn('Database contraction did not run. Database ' + 'contraction cannot be run before database expansion. ' + 'Run database expansion first using "glance-manage db ' + 'expand"', exit.code) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.data_migrations.' + 'has_pending_migrations') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_contract_before_migrate(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_curr_alembic_heads, + mock_has_pending_migrations): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_curr_alembic_heads.side_effect = ['pike_expand01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', + 'pike_expand01'] + mock_has_pending_migrations.return_value = [mock.Mock()] + exit = self.assertRaises(SystemExit, self.db.contract) + self.assertIn('Database contraction did not run. Database ' + 'contraction cannot be run before data migration is ' + 'complete. Run data migration using "glance-manage db ' + 'migrate".', exit.code) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.data_migrations.' + 'has_pending_migrations') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_migrate(self, mock_validate_engine, mock_get_alembic_branch_head, + mock_get_current_alembic_heads, + mock_has_pending_migrations): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.side_effect = ['pike_expand01', + 'pike_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', + 'pike_expand01'] + mock_has_pending_migrations.return_value = None + self.db.migrate() + self.assertIn('Database migration is up to date. ' + 'No migration needed.', self.output.getvalue()) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_migrate_db_is_already_sync(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['pike_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', + 'pike_expand01'] + self.assertRaises(SystemExit, self.db.migrate) + self.assertIn('Database is up to date. No migrations needed.', + self.output.getvalue()) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_migrate_already_sync(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['ocata_contract01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', + 'pike_expand01'] + exit = self.assertRaises(SystemExit, self.db.migrate) + self.assertIn('Data migration did not run. Data migration cannot be ' + 'run before database expansion. Run database expansion ' + 'first using "glance-manage db expand"', exit.code) + + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.data_migrations.' + 'has_pending_migrations') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_current_alembic_heads') + @mock.patch( + 'glance.db.sqlalchemy.alembic_migrations.get_alembic_branch_head') + @mock.patch.object(manage.DbCommands, '_validate_engine') + def test_migrate_before_expand(self, mock_validate_engine, + mock_get_alembic_branch_head, + mock_get_current_alembic_heads, + mock_has_pending_migrations): + engine = mock_validate_engine.return_value + engine.engine.name = 'mysql' + mock_get_current_alembic_heads.return_value = ['pike_expand01'] + mock_get_alembic_branch_head.side_effect = ['pike_contract01', + 'pike_expand01'] + mock_has_pending_migrations.return_value = None + self.db.migrate() + self.assertIn('Database migration is up to date. ' + 'No migration needed.', self.output.getvalue()) + @mock.patch.object(manage.DbCommands, 'version') def test_db_version(self, version): self._main_test_helper(['glance.cmd.manage', 'db', 'version'],