diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index d22b0897a3e..6261fabdd57 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -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): diff --git a/neutron/agent/ovsdb/api.py b/neutron/agent/ovsdb/api.py index 1c5b270d5de..a06431c5e9e 100644 --- a/neutron/agent/ovsdb/api.py +++ b/neutron/agent/ovsdb/api.py @@ -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 diff --git a/neutron/agent/ovsdb/impl_idl.py b/neutron/agent/ovsdb/impl_idl.py index daea1cdf3fd..8890b70b07f 100644 --- a/neutron/agent/ovsdb/impl_idl.py +++ b/neutron/agent/ovsdb/impl_idl.py @@ -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) diff --git a/neutron/agent/ovsdb/impl_vsctl.py b/neutron/agent/ovsdb/impl_vsctl.py index c4f7af7a1f6..490e43f67ca 100644 --- a/neutron/agent/ovsdb/impl_vsctl.py +++ b/neutron/agent/ovsdb/impl_vsctl.py @@ -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] diff --git a/neutron/agent/ovsdb/native/commands.py b/neutron/agent/ovsdb/native/commands.py index 8267fb59c06..0333c9f8ca7 100644 --- a/neutron/agent/ovsdb/native/commands.py +++ b/neutron/agent/ovsdb/native/commands.py @@ -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) diff --git a/neutron/agent/ovsdb/native/helpers.py b/neutron/agent/ovsdb/native/helpers.py index 0a4e42618cc..55a8481e9ab 100644 --- a/neutron/agent/ovsdb/native/helpers.py +++ b/neutron/agent/ovsdb/native/helpers.py @@ -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) diff --git a/neutron/tests/common/exclusive_resources/port.py b/neutron/tests/common/exclusive_resources/port.py index 191e0c3f910..af85eecb969 100644 --- a/neutron/tests/common/exclusive_resources/port.py +++ b/neutron/tests/common/exclusive_resources/port.py @@ -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() diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 4858d982f8f..6f8e3f9fa38 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -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): diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index b16130cc0e3..bfb7e326a6f 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -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) diff --git a/neutron/tests/unit/agent/ovsdb/native/test_helpers.py b/neutron/tests/unit/agent/ovsdb/native/test_helpers.py index 41495e5d159..c6b537dc02d 100644 --- a/neutron/tests/unit/agent/ovsdb/native/test_helpers.py +++ b/neutron/tests/unit/agent/ovsdb/native/test_helpers.py @@ -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)