[aim-mapping] Auto PTG per L2P

This change automatically creates a PTG per L2P. This PTG is created as a
reverse map of the "default" EPG that is being created per L2P per Neutron
network. We will henceforth refer to this PTG as "auto" PTG.

The ID of the auto PTG is derived from the ID of the L2P as a MD5 hash
calculation (for uniqueness) and persisted in the format:
"auto<hash_of_l2p_id>". It is thus always possible to determine the ID of the
auto PTG from the ID of the L2P and no additional state needs to be maintained.

The initial name of the auto PTG is derived from the ID of the L2P to ease
debugging and troubleshooting, and takes the form: "auto-ptg-<l2p_id>". This
name is mutable (just like any other PTG). The aim_mapping driver does not
have any specical meaning for this name, and does not care about after it
implicitly sets it at the time of the auto PTG creation. Note that changing
the name of the PTG will not result in a change in the display_name of the
default EPG that the auto PTG maps to (since this property is owned by the
apic_aim mechanism driver).

Any Contracts configured directly on the default EPG will be preserved
regardless of any changes to the auto PTG.

The auto PTG cannot be deleted by the end user and doing so will result in
an error.

The shared status of the auto PTG is made consistent with the shared status
of the L2P (once set, it cannot be changed).

The auto PTG is deleted when the corresponding L2P is deleted.

This patch also gets rid of the apic name_mapper's policy_target_group()
function which mapped a policy_target_group's identity to an APIC EPG name.
This is replaced by the function apic_epg_name_for_policy_target_group()
which is now part of the aim_mapping policy driver.

Change-Id: I76977449419fed7984efb534ab88612c88cd3d54
(cherry picked from commit f424a47183)

gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/apic_mapper.py

@@ -156,11 +156,6 @@ class APICNameMapper(object):
     def router(self, session, router_id, router_name=None):
         return router_name
 
-    @mapper(NAME_TYPE_POLICY_TARGET_GROUP)
-    def policy_target_group(self, session, policy_target_group_id,
-                            policy_target_group_name=None):
-        return policy_target_group_name
-
     @mapper(NAME_TYPE_L3_POLICY)
     def l3_policy(self, context, l3_policy_id):
         l3_policy = context._plugin.get_l3_policy(context._plugin_context,

gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py

@@ -10,20 +10,26 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import hashlib
+
 from aim.api import resource as aim_resource
 from aim import context as aim_context
+from aim import utils as aim_utils
 from neutron._i18n import _LE
 from neutron._i18n import _LI
 from neutron.agent.linux import dhcp
 from neutron.api.v2 import attributes
 from neutron.common import constants as n_constants
 from neutron.common import exceptions as n_exc
+from neutron import context as n_context
 from neutron import manager
 from oslo_concurrency import lockutils
+from oslo_config import cfg
 from oslo_log import helpers as log
 from oslo_log import log as logging
 from oslo_utils import excutils
 
+from gbpservice.neutron.db.grouppolicy import group_policy_mapping_db as gpmdb
 from gbpservice.neutron.extensions import cisco_apic
 from gbpservice.neutron.extensions import cisco_apic_gbp as aim_ext
 from gbpservice.neutron.extensions import cisco_apic_l3
@@ -51,6 +57,10 @@ FILTER_DIRECTIONS = {FORWARD: False, REVERSE: True}
 FORWARD_FILTER_ENTRIES = 'Forward-FilterEntries'
 REVERSE_FILTER_ENTRIES = 'Reverse-FilterEntries'
 ADDR_SCOPE_KEYS = ['address_scope_v4_id', 'address_scope_v6_id']
+AUTO_PTG_NAME_PREFIX = 'auto-ptg-%s'
+# Note that this prefix should not exceede 4 characters
+AUTO_PTG_PREFIX = 'auto'
+AUTO_PTG_ID_PREFIX = AUTO_PTG_PREFIX + '%s'
 
 # Definitions duplicated from apicapi lib
 APIC_OWNED = 'apic_owned_'
@@ -64,6 +74,19 @@ CONTRACT_SUBJECTS = 'contract_subjects'
 FILTERS = 'filters'
 FILTER_ENTRIES = 'filter_entries'
 
+# REVISIT: Auto-PTG is currently config driven to align with the
+# config driven behavior of the older driver but is slated for
+# removal.
+opts = [
+    cfg.BoolOpt('create_auto_ptg',
+                default=True,
+                help=_("Automatically create a PTG when a L2 Policy "
+                       "gets created. This is currently an aim_mapping "
+                       "policy driver specific feature.")),
+]
+
+cfg.CONF.register_opts(opts, "aim_mapping")
+
 
 class SimultaneousV4V6AddressScopesNotSupportedOnAimDriver(
     exc.GroupPolicyBadRequest):
@@ -86,6 +109,15 @@ class NoAddressScopeForSubnetpool(exc.GroupPolicyBadRequest):
     message = _("Subnetpool does not have an associated address scope.")
 
 
+class AutoPTGDeleteNotSupported(exc.GroupPolicyBadRequest):
+    message = _("Auto PTG %(id)s cannot be deleted.")
+
+
+class SharedAttributeUpdateNotSupported(exc.GroupPolicyBadRequest):
+    message = _("Resource shared attribute update not supported with AIM "
+                "GBP driver for resource of type %(type)s")
+
+
 class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
     """AIM Mapping Orchestration driver.
 
@@ -99,6 +131,11 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
         super(AIMMappingDriver, self).initialize()
         self._apic_aim_mech_driver = None
         self._apic_segmentation_label_driver = None
+        self.create_auto_ptg = cfg.CONF.aim_mapping.create_auto_ptg
+        if self.create_auto_ptg:
+            LOG.info(_LI('Auto PTG creation configuration set, '
+                         'this will result in automatic creation of a PTG '
+                         'per L2 Policy'))
         self.setup_opflex_rpc_listeners()
         self._ensure_apic_infra()
 
@@ -145,7 +182,7 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
         self.aim_mech_driver.ensure_tenant(plugin_context, tenant_id)
 
     def aim_display_name(self, name):
-        return name
+        return aim_utils.sanitize_display_name(name)
 
     @log.log_method_call
     def create_l3_policy_precommit(self, context):
@@ -352,8 +389,9 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
     @log.log_method_call
     def create_l2_policy_precommit(self, context):
         super(AIMMappingDriver, self).create_l2_policy_precommit(context)
+        l2p = context.current
         net = self._get_network(context._plugin_context,
-                                context.current['network_id'],
+                                l2p['network_id'],
                                 clean_session=False)
         default_epg_dn = net['apic:distinguished_names']['EndpointGroup']
         l2p_count = self._db_plugin(context._plugin).get_l2_policies_count(
@@ -362,21 +400,58 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
             # This is the first l2p for this tenant hence create the Infra
             # Services and Implicit Contracts and setup the default EPG
             self._create_implicit_contracts_and_configure_default_epg(
-                context, context.current, default_epg_dn)
+                context, l2p, default_epg_dn)
         else:
             # Services and Implicit Contracts already exist for this tenant,
             # only setup the default EPG
             self._configure_contracts_for_default_epg(
-                context, context.current, default_epg_dn)
+                context, l2p, default_epg_dn)
+        if self.create_auto_ptg:
+            desc = "System created auto PTG for L2P: %s" % l2p['id']
+            data = {
+                "id": self._get_auto_ptg_id(l2p['id']),
+                "name": self._get_auto_ptg_name(l2p),
+                "description": desc,
+                "l2_policy_id": l2p['id'],
+                "proxied_group_id": None,
+                "proxy_type": None,
+                "proxy_group_id": attributes.ATTR_NOT_SPECIFIED,
+                "network_service_policy_id": None,
+                "service_management": False,
+                "shared": l2p['shared'],
+            }
+            self._create_policy_target_group(
+                context._plugin_context, data, clean_session=False)
 
     @log.log_method_call
     def delete_l2_policy_precommit(self, context):
+        l2p_id = context.current['id']
         l2p_db = context._plugin._get_l2_policy(
-            context._plugin_context, context.current['id'])
+            context._plugin_context, l2p_id)
         net = self._get_network(context._plugin_context,
                                 l2p_db['network_id'],
                                 clean_session=False)
         default_epg_dn = net['apic:distinguished_names']['EndpointGroup']
+        auto_ptg_id = self._get_auto_ptg_id(l2p_id)
+        try:
+            auto_ptg = context._plugin._get_policy_target_group(
+                context._plugin_context, auto_ptg_id)
+            self._process_subnets_for_ptg_delete(
+                context, auto_ptg, l2p_id)
+            if auto_ptg['l2_policy_id']:
+                auto_ptg.update({'l2_policy_id': None})
+            # REVISIT: Consider calling the actual GBP plugin
+            # instead of it's base DB mixin class.
+            self._db_plugin(
+                context._plugin).delete_policy_target_group(
+                    context._plugin_context, auto_ptg['id'])
+        except gpolicy.PolicyTargetGroupNotFound:
+            LOG.info(_LI("Auto PTG with ID %(id)s for "
+                         "for L2P %(l2p)s not found. If create_auto_ptg "
+                         "configuration was not set at the time of the L2P "
+                         "creation, you can safely ignore this, else this "
+                         "could potentially be indication of an error."),
+                     {'id': auto_ptg_id, 'l2p': l2p_id})
         l2p_count = self._db_plugin(context._plugin).get_l2_policies_count(
             context._plugin_context)
         if (l2p_count == 1):
@@ -415,6 +490,12 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
 
     @log.log_method_call
     def create_policy_target_group_precommit(self, context):
+        session = context._plugin_context.session
+
+        if self._is_auto_ptg(context.current):
+            self._use_implicit_subnet(context)
+            return
+
         if context.current['subnets']:
             raise alib.ExplicitSubnetAssociationNotSupported()
 
@@ -436,8 +517,6 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
 
         self._use_implicit_subnet(context)
 
-        session = context._plugin_context.session
-
         bd_name = str(self.name_mapper.network(
             session, net['id'], net['name']))
         bd_tenant_name = str(self._aim_tenant_name(
@@ -461,71 +540,57 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
 
     @log.log_method_call
     def update_policy_target_group_precommit(self, context):
+        self._reject_shared_update(context, 'policy_target_group')
         session = context._plugin_context.session
-        provided_contracts, consumed_contracts = None, None
+        old_provided_contracts = self._get_aim_contract_names(
-        if 'provided_policy_rule_sets' in context.current:
+            session, context.original['provided_policy_rule_sets'])
-            provided_contracts = self._get_aim_contract_names(
+        old_consumed_contracts = self._get_aim_contract_names(
+            session, context.original['consumed_policy_rule_sets'])
+        new_provided_contracts = self._get_aim_contract_names(
             session, context.current['provided_policy_rule_sets'])
-        if 'consumed_policy_rule_sets' in context.current:
+        new_consumed_contracts = self._get_aim_contract_names(
-            consumed_contracts = self._get_aim_contract_names(
             session, context.current['consumed_policy_rule_sets'])
 
         aim_epg = self._get_aim_endpoint_group(session, context.current)
-        if aim_epg and ((provided_contracts is not None) or (
+        if aim_epg:
-            consumed_contracts is not None)):
+            if not self._is_auto_ptg(context.current):
-            if provided_contracts is not None:
+                aim_epg.display_name = (
-                aim_epg.provided_contract_names = provided_contracts
+                    self.aim_display_name(context.current['name']))
-            if consumed_contracts is not None:
+            aim_epg.provided_contract_names = (
-                aim_epg.consumed_contract_names = consumed_contracts
+                list((set(aim_epg.provided_contract_names) -
-            ptg_db = context._plugin._get_policy_target_group(
+                      set(old_provided_contracts)) |
-                context._plugin_context, context.current['id'])
+                     set(new_provided_contracts)))
-            if ptg_db['l2_policy_id']:
+            aim_epg.consumed_contract_names = (
-                l2p_db = context._plugin._get_l2_policy(
+                list((set(aim_epg.consumed_contract_names) -
-                    context._plugin_context, ptg_db['l2_policy_id'])
+                      set(old_consumed_contracts)) |
-                # The following will reset the EPG to only the implicit and
+                     set(new_consumed_contracts)))
-                # infra svc contracts
+
-                self._add_implicit_svc_contracts_to_epg(context, l2p_db,
+            self._add_contracts_for_epg(
-                                                        aim_epg)
+                aim_context.AimContext(session), aim_epg)
 
     @log.log_method_call
     def delete_policy_target_group_precommit(self, context):
         plugin_context = context._plugin_context
+        auto_ptg_id = self._get_auto_ptg_id(context.current['l2_policy_id'])
+        if context.current['id'] == auto_ptg_id:
+            raise AutoPTGDeleteNotSupported(id=context.current['id'])
         ptg_db = context._plugin._get_policy_target_group(
-            context._plugin_context, context.current['id'])
+            plugin_context, context.current['id'])
         session = context._plugin_context.session
 
         aim_ctx = self._get_aim_context(context)
         epg = self._aim_endpoint_group(session, context.current)
         self.aim.delete(aim_ctx, epg)
-        self.name_mapper.delete_apic_name(session, context.current['id'])
+        self._process_subnets_for_ptg_delete(
-
+            context, ptg_db, context.current['l2_policy_id'])
-        subnet_ids = [assoc['subnet_id'] for assoc in ptg_db['subnets']]
-
-        context._plugin._remove_subnets_from_policy_target_group(
-            context._plugin_context, ptg_db['id'])
-        if subnet_ids:
-            for subnet_id in subnet_ids:
-                # clean-up subnet if this is the last PTG using the L2P
-                if not context._plugin._get_ptgs_for_subnet(
-                    context._plugin_context, subnet_id):
-                    l2p_id = context.current['l2_policy_id']
-                    if l2p_id:
-                        l3p = self._get_l3p_for_l2policy(context, l2p_id)
-                        for router_id in l3p['routers']:
-                            self._detach_router_from_subnets(plugin_context,
-                                                             router_id,
-                                                             subnet_ids)
-                    self._cleanup_subnet(plugin_context, subnet_id,
-                                         clean_session=False)
 
         if ptg_db['l2_policy_id']:
             l2p_id = ptg_db['l2_policy_id']
             ptg_db.update({'l2_policy_id': None})
             l2p_db = context._plugin._get_l2_policy(
-                context._plugin_context, l2p_id)
+                plugin_context, l2p_id)
             if not l2p_db['policy_target_groups']:
                 self._cleanup_l2_policy(context, l2p_id, clean_session=False)
-        self.name_mapper.delete_apic_name(session, context.current['id'])
 
     @log.log_method_call
     def extend_policy_target_group_dict(self, session, result):
@@ -783,6 +848,10 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
         for r in routers:
             self._set_router_ext_contracts(context, r, None)
 
+    def _reject_shared_update(self, context, type):
+        if context.original.get('shared') != context.current.get('shared'):
+            raise SharedAttributeUpdateNotSupported(type=type)
+
     def _aim_tenant_name(self, session, tenant_id):
         # TODO(ivar): manage shared objects
         tenant_name = self.name_mapper.tenant(session, tenant_id)
@@ -800,7 +869,8 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
         tenant_name = self._aim_tenant_name(session, tenant_id)
         id = ptg['id']
         name = ptg['name']
-        epg_name = self.name_mapper.policy_target_group(session, id, name)
+        epg_name = self.apic_epg_name_for_policy_target_group(
+            session, id, name)
         display_name = self.aim_display_name(ptg['name'])
         LOG.debug("Mapped ptg_id %(id)s with name %(name)s to %(apic_name)s",
                   {'id': id, 'name': name, 'apic_name': epg_name})
@@ -1311,6 +1381,26 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
         else:
             return gp_const.STATUS_ACTIVE
 
+    def _process_subnets_for_ptg_delete(self, context, ptg, l2p_id):
+        plugin_context = context._plugin_context
+        subnet_ids = [assoc['subnet_id'] for assoc in ptg['subnets']]
+
+        context._plugin._remove_subnets_from_policy_target_group(
+            plugin_context, ptg['id'])
+        if subnet_ids:
+            for subnet_id in subnet_ids:
+                # clean-up subnet if this is the last PTG using the L2P
+                if not context._plugin._get_ptgs_for_subnet(
+                    plugin_context, subnet_id):
+                    if l2p_id:
+                        l3p = self._get_l3p_for_l2policy(context, l2p_id)
+                        for router_id in l3p['routers']:
+                            self._detach_router_from_subnets(plugin_context,
+                                                             router_id,
+                                                             subnet_ids)
+                    self._cleanup_subnet(plugin_context, subnet_id,
+                                         clean_session=False)
+
     def _map_aim_status(self, session, aim_resource_obj):
         # Note that this implementation assumes that this driver
         # is the only policy driver configured, and no merging
@@ -1346,6 +1436,9 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
             return super(gbp_plugin.GroupPolicyPlugin, plugin_obj)
 
     def _get_aim_context(self, context):
+        if hasattr(context, 'session'):
+            session = context.session
+        else:
             session = context._plugin_context.session
         return aim_context.AimContext(session)
 
@@ -1555,9 +1648,9 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
                         if a['active']]
         if active_addrs:
             others = self._get_ports(
-                plugin_context, filters={'network_id': [port['network_id']],
+                plugin_context,
-                                         'fixed_ips':
+                filters={'network_id': [port['network_id']],
-                                         {'ip_address': active_addrs}})
+                         'fixed_ips': {'ip_address': active_addrs}})
             fips_filter.extend([p['id'] for p in others])
         fips = self._get_fips(plugin_context,
                               filters={'port_id': fips_filter})
@@ -1804,3 +1897,37 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
             filters={'id': [r for l in l3ps for r in l['routers']]})
         return [r['id'] for r in routers
             if (r['external_gateway_info'] or {}).get('network_id') in ext_net]
+
+    def _get_auto_ptg_name(self, l2p):
+        return AUTO_PTG_NAME_PREFIX % l2p['id']
+
+    def _get_auto_ptg_id(self, l2p_id):
+        return AUTO_PTG_ID_PREFIX % hashlib.md5(l2p_id).hexdigest()
+
+    def _is_auto_ptg(self, ptg):
+        return ptg['id'].startswith(AUTO_PTG_PREFIX)
+
+    def _get_epg_name_from_dn(self, context, epg_dn):
+        aim_context = self._get_aim_context(context)
+        default_epg_name = self.aim.get(
+            aim_context, aim_resource.EndpointGroup.from_dn(epg_dn)).name
+        return default_epg_name
+
+    def apic_epg_name_for_policy_target_group(self, session, ptg_id,
+                                              name=None):
+        ptg_db = session.query(gpmdb.PolicyTargetGroupMapping).filter_by(
+            id=ptg_id).first()
+        if ptg_db and self._is_auto_ptg(ptg_db):
+            l2p_db = session.query(gpmdb.L2PolicyMapping).filter_by(
+                id=ptg_db['l2_policy_id']).first()
+            network_id = l2p_db['network_id']
+            admin_context = n_context.get_admin_context()
+            admin_context._session = session
+            net = self._get_network(admin_context, network_id,
+                                    clean_session=False)
+            default_epg_dn = net['apic:distinguished_names']['EndpointGroup']
+            default_epg_name = self._get_epg_name_from_dn(
+                admin_context, default_epg_dn)
+            return default_epg_name
+        else:
+            return ptg_id

gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py

@@ -12,6 +12,7 @@
 # limitations under the License.
 
 import copy
+import hashlib
 import mock
 import netaddr
 
@@ -38,6 +39,8 @@ from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import model
 from gbpservice.neutron.services.grouppolicy.common import (
     constants as gp_const)
 from gbpservice.neutron.services.grouppolicy import config
+from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import (
+    aim_mapping as aimd)
 from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import (
     apic_mapping as amap)
 from gbpservice.neutron.services.grouppolicy.drivers.cisco.apic import (
@@ -149,6 +152,15 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
         self.extension_attributes = ('router:external', DN,
                                      'apic:nat_type', 'apic:snat_host_pool',
                                      CIDR, PROV, CONS)
+        # REVISIT: Note that the aim_driver sets create_auto_ptg to
+        # True by default, hence this feature is always ON by default.
+        # However, as a better unit testing strategy, we turn this OFF
+        # for the base case, and turn it ON for select set of tests
+        # which test in addition to what has been already tested in the
+        # base case. It can be evaluated in the future if the base
+        # testing strategy itself should evolve to always test with
+        # the feature turned ON.
+        self.driver.create_auto_ptg = False
 
     def tearDown(self):
         engine = db_api.get_engine()
@@ -340,14 +352,14 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
         self.new_show_request('address-scopes', ascp_id,
                               fmt=self.fmt).get_response(self.ext_api)
 
-    def _get_provided_consumed_prs_lists(self):
+    def _get_provided_consumed_prs_lists(self, shared=False):
         prs_dict = {}
         prs_type = ['provided', 'consumed']
         for ptype in prs_type:
-            rules = self._create_3_direction_rules()
+            rules = self._create_3_direction_rules(shared)
             prs = self.create_policy_rule_set(
-                name="ctr", policy_rules=[x['id'] for x in rules])[
+                name="ctr", shared=shared,
-                    'policy_rule_set']
+                policy_rules=[x['id'] for x in rules])['policy_rule_set']
             prs_dict[ptype] = prs
         return prs_dict
 
@@ -380,6 +392,161 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
         return [self._show(resource_type_plural, res_id)[resource_type]
                 for res_id in ids]
 
+    def _delete_prs_dicts_and_rules(self, prs_dicts):
+        for prs in prs_dicts:
+            prs_id = prs_dicts[prs]['id']
+            rules = prs_dicts[prs]['policy_rules']
+            self.delete_policy_rule_set(prs_id, expected_res_status=204)
+            for rule in rules:
+                self.delete_policy_rule(rule, expected_res_status=204)
+
+    def _validate_contracts(self, ptg, aim_epg, prs_lists, l2p):
+        implicit_contract_name = str(self.name_mapper.policy_rule_set(
+            self._neutron_context.session, l2p['tenant_id'], l2p['tenant_id'],
+            prefix=alib.IMPLICIT_PREFIX))
+        service_contract_name = str(self.name_mapper.policy_rule_set(
+            self._neutron_context.session, l2p['tenant_id'], l2p['tenant_id'],
+            prefix=alib.SERVICE_PREFIX))
+        l3p = self.show_l3_policy(l2p['l3_policy_id'], expected_res_status=200)
+        router_id = l3p['l3_policy']['routers'][0]
+        router = self._get_object('routers', router_id, self.ext_api)['router']
+        router_contract_name = self.name_mapper.router(
+            self._neutron_context.session, router_id, router['name'])
+        expected_prov_contract_names = []
+        expected_cons_contract_names = []
+        if ptg['id'].startswith(aimd.AUTO_PTG_PREFIX):
+            expected_prov_contract_names = [implicit_contract_name,
+                                            service_contract_name,
+                                            router_contract_name]
+            expected_cons_contract_names = [implicit_contract_name,
+                                            router_contract_name]
+        else:
+            expected_cons_contract_names = [implicit_contract_name,
+                                            service_contract_name]
+        if prs_lists['provided']:
+            aim_prov_contract_name = str(self.name_mapper.policy_rule_set(
+                self._neutron_context.session, prs_lists['provided']['id']))
+            expected_prov_contract_names.append(aim_prov_contract_name)
+
+        self.assertItemsEqual(expected_prov_contract_names,
+                              aim_epg.provided_contract_names)
+
+        if prs_lists['consumed']:
+            aim_cons_contract_name = str(self.name_mapper.policy_rule_set(
+                self._neutron_context.session, prs_lists['consumed']['id']))
+            expected_cons_contract_names.append(aim_cons_contract_name)
+
+        self.assertItemsEqual(expected_cons_contract_names,
+                              aim_epg.consumed_contract_names)
+
+    def _validate_router_interface_created(self):
+        # check port is created on default router
+        ports = self._plugin.get_ports(self._context)
+        self.assertEqual(1, len(ports))
+        router_port = ports[0]
+        self.assertEqual('network:router_interface',
+                         router_port['device_owner'])
+        routers = self._l3_plugin.get_routers(self._context)
+        self.assertEqual(1, len(routers))
+        self.assertEqual(routers[0]['id'],
+                         router_port['device_id'])
+        subnets = self._plugin.get_subnets(self._context)
+        self.assertEqual(1, len(subnets))
+        self.assertEqual(1, len(router_port['fixed_ips']))
+        self.assertEqual(subnets[0]['id'],
+                         router_port['fixed_ips'][0]['subnet_id'])
+
+    def _test_policy_target_group_aim_mappings(self, ptg, prs_lists, l2p):
+        self._validate_router_interface_created()
+
+        ptg_id = ptg['id']
+        ptg_show = self.show_policy_target_group(
+            ptg_id, expected_res_status=200)['policy_target_group']
+        aim_epg_name = self.driver.apic_epg_name_for_policy_target_group(
+            self._neutron_context.session, ptg_id)
+        aim_tenant_name = str(self.name_mapper.tenant(
+            self._neutron_context.session, self._tenant_id))
+        aim_app_profile_name = self.driver.aim_mech_driver.ap_name
+        aim_app_profiles = self.aim_mgr.find(
+            self._aim_context, aim_resource.ApplicationProfile,
+            tenant_name=aim_tenant_name, name=aim_app_profile_name)
+        self.assertEqual(1, len(aim_app_profiles))
+        req = self.new_show_request('networks', l2p['network_id'],
+                                    fmt=self.fmt)
+        net = self.deserialize(self.fmt,
+                               req.get_response(self.api))['network']
+        bd = self.aim_mgr.get(
+            self._aim_context, aim_resource.BridgeDomain.from_dn(
+                net['apic:distinguished_names']['BridgeDomain']))
+        aim_epgs = self.aim_mgr.find(
+            self._aim_context, aim_resource.EndpointGroup, name=aim_epg_name)
+        self.assertEqual(1, len(aim_epgs))
+        self.assertEqual(aim_epg_name, aim_epgs[0].name)
+        self.assertEqual(aim_tenant_name, aim_epgs[0].tenant_name)
+        if not ptg['id'].startswith(aimd.AUTO_PTG_PREFIX):
+            # display_name of default EPG should not be mutated
+            # if the name of the auto-ptg is edited, but should
+            # and this should be validated in the auto-ptg tests
+            self.assertEqual(ptg['name'], aim_epgs[0].display_name)
+        self.assertEqual(bd.name, aim_epgs[0].bd_name)
+
+        self._validate_contracts(ptg, aim_epgs[0], prs_lists, l2p)
+
+        self.assertEqual(aim_epgs[0].dn,
+                         ptg['apic:distinguished_names']['EndpointGroup'])
+        self._test_aim_resource_status(aim_epgs[0], ptg)
+        self.assertEqual(aim_epgs[0].dn,
+                         ptg_show['apic:distinguished_names']['EndpointGroup'])
+        self._test_aim_resource_status(aim_epgs[0], ptg_show)
+
+    def _validate_implicit_contracts_deleted(self, l2p):
+        aim_tenant_name = str(self.name_mapper.tenant(
+            self._neutron_context.session, l2p['tenant_id']))
+        contracts = [alib.SERVICE_PREFIX, alib.IMPLICIT_PREFIX]
+
+        for contract_name_prefix in contracts:
+            contract_name = str(self.name_mapper.policy_rule_set(
+                self._neutron_context.session,
+                l2p['tenant_id'], l2p['tenant_id'],
+                prefix=contract_name_prefix))
+            aim_contracts = self.aim_mgr.find(
+                self._aim_context, aim_resource.Contract, name=contract_name)
+            self.assertEqual(0, len(aim_contracts))
+            aim_contract_subjects = self.aim_mgr.find(
+                self._aim_context, aim_resource.ContractSubject,
+                name=contract_name)
+            self.assertEqual(0, len(aim_contract_subjects))
+
+        aim_filters = self.aim_mgr.find(
+            self._aim_context, aim_resource.Filter,
+            tenant_name=aim_tenant_name)
+        self.assertEqual(1, len(aim_filters))  # belongs to MD
+        aim_filter_entries = self.aim_mgr.find(
+            self._aim_context, aim_resource.FilterEntry,
+            tenant_name=aim_tenant_name)
+        self.assertEqual(1, len(aim_filter_entries))  # belongs to MD
+
+    def _validate_l2_policy_deleted(self, l2p):
+        l2p_id = l2p['id']
+        l3p_id = l2p['l3_policy_id']
+        network_id = l2p['network_id']
+        self.delete_l2_policy(l2p_id, expected_res_status=204)
+        self.show_l2_policy(l2p_id, expected_res_status=404)
+        req = self.new_show_request('networks', network_id, fmt=self.fmt)
+        res = req.get_response(self.api)
+        self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
+        l2ps = self._gbp_plugin.get_l2_policies(
+            self._neutron_context)
+        if len(l2ps) == 0:
+            self._validate_implicit_contracts_deleted(l2p)
+            self.show_l3_policy(l3p_id, expected_res_status=404)
+            apic_tenant_name = self.name_mapper.tenant(
+                self._neutron_context.session, self._tenant_id)
+            epgs = self.aim_mgr.find(
+                self._aim_context, aim_resource.EndpointGroup,
+                tenant_name=apic_tenant_name)
+            self.assertEqual(0, len(epgs))
+
 
 class TestGBPStatus(AIMBaseTestCase):
 
@@ -912,33 +1079,6 @@ class TestL2PolicyBase(test_nr_base.TestL2Policy, AIMBaseTestCase):
 
 class TestL2Policy(TestL2PolicyBase):
 
-    def _validate_implicit_contracts_deleted(self, l2p):
-        aim_tenant_name = str(self.name_mapper.tenant(
-            self._neutron_context.session, l2p['tenant_id']))
-        contracts = [alib.SERVICE_PREFIX, alib.IMPLICIT_PREFIX]
-
-        for contract_name_prefix in contracts:
-            contract_name = str(self.name_mapper.policy_rule_set(
-                self._neutron_context.session,
-                l2p['tenant_id'], l2p['tenant_id'],
-                prefix=contract_name_prefix))
-            aim_contracts = self.aim_mgr.find(
-                self._aim_context, aim_resource.Contract, name=contract_name)
-            self.assertEqual(0, len(aim_contracts))
-            aim_contract_subjects = self.aim_mgr.find(
-                self._aim_context, aim_resource.ContractSubject,
-                name=contract_name)
-            self.assertEqual(0, len(aim_contract_subjects))
-
-        aim_filters = self.aim_mgr.find(
-            self._aim_context, aim_resource.Filter,
-            tenant_name=aim_tenant_name)
-        self.assertEqual(1, len(aim_filters))  # belongs to MD
-        aim_filter_entries = self.aim_mgr.find(
-            self._aim_context, aim_resource.FilterEntry,
-            tenant_name=aim_tenant_name)
-        self.assertEqual(1, len(aim_filter_entries))  # belongs to MD
-
     def test_l2_policy_lifecycle(self):
 
         self.assertEqual(0, len(self.aim_mgr.find(
@@ -979,23 +1119,85 @@ class TestL2Policy(TestL2PolicyBase):
             'l2_policy']
         self._validate_implicit_contracts_exist(l2p_tenant2)
         self._switch_to_tenant1()
-
+        self._validate_l2_policy_deleted(l2p)
-        self.delete_l2_policy(l2p_id, expected_res_status=204)
+        self._validate_l2_policy_deleted(l2p0)
-        self.show_l2_policy(l2p_id, expected_res_status=404)
-        req = self.new_show_request('networks', network_id, fmt=self.fmt)
-        res = req.get_response(self.api)
-        self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
-        self.delete_l2_policy(l2p0['id'], expected_res_status=204)
-        self._validate_implicit_contracts_deleted(l2p0)
-        self.show_l3_policy(l3p_id, expected_res_status=404)
         # Validate that the Contracts still exist in the other tenant
         self._switch_to_tenant2()
         self._validate_implicit_contracts_exist(l2p_tenant2)
-        self.delete_l2_policy(l2p_tenant2['id'],
+        self._validate_l2_policy_deleted(l2p_tenant2)
-                              expected_res_status=204)
         self._switch_to_tenant1()
 
 
+class TestL2PolicyWithAutoPTG(TestL2PolicyBase):
+
+    def setUp(self, **kwargs):
+        super(TestL2PolicyWithAutoPTG, self).setUp(**kwargs)
+        self.driver.create_auto_ptg = True
+
+    def _test_auto_ptg(self, l2p, shared=False):
+        ptg = self._gbp_plugin.get_policy_target_groups(
+            self._neutron_context)[0]
+        l2p_id = ptg['l2_policy_id']
+        auto_ptg_id = amap.AUTO_PTG_ID_PREFIX % hashlib.md5(l2p_id).hexdigest()
+        self.assertEqual(auto_ptg_id, ptg['id'])
+        self.assertEqual(aimd.AUTO_PTG_NAME_PREFIX % l2p_id, str(ptg['name']))
+        self.assertEqual(shared, ptg['shared'])
+        prs_lists = self._get_provided_consumed_prs_lists(shared)
+        ptg = self.update_policy_target_group(
+            ptg['id'], expected_res_status=webob.exc.HTTPOk.code,
+            name='new name', description='something-else',
+            provided_policy_rule_sets={prs_lists['provided']['id']:
+                                       'scope'},
+            consumed_policy_rule_sets={prs_lists['consumed']['id']:
+                                       'scope'})['policy_target_group']
+        self._test_policy_target_group_aim_mappings(
+            ptg, prs_lists, l2p)
+        self.update_policy_target_group(
+            ptg['id'], shared=(not shared),
+            expected_res_status=webob.exc.HTTPBadRequest.code)
+        # Auto PTG cannot be deleted by user
+        res = self.delete_policy_target_group(
+            ptg['id'], expected_res_status=webob.exc.HTTPBadRequest.code)
+        self.assertEqual('AutoPTGDeleteNotSupported',
+                         res['NeutronError']['type'])
+        aim_epg_name = self.driver.apic_epg_name_for_policy_target_group(
+            self._neutron_context.session, ptg['id'])
+        aim_epg = self.aim_mgr.find(
+            self._aim_context, aim_resource.EndpointGroup,
+            name=aim_epg_name)[0]
+        aim_epg_display_name = aim_epg.display_name
+        ptg = self.update_policy_target_group(
+            ptg['id'], expected_res_status=webob.exc.HTTPOk.code,
+            name='new name', description='something-else',
+            provided_policy_rule_sets={},
+            consumed_policy_rule_sets={})['policy_target_group']
+        self._test_policy_target_group_aim_mappings(
+            ptg, {'provided': None, 'consumed': None}, l2p)
+        aim_epg = self.aim_mgr.find(
+            self._aim_context, aim_resource.EndpointGroup,
+            name=aim_epg_name)[0]
+        self.assertEqual(aim_epg_display_name, aim_epg.display_name)
+        self._delete_prs_dicts_and_rules(prs_lists)
+        self._validate_l2_policy_deleted(l2p)
+        self.show_policy_target_group(ptg['id'], expected_res_status=404)
+
+    def _test_multiple_l2p_post_create(self, shared=False):
+        l2p = self.create_l2_policy(name="l2p0", shared=shared)['l2_policy']
+        self._test_auto_ptg(l2p, shared=shared)
+        # At this time first l2p and auto-ptg for that l2p are deleted
+        self.create_l2_policy(name="l2p1", shared=shared)['l2_policy']
+        self.create_l2_policy(name="l2p2", shared=shared)['l2_policy']
+        # Two new l2ps are created, and each one should have their own auto-ptg
+        ptgs = self._gbp_plugin.get_policy_target_groups(self._neutron_context)
+        self.assertEqual(2, len(ptgs))
+
+    def test_auto_ptg_lifecycle_shared(self):
+        self._test_multiple_l2p_post_create(shared=True)
+
+    def test_auto_ptg_lifecycle_unshared(self):
+        self._test_multiple_l2p_post_create()
+
+
 class TestL2PolicyRollback(TestL2PolicyBase):
 
     def test_l2_policy_create_fail(self):
@@ -1066,40 +1268,6 @@ class TestL2PolicyRollback(TestL2PolicyBase):
 
 class TestPolicyTargetGroup(AIMBaseTestCase):
 
-    def _validate_contracts(self, aim_epg, prs_lists, l2p):
-        implicit_contract_name = str(self.name_mapper.policy_rule_set(
-            self._neutron_context.session, l2p['tenant_id'], l2p['tenant_id'],
-            prefix=alib.IMPLICIT_PREFIX))
-        service_contract_name = str(self.name_mapper.policy_rule_set(
-            self._neutron_context.session, l2p['tenant_id'], l2p['tenant_id'],
-            prefix=alib.SERVICE_PREFIX))
-        aim_prov_contract_name = str(self.name_mapper.policy_rule_set(
-            self._neutron_context.session, prs_lists['provided']['id']))
-        self.assertEqual([aim_prov_contract_name],
-                         aim_epg.provided_contract_names)
-        aim_cons_contract_name = str(self.name_mapper.policy_rule_set(
-            self._neutron_context.session, prs_lists['consumed']['id']))
-        self.assertItemsEqual([aim_cons_contract_name, service_contract_name,
-                               implicit_contract_name],
-                              aim_epg.consumed_contract_names)
-
-    def _validate_router_interface_created(self):
-        # check port is created on default router
-        ports = self._plugin.get_ports(self._context)
-        self.assertEqual(1, len(ports))
-        router_port = ports[0]
-        self.assertEqual('network:router_interface',
-                         router_port['device_owner'])
-        routers = self._l3_plugin.get_routers(self._context)
-        self.assertEqual(1, len(routers))
-        self.assertEqual(routers[0]['id'],
-                         router_port['device_id'])
-        subnets = self._plugin.get_subnets(self._context)
-        self.assertEqual(1, len(subnets))
-        self.assertEqual(1, len(router_port['fixed_ips']))
-        self.assertEqual(subnets[0]['id'],
-                         router_port['fixed_ips'][0]['subnet_id'])
-
     def test_policy_target_group_aim_domains(self):
         self.aim_mgr.create(self._aim_context,
                             aim_resource.VMMDomain(type='OpenStack',
@@ -1118,8 +1286,8 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         ptg = self.create_policy_target_group(name="ptg1")[
             'policy_target_group']
 
-        aim_epg_name = str(self.name_mapper.policy_target_group(
+        aim_epg_name = self.driver.apic_epg_name_for_policy_target_group(
-            self._neutron_context.session, ptg['id'], ptg['name']))
+            self._neutron_context.session, ptg['id'])
         aim_tenant_name = str(self.name_mapper.tenant(
             self._neutron_context.session, self._tenant_id))
         aim_app_profile_name = self.driver.aim_mech_driver.ap_name
@@ -1144,8 +1312,6 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
             consumed_policy_rule_sets={prs_lists['consumed']['id']: 'scope'})[
                 'policy_target_group']
         ptg_id = ptg['id']
-        ptg_show = self.show_policy_target_group(
-            ptg_id, expected_res_status=200)['policy_target_group']
 
         l2p = self.show_l2_policy(ptg['l2_policy_id'],
                                   expected_res_status=200)['l2_policy']
@@ -1158,58 +1324,18 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         self.assertEqual(l3p['subnetpools_v4'][0],
                          subnet['subnetpool_id'])
 
-        self._validate_router_interface_created()
+        self._test_policy_target_group_aim_mappings(ptg, prs_lists, l2p)
-
-        ptg_name = ptg['name']
-        aim_epg_name = str(self.name_mapper.policy_target_group(
-            self._neutron_context.session, ptg_id, ptg_name))
-        aim_tenant_name = str(self.name_mapper.tenant(
-            self._neutron_context.session, self._tenant_id))
-        aim_app_profile_name = self.driver.aim_mech_driver.ap_name
-        aim_app_profiles = self.aim_mgr.find(
-            self._aim_context, aim_resource.ApplicationProfile,
-            tenant_name=aim_tenant_name, name=aim_app_profile_name)
-        self.assertEqual(1, len(aim_app_profiles))
-        req = self.new_show_request('networks', l2p['network_id'],
-                                    fmt=self.fmt)
-        net = self.deserialize(self.fmt,
-                               req.get_response(self.api))['network']
-        bd = self.aim_mgr.get(
-            self._aim_context, aim_resource.BridgeDomain.from_dn(
-                net['apic:distinguished_names']['BridgeDomain']))
-        aim_epgs = self.aim_mgr.find(
-            self._aim_context, aim_resource.EndpointGroup, name=aim_epg_name)
-        self.assertEqual(1, len(aim_epgs))
-        self.assertEqual(aim_epg_name, aim_epgs[0].name)
-        self.assertEqual(aim_tenant_name, aim_epgs[0].tenant_name)
-        self.assertEqual(ptg['name'], aim_epgs[0].display_name)
-        self.assertEqual(bd.name, aim_epgs[0].bd_name)
-
-        self._validate_contracts(aim_epgs[0], prs_lists, l2p)
-
-        self.assertEqual(aim_epgs[0].dn,
-                         ptg['apic:distinguished_names']['EndpointGroup'])
-        self._test_aim_resource_status(aim_epgs[0], ptg)
-        self.assertEqual(aim_epgs[0].dn,
-                         ptg_show['apic:distinguished_names']['EndpointGroup'])
-        self._test_aim_resource_status(aim_epgs[0], ptg_show)
 
         new_name = 'new name'
         new_prs_lists = self._get_provided_consumed_prs_lists()
-        self.update_policy_target_group(
+        ptg = self.update_policy_target_group(
             ptg_id, expected_res_status=200, name=new_name,
             provided_policy_rule_sets={new_prs_lists['provided']['id']:
                                        'scope'},
             consumed_policy_rule_sets={new_prs_lists['consumed']['id']:
                                        'scope'})['policy_target_group']
-        aim_epg_name = str(self.name_mapper.policy_target_group(
+
-            self._neutron_context.session, ptg_id, new_name))
+        self._test_policy_target_group_aim_mappings(ptg, new_prs_lists, l2p)
-        aim_epgs = self.aim_mgr.find(
-            self._aim_context, aim_resource.EndpointGroup, name=aim_epg_name)
-        self.assertEqual(1, len(aim_epgs))
-        self.assertEqual(aim_epg_name, aim_epgs[0].name)
-        self._validate_contracts(aim_epgs[0], new_prs_lists, l2p)
-        self.assertEqual(bd.name, aim_epgs[0].bd_name)
 
         self.delete_policy_target_group(ptg_id, expected_res_status=204)
         self.show_policy_target_group(ptg_id, expected_res_status=404)
@@ -1223,7 +1349,7 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         self.show_l2_policy(ptg['l2_policy_id'], expected_res_status=404)
 
         aim_epgs = self.aim_mgr.find(
-            self._aim_context, aim_resource.EndpointGroup, name=aim_epg_name)
+            self._aim_context, aim_resource.EndpointGroup)
         self.assertEqual(0, len(aim_epgs))
 
     def test_policy_target_group_lifecycle_explicit_l2p(self):
@@ -1244,8 +1370,8 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         self._validate_router_interface_created()
 
         ptg_name = ptg['name']
-        aim_epg_name = str(self.name_mapper.policy_target_group(
+        aim_epg_name = self.driver.apic_epg_name_for_policy_target_group(
-            self._neutron_context.session, ptg_id, ptg_name))
+            self._neutron_context.session, ptg_id, ptg_name)
         aim_tenant_name = str(self.name_mapper.tenant(
             self._neutron_context.session, self._tenant_id))
         aim_app_profile_name = self.driver.aim_mech_driver.ap_name
@@ -1281,13 +1407,13 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
                                        'scope'},
             consumed_policy_rule_sets={new_prs_lists['consumed']['id']:
                                        'scope'})['policy_target_group']
-        aim_epg_name = str(self.name_mapper.policy_target_group(
+        aim_epg_name = self.driver.apic_epg_name_for_policy_target_group(
-            self._neutron_context.session, ptg_id, new_name))
+            self._neutron_context.session, ptg_id, new_name)
         aim_epgs = self.aim_mgr.find(
             self._aim_context, aim_resource.EndpointGroup, name=aim_epg_name)
         self.assertEqual(1, len(aim_epgs))
         self.assertEqual(aim_epg_name, aim_epgs[0].name)
-        self._validate_contracts(aim_epgs[0], new_prs_lists, l2p)
+        self._validate_contracts(ptg, aim_epgs[0], new_prs_lists, l2p)
         self.assertEqual(bd.name, aim_epgs[0].bd_name)
 
         self.delete_policy_target_group(ptg_id, expected_res_status=204)
@@ -1589,7 +1715,7 @@ class TestPolicyTarget(AIMBaseTestCase):
             nctx.get_admin_context(),
             request={'device': 'tap%s' % pt1['port_id'], 'host': 'h1',
                      'timestamp': 0, 'request_id': 'request_id'})
-        epg_name = self.name_mapper.policy_target_group(
+        epg_name = self.driver.apic_epg_name_for_policy_target_group(
             self._neutron_context.session, ptg['id'], ptg['name'])
         epg_tenant = self.name_mapper.tenant(self._neutron_context.session,
                                              ptg['tenant_id'])