x/group-based-policy
Code Issues Proposed changes

Merge "[APIC mapping] Associate BDs to L3Outs even when NAT is disabled"

Browse Source
This commit is contained in:
Jenkins 2016-06-17 20:09:31 +00:00 committed by Gerrit Code Review
commit aab00f8688
2 changed files with 137 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py
View File

@ -905,7 +905,8 @@ class ApicMappingDriver(api.ResourceMappingDriver,
context._plugin_context, context.current['id'], context._plugin_context, context.current['id'],
subnets, [], transaction=trs) subnets, [], transaction=trs)
# query all the ESs under this vrf # query all the ESs for this VRF, connect BD to corresponding
# (shadow) L3Outs for no-NAT and edge-NAT cases.
if l3_policy_object['external_segments']: if l3_policy_object['external_segments']:
ess = context._plugin.get_external_segments( ess = context._plugin.get_external_segments(
context._plugin_context, context._plugin_context,
@ -914,12 +915,19 @@ class ApicMappingDriver(api.ResourceMappingDriver,
for es in ess: for es in ess:
ext_info = self.apic_manager.ext_net_dict.get( ext_info = self.apic_manager.ext_net_dict.get(
es['name']) es['name'])
if (ext_info and self._is_edge_nat(ext_info) and nat_enabled = self._is_nat_enabled_on_es(es)
self._is_nat_enabled_on_es(es)): is_edge_nat = (nat_enabled and ext_info and
self._is_edge_nat(ext_info))
if not nat_enabled or is_edge_nat:
es_name = self.name_mapper.external_segment( es_name = self.name_mapper.external_segment(
context, es, context, es,
prefix=self._get_shadow_prefix(context, prefix=self._get_shadow_prefix(context,
True, l3_policy_object, is_edge_nat=True)) is_edge_nat, l3_policy_object,
is_edge_nat=is_edge_nat))
if not is_edge_nat and self._is_pre_existing(es):
nm = self.name_mapper.name_mapper
es_name = nm.pre_existing(
context._plugin_context, es['name'])
self.apic_manager.set_l3out_for_bd(tenant, self.apic_manager.set_l3out_for_bd(tenant,
l2_policy, es_name, transaction=trs) l2_policy, es_name, transaction=trs)
@ -1866,6 +1874,8 @@ class ApicMappingDriver(api.ResourceMappingDriver,
es_name = self.name_mapper.external_segment(context, es, es_name = self.name_mapper.external_segment(context, es,
prefix=self._get_shadow_prefix(context, prefix=self._get_shadow_prefix(context,
is_shadow, context.current, self._is_edge_nat(ext_info))) is_shadow, context.current, self._is_edge_nat(ext_info)))
es_name_pre = self.name_mapper.name_mapper.pre_existing(
context._plugin_context, es['name'])
es_tenant = self._get_tenant_for_shadow(is_shadow, context.current, es) es_tenant = self._get_tenant_for_shadow(is_shadow, context.current, es)
nat_enabled = self._is_nat_enabled_on_es(es) nat_enabled = self._is_nat_enabled_on_es(es)
pre_existing = False if is_shadow else self._is_pre_existing(es) pre_existing = False if is_shadow else self._is_pre_existing(es)
@ -1888,14 +1898,12 @@ class ApicMappingDriver(api.ResourceMappingDriver,
transaction=trs) transaction=trs)
# Associate pre-existing, no-NAT L3-out with L3policy # Associate pre-existing, no-NAT L3-out with L3policy
if pre_existing and not nat_enabled: if pre_existing and not nat_enabled:
mapped_es = self.name_mapper.name_mapper.pre_existing( l3out_info = self._query_l3out_info(es_name_pre,
context._plugin_context, es['name'])
l3out_info = self._query_l3out_info(mapped_es,
self.name_mapper.tenant(es)) self.name_mapper.tenant(es))
if l3out_info: if l3out_info:
mapped_tenant = l3out_info['l3out_tenant'] mapped_tenant = l3out_info['l3out_tenant']
self.apic_manager.set_context_for_external_routed_network( self.apic_manager.set_context_for_external_routed_network(
mapped_tenant, mapped_es, l3_policy, transaction=trs) mapped_tenant, es_name_pre, l3_policy, transaction=trs)
is_details_needed = False is_details_needed = False
if not is_shadow and not pre_existing: if not is_shadow and not pre_existing:
@ -1947,19 +1955,29 @@ class ApicMappingDriver(api.ResourceMappingDriver,
(self.name_mapper.name_mapper.pre_existing( (self.name_mapper.name_mapper.pre_existing(
context, es['name']) if pre_existing else es_name), context, es['name']) if pre_existing else es_name),
transaction=trs) transaction=trs)
if nat_enabled and not is_shadow: if not is_shadow:
# create shadow external-networks if nat_enabled:
self._plug_l3p_to_es(context, es, True) # create shadow external-networks
# create shadow external EPGs indirectly by re-plugging self._plug_l3p_to_es(context, es, True)
# external policies to external segment # create shadow external EPGs indirectly by re-plugging
eps = context._plugin.get_external_policies( # external policies to external segment
context._plugin_context, eps = context._plugin.get_external_policies(
filters={'id': es['external_policies'], context._plugin_context,
'tenant_id': [context.current['tenant_id']]}) filters={'id': es['external_policies'],
for ep in eps: 'tenant_id': [context.current['tenant_id']]})
self._plug_external_policy_to_segment(context, ep, for ep in eps:
[es['id']], ep['provided_policy_rule_sets'], self._plug_external_policy_to_segment(context, ep,
ep['consumed_policy_rule_sets']) [es['id']], ep['provided_policy_rule_sets'],
ep['consumed_policy_rule_sets'])
else:
# Associate BDs of the VRF to L3-out
l2ps = self._get_l2_policies(context._plugin_context,
{'id': context.current['l2_policies']})
for l2p in l2ps:
self.apic_manager.set_l3out_for_bd(
self._tenant_by_sharing_policy(l2p),
self.name_mapper.l2_policy(context, l2p),
es_name_pre if pre_existing else es_name)
def _unplug_l3p_from_es(self, context, es, is_shadow=False): def _unplug_l3p_from_es(self, context, es, is_shadow=False):
is_edge_nat = False is_edge_nat = False
@ -1969,12 +1987,24 @@ class ApicMappingDriver(api.ResourceMappingDriver,
es_name = self.name_mapper.external_segment(context, es, es_name = self.name_mapper.external_segment(context, es,
prefix=self._get_shadow_prefix(context, prefix=self._get_shadow_prefix(context,
is_shadow, context.current, is_edge_nat)) is_shadow, context.current, is_edge_nat))
es_name_pre = self.name_mapper.name_mapper.pre_existing(
context._plugin_context, es['name'])
es_tenant = self._get_tenant_for_shadow(is_shadow, context.current, es) es_tenant = self._get_tenant_for_shadow(is_shadow, context.current, es)
nat_enabled = self._is_nat_enabled_on_es(es) nat_enabled = self._is_nat_enabled_on_es(es)
pre_existing = False if is_shadow else self._is_pre_existing(es) pre_existing = False if is_shadow else self._is_pre_existing(es)
# remove shadow external-networks if not is_shadow:
if nat_enabled and not is_shadow: if nat_enabled:
self._unplug_l3p_from_es(context, es, True) # remove shadow external-networks
self._unplug_l3p_from_es(context, es, True)
else:
# Dissociate BDs of the VRF from L3-out
l2ps = self._get_l2_policies(context._plugin_context,
{'id': context.current['l2_policies']})
for l2p in l2ps:
self.apic_manager.unset_l3out_for_bd(
self._tenant_by_sharing_policy(l2p),
self.name_mapper.l2_policy(context, l2p),
es_name_pre if pre_existing else es_name)
set_ctx = self.apic_manager.set_context_for_external_routed_network set_ctx = self.apic_manager.set_context_for_external_routed_network
if (is_shadow or if (is_shadow or
not [x for x in es['l3_policies'] if x != context.current['id']]): not [x for x in es['l3_policies'] if x != context.current['id']]):
@ -1985,13 +2015,11 @@ class ApicMappingDriver(api.ResourceMappingDriver,
# Dissociate L3policy from pre-existing, no-NAT L3-out # Dissociate L3policy from pre-existing, no-NAT L3-out
if pre_existing and not nat_enabled: if pre_existing and not nat_enabled:
mapped_es = self.name_mapper.name_mapper.pre_existing( l3out_info = self._query_l3out_info(es_name_pre,
context._plugin_context, es['name'])
l3out_info = self._query_l3out_info(mapped_es,
self.name_mapper.tenant(es)) self.name_mapper.tenant(es))
if l3out_info: if l3out_info:
mapped_tenant = l3out_info['l3out_tenant'] mapped_tenant = l3out_info['l3out_tenant']
set_ctx(mapped_tenant, mapped_es, None, set_ctx(mapped_tenant, es_name_pre, None,
transaction=trs) transaction=trs)
if nat_enabled and not is_shadow: if nat_enabled and not is_shadow:

123
gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py
View File

@ -2165,9 +2165,11 @@ class TestL3Policy(ApicMappingTestCase):
self.assertEqual(1, len(l3p['external_segments'][es['id']])) self.assertEqual(1, len(l3p['external_segments'][es['id']]))
self.assertEqual('169.254.0.2', l3p['external_segments'][es['id']][0]) self.assertEqual('169.254.0.2', l3p['external_segments'][es['id']][0])
if is_edge_nat: l2ps = [self.create_l2_policy(name='myl2p-%s' % x,
l2p = self.create_l2_policy(name='myl2p', tenant_id=l3p['tenant_id'],
l3_policy_id=l3p['id'])['l2_policy'] shared=shared_l3p,
l3_policy_id=l3p['id'])['l2_policy']
for x in range(0, 3)]
owner = self.common_tenant if shared_es else es['tenant_id'] owner = self.common_tenant if shared_es else es['tenant_id']
l3p_owner = self.common_tenant if shared_l3p else l3p['tenant_id'] l3p_owner = self.common_tenant if shared_l3p else l3p['tenant_id']
@ -2259,18 +2261,24 @@ class TestL3Policy(ApicMappingTestCase):
self.assertFalse(mgr.ensure_logical_node_profile_created.called) self.assertFalse(mgr.ensure_logical_node_profile_created.called)
self.assertFalse(mgr.ensure_static_route_created.called) self.assertFalse(mgr.ensure_static_route_created.called)
expected_set_l3out_for_bd_calls = []
if self.nat_enabled: if self.nat_enabled:
expected_set_l3out_for_bd_calls = [ expected_set_l3out_for_bd_calls.append(
mock.call(owner, "NAT-bd-%s" % es['id'], mock.call(owner, "NAT-bd-%s" % es['id'],
es['name' if self.pre_l3out else 'id'], es['name' if self.pre_l3out else 'id'],
transaction=mock.ANY)] transaction=mock.ANY))
if is_edge_nat: if is_edge_nat:
expected_set_l3out_for_bd_calls.append( expected_set_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'], mock.call(l3p_owner, l2p['id'],
l3out_str % (l3p['id'], es['id']), l3out_str % (l3p['id'], es['id']),
transaction=mock.ANY)) transaction=mock.ANY) for l2p in l2ps])
self._check_call_list(expected_set_l3out_for_bd_calls, else:
mgr.set_l3out_for_bd.call_args_list) expected_set_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'],
es['name' if self.pre_l3out else 'id'],
transaction=mock.ANY) for l2p in l2ps])
self._check_call_list(expected_set_l3out_for_bd_calls,
mgr.set_l3out_for_bd.call_args_list)
# Although the naming convention used here has been chosen poorly, # Although the naming convention used here has been chosen poorly,
# I'm separating the tests in order to get the mock re-set. # I'm separating the tests in order to get the mock re-set.
@ -2304,20 +2312,24 @@ class TestL3Policy(ApicMappingTestCase):
{'destination': '128.0.0.0/16', {'destination': '128.0.0.0/16',
'nexthop': None}])['external_segment'] 'nexthop': None}])['external_segment']
# Create with explicit address
l3p = self.create_l3_policy( l3p = self.create_l3_policy(
name='myl3p', name='myl3p',
expected_res_status=201, expected_res_status=201,
tenant_id=es['tenant_id'] if not shared_es else 'another_tenant', tenant_id=es['tenant_id'] if not shared_es else 'another_tenant',
shared=shared_l3p, shared=shared_l3p)['l3_policy']
external_segments={es['id']: []})['l3_policy'] l2ps = [self.create_l2_policy(name='myl2p-%s' % x,
tenant_id=l3p['tenant_id'],
shared=shared_l3p,
l3_policy_id=l3p['id'])['l2_policy']
for x in range(0, 3)]
# update L3P with ES
l3p = self.update_l3_policy(l3p['id'], tenant_id=l3p['tenant_id'],
external_segments={es['id']: []},
expected_res_status=200)['l3_policy']
self.assertEqual(1, len(l3p['external_segments'][es['id']])) self.assertEqual(1, len(l3p['external_segments'][es['id']]))
self.assertEqual('169.254.0.2', l3p['external_segments'][es['id']][0]) self.assertEqual('169.254.0.2', l3p['external_segments'][es['id']][0])
if is_edge_nat:
l2p = self.create_l2_policy(name='myl2p',
l3_policy_id=l3p['id'])['l2_policy']
mgr = self.driver.apic_manager mgr = self.driver.apic_manager
owner = self.common_tenant if shared_es else es['tenant_id'] owner = self.common_tenant if shared_es else es['tenant_id']
l3p_owner = self.common_tenant if shared_l3p else l3p['tenant_id'] l3p_owner = self.common_tenant if shared_l3p else l3p['tenant_id']
@ -2409,18 +2421,24 @@ class TestL3Policy(ApicMappingTestCase):
self.assertFalse(mgr.ensure_logical_node_profile_created.called) self.assertFalse(mgr.ensure_logical_node_profile_created.called)
self.assertFalse(mgr.ensure_static_route_created.called) self.assertFalse(mgr.ensure_static_route_created.called)
expected_set_l3out_for_bd_calls = []
if self.nat_enabled: if self.nat_enabled:
expected_set_l3out_for_bd_calls = [ expected_set_l3out_for_bd_calls.append(
mock.call(owner, "NAT-bd-%s" % es['id'], mock.call(owner, "NAT-bd-%s" % es['id'],
es['name' if self.pre_l3out else 'id'], es['name' if self.pre_l3out else 'id'],
transaction=mock.ANY)] transaction=mock.ANY))
if is_edge_nat: if is_edge_nat:
expected_set_l3out_for_bd_calls.append( expected_set_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'], mock.call(l3p_owner, l2p['id'],
l3out_str % (l3p['id'], es['id']), l3out_str % (l3p['id'], es['id']),
transaction=mock.ANY)) transaction=mock.ANY) for l2p in l2ps])
self._check_call_list(expected_set_l3out_for_bd_calls, else:
mgr.set_l3out_for_bd.call_args_list) expected_set_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'],
es['name' if self.pre_l3out else 'id'])
for l2p in l2ps])
self._check_call_list(expected_set_l3out_for_bd_calls,
mgr.set_l3out_for_bd.call_args_list)
# Although the naming convention used here has been chosen poorly, # Although the naming convention used here has been chosen poorly,
# I'm separating the tests in order to get the mock re-set. # I'm separating the tests in order to get the mock re-set.
@ -2597,9 +2615,11 @@ class TestL3Policy(ApicMappingTestCase):
external_segments={es1['id']: ['169.254.0.3']}, external_segments={es1['id']: ['169.254.0.3']},
expected_res_status=201)['l3_policy'] expected_res_status=201)['l3_policy']
if is_edge_nat: l2ps = [self.create_l2_policy(name='myl2p-%s' % x,
l2p = self.create_l2_policy(name='myl2p', tenant_id=l3p['tenant_id'],
l3_policy_id=l3p['id'])['l2_policy'] shared=shared_l3p,
l3_policy_id=l3p['id'])['l2_policy']
for x in range(0, 3)]
mgr = self.driver.apic_manager mgr = self.driver.apic_manager
owner = self.common_tenant if shared_es else es1['tenant_id'] owner = self.common_tenant if shared_es else es1['tenant_id']
@ -2640,19 +2660,24 @@ class TestL3Policy(ApicMappingTestCase):
expected_calls, expected_calls,
mgr.set_context_for_external_routed_network.call_args_list) mgr.set_context_for_external_routed_network.call_args_list)
expected_unset_l3out_for_bd_calls = []
if self.nat_enabled: if self.nat_enabled:
expected_unset_l3out_for_bd_calls = []
if is_edge_nat: if is_edge_nat:
expected_unset_l3out_for_bd_calls.append( expected_unset_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'], mock.call(l3p_owner, l2p['id'],
l3out_str % (l3p['id'], es1['id']), l3out_str % (l3p['id'], es1['id']),
transaction=mock.ANY)) transaction=mock.ANY) for l2p in l2ps])
expected_unset_l3out_for_bd_calls.append( expected_unset_l3out_for_bd_calls.append(
mock.call(owner, "NAT-bd-%s" % es1['id'], mock.call(owner, "NAT-bd-%s" % es1['id'],
es1['name' if self.pre_l3out else 'id'], es1['name' if self.pre_l3out else 'id'],
transaction=mock.ANY)) transaction=mock.ANY))
self._check_call_list(expected_unset_l3out_for_bd_calls, else:
mgr.unset_l3out_for_bd.call_args_list) expected_unset_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'],
es1['name' if self.pre_l3out else 'id'])
for l2p in l2ps])
self._check_call_list(expected_unset_l3out_for_bd_calls,
mgr.unset_l3out_for_bd.call_args_list)
if is_edge_nat and self.nat_enabled: if is_edge_nat and self.nat_enabled:
self.driver.l3out_vlan_alloc.release_vlan.assert_called_once_with( self.driver.l3out_vlan_alloc.release_vlan.assert_called_once_with(
@ -2718,19 +2743,24 @@ class TestL3Policy(ApicMappingTestCase):
self.assertFalse(mgr.ensure_static_route_created.called) self.assertFalse(mgr.ensure_static_route_created.called)
expected_set_l3out_for_bd_calls = []
if self.nat_enabled: if self.nat_enabled:
expected_set_l3out_for_bd_calls = []
expected_set_l3out_for_bd_calls.append( expected_set_l3out_for_bd_calls.append(
mock.call(owner, "NAT-bd-%s" % es2['id'], mock.call(owner, "NAT-bd-%s" % es2['id'],
es2['name' if self.pre_l3out else 'id'], es2['name' if self.pre_l3out else 'id'],
transaction=mock.ANY)) transaction=mock.ANY))
if is_edge_nat: if is_edge_nat:
expected_set_l3out_for_bd_calls.append( expected_set_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'], mock.call(l3p_owner, l2p['id'],
l3out_str % (l3p['id'], es2['id']), l3out_str % (l3p['id'], es2['id']),
transaction=mock.ANY)) transaction=mock.ANY) for l2p in l2ps])
self._check_call_list(expected_set_l3out_for_bd_calls, else:
mgr.set_l3out_for_bd.call_args_list) expected_set_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'],
es2['name' if self.pre_l3out else 'id'])
for l2p in l2ps])
self._check_call_list(expected_set_l3out_for_bd_calls,
mgr.set_l3out_for_bd.call_args_list)
self.driver.l3out_vlan_alloc.release_vlan.reset_mock() self.driver.l3out_vlan_alloc.release_vlan.reset_mock()
mgr.delete_external_routed_network.reset_mock() mgr.delete_external_routed_network.reset_mock()
@ -2757,17 +2787,17 @@ class TestL3Policy(ApicMappingTestCase):
self._check_call_list( self._check_call_list(
expected_delete_calls, expected_delete_calls,
mgr.delete_external_routed_network.call_args_list) mgr.delete_external_routed_network.call_args_list)
expected_unset_l3out_for_bd_calls = []
if self.nat_enabled: if self.nat_enabled:
expected_unset_l3out_for_bd_calls = []
if is_edge_nat: if is_edge_nat:
expected_unset_l3out_for_bd_calls.append( expected_unset_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'], mock.call(l3p_owner, l2p['id'],
l3out_str % (l3p['id'], es1['id']), l3out_str % (l3p['id'], es1['id']),
transaction=mock.ANY)) transaction=mock.ANY) for l2p in l2ps])
expected_unset_l3out_for_bd_calls.append( expected_unset_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'], mock.call(l3p_owner, l2p['id'],
l3out_str % (l3p['id'], es2['id']), l3out_str % (l3p['id'], es2['id']),
transaction=mock.ANY)) transaction=mock.ANY) for l2p in l2ps])
expected_unset_l3out_for_bd_calls.append( expected_unset_l3out_for_bd_calls.append(
mock.call(owner, "NAT-bd-%s" % es1['id'], mock.call(owner, "NAT-bd-%s" % es1['id'],
es1['name' if self.pre_l3out else 'id'], es1['name' if self.pre_l3out else 'id'],
@ -2776,8 +2806,17 @@ class TestL3Policy(ApicMappingTestCase):
mock.call(owner, "NAT-bd-%s" % es2['id'], mock.call(owner, "NAT-bd-%s" % es2['id'],
es2['name' if self.pre_l3out else 'id'], es2['name' if self.pre_l3out else 'id'],
transaction=mock.ANY)) transaction=mock.ANY))
self._check_call_list(expected_unset_l3out_for_bd_calls, else:
mgr.unset_l3out_for_bd.call_args_list) expected_unset_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'],
es1['name' if self.pre_l3out else 'id'])
for l2p in l2ps])
expected_unset_l3out_for_bd_calls.extend([
mock.call(l3p_owner, l2p['id'],
es2['name' if self.pre_l3out else 'id'])
for l2p in l2ps])
self._check_call_list(expected_unset_l3out_for_bd_calls,
mgr.unset_l3out_for_bd.call_args_list)
if self.pre_l3out and not self.nat_enabled: if self.pre_l3out and not self.nat_enabled:
expected_calls = [ expected_calls = [