From 6b2cd8d967d154fab85dd89b5b8cd6a89f08330e Mon Sep 17 00:00:00 2001 From: ajgoyalnoiro Date: Sun, 13 Oct 2019 03:09:13 -0700 Subject: [PATCH] Update_proj_descr in apic when project description is updated in os update mechanism driver to save project description on project creation and project update into aim database. Change-Id: Ibecd5bbad286f8f77b381171ca5a64ff7f4fc7ad --- .../plugins/ml2plus/drivers/apic_aim/cache.py | 50 ++++++---- .../drivers/apic_aim/mechanism_driver.py | 36 +++---- .../drivers/cisco/apic/aim_validation.py | 2 +- .../unit/plugins/ml2plus/test_apic_aim.py | 95 ++++++++++++++++--- 4 files changed, 135 insertions(+), 48 deletions(-) diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/cache.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/cache.py index cb8a541ca..2db81102d 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/cache.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/cache.py @@ -34,11 +34,11 @@ ksc_session.Session.register_conf_options(cfg.CONF, AUTH_GROUP) ksc_auth.register_conf_options(cfg.CONF, AUTH_GROUP) -class ProjectNameCache(object): - """Cache of Keystone project ID to project name mappings.""" +class ProjectDetailsCache(object): + """Cache of Keystone project ID to project details mappings.""" def __init__(self): - self.project_names = {} + self.project_details = {} self.keystone = None self.gbp = None self.enable_neutronclient_internal_ep_interface = ( @@ -78,7 +78,7 @@ class ProjectNameCache(object): inside a transaction with a project_id not already in the cache. """ - if project_id and project_id not in self.project_names: + if project_id and project_id not in self.project_details: self.load_projects() def load_projects(self): @@ -88,29 +88,45 @@ class ProjectNameCache(object): projects = self.keystone.projects.list() LOG.debug("Received projects: %s", projects) for project in projects: - self.project_names[project.id] = project.name + self.project_details[project.id] = (project.name, + project.description) - def get_project_name(self, project_id): - """Get name of project from cache. + def get_project_details(self, project_id): + """Get name and descr of project from cache. :param project_id: ID of the project - Get the name of the project identified by project_id from the - cache. If the cache contains project_id, the project's name is - returned. If not, None is returned. + If the cache contains project_id, a tuple with + project name and description is returned + else a tuple (None,None) is returned """ - return self.project_names.get(project_id) + if self.project_details.get(project_id): + return self.project_details[project_id] + return ('', '') - def update_project_name(self, project_id): + def update_project_details(self, project_id): + """Update project name and description from keystone. + + :param project_id: ID of the project + + Get the name and description of the project identified by + project_id from the keystone. If the value in cache doesn't + match values in keystone, update the cache and return 1, + to indicate that cache has been updated + """ if self.keystone is None: self._get_keystone_client() if self.keystone: - LOG.debug("Calling project API") project = self.keystone.projects.get(project_id) - # only return project name when there is a change - if project and self.project_names.get(project_id) != project.name: - self.project_names[project.id] = project.name - return project.name + if project: + prj_details = self.get_project_details(project_id) + if (prj_details[0] != project.name or + prj_details[1] != project.description): + self.project_details[project_id] = ( + project.name, project.description) + LOG.debug("Project updated %s " % ( + str(self.project_details[project_id]))) + return self.project_details[project_id] return None def purge_gbp(self, project_id): diff --git a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py index 19de2beb7..5491fe4c5 100644 --- a/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py +++ b/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py @@ -156,10 +156,11 @@ class KeystoneNotificationEndpoint(object): "tenant %(tenant_id)s", {'event_type': event_type, 'tenant_id': tenant_id}) + if event_type == 'identity.project.updated': - new_project_name = (self._driver.project_name_cache. - update_project_name(tenant_id)) - if not new_project_name: + prj_details = (self._driver.project_details_cache. + update_project_details(tenant_id)) + if not prj_details: return None # we only update tenants which have been created in APIC. For other @@ -172,8 +173,9 @@ class KeystoneNotificationEndpoint(object): if not self._driver.aim.get(aim_ctx, tenant): return None - self._driver.aim.update(aim_ctx, tenant, - display_name=aim_utils.sanitize_display_name(new_project_name)) + disp_name = aim_utils.sanitize_display_name(prj_details[0]) + self._driver.aim.update(aim_ctx, tenant, display_name + = disp_name, descr=prj_details[1]) return oslo_messaging.NotificationResult.HANDLED if event_type == 'identity.project.deleted': @@ -184,7 +186,7 @@ class KeystoneNotificationEndpoint(object): # of the gbp/neutron purge on the server side instead of # using gbp/neutron client API to do it which is not so # efficient. - self._driver.project_name_cache.purge_gbp(tenant_id) + self._driver.project_details_cache.purge_gbp(tenant_id) # delete the tenant and AP in AIM also session = db_api.get_session() @@ -207,7 +209,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, def initialize(self): LOG.info(_LI("APIC AIM MD initializing")) - self.project_name_cache = cache.ProjectNameCache() + self.project_details_cache = cache.ProjectDetailsCache() self.name_mapper = apic_mapper.APICNameMapper() self.aim = aim_manager.AimManager() self._core_plugin = None @@ -535,7 +537,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver, # mapping AIM resources under some actual Tenant. return - self.project_name_cache.ensure_project(project_id) + self.project_details_cache.ensure_project(project_id) # TODO(rkukura): Move the following to calls made from # precommit methods so AIM Tenants, ApplicationProfiles, and @@ -543,13 +545,12 @@ class ApicMechanismDriver(api_plus.MechanismDriver, session = plugin_context.session with session.begin(subtransactions=True): tenant_aname = self.name_mapper.project(session, project_id) - project_name = self.project_name_cache.get_project_name(project_id) - if project_name is None: - project_name = '' + project_details = (self.project_details_cache. + get_project_details(project_id)) aim_ctx = aim_context.AimContext(session) tenant = aim_resource.Tenant( - name=tenant_aname, descr=self.apic_system_id, - display_name=aim_utils.sanitize_display_name(project_name)) + name=tenant_aname, descr=project_details[1], display_name= + aim_utils.sanitize_display_name(project_details[0])) # NOTE(ivar): by overwriting the existing tenant, we make sure # existing deployments will update their description value. This # however negates any change to the Tenant object done by direct @@ -5475,10 +5476,11 @@ class ApicMechanismDriver(api_plus.MechanismDriver, mgr.expected_session, project_id) tenant = aim_resource.Tenant(name=tenant_name) - project_name = ( - self.project_name_cache.get_project_name(project_id) or '') - tenant.display_name = aim_utils.sanitize_display_name(project_name) - tenant.descr = self.apic_system_id + project_details = self.project_details_cache.get_project_details( + project_id) + tenant.display_name = aim_utils.sanitize_display_name( + project_details[0]) + tenant.descr = project_details[1] tenant.monitored = False mgr.expect_aim_resource(tenant) diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py index 1ff77facc..993f84436 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py @@ -63,7 +63,7 @@ class ValidationManager(object): # REVISIT: Validate configuration. # Load project names from Keystone. - self.md.project_name_cache.load_projects() + self.md.project_details_cache.load_projects() # Start transaction. # diff --git a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py index 3a5567d80..bc8b91d2a 100644 --- a/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py +++ b/gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py @@ -157,23 +157,45 @@ TEST_TENANT_NAMES = { # REVISIT(rkukura): Use mock for this instead? -class FakeTenant(object): - def __init__(self, id, name): +class FakeProject(object): + def __init__(self, id, name, description=''): self.id = id self.name = name + self.description = description class FakeProjectManager(object): + _instance = None + + def __init__(self): + self._projects = {k: FakeProject(k, v) + for k, v in TEST_TENANT_NAMES.items()} + def list(self): - return [FakeTenant(k, v) for k, v in six.iteritems(TEST_TENANT_NAMES)] + return self._projects.values() def get(self, project_id): - return FakeTenant('test-tenant', 'new_name') + return self._projects.get(project_id) + + @classmethod + def reset(cls): + cls._instance = None + + @classmethod + def get_instance(cls): + if not cls._instance: + cls._instance = FakeProjectManager() + return cls._instance + + @classmethod + def set(cls, project_id, name, description=''): + cls.get_instance()._projects[project_id] = FakeProject( + project_id, name, description) class FakeKeystoneClient(object): def __init__(self, **kwargs): - self.projects = FakeProjectManager() + self.projects = FakeProjectManager.get_instance() class AimSqlFixture(fixtures.Fixture): @@ -284,6 +306,7 @@ class ApicAimTestCase(test_address_scope.AddressScopeTestCase, self.validation_mgr = av.ValidationManager() + FakeProjectManager.reset() self.saved_keystone_client = ksc_client.Client ksc_client.Client = FakeKeystoneClient self.plugin = manager.NeutronManager.get_plugin() @@ -1978,22 +2001,68 @@ class TestAimMapping(ApicAimTestCase): self.driver.aim.get = mock.Mock(return_value=True) self.driver.aim.update = mock.Mock() self.driver.aim.delete = mock.Mock() - self.driver.project_name_cache.purge_gbp = mock.Mock() + self.driver.project_details_cache.purge_gbp = mock.Mock() payload = {} - payload['resource_info'] = 'test-tenant' + payload['resource_info'] = 'test-tenant-update' keystone_ep = md.KeystoneNotificationEndpoint(self.driver) - # first test with project.updated event + tenant_name = self.name_mapper.project(None, 'test-tenant-update') + tenant = aim_resource.Tenant(name=tenant_name) + + # Test project.updated event. Update both name and description. + FakeProjectManager.set('test-tenant-update', + 'new-tenant', 'new-descr') keystone_ep.info(None, None, 'identity.project.updated', payload, None) + assert(self.driver.aim.update.call_args_list[0] == mock.call( + mock.ANY, tenant, display_name='new-tenant', descr = 'new-descr')) + + # Test project.updated event. Update only the project name. + FakeProjectManager.set('test-tenant-update', 'name123', 'new-descr') + keystone_ep.info(None, None, 'identity.project.updated', payload, None) + assert(self.driver.aim.update.call_args_list[1] == mock.call( + mock.ANY, tenant, display_name='name123', descr = 'new-descr')) + + # Test project.updated event. Update only the project description. + FakeProjectManager.set('test-tenant-update', 'name123', 'descr123') + keystone_ep.info(None, None, 'identity.project.updated', payload, None) + assert(self.driver.aim.update.call_args_list[2] == mock.call( + mock.ANY, tenant, display_name='name123', descr='descr123')) + + # Test project.updated event. Clear the project description. + FakeProjectManager.set('test-tenant-update', 'name123', '') + keystone_ep.info(None, None, 'identity.project.updated', payload, None) + assert(self.driver.aim.update.call_args_list[3] == mock.call( + mock.ANY, tenant, display_name='name123', descr='')) + + # Test project.updated event. Update project name and description. + FakeProjectManager.set('test-tenant-update', 'prj1', 'prj2') + keystone_ep.info(None, None, 'identity.project.updated', payload, None) + assert(self.driver.aim.update.call_args_list[4] == mock.call( + mock.ANY, tenant, display_name='prj1', descr='prj2')) + + # Test project.updated event. Add new tenant. + FakeProjectManager.set('test-tenant-new', 'add-tenant', 'add-descr') + self.driver.project_details_cache.project_details[ + 'test-tenant-new'] = ['new-tenant', 'new-descr'] + tenant_name = self.name_mapper.project(None, 'test-tenant-new') + tenant = aim_resource.Tenant(name=tenant_name) + payload['resource_info'] = 'test-tenant-new' + keystone_ep.info(None, None, 'identity.project.updated', payload, None) + assert(self.driver.aim.update.call_args_list[5] == mock.call( + mock.ANY, tenant, display_name='add-tenant', descr='add-descr')) + + # Test project.updated event. No change in name or description. + payload['resource_info'] = 'test-tenant-new' + keystone_ep.info(None, None, 'identity.project.updated', payload, None) + assert(len(self.driver.aim.update.call_args_list) == 6) + + # Test with project.deleted event. + payload['resource_info'] = 'test-tenant' tenant_name = self.name_mapper.project(None, 'test-tenant') tenant = aim_resource.Tenant(name=tenant_name) - self.driver.aim.update.assert_called_once_with( - mock.ANY, tenant, display_name='new_name') - - # test again with project.deleted event self.driver.enable_keystone_notification_purge = True keystone_ep.info(None, None, 'identity.project.deleted', payload, None) - self.driver.project_name_cache.purge_gbp.assert_called_once_with( + self.driver.project_details_cache.purge_gbp.assert_called_once_with( 'test-tenant') tenant = aim_resource.Tenant(name=tenant_name) exp_calls = [