diff --git a/ironic/cmd/dbsync.py b/ironic/cmd/dbsync.py index 223cb7eafa..e75a9fb154 100644 --- a/ironic/cmd/dbsync.py +++ b/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(): - sys.stderr.write(_('The database is not compatible with this ' - 'release of ironic (%s). Please run ' - '"ironic-dbsync online_data_migrations" using ' - 'the previous release.\n') - % 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. + try: + if not dbapi.check_versions(): + sys.stderr.write( + _('The database is not compatible with this ' + 'release of ironic (%s). Please run ' + '"ironic-dbsync online_data_migrations" using ' + 'the previous release.\n') + % 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): diff --git a/ironic/common/exception.py b/ironic/common/exception.py index b08bb22190..78970d0d61 100644 --- a/ironic/common/exception.py +++ b/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") diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index a4db43be2b..bcc22a5271 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/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 diff --git a/ironic/tests/unit/db/test_api.py b/ironic/tests/unit/db/test_api.py index 7265cc480a..2fd8742b39 100644 --- a/ironic/tests/unit/db/test_api.py +++ b/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(), diff --git a/releasenotes/notes/better-handle-skip-upgrade-3b6f06ac24937aa4.yaml b/releasenotes/notes/better-handle-skip-upgrade-3b6f06ac24937aa4.yaml new file mode 100644 index 0000000000..cbed51c4a1 --- /dev/null +++ b/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 `_]