openstack/ironic
Code Issues Proposed changes

Allow initial versions to not be created yet

Browse Source 
The database schema upgrade check had support for an
explicit list of known versions to handle the upgrade.

who knew!

Anyhow, we haven't used it in three years since the addition,
and it seems to make more sense to just be able to indicate
"we know initial field versions may not be able to be retrieved
and act accordingly". As such, when no table is found, the
pre-upgrade version check execution will continue onward fearlessly!

Call it a spiritual successor to Ibcf0728bc5d1b0cbdd78796526f9c93fc99e8c08

Change-Id: Icae5162c2501b0d1217ad0e6ee34ebef40e95204
This commit is contained in:
Julia Kreger 2021-08-09 17:44:27 -07:00
parent 7b097f016b
commit ea8c10cc8b
5 changed files with 59 additions and 22 deletions
doc/source/contributor
releasing.rst
ironic
cmd
dbsync.py
db/sqlalchemy
api.py
tests/unit
cmd
test_dbsync.py
db
test_api.py

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

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

38
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

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

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