Merge "Allow initial versions to not be created yet"

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
 ------------------------------------------------------
 

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 '

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()
+            try:
-            if not db_utils.column_exists(engine,
+                engine = enginefacade.reader.get_engine()
-                                          model.__tablename__,
+                if not db_utils.column_exists(engine,
-                                          model.version.name):
+                                              model.__tablename__,
-                raise exception.DatabaseVersionTooOld()
+                                              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

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):

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(),