From 1f4f806584585e4bed02ec7ef784f57e11b05994 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Sat, 20 Aug 2016 00:24:15 -0500 Subject: [PATCH] Handle uuid references within an ovsdb transaction Enable the OVSDB API implementations to handle referencing a newly created object within the same transaction. For vsctl, this is via the --id=@name construct. For native, this is via storing the created row as the result, then replacing the result when the transaction completes. This uses an api.Command object passed as part of a column value in a db_set/db_create operation as a reference to that object in a transaction. For example, do: with self.ovsdb.transaction() as txn: queue = txn.add(self.ovsdb.db_create("Queue", ...)) qos = txn.add(self.ovsdb.db_create("QoS", queues={0: queue})) port = txn.add(self.ovsdb.db_set("Port", pname, ('qos', qos))) instead of having to do 5 separate transactions to: create a queue, find the queue, create the QoS entry, find the QoS entry, and finally to update the port with the QoS entry. Change-Id: I1781794958af1483dabc0f5d17f2df6fea828564 Closes-Bug: #1615105 --- neutron/agent/ovsdb/api.py | 3 +- neutron/agent/ovsdb/impl_idl.py | 4 +- neutron/agent/ovsdb/impl_vsctl.py | 19 +++++++- neutron/agent/ovsdb/native/commands.py | 12 ++++- neutron/agent/ovsdb/native/idlutils.py | 27 ++++++++++++ .../tests/functional/agent/test_ovs_lib.py | 18 ++++++++ .../unit/agent/ovsdb/native/test_idlutils.py | 44 +++++++++++++++++++ 7 files changed, 122 insertions(+), 5 deletions(-) diff --git a/neutron/agent/ovsdb/api.py b/neutron/agent/ovsdb/api.py index e5631568c91..e8d30e79c06 100644 --- a/neutron/agent/ovsdb/api.py +++ b/neutron/agent/ovsdb/api.py @@ -381,4 +381,5 @@ def py_to_val(pyval): elif pyval == '': return '""' else: - return pyval + # NOTE(twilson) If a Command object, return its record_id as a value + return getattr(pyval, "record_id", pyval) diff --git a/neutron/agent/ovsdb/impl_idl.py b/neutron/agent/ovsdb/impl_idl.py index aa0de3ff0ef..af074c3d912 100644 --- a/neutron/agent/ovsdb/impl_idl.py +++ b/neutron/agent/ovsdb/impl_idl.py @@ -81,7 +81,8 @@ class Transaction(api.Transaction): pass def post_commit(self, txn): - pass + for command in self.commands: + command.post_commit(txn) def do_commit(self): self.start_time = time.time() @@ -144,6 +145,7 @@ class NeutronOVSDBTransaction(Transaction): txn.expected_ifaces = set() def post_commit(self, txn): + super(NeutronOVSDBTransaction, self).post_commit(txn) # ovs-vsctl only logs these failures and does not return nonzero try: self.do_post_commit(txn) diff --git a/neutron/agent/ovsdb/impl_vsctl.py b/neutron/agent/ovsdb/impl_vsctl.py index 89df8df7d5d..be648098df1 100644 --- a/neutron/agent/ovsdb/impl_vsctl.py +++ b/neutron/agent/ovsdb/impl_vsctl.py @@ -14,6 +14,7 @@ import collections import itertools +import uuid from oslo_log import log as logging from oslo_serialization import jsonutils @@ -149,6 +150,22 @@ class DbGetCommand(DbCommand): self._result = list(self._result[0].values())[0] +class DbCreateCommand(BaseCommand): + def __init__(self, context, opts=None, args=None): + super(DbCreateCommand, self).__init__(context, "create", opts, args) + # NOTE(twilson) pre-commit result used for intra-transaction reference + self.record_id = "@%s" % uuid.uuid4() + self.opts.append("--id=%s" % self.record_id) + + @property + def result(self): + return self._result + + @result.setter + def result(self, val): + self._result = uuid.UUID(val) if val else val + + class BrExistsCommand(DbCommand): @DbCommand.result.setter def result(self, val): @@ -194,7 +211,7 @@ class OvsdbVsctl(ovsdb.API): def db_create(self, table, **col_values): args = [table] args += _set_colval_args(*col_values.items()) - return BaseCommand(self.context, 'create', args=args) + return DbCreateCommand(self.context, args=args) def db_destroy(self, table, record): args = [table, record] diff --git a/neutron/agent/ovsdb/native/commands.py b/neutron/agent/ovsdb/native/commands.py index 318dc4ccee9..8803941c696 100644 --- a/neutron/agent/ovsdb/native/commands.py +++ b/neutron/agent/ovsdb/native/commands.py @@ -41,6 +41,9 @@ class BaseCommand(api.Command): if not check_error: ctx.reraise = False + def post_commit(self, txn): + pass + def __str__(self): command_info = self.__dict__ return "%s(%s)" % ( @@ -164,9 +167,14 @@ class DbCreateCommand(BaseCommand): def run_idl(self, txn): row = txn.insert(self.api._tables[self.table]) for col, val in self.columns.items(): - setattr(row, col, val) + setattr(row, col, idlutils.db_replace_record(val)) + # This is a temporary row to be used within the transaction self.result = row + def post_commit(self, txn): + # Replace the temporary row with the post-commit UUID to match vsctl + self.result = txn.get_insert_uuid(self.result.uuid) + class DbDestroyCommand(BaseCommand): def __init__(self, api, table, record): @@ -194,7 +202,7 @@ class DbSetCommand(BaseCommand): # this soon. if isinstance(val, collections.OrderedDict): val = dict(val) - setattr(record, col, val) + setattr(record, col, idlutils.db_replace_record(val)) class DbClearCommand(BaseCommand): diff --git a/neutron/agent/ovsdb/native/idlutils.py b/neutron/agent/ovsdb/native/idlutils.py index f6d63f37856..d70dec449ef 100644 --- a/neutron/agent/ovsdb/native/idlutils.py +++ b/neutron/agent/ovsdb/native/idlutils.py @@ -22,8 +22,10 @@ from ovs.db import idl from ovs import jsonrpc from ovs import poller from ovs import stream +import six from neutron._i18n import _ +from neutron.agent.ovsdb import api RowLookup = collections.namedtuple('RowLookup', @@ -233,3 +235,28 @@ def get_index_column(table): idx = table.indexes[0] if len(idx) == 1: return idx[0].name + + +def db_replace_record(obj): + """Replace any api.Command objects with their results + + This method should leave obj untouched unless the object contains an + api.Command object. + """ + if isinstance(obj, collections.Mapping): + for k, v in six.iteritems(obj): + if isinstance(v, api.Command): + obj[k] = v.result + elif (isinstance(obj, collections.Sequence) + and not isinstance(obj, six.string_types)): + for i, v in enumerate(obj): + if isinstance(v, api.Command): + try: + obj[i] = v.result + except TypeError: + # NOTE(twilson) If someone passes a tuple, then just return + # a tuple with the Commands replaced with their results + return type(obj)(getattr(v, "result", v) for v in obj) + elif isinstance(obj, api.Command): + obj = obj.result + return obj diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index cbfd7df51c9..cafedc9302c 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -334,6 +334,24 @@ class OVSBridgeTestCase(OVSBridgeTestBase): self.assertIsNone(max_rate) self.assertIsNone(burst) + def test_db_create_references(self): + with self.ovs.ovsdb.transaction(check_error=True) as txn: + queue = txn.add(self.ovs.ovsdb.db_create("Queue", + other_config={'a': '1'})) + qos = txn.add(self.ovs.ovsdb.db_create("QoS", queues={0: queue})) + txn.add(self.ovs.ovsdb.db_set("Port", self.br.br_name, + ('qos', qos))) + + def cleanup(): + with self.ovs.ovsdb.transaction() as t: + t.add(self.ovs.ovsdb.db_destroy("QoS", qos.result)) + t.add(self.ovs.ovsdb.db_destroy("Queue", queue.result)) + t.add(self.ovs.ovsdb.db_clear("Port", self.br.br_name, 'qos')) + + self.addCleanup(cleanup) + val = self.ovs.ovsdb.db_get("Port", self.br.br_name, 'qos').execute() + self.assertEqual(qos.result, val) + class OVSLibTestCase(base.BaseOVSLinuxTestCase): diff --git a/neutron/tests/unit/agent/ovsdb/native/test_idlutils.py b/neutron/tests/unit/agent/ovsdb/native/test_idlutils.py index 1cde37f1fd5..9cbcfcd551b 100644 --- a/neutron/tests/unit/agent/ovsdb/native/test_idlutils.py +++ b/neutron/tests/unit/agent/ovsdb/native/test_idlutils.py @@ -14,6 +14,7 @@ import mock +from neutron.agent.ovsdb import api from neutron.agent.ovsdb.native import idlutils from neutron.tests import base @@ -47,6 +48,14 @@ class MockRow(object): return super(MockRow, self).__getattr__(attr) +class MockCommand(api.Command): + def __init__(self, result): + self.result = result + + def execute(self, **kwargs): + pass + + class TestIdlUtils(base.BaseTestCase): def test_condition_match(self): """ @@ -98,3 +107,38 @@ class TestIdlUtils(base.BaseTestCase): row, ("comments", "=", ["c", "b"]))) self.assertTrue(idlutils.condition_match( row, ("comments", "!=", ["d"]))) + + def test_db_replace_record_dict(self): + obj = {'a': 1, 'b': 2} + self.assertIs(obj, idlutils.db_replace_record(obj)) + + def test_db_replace_record_dict_cmd(self): + obj = {'a': 1, 'b': MockCommand(2)} + res = {'a': 1, 'b': 2} + self.assertEqual(res, idlutils.db_replace_record(obj)) + + def test_db_replace_record_list(self): + obj = [1, 2, 3] + self.assertIs(obj, idlutils.db_replace_record(obj)) + + def test_db_replace_record_list_cmd(self): + obj = [1, MockCommand(2), 3] + res = [1, 2, 3] + self.assertEqual(res, idlutils.db_replace_record(obj)) + + def test_db_replace_record_tuple(self): + obj = (1, 2, 3) + self.assertIs(obj, idlutils.db_replace_record(obj)) + + def test_db_replace_record_tuple_cmd(self): + obj = (1, MockCommand(2), 3) + res = (1, 2, 3) + self.assertEqual(res, idlutils.db_replace_record(obj)) + + def test_db_replace_record(self): + obj = "test" + self.assertIs(obj, idlutils.db_replace_record(obj)) + + def test_db_replace_record_cmd(self): + obj = MockCommand("test") + self.assertEqual("test", idlutils.db_replace_record(obj))