]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix 'null' response on router-interface-remove operation
authorarmando-migliaccio <amigliaccio@nicira.com>
Tue, 30 Apr 2013 23:19:04 +0000 (16:19 -0700)
committerarmando-migliaccio <amigliaccio@nicira.com>
Tue, 30 Apr 2013 23:50:56 +0000 (16:50 -0700)
To avoid a 'null' response body, the delete operation returns a response
that is consistent with its add counterpart, i.e. we return the router
id with details about the interface being affected by the operation, as
well as the tenant id.

A unit test is added to ensure that the right body is returned and minor
adjustments have been made to the plugins affected by the change.

Long-term, a delete operation should really return 204 w/o a body, but
this requires some major rework of the WSGI handling within Quantum.
This is an interim solution that deals with an 'ugly' response body,
whilst keeping backward compatibility.

Fixes bug 1173284

Change-Id: Icaab87ad0c8561c0690c8f0a14db815d8886bc71

quantum/db/l3_db.py
quantum/plugins/midonet/plugin.py
quantum/plugins/nicira/QuantumPlugin.py
quantum/tests/unit/test_l3_plugin.py

index 953756ac720bc1b2aaa517e506c932571b5ac0e4..f98a05b6f5b3a78c6c660e076bbafb611686983a 100644 (file)
@@ -457,6 +457,7 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                             'router.interface.delete',
                             notifier_api.CONF.default_notification_level,
                             {'router.interface': info})
+        return info
 
     def _get_floatingip(self, context, id):
         try:
index 15a4a6293bab2e61edbfd7bb48d8874aa3894cac..9baff5d88483d10ade639641f6844bd262f5500e 100644 (file)
@@ -869,9 +869,10 @@ class MidonetPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
                 break
         assert found
 
-        super(MidonetPluginV2, self).remove_router_interface(
+        info = super(MidonetPluginV2, self).remove_router_interface(
             context, router_id, interface_info)
         LOG.debug(_("MidonetPluginV2.remove_router_interface exiting"))
+        return info
 
     def update_floatingip(self, context, id, floatingip):
         LOG.debug(_("MidonetPluginV2.update_floatingip called: id=%(id)s "
index 9aed89bc8e3c2bb8c653f7251e939d8eabef7157..882c9136262a3c6ca6d6612b89debbdf3e09842f 100644 (file)
@@ -1648,9 +1648,8 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
                         {'port_id': port_id, 'router_id': router_id})
         # Finally remove the data from the Quantum DB
         # This will also destroy the port on the logical switch
-        super(NvpPluginV2, self).remove_router_interface(context,
-                                                         router_id,
-                                                         interface_info)
+        info = super(NvpPluginV2, self).remove_router_interface(
+            context, router_id, interface_info)
         # Destroy router port (no need to unplug the attachment)
         # FIXME(salvatore-orlando): In case of failures in the Quantum plugin
         # this migth leave a dangling port. We perform the operation here
@@ -1659,7 +1658,7 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
             LOG.warning(_("Unable to find NVP logical router port for "
                           "Quantum port id:%s. Was this port ever paired "
                           "with a logical router?"), port_id)
-            return
+            return info
 
         # Ensure the connection to the 'metadata access network'
         # is removed  (with the network) if this the last subnet
@@ -1691,6 +1690,7 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
             raise nvp_exc.NvpPluginException(
                 err_msg=(_("Unable to update logical router"
                            "on NVP Platform")))
+        return info
 
     def _retrieve_and_delete_nat_rules(self, floating_ip_address,
                                        internal_ip, router_id,
index 0152830fac510083b79b8d5e3b5461a68f02c832..6060661b440beb4736daf665353bb21ebd80bb1f 100644 (file)
@@ -360,7 +360,8 @@ class L3NatTestCaseMixin(object):
                             expected_code=expected_code)
 
     def _router_interface_action(self, action, router_id, subnet_id, port_id,
-                                 expected_code=exc.HTTPOk.code):
+                                 expected_code=exc.HTTPOk.code,
+                                 expected_body=None):
         interface_data = {}
         if subnet_id:
             interface_data.update({'subnet_id': subnet_id})
@@ -371,7 +372,10 @@ class L3NatTestCaseMixin(object):
                                       "%s_router_interface" % action)
         res = req.get_response(self.ext_api)
         self.assertEqual(res.status_int, expected_code)
-        return self.deserialize(self.fmt, res)
+        response = self.deserialize(self.fmt, res)
+        if expected_body:
+            self.assertEqual(response, expected_body)
+        return response
 
     @contextlib.contextmanager
     def router(self, name='router1', admin_state_up=True,
@@ -1015,6 +1019,19 @@ class L3NatDBTestCase(L3NatTestCaseBase):
                                                   None,
                                                   p['port']['id'])
 
+    def test_router_remove_interface_returns_200(self):
+        with self.router() as r:
+            with self.port(no_delete=True) as p:
+                body = self._router_interface_action('add',
+                                                     r['router']['id'],
+                                                     None,
+                                                     p['port']['id'])
+                self._router_interface_action('remove',
+                                              r['router']['id'],
+                                              None,
+                                              p['port']['id'],
+                                              expected_body=body)
+
     def test_router_remove_interface_wrong_port_returns_404(self):
         with self.router() as r:
             with self.subnet():