From 55f3d476a12dce8a70d3e485f0f2f9c752cf0b3d Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Tue, 2 Feb 2016 00:35:24 -0800 Subject: [PATCH] Implement setup_networks_on_host for Neutron networks setup_networks_on_host has not been implemented for neutron networks and only implemented for nova net. In order to address the L3 network issues related to the live-migration in neutron, 'setup_networks_on_host' should be implemented in the neutronv2/api. This patch implements the function and updates the portbinding profile dictionary with the 'migrating_to' key pointing to the destination host in pre-live migration phase. port:{'binding:profile':{'migrating_to': 'host'}} When migrate_instance_finish() is called, it should clear the migration profile before binding the host to the destination port to prevent neutron from taking any action when the port-update happens after the port migrates. Based on the port profile update with the destination host, the neutron will be able to create any associated L3 networks on the destination host. Further work is planned to issue a status update notification to nova during the pre-live migration phase after the L3 networks have been created on the destination host and before the port lands on the destination host. This will be addressed in a different patch, since we don't have such wait state in nova at this time. The neutron side changes are handled in different patch sets shown below [1] server side and [2] agent side. [1] https://review.openstack.org/#/c/275420/ [2] https://review.openstack.org/#/c/260738/ NOTE: Older versions of neutron may ignore the new port binding migrating profile information. Change-Id: Ib1cc44bf9d01baf4d1f1d26c2a368a5ca7c6ab68 Partial-Bug: #1456073 --- nova/compute/manager.py | 1 - nova/network/neutronv2/api.py | 91 ++++++++++++- nova/tests/unit/network/test_neutronv2.py | 152 ++++++++++++++++++++++ 3 files changed, 238 insertions(+), 6 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index ee0db485421a..2f8c47c290f1 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2795,7 +2795,6 @@ class ComputeManager(manager.Manager): instance.save(expected_task_state=[task_states.REBUILDING]) if recreate: - # Needed for nova-network, does nothing for neutron self.network_api.setup_networks_on_host( context, instance, self.host) # For nova-network this is needed to move floating IPs diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 57a705301cde..db267101be15 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -49,6 +49,9 @@ _SESSION = None _ADMIN_AUTH = None DEFAULT_SECGROUP = 'default' +BINDING_PROFILE = 'binding:profile' +BINDING_HOST_ID = 'binding:host_id' +MIGRATING_ATTR = 'migrating_to' def reset_state(): @@ -242,9 +245,82 @@ class API(base_api.NetworkAPI): self.last_neutron_extension_sync = None self.extensions = {} + def _update_port_with_migration_profile( + self, instance, port_id, port_profile, admin_client): + try: + updated_port = admin_client.update_port( + port_id, {'port': {BINDING_PROFILE: port_profile}}) + return updated_port + except Exception as ex: + with excutils.save_and_reraise_exception(): + LOG.error(_LE("Unable to update binding profile " + "for port: %(port)s due to failure: %(error)s"), + {'port': port_id, 'error': ex}, + instance=instance) + + def _clear_migration_port_profile( + self, context, instance, admin_client, ports): + for p in ports: + # If the port already has a migration profile and if + # it is to be torn down, then we need to clean up + # the migration profile. + port_profile = p.get(BINDING_PROFILE) + if not port_profile: + continue + if MIGRATING_ATTR in port_profile: + del port_profile[MIGRATING_ATTR] + LOG.debug("Removing port %s migration profile", p['id'], + instance=instance) + self._update_port_with_migration_profile( + instance, p['id'], port_profile, admin_client) + + def _setup_migration_port_profile( + self, context, instance, host, admin_client, ports): + # Migrating to a new host + for p in ports: + # If the host hasn't changed, there is nothing to do. + # But if the destination host is different than the + # current one, please update the port_profile with + # the 'migrating_to'(MIGRATING_ATTR) key pointing to + # the given 'host'. + host_id = p.get(BINDING_HOST_ID) + if host_id != host: + port_profile = p.get(BINDING_PROFILE, {}) + port_profile[MIGRATING_ATTR] = host + self._update_port_with_migration_profile( + instance, p['id'], port_profile, admin_client) + LOG.debug("Port %(port_id)s updated with migration " + "profile %(profile_data)s successfully", + {'port_id': p['id'], + 'profile_data': port_profile}, + instance=instance) + def setup_networks_on_host(self, context, instance, host=None, teardown=False): """Setup or teardown the network structures.""" + if not self._has_port_binding_extension(context, refresh_cache=True): + return + # Check if the instance is migrating to a new host. + port_migrating = host and (instance.host != host) + # If the port is migrating to a new host or if it is a + # teardown on the original host, then proceed. + if port_migrating or teardown: + search_opts = {'device_id': instance.uuid, + 'tenant_id': instance.project_id, + BINDING_HOST_ID: instance.host} + # Now get the port details to process the ports + # binding profile info. + data = self.list_ports(context, **search_opts) + ports = data['ports'] + admin_client = get_client(context, admin=True) + if teardown: + # Reset the port profile + self._clear_migration_port_profile( + context, instance, admin_client, ports) + elif port_migrating: + # Setup the port profile + self._setup_migration_port_profile( + context, instance, host, admin_client, ports) def _get_available_networks(self, context, project_id, net_ids=None, neutron=None, @@ -2286,11 +2362,17 @@ class API(base_api.NetworkAPI): ports = data['ports'] for p in ports: updates = {} + binding_profile = p.get(BINDING_PROFILE, {}) # If the host hasn't changed, like in the case of resizing to the # same host, there is nothing to do. - if p.get('binding:host_id') != host: - updates['binding:host_id'] = host + if p.get(BINDING_HOST_ID) != host: + updates[BINDING_HOST_ID] = host + # NOTE: Before updating the port binding make sure we + # remove the pre-migration status from the binding profile + if binding_profile.get(MIGRATING_ATTR): + del binding_profile[MIGRATING_ATTR] + updates[BINDING_PROFILE] = binding_profile # Update port with newly allocated PCI devices. Even if the # resize is happening on the same host, a new PCI device can be @@ -2301,12 +2383,11 @@ class API(base_api.NetworkAPI): pci_mapping = self._get_pci_mapping_for_migration(context, instance, migration) - binding_profile = p.get('binding:profile', {}) pci_slot = binding_profile.get('pci_slot') new_dev = pci_mapping.get(pci_slot) if new_dev: - updates['binding:profile'] = \ - get_pci_device_profile(new_dev) + binding_profile.update(get_pci_device_profile(new_dev)) + updates[BINDING_PROFILE] = binding_profile else: raise exception.PortUpdateFailed(port_id=p['id'], reason=_("Unable to correlate PCI slot %s") % diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 70e7e96b8125..e4f9aadcd603 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -3630,6 +3630,38 @@ class TestNeutronv2WithMock(test.TestCase): mock_client.update_port.assert_called_once_with('fake-port-id', port_req_body) + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_bindings_for_instance_with_migration_profile( + self, get_client_mock): + instance = fake_instance.fake_instance_obj(self.context) + self.api._has_port_binding_extension = mock.Mock(return_value=True) + + # We pass in a port profile which has a migration attribute and also + # a second port profile attribute 'fake_profile' this can be + # an sriov port profile attribute or a pci_slot attribute, but for + # now we are just using a fake one to show that the code does not + # remove the portbinding_profile it there is one. + binding_profile = {'fake_profile': 'fake_data', + neutronapi.MIGRATING_ATTR: 'my-dest-host'} + fake_ports = {'ports': [ + {'id': 'fake-port-1', + neutronapi.BINDING_PROFILE: binding_profile, + 'binding:host_id': instance.host}]} + list_ports_mock = mock.Mock(return_value=fake_ports) + get_client_mock.return_value.list_ports = list_ports_mock + update_port_mock = mock.Mock() + get_client_mock.return_value.update_port = update_port_mock + + self.api._update_port_binding_for_instance(self.context, instance, + 'my-host') + # Assert that update_port was called on the port with a + # different host and also the migration profile from the port is + # removed since it does not match with the current host. + update_port_mock.assert_called_once_with( + 'fake-port-1', {'port': {'binding:host_id': 'my-host', + neutronapi.BINDING_PROFILE: { + 'fake_profile': 'fake_data'}}}) + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_bindings_for_instance_same_host(self, get_client_mock): @@ -3808,6 +3840,126 @@ class TestNeutronv2WithMock(test.TestCase): self.assertEqual( {new_pci_devices[0].address: old_pci_devices[0]}, pci_mapping) + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_profile_for_migration_teardown_false( + self, get_client_mock): + + instance = fake_instance.fake_instance_obj(self.context) + self.api._has_port_binding_extension = mock.Mock(return_value=True) + # We test with an instance host and destination_host where the + # port will be moving. + get_ports = {'ports': [ + {'id': uuids.port_id, + neutronapi.BINDING_HOST_ID: instance.host}]} + self.api.list_ports = mock.Mock(return_value=get_ports) + update_port_mock = mock.Mock() + get_client_mock.return_value.update_port = update_port_mock + migrate_profile = {neutronapi.MIGRATING_ATTR: 'my-new-host'} + port_data = {'port': + {neutronapi.BINDING_PROFILE: migrate_profile}} + + self.api.setup_networks_on_host(self.context, + instance, + host='my-new-host', + teardown=False) + update_port_mock.assert_called_once_with( + uuids.port_id, + port_data) + + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test__setup_migration_port_profile_called_on_teardown_false( + self, get_client_mock): + + instance = fake_instance.fake_instance_obj(self.context) + self.api._has_port_binding_extension = mock.Mock(return_value=True) + port_id = uuids.port_id + get_ports = {'ports': [ + {'id': port_id, + neutronapi.BINDING_HOST_ID: instance.host}]} + self.api.list_ports = mock.Mock(return_value=get_ports) + self.api._setup_migration_port_profile = mock.Mock() + self.api.setup_networks_on_host(self.context, + instance, + host='my-new-host', + teardown=False) + self.api._setup_migration_port_profile.assert_called_once_with( + self.context, instance, 'my-new-host', + mock.ANY, get_ports['ports']) + + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test__setup_migration_port_profile_not_called_with_host_match( + self, get_client_mock): + + instance = fake_instance.fake_instance_obj(self.context) + self.api._has_port_binding_extension = mock.Mock(return_value=True) + get_ports = {'ports': [ + {'id': uuids.port_id, + neutronapi.BINDING_HOST_ID: instance.host}]} + self.api.list_ports = mock.Mock(return_value=get_ports) + self.api._setup_migration_port_profile = mock.Mock() + self.api._clear_migration_port_profile = mock.Mock() + self.api.setup_networks_on_host(self.context, + instance, + host=instance.host, + teardown=False) + self.api._setup_migration_port_profile.assert_not_called() + self.api._clear_migration_port_profile.assert_not_called() + + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_profile_for_migration_teardown_true_with_profile( + self, get_client_mock): + + instance = fake_instance.fake_instance_obj(self.context) + self.api._has_port_binding_extension = mock.Mock(return_value=True) + migrate_profile = {neutronapi.MIGRATING_ATTR: instance.host} + # Pass a port with an migration porfile attribute. + port_id = uuids.port_id + get_ports = {'ports': [ + {'id': port_id, + neutronapi.BINDING_PROFILE: migrate_profile, + neutronapi.BINDING_HOST_ID: instance.host}]} + self.api.list_ports = mock.Mock(return_value=get_ports) + update_port_mock = mock.Mock() + get_client_mock.return_value.update_port = update_port_mock + self.api.setup_networks_on_host(self.context, + instance, + host=instance.host, + teardown=True) + update_port_mock.assert_called_once_with( + port_id, {'port': {neutronapi.BINDING_PROFILE: migrate_profile}}) + + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_profile_for_migration_teardown_true_no_profile( + self, get_client_mock): + + instance = fake_instance.fake_instance_obj(self.context) + self.api._has_port_binding_extension = mock.Mock(return_value=True) + # Pass a port without any migration porfile attribute. + get_ports = {'ports': [ + {'id': uuids.port_id, + neutronapi.BINDING_HOST_ID: instance.host}]} + self.api.list_ports = mock.Mock(return_value=get_ports) + update_port_mock = mock.Mock() + get_client_mock.return_value.update_port = update_port_mock + self.api.setup_networks_on_host(self.context, + instance, + host=instance.host, + teardown=True) + update_port_mock.assert_not_called() + + def test__update_port_with_migration_profile_raise_exception(self): + + instance = fake_instance.fake_instance_obj(self.context) + port_id = uuids.port_id + migrate_profile = {'fake-attribute': 'my-new-host'} + port_profile = {'port': {neutronapi.BINDING_PROFILE: migrate_profile}} + update_port_mock = mock.Mock(side_effect=test.TestingException()) + admin_client = mock.Mock(update_port=update_port_mock) + self.assertRaises(test.TestingException, + self.api._update_port_with_migration_profile, + instance, port_id, migrate_profile, admin_client) + update_port_mock.assert_called_once_with(port_id, port_profile) + @mock.patch('nova.network.neutronv2.api.compute_utils') def test_get_preexisting_port_ids(self, mocked_comp_utils): mocked_comp_utils.get_nw_info_for_instance.return_value = [model.VIF(