[AIM] Eliminate name mapping DB

AIM resource names are now Neutron and GBP resource UUIDs, with an
optional prefix.

This is the first step of phasing out the name mapper. Once an
appropriate function, similar to aim.utils.sanitize_display_name, is
available from AIM, calls to the name mapper should be replaced with
calls to that function. Once this is complete, the name mapper module
and class will be removed.

Change-Id: I6b4b180f8722e4378e3afee7d62a71292eea233d
(cherry picked from commit 9e06d9d506)

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

@@ -13,12 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import re
+from neutron.common import exceptions
-
-from neutron._i18n import _LI
-
-LOG = None
-
 
 NAME_TYPE_TENANT = 'tenant'
 NAME_TYPE_NETWORK = 'network'
@@ -33,112 +28,33 @@ NAME_TYPE_EXTERNAL_SEGMENT = 'external_segment'
 NAME_TYPE_EXTERNAL_POLICY = 'external_policy'
 NAME_TYPE_NAT_POOL = 'nat_pool'
 
-MAX_APIC_NAME_LENGTH = 46
+
+class InvalidResourceId(exceptions.BadRequest):
+    message = _("The %(type)s ID '%(id)s' is invalid.")
 
 
-# TODO(rkukura): This is name mapper is copied from the apicapi repo,
+# TODO(rkukura): This module and class are being phased out. Uses of
-# and modified to pass in resource names rather than calling the core
+# this class should be replaced with calls to a function, similar to
-# plugin to get them, and to use the existing DB session. We need
+# aim.utils.sanitize_display_name, that does any concatenation,
-# decide whether to make these changes in apicapi (maybe on a branch),
+# character or substring translation, or truncation is needed to
-# move this some other repo, or keep it here. The changes are not
+# ensure the supplied string (and optional prefix) form a valid APIC
-# backwards compatible. The implementation should also be cleaned up
+# name. One this phase out is complete, this module will be deleted.
-# and simplified. For example, sessions should be passed in place of
-# contexts, and the core plugin calls eliminated.
-
-
-def truncate(string, max_length):
-    if max_length < 0:
-        return ''
-    return string[:max_length] if len(string) > max_length else string
-
 
 class APICNameMapper(object):
-    def __init__(self, db, log):
-        self.db = db
-        self.min_suffix = 5
-        global LOG
-        LOG = log.getLogger(__name__)
-
     def mapper(name_type):
         """Wrapper to land all the common operations between mappers."""
         def wrap(func):
             def inner(inst, session, resource_id, resource_name=None,
                       prefix=None):
-                # REVISIT(Bob): Optional argument for reserving characters in
+                if not resource_id:
-                # the prefix?
+                    raise InvalidResourceId(type=name_type, id=resource_id)
-                saved_name = inst.db.get_apic_name(session,
+                result = resource_id
-                                                   resource_id,
-                                                   name_type)
-                if saved_name:
-                    result = saved_name[0]
-                    # REVISIT(Sumit): Should this name mapper be aware of
-                    # this prefixing logic, or should we instead prepend
-                    # the prefix at the point from where this is being
-                    # invoked. The latter approach has the disadvantage
-                    # of having to replicate the logic in many places.
                 if prefix:
                     result = prefix + result
-                        result = truncate(result, MAX_APIC_NAME_LENGTH)
-                    return result
-                name = ''
-                try:
-                    name = func(inst, session, resource_id, resource_name)
-                except Exception as e:
-                    LOG.warn(("Exception in looking up name %s"), name_type)
-                    LOG.error(e.message)
-
-                purged_id = re.sub(r"-+", "-", resource_id)
-                result = purged_id[:inst.min_suffix]
-                if name:
-                    name = re.sub(r"-+", "-", name)
-                    # Keep as many uuid chars as possible
-                    id_suffix = "_" + result
-                    max_name_length = MAX_APIC_NAME_LENGTH - len(id_suffix)
-                    result = truncate(name, max_name_length) + id_suffix
-
-                    result = truncate(result, MAX_APIC_NAME_LENGTH)
-                    # Remove forbidden whitespaces
-                    result = result.replace(' ', '')
-                    result = inst._grow_id_if_needed(
-                        session, purged_id, name_type, result,
-                        start=inst.min_suffix)
-                else:
-                    result = purged_id
-
-                inst.db.add_apic_name(session, resource_id,
-                                      name_type, result)
-                if prefix:
-                    result = prefix + result
-                    result = truncate(result, MAX_APIC_NAME_LENGTH)
                 return result
             return inner
         return wrap
 
-    def _grow_id_if_needed(self, session, resource_id, name_type,
-                           current_result, start=0):
-        result = current_result
-        if result.endswith('_'):
-            result = result[:-1]
-        try:
-            x = 0
-            while True:
-                if self.db.get_filtered_apic_names(session,
-                                                   neutron_type=name_type,
-                                                   apic_name=result):
-                    if x == 0 and start == 0:
-                        result += '_'
-                    # This name overlaps, add more ID characters
-                    result += resource_id[start + x]
-                    x += 1
-                else:
-                    break
-        except AttributeError:
-            LOG.info(_LI("Current DB API doesn't support "
-                         "get_filtered_apic_names."))
-        except IndexError:
-            LOG.debug("Ran out of ID characters.")
-        return result
-
     @mapper(NAME_TYPE_TENANT)
     def tenant(self, session, tenant_id, tenant_name=None):
         return tenant_name
@@ -195,4 +111,6 @@ class APICNameMapper(object):
         return nat_pool['name']
 
     def delete_apic_name(self, session, object_id):
-        self.db.delete_apic_name(session, object_id)
+        # This method is kept for compatibility with existing code
+        # until the name mapper is phased out entirely.
+        pass

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

@@ -52,6 +52,9 @@ class ProjectNameCache(object):
         cache.
         """
 
+        if not project_id:
+            return
+
         # TODO(rkukura): It seems load_from_conf_options() and
         # keystoneclient auth plugins have been deprecated, and we
         # should use keystoneauth instead.

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

@@ -46,7 +46,6 @@ from gbpservice.neutron.plugins.ml2plus import driver_api as api_plus
 from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import apic_mapper
 from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import cache
 from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import extension_db
-from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import model
 from oslo_serialization.jsonutils import netaddr
 
 LOG = log.getLogger(__name__)
@@ -99,8 +98,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver):
     def initialize(self):
         LOG.info(_LI("APIC AIM MD initializing"))
         self.project_name_cache = cache.ProjectNameCache()
-        self.db = model.DbModel()
+        self.name_mapper = apic_mapper.APICNameMapper()
-        self.name_mapper = apic_mapper.APICNameMapper(self.db, log)
         self.aim = aim_manager.AimManager()
         self._core_plugin = None
         self._l3_plugin = None
@@ -119,6 +117,15 @@ class ApicMechanismDriver(api_plus.MechanismDriver):
     def ensure_tenant(self, plugin_context, tenant_id):
         LOG.debug("APIC AIM MD ensuring tenant_id: %s", tenant_id)
 
+        if not tenant_id:
+            # The l3_db module creates gateway ports with empty string
+            # project IDs in order to hide those ports from
+            # users. Since we are not currently mapping ports to
+            # anything in AIM, we can ignore these. Any other cases
+            # where empty string project IDs are used may require
+            # mapping AIM resources under some actual Tenant.
+            return
+
         self.project_name_cache.ensure_project(tenant_id)
 
         # TODO(rkukura): Move the following to calls made from

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

@@ -1,63 +0,0 @@
-# Copyright (c) 2016 Cisco Systems Inc.
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-from apic_ml2.neutron.plugins.ml2.drivers.cisco.apic import (
-    apic_model as old_model)
-from neutron._i18n import _LI
-from oslo_log import log
-from sqlalchemy import orm
-
-LOG = log.getLogger(__name__)
-
-
-# REVISIT(rkukura): Temporarily using ApicName model defined in old
-# apic-ml2 driver with migration in neutron. We should define our
-# own, and may want to switch to per-resource name mapping tables with
-# foriegn keys.
-
-class DbModel(object):
-
-    def __init__(self):
-        LOG.info(_LI("APIC AIM DbModel __init__"))
-
-    def add_apic_name(self, session, neutron_id, neutron_type, apic_name):
-        name = old_model.ApicName(neutron_id=neutron_id,
-                                  neutron_type=neutron_type,
-                                  apic_name=apic_name)
-        with session.begin(subtransactions=True):
-            session.add(name)
-
-    def get_apic_name(self, session, neutron_id, neutron_type):
-        return session.query(old_model.ApicName.apic_name).filter_by(
-            neutron_id=neutron_id, neutron_type=neutron_type).first()
-
-    def delete_apic_name(self, session, neutron_id):
-        with session.begin(subtransactions=True):
-            try:
-                session.query(old_model.ApicName).filter_by(
-                    neutron_id=neutron_id).delete()
-            except orm.exc.NoResultFound:
-                return
-
-    def get_filtered_apic_names(self, session, neutron_id=None,
-                                neutron_type=None, apic_name=None):
-        query = session.query(old_model.ApicName.apic_name)
-        if neutron_id:
-            query = query.filter_by(neutron_id=neutron_id)
-        if neutron_type:
-            query = query.filter_by(neutron_type=neutron_type)
-        if apic_name:
-            query = query.filter_by(apic_name=apic_name)
-        return query.all()

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

@@ -34,7 +34,6 @@ 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
 from gbpservice.neutron.extensions import group_policy as gpolicy
-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.common import constants as g_const
@@ -127,7 +126,6 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
     @log.log_method_call
     def initialize(self):
         LOG.info(_LI("APIC AIM Policy Driver initializing"))
-        self.db = model.DbModel()
         super(AIMMappingDriver, self).initialize()
         self._apic_aim_mech_driver = None
         self._apic_segmentation_label_driver = None

gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py

@@ -173,8 +173,8 @@ class ApicAimTestCase(test_address_scope.AddressScopeTestCase,
         super(ApicAimTestCase, self).tearDown()
 
     def _map_name(self, resource):
-        # Assumes no conflicts and no substition needed.
+        # TODO(rkukura): Eliminate.
-        return resource['name'][:40] + '_' + resource['id'][:5]
+        return resource['id']
 
     def _find_by_dn(self, dn, cls):
         aim_ctx = aim_context.AimContext(self.db_session)
@@ -1923,8 +1923,8 @@ class TestExternalConnectivityBase(object):
         contract = self._map_name(router)
         a_ext_net1 = aim_resource.ExternalNetwork(
             tenant_name='t1', l3out_name='l1', name='n1',
-            provided_contract_names=['pr-1', contract],
+            provided_contract_names=sorted(['pr-1', contract]),
-            consumed_contract_names=['co-1', contract])
+            consumed_contract_names=sorted(['co-1', contract]))
         a_vrf = aim_resource.VRF(tenant_name=self._tenant_name,
                                  name='DefaultVRF')
         if use_addr_scope:
@@ -1938,8 +1938,8 @@ class TestExternalConnectivityBase(object):
                                                  ext_net2['id']}}})
         a_ext_net2 = aim_resource.ExternalNetwork(
             tenant_name='t1', l3out_name='l2', name='n2',
-            provided_contract_names=['pr-1', contract],
+            provided_contract_names=sorted(['pr-1', contract]),
-            consumed_contract_names=['co-1', contract])
+            consumed_contract_names=sorted(['co-1', contract]))
         a_ext_net1.provided_contract_names = []
         a_ext_net1.consumed_contract_names = []
         dv.assert_called_once_with(mock.ANY, a_ext_net1, a_vrf)

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

@@ -35,7 +35,6 @@ from oslo_utils import uuidutils
 import webob.exc
 
 from gbpservice.network.neutronv2 import local_api
-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
@@ -148,7 +147,6 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
         vm.name = 'someid'
         nova_client.return_value = vm
 
-        self._db = model.DbModel()
         self.extension_attributes = ('router:external', DN,
                                      'apic:nat_type', 'apic:snat_host_pool',
                                      CIDR, PROV, CONS)