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

@@ -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(

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,
+        with mock.patch.object(self.api, 'delete_port_binding') as del_binding:
-                                        instance,
+            with mock.patch.object(self.api, 'supports_port_binding_extension',
-                                        host=instance.host,
+                                   return_value=True):
-                                        teardown=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,
+        with mock.patch.object(self.api, 'supports_port_binding_extension',
-                                        instance,
+                               return_value=False):
-                                        host=instance.host,
+            self.api.setup_networks_on_host(self.context,
-                                        teardown=True)
+                                            instance,
+                                            host=instance.host,
+                                            teardown=True)
         update_port_mock.assert_not_called()
 
     def test__update_port_with_migration_profile_raise_exception(self):