diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovs.py b/ovn_bgp_agent/drivers/openstack/utils/ovs.py index 5f27aba5..ce1f6440 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovs.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovs.py @@ -34,8 +34,8 @@ def _find_ovs_port(bridge): # TODO(ltomasbo): What happens if there are several patch ports on the # same bridge? ovs_port = None - ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-vsctl', ['list-ports', bridge])[0].rstrip() + ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl( + ['list-ports', bridge])[0].rstrip() for p in ovs_ports.split('\n'): if p.startswith(constants.OVS_PATCH_PROVNET_PORT_PREFIX): ovs_port = p @@ -46,8 +46,8 @@ def get_bridge_flows(bridge, filter_=None): args = ['dump-flows', bridge] if filter_ is not None: args.append(filter_) - return ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', args)[0].split('\n')[1:-1] + return ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl( + args)[0].split('\n')[1:-1] @tenacity.retry( @@ -57,8 +57,8 @@ def get_bridge_flows(bridge, filter_=None): reraise=True) def get_device_port_at_ovs(device): try: - ofport = ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-vsctl', ['get', 'Interface', device, 'ofport'] + ofport = ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl( + ['get', 'Interface', device, 'ofport'] )[0].rstrip() except Exception: raise agent_exc.PortNotFound(port=device) @@ -71,8 +71,8 @@ def get_device_port_at_ovs(device): def get_ovs_ports_info(bridge): - ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-vsctl', ['list-ports', bridge])[0].rstrip() + ovs_ports = ovn_bgp_agent.privileged.ovs_vsctl.ovs_vsctl( + ['list-ports', bridge])[0].rstrip() return ovs_ports.split("\n") @@ -113,11 +113,11 @@ def ensure_mac_tweak_flows(bridge, mac, ports, cookie): exist_flow_v6 = True if not exist_flow: - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['add-flow', bridge, flow]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl( + ['add-flow', bridge, flow]) if not exist_flow_v6: - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['add-flow', bridge, flow_v6]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl( + ['add-flow', bridge, flow_v6]) 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: del_flow = ('{},{}').format( cookie_id, flow.split("priority=900,")[1].split(" actions")[0]) - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['del-flows', bridge, del_flow]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl( + ['del-flows', bridge, del_flow]) def ensure_flow(bridge, flow): - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['add-flow', bridge, flow]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['add-flow', bridge, flow]) 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( cookie, ovs_ofport, mac, net, port_dst_mac, strip_vlan_opt, vrf_ofport)) - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['add-flow', bridge, flow]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['add-flow', bridge, flow]) 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) flow = ("{},ip,in_port={},dl_src:{}".format( cookie_id, ovs_ofport, mac)) - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['del-flows', bridge, flow]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['del-flows', bridge, flow]) flow_v6 = ("{},ipv6,in_port={},dl_src:{}".format(cookie_id, ovs_ofport, mac)) - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['del-flows', bridge, flow_v6]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl( + ['del-flows', bridge, flow_v6]) 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: flow = ("{},ip,in_port={},dl_src:{},nw_src={}".format( cookie_id, ovs_ofport, mac, net)) - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['del-flows', bridge, flow]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl(['del-flows', bridge, flow]) def add_device_to_ovs_bridge(device, bridge, vlan_tag=None): args = ['--may-exist', 'add-port', bridge, device] if vlan_tag is not None: 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): @@ -225,7 +221,7 @@ def del_device_from_ovs_bridge(device, bridge=None): if bridge: args.append(bridge) 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): @@ -234,15 +230,15 @@ def add_vlan_port_to_ovs_bridge(bridge, vlan, vlan_tag): args = [ '--may-exist', 'add-port', bridge, vlan, 'tag={}'.format(vlan_tag), '--', '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): cookie_id = "cookie={}/-1".format(cookie) f = '{},priority{}'.format( cookie_id, flow.split(' actions')[0].split(' priority')[1]) - ovn_bgp_agent.privileged.ovs_vsctl.ovs_cmd( - 'ovs-ofctl', ['--strict', 'del-flows', bridge, f]) + ovn_bgp_agent.privileged.ovs_vsctl.ovs_ofctl( + ['--strict', 'del-flows', bridge, f]) def get_flow_info(flow): diff --git a/ovn_bgp_agent/privileged/ovs_vsctl.py b/ovn_bgp_agent/privileged/ovs_vsctl.py index 19ca0916..efc3b612 100644 --- a/ovn_bgp_agent/privileged/ovs_vsctl.py +++ b/ovn_bgp_agent/privileged/ovs_vsctl.py @@ -28,15 +28,17 @@ def ovs_cmd(command, args, timeout=None): full_args += args try: return processutils.execute(*full_args) - except processutils.ProcessExecutionError: - full_args += ['-O', 'OpenFlow13'] - try: - return processutils.execute(*full_args) - except Exception as e: - LOG.exception("Unable to execute %s %s. Exception: %s", - command, full_args, e) - raise - except Exception as e: - LOG.exception("Unable to execute %s %s. Exception: %s", command, - full_args, e) + except Exception: + 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: + return ovs_cmd('ovs-ofctl', args, timeout) + except processutils.ProcessExecutionError: + return ovs_cmd('ovs-ofctl', args + ['-O', 'OpenFlow13'], timeout) diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovs.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovs.py index 41b7aef9..1272d6a4 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovs.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovs.py @@ -44,7 +44,7 @@ class TestOVS(test_base.TestCase): fake_flow_1 = '{},ipv6,in_port={}'.format(self.cookie_id, port_iface) fake_filter = 'cookie=fake-cookie/-1' if has_filter else None 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) @@ -52,8 +52,7 @@ class TestOVS(test_base.TestCase): if has_filter: expected_args.append(fake_filter) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-ofctl', expected_args) + self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with(expected_args) self.assertEqual([fake_flow_0, fake_flow_1], ret) def test_get_bridge_flows(self): @@ -65,77 +64,76 @@ class TestOVS(test_base.TestCase): def test_get_device_port_at_ovs(self): port = 'fake-port' 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) self.assertEqual(port_iface, ret) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', ['get', 'Interface', port, 'ofport']) + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with( + ['get', 'Interface', port, 'ofport']) def test_get_ovs_ports_info(self): bridge = 'fake-bridge' 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) self.assertEqual(bridge_ports, ret) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', ['list-ports', bridge]) + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with( + ['list-ports', bridge]) def test_get_ovs_patch_port_ofport(self): patch = 'fake-patch' 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) self.assertEqual(ofport[0], ret) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with( ['get', 'Interface', 'patch-fake-patch-to-br-int', 'ofport']) def test_get_ovs_patch_port_ofport_exception(self): 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, ovs_utils.get_ovs_patch_port_ofport, patch) expected_calls = [ - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport'])] - self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport'])] + self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(expected_calls) def test_get_ovs_patch_port_ofport_no_port(self): patch = 'fake-patch' ofport = ['[]'] - self.mock_ovs_vsctl.ovs_cmd.return_value = ofport + self.mock_ovs_vsctl.ovs_vsctl.return_value = ofport self.assertRaises(agent_exc.PortNotFound, ovs_utils.get_ovs_patch_port_ofport, patch) expected_calls = [ - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport']), - mock.call('ovs-vsctl', ['get', 'Interface', - 'patch-fake-patch-to-br-int', 'ofport'])] - self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport']), + mock.call(['get', 'Interface', + 'patch-fake-patch-to-br-int', 'ofport'])] + self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(expected_calls) @mock.patch.object(ovs_utils, 'get_bridge_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, extra_port_iface)) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-ofctl', ['del-flows', self.bridge, expected_del_flow]) + self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with( + ['del-flows', self.bridge, expected_del_flow]) mock_flows.assert_called_once_with(self.bridge, self.cookie_id) def test_ensure_flow(self): @@ -171,8 +169,8 @@ class TestOVS(test_base.TestCase): ovs_utils.ensure_flow(bridge, flow) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-ofctl', ['add-flow', bridge, flow]) + self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with( + ['add-flow', bridge, flow]) @mock.patch.object(ovs_utils, 'get_device_port_at_ovs') @mock.patch.object(linux_net, 'get_interface_address') @@ -188,8 +186,9 @@ class TestOVS(test_base.TestCase): port_iface = '1' ovs_port_iface = '2' net = 'fake-net' - self.mock_ovs_vsctl.ovs_cmd.side_effect = ( - ['%s\n%s\n' % (port, ovs_port)], None) + self.mock_ovs_vsctl.ovs_vsctl.return_value = [ + '%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) # Invoke the method @@ -211,15 +210,14 @@ class TestOVS(test_base.TestCase): "ipv6_src={} actions=mod_dl_dst:{},{}output={}".format( self.cookie, ovs_port_iface, self.mac, net, address, strip_vlan_opt, port_iface)) - expected_calls = [ - mock.call('ovs-vsctl', ['list-ports', self.bridge]), - mock.call('ovs-ofctl', ['add-flow', self.bridge, expected_flow])] - self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) - self.assertEqual(len(expected_calls), - self.mock_ovs_vsctl.ovs_cmd.call_count) + vsctl_expected_calls = [ + mock.call(['list-ports', self.bridge])] + ofctl_expected_calls = [ + mock.call(['add-flow', self.bridge, expected_flow])] + self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls) + self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls) expected_calls_ofport = [mock.call(ovs_port), mock.call(port)] 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): 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): 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( self.bridge, self.cookie, self.mac, port, 'fake-port-dst', 'fake-net') self.assertIsNone(ret) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', ['list-ports', self.bridge]) + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with( + ['list-ports', self.bridge]) @mock.patch.object(ovs_utils, 'get_device_port_at_ovs') def test_remove_evpn_router_ovs_flows(self, mock_ofport): ovs_port = constants.OVS_PATCH_PROVNET_PORT_PREFIX + 'fake-port' 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 # Invoke the method @@ -263,26 +262,27 @@ class TestOVS(test_base.TestCase): expected_flow_v6 = '{},ipv6,in_port={},dl_src:{}'.format( self.cookie_id, ovs_port_iface, self.mac) - expected_calls = [ - mock.call('ovs-vsctl', ['list-ports', self.bridge]), - mock.call('ovs-ofctl', ['del-flows', self.bridge, expected_flow]), - mock.call('ovs-ofctl', ['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_cmd.call_count) + vsctl_expected_calls = [ + mock.call(['list-ports', self.bridge])] + ofctl_expected_calls = [ + mock.call(['del-flows', self.bridge, expected_flow]), + mock.call(['del-flows', self.bridge, expected_flow_v6])] + + self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls) + self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls) + mock_ofport.assert_called_once_with(ovs_port) def test_remove_evpn_router_ovs_flows_no_ovs_port(self): 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( self.bridge, self.cookie, self.mac) self.assertIsNone(ret) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', ['list-ports', self.bridge]) + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with( + ['list-ports', self.bridge]) @mock.patch.object(ovs_utils, 'get_device_port_at_ovs') @mock.patch.object(linux_net, 'get_ip_version') @@ -293,7 +293,8 @@ class TestOVS(test_base.TestCase): net = 'fake-net' mock_ip_version.return_value = ip_version 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( 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( self.cookie_id, ovs_port_iface, self.mac, net)) - expected_calls = [ - mock.call('ovs-vsctl', ['list-ports', self.bridge]), - mock.call('ovs-ofctl', ['del-flows', self.bridge, expected_flow])] - self.mock_ovs_vsctl.ovs_cmd.assert_has_calls(expected_calls) - self.assertEqual(len(expected_calls), - self.mock_ovs_vsctl.ovs_cmd.call_count) + vsctl_expected_calls = [ + mock.call(['list-ports', self.bridge])] + ofctl_expected_calls = [ + mock.call(['del-flows', self.bridge, expected_flow])] + self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls) + self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls) mock_ip_version.assert_called_once_with(net) 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): 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( self.bridge, self.cookie, self.mac, 'fake-net') - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', ['list-ports', self.bridge]) + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with( + ['list-ports', self.bridge]) def _test_add_device_to_ovs_bridge(self, vlan_tag=False): device = 'ethX' @@ -341,8 +342,7 @@ class TestOVS(test_base.TestCase): if vlan_tag: expected_args.append('tag=%s' % vtag) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', expected_args) + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(expected_args) def test_add_device_to_ovs_bridge(self): self._test_add_device_to_ovs_bridge() @@ -361,8 +361,7 @@ class TestOVS(test_base.TestCase): expected_args.append(br) expected_args.append(device) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-vsctl', expected_args) + self.mock_ovs_vsctl.ovs_vsctl.assert_called_once_with(expected_args) def test_del_device_from_ovs_bridge(self): 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,' 'nw_src=20.0.0.0/24'.format(self.cookie_id)) - self.mock_ovs_vsctl.ovs_cmd.assert_called_once_with( - 'ovs-ofctl', ['--strict', 'del-flows', self.bridge, expected_flow]) + self.mock_ovs_vsctl.ovs_ofctl.assert_called_once_with( + ['--strict', 'del-flows', self.bridge, expected_flow]) def test_get_flow_info(self): flow = ('cookie=0x3e6, duration=11.647s, table=0, n_packets=0, ' diff --git a/ovn_bgp_agent/tests/unit/privileged/test_ovs_vsctl.py b/ovn_bgp_agent/tests/unit/privileged/test_ovs_vsctl.py index 6697a452..6eb977cf 100644 --- a/ovn_bgp_agent/tests/unit/privileged/test_ovs_vsctl.py +++ b/ovn_bgp_agent/tests/unit/privileged/test_ovs_vsctl.py @@ -38,47 +38,68 @@ class TestPrivilegedOvsVsctl(test_base.TestCase): # Mock processutils.execute() self.mock_exc = mock.patch.object(processutils, 'execute').start() - def test_ovs_cmd(self): - ovs_vsctl.ovs_cmd( - 'ovs-vsctl', ['--if-exists', 'del-port', 'fake-port']) + def test_ovs_vsctl(self): + ovs_vsctl.ovs_vsctl( + ['--if-exists', 'del-port', 'fake-port']) self.mock_exc.assert_called_once_with( 'ovs-vsctl', '--if-exists', 'del-port', 'fake-port') - def test_ovs_cmd_timeout(self): - ovs_vsctl.ovs_cmd( - 'ovs-vsctl', ['--if-exists', 'del-port', 'fake-port'], timeout=10) + def test_ovs_ofctl(self): + ovs_vsctl.ovs_ofctl( + ['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( 'ovs-vsctl', '--timeout=10', '--if-exists', 'del-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 = ( processutils.ProcessExecutionError(), None) - ovs_vsctl.ovs_cmd( - 'ovs-vsctl', ['--if-exists', 'del-port', 'fake-port']) + ovs_vsctl.ovs_ofctl( + ['--strict', 'del-flows', 'br-ex', 'dummy-flow']) - calls = [mock.call('ovs-vsctl', '--if-exists', 'del-port', - 'fake-port'), - mock.call('ovs-vsctl', '--if-exists', 'del-port', - 'fake-port', '-O', 'OpenFlow13')] + calls = [mock.call('ovs-ofctl', '--strict', 'del-flows', + 'br-ex', 'dummy-flow'), + mock.call('ovs-ofctl', '--strict', 'del-flows', + 'br-ex', 'dummy-flow', '-O', 'OpenFlow13')] self.mock_exc.assert_has_calls(calls) - def test_ovs_cmd_exception(self): - self.mock_exc.side_effect = FakeException() + def test_ovs_vsctl_process_execution_error_no_fallback(self): + # fallback does not apply to ovs-vsctl command + self.mock_exc.side_effect = processutils.ProcessExecutionError() self.assertRaises( - FakeException, ovs_vsctl.ovs_cmd, 'ovs-vsctl', + processutils.ProcessExecutionError, ovs_vsctl.ovs_vsctl, ['--if-exists', 'del-port', 'fake-port']) self.mock_exc.assert_called_once_with( '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): self.mock_exc.side_effect = ( processutils.ProcessExecutionError(), FakeException()) self.assertRaises( - FakeException, ovs_vsctl.ovs_cmd, 'ovs-vsctl', - ['--if-exists', 'del-port', 'fake-port']) - calls = [mock.call('ovs-vsctl', '--if-exists', 'del-port', - 'fake-port'), - mock.call('ovs-vsctl', '--if-exists', 'del-port', - 'fake-port', '-O', 'OpenFlow13')] + FakeException, ovs_vsctl.ovs_ofctl, + ['add-flow', 'br-ex', 'dummy-flow']) + calls = [mock.call('ovs-ofctl', 'add-flow', 'br-ex', 'dummy-flow'), + mock.call('ovs-ofctl', 'add-flow', 'br-ex', 'dummy-flow', + '-O', 'OpenFlow13')] self.mock_exc.assert_has_calls(calls)