From: Robert Pothier Date: Thu, 7 Mar 2013 21:43:54 +0000 (-0500) Subject: Add InvalidExternalNetwork exception to display correct error X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d735ccaf2c99c66ed050cd7cfa44c3e73bc7d8ab;p=openstack-build%2Fneutron-build.git Add InvalidExternalNetwork exception to display correct error Fixes bug 1075089 The message in the BadRequest exception raised in file quantum/tests/unit/test_l3_plugin.py is being muted in the logfile since the BadRequest is catched and writes its own log message. Change-Id: Ia0140620205a80dd4b3637b2d41562adb7992b5c --- diff --git a/quantum/common/exceptions.py b/quantum/common/exceptions.py index 493e3dda1..83f6bae02 100644 --- a/quantum/common/exceptions.py +++ b/quantum/common/exceptions.py @@ -249,6 +249,11 @@ class InvalidExtensionEnv(BadRequest): message = _("Invalid extension environment: %(reason)s") +class ExternalIpAddressExhausted(BadRequest): + message = _("Unable to find any IP address on external " + "network %(net_id)s.") + + class TooManyExternalNetworks(QuantumException): message = _("More than one external network exists") diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py index 19a363622..c37448b84 100644 --- a/quantum/db/l3_db.py +++ b/quantum/db/l3_db.py @@ -626,50 +626,39 @@ class L3_NAT_db_mixin(l3.RouterPluginBase): msg = _("Network %s is not a valid external network") % f_net_id raise q_exc.BadRequest(resource='floatingip', msg=msg) - try: - with context.session.begin(subtransactions=True): - # This external port is never exposed to the tenant. - # it is used purely for internal system and admin use when - # managing floating IPs. - external_port = self.create_port(context.elevated(), { - 'port': - {'tenant_id': '', # tenant intentionally not set - 'network_id': f_net_id, - 'mac_address': attributes.ATTR_NOT_SPECIFIED, - 'fixed_ips': attributes.ATTR_NOT_SPECIFIED, - 'admin_state_up': True, - 'device_id': fip_id, - 'device_owner': DEVICE_OWNER_FLOATINGIP, - 'name': ''}}) - # Ensure IP addresses are allocated on external port - if not external_port['fixed_ips']: - msg = _("Unable to find any IP address on external " - "network") - raise q_exc.BadRequest(resource='floatingip', msg=msg) - - floating_fixed_ip = external_port['fixed_ips'][0] - floating_ip_address = floating_fixed_ip['ip_address'] - floatingip_db = FloatingIP( - id=fip_id, - tenant_id=tenant_id, - floating_network_id=fip['floating_network_id'], - floating_ip_address=floating_ip_address, - floating_port_id=external_port['id']) - fip['tenant_id'] = tenant_id - # Update association with internal port - # and define external IP address - self._update_fip_assoc(context, fip, - floatingip_db, external_port) - context.session.add(floatingip_db) - # TODO(salvatore-orlando): Avoid broad catch - # Maybe by introducing base class for L3 exceptions - except q_exc.BadRequest: - LOG.exception(_("Unable to create Floating ip due to a " - "malformed request")) - raise - except Exception: - LOG.exception(_("Floating IP association failed")) - raise + with context.session.begin(subtransactions=True): + # This external port is never exposed to the tenant. + # it is used purely for internal system and admin use when + # managing floating IPs. + external_port = self.create_port(context.elevated(), { + 'port': + {'tenant_id': '', # tenant intentionally not set + 'network_id': f_net_id, + 'mac_address': attributes.ATTR_NOT_SPECIFIED, + 'fixed_ips': attributes.ATTR_NOT_SPECIFIED, + 'admin_state_up': True, + 'device_id': fip_id, + 'device_owner': DEVICE_OWNER_FLOATINGIP, + 'name': ''}}) + # Ensure IP addresses are allocated on external port + if not external_port['fixed_ips']: + raise q_exc.ExternalIpAddressExhausted(net_id=f_net_id) + + floating_fixed_ip = external_port['fixed_ips'][0] + floating_ip_address = floating_fixed_ip['ip_address'] + floatingip_db = FloatingIP( + id=fip_id, + tenant_id=tenant_id, + floating_network_id=fip['floating_network_id'], + floating_ip_address=floating_ip_address, + floating_port_id=external_port['id']) + fip['tenant_id'] = tenant_id + # Update association with internal port + # and define external IP address + self._update_fip_assoc(context, fip, + floatingip_db, external_port) + context.session.add(floatingip_db) + router_id = floatingip_db['router_id'] if router_id: routers = self.get_sync_data(context.elevated(), [router_id]) diff --git a/quantum/tests/unit/bigswitch/test_router_db.py b/quantum/tests/unit/bigswitch/test_router_db.py index 7a050e4ad..1c75a3048 100644 --- a/quantum/tests/unit/bigswitch/test_router_db.py +++ b/quantum/tests/unit/bigswitch/test_router_db.py @@ -152,6 +152,10 @@ class RouterDBTestCase(test_l3_plugin.L3NatDBTestCase): # remove extra port created self._delete('ports', p2['port']['id']) + def test_floatingip_with_invalid_create_port(self): + self._test_floatingip_with_invalid_create_port( + 'quantum.plugins.bigswitch.plugin.QuantumRestProxyV2') + def test_create_floatingip_no_ext_gateway_return_404(self): with self.subnet(cidr='10.0.10.0/24') as public_sub: self._set_net_external(public_sub['subnet']['network_id']) diff --git a/quantum/tests/unit/nicira/test_nicira_plugin.py b/quantum/tests/unit/nicira/test_nicira_plugin.py index e98ebbd4e..2b8e21c85 100644 --- a/quantum/tests/unit/nicira/test_nicira_plugin.py +++ b/quantum/tests/unit/nicira/test_nicira_plugin.py @@ -422,6 +422,11 @@ class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase, 'quantum.plugins.nicira.' 'QuantumPlugin.NvpPluginV2') + def test_floatingip_with_invalid_create_port(self): + self._test_floatingip_with_invalid_create_port( + 'quantum.plugins.nicira.' + 'QuantumPlugin.NvpPluginV2') + def _nvp_metadata_setup(self): cfg.CONF.set_override('metadata_mode', 'access_network', 'NVP') diff --git a/quantum/tests/unit/test_l3_plugin.py b/quantum/tests/unit/test_l3_plugin.py index e71055539..2766fe6cf 100644 --- a/quantum/tests/unit/test_l3_plugin.py +++ b/quantum/tests/unit/test_l3_plugin.py @@ -1187,11 +1187,9 @@ class L3NatDBTestCase(L3NatTestCaseBase): public_sub['subnet']['network_id'], port_id=private_port['port']['id']) self.assertEqual(res.status_int, 400) - for p in self._list('ports')['ports']: if p['device_owner'] == 'network:floatingip': self.fail('garbage port is not deleted') - self._remove_external_gateway_from_router( r['router']['id'], public_sub['subnet']['network_id']) @@ -1204,6 +1202,41 @@ class L3NatDBTestCase(L3NatTestCaseBase): self._test_floatingip_with_assoc_fails( 'quantum.db.l3_db.L3_NAT_db_mixin') + def _test_floatingip_with_ip_generation_failure(self, plugin_class): + with self.subnet(cidr='200.0.0.1/24') as public_sub: + self._set_net_external(public_sub['subnet']['network_id']) + with self.port() as private_port: + with self.router() as r: + sid = private_port['port']['fixed_ips'][0]['subnet_id'] + private_sub = {'subnet': {'id': sid}} + self._add_external_gateway_to_router( + r['router']['id'], + public_sub['subnet']['network_id']) + self._router_interface_action('add', r['router']['id'], + private_sub['subnet']['id'], + None) + method = plugin_class + '._update_fip_assoc' + with mock.patch(method) as pl: + pl.side_effect = q_exc.IpAddressGenerationFailure( + net_id='netid') + res = self._create_floatingip( + self.fmt, + public_sub['subnet']['network_id'], + port_id=private_port['port']['id']) + self.assertEqual(res.status_int, exc.HTTPConflict.code) + + for p in self._list('ports')['ports']: + if p['device_owner'] == 'network:floatingip': + self.fail('garbage port is not deleted') + + self._remove_external_gateway_from_router( + r['router']['id'], + public_sub['subnet']['network_id']) + self._router_interface_action('remove', + r['router']['id'], + private_sub['subnet']['id'], + None) + def test_floatingip_update(self): with self.port() as p: private_sub = {'subnet': {'id': @@ -1266,6 +1299,40 @@ class L3NatDBTestCase(L3NatTestCaseBase): found = True self.assertTrue(found) + def _test_floatingip_with_invalid_create_port(self, plugin_class): + with self.port() as p: + private_sub = {'subnet': {'id': + p['port']['fixed_ips'][0]['subnet_id']}} + with self.subnet(cidr='12.0.0.0/24') as public_sub: + self._set_net_external(public_sub['subnet']['network_id']) + res = self._create_router(self.fmt, _uuid()) + r = self.deserialize(self.fmt, res) + self._add_external_gateway_to_router( + r['router']['id'], + public_sub['subnet']['network_id']) + self._router_interface_action( + 'add', r['router']['id'], + private_sub['subnet']['id'], + None) + + with mock.patch(plugin_class + '.create_port') as createport: + createport.return_value = {'fixed_ips': []} + res = self._create_floatingip( + self.fmt, public_sub['subnet']['network_id'], + port_id=p['port']['id']) + self.assertEqual(res.status_int, + exc.HTTPBadRequest.code) + self._router_interface_action('remove', + r['router']['id'], + private_sub + ['subnet']['id'], + None) + self._delete('routers', r['router']['id']) + + def test_floatingip_with_invalid_create_port(self): + self._test_floatingip_with_invalid_create_port( + 'quantum.db.db_base_plugin_v2.QuantumDbPluginV2') + def test_create_floatingip_no_ext_gateway_return_404(self): with self.subnet() as public_sub: self._set_net_external(public_sub['subnet']['network_id'])