diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 069dac69816..37d9b861b33 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -696,54 +696,40 @@ 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 = _("%(tbl)s %(rec)s does not exist. " + "Can't update external IDs") % { + 'tbl': self.table, 'rec': 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): @@ -817,11 +803,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): @@ -845,20 +827,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): @@ -876,9 +851,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) @@ -986,21 +959,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', {}) @@ -1015,7 +984,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): diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index 52b22f81564..b5eea80bc67 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -360,9 +360,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): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py index 47d6cc7c369..d24774b95d7 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py @@ -1319,7 +1319,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', @@ -1338,7 +1339,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', @@ -1353,7 +1355,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', @@ -1368,7 +1371,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',