openstack/ovn-bgp-agent
Code Issues Proposed changes

Refactor ovn_bgp_agent.privileged.ovs_vsctl

Browse Source 
When ovs commands fail, we re-try after adding `-O OpenFlow13`, but that
is only valid for `ovs-ofctl`.
To fix this issue, this patch implements a small refactor of the module
in order to separete calls to ovs-ofctl and ovs-vsctl.

Related-Bug: #2080258
Change-Id: Ib8ee872d4bd587ade325bc4f6da3e9d8804c28c0
(cherry picked from commit 7e348daca0)
This commit is contained in:
Eduardo Olivares 2024-10-08 09:55:47 +02:00
parent 4c7b76f3db
commit 855df3a483
4 changed files with 160 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
ovn_bgp_agent/drivers/openstack/utils/ovs.py
View File

@ -34,8 +34,8 @@ def _find_ovs_port(bridge):
# TODO(ltomasbo): What happens if there are several patch ports on the # TODO(ltomasbo): What happens if there are several patch ports on the
# same bridge? # same bridge?
ovs_port = None ovs_port = None
ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl(
'ovs-vsctl', ['list-ports', bridge])[0].rstrip() ['list-ports', bridge])[0].rstrip()
for p in ovs_ports.split('\n'): for p in ovs_ports.split('\n'):
if p.startswith(constants.OVS_PATCH_PROVNET_PORT_PREFIX): if p.startswith(constants.OVS_PATCH_PROVNET_PORT_PREFIX):
ovs_port = p ovs_port = p
@ -46,8 +46,8 @@ def get_bridge_flows(bridge, filter_=None):
args = ['dump-flows', bridge] args = ['dump-flows', bridge]
if filter_ is not None: if filter_ is not None:
args.append(filter_) args.append(filter_)
return ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( return ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(
'ovs-ofctl', args)[0].split('\n')[1:-1] args)[0].split('\n')[1:-1]
@tenacity.retry( @tenacity.retry(
@ -57,8 +57,8 @@ def get_bridge_flows(bridge, filter_=None):
reraise=True) reraise=True)
def get_device_port_at_ovs(device): def get_device_port_at_ovs(device):
try: try:
ofport = ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ofport = ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl(
'ovs-vsctl', ['get', 'Interface', device, 'ofport'] ['get', 'Interface', device, 'ofport']
)[0].rstrip() )[0].rstrip()
except Exception: except Exception:
raise agent_exc.PortNotFound(port=device) raise agent_exc.PortNotFound(port=device)
@ -71,8 +71,8 @@ def get_device_port_at_ovs(device):
def get_ovs_ports_info(bridge): def get_ovs_ports_info(bridge):
ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl(
'ovs-vsctl', ['list-ports', bridge])[0].rstrip() ['list-ports', bridge])[0].rstrip()
return ovs_ports.split("\n") return ovs_ports.split("\n")
@ -113,11 +113,11 @@ def ensure_mac_tweak_flows(bridge, mac, ports, cookie):
exist_flow_v6 = True exist_flow_v6 = True
if not exist_flow: if not exist_flow:
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(
'ovs-ofctl', ['add-flow', bridge, flow]) ['add-flow', bridge, flow])
if not exist_flow_v6: if not exist_flow_v6:
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(
'ovs-ofctl', ['add-flow', bridge, flow_v6]) ['add-flow', bridge, flow_v6])
def remove_extra_ovs_flows(ovs_flows, bridge, cookie): def remove_extra_ovs_flows(ovs_flows, bridge, cookie):
@ -143,13 +143,12 @@ def remove_extra_ovs_flows(ovs_flows, bridge, cookie):
if flow.split("priority")[1] not in expected_flows: if flow.split("priority")[1] not in expected_flows:
del_flow = ('{},{}').format( del_flow = ('{},{}').format(
cookie_id, flow.split("priority=900,")[1].split(" actions")[0]) cookie_id, flow.split("priority=900,")[1].split(" actions")[0])
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(
'ovs-ofctl', ['del-flows', bridge, del_flow]) ['del-flows', bridge, del_flow])
def ensure_flow(bridge, flow): def ensure_flow(bridge, flow):
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['add-flow', bridge, flow])
'ovs-ofctl', ['add-flow', bridge, flow])
def ensure_evpn_ovs_flow(bridge, cookie, mac, output_port, port_dst, net, def ensure_evpn_ovs_flow(bridge, cookie, mac, output_port, port_dst, net,
@ -175,8 +174,7 @@ def ensure_evpn_ovs_flow(bridge, cookie, mac, output_port, port_dst, net,
"actions=mod_dl_dst:{},{}output={}".format( "actions=mod_dl_dst:{},{}output={}".format(
cookie, ovs_ofport, mac, net, port_dst_mac, strip_vlan_opt, cookie, ovs_ofport, mac, net, port_dst_mac, strip_vlan_opt,
vrf_ofport)) vrf_ofport))
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['add-flow', bridge, flow])
'ovs-ofctl', ['add-flow', bridge, flow])
def remove_evpn_router_ovs_flows(bridge, cookie, mac): def remove_evpn_router_ovs_flows(bridge, cookie, mac):
@ -187,13 +185,12 @@ def remove_evpn_router_ovs_flows(bridge, cookie, mac):
cookie_id = "cookie={}/-1".format(cookie) cookie_id = "cookie={}/-1".format(cookie)
flow = ("{},ip,in_port={},dl_src:{}".format( flow = ("{},ip,in_port={},dl_src:{}".format(
cookie_id, ovs_ofport, mac)) cookie_id, ovs_ofport, mac))
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['del-flows', bridge, flow])
'ovs-ofctl', ['del-flows', bridge, flow])
flow_v6 = ("{},ipv6,in_port={},dl_src:{}".format(cookie_id, ovs_ofport, flow_v6 = ("{},ipv6,in_port={},dl_src:{}".format(cookie_id, ovs_ofport,
mac)) mac))
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(
'ovs-ofctl', ['del-flows', bridge, flow_v6]) ['del-flows', bridge, flow_v6])
def remove_evpn_network_ovs_flow(bridge, cookie, mac, net): def remove_evpn_network_ovs_flow(bridge, cookie, mac, net):
@ -209,15 +206,14 @@ def remove_evpn_network_ovs_flow(bridge, cookie, mac, net):
else: else:
flow = ("{},ip,in_port={},dl_src:{},nw_src={}".format( flow = ("{},ip,in_port={},dl_src:{},nw_src={}".format(
cookie_id, ovs_ofport, mac, net)) cookie_id, ovs_ofport, mac, net))
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['del-flows', bridge, flow])
'ovs-ofctl', ['del-flows', bridge, flow])
def add_device_to_ovs_bridge(device, bridge, vlan_tag=None): def add_device_to_ovs_bridge(device, bridge, vlan_tag=None):
args = ['--may-exist', 'add-port', bridge, device] args = ['--may-exist', 'add-port', bridge, device]
if vlan_tag is not None: if vlan_tag is not None:
args.append('tag=%s' % vlan_tag) args.append('tag=%s' % vlan_tag)
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd('ovs-vsctl', args) ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl(args)
def del_device_from_ovs_bridge(device, bridge=None): def del_device_from_ovs_bridge(device, bridge=None):
@ -225,7 +221,7 @@ def del_device_from_ovs_bridge(device, bridge=None):
if bridge: if bridge:
args.append(bridge) args.append(bridge)
args.append(device) args.append(device)
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd('ovs-vsctl', args) ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl(args)
def add_vlan_port_to_ovs_bridge(bridge, vlan, vlan_tag): def add_vlan_port_to_ovs_bridge(bridge, vlan, vlan_tag):
@ -234,15 +230,15 @@ def add_vlan_port_to_ovs_bridge(bridge, vlan, vlan_tag):
args = [ args = [
'--may-exist', 'add-port', bridge, vlan, 'tag={}'.format(vlan_tag), '--may-exist', 'add-port', bridge, vlan, 'tag={}'.format(vlan_tag),
'--', 'set', 'interface', vlan, 'type=internal'] '--', 'set', 'interface', vlan, 'type=internal']
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd('ovs-vsctl', args) ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl(args)
def del_flow(flow, bridge, cookie): def del_flow(flow, bridge, cookie):
cookie_id = "cookie={}/-1".format(cookie) cookie_id = "cookie={}/-1".format(cookie)
f = '{},priority{}'.format( f = '{},priority{}'.format(
cookie_id, flow.split(' actions')[0].split(' priority')[1]) cookie_id, flow.split(' actions')[0].split(' priority')[1])
ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(
'ovs-ofctl', ['--strict', 'del-flows', bridge, f]) ['--strict', 'del-flows', bridge, f])
def get_flow_info(flow): def get_flow_info(flow):

24
ovn_bgp_agent/privileged/ovs_vsctl.py
View File

@ -28,15 +28,17 @@ def ovs_cmd(command, args, timeout=None):
full_args += args full_args += args
try: try:
return processutils.execute(*full_args) return processutils.execute(*full_args)
except processutils.ProcessExecutionError: except Exception:
full_args += ['-O', 'OpenFlow13'] LOG.error("Unable to execute %s %s", command, full_args)
raise
def ovs_vsctl(args, timeout=None):
return ovs_cmd('ovs-vsctl', args, timeout)
def ovs_ofctl(args, timeout=None):
try: try:
return processutils.execute(*full_args) return ovs_cmd('ovs-ofctl', args, timeout)
except Exception as e: except processutils.ProcessExecutionError:
LOG.exception("Unable to execute %s %s. Exception: %s", return ovs_cmd('ovs-ofctl', args + ['-O', 'OpenFlow13'], timeout)
command, full_args, e)
raise
except Exception as e:
LOG.exception("Unable to execute %s %s. Exception: %s", command,
full_args, e)
raise

139
ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovs.py
View File

@ -44,7 +44,7 @@ class TestOVS(test_base.TestCase):
fake_flow_1 = '{},ipv6,in_port={}'.format(self.cookie_id, port_iface) fake_flow_1 = '{},ipv6,in_port={}'.format(self.cookie_id, port_iface)
fake_filter = 'cookie=fake-cookie/-1' if has_filter else None fake_filter = 'cookie=fake-cookie/-1' if has_filter else None
flows = 'HEADER\n%s\n%s\n' % (fake_flow_0, fake_flow_1) flows = 'HEADER\n%s\n%s\n' % (fake_flow_0, fake_flow_1)
self.mock_ovs_vsctl.ovs_cmd.return_value = [flows] self.mock_ovs_vsctl.ovs_ofctl.return_value = [flows]
ret = ovs_utils.get_bridge_flows(self.bridge, filter_=fake_filter) ret = ovs_utils.get_bridge_flows(self.bridge, filter_=fake_filter)
@ -52,8 +52,7 @@ class TestOVS(test_base.TestCase):
if has_filter: if has_filter:
expected_args.append(fake_filter) expected_args.append(fake_filter)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with(expected_args)
'ovs-ofctl', expected_args)
self.assertEqual([fake_flow_0, fake_flow_1], ret) self.assertEqual([fake_flow_0, fake_flow_1], ret)
def test_get_bridge_flows(self): def test_get_bridge_flows(self):
@ -65,77 +64,76 @@ class TestOVS(test_base.TestCase):
def test_get_device_port_at_ovs(self): def test_get_device_port_at_ovs(self):
port = 'fake-port' port = 'fake-port'
port_iface = '1' port_iface = '1'
self.mock_ovs_vsctl.ovs_cmd.return_value = port_iface self.mock_ovs_vsctl.ovs_vsctl.return_value = port_iface
ret = ovs_utils.get_device_port_at_ovs(port) ret = ovs_utils.get_device_port_at_ovs(port)
self.assertEqual(port_iface, ret) self.assertEqual(port_iface, ret)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(
'ovs-vsctl', ['get', 'Interface', port, 'ofport']) ['get', 'Interface', port, 'ofport'])
def test_get_ovs_ports_info(self): def test_get_ovs_ports_info(self):
bridge = 'fake-bridge' bridge = 'fake-bridge'
bridge_ports = ['br-ex'] bridge_ports = ['br-ex']
self.mock_ovs_vsctl.ovs_cmd.return_value = bridge_ports self.mock_ovs_vsctl.ovs_vsctl.return_value = bridge_ports
ret = ovs_utils.get_ovs_ports_info(bridge) ret = ovs_utils.get_ovs_ports_info(bridge)
self.assertEqual(bridge_ports, ret) self.assertEqual(bridge_ports, ret)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(
'ovs-vsctl', ['list-ports', bridge]) ['list-ports', bridge])
def test_get_ovs_patch_port_ofport(self): def test_get_ovs_patch_port_ofport(self):
patch = 'fake-patch' patch = 'fake-patch'
ofport = ['1'] ofport = ['1']
self.mock_ovs_vsctl.ovs_cmd.return_value = ofport self.mock_ovs_vsctl.ovs_vsctl.return_value = ofport
ret = ovs_utils.get_ovs_patch_port_ofport(patch) ret = ovs_utils.get_ovs_patch_port_ofport(patch)
self.assertEqual(ofport[0], ret) self.assertEqual(ofport[0], ret)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(
'ovs-vsctl',
['get', 'Interface', 'patch-fake-patch-to-br-int', 'ofport']) ['get', 'Interface', 'patch-fake-patch-to-br-int', 'ofport'])
def test_get_ovs_patch_port_ofport_exception(self): def test_get_ovs_patch_port_ofport_exception(self):
patch = 'fake-patch' patch = 'fake-patch'
self.mock_ovs_vsctl.ovs_cmd.side_effect = Exception self.mock_ovs_vsctl.ovs_vsctl.side_effect = Exception
self.assertRaises(agent_exc.PortNotFound, self.assertRaises(agent_exc.PortNotFound,
ovs_utils.get_ovs_patch_port_ofport, patch) ovs_utils.get_ovs_patch_port_ofport, patch)
expected_calls = [ expected_calls = [
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport'])] 'patch-fake-patch-to-br-int', 'ofport'])]
self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(expected_calls)
def test_get_ovs_patch_port_ofport_no_port(self): def test_get_ovs_patch_port_ofport_no_port(self):
patch = 'fake-patch' patch = 'fake-patch'
ofport = ['[]'] ofport = ['[]']
self.mock_ovs_vsctl.ovs_cmd.return_value = ofport self.mock_ovs_vsctl.ovs_vsctl.return_value = ofport
self.assertRaises(agent_exc.PortNotFound, self.assertRaises(agent_exc.PortNotFound,
ovs_utils.get_ovs_patch_port_ofport, patch) ovs_utils.get_ovs_patch_port_ofport, patch)
expected_calls = [ expected_calls = [
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport']), 'patch-fake-patch-to-br-int', 'ofport']),
mock.call('ovs-vsctl', ['get', 'Interface', mock.call(['get', 'Interface',
'patch-fake-patch-to-br-int', 'ofport'])] 'patch-fake-patch-to-br-int', 'ofport'])]
self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(expected_calls)
@mock.patch.object(ovs_utils, 'get_bridge_flows') @mock.patch.object(ovs_utils, 'get_bridge_flows')
def test_remove_extra_ovs_flows(self, mock_flows): def test_remove_extra_ovs_flows(self, mock_flows):
@ -161,8 +159,8 @@ class TestOVS(test_base.TestCase):
expected_del_flow = ('%s,ip,in_port=%s' % (self.cookie_id, expected_del_flow = ('%s,ip,in_port=%s' % (self.cookie_id,
extra_port_iface)) extra_port_iface))
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with(
'ovs-ofctl', ['del-flows', self.bridge, expected_del_flow]) ['del-flows', self.bridge, expected_del_flow])
mock_flows.assert_called_once_with(self.bridge, self.cookie_id) mock_flows.assert_called_once_with(self.bridge, self.cookie_id)
def test_ensure_flow(self): def test_ensure_flow(self):
@ -171,8 +169,8 @@ class TestOVS(test_base.TestCase):
ovs_utils.ensure_flow(bridge, flow) ovs_utils.ensure_flow(bridge, flow)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with(
'ovs-ofctl', ['add-flow', bridge, flow]) ['add-flow', bridge, flow])
@mock.patch.object(ovs_utils, 'get_device_port_at_ovs') @mock.patch.object(ovs_utils, 'get_device_port_at_ovs')
@mock.patch.object(linux_net, 'get_interface_address') @mock.patch.object(linux_net, 'get_interface_address')
@ -188,8 +186,9 @@ class TestOVS(test_base.TestCase):
port_iface = '1' port_iface = '1'
ovs_port_iface = '2' ovs_port_iface = '2'
net = 'fake-net' net = 'fake-net'
self.mock_ovs_vsctl.ovs_cmd.side_effect = ( self.mock_ovs_vsctl.ovs_vsctl.return_value = [
['%s\n%s\n' % (port, ovs_port)], None) '%s\n%s\n' % (port, ovs_port)]
self.mock_ovs_vsctl.ovs_ofctl.return_value = None
mock_ofport.side_effect = (ovs_port_iface, port_iface) mock_ofport.side_effect = (ovs_port_iface, port_iface)
# Invoke the method # Invoke the method
@ -211,15 +210,14 @@ class TestOVS(test_base.TestCase):
"ipv6_src={} actions=mod_dl_dst:{},{}output={}".format( "ipv6_src={} actions=mod_dl_dst:{},{}output={}".format(
self.cookie, ovs_port_iface, self.mac, net, address, self.cookie, ovs_port_iface, self.mac, net, address,
strip_vlan_opt, port_iface)) strip_vlan_opt, port_iface))
expected_calls = [ vsctl_expected_calls = [
mock.call('ovs-vsctl', ['list-ports', self.bridge]), mock.call(['list-ports', self.bridge])]
mock.call('ovs-ofctl', ['add-flow', self.bridge, expected_flow])] ofctl_expected_calls = [
self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) mock.call(['add-flow', self.bridge, expected_flow])]
self.assertEqual(len(expected_calls), self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls)
self.mock_ovs_vsctl.ovs_cmd.call_count) self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls)
expected_calls_ofport = [mock.call(ovs_port), mock.call(port)] expected_calls_ofport = [mock.call(ovs_port), mock.call(port)]
mock_ofport.assert_has_calls(expected_calls_ofport) mock_ofport.assert_has_calls(expected_calls_ofport)
self.assertEqual(len(expected_calls_ofport), mock_ofport.call_count)
def test_ensure_evpn_ovs_flow_ipv4(self): def test_ensure_evpn_ovs_flow_ipv4(self):
self._test_ensure_evpn_ovs_flow(ip_version=constants.IP_VERSION_4) self._test_ensure_evpn_ovs_flow(ip_version=constants.IP_VERSION_4)
@ -237,21 +235,22 @@ class TestOVS(test_base.TestCase):
def test_ensure_evpn_ovs_flow_no_ovs_ports(self): def test_ensure_evpn_ovs_flow_no_ovs_ports(self):
port = 'non-patch-provnet-port' port = 'non-patch-provnet-port'
self.mock_ovs_vsctl.ovs_cmd.return_value = [port] self.mock_ovs_vsctl.ovs_vsctl.return_value = [port]
ret = ovs_utils.ensure_evpn_ovs_flow( ret = ovs_utils.ensure_evpn_ovs_flow(
self.bridge, self.cookie, self.mac, port, 'fake-port-dst', self.bridge, self.cookie, self.mac, port, 'fake-port-dst',
'fake-net') 'fake-net')
self.assertIsNone(ret) self.assertIsNone(ret)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(
'ovs-vsctl', ['list-ports', self.bridge]) ['list-ports', self.bridge])
@mock.patch.object(ovs_utils, 'get_device_port_at_ovs') @mock.patch.object(ovs_utils, 'get_device_port_at_ovs')
def test_remove_evpn_router_ovs_flows(self, mock_ofport): def test_remove_evpn_router_ovs_flows(self, mock_ofport):
ovs_port = constants.OVS_PATCH_PROVNET_PORT_PREFIX + 'fake-port' ovs_port = constants.OVS_PATCH_PROVNET_PORT_PREFIX + 'fake-port'
ovs_port_iface = '1' ovs_port_iface = '1'
self.mock_ovs_vsctl.ovs_cmd.side_effect = ([ovs_port], None, None) self.mock_ovs_vsctl.ovs_vsctl.return_value = [ovs_port]
self.mock_ovs_vsctl.ovs_ofctl.side_effect = (None, None)
mock_ofport.return_value = ovs_port_iface mock_ofport.return_value = ovs_port_iface
# Invoke the method # Invoke the method
@ -263,26 +262,27 @@ class TestOVS(test_base.TestCase):
expected_flow_v6 = '{},ipv6,in_port={},dl_src:{}'.format( expected_flow_v6 = '{},ipv6,in_port={},dl_src:{}'.format(
self.cookie_id, ovs_port_iface, self.mac) self.cookie_id, ovs_port_iface, self.mac)
expected_calls = [ vsctl_expected_calls = [
mock.call('ovs-vsctl', ['list-ports', self.bridge]), mock.call(['list-ports', self.bridge])]
mock.call('ovs-ofctl', ['del-flows', self.bridge, expected_flow]), ofctl_expected_calls = [
mock.call('ovs-ofctl', ['del-flows', self.bridge, mock.call(['del-flows', self.bridge, expected_flow]),
expected_flow_v6])] mock.call(['del-flows', self.bridge, expected_flow_v6])]
self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls)
self.assertEqual(len(expected_calls), self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls)
self.mock_ovs_vsctl.ovs_cmd.call_count) self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls)
mock_ofport.assert_called_once_with(ovs_port) mock_ofport.assert_called_once_with(ovs_port)
def test_remove_evpn_router_ovs_flows_no_ovs_port(self): def test_remove_evpn_router_ovs_flows_no_ovs_port(self):
port = 'non-patch-provnet-port' port = 'non-patch-provnet-port'
self.mock_ovs_vsctl.ovs_cmd.return_value = [port] self.mock_ovs_vsctl.ovs_vsctl.return_value = [port]
ret = ovs_utils.remove_evpn_router_ovs_flows( ret = ovs_utils.remove_evpn_router_ovs_flows(
self.bridge, self.cookie, self.mac) self.bridge, self.cookie, self.mac)
self.assertIsNone(ret) self.assertIsNone(ret)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(
'ovs-vsctl', ['list-ports', self.bridge]) ['list-ports', self.bridge])
@mock.patch.object(ovs_utils, 'get_device_port_at_ovs') @mock.patch.object(ovs_utils, 'get_device_port_at_ovs')
@mock.patch.object(linux_net, 'get_ip_version') @mock.patch.object(linux_net, 'get_ip_version')
@ -293,7 +293,8 @@ class TestOVS(test_base.TestCase):
net = 'fake-net' net = 'fake-net'
mock_ip_version.return_value = ip_version mock_ip_version.return_value = ip_version
mock_ofport.return_value = ovs_port_iface mock_ofport.return_value = ovs_port_iface
self.mock_ovs_vsctl.ovs_cmd.side_effect = ([ovs_port], None) self.mock_ovs_vsctl.ovs_vsctl.return_value = [ovs_port]
self.mock_ovs_vsctl.ovs_ofctl.side_effect = None
ovs_utils.remove_evpn_network_ovs_flow( ovs_utils.remove_evpn_network_ovs_flow(
self.bridge, self.cookie, self.mac, net) self.bridge, self.cookie, self.mac, net)
@ -305,12 +306,12 @@ class TestOVS(test_base.TestCase):
expected_flow = ("{},ip,in_port={},dl_src:{},nw_src={}".format( expected_flow = ("{},ip,in_port={},dl_src:{},nw_src={}".format(
self.cookie_id, ovs_port_iface, self.mac, net)) self.cookie_id, ovs_port_iface, self.mac, net))
expected_calls = [ vsctl_expected_calls = [
mock.call('ovs-vsctl', ['list-ports', self.bridge]), mock.call(['list-ports', self.bridge])]
mock.call('ovs-ofctl', ['del-flows', self.bridge, expected_flow])] ofctl_expected_calls = [
self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) mock.call(['del-flows', self.bridge, expected_flow])]
self.assertEqual(len(expected_calls), self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls)
self.mock_ovs_vsctl.ovs_cmd.call_count) self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls)
mock_ip_version.assert_called_once_with(net) mock_ip_version.assert_called_once_with(net)
def test_remove_evpn_network_ovs_flow_ipv4(self): def test_remove_evpn_network_ovs_flow_ipv4(self):
@ -323,13 +324,13 @@ class TestOVS(test_base.TestCase):
def test_remove_evpn_network_ovs_flow_no_ovs_port(self): def test_remove_evpn_network_ovs_flow_no_ovs_port(self):
port = 'non-patch-provnet-port' port = 'non-patch-provnet-port'
self.mock_ovs_vsctl.ovs_cmd.return_value = [port] self.mock_ovs_vsctl.ovs_vsctl.return_value = [port]
ovs_utils.remove_evpn_network_ovs_flow( ovs_utils.remove_evpn_network_ovs_flow(
self.bridge, self.cookie, self.mac, 'fake-net') self.bridge, self.cookie, self.mac, 'fake-net')
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(
'ovs-vsctl', ['list-ports', self.bridge]) ['list-ports', self.bridge])
def _test_add_device_to_ovs_bridge(self, vlan_tag=False): def _test_add_device_to_ovs_bridge(self, vlan_tag=False):
device = 'ethX' device = 'ethX'
@ -341,8 +342,7 @@ class TestOVS(test_base.TestCase):
if vlan_tag: if vlan_tag:
expected_args.append('tag=%s' % vtag) expected_args.append('tag=%s' % vtag)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(expected_args)
'ovs-vsctl', expected_args)
def test_add_device_to_ovs_bridge(self): def test_add_device_to_ovs_bridge(self):
self._test_add_device_to_ovs_bridge() self._test_add_device_to_ovs_bridge()
@ -361,8 +361,7 @@ class TestOVS(test_base.TestCase):
expected_args.append(br) expected_args.append(br)
expected_args.append(device) expected_args.append(device)
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(expected_args)
'ovs-vsctl', expected_args)
def test_del_device_from_ovs_bridge(self): def test_del_device_from_ovs_bridge(self):
self._test_del_device_from_ovs_bridge() self._test_del_device_from_ovs_bridge()
@ -379,8 +378,8 @@ class TestOVS(test_base.TestCase):
expected_flow = ('{},priority=1000,ip,dl_src=fa:16:3e:15:9e:f0,' expected_flow = ('{},priority=1000,ip,dl_src=fa:16:3e:15:9e:f0,'
'nw_src=20.0.0.0/24'.format(self.cookie_id)) 'nw_src=20.0.0.0/24'.format(self.cookie_id))
self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with(
'ovs-ofctl', ['--strict', 'del-flows', self.bridge, expected_flow]) ['--strict', 'del-flows', self.bridge, expected_flow])
def test_get_flow_info(self): def test_get_flow_info(self):
flow = ('cookie=0x3e6, duration=11.647s, table=0, n_packets=0, ' flow = ('cookie=0x3e6, duration=11.647s, table=0, n_packets=0, '

65
ovn_bgp_agent/tests/unit/privileged/test_ovs_vsctl.py
View File

@ -38,47 +38,68 @@ class TestPrivilegedOvsVsctl(test_base.TestCase):
# Mock processutils.execute() # Mock processutils.execute()
self.mock_exc = mock.patch.object(processutils, 'execute').start() self.mock_exc = mock.patch.object(processutils, 'execute').start()
def test_ovs_cmd(self): def test_ovs_vsctl(self):
ovs_vsctl.ovs_cmd( ovs_vsctl.ovs_vsctl(
'ovs-vsctl', ['--if-exists', 'del-port', 'fake-port']) ['--if-exists', 'del-port', 'fake-port'])
self.mock_exc.assert_called_once_with( self.mock_exc.assert_called_once_with(
'ovs-vsctl', '--if-exists', 'del-port', 'fake-port') 'ovs-vsctl', '--if-exists', 'del-port', 'fake-port')
def test_ovs_cmd_timeout(self): def test_ovs_ofctl(self):
ovs_vsctl.ovs_cmd( ovs_vsctl.ovs_ofctl(
'ovs-vsctl', ['--if-exists', 'del-port', 'fake-port'], timeout=10) ['dump-flows', 'dummy-br'])
self.mock_exc.assert_called_once_with(
'ovs-ofctl', 'dump-flows', 'dummy-br')
def test_ovs_vsctl_timeout(self):
ovs_vsctl.ovs_vsctl(
['--if-exists', 'del-port', 'fake-port'], timeout=10)
self.mock_exc.assert_called_once_with( self.mock_exc.assert_called_once_with(
'ovs-vsctl', '--timeout=10', '--if-exists', 'del-port', 'ovs-vsctl', '--timeout=10', '--if-exists', 'del-port',
'fake-port') 'fake-port')
def test_ovs_cmd_fallback_OF_version(self): def test_ovs_ofctl_timeout(self):
ovs_vsctl.ovs_ofctl(
['dump-flows', 'dummy-br'], timeout=10)
self.mock_exc.assert_called_once_with(
'ovs-ofctl', '--timeout=10', 'dump-flows', 'dummy-br')
def test_ovs_ofctl_fallback_OF_version(self):
# fallback only applies to ovs-ofctl command
self.mock_exc.side_effect = ( self.mock_exc.side_effect = (
processutils.ProcessExecutionError(), None) processutils.ProcessExecutionError(), None)
ovs_vsctl.ovs_cmd( ovs_vsctl.ovs_ofctl(
'ovs-vsctl', ['--if-exists', 'del-port', 'fake-port']) ['--strict', 'del-flows', 'br-ex', 'dummy-flow'])
calls = [mock.call('ovs-vsctl', '--if-exists', 'del-port', calls = [mock.call('ovs-ofctl', '--strict', 'del-flows',
'fake-port'), 'br-ex', 'dummy-flow'),
mock.call('ovs-vsctl', '--if-exists', 'del-port', mock.call('ovs-ofctl', '--strict', 'del-flows',
'fake-port', '-O', 'OpenFlow13')] 'br-ex', 'dummy-flow', '-O', 'OpenFlow13')]
self.mock_exc.assert_has_calls(calls) self.mock_exc.assert_has_calls(calls)
def test_ovs_cmd_exception(self): def test_ovs_vsctl_process_execution_error_no_fallback(self):
self.mock_exc.side_effect = FakeException() # fallback does not apply to ovs-vsctl command
self.mock_exc.side_effect = processutils.ProcessExecutionError()
self.assertRaises( self.assertRaises(
FakeException, ovs_vsctl.ovs_cmd, 'ovs-vsctl', processutils.ProcessExecutionError, ovs_vsctl.ovs_vsctl,
['--if-exists', 'del-port', 'fake-port']) ['--if-exists', 'del-port', 'fake-port'])
self.mock_exc.assert_called_once_with( self.mock_exc.assert_called_once_with(
'ovs-vsctl', '--if-exists', 'del-port', 'fake-port') 'ovs-vsctl', '--if-exists', 'del-port', 'fake-port')
def test_ovs_ofctl_exception(self):
self.mock_exc.side_effect = FakeException()
self.assertRaises(
FakeException, ovs_vsctl.ovs_ofctl,
['add-flow', 'br-ex', 'dummy-flow'])
self.mock_exc.assert_called_once_with(
'ovs-ofctl', 'add-flow', 'br-ex', 'dummy-flow')
def test_ovs_cmd_fallback_exception(self): def test_ovs_cmd_fallback_exception(self):
self.mock_exc.side_effect = ( self.mock_exc.side_effect = (
processutils.ProcessExecutionError(), FakeException()) processutils.ProcessExecutionError(), FakeException())
self.assertRaises( self.assertRaises(
FakeException, ovs_vsctl.ovs_cmd, 'ovs-vsctl', FakeException, ovs_vsctl.ovs_ofctl,
['--if-exists', 'del-port', 'fake-port']) ['add-flow', 'br-ex', 'dummy-flow'])
calls = [mock.call('ovs-vsctl', '--if-exists', 'del-port', calls = [mock.call('ovs-ofctl', 'add-flow', 'br-ex', 'dummy-flow'),
'fake-port'), mock.call('ovs-ofctl', 'add-flow', 'br-ex', 'dummy-flow',
mock.call('ovs-vsctl', '--if-exists', 'del-port', '-O', 'OpenFlow13')]
'fake-port', '-O', 'OpenFlow13')]
self.mock_exc.assert_has_calls(calls) self.mock_exc.assert_has_calls(calls)