enable OVSDB native interface by default

- unit tests were fixed mainly by mocking
  Connection class of native implementation.

- some ovs-lib tests rely on direct ovs-vsctl
  output. Temporarily decorated with @vsctl_only.

UpgradeImpact

Change-Id: I2632b0e21edd61536867a9fc830a45d9899091e4
This commit is contained in:
Inessa Vasilevskaya 2016-03-31 01:09:39 +03:00
parent accb03d0ff
commit bdeb7bcc2b
15 changed files with 172 additions and 74 deletions

View File

@ -30,7 +30,7 @@ interface_map = {
OPTS = [
cfg.StrOpt('ovsdb_interface',
choices=interface_map.keys(),
default='vsctl',
default='native',
help=_('The interface for interacting with the OVSDB')),
cfg.StrOpt('ovsdb_connection',
default='tcp:127.0.0.1:6640',

View File

@ -20,6 +20,7 @@ from oslo_serialization import jsonutils
from oslo_utils import uuidutils
import testtools
from neutron.agent.common import config
from neutron.agent.common import ovs_lib
from neutron.agent.common import utils
from neutron.plugins.common import constants
@ -59,6 +60,16 @@ class OFCTLParamListMatcher(object):
__repr__ = __str__
def vsctl_only(f):
# NOTE(ivasilevskaya) as long as some tests rely heavily on mocking
# direct vsctl commands, need to ensure that ovsdb_interface = 'vsctl'
# TODO(ivasilevskaya) introduce alternative tests for native interface?
def wrapper(*args, **kwargs):
config.cfg.CONF.set_override("ovsdb_interface", "vsctl", group="OVS")
return f(*args, **kwargs)
return wrapper
class OVS_Lib_Test(base.BaseTestCase):
"""A test suite to exercise the OVS libraries shared by Neutron agents.
@ -66,6 +77,7 @@ class OVS_Lib_Test(base.BaseTestCase):
can run on any system. That does, however, limit their scope.
"""
@vsctl_only
def setUp(self):
super(OVS_Lib_Test, self).setUp()
self.BR_NAME = "br-int"
@ -906,11 +918,13 @@ class TestDeferredOVSBridge(base.BaseTestCase):
with ovs_lib.DeferredOVSBridge(self.br) as deferred_br:
self.assertRaises(AttributeError, getattr, deferred_br, 'failure')
@vsctl_only
def test_default_cookie(self):
self.br = ovs_lib.OVSBridge("br-tun")
uuid_stamp1 = self.br.default_cookie
self.assertEqual(uuid_stamp1, self.br.default_cookie)
@vsctl_only
def test_cookie_passed_to_addmod(self):
self.br = ovs_lib.OVSBridge("br-tun")
stamp = str(self.br.default_cookie)

View File

@ -137,6 +137,10 @@ class QosExtensionBaseTestCase(base.BaseTestCase):
def setUp(self):
super(QosExtensionBaseTestCase, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.qos_ext = qos.QosAgentExtension()
self.context = context.get_admin_context()
self.connection = mock.Mock()

View File

@ -393,42 +393,46 @@ class TestOVSInterfaceDriver(TestBase):
def _test_plug(self, additional_expectation=None, bridge=None,
namespace=None):
additional_expectation = additional_expectation or []
if not bridge:
bridge = 'br-int'
with mock.patch('neutron.agent.ovsdb.native.connection.'
'Connection.start'):
additional_expectation = additional_expectation or []
if not bridge:
bridge = 'br-int'
def device_exists(dev, namespace=None):
return dev == bridge
def device_exists(dev, namespace=None):
return dev == bridge
with mock.patch.object(ovs_lib.OVSBridge, 'replace_port') as replace:
ovs = interface.OVSInterfaceDriver(self.conf)
self.device_exists.side_effect = device_exists
ovs.plug('01234567-1234-1234-99',
'port-1234',
'tap0',
'aa:bb:cc:dd:ee:ff',
bridge=bridge,
namespace=namespace)
replace.assert_called_once_with(
'tap0',
('type', 'internal'),
('external_ids', {
'iface-id': 'port-1234',
'iface-status': 'active',
'attached-mac': 'aa:bb:cc:dd:ee:ff'}))
with mock.patch.object(ovs_lib.OVSBridge,
'replace_port') as replace:
ovs = interface.OVSInterfaceDriver(self.conf)
self.device_exists.side_effect = device_exists
ovs.plug('01234567-1234-1234-99',
'port-1234',
'tap0',
'aa:bb:cc:dd:ee:ff',
bridge=bridge,
namespace=namespace)
replace.assert_called_once_with(
'tap0',
('type', 'internal'),
('external_ids', {
'iface-id': 'port-1234',
'iface-status': 'active',
'attached-mac': 'aa:bb:cc:dd:ee:ff'}))
expected = [mock.call(),
mock.call().device('tap0'),
mock.call().device().link.set_address('aa:bb:cc:dd:ee:ff')]
expected.extend(additional_expectation)
if namespace:
expected.extend(
[mock.call().ensure_namespace(namespace),
mock.call().ensure_namespace().add_device_to_namespace(
mock.ANY)])
expected.extend([mock.call().device().link.set_up()])
expected = [
mock.call(),
mock.call().device('tap0'),
mock.call().device().link.set_address('aa:bb:cc:dd:ee:ff')]
expected.extend(additional_expectation)
if namespace:
expected.extend(
[mock.call().ensure_namespace(namespace),
mock.call().ensure_namespace().add_device_to_namespace(
mock.ANY)])
expected.extend([mock.call().device().link.set_up()])
self.ip.assert_has_calls(expected)
self.ip.assert_has_calls(expected)
def test_mtu_int(self):
self.assertIsNone(self.conf.network_device_mtu)
@ -472,49 +476,52 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):
def _test_plug(self, devname=None, bridge=None, namespace=None,
prefix=None, mtu=None):
with mock.patch('neutron.agent.ovsdb.native.connection.'
'Connection.start'):
if not devname:
devname = 'ns-0'
if not bridge:
bridge = 'br-int'
if not devname:
devname = 'ns-0'
if not bridge:
bridge = 'br-int'
def device_exists(dev, namespace=None):
return dev == bridge
def device_exists(dev, namespace=None):
return dev == bridge
ovs = interface.OVSInterfaceDriver(self.conf)
self.device_exists.side_effect = device_exists
ovs = interface.OVSInterfaceDriver(self.conf)
self.device_exists.side_effect = device_exists
root_dev = mock.Mock()
ns_dev = mock.Mock()
self.ip().add_veth = mock.Mock(return_value=(root_dev, ns_dev))
expected = [mock.call(),
mock.call().add_veth('tap0', devname,
namespace2=namespace)]
root_dev = mock.Mock()
ns_dev = mock.Mock()
self.ip().add_veth = mock.Mock(return_value=(root_dev, ns_dev))
expected = [mock.call(),
mock.call().add_veth('tap0', devname,
namespace2=namespace)]
with mock.patch.object(ovs_lib.OVSBridge, 'replace_port') as replace:
ovs.plug('01234567-1234-1234-99',
'port-1234',
devname,
'aa:bb:cc:dd:ee:ff',
bridge=bridge,
namespace=namespace,
prefix=prefix)
replace.assert_called_once_with(
'tap0',
('external_ids', {
'iface-id': 'port-1234',
'iface-status': 'active',
'attached-mac': 'aa:bb:cc:dd:ee:ff'}))
with mock.patch.object(ovs_lib.OVSBridge,
'replace_port') as replace:
ovs.plug('01234567-1234-1234-99',
'port-1234',
devname,
'aa:bb:cc:dd:ee:ff',
bridge=bridge,
namespace=namespace,
prefix=prefix)
replace.assert_called_once_with(
'tap0',
('external_ids', {
'iface-id': 'port-1234',
'iface-status': 'active',
'attached-mac': 'aa:bb:cc:dd:ee:ff'}))
ns_dev.assert_has_calls(
[mock.call.link.set_address('aa:bb:cc:dd:ee:ff')])
if mtu:
ns_dev.assert_has_calls([mock.call.link.set_mtu(mtu)])
root_dev.assert_has_calls([mock.call.link.set_mtu(mtu)])
ns_dev.assert_has_calls(
[mock.call.link.set_address('aa:bb:cc:dd:ee:ff')])
if mtu:
ns_dev.assert_has_calls([mock.call.link.set_mtu(mtu)])
root_dev.assert_has_calls([mock.call.link.set_mtu(mtu)])
self.ip.assert_has_calls(expected)
root_dev.assert_has_calls([mock.call.link.set_up()])
ns_dev.assert_has_calls([mock.call.link.set_up()])
self.ip.assert_has_calls(expected)
root_dev.assert_has_calls([mock.call.link.set_up()])
ns_dev.assert_has_calls([mock.call.link.set_up()])
def test_plug_mtu(self):
self.conf.set_override('network_device_mtu', 9000)

View File

@ -20,6 +20,12 @@ from neutron.tests import base
class TestNetnsCleanup(base.BaseTestCase):
def setUp(self):
super(TestNetnsCleanup, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
def test_kill_dhcp(self, dhcp_active=True):
conf = mock.Mock()

View File

@ -26,6 +26,7 @@ from neutron.tests import base
class TestOVSCleanup(base.BaseTestCase):
@mock.patch('neutron.agent.ovsdb.native.connection.Connection.start')
@mock.patch('neutron.common.config.setup_logging')
@mock.patch('neutron.cmd.ovs_cleanup.setup_conf')
@mock.patch('neutron.agent.common.ovs_lib.BaseOVS.get_bridges')
@ -33,7 +34,7 @@ class TestOVSCleanup(base.BaseTestCase):
@mock.patch.object(util, 'collect_neutron_ports')
@mock.patch.object(util, 'delete_neutron_ports')
def test_main(self, mock_delete, mock_collect, mock_ovs,
mock_get_bridges, mock_conf, mock_logging):
mock_get_bridges, mock_conf, mock_logging, mock_conn):
bridges = ['br-int', 'br-ex']
ports = ['p1', 'p2', 'p3']
conf = mock.Mock()

View File

@ -32,6 +32,10 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
def setUp(self):
super(QosOVSAgentDriverTestCase, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.context = context.get_admin_context()
self.qos_driver = qos_driver.QosOVSAgentDriver()
self.agent_api = ovs_ext_api.OVSAgentExtensionAPI(

View File

@ -32,6 +32,10 @@ class OVSPhysicalBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase,
def setUp(self):
super(OVSPhysicalBridgeTest, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.setup_bridge_mock('br-phys', self.br_phys_cls)
self.stamp = self.br.default_cookie

View File

@ -31,7 +31,20 @@ class OVSTunnelBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase,
dvr_process_next_table_id = ovs_const.PATCH_LV_TO_TUN
def setUp(self):
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
super(OVSTunnelBridgeTest, self).setUp()
# NOTE(ivasilevskaya) The behaviour of oslotest.base.addCleanup()
# according to https://review.openstack.org/#/c/119201/4 guarantees
# that all started mocks will be stopped even without direct call to
# patcher.stop().
# If any individual mocks should be stopped by other than default
# mechanism, their cleanup has to be added after
# oslotest.BaseTestCase.setUp() not to be included in the stopall set
# that will be cleaned up by mock.patch.stopall. This way the mock
# won't be attempted to be stopped twice.
self.addCleanup(conn_patcher.stop)
self.setup_bridge_mock('br-tun', self.br_tun_cls)
self.stamp = self.br.default_cookie

View File

@ -13,6 +13,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import mock
from neutron.agent.common import ovs_lib
from neutron.plugins.ml2.drivers.openvswitch.agent.openflow.ovs_ofctl \
import ovs_bridge
@ -23,6 +25,10 @@ class TestBRCookieOpenflow(base.BaseTestCase):
def setUp(self):
super(TestBRCookieOpenflow, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.br = ovs_bridge.OVSAgentBridge('br-int')
def test_reserved_cookies(self):

View File

@ -41,6 +41,10 @@ class OVSAgentConfigTestBase(base.BaseTestCase):
class OVSAgentTestBase(OVSAgentConfigTestBase):
def setUp(self):
super(OVSAgentTestBase, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.br_int_cls = importutils.import_class(self._BR_INT_CLASS)
self.br_phys_cls = importutils.import_class(self._BR_PHYS_CLASS)
self.br_tun_cls = importutils.import_class(self._BR_TUN_CLASS)

View File

@ -28,6 +28,10 @@ class TestOVSAgentExtensionAPI(base.BaseTestCase):
def setUp(self):
super(base.BaseTestCase, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.br_int = ovs_bridge.OVSAgentBridge("br-int")
self.br_tun = ovs_bridge.OVSAgentBridge("br-tun")
@ -61,6 +65,10 @@ class TestOVSCookieBridge(base.DietTestCase):
def setUp(self):
super(TestOVSCookieBridge, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.bridge = ovs_bridge.OVSAgentBridge("br-foo")
self.bridge.do_action_flows = mock.Mock()
self.tested_bridge = ovs_ext_agt.OVSCookieBridge(self.bridge)
@ -146,6 +154,10 @@ class TestOVSDeferredCookieBridge(base.DietTestCase):
def setUp(self):
super(TestOVSDeferredCookieBridge, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
self.bridge = ovs_bridge.OVSAgentBridge("br-foo")
self.bridge.do_action_flows = mock.Mock()
self.cookie_bridge = ovs_ext_agt.OVSCookieBridge(self.bridge)

View File

@ -1161,9 +1161,9 @@ class TestOvsNeutronAgent(object):
def _test_setup_physical_bridges(self, port_exists=False):
with mock.patch.object(ip_lib.IPDevice, "exists") as devex_fn,\
mock.patch.object(sys, "exit"),\
mock.patch.object(utils, "execute"),\
mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
mock.patch.object(self.agent, 'int_br') as int_br:
mock.patch.object(self.agent, 'int_br') as int_br,\
mock.patch.object(ovs_lib.BaseOVS, 'get_bridges'):
devex_fn.return_value = True
parent = mock.MagicMock()
phys_br = phys_br_cls()
@ -1274,11 +1274,11 @@ class TestOvsNeutronAgent(object):
def _test_setup_physical_bridges_change_from_veth_to_patch_conf(
self, port_exists=False):
with mock.patch.object(sys, "exit"),\
mock.patch.object(utils, "execute"),\
mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
mock.patch.object(self.agent, 'int_br') as int_br,\
mock.patch.object(self.agent.int_br, 'db_get_val',
return_value='veth'):
return_value='veth'),\
mock.patch.object(ovs_lib.BaseOVS, 'get_bridges'):
phys_br = phys_br_cls()
parent = mock.MagicMock()
parent.attach_mock(phys_br_cls, 'phys_br_cls')
@ -2118,6 +2118,10 @@ class AncillaryBridgesTest(object):
def setUp(self):
super(AncillaryBridgesTest, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
notifier_p = mock.patch(NOTIFIER)
notifier_cls = notifier_p.start()
self.notifier = mock.Mock()

View File

@ -74,6 +74,10 @@ class TunnelTest(object):
def setUp(self):
super(TunnelTest, self).setUp()
conn_patcher = mock.patch(
'neutron.agent.ovsdb.native.connection.Connection.start')
conn_patcher.start()
self.addCleanup(conn_patcher.stop)
cfg.CONF.set_default('firewall_driver',
'neutron.agent.firewall.NoopFirewallDriver',
group='SECURITYGROUP')

View File

@ -0,0 +1,15 @@
---
prelude: >
Prior to Newton, the default option for 'ovsdb_interface'
was 'vsctl'. In Newton 'ovsdb_interface' defaults to
'native'. This change switches the way of communication
with OVSDB from the ovs-vsctl tool to Open vSwitch python
api to improve out-of-the-box performance for typical
deployments.
upgrade:
- To keep the old default value use 'ovsdb_interface = vsctl'
in '[ovs]' section of openvswitch_agent.ini
(common path '/etc/neutron/plugins/ml2/openvswitch_agent.ini')
if there is a separate openvswitch agent configuration file;
otherwise apply changes mentioned above to ml2_conf.ini
(common path '/etc/neutron/plugins/ml2/ml2_conf.ini').