x/group-based-policy
Code Issues Proposed changes

Merge "When a subnet added to a bound SVI port, ensure it is added to SVI port" into stable/queens

Browse Source
This commit is contained in:
Zuul
2020-08-20 21:18:11 +00:00
committed by Gerrit Code Review
parent 1d7e6a23d9 488ed3a7a8
commit e6ac33dac3
2 changed files with 264 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

116
gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py
View File

@@ -2694,6 +2694,36 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
self.delete_port_id_for_ha_ipaddress(
port['id'], ip, session=session)
def _update_svi_ports_for_added_subnet(self, ctx, new_subnets, network_id):
""" Subnet added to bound port on SVI net, add to SVI ports if needed.
We have to ensure that the SVI ports are updated with the added
subnet _before_ we get into updating the static paths to generate
the Interface Profile for the AF of tne new subnet. Doing this
in the context of the BEFORE_UPDATE event guarantees that there
is no race condition.
"""
# Get the SVI ports
filters = {'network_id': [network_id],
'device_owner': [aim_cst.DEVICE_OWNER_SVI_PORT]}
svi_ports = self.plugin.get_ports(ctx, filters)
for svi_port in svi_ports:
subnet_added = False
svi_port_subnets = [s['subnet_id']
for s in svi_port['fixed_ips']]
# update the SVI port if the added subnet is not present.
for s in new_subnets:
if s not in svi_port_subnets:
svi_port['fixed_ips'].append(
{'subnet_id': s})
subnet_added = True
if subnet_added:
port_info = {
'port': {'fixed_ips': svi_port['fixed_ips']}
}
self.plugin.update_port(ctx, svi_port['id'], port_info)
# NOTE: This event is generated by the ML2 plugin in three places,
# all outside of transactions. It is generated from update_port
# and from _update_individual_port_db_status without the
@@ -2703,6 +2733,28 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
def _before_update_port(self, resource, event, trigger, context,
port, original_port,
orig_binding=None, new_binding=None):
if self._is_port_bound(original_port) and 'fixed_ips' in port:
# When a bound port is updated with a subnet, if the port
# is on a SVI network, we need to ensure that the SVI ports
# which are used for creating the Interface Profile for each
# AF also gets updated with the new subnet.
#
# Since we are not in the port binding workflow, can't get the
# bind_context
curr = [s['subnet_id']
for s in port['fixed_ips']]
orig = [s['subnet_id']
for s in original_port['fixed_ips']]
new_subnets = list(set(curr) - set(orig))
if not new_subnets:
return
ctx = nctx.get_admin_context()
net_db = self.plugin._get_network(ctx, original_port['network_id'])
if self._is_svi_db(net_db):
self._update_svi_ports_for_added_subnet(ctx, new_subnets,
original_port['network_id'])
# We are only interested in port update callbacks from
# _commit_port_binding, in which the port is about to become
# bound. Any other callbacks using this event are ignored.
@@ -4791,7 +4843,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
{'port': attrs, 'ex': e})
def _update_static_path_for_svi(self, session, plugin_context, network,
l3out, static_port, remove=False):
l3out, static_port, remove=False,
del_subnet=None):
"""Configure static path for an SVI on a L3 Out
Add, update, or delete a single static path on an SVI. Adds
@@ -4809,7 +4862,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
# so if we're creating new nodes, then we need to allocate
# new IDs for each one.
aim_ctx = aim_context.AimContext(db_session=session)
if not remove and path:
if not remove and not del_subnet and path:
for node_path in node_paths:
# REVISIT: We should check to see if this node is
# already present under the node profile.
@@ -4841,7 +4894,6 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
'mask': str(netaddr.IPNetwork(
subnet['cidr']).prefixlen),
'primary_ips': []}
for node in nodes:
# Get the IP of the SVI port for this node on this network.
#
@@ -4855,6 +4907,8 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
for fixed_ip in svi_ports[0]['fixed_ips']:
ip_vers = netaddr.IPAddress(
fixed_ip['ip_address']).version
if ip_vers not in subnets_dict:
continue
subnets_dict[ip_vers]['primary_ips'].append(
fixed_ip['ip_address'] + '/' + (
@@ -4915,10 +4969,26 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
self.aim.create(aim_ctx, aim_bgp_peer_prefix,
overwrite=True)
if remove:
if remove or del_subnet:
if del_subnet:
# This subnet is not present on this host, Find the AF,
# to map the IProfile to remove.
query = BAKERY(lambda s: s.query(
models_v2.Subnet))
query += lambda q: q.filter(
models_v2.Subnet.id == sa.bindparam('subnet_id'))
subnet = query(session).params(
subnet_id=del_subnet[0]).one_or_none()
if subnet.ip_version == 6:
if_profiles = [L3OUT_IF_PROFILE_NAME6]
else:
if_profiles = [L3OUT_IF_PROFILE_NAME]
else:
# Last port being removed so remove all IProfiles.
if_profiles = [L3OUT_IF_PROFILE_NAME, L3OUT_IF_PROFILE_NAME6]
# REVISIT: Should we also delete the node profiles if there aren't
# any more instances on this host?
for if_profile in [L3OUT_IF_PROFILE_NAME, L3OUT_IF_PROFILE_NAME6]:
for if_profile in if_profiles:
aim_l3out_if = aim_resource.L3OutInterface(
tenant_name=l3out.tenant_name,
l3out_name=l3out.name,
@@ -5292,7 +5362,20 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
LOG.debug('Port %s is not bound to any segment',
port_context.current['id'])
return
if remove:
del_subnet = []
if port_context.current and port_context.original:
# We are in a port update.
# If a subnet is deleted on this bound port, we need
# evaluate if we need to remove the corresponding
# IProfile.
curr = [s['subnet_id']
for s in port_context.current['fixed_ips']]
orig = [s['subnet_id']
for s in port_context.original['fixed_ips']]
del_subnet = list(set(orig) - set(curr))
if remove or del_subnet:
# check if there are any other ports from this network on the host
query = BAKERY(lambda s: s.query(
models.PortBindingLevel))
@@ -5304,11 +5387,26 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
exist = query(session).params(
host=host,
segment_id=segment['id'],
port_id=port_context.current['id']).first()
port_id=port_context.current['id']).all()
if exist:
if remove and exist:
# We are removing a bound port, but there is at least
# one other port on this host so don't remove the
# IProfile.
return
if del_subnet and exist:
# Subnet is being removed from the port, if there is
# at least one other port with the subnet on the host -
# Leave the IProfile for this AF intact.
host_ports = [p['port_id'] for p in exist]
if session.query(models_v2.Port).filter(
models_v2.Port.id.in_(host_ports)).filter(
models_v2.Port.fixed_ips.any(
models_v2.IPAllocation.subnet_id.in_(
del_subnet))).all():
return
static_ports = self._get_static_ports(port_context._plugin_context,
host, segment,
port_context=port_context)
@@ -5319,7 +5417,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
self._update_static_path_for_svi(
session, port_context._plugin_context,
port_context.network.current, l3out,
static_port, remove=remove)
static_port, remove=remove, del_subnet=del_subnet)
else:
self._update_static_path_for_network(
session, port_context.network.current,

157
gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py
View File

@@ -5109,6 +5109,163 @@ class TestPortBinding(ApicAimTestCase):
bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer)
self.assertIsNone(bgp_peer)
def test_dualstack_svi_opflex_agent_add_subnet(self):
aim_ctx = aim_context.AimContext(self.db_session)
hlink1 = aim_infra.HostLink(
host_name='h1',
interface_name='eth1',
path='topology/pod-1/paths-102/pathep-[eth1/8]')
self.aim_mgr.create(aim_ctx, hlink1)
self._register_agent('h1', AGENT_CONF_OPFLEX)
net1 = self._make_network(self.fmt, 'net1', True,
arg_list=self.extension_attributes,
**{'apic:svi': 'True',
'provider:network_type': u'vlan',
'apic:bgp_enable': 'True',
'apic:bgp_asn': '2'})['network']
with self.subnet(network={'network': net1}) as sub1:
with self.port(subnet=sub1) as p1:
# bind p1 to host h1
p1 = self._bind_port_to_host(p1['port']['id'], 'h1')
vlan_p1 = self._check_binding(p1['port']['id'],
expected_binding_info=[('apic_aim', 'vlan')])
ext_net = aim_resource.ExternalNetwork.from_dn(
net1[DN]['ExternalNetwork'])
l3out_np = aim_resource.L3OutNodeProfile(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
name=md.L3OUT_NODE_PROFILE_NAME)
l3out_np = self.aim_mgr.get(aim_ctx, l3out_np)
self.assertEqual(l3out_np.name, md.L3OUT_NODE_PROFILE_NAME)
l3out_node = aim_resource.L3OutNode(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
node_path='topology/pod-1/node-102')
l3out_node = self.aim_mgr.get(aim_ctx, l3out_node)
self.assertIsNotNone(l3out_node)
l3out_ifs = []
bgp_peers = []
for if_profile in [md.L3OUT_IF_PROFILE_NAME,
md.L3OUT_IF_PROFILE_NAME6]:
l3out_ip = aim_resource.L3OutInterfaceProfile(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
name=if_profile)
l3out_ip = self.aim_mgr.get(aim_ctx, l3out_ip)
l3out_if = aim_resource.L3OutInterface(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
interface_profile_name=if_profile,
interface_path=hlink1.path)
l3out_if = self.aim_mgr.get(aim_ctx, l3out_if)
if if_profile == md.L3OUT_IF_PROFILE_NAME:
self.assertEqual(l3out_ip.name, if_profile)
self.assertEqual(l3out_if.encap, 'vlan-%s' % vlan_p1)
self.assertEqual(l3out_if.secondary_addr_a_list,
[{'addr': '10.0.0.1/24'}])
primary = netaddr.IPNetwork(l3out_if.primary_addr_a)
subnet = str(primary.cidr)
bgp_peer = aim_resource.L3OutInterfaceBgpPeerP(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
interface_profile_name=if_profile,
interface_path=hlink1.path,
addr=subnet)
bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer)
self.assertEqual(bgp_peer.asn, '2')
else:
self.assertIsNone(l3out_ip)
self.assertIsNone(l3out_if)
sub2 = self._make_subnet(self.fmt,
{'network': net1}, '2001:db8::1',
'2001:db8::0/64', ip_version=6)['subnet']
p1['port']['fixed_ips'].append({'subnet_id': sub2['id']})
port_info = {'port': {'fixed_ips': p1['port']['fixed_ips']}}
p1 = self._update('ports', p1['port']['id'], port_info)
# The bound port now has the newly created v6 subnet,
# now we should have the v6 Interface Profile
for if_profile in [md.L3OUT_IF_PROFILE_NAME,
md.L3OUT_IF_PROFILE_NAME6]:
l3out_ip = aim_resource.L3OutInterfaceProfile(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
name=if_profile)
l3out_ip = self.aim_mgr.get(aim_ctx, l3out_ip)
self.assertEqual(l3out_ip.name, if_profile)
l3out_if = aim_resource.L3OutInterface(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
interface_profile_name=if_profile,
interface_path=hlink1.path)
l3out_if = self.aim_mgr.get(aim_ctx, l3out_if)
self.assertEqual(l3out_if.encap, 'vlan-%s' % vlan_p1)
if if_profile == md.L3OUT_IF_PROFILE_NAME:
self.assertEqual(l3out_if.secondary_addr_a_list,
[{'addr': '10.0.0.1/24'}])
else:
self.assertEqual(l3out_if.secondary_addr_a_list,
[{'addr': '2001:db8::1/64'}])
l3out_ifs.append(l3out_if)
primary = netaddr.IPNetwork(l3out_if.primary_addr_a)
subnet = str(primary.cidr)
bgp_peer = aim_resource.L3OutInterfaceBgpPeerP(
tenant_name=ext_net.tenant_name,
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
interface_profile_name=if_profile,
interface_path=hlink1.path,
addr=subnet)
bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer)
self.assertEqual(bgp_peer.asn, '2')
bgp_peers.append(bgp_peer)
# Remove the v6 subnet from the port and verify that
# the v6 Profile is deleted.
v4_only = [d for d in p1['port']['fixed_ips']
if not (d['subnet_id'] == sub2['id'])]
port_info = {'port': {'fixed_ips': v4_only}}
p1 = self._update('ports', p1['port']['id'], port_info)
for l3out_if in l3out_ifs:
l3out_if = self.aim_mgr.get(aim_ctx, l3out_if)
if l3out_if and l3out_if.interface_profile_name == (
md.L3OUT_IF_PROFILE_NAME):
self.assertEqual(l3out_if.secondary_addr_a_list,
[{'addr': '10.0.0.1/24'}])
else:
self.assertIsNone(l3out_if)
self._delete('ports', p1['port']['id'])
self._check_no_dynamic_segment(net1['id'])
for l3out_if in l3out_ifs:
l3out_if = self.aim_mgr.get(aim_ctx, l3out_if)
self.assertIsNone(l3out_if)
for bgp_peer in bgp_peers:
bgp_peer = self.aim_mgr.get(aim_ctx, bgp_peer)
self.assertIsNone(bgp_peer)
def test_bind_opflex_agent_svi(self):
self._register_agent('host1', AGENT_CONF_OPFLEX)
aim_ctx = aim_context.AimContext(self.db_session)