Fix online upgrades for Bios/Traits
... And tags, but nobody uses tags since it is not available
via the API.
Anyhow, the online upgrade code was written under the assumption
that *all* tables had an "id" column. This is not always true
in the ironic data model for tables which started as pure extensions
of the Nodes table, and fails in particular when:
1) A database row has data stored in an ealier version of the object
2) That same object gets a version upgrade.
In the case which discovered this, BIOSSetting was added at version
1.0, and later updated to include additional fields which incremented
the version to 1.1. When the upgrade went to evaluate and iterate
through the fields, the command failed because the table was designed
around "node_id" instead of "id".
Story: 2010632
Task: 47590
Change-Id: I7bec6cfacb9d1558bc514c07386583436759f4df
(cherry picked from commit cbe5f86ce7)
This commit is contained in:
Julia Kreger
Mark Goddard
parent
59cdb9aeb3
commit
ad227de245
|
|
@ -1715,9 +1715,12 @@ class Connection(api.Connection):
|
|||
max_to_migrate = max_count or total_to_migrate
|
||||
|
||||
for model in sql_models:
|
||||
use_node_id = False
|
||||
if (not hasattr(model, 'id') and hasattr(model, 'node_id')):
|
||||
use_node_id = True
|
||||
version = mapping[model.__name__][0]
|
||||
num_migrated = 0
|
||||
with _session_for_write():
|
||||
with _session_for_write() as session:
|
||||
query = model_query(model).filter(model.version != version)
|
||||
# NOTE(rloo) Caution here; after doing query.count(), it is
|
||||
# possible that the value is different in the
|
||||
|
|
@ -1728,13 +1731,27 @@ class Connection(api.Connection):
|
|||
# max_to_migrate objects.
|
||||
ids = []
|
||||
for obj in query.slice(0, max_to_migrate):
|
||||
ids.append(obj['id'])
|
||||
num_migrated = (
|
||||
model_query(model).
|
||||
filter(sql.and_(model.id.in_(ids),
|
||||
model.version != version)).
|
||||
update({model.version: version},
|
||||
synchronize_session=False))
|
||||
if not use_node_id:
|
||||
ids.append(obj['id'])
|
||||
else:
|
||||
# BIOSSettings, NodeTrait, NodeTag do not have id
|
||||
# columns, fallback to node_id as they both have
|
||||
# it.
|
||||
ids.append(obj['node_id'])
|
||||
if not use_node_id:
|
||||
num_migrated = (
|
||||
session.query(model).
|
||||
filter(sql.and_(model.id.in_(ids),
|
||||
model.version != version)).
|
||||
update({model.version: version},
|
||||
synchronize_session=False))
|
||||
else:
|
||||
num_migrated = (
|
||||
session.query(model).
|
||||
filter(sql.and_(model.node_id.in_(ids),
|
||||
model.version != version)).
|
||||
update({model.version: version},
|
||||
synchronize_session=False))
|
||||
else:
|
||||
num_migrated = (
|
||||
model_query(model).
|
||||
|
|
|
|||
|
|
@ -226,6 +226,11 @@ class UpdateToLatestVersionsTestCase(base.DbTestCase):
|
|||
for i in range(0, num_nodes):
|
||||
node = utils.create_test_node(version=version,
|
||||
uuid=uuidutils.generate_uuid())
|
||||
# Create entries on the tables so we force field upgrades
|
||||
utils.create_test_node_trait(node_id=node.id, trait='foo',
|
||||
version='0.0')
|
||||
utils.create_test_bios_setting(node_id=node.id, version='1.0')
|
||||
|
||||
nodes.append(node.uuid)
|
||||
for uuid in nodes:
|
||||
node = self.dbapi.get_node_by_uuid(uuid)
|
||||
|
|
@ -238,10 +243,15 @@ class UpdateToLatestVersionsTestCase(base.DbTestCase):
|
|||
return
|
||||
|
||||
nodes = self._create_nodes(5)
|
||||
# Check/migrate 2, 10 remain.
|
||||
self.assertEqual(
|
||||
(5, 2), self.dbapi.update_to_latest_versions(self.context, 2))
|
||||
(10, 2), self.dbapi.update_to_latest_versions(self.context, 2))
|
||||
# Check/migrate 10, 8 migrated, 8 remain.
|
||||
self.assertEqual(
|
||||
(3, 3), self.dbapi.update_to_latest_versions(self.context, 10))
|
||||
(8, 8), self.dbapi.update_to_latest_versions(self.context, 10))
|
||||
# Just make sure it is still 0, 0 in case more things are added.
|
||||
self.assertEqual(
|
||||
(0, 0), self.dbapi.update_to_latest_versions(self.context, 10))
|
||||
for uuid in nodes:
|
||||
node = self.dbapi.get_node_by_uuid(uuid)
|
||||
self.assertEqual(self.node_ver, node.version)
|
||||
|
|
@ -250,10 +260,19 @@ class UpdateToLatestVersionsTestCase(base.DbTestCase):
|
|||
if self.node_version_same:
|
||||
# can't test if we don't have diff versions of the node
|
||||
return
|
||||
|
||||
nodes = self._create_nodes(5)
|
||||
vm_count = 5
|
||||
nodes = self._create_nodes(vm_count)
|
||||
# NOTE(TheJulia): Under current testing, 5 node will result in 10
|
||||
# records implicitly needing to be migrated.
|
||||
migrate_count = vm_count * 2
|
||||
self.assertEqual(
|
||||
(5, 5), self.dbapi.update_to_latest_versions(self.context, 5))
|
||||
(migrate_count, migrate_count),
|
||||
self.dbapi.update_to_latest_versions(self.context,
|
||||
migrate_count))
|
||||
self.assertEqual(
|
||||
(0, 0), self.dbapi.update_to_latest_versions(self.context,
|
||||
migrate_count))
|
||||
|
||||
for uuid in nodes:
|
||||
node = self.dbapi.get_node_by_uuid(uuid)
|
||||
self.assertEqual(self.node_ver, node.version)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,14 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixes an issue in the online upgrade logic where database models for
|
||||
Node Traits and BIOS Settings resulted in an error when performing
|
||||
the online data migration. This was because these tables were originally
|
||||
created as extensions of the Nodes database table, and the schema
|
||||
of the database was slightly different enough to result in an error
|
||||
if there was data to migrate in these tables upon upgrade,
|
||||
which would have occured if an early BIOS Setting adopter had
|
||||
data in the database prior to upgrading to the Yoga release of Ironic.
|
||||
|
||||
The online upgrade parameter now subsitutes an alternate primary key name
|
||||
name when applicable.
|
||||
Loading…
Reference in New Issue