From 2516c136549195a92a20aa6570e27af5dfbef28a Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Tue, 30 Apr 2013 16:19:04 -0700 Subject: [PATCH] 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 --- quantum/db/l3_db.py | 1 + quantum/plugins/midonet/plugin.py | 3 ++- quantum/plugins/nicira/QuantumPlugin.py | 8 ++++---- quantum/tests/unit/test_l3_plugin.py | 21 +++++++++++++++++++-- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py index 953756ac72..f98a05b6f5 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 15a4a6293b..9baff5d884 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 9aed89bc8e..882c913626 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 0152830fac..6060661b44 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():