diff options
| -rw-r--r-- | doc/source/contributor/releasing.rst | 2 | ||||
| -rw-r--r-- | ironic/cmd/dbsync.py | 13 | ||||
| -rw-r--r-- | ironic/db/sqlalchemy/api.py | 38 | ||||
| -rw-r--r-- | ironic/tests/unit/cmd/test_dbsync.py | 8 | ||||
| -rw-r--r-- | ironic/tests/unit/db/test_api.py | 20 |
5 files changed, 59 insertions, 22 deletions
diff --git a/doc/source/contributor/releasing.rst b/doc/source/contributor/releasing.rst index f7735c849..6b1d49b3c 100644 --- a/doc/source/contributor/releasing.rst +++ b/doc/source/contributor/releasing.rst @@ -329,8 +329,6 @@ We need to submit patches for changes on master to: are used to migrate from an old release to this latest release; they shouldn't be needed after that.) - * remove any model class names from ``ironic.cmd.dbsync.NEW_MODELS``. - When a release is done that results in a bugfix branch ------------------------------------------------------ diff --git a/ironic/cmd/dbsync.py b/ironic/cmd/dbsync.py index 81524aa96..484e8096c 100644 --- a/ironic/cmd/dbsync.py +++ b/ironic/cmd/dbsync.py @@ -68,11 +68,6 @@ ONLINE_MIGRATIONS = ( (dbapi, 'update_to_latest_versions'), ) -# These are the models added in supported releases. We skip the version check -# for them since the tables do not exist when it happens. -NEW_MODELS = [ -] - class DBCommand(object): @@ -89,14 +84,10 @@ class DBCommand(object): # no tables, nothing to check return - if ignore_missing_tables: - ignore_models = NEW_MODELS - else: - ignore_models = () - msg = None try: - if not dbapi.check_versions(ignore_models=ignore_models): + if not dbapi.check_versions( + permit_initial_version=ignore_missing_tables): msg = (_('The database is not compatible with this ' 'release of ironic (%s). Please run ' '"ironic-dbsync online_data_migrations" using ' diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 5de1dc54a..1de3add32 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -1587,7 +1587,7 @@ class Connection(api.Connection): model.version.notin_(versions))) return query.all() - def check_versions(self, ignore_models=()): + def check_versions(self, ignore_models=(), permit_initial_version=False): """Checks the whole database for incompatible objects. This scans all the tables in search of objects that are not supported; @@ -1596,10 +1596,16 @@ class Connection(api.Connection): that have null 'version' values. :param ignore_models: List of model names to skip. + :param permit_initial_version: Boolean, default False, to permit a + NoSuchTableError exception to be raised + by SQLAlchemy and accordingly bypass + when an object has it's initial object + version. :returns: A Boolean. True if all the objects have supported versions; False otherwise. """ object_versions = release_mappings.get_object_versions() + table_missing_ok = False for model in models.Base.__subclasses__(): if model.__name__ not in object_versions: continue @@ -1611,16 +1617,36 @@ class Connection(api.Connection): if not supported_versions: continue + if permit_initial_version and supported_versions == {'1.0'}: + # We're getting called from someplace it is okay to handle + # a missing table, i.e. database upgrades which will create + # the table *and* the field version is 1.0, which means we + # are likely about to *create* the table, but first have to + # pass the version/compatability checking logic. + table_missing_ok = True + # NOTE(mgagne): Additional safety check to detect old database # version which does not have the 'version' columns available. # This usually means a skip version upgrade is attempted # from a version earlier than Pike which added # those columns required for the next check. - engine = enginefacade.reader.get_engine() - if not db_utils.column_exists(engine, - model.__tablename__, - model.version.name): - raise exception.DatabaseVersionTooOld() + try: + engine = enginefacade.reader.get_engine() + if not db_utils.column_exists(engine, + model.__tablename__, + model.version.name): + raise exception.DatabaseVersionTooOld() + except sa.exc.NoSuchTableError: + if table_missing_ok: + # This is to be expected, it is okay. Moving along. + LOG.warning('Observed missing table while performing ' + 'upgrade version checking. This is not fatal ' + 'as the expected version is only 1.0 and ' + 'the check has been called before the table ' + 'is to be created. Model: %s', + model.__tablename__) + continue + raise # NOTE(rloo): we use model.version, not model, because we # know that the object has a 'version' column diff --git a/ironic/tests/unit/cmd/test_dbsync.py b/ironic/tests/unit/cmd/test_dbsync.py index 530b576e8..618b8bcd5 100644 --- a/ironic/tests/unit/cmd/test_dbsync.py +++ b/ironic/tests/unit/cmd/test_dbsync.py @@ -42,7 +42,8 @@ class OnlineMigrationTestCase(db_base.DbTestCase): mock_check_versions.return_value = True msg = self.db_cmds.check_obj_versions() self.assertIsNone(msg) - mock_check_versions.assert_called_once_with(ignore_models=()) + mock_check_versions.assert_called_once_with( + permit_initial_version=False) def test_check_obj_versions_bad(self): with mock.patch.object(self.dbapi, 'check_versions', @@ -50,7 +51,8 @@ class OnlineMigrationTestCase(db_base.DbTestCase): mock_check_versions.return_value = False msg = self.db_cmds.check_obj_versions() self.assertIsNotNone(msg) - mock_check_versions.assert_called_once_with(ignore_models=()) + mock_check_versions.assert_called_once_with( + permit_initial_version=False) def test_check_obj_versions_ignore_models(self): with mock.patch.object(self.dbapi, 'check_versions', @@ -59,7 +61,7 @@ class OnlineMigrationTestCase(db_base.DbTestCase): msg = self.db_cmds.check_obj_versions(ignore_missing_tables=True) self.assertIsNone(msg) mock_check_versions.assert_called_once_with( - ignore_models=dbsync.NEW_MODELS) + permit_initial_version=True) @mock.patch.object(dbsync.DBCommand, 'check_obj_versions', autospec=True) def test_check_versions_bad(self, mock_check_versions): diff --git a/ironic/tests/unit/db/test_api.py b/ironic/tests/unit/db/test_api.py index f855e9c91..6142fdfae 100644 --- a/ironic/tests/unit/db/test_api.py +++ b/ironic/tests/unit/db/test_api.py @@ -15,6 +15,7 @@ from unittest import mock from oslo_db.sqlalchemy import utils as db_utils from oslo_utils import uuidutils +import sqlalchemy as sa from testtools import matchers from ironic.common import context @@ -42,6 +43,25 @@ class UpgradingTestCase(base.DbTestCase): self.assertRaises(exception.DatabaseVersionTooOld, self.dbapi.check_versions) + @mock.patch.object(release_mappings, 'get_object_versions', autospec=True) + @mock.patch.object(db_utils, 'column_exists', autospec=True) + def test_check_versions_handles_missing_table( + self, column_exists, mock_release_mappings): + column_exists.side_effect = sa.exc.NoSuchTableError('meow') + mock_release_mappings.return_value = {'Node': {'1.0'}} + self.assertTrue( + self.dbapi.check_versions(permit_initial_version=True)) + self.assertEqual(1, column_exists.call_count) + + @mock.patch.object(release_mappings, 'get_object_versions', autospec=True) + @mock.patch.object(db_utils, 'column_exists', autospec=True) + def test_check_versions_raises_missing_table( + self, column_exists, mock_release_mappings): + column_exists.side_effect = sa.exc.NoSuchTableError('meow') + mock_release_mappings.return_value = {'Node': {'1.0', '1.1'}} + self.assertRaises(sa.exc.NoSuchTableError, self.dbapi.check_versions) + self.assertEqual(1, column_exists.call_count) + def test_check_versions(self): for v in self.object_versions['Node']: node = utils.create_test_node(uuid=uuidutils.generate_uuid(), |