Remove some unnecessary usages of verify()
The general usage of verify() is if you are doing something like:
ports = row.ports
ports.append('newport')
row.ports = ports
where you read a value from the db, modify it, then overwrite it.
The setkey/addvalue/delvalue mutate functions make it largely
unnecessary to use verify(), so this patch removes those.
Change-Id: Idd42491dcaa10ec36c963b477438eaa2336ef3a0
This commit is contained in:
Terry Wilson
parent
9717fea647
commit
b11dd3836e
|
|
@ -533,54 +533,39 @@ class DelAddrSetCommand(command.BaseCommand):
|
|||
self.api._tables['Address_Set'].rows[addrset.uuid].delete()
|
||||
|
||||
|
||||
class UpdateChassisExtIdsCommand(command.BaseCommand):
|
||||
def __init__(self, api, name, external_ids, if_exists):
|
||||
super(UpdateChassisExtIdsCommand, self).__init__(api)
|
||||
self.name = name
|
||||
class UpdateObjectExtIdsCommand(command.BaseCommand):
|
||||
table = None
|
||||
field = 'name'
|
||||
|
||||
def __init__(self, api, record, external_ids, if_exists):
|
||||
super(UpdateObjectExtIdsCommand, self).__init__(api)
|
||||
self.record = record
|
||||
self.external_ids = external_ids
|
||||
self.if_exists = if_exists
|
||||
|
||||
def run_idl(self, txn):
|
||||
try:
|
||||
chassis = idlutils.row_by_value(self.api.idl, 'Chassis',
|
||||
'name', self.name)
|
||||
# api.lookup() would be better as it doesn't rely on hardcoded col
|
||||
obj = idlutils.row_by_value(self.api.idl, self.table, self.field,
|
||||
self.record)
|
||||
except idlutils.RowNotFound:
|
||||
if self.if_exists:
|
||||
return
|
||||
msg = _("Chassis %s does not exist. "
|
||||
"Can't update external IDs") % self.name
|
||||
msg = _("%s %s does not exist. "
|
||||
"Can't update external IDs") % (self.table, self.record)
|
||||
raise RuntimeError(msg)
|
||||
|
||||
chassis.verify('external_ids')
|
||||
chassis_external_ids = getattr(chassis, 'external_ids', {})
|
||||
for ext_id_key, ext_id_value in self.external_ids.items():
|
||||
chassis_external_ids[ext_id_key] = ext_id_value
|
||||
chassis.external_ids = chassis_external_ids
|
||||
obj.setkey('external_ids', ext_id_key, ext_id_value)
|
||||
|
||||
|
||||
class UpdatePortBindingExtIdsCommand(command.BaseCommand):
|
||||
def __init__(self, api, name, external_ids, if_exists):
|
||||
super(UpdatePortBindingExtIdsCommand, self).__init__(api)
|
||||
self.name = name
|
||||
self.external_ids = external_ids
|
||||
self.if_exists = if_exists
|
||||
class UpdateChassisExtIdsCommand(UpdateObjectExtIdsCommand):
|
||||
table = 'Chassis'
|
||||
|
||||
def run_idl(self, txn):
|
||||
try:
|
||||
port = idlutils.row_by_value(self.api.idl, 'Port_Binding',
|
||||
'logical_port', self.name)
|
||||
except idlutils.RowNotFound:
|
||||
if self.if_exists:
|
||||
return
|
||||
msg = _("Port %s does not exist. "
|
||||
"Can't update external IDs") % self.name
|
||||
raise RuntimeError(msg)
|
||||
|
||||
port.verify('external_ids')
|
||||
port_external_ids = getattr(port, 'external_ids', {})
|
||||
for ext_id_key, ext_id_value in self.external_ids.items():
|
||||
port_external_ids[ext_id_key] = ext_id_value
|
||||
port.external_ids = port_external_ids
|
||||
class UpdatePortBindingExtIdsCommand(UpdateObjectExtIdsCommand):
|
||||
table = 'Port_Binding'
|
||||
field = 'logical_port'
|
||||
|
||||
|
||||
class AddDHCPOptionsCommand(command.BaseCommand):
|
||||
|
|
@ -654,11 +639,7 @@ class AddNATRuleInLRouterCommand(command.BaseCommand):
|
|||
row = txn.insert(self.api._tables['NAT'])
|
||||
for col, val in self.columns.items():
|
||||
setattr(row, col, val)
|
||||
# TODO(chandrav): convert this to ovs transaction mutate
|
||||
lrouter.verify('nat')
|
||||
nat = getattr(lrouter, 'nat', [])
|
||||
nat.append(row.uuid)
|
||||
setattr(lrouter, 'nat', nat)
|
||||
lrouter.addvalue('nat', row.uuid)
|
||||
|
||||
|
||||
class DeleteNATRuleInLRouterCommand(command.BaseCommand):
|
||||
|
|
@ -682,20 +663,13 @@ class DeleteNATRuleInLRouterCommand(command.BaseCommand):
|
|||
msg = _("Logical Router %s does not exist") % self.lrouter
|
||||
raise RuntimeError(msg)
|
||||
|
||||
lrouter.verify('nat')
|
||||
# TODO(chandrav): convert this to ovs transaction mutate
|
||||
nats = getattr(lrouter, 'nat', [])
|
||||
for nat in nats:
|
||||
type = getattr(nat, 'type', '')
|
||||
external_ip = getattr(nat, 'external_ip', '')
|
||||
logical_ip = getattr(nat, 'logical_ip', '')
|
||||
if (self.type == type and
|
||||
self.external_ip == external_ip and
|
||||
self.logical_ip == logical_ip):
|
||||
nats.remove(nat)
|
||||
for nat in lrouter.nat:
|
||||
if (self.type == nat.type and
|
||||
self.external_ip == nat.external_ip and
|
||||
self.logical_ip == nat.logical_ip):
|
||||
lrouter.delvalue('nat', nat)
|
||||
nat.delete()
|
||||
break
|
||||
setattr(lrouter, 'nat', nats)
|
||||
|
||||
|
||||
class SetNATRuleInLRouterCommand(command.BaseCommand):
|
||||
|
|
@ -713,9 +687,7 @@ class SetNATRuleInLRouterCommand(command.BaseCommand):
|
|||
msg = _("Logical Router %s does not exist") % self.lrouter
|
||||
raise RuntimeError(msg)
|
||||
|
||||
lrouter.verify('nat')
|
||||
nat_rules = getattr(lrouter, 'nat', [])
|
||||
for nat_rule in nat_rules:
|
||||
for nat_rule in lrouter.nat:
|
||||
if nat_rule.uuid == self.nat_rule_uuid:
|
||||
for col, val in self.columns.items():
|
||||
setattr(nat_rule, col, val)
|
||||
|
|
@ -823,21 +795,17 @@ class DeleteLRouterExtGwCommand(command.BaseCommand):
|
|||
msg = _("Logical Router %s does not exist") % self.lrouter
|
||||
raise RuntimeError(msg)
|
||||
|
||||
lrouter.verify('static_routes')
|
||||
static_routes = getattr(lrouter, 'static_routes', [])
|
||||
for route in static_routes:
|
||||
for route in lrouter.static_routes:
|
||||
external_ids = getattr(route, 'external_ids', {})
|
||||
if ovn_const.OVN_ROUTER_IS_EXT_GW in external_ids:
|
||||
_delvalue_from_list(lrouter, 'static_routes', route)
|
||||
lrouter.delvalue('static_routes', route)
|
||||
route.delete()
|
||||
break
|
||||
|
||||
lrouter.verify('nat')
|
||||
nats = getattr(lrouter, 'nat', [])
|
||||
for nat in nats:
|
||||
for nat in lrouter.nat:
|
||||
if nat.type != 'snat':
|
||||
continue
|
||||
_delvalue_from_list(lrouter, 'nat', nat)
|
||||
lrouter.delvalue('nat', nat)
|
||||
nat.delete()
|
||||
|
||||
lrouter_ext_ids = getattr(lrouter, 'external_ids', {})
|
||||
|
|
@ -852,7 +820,7 @@ class DeleteLRouterExtGwCommand(command.BaseCommand):
|
|||
except idlutils.RowNotFound:
|
||||
return
|
||||
|
||||
_delvalue_from_list(lrouter, 'ports', lrouter_port)
|
||||
lrouter.delvalue('ports', lrouter_port)
|
||||
|
||||
|
||||
class SetLSwitchPortToVirtualTypeCommand(command.BaseCommand):
|
||||
|
|
|
|||
|
|
@ -358,9 +358,29 @@ class FakeOvsdbRow(FakeResource):
|
|||
ovsdb_row_attrs.update(attrs)
|
||||
ovsdb_row_methods.update(methods)
|
||||
|
||||
return FakeResource(info=copy.deepcopy(ovsdb_row_attrs),
|
||||
loaded=True,
|
||||
methods=copy.deepcopy(ovsdb_row_methods))
|
||||
result = FakeResource(info=copy.deepcopy(ovsdb_row_attrs),
|
||||
loaded=True,
|
||||
methods=copy.deepcopy(ovsdb_row_methods))
|
||||
result.setkey.side_effect = lambda col, k, v: (
|
||||
getattr(result, col).__setitem__(k, v))
|
||||
|
||||
def fake_addvalue(col, val):
|
||||
try:
|
||||
getattr(result, col).append(val)
|
||||
except AttributeError:
|
||||
# Not all tests set up fake rows to have all used cols
|
||||
pass
|
||||
|
||||
def fake_delvalue(col, val):
|
||||
try:
|
||||
getattr(result, col).remove(val)
|
||||
except (AttributeError, ValueError):
|
||||
# Some tests also fake adding values
|
||||
pass
|
||||
|
||||
result.addvalue.side_effect = fake_addvalue
|
||||
result.delvalue.side_effect = fake_delvalue
|
||||
return result
|
||||
|
||||
|
||||
class FakeOvsdbTable(FakeResource):
|
||||
|
|
|
|||
|
|
@ -1207,7 +1207,8 @@ class TestDeleteLRouterExtGwCommand(TestBaseCommand):
|
|||
fake_route_2 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'ip_prefix': '50.0.0.0/24', 'nexthop': '40.0.0.101'})
|
||||
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'static_routes': [fake_route_1, fake_route_2]})
|
||||
attrs={'static_routes': [fake_route_1, fake_route_2],
|
||||
'nat': []})
|
||||
with mock.patch.object(self.ovn_api, "is_col_present",
|
||||
return_value=True):
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
|
|
@ -1226,7 +1227,8 @@ class TestDeleteLRouterExtGwCommand(TestBaseCommand):
|
|||
attrs={'external_ip': '192.168.1.8',
|
||||
'logical_ip': '10.0.0.5', 'type': 'badtype'})
|
||||
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'nat': [fake_nat_1, fake_nat_2]})
|
||||
attrs={'nat': [fake_nat_1, fake_nat_2],
|
||||
'static_routes': []})
|
||||
with mock.patch.object(self.ovn_api, "is_col_present",
|
||||
return_value=True):
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
|
|
@ -1241,7 +1243,8 @@ class TestDeleteLRouterExtGwCommand(TestBaseCommand):
|
|||
port_id = 'fake-port-id'
|
||||
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'external_ids':
|
||||
{ovn_const.OVN_GW_PORT_EXT_ID_KEY: port_id}})
|
||||
{ovn_const.OVN_GW_PORT_EXT_ID_KEY: port_id},
|
||||
'static_routes': [], 'nat': []})
|
||||
with mock.patch.object(self.ovn_api, "is_col_present",
|
||||
return_value=True):
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
|
|
@ -1256,7 +1259,8 @@ class TestDeleteLRouterExtGwCommand(TestBaseCommand):
|
|||
port_id = 'fake-port-id'
|
||||
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row(
|
||||
attrs={'external_ids':
|
||||
{ovn_const.OVN_GW_PORT_EXT_ID_KEY: port_id}})
|
||||
{ovn_const.OVN_GW_PORT_EXT_ID_KEY: port_id},
|
||||
'static_routes': [], 'nat': []})
|
||||
with mock.patch.object(self.ovn_api, "is_col_present",
|
||||
return_value=True):
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
|
|
|
|||
Loading…
Reference in New Issue