Merge "Detect skip version upgrades from version earlier than Pike"

ironic/cmd/dbsync.py

@@ -24,6 +24,7 @@ import sys
 from oslo_config import cfg
 
 from ironic.common import context
+from ironic.common import exception
 from ironic.common.i18n import _
 from ironic.common import service
 from ironic.conf import CONF
@@ -88,14 +89,28 @@ class DBCommand(object):
             # no tables, nothing to check
             return
 
-        if not dbapi.check_versions():
+        try:
-            sys.stderr.write(_('The database is not compatible with this '
+            if not dbapi.check_versions():
-                               'release of ironic (%s). Please run '
+                sys.stderr.write(
-                               '"ironic-dbsync online_data_migrations" using '
+                    _('The database is not compatible with this '
-                               'the previous release.\n')
+                      'release of ironic (%s). Please run '
-                             % version.version_info.release_string())
+                      '"ironic-dbsync online_data_migrations" using '
-            # NOTE(rloo): We return 1 in online_data_migrations() to indicate
+                      'the previous release.\n')
-            # that there are more objects to migrate, so don't use 1 here.
+                    % version.version_info.release_string())
+                # NOTE(rloo): We return 1 in online_data_migrations() to
+                # indicate that there are more objects to migrate,
+                # so don't use 1 here.
+                sys.exit(2)
+        except exception.DatabaseVersionTooOld:
+            sys.stderr.write(
+                _('The database version is not compatible with this '
+                  'release of ironic (%s). This can happen if you are '
+                  'attempting to upgrade from a version older than '
+                  'the previous release (skip versions upgrade). '
+                  'This is an unsupported upgrade method. '
+                  'Please run "ironic-dbsync upgrade" using the previous '
+                  'releases for a fast-forward upgrade.\n')
+                % version.version_info.release_string())
             sys.exit(2)
 
     def upgrade(self):

ironic/common/exception.py

@@ -783,3 +783,7 @@ class BIOSSettingNotFound(NotFound):
 
 class BIOSSettingListNotFound(NotFound):
     _msg_fmt = _("Node %(node)s doesn't have BIOS settings '%(names)s'")
+
+
+class DatabaseVersionTooOld(IronicException):
+    _msg_fmt = _("Database version is too old")

ironic/db/sqlalchemy/api.py

@@ -1204,6 +1204,17 @@ class Connection(api.Connection):
                 if not supported_versions:
                     continue
 
+                # 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()
+
                 # NOTE(rloo): we use model.version, not model, because we
                 #             know that the object has a 'version' column
                 #             but we don't know whether the entire object is

ironic/tests/unit/db/test_api.py

@@ -10,6 +10,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
+from oslo_db.sqlalchemy import utils as db_utils
 from oslo_utils import uuidutils
 from testtools import matchers
 
@@ -31,6 +33,12 @@ class UpgradingTestCase(base.DbTestCase):
         # nothing in the DB
         self.assertTrue(self.dbapi.check_versions())
 
+    @mock.patch.object(db_utils, 'column_exists', autospec=True)
+    def test_check_versions_missing_version_columns(self, column_exists):
+        column_exists.return_value = False
+        self.assertRaises(exception.DatabaseVersionTooOld,
+                          self.dbapi.check_versions)
+
     def test_check_versions(self):
         for v in self.object_versions['Node']:
             node = utils.create_test_node(uuid=uuidutils.generate_uuid(),

releasenotes/notes/better-handle-skip-upgrade-3b6f06ac24937aa4.yaml

@@ -0,0 +1,11 @@
+---
+fixes:
+  - |
+    Better handles the case when an operator attempts to perform an upgrade
+    from a release older than Pike, directly to a release newer than Pike,
+    skipping one or more releases in between (i.e. a "skip version upgrade").
+    Instead of crashing, the operator will be informed that
+    upgrading from a version older than the previous release is not supported
+    (skip version upgrades) and that (as of Pike) all database migrations need
+    to be performed using the previous releases for a fast-forward upgrade.
+    [Bug `2002558 <https://storyboard.openstack.org/#!/story/2002558>`_]