Remove PCI parent_addr online migration

This migration was added in mitaka and should have been done either in
mitaka or newton. Newton had some migrations that came after it which
were higher-visibility and should have forced everyone to have done
this by now.

Because of the nature of the migration, there isn't really an easy or
efficient way to validate that they have done this like we have done
for some other migrations.

Change-Id: Idb033e9e52b149372308eabb19c5774e10c56156

nova/cmd/manage.py

@@ -784,7 +784,6 @@ class DbCommands(object):
     """Class for managing the main database."""
 
     online_migrations = (
-        db.pcidevice_online_data_migration,
         db.aggregate_uuids_online_data_migration,
         flavor_obj.migrate_flavors,
         flavor_obj.migrate_flavor_reset_autoincrement,

nova/db/sqlalchemy/api.py

@@ -6413,31 +6413,6 @@ def archive_deleted_rows(max_rows=None):
     return table_to_rows_archived
 
 
-@main_context_manager.writer
-def pcidevice_online_data_migration(context, max_count):
-    from nova.objects import pci_device as pci_dev_obj
-
-    count_all = 0
-    count_hit = 0
-
-    if not pci_dev_obj.PciDevice.should_migrate_data():
-        LOG.error(_LE("Data migrations for PciDevice are not safe, likely "
-                      "because not all services that access the DB directly "
-                      "are updated to the latest version"))
-    else:
-        results = model_query(context, models.PciDevice).filter_by(
-            parent_addr=None).limit(max_count)
-
-        for db_dict in results:
-            count_all += 1
-            pci_dev = pci_dev_obj.PciDevice._from_db_object(
-                context, pci_dev_obj.PciDevice(), db_dict)
-            if pci_dev.obj_what_changed():
-                pci_dev.save()
-                count_hit += 1
-    return count_all, count_hit
-
-
 @main_context_manager.writer
 def aggregate_uuids_online_data_migration(context, max_count):
     from nova.objects import aggregate

nova/objects/pci_device.py

@@ -19,7 +19,6 @@ from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import versionutils
 
-from nova import context
 from nova import db
 from nova import exception
 from nova import objects
@@ -114,18 +113,6 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
         'parent_addr': fields.StringField(nullable=True),
     }
 
-    @staticmethod
-    def should_migrate_data():
-        # NOTE(ndipanov): Only migrate parent_addr if all services are up to at
-        # least version 4 - this should only ever be called from save()
-        services = ('conductor', 'api')
-        min_parent_addr_version = 4
-
-        min_deployed = min(objects.Service.get_minimum_version(
-            context.get_admin_context(), 'nova-' + service)
-            for service in services)
-        return min_deployed >= min_parent_addr_version
-
     def obj_make_compatible(self, primitive, target_version):
         target_version = versionutils.convert_version_to_tuple(target_version)
         if target_version < (1, 2) and 'request_id' in primitive:
@@ -199,10 +186,6 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
                 pci_device.extra_info = jsonutils.loads(extra_info)
         pci_device._context = context
         pci_device.obj_reset_changes()
-        # NOTE(ndipanov): As long as there is PF data in the old location, we
-        # want to load it as it may have be the only place we have it
-        if 'phys_function' in pci_device.extra_info:
-            pci_device.parent_addr = pci_device.extra_info['phys_function']
 
         return pci_device
 
@@ -238,23 +221,7 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
                                   self.address)
         elif self.status != fields.PciDeviceStatus.DELETED:
             updates = self.obj_get_changes()
-            if not self.should_migrate_data():
+            updates['extra_info'] = self.extra_info
-                # NOTE(ndipanov): If we are not migrating data yet, make sure
-                # that any changes to parent_addr are also in the old location
-                # in extra_info
-                if 'parent_addr' in updates and updates['parent_addr']:
-                    extra_update = updates.get('extra_info', {})
-                    if not extra_update and self.obj_attr_is_set('extra_info'):
-                        extra_update = self.extra_info
-                    extra_update['phys_function'] = updates['parent_addr']
-                    updates['extra_info'] = extra_update
-            else:
-                # NOTE(ndipanov): Once we start migrating, meaning all control
-                # plane has been upgraded - aggressively migrate on every save
-                pf_extra = self.extra_info.pop('phys_function', None)
-                if pf_extra and 'parent_addr' not in updates:
-                    updates['parent_addr'] = pf_extra
-                updates['extra_info'] = self.extra_info
 
             if 'extra_info' in updates:
                 updates['extra_info'] = jsonutils.dumps(updates['extra_info'])

nova/tests/unit/db/test_db_api.py

@@ -9677,45 +9677,6 @@ class PciDeviceDBApiTestCase(test.TestCase, ModelsObjectComparatorMixin):
             db.pci_device_update(self.admin_context, v['compute_node_id'],
                                  v['address'], v)
 
-    @mock.patch.object(objects.PciDevice, 'should_migrate_data',
-                       return_value=False)
-    def test_pcidevice_online_mig_not_ready(self, mock_should_migrate):
-        self._create_fake_pci_devs_old_format()
-
-        found, done = db.pcidevice_online_data_migration(self.admin_context,
-                                                         None)
-        self.assertEqual(0, found)
-        self.assertEqual(0, done)
-
-    @mock.patch.object(objects.PciDevice, 'should_migrate_data',
-                       return_value=True)
-    def test_pcidevice_online_mig_data_migrated_limit(self,
-                                                      mock_should_migrate):
-        self._create_fake_pci_devs_old_format()
-
-        found, done = db.pcidevice_online_data_migration(self.admin_context, 1)
-        self.assertEqual(1, found)
-        self.assertEqual(1, done)
-
-    @mock.patch.object(objects.PciDevice, 'should_migrate_data',
-                       return_value=True)
-    def test_pcidevice_online_mig(self, mock_should_migrate):
-        self._create_fake_pci_devs_old_format()
-
-        found, done = db.pcidevice_online_data_migration(self.admin_context,
-                                                         50)
-        self.assertEqual(2, found)
-        self.assertEqual(2, done)
-        results = db.pci_device_get_all_by_node(self.admin_context,
-                                                self.compute_node['id'])
-        for result in results:
-            self.assertEqual('fake-phys-func', result['parent_addr'])
-
-        found, done = db.pcidevice_online_data_migration(self.admin_context,
-                                                         50)
-        self.assertEqual(0, found)
-        self.assertEqual(0, done)
-
     def test_migrate_aggregates(self):
         db.aggregate_create(self.context, {'name': 'foo'})
         db.aggregate_create(self.context, {'name': 'bar',

nova/tests/unit/objects/test_pci_device.py

@@ -177,13 +177,6 @@ class _TestPciDeviceObject(object):
         self.assertEqual(self.pci_device.product_id, 'p')
         self.assertEqual(self.pci_device.obj_what_changed(), set())
 
-    def test_from_db_obj_pre_1_4_format(self):
-        ctxt = context.get_admin_context()
-        dev = pci_device.PciDevice._from_db_object(
-            ctxt, pci_device.PciDevice(), fake_db_dev_old)
-        self.assertEqual('blah', dev.parent_addr)
-        self.assertEqual({'phys_function': 'blah'}, dev.extra_info)
-
     def test_from_db_obj_pre_1_5_format(self):
         ctxt = context.get_admin_context()
         fake_dev_pre_1_5 = copy.deepcopy(fake_db_dev_old)
@@ -212,6 +205,7 @@ class _TestPciDeviceObject(object):
         self.pci_device.status = fields.PciDeviceStatus.ALLOCATED
         self.pci_device.instance_uuid = uuids.instance2
         expected_updates = dict(status=fields.PciDeviceStatus.ALLOCATED,
+                                extra_info='{}',
                                 instance_uuid=uuids.instance2)
         self.mox.StubOutWithMock(db, 'pci_device_update')
         db.pci_device_update(ctxt, 1, 'a',
@@ -263,49 +257,6 @@ class _TestPciDeviceObject(object):
         self.pci_device.save()
         self.assertFalse(self.called)
 
-    @mock.patch.object(objects.Service, 'get_minimum_version', return_value=4)
-    def test_save_migrate_parent_addr(self, get_min_ver_mock):
-        ctxt = context.get_admin_context()
-        dev = pci_device.PciDevice._from_db_object(
-            ctxt, pci_device.PciDevice(), fake_db_dev_old)
-        with mock.patch.object(db, 'pci_device_update',
-                               return_value=fake_db_dev_old) as update_mock:
-            dev.save()
-            update_mock.assert_called_once_with(
-                ctxt, dev.compute_node_id, dev.address,
-                {'extra_info': '{}', 'parent_addr': 'blah'})
-
-    @mock.patch.object(objects.Service, 'get_minimum_version', return_value=4)
-    def test_save_migrate_parent_addr_updated(self, get_min_ver_mock):
-        ctxt = context.get_admin_context()
-        dev = pci_device.PciDevice._from_db_object(
-            ctxt, pci_device.PciDevice(), fake_db_dev_old)
-        # Note that the pci manager code will never update parent_addr alone,
-        # but we want to make it future proof so we guard against it
-        dev.parent_addr = 'doh!'
-        with mock.patch.object(db, 'pci_device_update',
-                               return_value=fake_db_dev_old) as update_mock:
-            dev.save()
-            update_mock.assert_called_once_with(
-                ctxt, dev.compute_node_id, dev.address,
-                {'extra_info': '{}', 'parent_addr': 'doh!'})
-
-    @mock.patch.object(objects.Service, 'get_minimum_version', return_value=2)
-    def test_save_dont_migrate_parent_addr(self, get_min_ver_mock):
-        ctxt = context.get_admin_context()
-        dev = pci_device.PciDevice._from_db_object(
-            ctxt, pci_device.PciDevice(), fake_db_dev_old)
-        dev.extra_info['other'] = "blahtoo"
-        with mock.patch.object(db, 'pci_device_update',
-                               return_value=fake_db_dev_old) as update_mock:
-            dev.save()
-            self.assertEqual("blah",
-                             update_mock.call_args[0][3]['parent_addr'])
-            self.assertIn("phys_function",
-                          update_mock.call_args[0][3]['extra_info'])
-            self.assertIn("other",
-                          update_mock.call_args[0][3]['extra_info'])
-
     def test_update_numa_node(self):
         self.pci_device = pci_device.PciDevice.create(None, dev_dict)
         self.assertEqual(0, self.pci_device.numa_node)

releasenotes/notes/removing-pci-parent_addr-migration-f7dfa2b441cf30e8.yaml

@@ -0,0 +1,7 @@
+---
+upgrade:
+  - In mitaka, an online migration was added to migrate older SRIOV parent
+    device information from extra_info to a new column. Since two releases
+    have gone out with that migration, it is removed in Ocata and operators
+    are expetected to have run it as part of either of the previous two
+    releases, if applicable.