Fix for unbinding baremetal VNIC ports

Unbinding baremetal VNIC ports before deleting them results in
removing the topology information from the binding:profile. This
prevents the static port from being deleted from the EPG when the
port is deleted. The topology information still exists in the port
context passed to the mechanism driver, under the "original" member.
This patch handles port unbinding by getting the topology infomration
from the original member, in order to identify the static port
to remove from the EPG.

Change-Id: I6a0e31a771cfc678d1f3bdaa576751f05f2c173a
This commit is contained in:
Thomas Bachman 2020-03-09 23:00:34 +00:00 committed by Thomas Bachman
parent a90aa7a3b8
commit 18344dd4be
2 changed files with 47 additions and 32 deletions

View File

@ -2792,7 +2792,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
the port to vlan type segments, and as the physical_network
when creating dynamic vlan type segments for HPB.
"""
if not self._is_baremetal_port_bindable(context.current):
if not self._is_baremetal_port_bindable(context):
return False
_, _, physnet, _ = self._get_baremetal_topology(context.current)
# First attempt binding vlan type segments, in order to avoid
@ -4322,13 +4322,14 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
epg, epg.static_paths)
self.aim.update(aim_ctx, epg, static_paths=epg.static_paths)
def _get_static_ports(self, plugin_context, host, segment, port=None):
def _get_static_ports(self, plugin_context, host, segment,
port_context=None):
"""Get StaticPort objects for ACI.
:param plugin_context : plugin context
:param host : host ID for the static port
:param segment : bound segment of this host
:param port : port instance
:param port_context : port context instance
:returns: List of zero or more static port objects
This method should be called when a neutron port requires
@ -4346,14 +4347,23 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
encap = self._segment_to_vlan_encap(segment)
if not encap:
return []
if port and self._is_baremetal_vnic_type(port) and (
self._is_baremetal_port_bindable(port)):
if port_context and self._is_baremetal_vnic_type(port_context.current):
# Check if there's any topology information available
topology = self._get_baremetal_topology(port_context.current)
if not any(topology):
topology = self._get_baremetal_topology(port_context.original)
if not any(topology):
LOG.warning("Invalid topology: port %(port)s does not "
"contain required topology information in the "
"binding:profile's local_link_information "
"array.", {'port': port_context.current['id']})
return []
# The local_link_information should be populated, and
# will have the static path.
sp, ifs, pn, pd = self._get_baremetal_topology(port)
static_path, _, _, _ = topology
hlink = aim_infra.HostLink(host_name='',
interface_name='', path=sp)
interface_name='', path=static_path)
return [StaticPort(hlink, encap, 'untagged')]
else:
# If it's not baremetal, return qualifying entries from the
@ -4367,18 +4377,18 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
self._filter_host_links_by_segment(session,
segment, host_links)]
def _is_baremetal_port_bindable(self, port):
def _is_baremetal_port_bindable(self, port_context):
"""Verify that a port is a valid baremetal port.
:param port : Port instance
:param port_context : Port context instance
:returns: True if bindable baremetal vnic, False otherwise
The binding is valid for a baremetal port which has valid topology
information in the local_link_information list contained inside the
binding:profile.
"""
if self._is_baremetal_vnic_type(port):
if any(self._get_baremetal_topology(port)):
if self._is_baremetal_vnic_type(port_context.current):
if any(self._get_baremetal_topology(port_context.current)):
return True
return False
@ -4488,10 +4498,6 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
# We at least need the static path and physical_network
if not static_path or not physical_network:
LOG.warning("Invalid topology: port %(port)s does not contain "
"required topology information in the "
"binding:profile's local_link_information array.",
{'port': port['id']})
return (None, None, None, None)
return (static_path, interfaces, physical_network, physical_domain)
@ -4525,7 +4531,7 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
static_ports = self._get_static_ports(port_context._plugin_context,
host, segment,
port=port_context.current)
port_context=port_context)
for static_port in static_ports:
if self._is_svi(port_context.network.current):
l3out, _, _ = self._get_aim_external_objects(

View File

@ -8882,6 +8882,12 @@ class TestPortOnPhysicalNode(TestPortVlanNetwork):
self.assertEqual(set(['ph1', 'ph2']),
set(self._doms(epg1.physical_domains,
with_type=False)))
# Unbind the port and verify that the right static path is removed
kwargs = {portbindings.PROFILE: {}}
self._bind_port_to_host(p1['id'], None, **kwargs)['port']
epg1 = self.aim_mgr.get(aim_ctx, epg1)
self.assertNotIn(static_path_1, epg1.static_paths)
self.assertIn(static_path_2, epg1.static_paths)
def test_no_host_domain_mappings(self):
aim_ctx = aim_context.AimContext(self.db_session)
@ -9289,8 +9295,8 @@ class TestPortOnPhysicalNodeSingleDriver(TestPortOnPhysicalNode):
def validate_static_path_and_doms(aim_ctx, is_svi, net, kv_dict,
physical_domain, vlan, delete=False):
static_path = {'path': kv_dict['apic_dn'],
'encap': 'vlan-%s' % vlan, 'mode': 'untagged'}
static_path = [{'path': kv_dict['apic_dn'],
'encap': 'vlan-%s' % vlan, 'mode': 'untagged'}]
if is_svi:
ext_net = aim_resource.ExternalNetwork.from_dn(
net[DN]['ExternalNetwork'])
@ -9307,7 +9313,7 @@ class TestPortOnPhysicalNodeSingleDriver(TestPortOnPhysicalNode):
l3out_name=ext_net.l3out_name,
node_profile_name=md.L3OUT_NODE_PROFILE_NAME,
interface_profile_name=md.L3OUT_IF_PROFILE_NAME,
interface_path=static_path['path'])
interface_path=static_path[0]['path'])
l3out_if = self.aim_mgr.get(aim_ctx, l3out_if)
if delete:
self.assertIsNone(l3out_if)
@ -9318,16 +9324,17 @@ class TestPortOnPhysicalNodeSingleDriver(TestPortOnPhysicalNode):
else:
epg = self._net_2_epg(net)
epg = self.aim_mgr.get(aim_ctx, epg)
# REVISIT: It looks like dissociating PhysDoms when using
# the default mapping fails (likely not specific to baremetal
# VNICs).
doms = [physical_domain]
if delete:
doms = []
else:
doms = [physical_domain]
static_path = []
self.assertEqual(static_path, epg.static_paths)
self.assertEqual(set(doms),
set(self._doms(epg.physical_domains,
with_type=False)))
self.assertEqual([static_path], epg.static_paths)
if net_type == 'vlan':
expected_binding_info = [('apic_aim', 'vlan')]
else:
@ -9376,21 +9383,23 @@ class TestPortOnPhysicalNodeSingleDriver(TestPortOnPhysicalNode):
self.assertNotEqual(vlan_p1, vlan_p2)
validate_static_path_and_doms(aim_ctx, is_svi, net2, kv_dict_2,
physical_domain, vlan_p2)
# Verify port and EPG after unbinding
kwargs = {portbindings.PROFILE: {}}
p2 = self._bind_port_to_host(p2['id'], None, **kwargs)['port']
validate_static_path_and_doms(aim_ctx, is_svi, net2, kv_dict_2,
physical_domain, vlan_p2,
delete=True)
self._delete('ports', p2['id'])
self._check_no_dynamic_segment(net2['id'])
validate_static_path_and_doms(aim_ctx, is_svi, net1, kv_dict_1,
physical_domain, vlan_p1)
# REVISIT: It looks like dissociation PhysDoms when using
# the default mapping fails (likely not specific to baremetal VNICs)
#validate_static_path_and_doms(aim_ctx, is_svi, net2, kv_dict_2,
# physical_domain, vlan_p2, delete=True)
validate_static_path_and_doms(aim_ctx, is_svi, net2, kv_dict_2,
physical_domain, vlan_p2, delete=True)
self._delete('ports', p1['id'])
self._check_no_dynamic_segment(net1['id'])
# REVISIT: It looks like dissociation PhysDoms when using
# the default mapping fails (likely not specific to baremetal VNICs)
#validate_static_path_and_doms(aim_ctx, is_svi, net1, kv_dict_1,
# physical_domain, vlan_p1, delete=True)
validate_static_path_and_doms(aim_ctx, is_svi, net1, kv_dict_1,
physical_domain, vlan_p1, delete=True)
class TestOpflexRpc(ApicAimTestCase):