openstack/neutron
Code Issues Proposed changes

ovsdb: don't erase existing ovsdb managers

Browse Source 
The current existing agent erases already set ovsdb managers
entries. In some use cases, cloud admin sets ovsdb managers.
eg, for SDN controllers or monitoring purpose. Neutron agent
shouldn't unconditionally erase the existing ovsdb managers.

This patch implements a new api add_manager (along with
get_manager and remove_manager) to the ovsdb api which will
allow us to configure a manager on a switch without overriding
the existing managers.

Closes-Bug: #1614766
Change-Id: Ibf9bd02fac3070d166546cac478ef984e4e43f28
Co-Authored-By: sridhargaddam <sgaddam@redhat.com>
Co-Authored-By: Terry Wilson <twilson@redhat.com>
This commit is contained in:
Isaku Yamahata 2016-08-18 15:32:12 -07:00 committed by Sridhar Gaddam
parent efe87156c5
commit 7d42176853
10 changed files with 160 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
neutron/agent/common/ovs_lib.py
View File

@ -106,6 +106,15 @@ class BaseOVS(object):
self.vsctl_timeout = cfg.CONF.ovs_vsctl_timeout
self.ovsdb = ovsdb.API.get(self)
def add_manager(self, connection_uri):
self.ovsdb.add_manager(connection_uri).execute()
def get_manager(self):
return self.ovsdb.get_manager().execute()
def remove_manager(self, connection_uri):
self.ovsdb.remove_manager(connection_uri).execute()
def add_bridge(self, bridge_name,
datapath_type=constants.OVS_DATAPATH_SYSTEM):

30
neutron/agent/ovsdb/api.py
View File

@ -100,6 +100,36 @@ class API(object):
:rtype: :class:`Transaction`
"""
@abc.abstractmethod
def add_manager(self, connection_uri):
"""Create a command to add a Manager to the OVS switch
This API will add a new manager without overriding the existing ones.
:param connection_uri: target to which manager needs to be set
:type connection_uri: string, see ovs-vsctl manpage for format
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def get_manager(self):
"""Create a command to get Manager list from the OVS switch
:returns: :class:`Command` with list of Manager names result
"""
@abc.abstractmethod
def remove_manager(self, connection_uri):
"""Create a command to remove a Manager from the OVS switch
This API will remove the manager configured on the OVS switch.
:param connection_uri: target identifying the manager uri that
needs to be removed.
:type connection_uri: string, see ovs-vsctl manpage for format
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def add_br(self, name, may_exist=True, datapath_type=None):
"""Create a command to add an OVS bridge

9
neutron/agent/ovsdb/impl_idl.py
View File

@ -211,6 +211,15 @@ class OvsdbIdl(api.API):
self.context.vsctl_timeout,
check_error, log_errors)
def add_manager(self, connection_uri):
return cmd.AddManagerCommand(self, connection_uri)
def get_manager(self):
return cmd.GetManagerCommand(self)
def remove_manager(self, connection_uri):
return cmd.RemoveManagerCommand(self, connection_uri)
def add_br(self, name, may_exist=True, datapath_type=None):
return cmd.AddBridgeCommand(self, name, may_exist, datapath_type)

15
neutron/agent/ovsdb/impl_vsctl.py
View File

@ -180,6 +180,21 @@ class OvsdbVsctl(ovsdb.API):
def transaction(self, check_error=False, log_errors=True, **kwargs):
return Transaction(self.context, check_error, log_errors, **kwargs)
def add_manager(self, connection_uri):
# This will add a new manager without overriding existing ones.
conn_uri = 'target="%s"' % connection_uri
args = ['create', 'Manager', conn_uri, '--', 'add', 'Open_vSwitch',
'.', 'manager_options', '@manager']
return BaseCommand(self.context, '--id=@manager', args=args)
def get_manager(self):
return MultiLineCommand(self.context, 'get-manager')
def remove_manager(self, connection_uri):
args = ['get', 'Manager', connection_uri, '--', 'remove',
'Open_vSwitch', '.', 'manager_options', '@manager']
return BaseCommand(self.context, '--id=@manager', args=args)
def add_br(self, name, may_exist=True, datapath_type=None):
opts = ['--may-exist'] if may_exist else None
params = [name]

41
neutron/agent/ovsdb/native/commands.py
View File

@ -55,6 +55,47 @@ class BaseCommand(api.Command):
__repr__ = __str__
class AddManagerCommand(BaseCommand):
def __init__(self, api, target):
super(AddManagerCommand, self).__init__(api)
self.target = target
def run_idl(self, txn):
row = txn.insert(self.api._tables['Manager'])
row.target = self.target
self.api._ovs.verify('manager_options')
self.api._ovs.manager_options = self.api._ovs.manager_options + [row]
class GetManagerCommand(BaseCommand):
def __init__(self, api):
super(GetManagerCommand, self).__init__(api)
def run_idl(self, txn):
self.result = [m.target for m in
self.api._tables['Manager'].rows.values()]
class RemoveManagerCommand(BaseCommand):
def __init__(self, api, target):
super(RemoveManagerCommand, self).__init__(api)
self.target = target
def run_idl(self, txn):
try:
manager = idlutils.row_by_value(self.api.idl, 'Manager', 'target',
self.target)
except idlutils.RowNotFound:
msg = _("Manager with target %s does not exist") % self.target
LOG.error(msg)
raise RuntimeError(msg)
self.api._ovs.verify('manager_options')
manager_list = self.api._ovs.manager_options
manager_list.remove(manager)
self.api._ovs.manager_options = manager_list
self.api._tables['Manager'].rows[manager.uuid].delete()
class AddBridgeCommand(BaseCommand):
def __init__(self, api, name, may_exist, datapath_type):
super(AddBridgeCommand, self).__init__(api)

12
neutron/agent/ovsdb/native/helpers.py
View File

@ -12,7 +12,11 @@
# License for the specific language governing permissions and limitations
# under the License.
from neutron.agent.common import utils
from oslo_config import cfg
from neutron.agent.ovsdb import api as ovsdb
cfg.CONF.import_opt('ovs_vsctl_timeout', 'neutron.agent.common.ovs_lib')
def _connection_to_manager_uri(conn_uri):
@ -25,5 +29,9 @@ def _connection_to_manager_uri(conn_uri):
def enable_connection_uri(conn_uri):
class OvsdbVsctlContext(object):
vsctl_timeout = cfg.CONF.ovs_vsctl_timeout
manager_uri = _connection_to_manager_uri(conn_uri)
utils.execute(['ovs-vsctl', 'set-manager', manager_uri], run_as_root=True)
api = ovsdb.API.get(OvsdbVsctlContext, 'vsctl')
api.add_manager(manager_uri).execute(check_error=False, log_errors=True)

5
neutron/tests/common/exclusive_resources/port.py
View File

@ -25,10 +25,11 @@ class ExclusivePort(resource_allocator.ExclusiveResource):
:type port: int
"""
def __init__(self, protocol):
def __init__(self, protocol, start=1024, end=None):
super(ExclusivePort, self).__init__(
'ports',
functools.partial(net_helpers.get_free_namespace_port, protocol))
functools.partial(net_helpers.get_free_namespace_port, protocol,
start=start, end=end))
def _setUp(self):
super(ExclusivePort, self)._setUp()

11
neutron/tests/common/net_helpers.py
View File

@ -74,6 +74,9 @@ CHILD_PROCESS_SLEEP = os.environ.get('OS_TEST_CHILD_PROCESS_SLEEP', 0.5)
TRANSPORT_PROTOCOLS = (n_const.PROTO_NAME_TCP, n_const.PROTO_NAME_UDP)
OVS_MANAGER_TEST_PORT_FIRST = 6610
OVS_MANAGER_TEST_PORT_LAST = 6639
def increment_ip_cidr(ip_cidr, offset=1):
"""Increment ip_cidr offset times.
@ -189,7 +192,7 @@ def get_unused_port(used, start=1024, end=None):
return random.choice(list(candidates - used))
def get_free_namespace_port(protocol, namespace=None):
def get_free_namespace_port(protocol, namespace=None, start=1024, end=None):
"""Return an unused port from given namespace
WARNING: This function returns a port that is free at the execution time of
@ -199,6 +202,10 @@ def get_free_namespace_port(protocol, namespace=None):
:param protocol: Return free port for given protocol. Supported protocols
are 'tcp' and 'udp'.
:param namespace: Namespace in which free port has to be returned.
:param start: The starting port number.
:param end: The ending port number (free port that is returned would be
between (start, end) values.
"""
if protocol == n_const.PROTO_NAME_TCP:
param = '-tna'
@ -211,7 +218,7 @@ def get_free_namespace_port(protocol, namespace=None):
output = ip_wrapper.netns.execute(['ss', param])
used_ports = _get_source_ports_from_ss_output(output)
return get_unused_port(used_ports)
return get_unused_port(used_ports, start, end)
def create_patch_ports(source, destination):

34
neutron/tests/functional/agent/test_ovs_lib.py
View File

@ -17,10 +17,12 @@ import collections
import uuid
import mock
from neutron_lib import constants as const
from neutron.agent.common import ovs_lib
from neutron.agent.linux import ip_lib
from neutron.common import utils
from neutron.tests.common.exclusive_resources import port
from neutron.tests.common import net_helpers
from neutron.tests.functional.agent.linux import base
@ -423,6 +425,38 @@ class OVSLibTestCase(base.BaseOVSLinuxTestCase):
super(OVSLibTestCase, self).setUp()
self.ovs = ovs_lib.BaseOVS()
def test_add_manager_appends(self):
port1 = self.useFixture(port.ExclusivePort(const.PROTO_NAME_TCP,
start=net_helpers.OVS_MANAGER_TEST_PORT_FIRST,
end=net_helpers.OVS_MANAGER_TEST_PORT_LAST)).port
port2 = self.useFixture(port.ExclusivePort(const.PROTO_NAME_TCP,
start=net_helpers.OVS_MANAGER_TEST_PORT_FIRST,
end=net_helpers.OVS_MANAGER_TEST_PORT_LAST)).port
manager_list = ["ptcp:%s:127.0.0.1" % port1,
"ptcp:%s:127.0.0.1" % port2]
# Verify that add_manager does not override the existing manager
expected_manager_list = list()
for conn_uri in manager_list:
self.ovs.add_manager(conn_uri)
self.addCleanup(self.ovs.remove_manager, conn_uri)
self.assertIn(conn_uri, self.ovs.get_manager())
expected_manager_list.append(conn_uri)
# Verify that switch is configured with both the managers
for manager_uri in expected_manager_list:
self.assertIn(manager_uri, manager_list)
def test_add_manager_lifecycle_baseovs(self):
port1 = self.useFixture(port.ExclusivePort(const.PROTO_NAME_TCP,
start=net_helpers.OVS_MANAGER_TEST_PORT_FIRST,
end=net_helpers.OVS_MANAGER_TEST_PORT_LAST)).port
conn_uri = "ptcp:%s:127.0.0.1" % port1
self.addCleanup(self.ovs.remove_manager, conn_uri)
self.ovs.add_manager(conn_uri)
self.assertIn(conn_uri, self.ovs.get_manager())
self.ovs.remove_manager(conn_uri)
self.assertNotIn(conn_uri, self.ovs.get_manager())
def test_bridge_lifecycle_baseovs(self):
name = utils.get_rand_name(prefix=net_helpers.BR_PREFIX)
self.addCleanup(self.ovs.delete_bridge, name)

7
neutron/tests/unit/agent/ovsdb/native/test_helpers.py
View File

@ -34,10 +34,3 @@ class TestOVSNativeHelpers(base.BaseTestCase):
for conn_uri, expected in CONNECTION_TO_MANAGER_URI_MAP:
self.assertEqual(expected,
helpers._connection_to_manager_uri(conn_uri))
def test_enable_connection_uri(self):
for conn_uri, manager_uri in CONNECTION_TO_MANAGER_URI_MAP:
helpers.enable_connection_uri(conn_uri)
self.execute.assert_called_with(
['ovs-vsctl', 'set-manager', manager_uri],
run_as_root=True)