objects: Fix VIFMigrateData.supports_os_vif_delegation setter

We're using a 'profile' getter/setter in the 'VIFMigrateData' object to
ensure we transform the JSON encoded string we store in the database to
an actual dictionary on load and vice versa on save. However, because
the getter is returning a new object (constructed from 'json.loads')
rather than a reference to the original data (which is a string),
modifying this object doesn't actually change the underlying data in the
object. We were relying on this broken behavior to set the
'supports_os_vif_delegation' attribute of the 'VIFMigrateData' object
and trigger the delegated port creation introduced in change
I11fb5d3ada7f27b39c183157ea73c8b72b4e672e, which means that code isn't
actually doing anything yet. Resolve this.

Change-Id: I362deb1088c88cdcd8219922da9dc9a01b10a940
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Related-Bug: #1734320
Related-Bug: #1815989
This commit is contained in:
Stephen Finucane 2021-06-18 17:55:50 +01:00
parent a0ec2de968
commit 99cf5292c7
3 changed files with 47 additions and 1 deletions

View File

@ -86,7 +86,10 @@ class VIFMigrateData(obj_base.NovaObject):
# info in the profile catch-all blob
@supports_os_vif_delegation.setter
def supports_os_vif_delegation(self, supported):
self.profile[OS_VIF_DELEGATION] = supported
# we can't simply set the attribute using dict notation since the
# getter returns a copy of the data, not the data itself
self.profile = dict(
self.profile or {}, **{OS_VIF_DELEGATION: supported})
def get_dest_vif(self):
"""Get a destination VIF representation of this object.

View File

@ -316,3 +316,24 @@ class TestVIFMigrateData(test.NoDBTestCase):
self.assertEqual(len(vifs), len(mig_vifs))
self.assertEqual([vif['id'] for vif in vifs],
[mig_vif.port_id for mig_vif in mig_vifs])
def test_supports_os_vif_delegation(self):
# first try setting on a object without 'profile' defined
migrate_vif = objects.VIFMigrateData(
port_id=uuids.port_id, vnic_type=network_model.VNIC_TYPE_NORMAL,
vif_type=network_model.VIF_TYPE_OVS, vif_details={},
host='fake-dest-host')
migrate_vif.supports_os_vif_delegation = True
self.assertTrue(migrate_vif.supports_os_vif_delegation)
self.assertEqual(migrate_vif.profile, {'os_vif_delegation': True})
# now do the same but with profile defined
migrate_vif = objects.VIFMigrateData(
port_id=uuids.port_id, vnic_type=network_model.VNIC_TYPE_NORMAL,
vif_type=network_model.VIF_TYPE_OVS, vif_details={},
host='fake-dest-host', profile={'interface_name': 'eth0'})
migrate_vif.supports_os_vif_delegation = True
self.assertTrue(migrate_vif.supports_os_vif_delegation)
self.assertEqual(
migrate_vif.profile,
{'os_vif_delegation': True, 'interface_name': 'eth0'})

View File

@ -10901,6 +10901,28 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.context, instance_ref,
compute_info, compute_info, False)
@mock.patch(
'nova.network.neutron.API.supports_port_binding_extension',
new=mock.Mock(return_value=True))
@mock.patch.object(
libvirt_driver.LibvirtDriver,
'_create_shared_storage_test_file',
return_value='fake')
@mock.patch.object(libvirt_driver.LibvirtDriver, '_compare_cpu')
def test_check_can_live_migrate_dest_create_dummy_vif(
self, mock_cpu, mock_test_file,
):
instance_ref = objects.Instance(**self.test_instance)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
compute_info = {'cpu_info': 'asdf', 'disk_available_least': 1}
result = drvr.check_can_live_migrate_destination(
self.context, instance_ref, compute_info, compute_info)
self.assertIn('vifs', result)
self.assertIsInstance(result.vifs, list)
for vif in result.vifs:
self.assertTrue(vif.supports_os_vif_delegation)
@mock.patch.object(host.Host, 'compare_cpu')
@mock.patch.object(nova.virt.libvirt, 'config')
def test_compare_cpu_compatible_host_cpu(self, mock_vconfig, mock_compare):