Catch neutronclient.NotFound on floating deletion

In some cases, trying to delete a floating IP multiple times in a short
delay can trigger an exception beacause the floating ip deletion
operation is not atomic. If neutronclient's call to delete fails with a
NotFound error, we raise a 404 error to nova's client instead of a 500.

Change-Id: I49ea7e52073148457e794d641ed17d4ef58616f8
Co-Authored-By: Stephen Finucane <sfinucan@redhat.com>
Closes-Bug: #1649852
(cherry picked from commit d99197aece6451013d1de1f08c1af16832ee0e7e)

diff --git a/nova/api/openstack/compute/floating_ips.py b/nova/api/openstack/compute/floating_ips.py
index db021e5e96b4fc0f416236357396e9de6ad1fd53..8275f94fa472e2cf68c0ee3275da5612842b1279 100644 (file)
--- a/nova/api/openstack/compute/floating_ips.py
+++ b/nova/api/openstack/compute/floating_ips.py
@@ -201,6 +201,8 @@ class FloatingIPController(wsgi.Controller):
         except exception.CannotDisassociateAutoAssignedFloatingIP:
             msg = _('Cannot disassociate auto assigned floating IP')
             raise webob.exc.HTTPForbidden(explanation=msg)
+        except exception.FloatingIpNotFoundForAddress as exc:
+            raise webob.exc.HTTPNotFound(explanation=exc.format_message())
 
 
 class FloatingIPActionController(wsgi.Controller):

diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py
index ef1d4cc8f2fcf29a0a7558135020b8675e8e635a..6ae0e4f6232a9664cea9e690d3299ae0add775ff 100644 (file)
--- a/nova/network/neutronv2/api.py
+++ b/nova/network/neutronv2/api.py
@@ -2008,7 +2008,12 @@ class API(base_api.NetworkAPI):
 
         if raise_if_associated and fip['port_id']:
             raise exception.FloatingIpAssociated(address=address)
-        client.delete_floatingip(fip['id'])
+        try:
+            client.delete_floatingip(fip['id'])
+        except neutron_client_exc.NotFound:
+            raise exception.FloatingIpNotFoundForAddress(
+                address=address
+            )
 
     @base_api.refresh_cache
     def disassociate_floating_ip(self, context, instance, address,

diff --git a/nova/tests/unit/api/openstack/compute/test_floating_ips.py b/nova/tests/unit/api/openstack/compute/test_floating_ips.py
index d184769fdbe7a3e4e8fc4fed21df4a0832e5f34f..d75cb4daf04d471b55dc267dc38180dc440db214 100644 (file)
--- a/nova/tests/unit/api/openstack/compute/test_floating_ips.py
+++ b/nova/tests/unit/api/openstack/compute/test_floating_ips.py
@@ -159,6 +159,44 @@ class FloatingIpTestNeutronV21(test.NoDBTestCase):
         ex = exception.InvalidID(id=1)
         self._test_floatingip_delete_not_found(ex, webob.exc.HTTPBadRequest)
 
+    def _test_floatingip_delete_error_disassociate(self, raised_exc,
+                                                   expected_exc):
+        """Ensure that various exceptions are correctly transformed.
+
+        Handle the myriad exceptions that could be raised from the
+        'disassociate_and_release_floating_ip' call.
+        """
+        req = fakes.HTTPRequest.blank('')
+        with mock.patch.object(self.controller.network_api,
+                               'get_floating_ip',
+                               return_value={'address': 'foo'}), \
+             mock.patch.object(self.controller.network_api,
+                               'get_instance_id_by_floating_address',
+                               return_value=None), \
+             mock.patch.object(self.controller.network_api,
+                               'disassociate_and_release_floating_ip',
+                               side_effect=raised_exc):
+            self.assertRaises(expected_exc,
+                              self.controller.delete, req, 1)
+
+    def test_floatingip_delete_error_disassociate_1(self):
+        raised_exc = exception.Forbidden
+        expected_exc = webob.exc.HTTPForbidden
+        self._test_floatingip_delete_error_disassociate(raised_exc,
+                                                        expected_exc)
+
+    def test_floatingip_delete_error_disassociate_2(self):
+        raised_exc = exception.CannotDisassociateAutoAssignedFloatingIP
+        expected_exc = webob.exc.HTTPForbidden
+        self._test_floatingip_delete_error_disassociate(raised_exc,
+                                                        expected_exc)
+
+    def test_floatingip_delete_error_disassociate_3(self):
+        raised_exc = exception.FloatingIpNotFoundForAddress(address='1.1.1.1')
+        expected_exc = webob.exc.HTTPNotFound
+        self._test_floatingip_delete_error_disassociate(raised_exc,
+                                                        expected_exc)
+
 
 class FloatingIpTestV21(test.TestCase):
     floating_ip = "10.10.10.10"

diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py
index 33c9d32b9e53f58c2a7fc49a185445892a917a38..9bd37d36fd7f5711a3b6522553492593e1e79cf5 100644 (file)
--- a/nova/tests/unit/network/test_neutronv2.py
+++ b/nova/tests/unit/network/test_neutronv2.py
@@ -3516,6 +3516,26 @@ class TestNeutronv2WithMock(test.TestCase):
                               api.allocate_floating_ip, self.context,
                               'ext_net')
 
+    @mock.patch('nova.network.neutronv2.api.get_client')
+    @mock.patch('nova.network.neutronv2.api.API._get_floating_ip_by_address',
+                return_value={'port_id': None, 'id': 'abc'})
+    def test_release_floating_ip_not_found(self, mock_get_ip, mock_ntrn):
+        """Ensure neutron's NotFound exception is correctly handled.
+
+        Sometimes, trying to delete a floating IP multiple times in a short
+        delay can trigger an exception because the operation is not atomic. If
+        neutronclient's call to delete fails with a NotFound error, then we
+        should correctly handle this.
+        """
+        mock_nc = mock.Mock()
+        mock_ntrn.return_value = mock_nc
+        mock_nc.delete_floatingip.side_effect = exceptions.NotFound()
+        address = '172.24.4.227'
+
+        self.assertRaises(exception.FloatingIpNotFoundForAddress,
+                          self.api.release_floating_ip,
+                          self.context, address)
+
     @mock.patch.object(client.Client, 'create_port')
     def test_create_port_minimal_raise_no_more_ip(self, create_port_mock):
         instance = fake_instance.fake_instance_obj(self.context)