From 1052ad4d5adbb2cb2794f266f21961340c534b69 Mon Sep 17 00:00:00 2001 From: Guillaume Espanel Date: Wed, 14 Dec 2016 15:29:02 +0100 Subject: [PATCH] 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 Closes-Bug: #1649852 (cherry picked from commit d99197aece6451013d1de1f08c1af16832ee0e7e) --- nova/api/openstack/compute/floating_ips.py | 2 + nova/network/neutronv2/api.py | 7 +++- .../openstack/compute/test_floating_ips.py | 38 +++++++++++++++++++ nova/tests/unit/network/test_neutronv2.py | 20 ++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/compute/floating_ips.py b/nova/api/openstack/compute/floating_ips.py index db021e5e96b4..8275f94fa472 100644 --- 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 ef1d4cc8f2fc..6ae0e4f6232a 100644 --- 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 d184769fdbe7..d75cb4daf04d 100644 --- 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 33c9d32b9e53..9bd37d36fd7f 100644 --- 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)