From d404a3cd4af818518ad918968a6b6031d70cf7b8 Mon Sep 17 00:00:00 2001 From: Ilya Pekelny Date: Wed, 26 Nov 2014 18:19:01 +0200 Subject: [PATCH] Migrate_repo init version helper Helper function to determine init version for migrate repository. We need single automated entry point to determine an initial versions of repositories. Hardcoded and distributed initial version values could contain wrong (for example old) value. Co-Authored-By: Victor Sergeyev Co-Authored-By: Sean Perry Closes-Bug: #1397903 Change-Id: I89616a2cf7a752fbe97591d5694b14f8b86e4539 --- keystone/common/sql/migrate_repo/__init__.py | 17 ---- keystone/common/sql/migration_helpers.py | 30 ++++++- keystone/tests/unit/test_sql_upgrade.py | 86 ++++++++++++++++---- 3 files changed, 98 insertions(+), 35 deletions(-) diff --git a/keystone/common/sql/migrate_repo/__init__.py b/keystone/common/sql/migrate_repo/__init__.py index 9ac5170154..e69de29bb2 100644 --- a/keystone/common/sql/migrate_repo/__init__.py +++ b/keystone/common/sql/migrate_repo/__init__.py @@ -1,17 +0,0 @@ -# Copyright 2014 Mirantis.inc -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -# NOTE(stevemar): This has to be one less than the current lowest db migration -DB_INIT_VERSION = 66 diff --git a/keystone/common/sql/migration_helpers.py b/keystone/common/sql/migration_helpers.py index 8c33989456..40c1fbb533 100644 --- a/keystone/common/sql/migration_helpers.py +++ b/keystone/common/sql/migration_helpers.py @@ -26,7 +26,6 @@ import six import sqlalchemy from keystone.common import sql -from keystone.common.sql import migrate_repo from keystone import contrib from keystone import exception from keystone.i18n import _ @@ -129,7 +128,7 @@ def find_migrate_repo(package=None, repo_name='migrate_repo'): def _sync_common_repo(version): abs_path = find_migrate_repo() - init_version = migrate_repo.DB_INIT_VERSION + init_version = get_init_version() with sql.session_for_write() as session: engine = session.get_bind() _assert_not_schema_downgrade(version=version) @@ -137,6 +136,28 @@ def _sync_common_repo(version): init_version=init_version, sanity_check=False) +def get_init_version(abs_path=None): + """Get the initial version of a migrate repository + + :param abs_path: Absolute path to migrate repository. + :return: initial version number or None, if DB is empty. + """ + if abs_path is None: + abs_path = find_migrate_repo() + + repo = migrate.versioning.repository.Repository(abs_path) + + # Sadly, Repository has a `latest` but not an `oldest`. + # The value is a VerNum object which needs to be converted into an int. + oldest = int(min(repo.versions.versions)) + + if oldest < 1: + return None + + # The initial version is one less + return oldest - 1 + + def _assert_not_schema_downgrade(extension=None, version=None): if version is not None: try: @@ -154,7 +175,6 @@ def _sync_extension_repo(extension, version): if extension in MIGRATED_EXTENSIONS: raise exception.MigrationMovedFailure(extension=extension) - init_version = 0 with sql.session_for_write() as session: engine = session.get_bind() @@ -179,6 +199,8 @@ def _sync_extension_repo(extension, version): _assert_not_schema_downgrade(extension=extension, version=version) + init_version = get_init_version(abs_path=abs_path) + migration.db_sync(engine, abs_path, version=version, init_version=init_version, sanity_check=False) @@ -200,7 +222,7 @@ def get_db_version(extension=None): with sql.session_for_write() as session: return migration.db_version(session.get_bind(), find_migrate_repo(), - migrate_repo.DB_INIT_VERSION) + get_init_version()) try: package_name = '.'.join((contrib.__name__, extension)) diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py index 005ca77574..0f524d5cf0 100644 --- a/keystone/tests/unit/test_sql_upgrade.py +++ b/keystone/tests/unit/test_sql_upgrade.py @@ -33,6 +33,7 @@ import json import uuid from migrate.versioning import api as versioning_api +from migrate.versioning import repository import mock from oslo_config import cfg from oslo_db import exception as db_exception @@ -44,7 +45,6 @@ from sqlalchemy import schema from testtools import matchers from keystone.common import sql -from keystone.common.sql import migrate_repo from keystone.common.sql import migration_helpers from keystone import exception from keystone.tests import unit @@ -119,7 +119,70 @@ INITIAL_TABLE_STRUCTURE = { } +# Test migration_helpers.get_init_version separately to ensure it works before +# using in the SqlUpgrade tests. +class MigrationHelpersGetInitVersionTests(unit.TestCase): + @mock.patch.object(repository, 'Repository') + def test_get_init_version_no_path(self, repo): + migrate_versions = mock.MagicMock() + # make a version list starting with zero. `get_init_version` will + # return None for this value. + migrate_versions.versions.versions = list(range(0, 5)) + repo.return_value = migrate_versions + + # os.path.isdir() is called by `find_migrate_repo()`. Mock it to avoid + # an exception. + with mock.patch('os.path.isdir', return_value=True): + # since 0 is the smallest version expect None + version = migration_helpers.get_init_version() + self.assertIsNone(version) + + # check that the default path was used as the first argument to the + # first invocation of repo. Cannot match the full path because it is + # based on where the test is run. + param = repo.call_args_list[0][0][0] + self.assertTrue(param.endswith('/sql/migrate_repo')) + + @mock.patch.object(repository, 'Repository') + def test_get_init_version_with_path_initial_version_0(self, repo): + migrate_versions = mock.MagicMock() + # make a version list starting with zero. `get_init_version` will + # return None for this value. + migrate_versions.versions.versions = list(range(0, 5)) + repo.return_value = migrate_versions + + # os.path.isdir() is called by `find_migrate_repo()`. Mock it to avoid + # an exception. + with mock.patch('os.path.isdir', return_value=True): + path = '/keystone/migrate_repo/' + + # since 0 is the smallest version expect None + version = migration_helpers.get_init_version(abs_path=path) + self.assertIsNone(version) + + @mock.patch.object(repository, 'Repository') + def test_get_init_version_with_path(self, repo): + initial_version = 10 + + migrate_versions = mock.MagicMock() + migrate_versions.versions.versions = list(range(initial_version + 1, + initial_version + 5)) + repo.return_value = migrate_versions + + # os.path.isdir() is called by `find_migrate_repo()`. Mock it to avoid + # an exception. + with mock.patch('os.path.isdir', return_value=True): + path = '/keystone/migrate_repo/' + + version = migration_helpers.get_init_version(abs_path=path) + self.assertEqual(initial_version, version) + + class SqlMigrateBase(unit.SQLDriverOverrides, unit.TestCase): + # override this in subclasses. The default of zero covers tests such + # as extensions upgrades. + _initial_db_version = 0 + def initialize_sql(self): self.metadata = sqlalchemy.MetaData() self.metadata.bind = self.engine @@ -163,7 +226,7 @@ class SqlMigrateBase(unit.SQLDriverOverrides, unit.TestCase): self.schema = versioning_api.ControlledSchema.create( self.engine, self.repo_path, - self.initial_db_version) + self._initial_db_version) # auto-detect the highest available schema version in the migrate_repo self.max_version = self.schema.repository.version().version @@ -273,14 +336,9 @@ class SqlMigrateBase(unit.SQLDriverOverrides, unit.TestCase): self.assertItemsEqual(expected_cols, actual_cols, '%s table' % table_name) - @property - def initial_db_version(self): - return getattr(self, '_initial_db_version', 0) - class SqlUpgradeTests(SqlMigrateBase): - - _initial_db_version = migrate_repo.DB_INIT_VERSION + _initial_db_version = migration_helpers.get_init_version() def test_blank_db_to_start(self): self.assertTableDoesNotExist('user') @@ -288,14 +346,14 @@ class SqlUpgradeTests(SqlMigrateBase): def test_start_version_db_init_version(self): with sql.session_for_write() as session: version = migration.db_version(session.get_bind(), self.repo_path, - migrate_repo.DB_INIT_VERSION) + self._initial_db_version) self.assertEqual( - migrate_repo.DB_INIT_VERSION, + self._initial_db_version, version, - 'DB is not at version %s' % migrate_repo.DB_INIT_VERSION) + 'DB is not at version %s' % self._initial_db_version) def test_upgrade_add_initial_tables(self): - self.upgrade(migrate_repo.DB_INIT_VERSION + 1) + self.upgrade(self._initial_db_version + 1) self.check_initial_table_structure() def check_initial_table_structure(self): @@ -988,12 +1046,12 @@ class SqlUpgradeTests(SqlMigrateBase): class VersionTests(SqlMigrateBase): - _initial_db_version = migrate_repo.DB_INIT_VERSION + _initial_db_version = migration_helpers.get_init_version() def test_core_initial(self): """Get the version before migrated, it's the initial DB version.""" version = migration_helpers.get_db_version() - self.assertEqual(migrate_repo.DB_INIT_VERSION, version) + self.assertEqual(self._initial_db_version, version) def test_core_max(self): """When get the version after upgrading, it's the new version."""