Merge "Support floating ip deletion"

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)

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,
+    def test_disassociate_floatingip(self, mock_context, mock_delete):
-                                     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}})
+        calls = [mock.call(t_ctx, fip_id1),
-        mock_delete.assert_called_once_with(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()

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_ext_client.delete_floatingips(ctx, fip['id'])
-                b_client.update_floatingips(ctx, fip['id'],
-                                            {'floatingip': {'port_id': None}})
 
     @_job_handle(constants.JT_ROUTER_SETUP)
     def setup_bottom_router(self, ctx, payload):