From cc8b300f67caf0e6b90251dea989064ba6337811 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 18 Aug 2020 14:28:34 +0100 Subject: [PATCH] conductor: Don't use setattr setattr kills discoverability, making it hard to figure out who's setting various fields. Don't do it. While we're here, we drop legacy compat handlers for pre-Train compute nodes. Change-Id: Ie694a80e89f99c8d3e326eebb4590d93c0ebf671 Signed-off-by: Stephen Finucane --- nova/conductor/tasks/live_migrate.py | 18 ++++----- nova/tests/fixtures.py | 17 ++++++--- nova/tests/functional/libvirt/base.py | 6 ++- .../unit/conductor/tasks/test_live_migrate.py | 38 ------------------- 4 files changed, 24 insertions(+), 55 deletions(-) diff --git a/nova/conductor/tasks/live_migrate.py b/nova/conductor/tasks/live_migrate.py index 36a856d779db..de8a91fd9a73 100644 --- a/nova/conductor/tasks/live_migrate.py +++ b/nova/conductor/tasks/live_migrate.py @@ -26,7 +26,6 @@ from nova.i18n import _ from nova.network import neutron from nova import objects from nova.objects import fields as obj_fields -from nova.objects import migrate_data as migrate_data_obj from nova.scheduler import utils as scheduler_utils LOG = logging.getLogger(__name__) @@ -369,14 +368,6 @@ class LiveMigrationTask(base.TaskBase): # Check to see that neutron supports the binding-extended API. if self.network_api.supports_port_binding_extension(self.context): - if 'vifs' not in self.migrate_data: - # migrate data vifs were not constructed in dest compute - # during check_can_live_migrate_destination, construct a - # skeleton to be updated after port binding. - # TODO(adrianc): This can be removed once we move to U release - self.migrate_data.vifs = migrate_data_obj.VIFMigrateData.\ - create_skeleton_migrate_vifs( - self.instance.get_network_info()) bindings = self._bind_ports_on_destination( destination, provider_mapping) self._update_migrate_vifs_from_bindings(self.migrate_data.vifs, @@ -432,8 +423,13 @@ class LiveMigrationTask(base.TaskBase): def _update_migrate_vifs_from_bindings(self, migrate_vifs, bindings): for migrate_vif in migrate_vifs: - for attr_name, attr_val in bindings[migrate_vif.port_id].items(): - setattr(migrate_vif, attr_name, attr_val) + binding = bindings[migrate_vif.port_id] + migrate_vif.profile = binding.get('profile') + migrate_vif.vnic_type = binding['vnic_type'] + if 'vif_details' in binding: + migrate_vif.vif_details = binding['vif_details'] + if 'vif_type' in binding: + migrate_vif.vif_type = binding['vif_type'] def _get_source_cell_mapping(self): """Returns the CellMapping for the cell in which the instance lives diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index 04212c5a2095..8a3b23801cd9 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -1325,8 +1325,9 @@ class NeutronFixture(fixtures.Fixture): 'project_id': tenant_id, 'device_id': '', 'binding:profile': {}, - 'binding:vnic_type': 'normal', + 'binding:vif_details': {}, 'binding:vif_type': 'ovs', + 'binding:vnic_type': 'normal', 'port_security_enabled': True, 'security_groups': [ security_group['id'], @@ -1351,8 +1352,9 @@ class NeutronFixture(fixtures.Fixture): 'project_id': tenant_id, 'device_id': '', 'binding:profile': {}, - 'binding:vnic_type': 'normal', + 'binding:vif_details': {}, 'binding:vif_type': 'ovs', + 'binding:vnic_type': 'normal', 'port_security_enabled': True, 'security_groups': [ security_group['id'], @@ -1377,8 +1379,9 @@ class NeutronFixture(fixtures.Fixture): 'project_id': tenant_id, 'device_id': '', 'binding:profile': {}, - 'binding:vnic_type': 'normal', + 'binding:vif_details': {}, 'binding:vif_type': 'ovs', + 'binding:vnic_type': 'normal', 'resource_request': { "resources": { orc.NET_BW_IGR_KILOBIT_PER_SEC: 1000, @@ -1462,9 +1465,9 @@ class NeutronFixture(fixtures.Fixture): 'device_id': '', 'resource_request': {}, 'binding:profile': {}, - 'binding:vnic_type': 'direct', - 'binding:vif_type': 'hw_veb', 'binding:vif_details': {'vlan': 100}, + 'binding:vif_type': 'hw_veb', + 'binding:vnic_type': 'direct', 'port_security_enabled': False, } @@ -1616,6 +1619,8 @@ class NeutronFixture(fixtures.Fixture): "required": ["CUSTOM_PHYSNET2", "CUSTOM_VNIC_TYPE_DIRECT"] }, 'binding:profile': {}, + 'binding:vif_details': {}, + 'binding:vif_type': 'hw_veb', 'binding:vnic_type': 'direct', 'port_security_enabled': False, } @@ -1645,6 +1650,8 @@ class NeutronFixture(fixtures.Fixture): "required": ["CUSTOM_PHYSNET2", "CUSTOM_VNIC_TYPE_MACVTAP"] }, 'binding:profile': {}, + 'binding:vif_details': {}, + 'binding:vif_type': 'hw_veb', 'binding:vnic_type': 'macvtap', 'port_security_enabled': False, } diff --git a/nova/tests/functional/libvirt/base.py b/nova/tests/functional/libvirt/base.py index 053a874c3339..9fe5bdba13e3 100644 --- a/nova/tests/functional/libvirt/base.py +++ b/nova/tests/functional/libvirt/base.py @@ -261,6 +261,7 @@ class LibvirtNeutronFixture(nova_fixtures.NeutronFixture): 'subnet_id': subnet_1['id'] } ], + 'binding:vif_details': {}, 'binding:vif_type': 'ovs', 'binding:vnic_type': 'normal', } @@ -275,6 +276,7 @@ class LibvirtNeutronFixture(nova_fixtures.NeutronFixture): 'subnet_id': subnet_1['id'] } ], + 'binding:vif_details': {}, 'binding:vif_type': 'ovs', 'binding:vnic_type': 'normal', } @@ -289,6 +291,7 @@ class LibvirtNeutronFixture(nova_fixtures.NeutronFixture): 'subnet_id': subnet_2['id'] } ], + 'binding:vif_details': {}, 'binding:vif_type': 'ovs', 'binding:vnic_type': 'normal', } @@ -303,6 +306,7 @@ class LibvirtNeutronFixture(nova_fixtures.NeutronFixture): 'subnet_id': subnet_3['id'] } ], + 'binding:vif_details': {}, 'binding:vif_type': 'ovs', 'binding:vnic_type': 'normal', } @@ -317,9 +321,9 @@ class LibvirtNeutronFixture(nova_fixtures.NeutronFixture): 'subnet_id': subnet_4['id'] } ], + 'binding:vif_details': {'vlan': 42}, 'binding:vif_type': 'hw_veb', 'binding:vnic_type': 'direct', - 'binding:vif_details': {'vlan': 42}, 'binding:profile': {'pci_vendor_info': '1377:0047', 'pci_slot': '0000:81:00.1', 'physical_network': 'physnet4'}, diff --git a/nova/tests/unit/conductor/tasks/test_live_migrate.py b/nova/tests/unit/conductor/tasks/test_live_migrate.py index 9a109becd06d..65841cdad131 100644 --- a/nova/tests/unit/conductor/tasks/test_live_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_live_migrate.py @@ -22,7 +22,6 @@ from nova.compute import vm_states from nova.conductor.tasks import live_migrate from nova import context as nova_context from nova import exception -from nova.network import model as network_model from nova import objects from nova.scheduler.client import query from nova.scheduler.client import report @@ -602,43 +601,6 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): self.assertRaises(exception.MigrationPreCheckError, self.task._call_livem_checks_on_host, {}, {}) - def test_call_livem_checks_on_host_bind_ports(self): - data = objects.LibvirtLiveMigrateData() - bindings = { - uuids.port1: {'host': 'dest-host'}, - uuids.port2: {'host': 'dest-host'} - } - - @mock.patch.object(self.task, '_check_can_migrate_pci') - @mock.patch.object(self.task.compute_rpcapi, - 'check_can_live_migrate_destination', - return_value=data) - @mock.patch.object(self.task.network_api, - 'supports_port_binding_extension', - return_value=True) - @mock.patch.object(self.task.network_api, - 'bind_ports_to_host', return_value=bindings) - def _test(mock_bind_ports_to_host, - mock_supports_port_binding, - mock_check_can_live_migrate_dest, - mock_check_can_migrate_pci): - nwinfo = network_model.NetworkInfo([ - network_model.VIF(uuids.port1), - network_model.VIF(uuids.port2)]) - self.instance.info_cache = objects.InstanceInfoCache( - network_info=nwinfo) - self.task._call_livem_checks_on_host('dest-host', {}) - # Assert the migrate_data set on the task based on the port - # bindings created. - self.assertIn('vifs', data) - self.assertEqual(2, len(data.vifs)) - for vif in data.vifs: - self.assertIn('source_vif', vif) - self.assertEqual('dest-host', vif.host) - self.assertEqual(vif.port_id, vif.source_vif['id']) - - _test() - @mock.patch('nova.network.neutron.API.bind_ports_to_host') def test_bind_ports_on_destination_merges_profiles(self, mock_bind_ports): """Assert that if both the migration_data and the provider mapping