Add "delete_port_binding" network API method

This adds a method to delete the binding for
a given port and host, which is something the
compute service will need to do for port bindings
on the source host during post-live-migration or
the destination host when rolling back from a failed
live migration.

There is no top-level definition of this in
nova.network.base_api.NetworkAPI because the
later patches that use this will only do so
via other public interfaces into the neutronv2
API code.

Part of blueprint neutron-new-port-binding-api

Change-Id: Ia18d3d78d8782a0c5827fc90b8ab2d346f28ccab

nova/exception.py

@@ -875,6 +875,11 @@ class PortBindingFailed(Invalid):
                 "logs for more information.")
 
 
+class PortBindingDeletionFailed(NovaException):
+    msg_fmt = _("Failed to delete binding for port %(port_id)s and host "
+                "%(host)s.")
+
+
 class PortUpdateFailed(Invalid):
     msg_fmt = _("Port update failed for port %(port_id)s: %(reason)s")
 

nova/network/neutronv2/api.py

@@ -1241,20 +1241,49 @@ class API(base_api.NetworkAPI):
                           'Error: (%s %s)',
                           port_id, host, resp.status_code, resp.text,
                           instance=instance)
-                # TODO(mriedem): move this cleanup code to a separate method
                 for rollback_port_id in bindings_by_port_id:
-                    url = '/v2.0/ports/%s/bindings/%s' % (
-                        rollback_port_id, host)
-                    resp = client.delete(url, raise_exc=False)
-                    if resp.status_code >= 400 and resp.status_code != 404:
+                    try:
+                        self.delete_port_binding(
+                            context, rollback_port_id, host)
+                    except exception.PortBindingDeletionFailed:
                         LOG.warning('Failed to remove binding for port %s on '
-                                    'host %s: (%s %s)', rollback_port_id,
-                                    host, resp.status_code, resp.text,
+                                    'host %s.', rollback_port_id, host,
                                     instance=instance)
                 raise exception.PortBindingFailed(port_id=port_id)
 
         return bindings_by_port_id
 
+    def delete_port_binding(self, context, port_id, host):
+        """Delete the port binding for the given port ID and host
+
+        This method should not be used if "supports_port_binding_extension"
+        returns False.
+
+        :param context: The request context for the operation.
+        :param port_id: The ID of the port with a binding to the host.
+        :param host: The host from which port bindings should be deleted.
+        :raises: nova.exception.PortBindingDeletionFailed if a non-404 error
+            response is received from neutron.
+        """
+        client = _get_ksa_client(context, admin=True)
+        resp = client.delete(
+            '/v2.0/ports/%s/bindings/%s' % (port_id, host),
+            raise_exc=False)
+        if resp:
+            LOG.debug('Deleted binding for port %s and host %s.',
+                      port_id, host)
+        else:
+            # We can safely ignore 404s since we're trying to delete
+            # the thing that wasn't found anyway.
+            if resp.status_code != 404:
+                # Log the details, raise an exception.
+                LOG.error('Unexpected error trying to delete binding '
+                          'for port %s and host %s. Code: %s. '
+                          'Error: %s', port_id, host,
+                          resp.status_code, resp.text)
+                raise exception.PortBindingDeletionFailed(
+                    port_id=port_id, host=host)
+
     def _get_pci_device_profile(self, pci_dev):
         dev_spec = self.pci_whitelist.get_devspec(pci_dev)
         if dev_spec:

nova/tests/unit/network/test_neutronv2.py

@@ -5428,16 +5428,41 @@ class TestPortBindingWithMock(test.NoDBTestCase):
             return mock_response
 
         mock_client.return_value.post.side_effect = fake_post
-        mock_client.return_value.delete.return_value = (
-            fake_req.FakeResponse(204))
-        self.assertRaises(exception.PortBindingFailed,
-                          self.api.bind_ports_to_host, ctxt, inst, 'fake-host')
+        with mock.patch.object(self.api, 'delete_port_binding',
+                               # This will be logged but not re-raised.
+                               side_effect=exception.PortBindingDeletionFailed(
+                                   port_id=uuids.ok, host='fake-host'
+                               )) as mock_delete:
+            self.assertRaises(exception.PortBindingFailed,
+                              self.api.bind_ports_to_host,
+                              ctxt, inst, 'fake-host')
         # assert that post was called twice and delete once
         self.assertEqual(2, mock_client.return_value.post.call_count)
-        self.assertEqual(1, mock_client.return_value.delete.call_count)
-        # and that delete was called on the first port
-        self.assertIn(uuids.ok,
-                      mock_client.return_value.delete.call_args[0][0])
+        mock_delete.assert_called_once_with(ctxt, uuids.ok, 'fake-host')
+
+    @mock.patch('nova.network.neutronv2.api._get_ksa_client')
+    def test_delete_port_binding(self, mock_client):
+        # Create three ports where:
+        # - one is successfully unbound
+        # - one is not found
+        # - one fails to be unbound
+        ctxt = context.get_context()
+
+        def fake_delete(url, *args, **kwargs):
+            if uuids.ok in url:
+                return fake_req.FakeResponse(204)
+            else:
+                status_code = 404 if uuids.notfound in url else 500
+                return fake_req.FakeResponse(status_code)
+
+        mock_client.return_value.delete.side_effect = fake_delete
+        for port_id in (uuids.ok, uuids.notfound, uuids.fail):
+            if port_id == uuids.fail:
+                self.assertRaises(exception.PortBindingDeletionFailed,
+                                  self.api.delete_port_binding,
+                                  ctxt, port_id, 'fake-host')
+            else:
+                self.api.delete_port_binding(ctxt, port_id, 'fake-host')
 
 
 class TestAllocateForInstance(test.NoDBTestCase):