From bfe3b679096e73015bae6592f926b26fa427f112 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 19 Mar 2015 12:43:21 -0500 Subject: [PATCH] Handle non-index lookups in native OVSDB backend ovs-vsctl get/set/clear/list can use a record_id that is not an index on the table being queried. For example, the Controller table can be queried by a bridge name. This patch implements the lookup table that ovs-vsctl uses to do these lookups. Change-Id: I1983c48c5839df016046ba2596c7c4affa1ebe00 Closes-Bug: 1435567 --- neutron/agent/ovsdb/native/commands.py | 74 ++++++++----------- neutron/agent/ovsdb/native/idlutils.py | 66 +++++++++++++++++ .../tests/functional/agent/test_ovs_lib.py | 10 +++ 3 files changed, 105 insertions(+), 45 deletions(-) diff --git a/neutron/agent/ovsdb/native/commands.py b/neutron/agent/ovsdb/native/commands.py index 5928e123c79..5be5847e991 100644 --- a/neutron/agent/ovsdb/native/commands.py +++ b/neutron/agent/ovsdb/native/commands.py @@ -19,16 +19,11 @@ from oslo_utils import excutils from neutron.agent.ovsdb import api from neutron.agent.ovsdb.native import idlutils -from neutron.common import exceptions from neutron.i18n import _LE LOG = logging.getLogger(__name__) -class RowNotFound(exceptions.NeutronException): - message = _("Table %(table)s has no row with %(col)s=%(match)s") - - class BaseCommand(api.Command): def __init__(self, api): self.api = api @@ -46,22 +41,6 @@ class BaseCommand(api.Command): if not check_error: ctx.reraise = False - def row_by_index(self, table, match, *default): - tab = self.api._tables[table] - idx = idlutils.get_index_column(tab) - return self.row_by_value(table, idx, match, *default) - - def row_by_value(self, table, column, match, *default): - tab = self.api._tables[table] - try: - return next(r for r in tab.rows.values() - if getattr(r, column) == match) - except StopIteration: - if len(default) == 1: - return default[0] - else: - raise RowNotFound(table=table, col=column, match=match) - def __str__(self): command_info = self.__dict__ return "%s(%s)" % ( @@ -78,7 +57,8 @@ class AddBridgeCommand(BaseCommand): def run_idl(self, txn): if self.may_exist: - br = self.row_by_value('Bridge', 'name', self.name, None) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', + self.name, None) if br: return row = txn.insert(self.api._tables['Bridge']) @@ -103,8 +83,9 @@ class DelBridgeCommand(BaseCommand): def run_idl(self, txn): try: - br = self.row_by_value('Bridge', 'name', self.name) - except RowNotFound: + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', + self.name) + except idlutils.RowNotFound: if self.if_exists: return else: @@ -128,8 +109,8 @@ class BridgeExistsCommand(BaseCommand): self.name = name def run_idl(self, txn): - self.result = bool(self.row_by_value('Bridge', - 'name', self.name, None)) + self.result = bool(idlutils.row_by_value(self.api.idl, 'Bridge', + 'name', self.name, None)) class ListBridgesCommand(BaseCommand): @@ -149,7 +130,7 @@ class BrGetExternalIdCommand(BaseCommand): self.field = field def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.name) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.name) self.result = br.external_ids[self.field] @@ -161,7 +142,7 @@ class BrSetExternalIdCommand(BaseCommand): self.value = value def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.name) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.name) external_ids = getattr(br, 'external_ids', {}) external_ids[self.field] = self.value br.external_ids = external_ids @@ -175,7 +156,7 @@ class DbSetCommand(BaseCommand): self.col_values = col_values def run_idl(self, txn): - record = self.row_by_index(self.table, self.record) + record = idlutils.row_by_record(self.api.idl, self.table, self.record) for col, val in self.col_values: # TODO(twilson) Ugh, the OVS library doesn't like OrderedDict # We're only using it to make a unit test work, so we should fix @@ -193,7 +174,7 @@ class DbClearCommand(BaseCommand): self.column = column def run_idl(self, txn): - record = self.row_by_index(self.table, self.record) + record = idlutils.row_by_record(self.api.idl, self.table, self.record) # Create an empty value of the column type value = type(getattr(record, self.column))() setattr(record, self.column, value) @@ -207,7 +188,7 @@ class DbGetCommand(BaseCommand): self.column = column def run_idl(self, txn): - record = self.row_by_index(self.table, self.record) + record = idlutils.row_by_record(self.api.idl, self.table, self.record) # TODO(twilson) This feels wrong, but ovs-vsctl returns single results # on set types without the list. The IDL is returning them as lists, # even if the set has the maximum number of items set to 1. Might be @@ -226,7 +207,7 @@ class SetControllerCommand(BaseCommand): self.targets = targets def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.bridge) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) controllers = [] for target in self.targets: controller = txn.insert(self.api._tables['Controller']) @@ -242,7 +223,7 @@ class DelControllerCommand(BaseCommand): self.bridge = bridge def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.bridge) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) br.controller = [] @@ -252,7 +233,7 @@ class GetControllerCommand(BaseCommand): self.bridge = bridge def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.bridge) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) br.verify('controller') self.result = [c.target for c in br.controller] @@ -264,7 +245,7 @@ class SetFailModeCommand(BaseCommand): self.mode = mode def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.bridge) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) br.verify('fail_mode') br.fail_mode = self.mode @@ -277,9 +258,10 @@ class AddPortCommand(BaseCommand): self.may_exist = may_exist def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.bridge) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) if self.may_exist: - port = self.row_by_value('Port', 'name', self.port, None) + port = idlutils.row_by_value(self.api.idl, 'Port', 'name', + self.port, None) if port: return port = txn.insert(self.api._tables['Port']) @@ -306,14 +288,16 @@ class DelPortCommand(BaseCommand): def run_idl(self, txn): try: - port = self.row_by_value('Port', 'name', self.port) - except RowNotFound: + port = idlutils.row_by_value(self.api.idl, 'Port', 'name', + self.port) + except idlutils.RowNotFound: if self.if_exists: return msg = _LE("Port %s does not exist") % self.port raise RuntimeError(msg) if self.bridge: - br = self.row_by_value('Bridge', 'name', self.bridge) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', + self.bridge) else: br = next(b for b in self.api._tables['Bridge'].rows.values() if port in b.ports) @@ -344,7 +328,7 @@ class ListPortsCommand(BaseCommand): self.bridge = bridge def run_idl(self, txn): - br = self.row_by_value('Bridge', 'name', self.bridge) + br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) self.result = [p.name for p in br.ports if p.name != self.bridge] @@ -359,7 +343,7 @@ class PortToBridgeCommand(BaseCommand): # name on the Port's (or Interface's for iface_to_br) external_id field # In fact, if we did that, the only place that uses to_br functions # could just add the external_id field to the conditions passed to find - port = self.row_by_value('Port', 'name', self.name) + port = idlutils.row_by_value(self.api.idl, 'Port', 'name', self.name) bridges = self.api._tables['Bridge'].rows.values() self.result = next(br.name for br in bridges if port in br.ports) @@ -370,10 +354,10 @@ class DbListCommand(BaseCommand): self.table = self.api._tables[table] self.columns = columns or self.table.columns.keys() + ['_uuid'] self.if_exists = if_exists - idx = idlutils.get_index_column(self.table) if records: - self.records = [uuid for uuid, row in self.table.rows.items() - if getattr(row, idx) in records] + self.records = [ + idlutils.row_by_record(self.api.idl, table, record).uuid + for record in records] else: self.records = self.table.rows.keys() diff --git a/neutron/agent/ovsdb/native/idlutils.py b/neutron/agent/ovsdb/native/idlutils.py index 89f32245395..7b5e746917a 100644 --- a/neutron/agent/ovsdb/native/idlutils.py +++ b/neutron/agent/ovsdb/native/idlutils.py @@ -12,14 +12,80 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import os import time +import uuid from ovs.db import idl from ovs import jsonrpc from ovs import poller from ovs import stream +from neutron.common import exceptions + + +RowLookup = collections.namedtuple('RowLookup', + ['table', 'column', 'uuid_column']) + +# Tables with no index in OVSDB and special record lookup rules +_LOOKUP_TABLE = { + 'Controller': RowLookup('Bridge', 'name', 'controller'), + 'Flow_Table': RowLookup('Flow_Table', 'name', None), + 'IPFIX': RowLookup('Bridge', 'name', 'ipfix'), + 'Mirror': RowLookup('Mirror', 'name', None), + 'NetFlow': RowLookup('Bridge', 'name', 'netflow'), + 'QoS': RowLookup('Port', 'name', 'qos'), + 'Queue': RowLookup(None, None, None), + 'sFlow': RowLookup('Bridge', 'name', 'sflow'), + 'SSL': RowLookup('Open_vSwitch', None, 'ssl'), +} + +_NO_DEFAULT = object() + + +class RowNotFound(exceptions.NeutronException): + message = _("Cannot find %(table)s with %(col)s=%(match)s") + + +def row_by_value(idl_, table, column, match, default=_NO_DEFAULT): + """Lookup an IDL row in a table by column/value""" + tab = idl_.tables[table] + for r in tab.rows.values(): + if getattr(r, column) == match: + return r + if default is not _NO_DEFAULT: + return default + raise RowNotFound(table=table, col=column, match=match) + + +def row_by_record(idl_, table, record): + t = idl_.tables[table] + try: + if isinstance(record, uuid.UUID): + return t.rows[record] + uuid_ = uuid.UUID(record) + return t.rows[uuid_] + except ValueError: + # Not a UUID string, continue lookup by other means + pass + except KeyError: + raise RowNotFound(table=table, col='uuid', match=record) + + rl = _LOOKUP_TABLE.get(table, RowLookup(table, get_index_column(t), None)) + # no table means uuid only, no column is just SSL which we don't need + if rl.table is None: + raise ValueError(_("Table %s can only be queried by UUID") % table) + if rl.column is None: + raise NotImplementedError(_("'.' searches are not implemented")) + row = row_by_value(idl_, rl.table, rl.column, record) + if rl.uuid_column: + rows = getattr(row, rl.uuid_column) + if len(rows) != 1: + raise RowNotFound(table=table, col=_('record'), match=record) + row = rows[0] + return row + def get_schema_helper(connection): err, strm = stream.Stream.open_block( diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index ad5b1700433..45055b24c14 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -102,6 +102,16 @@ class OVSBridgeTestCase(base.BaseOVSLinuxTestCase): self.br.del_controller() self.assertEqual([], self.br.get_controller()) + def test_non_index_queries(self): + controllers = ['tcp:127.0.0.1:6633'] + self.br.set_controller(controllers) + cmd = self.br.ovsdb.db_set('Controller', self.br.br_name, + ('connection_mode', 'out-of-band')) + cmd.execute(check_error=True) + self.assertEqual('out-of-band', + self.br.db_get_val('Controller', self.br.br_name, + 'connection_mode')) + def test_set_fail_mode(self): self.br.set_secure_mode() self._assert_br_fail_mode(ovs_lib.FAILMODE_SECURE)