diff --git a/doc/source/cli/ironic-dbsync.rst b/doc/source/cli/ironic-dbsync.rst
index e30f8b351b..bba2633d8c 100644
--- a/doc/source/cli/ironic-dbsync.rst
+++ b/doc/source/cli/ironic-dbsync.rst
@@ -188,14 +188,12 @@ upgrade
This command will upgrade existing database tables to the most recent version,
or to the version specified with the :option:`--revision` option.
-..
- TODO(rloo): add this in Queens; doesn't make sense to add in Pike
- Before this ``upgrade`` is invoked, the command
- `ironic db-sync online_data_migrations` must have been successfully run using
- the previous version of ironic (if you are doing an upgrade as opposed to a
- new installation of ironic). If it wasn't run, the database will not be
- compatible with this recent version of ironic, and this command will return
- 2 (error).
+Before this ``upgrade`` is invoked, the command
+:command:`ironic-dbsync online_data_migrations` must have been successfully run
+using the previous version of ironic (if you are doing an upgrade as opposed to
+a new installation of ironic). If it wasn't run, the database will not be
+compatible with this recent version of ironic, and this command will return
+2 (error).
If there are no existing tables, then new tables are created, beginning
with the oldest known version, and successively upgraded using all of the
diff --git a/ironic/cmd/dbsync.py b/ironic/cmd/dbsync.py
index 93a1ffd467..6c34bee219 100644
--- a/ironic/cmd/dbsync.py
+++ b/ironic/cmd/dbsync.py
@@ -75,6 +75,10 @@ class DBCommand(object):
If it isn't compatible, we exit the program, returning 2.
"""
+ if migration.version() is None:
+ # 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 '
@@ -86,13 +90,7 @@ class DBCommand(object):
sys.exit(2)
def upgrade(self):
- # TODO(rloo): enable this in Queens because we want the check done
- # before someone tries to upgrade to Queens.
- # It won't work now because the check looks at the value in the new
- # 'version' column for all objects. And this column doesn't exist until
- # *after* an upgrade to this Pike release (i.e., after this
- # 'ironic-dbsync upgrade' is run).
- # self._check_versions()
+ self._check_versions()
migration.upgrade(CONF.command.revision)
def revision(self):
@@ -108,12 +106,7 @@ class DBCommand(object):
migration.create_schema()
def online_data_migrations(self):
- # TODO(rloo): enable this in Queens.
- # It won't work now because the check looks at the value in the new
- # 'version' column for all objects, which cannot be null/None. In Pike,
- # only after running this 'ironic-dbsync online_data_migrations'
- # command, will the version column be populated.
- # self._check_versions()
+ self._check_versions()
self._run_online_data_migrations(max_count=CONF.command.max_count)
def _run_migration_functions(self, context, max_count):
@@ -217,16 +210,14 @@ def add_command_parsers(subparsers):
parser = subparsers.add_parser(
'upgrade',
- # TODO(rloo): Add this to the help string in Queens (because we need
- # to wait until online_data_migrations exists in older release first):
- # It returns 2 (error) if the database is "
- # "not compatible with this version. If this happens, the "
- # "'ironic-dbsync online_data_migrations' command should be run "
- # "using the previous version of ironic, before upgrading and "
- # "running this command."))
help=_("Upgrade the database schema to the latest version. "
"Optionally, use --revision to specify an alembic revision "
- "string to upgrade to."))
+ "string to upgrade to. It returns 2 (error) if the database is "
+ "not compatible with this version. If this happens, the "
+ "'ironic-dbsync online_data_migrations' command should be run "
+ "using the previous version of ironic, before upgrading and "
+ "running this command."))
+
parser.set_defaults(func=command_object.upgrade)
parser.add_argument('--revision', nargs='?')
diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py
index 0b963cac63..122dd363fa 100644
--- a/ironic/db/sqlalchemy/api.py
+++ b/ironic/db/sqlalchemy/api.py
@@ -1149,9 +1149,25 @@ class Connection(api.Connection):
supported_versions = object_versions[model.__name__]
if not supported_versions:
continue
+
+ # TODO(rloo). Because we forgot to set the version of
+ # conductors in Pike, we allow the Conductor version to
+ # be null now (Queens). In Rocky, it cannot be null,
+ # and this 'if...' can be deleted then.
+ if model.__name__ == 'Conductor':
+ query = model_query(model.version).filter(
+ model.version.notin_(supported_versions))
+ if query.count():
+ return False
+ continue
+
+ # 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
+ # compatible with its (old) DB representation.
# NOTE(rloo): .notin_ does not handle null:
# http://docs.sqlalchemy.org/en/latest/core/sqlelement.html#sqlalchemy.sql.operators.ColumnOperators.notin_
- query = model_query(model).filter(
+ query = model_query(model.version).filter(
sql.or_(model.version == sql.null(),
model.version.notin_(supported_versions)))
if query.count():
diff --git a/ironic/tests/unit/db/test_api.py b/ironic/tests/unit/db/test_api.py
index 86b2002343..19cf9f70d8 100644
--- a/ironic/tests/unit/db/test_api.py
+++ b/ironic/tests/unit/db/test_api.py
@@ -50,6 +50,27 @@ class UpgradingTestCase(base.DbTestCase):
self.assertEqual('1.0', node.version)
self.assertFalse(self.dbapi.check_versions())
+ def test_check_versions_conductor(self):
+ for v in self.object_versions['Conductor']:
+ conductor = utils.create_test_conductor(
+ uuid=uuidutils.generate_uuid(), version=v)
+ conductor = self.dbapi.get_conductor(conductor.hostname)
+ self.assertEqual(v, conductor.version)
+ self.assertTrue(self.dbapi.check_versions())
+
+ def test_check_versions_conductor_no_version(self):
+ # works in Queens only
+ conductor = utils.create_test_conductor(version=None)
+ conductor = self.dbapi.get_conductor(conductor.hostname)
+ self.assertIsNone(conductor.version)
+ self.assertTrue(self.dbapi.check_versions())
+
+ def test_check_versions_conductor_old(self):
+ conductor = utils.create_test_conductor(version='1.0')
+ conductor = self.dbapi.get_conductor(conductor.hostname)
+ self.assertEqual('1.0', conductor.version)
+ self.assertFalse(self.dbapi.check_versions())
+
class BackfillVersionTestCase(base.DbTestCase):
diff --git a/releasenotes/notes/dbsync-check-version-c71d5f4fd89ed117.yaml b/releasenotes/notes/dbsync-check-version-c71d5f4fd89ed117.yaml
new file mode 100644
index 0000000000..65ce108d95
--- /dev/null
+++ b/releasenotes/notes/dbsync-check-version-c71d5f4fd89ed117.yaml
@@ -0,0 +1,11 @@
+---
+upgrade:
+ - |
+ The ``ironic-dbsync`` command will check the object versions to make
+ sure they are compatible with the new ironic release, before doing the
+ ``upgrade`` or ``online_data_migrations``.
+other:
+ - |
+ The ``ironic-dbsync`` command will check the object versions to make
+ sure they are compatible with the new ironic release, before doing the
+ ``upgrade`` or ``online_data_migrations``.