From 88bae4c30feceb50699316c6f70e107c79afa71e Mon Sep 17 00:00:00 2001
From: Ivar Lazzaro <ivarlazzaro@gmail.com>
Date: Wed, 10 Feb 2016 16:37:53 -0800
Subject: [PATCH] [apic-mapping] retrieve the right subnets during PTG
 allocation

Change-Id: Ib602402cc65c8d6946f9de89d3dc58e0d4859b05
Closes-Bug: 1544369
---
 .../drivers/cisco/apic/apic_mapping.py           | 12 +++++++++++-
 .../grouppolicy/drivers/resource_mapping.py      |  9 ++++++---
 .../services/grouppolicy/test_apic_mapping.py    | 16 ++++++++++++++++
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py
index edb28d0e8..5f8f71e84 100644
--- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py
+++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py
@@ -2182,8 +2182,11 @@ class ApicMappingDriver(api.ResourceMappingDriver,
 
     def _get_l2p_subnets(self, plugin_context, l2p_id):
         l2p = self.gbp_plugin.get_l2_policy(plugin_context, l2p_id)
+        return self._get_l2ps_subnets(plugin_context, [l2p])
+
+    def _get_l2ps_subnets(self, plugin_context, l2ps):
         return self._core_plugin.get_subnets(
-            plugin_context, {'network_id': [l2p['network_id']]})
+            plugin_context, {'network_id': [x['network_id'] for x in l2ps]})
 
     def _configure_implicit_contract(self, context, l2p, transaction=None):
         with self.apic_manager.apic.transaction(transaction) as trs:
@@ -2974,3 +2977,10 @@ class ApicMappingDriver(api.ResourceMappingDriver,
                     rule_name, owner=tenant, transaction=trs,
                     entry=apic_manager.CP_ENTRY + '-' + str(x), **entry)
                 x += 1
+
+    def _get_l3p_allocated_subnets(self, context, l3p_id):
+        l2ps = self._get_l2_policies(context._plugin_context,
+                                     {'l3_policy_id': [l3p_id]})
+        subnets = [x['cidr'] for x in
+                   self._get_l2ps_subnets(context._plugin_context, l2ps)]
+        return subnets
diff --git a/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py b/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py
index 092d82f6c..239d12ad9 100644
--- a/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py
+++ b/gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py
@@ -1443,10 +1443,8 @@ class ResourceMappingDriver(api.PolicyDriver, local_api.LocalAPI,
             l3p['proxy_subnet_prefix_length'] if is_proxy
             else l3p['subnet_prefix_length'])
         l3p_id = l3p['id']
-        ptgs = context._plugin._get_l3p_ptgs(
-            context._plugin_context.elevated(), l3p_id)
         allocated = netaddr.IPSet(
-            iterable=self._get_ptg_cidrs(context, None, ptg_dicts=ptgs))
+            iterable=self._get_l3p_allocated_subnets(context, l3p_id))
         available = pool - allocated
         available.compact()
 
@@ -2568,3 +2566,8 @@ class ResourceMappingDriver(api.PolicyDriver, local_api.LocalAPI,
         master_mac = master_port['mac_address']
         master_ips = [x['ip_address'] for x in master_port['fixed_ips']]
         return master_mac, master_ips
+
+    def _get_l3p_allocated_subnets(self, context, l3p_id):
+        ptgs = context._plugin._get_l3p_ptgs(
+            context._plugin_context.elevated(), l3p_id)
+        return self._get_ptg_cidrs(context, None, ptg_dicts=ptgs)
\ No newline at end of file
diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py
index 333a55a5b..02edb1855 100644
--- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py
+++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py
@@ -1161,6 +1161,22 @@ class TestL2Policy(ApicMappingTestCase):
         self.assertEqual('L3PolicyUpdateOfL2PolicyNotSupported',
                          res['NeutronError']['type'])
 
+    def test_subnet_deallocated(self):
+        l2p = self.create_l2_policy()['l2_policy']
+        ptg = self.create_policy_target_group(
+            l2_policy_id=l2p['id'])['policy_target_group']
+        subnet = netaddr.IPSet(
+            [self._show_subnet(x)['subnet']['cidr'] for x in ptg['subnets']])
+        self.delete_policy_target_group(ptg['id'])
+
+        l2p2 = self.create_l2_policy()['l2_policy']
+        ptg = self.create_policy_target_group(
+            l2_policy_id=l2p2['id'])['policy_target_group']
+
+        subnet2 = netaddr.IPSet(
+            [self._show_subnet(x)['subnet']['cidr'] for x in ptg['subnets']])
+        self.assertFalse(subnet & subnet2)
+
 
 class TestL3Policy(ApicMappingTestCase):