From c3ab03e8e0a46969b381fc935ca785bb3b7f921e Mon Sep 17 00:00:00 2001 From: Kent Wu Date: Thu, 17 Nov 2016 16:30:22 -0800 Subject: [PATCH] [apic-mapping] Updating nameAlias field also. This patch will cover VRF, BD, EPG and contract. Don't need to cover l3out and external EPG as they are all preexisting in customer env. Tenant and AP will be covered in a separate patch as currently we don't get tenant name change notification in our MD...We don't plan to cover policy rule/classifier/action though based on the discussion with Mandeep. Partial-Bug: 1642783 Change-Id: Id5f675db22d2d886e38c205913f982f54d8a1f61 (cherry picked from commit 10140e85bf97514e158116103776c55ae431a4cb) --- .../drivers/cisco/apic/apic_mapping.py | 59 +++++++++++++++- .../services/grouppolicy/test_apic_mapping.py | 69 +++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) 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 13609e399..60279f86b 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py @@ -839,6 +839,9 @@ class ApicMappingDriver(api.ResourceMappingDriver, {'id': context.current['policy_rules']}) self._apply_policy_rule_set_rules( context, context.current, rules, transaction=trs) + self.apic_manager.update_name_alias( + self.apic_manager.apic.vzBrCP, tenant, contract, + nameAlias=context.current['name']) def create_policy_target_precommit(self, context): ptg = self._get_policy_target_group( @@ -929,6 +932,10 @@ class ApicMappingDriver(api.ResourceMappingDriver, context, context.current, l2p, epg, transaction=trs) self._configure_epg_implicit_contract( context, context.current, l2p, epg, transaction=trs) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvAEPg, tenant, + self.apic_manager.app_profile_name, + epg, nameAlias=context.current['name']) l3p = context._plugin.get_l3_policy( context._plugin_context, l2_policy_object['l3_policy_id']) @@ -1034,9 +1041,29 @@ class ApicMappingDriver(api.ResourceMappingDriver, context._plugin_context, es['name']) self.apic_manager.set_l3out_for_bd(tenant, l2_policy, es_name, transaction=trs) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvBD, tenant, l2_policy, + nameAlias=context.current['name']) + shadow_epg = self.name_mapper.l2_policy( + context, context.current, prefix=SHADOW_PREFIX) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvAEPg, tenant, + self.apic_manager.app_profile_name, shadow_epg, + nameAlias=self._get_shadow_name(context.current['name'])) def update_l2_policy_postcommit(self, context): - pass + if context.original['name'] != context.current['name']: + tenant = self._tenant_by_sharing_policy(context.current) + l2_policy = self.name_mapper.l2_policy(context, context.current) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvBD, tenant, l2_policy, + nameAlias=context.current['name']) + shadow_epg = self.name_mapper.l2_policy( + context, context.current, prefix=SHADOW_PREFIX) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvAEPg, tenant, + self.apic_manager.app_profile_name, shadow_epg, + nameAlias=self._get_shadow_name(context.current['name'])) def create_l3_policy_precommit(self, context): if not self.name_mapper._is_apic_reference(context.current): @@ -1049,6 +1076,9 @@ class ApicMappingDriver(api.ResourceMappingDriver, tenant = self._tenant_by_sharing_policy(context.current) l3_policy = self.name_mapper.l3_policy(context, context.current) self.apic_manager.ensure_context_enforced(tenant, l3_policy) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvCtx, tenant, l3_policy, + nameAlias=context.current['name']) external_segments = context.current['external_segments'] if external_segments: # Create a L3 ext for each External Segment @@ -1211,6 +1241,14 @@ class ApicMappingDriver(api.ResourceMappingDriver, raise alib.HierarchicalContractsNotSupported() def update_policy_rule_set_postcommit(self, context): + if context.original['name'] != context.current['name']: + tenant = self._tenant_by_sharing_policy(context.current) + contract = self.name_mapper.policy_rule_set(context, + context.current) + self.apic_manager.update_name_alias( + self.apic_manager.apic.vzBrCP, tenant, contract, + nameAlias=context.current['name']) + # Update policy_rule_set rules old_rules = set(context.original['policy_rules']) new_rules = set(context.current['policy_rules']) @@ -1282,6 +1320,15 @@ class ApicMappingDriver(api.ResourceMappingDriver, def update_policy_target_group_postcommit(self, context): if not self.name_mapper._is_apic_reference(context.current): + if context.original['name'] != context.current['name']: + tenant = self._tenant_by_sharing_policy(context.current) + epg = self.name_mapper.policy_target_group(context, + context.current) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvAEPg, tenant, + self.apic_manager.app_profile_name, + epg, nameAlias=context.current['name']) + # TODO(ivar): refactor parent to avoid code duplication orig_provided_policy_rule_sets = context.original[ 'provided_policy_rule_sets'] @@ -1345,6 +1392,13 @@ class ApicMappingDriver(api.ResourceMappingDriver, def update_l3_policy_postcommit(self, context): if not self.name_mapper._is_apic_reference(context.current): + if context.original['name'] != context.current['name']: + tenant = self._tenant_by_sharing_policy(context.current) + l3_policy = self.name_mapper.l3_policy(context, + context.current) + self.apic_manager.update_name_alias( + self.apic_manager.apic.fvCtx, tenant, l3_policy, + nameAlias=context.current['name']) old_segment_dict = context.original['external_segments'] new_segment_dict = context.current['external_segments'] if (context.current['external_segments'] != @@ -2557,6 +2611,9 @@ class ApicMappingDriver(api.ResourceMappingDriver, str(self.name_mapper.l3_policy(context, l3_obj)))) or '') + def _get_shadow_name(self, name): + return "%s%s" % (SHADOW_PREFIX, name) + def _get_tenant_for_shadow(self, is_shadow, shadow_obj, obj): return self._tenant_by_sharing_policy( shadow_obj if is_shadow else obj) 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 746b8b805..c78c8699c 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py @@ -1842,6 +1842,28 @@ class TestPolicyTargetGroup(ApicMappingTestCase): transaction=mock.ANY)] self._check_call_list( expected_calls, mgr.ensure_epg_created.call_args_list) + expected_calls = [ + mock.call(mgr.apic.fvCtx, tenant, mock.ANY, + nameAlias='default'), + mock.call(mgr.apic.fvBD, tenant, ptg['l2_policy_id'], + nameAlias='ptg1'), + mock.call(mgr.apic.fvAEPg, tenant, mgr.app_profile_name, + amap.SHADOW_PREFIX + ptg['l2_policy_id'], + nameAlias=amap.SHADOW_PREFIX + 'ptg1'), + mock.call(mgr.apic.fvAEPg, tenant, mgr.app_profile_name, + ptg['id'], nameAlias='ptg1')] + self._check_call_list(expected_calls, + mgr.update_name_alias.call_args_list) + + mgr.reset_mock() + self.update_policy_target_group( + ptg['id'], description='desc', expected_res_status=200) + self.assertFalse(mgr.update_name_alias.called) + self.update_policy_target_group( + ptg['id'], name='ptg1_1', expected_res_status=200) + mgr.update_name_alias.assert_called_once_with( + mgr.apic.fvAEPg, tenant, mgr.app_profile_name, + ptg['id'], nameAlias='ptg1_1') def test_policy_target_group_created_on_apic(self): self._test_policy_target_group_created_on_apic() @@ -2252,6 +2274,31 @@ class TestL2PolicyBase(ApicMappingTestCase): mgr.ensure_epg_created.assert_called_once_with( tenant, amap.SHADOW_PREFIX + l2p['id'], bd_owner=tenant, bd_name=l2p['id'], transaction=mock.ANY) + expected_calls = [ + mock.call(mgr.apic.fvCtx, tenant, l2p['l3_policy_id'], + nameAlias='default'), + mock.call(mgr.apic.fvBD, tenant, l2p['id'], + nameAlias='l2p'), + mock.call(mgr.apic.fvAEPg, tenant, mgr.app_profile_name, + amap.SHADOW_PREFIX + l2p['id'], + nameAlias=amap.SHADOW_PREFIX + 'l2p')] + self._check_call_list(expected_calls, + mgr.update_name_alias.call_args_list) + + mgr.reset_mock() + self.update_l2_policy( + l2p['id'], description='desc', expected_res_status=200) + self.assertFalse(mgr.update_name_alias.called) + self.update_l2_policy( + l2p['id'], name='l2p_1', expected_res_status=200) + expected_calls = [ + mock.call(mgr.apic.fvBD, tenant, l2p['id'], + nameAlias='l2p_1'), + mock.call(mgr.apic.fvAEPg, tenant, mgr.app_profile_name, + amap.SHADOW_PREFIX + l2p['id'], + nameAlias=amap.SHADOW_PREFIX + 'l2p_1')] + self._check_call_list(expected_calls, + mgr.update_name_alias.call_args_list) def _test_l2_policy_deleted_on_apic(self, shared=False): l2p = self.create_l2_policy(name="l2p", shared=shared)['l2_policy'] @@ -2484,6 +2531,17 @@ class TestL3Policy(ApicMappingTestCase): mgr = self.driver.apic_manager mgr.ensure_context_enforced.assert_called_once_with( tenant, l3p['id']) + mgr.update_name_alias.assert_called_once_with( + mgr.apic.fvCtx, tenant, l3p['id'], nameAlias='l3p') + + mgr.reset_mock() + self.update_l3_policy( + l3p['id'], description='desc', expected_res_status=200) + self.assertFalse(mgr.update_name_alias.called) + self.update_l3_policy( + l3p['id'], name='l3p_1', expected_res_status=200) + mgr.update_name_alias.assert_called_once_with( + mgr.apic.fvCtx, tenant, l3p['id'], nameAlias='l3p_1') def test_l3_policy_created_on_apic(self): self._test_l3_policy_created_on_apic() @@ -3752,6 +3810,17 @@ class TestPolicyRuleSet(ApicMappingTestCase): mgr = self.driver.apic_manager mgr.create_contract.assert_called_once_with( ct['id'], owner=tenant, transaction=mock.ANY) + mgr.update_name_alias.assert_called_once_with( + mgr.apic.vzBrCP, tenant, ct['id'], nameAlias='ctr') + + mgr.reset_mock() + self.update_policy_rule_set( + ct['id'], description='desc', expected_res_status=200) + self.assertFalse(mgr.update_name_alias.called) + self.update_policy_rule_set( + ct['id'], name='ctr_1', expected_res_status=200) + mgr.update_name_alias.assert_called_once_with( + mgr.apic.vzBrCP, tenant, ct['id'], nameAlias='ctr_1') def test_policy_rule_set_created_on_apic(self): self._test_policy_rule_set_created_on_apic()