From 7e8f1fcb5349591c26ba5651043058d451351011 Mon Sep 17 00:00:00 2001 From: zhiyuan_cai Date: Thu, 11 Aug 2016 16:33:02 +0800 Subject: [PATCH] Support floating ip deletion 1. What is the problem User cannot correctly delete floating ip via the Tricirce plugin. 2. What is the solution to the problem Overwrite delete_floatingip method in the Tricircle plugin. We first update the floating ip to disassociate it with fixed ip, then invoke delete_floatingip method in the base class to delete the database record. 3. What the features need to be implemented to the Tricircle to realize the solution Deletion of floating ip is now supported. Change-Id: Ia8bf6e7499321c1be085533e8695eea6ac8ef26d --- tricircle/network/plugin.py | 12 ++++- tricircle/tests/unit/network/test_plugin.py | 58 ++++++++++++++++++--- tricircle/xjob/xmanager.py | 12 ++--- 3 files changed, 68 insertions(+), 14 deletions(-) diff --git a/tricircle/network/plugin.py b/tricircle/network/plugin.py index 1238a2cf..366261db 100644 --- a/tricircle/network/plugin.py +++ b/tricircle/network/plugin.py @@ -1098,7 +1098,7 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2, :param context: request context :param _id: ID of the floating ip :param floatingip: data of floating ip we update to - :return: updated floating ip ojbect + :return: updated floating ip object """ org_floatingip_dict = self._make_floatingip_dict( self._get_floatingip(context, _id)) @@ -1171,3 +1171,13 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2, self.xjob_handler.setup_bottom_router( t_ctx, net_id, ori_floatingip_db['router_id'], b_int_net_pod['pod_id']) + + def delete_floatingip(self, context, _id): + """Disassociate floating ip if needed then delete it + + :param context: request context + :param _id: ID of the floating ip + :return: None + """ + self.update_floatingip(context, _id, {'floatingip': {'port_id': None}}) + super(TricirclePlugin, self).delete_floatingip(context, _id) diff --git a/tricircle/tests/unit/network/test_plugin.py b/tricircle/tests/unit/network/test_plugin.py index b1ef9698..e843e056 100644 --- a/tricircle/tests/unit/network/test_plugin.py +++ b/tricircle/tests/unit/network/test_plugin.py @@ -2105,10 +2105,8 @@ class PluginTest(unittest.TestCase, @patch.object(l3_db.L3_NAT_dbonly_mixin, 'update_floatingip', new=update_floatingip) @patch.object(FakeClient, 'delete_floatingips') - @patch.object(FakeClient, 'update_floatingips') @patch.object(context, 'get_context_from_neutron_context') - def test_disassociate_floatingip(self, mock_context, mock_update, - mock_delete): + def test_disassociate_floatingip(self, mock_context, mock_delete): fake_plugin = FakePlugin() q_ctx = FakeNeutronContext() t_ctx = context.get_db_context() @@ -2138,9 +2136,10 @@ class PluginTest(unittest.TestCase, fip_id1 = BOTTOM1_FIPS[0]['id'] fip_id2 = BOTTOM2_FIPS[0]['id'] - mock_update.assert_called_once_with( - t_ctx, fip_id2, {'floatingip': {'port_id': None}}) - mock_delete.assert_called_once_with(t_ctx, fip_id1) + + calls = [mock.call(t_ctx, fip_id1), + mock.call(t_ctx, fip_id2)] + mock_delete.assert_has_calls(calls) mapping = db_api.get_bottom_id_by_top_id_pod_name( t_ctx, bridge_port_name, t_pod['pod_name'], constants.RT_PORT) # check routing for bridge port in top pod is deleted @@ -2151,6 +2150,53 @@ class PluginTest(unittest.TestCase, self.assertIsNone(TOP_FLOATINGIPS[0]['fixed_ip_address']) self.assertIsNone(TOP_FLOATINGIPS[0]['router_id']) + @patch.object(driver.Pool, 'get_instance', new=fake_get_instance) + @patch.object(ipam_pluggable_backend.IpamPluggableBackend, + '_allocate_ips_for_port', new=fake_allocate_ips_for_port) + @patch.object(l3_db.L3_NAT_dbonly_mixin, '_make_router_dict', + new=fake_make_router_dict) + @patch.object(db_base_plugin_common.DbBasePluginCommon, + '_make_subnet_dict', new=fake_make_subnet_dict) + @patch.object(l3_db.L3_NAT_dbonly_mixin, 'update_floatingip', + new=update_floatingip) + @patch.object(FakeClient, 'delete_floatingips') + @patch.object(context, 'get_context_from_neutron_context') + def test_delete_floatingip(self, mock_context, mock_delete): + fake_plugin = FakePlugin() + q_ctx = FakeNeutronContext() + t_ctx = context.get_db_context() + mock_context.return_value = t_ctx + + (t_port_id, b_port_id, + fip, e_net) = self._prepare_associate_floatingip_test(t_ctx, q_ctx, + fake_plugin) + + # associate floating ip + fip_body = {'port_id': t_port_id} + fake_plugin.update_floatingip(q_ctx, fip['id'], + {'floatingip': fip_body}) + + bridge_port_name = constants.ns_bridge_port_name % ( + e_net['tenant_id'], None, b_port_id) + t_pod = db_api.get_top_pod(t_ctx) + mapping = db_api.get_bottom_id_by_top_id_pod_name( + t_ctx, bridge_port_name, t_pod['pod_name'], constants.RT_PORT) + # check routing for bridge port in top pod exists + self.assertIsNotNone(mapping) + + fake_plugin.delete_floatingip(q_ctx, fip['id']) + + fip_id1 = BOTTOM1_FIPS[0]['id'] + fip_id2 = BOTTOM2_FIPS[0]['id'] + + calls = [mock.call(t_ctx, fip_id1), + mock.call(t_ctx, fip_id2)] + mock_delete.assert_has_calls(calls) + mapping = db_api.get_bottom_id_by_top_id_pod_name( + t_ctx, bridge_port_name, t_pod['pod_name'], constants.RT_PORT) + # check routing for bridge port in top pod is deleted + self.assertIsNone(mapping) + @patch.object(context, 'get_context_from_neutron_context') def test_create_security_group_rule(self, mock_context): self._basic_pod_route_setup() diff --git a/tricircle/xjob/xmanager.py b/tricircle/xjob/xmanager.py index f1f47933..9148eb95 100644 --- a/tricircle/xjob/xmanager.py +++ b/tricircle/xjob/xmanager.py @@ -415,9 +415,8 @@ class XManager(PeriodicTasks): b_fips = b_ext_client.list_floatingips( ctx, [{'key': 'floating_network_id', 'comparator': 'eq', 'value': b_ext_net_id}]) - # skip unbound bottom floatingip b_ip_fip_map = dict([(fip['floating_ip_address'], - fip) for fip in b_fips if fip['port_id']]) + fip) for fip in b_fips]) add_fips = [ip for ip in t_ip_fip_map if ip not in b_ip_fip_map] del_fips = [ip for ip in b_ip_fip_map if ip not in t_ip_fip_map] @@ -471,6 +470,9 @@ class XManager(PeriodicTasks): for del_fip in del_fips: fip = b_ip_fip_map[del_fip] + if not fip['port_id']: + b_ext_client.delete_floatingips(ctx, fip['id']) + continue if need_ns_bridge: b_ns_bridge_port = b_ext_client.get_ports(ctx, fip['port_id']) entries = core.query_resource( @@ -490,8 +492,6 @@ class XManager(PeriodicTasks): 'value': b_ns_bridge_net_id}]) if b_int_fips: b_client.delete_floatingips(ctx, b_int_fips[0]['id']) - b_ext_client.update_floatingips( - ctx, fip['id'], {'floatingip': {'port_id': None}}) # for bridge port, we have two resource routing entries, one # for bridge port in top pod, another for bridge port in bottom @@ -523,9 +523,7 @@ class XManager(PeriodicTasks): [{'key': 'bottom_id', 'comparator': 'eq', 'value': t_ns_bridge_port_id}]) - else: - b_client.update_floatingips(ctx, fip['id'], - {'floatingip': {'port_id': None}}) + b_ext_client.delete_floatingips(ctx, fip['id']) @_job_handle(constants.JT_ROUTER_SETUP) def setup_bottom_router(self, ctx, payload):