From: armando-migliaccio <amigliaccio@nicira.com>
Date: Tue, 30 Apr 2013 23:19:04 +0000 (-0700)
Subject: Fix 'null' response on router-interface-remove operation
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2516c136549195a92a20aa6570e27af5dfbef28a;p=openstack-build%2Fneutron-build.git

Fix 'null' response on router-interface-remove operation

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
---

diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py
index 953756ac7..f98a05b6f 100644
--- a/quantum/db/l3_db.py
+++ b/quantum/db/l3_db.py
@@ -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:
diff --git a/quantum/plugins/midonet/plugin.py b/quantum/plugins/midonet/plugin.py
index 15a4a6293..9baff5d88 100644
--- a/quantum/plugins/midonet/plugin.py
+++ b/quantum/plugins/midonet/plugin.py
@@ -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 "
diff --git a/quantum/plugins/nicira/QuantumPlugin.py b/quantum/plugins/nicira/QuantumPlugin.py
index 9aed89bc8..882c91362 100644
--- a/quantum/plugins/nicira/QuantumPlugin.py
+++ b/quantum/plugins/nicira/QuantumPlugin.py
@@ -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,
diff --git a/quantum/tests/unit/test_l3_plugin.py b/quantum/tests/unit/test_l3_plugin.py
index 0152830fa..6060661b4 100644
--- a/quantum/tests/unit/test_l3_plugin.py
+++ b/quantum/tests/unit/test_l3_plugin.py
@@ -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():