diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index d5f4a9d650..93a211fc3c 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -1834,6 +1834,9 @@ 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() as session: @@ -1847,13 +1850,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 = ( - session.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 = ( session.query(model). diff --git a/ironic/tests/unit/db/test_api.py b/ironic/tests/unit/db/test_api.py index 6142fdfae6..2396b12532 100644 --- a/ironic/tests/unit/db/test_api.py +++ b/ironic/tests/unit/db/test_api.py @@ -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) diff --git a/releasenotes/notes/fix-online-version-migration-db432a7b239647fa.yaml b/releasenotes/notes/fix-online-version-migration-db432a7b239647fa.yaml new file mode 100644 index 0000000000..824185aab5 --- /dev/null +++ b/releasenotes/notes/fix-online-version-migration-db432a7b239647fa.yaml @@ -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.