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

@@ -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)

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)

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]

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):

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

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):
 

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))