Merge "Fix online upgrades for Bios/Traits"

3 files changed, 62 insertions, 12 deletions

diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py
index d5f4a9d65..93a211fc3 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 6142fdfae..2396b1253 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(
+            (10, 2), self.dbapi.update_to_latest_versions(self.context, 2))
+        # Check/migrate 10, 8 migrated, 8 remain.
         self.assertEqual(
-            (5, 2), self.dbapi.update_to_latest_versions(self.context, 2))
+            (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(
-            (3, 3), self.dbapi.update_to_latest_versions(self.context, 10))
+            (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(
+            (migrate_count, migrate_count),
+            self.dbapi.update_to_latest_versions(self.context,
+                                                 migrate_count))
         self.assertEqual(
-            (5, 5), self.dbapi.update_to_latest_versions(self.context, 5))
+            (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 000000000..824185aab
--- /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.