bgp: Use NIC types when obtaining peer NIC
The physical interface connecting to a leaf switch should be a type 'system'. The patch adds this type when querying if the configured BGP bridge has already a NIC attached to it. The original empty string is kept there but maybe we should always assume system. Closes-Bug: #2148246 Change-Id: I5e0983ad09eea1c7461d9ca894a49e77c109a257 Signed-off-by: Jakub Libosvar <libosvar@redhat.com>
This commit is contained in:
@@ -668,9 +668,9 @@ class OVSBridge(BaseOVS):
|
||||
# get the interface name list for this bridge
|
||||
return self.ovsdb.list_ifaces(self.br_name).execute(check_error=True)
|
||||
|
||||
def get_iface_ofports_by_type(self, type_):
|
||||
return _GetBridgeInterfacesOfportsByTypeCommand(
|
||||
self.ovsdb, self.br_name, type_).execute(check_error=True)
|
||||
def get_iface_ofports_by_types(self, *types):
|
||||
return _GetBridgeInterfacesOfportsByTypesCommand(
|
||||
self.ovsdb, self.br_name, *types).execute(check_error=True)
|
||||
|
||||
def get_port_name_list(self):
|
||||
# get the port name list for this bridge
|
||||
@@ -1398,18 +1398,18 @@ class DeferredOVSBridge:
|
||||
self.br.br_name)
|
||||
|
||||
|
||||
class _GetBridgeInterfacesOfportsByTypeCommand(ovs_cmd.ReadOnlyCommand):
|
||||
def __init__(self, api, bridge_name, type_):
|
||||
class _GetBridgeInterfacesOfportsByTypesCommand(ovs_cmd.ReadOnlyCommand):
|
||||
def __init__(self, api, bridge_name, *types):
|
||||
super().__init__(api)
|
||||
self.bridge_name = bridge_name
|
||||
self.type = type_
|
||||
self.types = types
|
||||
|
||||
def run_idl(self, txn):
|
||||
br = idlutils.row_by_value(
|
||||
self.api.idl, 'Bridge', 'name', self.bridge_name)
|
||||
self.result = [
|
||||
i.ofport[0] for p in br.ports if p.name != self.bridge_name
|
||||
for i in p.interfaces if i.type == self.type and i.ofport]
|
||||
for i in p.interfaces if i.type in self.types and i.ofport]
|
||||
|
||||
|
||||
def _build_flow_expr_str(flow_dict, cmd, strict):
|
||||
|
||||
@@ -112,22 +112,22 @@ class BGPAgentExtension(ovn_ext_mgr.OVNAgentExtension):
|
||||
bridge_name_list
|
||||
).execute(check_error=True)
|
||||
|
||||
def watch_port_created_event(self, bgp_bridge, port_type):
|
||||
def watch_port_created_event(self, bgp_bridge, *port_types):
|
||||
# Check the port doesn't exist on the bridge
|
||||
ports_ofports = bgp_bridge.ovs_bridge.get_iface_ofports_by_type(
|
||||
port_type)
|
||||
ports_ofports = bgp_bridge.ovs_bridge.get_iface_ofports_by_types(
|
||||
*port_types)
|
||||
|
||||
if not ports_ofports:
|
||||
LOG.debug("Waiting for a %s port creation on bridge %s",
|
||||
port_type, bgp_bridge.name)
|
||||
port_types, bgp_bridge.name)
|
||||
event_handler = self.agent_api.ovs_idl.idl.notify_handler
|
||||
event = events.BGPBridgePortCreatedEvent(
|
||||
self.agent_api, bgp_bridge.name, port_type)
|
||||
self.agent_api, bgp_bridge.name, *port_types)
|
||||
event_handler.watch_event(event)
|
||||
|
||||
# Check the port again in case it was created in the meantime
|
||||
ports_ofports = (
|
||||
bgp_bridge.ovs_bridge.get_iface_ofports_by_type(port_type))
|
||||
bgp_bridge.ovs_bridge.get_iface_ofports_by_types(*port_types))
|
||||
|
||||
# FIXME(jlibosva): Check if there could be a race condition here
|
||||
# where we receive the event, it configures the
|
||||
@@ -137,12 +137,12 @@ class BGPAgentExtension(ovn_ext_mgr.OVNAgentExtension):
|
||||
LOG.debug(
|
||||
"The %s port was created in the meantime on bridge %s "
|
||||
"with ofport %d, removing the onetime event from the "
|
||||
"queue.", port_type, bgp_bridge.name, ports_ofports[0])
|
||||
"queue.", port_types, bgp_bridge.name, ports_ofports[0])
|
||||
event_handler.unwatch_event(event)
|
||||
if bgp_bridge.check_requirements_for_flows_met():
|
||||
bgp_bridge.configure_flows()
|
||||
else:
|
||||
LOG.debug("The BGP bridge %s already has a %s port with ofport"
|
||||
" %d", bgp_bridge.name, port_type, ports_ofports[0])
|
||||
" %d", bgp_bridge.name, port_types, ports_ofports[0])
|
||||
if bgp_bridge.check_requirements_for_flows_met():
|
||||
bgp_bridge.configure_flows()
|
||||
|
||||
@@ -90,7 +90,7 @@ class BGPChassisBridge(Bridge):
|
||||
|
||||
@property
|
||||
def patch_port_ofport(self):
|
||||
patch_ports_ofports = self.ovs_bridge.get_iface_ofports_by_type(
|
||||
patch_ports_ofports = self.ovs_bridge.get_iface_ofports_by_types(
|
||||
'patch')
|
||||
if len(patch_ports_ofports) > 1:
|
||||
LOG.warning("The patch port for bridge %s has multiple ofports: "
|
||||
@@ -106,7 +106,8 @@ class BGPChassisBridge(Bridge):
|
||||
@property
|
||||
def nic_ofport(self):
|
||||
# REVISIT(jlibosva): we can consider supporting OVS bonds too
|
||||
nics_ofports = self.ovs_bridge.get_iface_ofports_by_type('')
|
||||
nics_ofports = self.ovs_bridge.get_iface_ofports_by_types(
|
||||
*bgp_const.BGP_BRIDGE_NIC_TYPES)
|
||||
if len(nics_ofports) != 1:
|
||||
LOG.warning("Expected 1 NIC for bridge %s, got %s",
|
||||
self.name, len(nics_ofports))
|
||||
|
||||
@@ -180,8 +180,8 @@ class NewBgpBridgeEvent(BGPAgentEvent):
|
||||
def run(self, event, row, old):
|
||||
bgp_bridge = self.bgp_agent.create_bgp_bridge(row.name)
|
||||
self.bgp_agent.watch_port_created_event(bgp_bridge, 'patch')
|
||||
# Empty string is for the NIC connecting to the leaf switch
|
||||
self.bgp_agent.watch_port_created_event(bgp_bridge, '')
|
||||
self.bgp_agent.watch_port_created_event(
|
||||
bgp_bridge, *constants.BGP_BRIDGE_NIC_TYPES)
|
||||
if bgp_bridge.check_requirements_for_flows_met():
|
||||
bgp_bridge.configure_flows()
|
||||
|
||||
@@ -228,15 +228,15 @@ class BGPBridgePortCreatedEvent(BGPAgentEvent):
|
||||
TABLE = 'Interface'
|
||||
ONETIME = True
|
||||
|
||||
def __init__(self, agent_api, bgp_bridge_name, port_type):
|
||||
def __init__(self, agent_api, bgp_bridge_name, *port_types):
|
||||
super().__init__(agent_api)
|
||||
self.bgp_bridge_name = bgp_bridge_name
|
||||
self.port_type = port_type
|
||||
self.port_types = port_types
|
||||
|
||||
@property
|
||||
def key(self):
|
||||
return (self.__class__, self.table,
|
||||
tuple(self.events), self.bgp_bridge_name)
|
||||
tuple(self.events), self.bgp_bridge_name, *self.port_types)
|
||||
|
||||
def _get_port_bridge(self, port_name):
|
||||
# We just need access to BaseOVS
|
||||
@@ -247,7 +247,7 @@ class BGPBridgePortCreatedEvent(BGPAgentEvent):
|
||||
if not super().match_fn(event, row, old):
|
||||
return False
|
||||
|
||||
if row.type != self.port_type:
|
||||
if row.type not in self.port_types:
|
||||
return False
|
||||
|
||||
try:
|
||||
|
||||
@@ -289,12 +289,12 @@ class OVSBridgeTestCase(OVSBridgeTestBase):
|
||||
ifaces = {self.create_ovs_port()[0] for i in range(5)}
|
||||
self.assertSetEqual(ifaces, set(self.br.get_iface_name_list()))
|
||||
|
||||
def test_get_iface_ofports_by_type(self):
|
||||
def test_get_iface_ofports_by_types(self):
|
||||
internal_port_ofport = self.create_ovs_port()[1]
|
||||
patch_port_ofport = self.create_ovs_port(('type', 'patch'))[1]
|
||||
observed_internal_ofports = self.br.get_iface_ofports_by_type(
|
||||
observed_internal_ofports = self.br.get_iface_ofports_by_types(
|
||||
'internal')
|
||||
observed_patch_ofports = self.br.get_iface_ofports_by_type(
|
||||
observed_patch_ofports = self.br.get_iface_ofports_by_types(
|
||||
'patch')
|
||||
self.assertCountEqual(
|
||||
observed_internal_ofports, [internal_port_ofport])
|
||||
|
||||
@@ -31,7 +31,8 @@ class TestBGPAgentExtensionWatchPortCreatedEvent(base.BaseTestCase):
|
||||
self.bgp_bridge.name = 'br-bgp'
|
||||
|
||||
def test_port_already_exists_configures_flows(self):
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_type.return_value = [5]
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_types.return_value = [
|
||||
5]
|
||||
self.bgp_bridge.check_requirements_for_flows_met.return_value = True
|
||||
|
||||
self.extension.watch_port_created_event(self.bgp_bridge, 'patch')
|
||||
@@ -39,7 +40,8 @@ class TestBGPAgentExtensionWatchPortCreatedEvent(base.BaseTestCase):
|
||||
self.bgp_bridge.configure_flows.assert_called_once()
|
||||
|
||||
def test_port_already_exists_requirements_not_met_no_configure_flows(self):
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_type.return_value = [5]
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_types.return_value = [
|
||||
5]
|
||||
self.bgp_bridge.check_requirements_for_flows_met.return_value = False
|
||||
|
||||
self.extension.watch_port_created_event(self.bgp_bridge, 'patch')
|
||||
@@ -47,7 +49,7 @@ class TestBGPAgentExtensionWatchPortCreatedEvent(base.BaseTestCase):
|
||||
self.bgp_bridge.configure_flows.assert_not_called()
|
||||
|
||||
def test_port_missing_watches_event(self):
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_type.return_value = []
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_types.return_value = []
|
||||
|
||||
self.extension.watch_port_created_event(self.bgp_bridge, 'patch')
|
||||
|
||||
@@ -55,7 +57,7 @@ class TestBGPAgentExtensionWatchPortCreatedEvent(base.BaseTestCase):
|
||||
self.bgp_bridge.configure_flows.assert_not_called()
|
||||
|
||||
def test_port_created_in_meantime_unwatches_and_configures_flows(self):
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_type.side_effect = [
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_types.side_effect = [
|
||||
[],
|
||||
[7],
|
||||
]
|
||||
@@ -69,7 +71,7 @@ class TestBGPAgentExtensionWatchPortCreatedEvent(base.BaseTestCase):
|
||||
|
||||
def test_port_created_in_meantime_requirements_not_met_no_configure_flows(
|
||||
self):
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_type.side_effect = [
|
||||
self.bgp_bridge.ovs_bridge.get_iface_ofports_by_types.side_effect = [
|
||||
[],
|
||||
[3],
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user