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 493e3dda1620d53b36f611f38ea5d00508a1c5c4..83f6bae02663e9d27d6b753ad2c716b97d78dace 100644 (file)
--- 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 19a363622e344e2c05d4d9153b2f1a0aa7cbe776..c37448b84014d77d570bcfdbdd3aad263b02b2e5 100644 (file)
--- 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 7a050e4ad7a8de70c1ac6a70a93290fb7e2505af..1c75a3048d5bd1a3cf97124ad811ca8b2796200e 100644 (file)
--- 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 e98ebbd4e95e9b5f5c6f8252f016b03b4830db89..2b8e21c85930d9689a8b906e61e968885059ef48 100644 (file)
--- 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 e71055539011ab625b985fc676cb844da06adbf3..2766fe6cf6670659da40aef5414fb17ff3ffc5f2 100644 (file)
--- 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'])