diff --git a/neutron/agent/ovsdb/native/commands.py b/neutron/agent/ovsdb/native/commands.py index 34cd50ce505..deb4cdbfe77 100644 --- a/neutron/agent/ovsdb/native/commands.py +++ b/neutron/agent/ovsdb/native/commands.py @@ -59,8 +59,12 @@ class AddManagerCommand(BaseCommand): 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] + try: + self.api._ovs.addvalue('manager_options', row) + except AttributeError: # OVS < 2.6 + self.api._ovs.verify('manager_options') + self.api._ovs.manager_options = ( + self.api._ovs.manager_options + [row]) class GetManagerCommand(BaseCommand): @@ -85,11 +89,14 @@ class RemoveManagerCommand(BaseCommand): 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() + try: + self.api._ovs.delvalue('manager_options', manager) + except AttributeError: # OVS < 2.6 + self.api._ovs.verify('manager_options') + manager_list = self.api._ovs.manager_options + manager_list.remove(manager) + self.api._ovs.manager_options = manager_list + manager.delete() class AddBridgeCommand(BaseCommand): @@ -111,8 +118,11 @@ class AddBridgeCommand(BaseCommand): row.name = self.name if self.datapath_type: row.datapath_type = self.datapath_type - self.api._ovs.verify('bridges') - self.api._ovs.bridges = self.api._ovs.bridges + [row] + try: + self.api._ovs.addvalue('bridges', row) + except AttributeError: # OVS < 2.6 + self.api._ovs.verify('bridges') + self.api._ovs.bridges = self.api._ovs.bridges + [row] # Add the internal bridge port cmd = AddPortCommand(self.api, self.name, self.name, self.may_exist) @@ -140,15 +150,19 @@ class DelBridgeCommand(BaseCommand): msg = _("Bridge %s does not exist") % self.name LOG.error(msg) raise RuntimeError(msg) - self.api._ovs.verify('bridges') + # Clean up cached ports/interfaces for port in br.ports: - cmd = DelPortCommand(self.api, port.name, self.name, - if_exists=True) - cmd.run_idl(txn) - bridges = self.api._ovs.bridges - bridges.remove(br) - self.api._ovs.bridges = bridges - self.api._tables['Bridge'].rows[br.uuid].delete() + for interface in port.interfaces: + interface.delete() + port.delete() + try: + self.api._ovs.delvalue('bridges', br) + except AttributeError: # OVS < 2.6 + self.api._ovs.verify('bridges') + bridges = self.api._ovs.bridges + bridges.remove(br) + self.api._ovs.bridges = bridges + br.delete() class BridgeExistsCommand(BaseCommand): @@ -285,7 +299,7 @@ class SetControllerCommand(BaseCommand): controller = txn.insert(self.api._tables['Controller']) controller.target = target controllers.append(controller) - br.verify('controller') + # Don't need to verify because we unconditionally overwrite br.controller = controllers @@ -306,7 +320,6 @@ class GetControllerCommand(BaseCommand): def run_idl(self, txn): br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) - br.verify('controller') self.result = [c.target for c in br.controller] @@ -318,7 +331,6 @@ class SetFailModeCommand(BaseCommand): def run_idl(self, txn): br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge) - br.verify('fail_mode') br.fail_mode = self.mode @@ -338,18 +350,19 @@ class AddPortCommand(BaseCommand): return port = txn.insert(self.api._tables['Port']) port.name = self.port - br.verify('ports') - ports = getattr(br, 'ports', []) - ports.append(port) - br.ports = ports + try: + br.addvalue('ports', port) + except AttributeError: # OVS < 2.6 + br.verify('ports') + ports = getattr(br, 'ports', []) + ports.append(port) + br.ports = ports iface = txn.insert(self.api._tables['Interface']) txn.expected_ifaces.add(iface.uuid) iface.name = self.port - port.verify('interfaces') - ifaces = getattr(port, 'interfaces', []) - ifaces.append(iface) - port.interfaces = ifaces + # This is a new port, so it won't have any existing interfaces + port.interfaces = [iface] class DelPortCommand(BaseCommand): @@ -375,24 +388,26 @@ class DelPortCommand(BaseCommand): br = next(b for b in self.api._tables['Bridge'].rows.values() if port in b.ports) - if port.uuid not in br.ports and not self.if_exists: + if port not in br.ports and not self.if_exists: # TODO(twilson) Make real errors across both implementations msg = _("Port %(port)s does not exist on %(bridge)s!") % { - 'port': self.name, 'bridge': self.bridge + 'port': self.port, 'bridge': self.bridge } LOG.error(msg) raise RuntimeError(msg) - br.verify('ports') - ports = br.ports - ports.remove(port) - br.ports = ports + try: + br.delvalue('ports', port) + except AttributeError: # OVS < 2.6 + br.verify('ports') + ports = br.ports + ports.remove(port) + br.ports = ports - # Also remove port/interface directly for indexing? - port.verify('interfaces') - for iface in port.interfaces: - self.api._tables['Interface'].rows[iface.uuid].delete() - self.api._tables['Port'].rows[port.uuid].delete() + # The interface on the port will be cleaned up by ovsdb-server + for interface in port.interfaces: + interface.delete() + port.delete() class ListPortsCommand(BaseCommand): diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index e1c6ed61296..221838c2c41 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -21,6 +21,7 @@ from neutron_lib import constants as const from neutron.agent.common import ovs_lib from neutron.agent.linux import ip_lib +from neutron.agent.ovsdb.native import idlutils from neutron.common import utils from neutron.tests.common.exclusive_resources import port from neutron.tests.common import net_helpers @@ -336,6 +337,20 @@ class OVSBridgeTestCase(OVSBridgeTestBase): self.assertIsNone(max_rate) self.assertIsNone(burst) + def test_cascading_del_in_txn(self): + ovsdb = self.ovs.ovsdb + port_name, _ = self.create_ovs_port() + + def del_port_mod_iface(): + with ovsdb.transaction(check_error=True) as txn: + txn.add(ovsdb.del_port(port_name, self.br.br_name, + if_exists=False)) + txn.add(ovsdb.db_set('Interface', port_name, + ('type', 'internal'))) + # native gives a more specific exception than vsctl + self.assertRaises((RuntimeError, idlutils.RowNotFound), + del_port_mod_iface) + class OVSLibTestCase(base.BaseOVSLinuxTestCase):