From 724f67231b7071d76e3725fbea5489c47cdf5495 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Sun, 25 Mar 2018 18:00:36 -0400 Subject: [PATCH] Delete port bindings in setup_networks_on_host if teardown=True This change adds some logic to the neutronv2 API implementation of setup_networks_on_host such that if teardown=True and a host is provided (which is not the current instance.host), port bindings, for the instance in scope, for that host are deleted. If the port bindings don't exist, then Neutron will return a 404 which we ignore. This will be used later in _post_live_migration and _rollback_live_migration flows. In the post case, the live migration was successful so the inactive port bindings on the source host will be deleted. In the rollback case, the live migration failed so the inactive port bindings on the destination host will be deleted. Part of blueprint neutron-new-port-binding-api Change-Id: I588d6845c69a8cb751dc17b615884985a5c40155 --- nova/network/neutronv2/api.py | 28 ++++++++++- nova/tests/unit/network/test_neutronv2.py | 61 +++++++++++++++++++---- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index fa845f8aea10..781cc77b10dc 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -367,7 +367,23 @@ class API(base_api.NetworkAPI): def setup_networks_on_host(self, context, instance, host=None, teardown=False): - """Setup or teardown the network structures.""" + """Setup or teardown the network structures. + + :param context: The user request context. + :param instance: The instance with attached ports. + :param host: Optional host used to control the setup. If provided and + is not the same as the current instance.host, this method assumes + the instance is being migrated and sets the "migrating_to" + attribute in the binding profile for the attached ports. + :param teardown: Whether or not network information for the ports + should be cleaned up. If True, at a minimum the "migrating_to" + attribute is cleared in the binding profile for the ports. If a + host is also provided, then port bindings for that host are + deleted when teardown is True as long as the host does not match + the current instance.host. + :raises: nova.exception.PortBindingDeletionFailed if host is not None, + teardown is True, and port binding deletion fails. + """ # 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 @@ -385,6 +401,16 @@ class API(base_api.NetworkAPI): # Reset the port profile self._clear_migration_port_profile( context, instance, admin_client, ports) + # If a host was provided, delete any bindings between that + # host and the ports as long as the host isn't the same as + # the current instance.host. + has_binding_ext = self.supports_port_binding_extension(context) + if port_migrating and has_binding_ext: + for port in ports: + # This call is safe in that 404s for non-existing + # bindings are ignored. + self.delete_port_binding( + context, port['id'], host) elif port_migrating: # Setup the port profile self._setup_migration_port_profile( diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index c6aca3bf1011..7e5c89cd0ff0 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -4311,7 +4311,7 @@ class TestNeutronv2WithMock(test.TestCase): 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} + migrate_profile = {neutronapi.MIGRATING_ATTR: 'new-host'} # Pass a port with an migration porfile attribute. port_id = uuids.port_id get_ports = {'ports': [ @@ -4321,12 +4321,53 @@ class TestNeutronv2WithMock(test.TestCase): 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) + with mock.patch.object(self.api, 'delete_port_binding') as del_binding: + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=True): + self.api.setup_networks_on_host(self.context, + instance, + host='new-host', + teardown=True) update_port_mock.assert_called_once_with( port_id, {'port': {neutronapi.BINDING_PROFILE: migrate_profile}}) + del_binding.assert_called_once_with( + self.context, port_id, 'new-host') + + @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) + def test_update_port_profile_for_migration_teardown_true_with_profile_exc( + self, get_client_mock): + """Tests that delete_port_binding raises PortBindingDeletionFailed + which is raised through to the caller. + """ + instance = fake_instance.fake_instance_obj(self.context) + self.api._has_port_binding_extension = mock.Mock(return_value=True) + migrate_profile = {neutronapi.MIGRATING_ATTR: 'new-host'} + # Pass a port with an migration porfile attribute. + get_ports = { + 'ports': [ + {'id': uuids.port1, + neutronapi.BINDING_PROFILE: migrate_profile, + neutronapi.BINDING_HOST_ID: instance.host}, + {'id': uuids.port2, + neutronapi.BINDING_PROFILE: migrate_profile, + neutronapi.BINDING_HOST_ID: instance.host}]} + self.api.list_ports = mock.Mock(return_value=get_ports) + self.api._clear_migration_port_profile = mock.Mock() + with mock.patch.object( + self.api, 'delete_port_binding', + side_effect=exception.PortBindingDeletionFailed( + port_id=uuids.port1, host='new-host')) as del_binding: + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=True): + self.assertRaises(exception.PortBindingDeletionFailed, + self.api.setup_networks_on_host, + self.context, instance, host='new-host', + teardown=True) + self.api._clear_migration_port_profile.assert_called_once_with( + self.context, instance, get_client_mock.return_value, + get_ports['ports']) + del_binding.assert_called_once_with( + self.context, uuids.port1, 'new-host') @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock()) def test_update_port_profile_for_migration_teardown_true_no_profile( @@ -4341,10 +4382,12 @@ class TestNeutronv2WithMock(test.TestCase): 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) + with mock.patch.object(self.api, 'supports_port_binding_extension', + return_value=False): + 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):