Merge "[OVN] Remove ``create_lrouter`` and ``delete_lrouter`` implementation"
This commit is contained in:
Zuul
Gerrit Code Review
commit
89d2108390
|
|
@ -77,20 +77,6 @@ class API(api.API, metaclass=abc.ABCMeta):
|
|||
:returns: :class:`Command` with no result
|
||||
"""
|
||||
|
||||
@abc.abstractmethod
|
||||
def create_lrouter(self, name, may_exist=True, **columns):
|
||||
"""Create a command to add an OVN lrouter
|
||||
|
||||
:param name: The id of the lrouter
|
||||
:type name: string
|
||||
:param may_exist: Do not fail if lrouter already exists
|
||||
:type may_exist: bool
|
||||
:param columns: Dictionary of lrouter columns
|
||||
Supported columns: external_ids, default_gw, ip
|
||||
:type columns: dictionary
|
||||
:returns: :class:`Command` with no result
|
||||
"""
|
||||
|
||||
@abc.abstractmethod
|
||||
def update_lrouter(self, name, if_exists=True, **columns):
|
||||
"""Update a command to add an OVN lrouter
|
||||
|
|
@ -105,17 +91,6 @@ class API(api.API, metaclass=abc.ABCMeta):
|
|||
:returns: :class:`Command` with no result
|
||||
"""
|
||||
|
||||
@abc.abstractmethod
|
||||
def delete_lrouter(self, name, if_exists=True):
|
||||
"""Create a command to delete an OVN lrouter
|
||||
|
||||
:param name: The id of the lrouter
|
||||
:type name: string
|
||||
:param if_exists: Do not fail if the lrouter does not exist
|
||||
:type if_exists: bool
|
||||
:returns: :class:`Command` with no result
|
||||
"""
|
||||
|
||||
@abc.abstractmethod
|
||||
def add_lrouter_port(self, name, lrouter, may_exist=True,
|
||||
**columns):
|
||||
|
|
|
|||
|
|
@ -297,26 +297,6 @@ class DelLSwitchPortCommand(command.BaseCommand):
|
|||
self.api._tables['Logical_Switch_Port'].rows[lport.uuid].delete()
|
||||
|
||||
|
||||
class AddLRouterCommand(command.BaseCommand):
|
||||
def __init__(self, api, name, may_exist, **columns):
|
||||
super(AddLRouterCommand, self).__init__(api)
|
||||
self.name = name
|
||||
self.columns = columns
|
||||
self.may_exist = may_exist
|
||||
|
||||
def run_idl(self, txn):
|
||||
if self.may_exist:
|
||||
lrouter = idlutils.row_by_value(self.api.idl, 'Logical_Router',
|
||||
'name', self.name, None)
|
||||
if lrouter:
|
||||
return
|
||||
|
||||
row = txn.insert(self.api._tables['Logical_Router'])
|
||||
row.name = self.name
|
||||
for col, val in self.columns.items():
|
||||
setattr(row, col, val)
|
||||
|
||||
|
||||
class UpdateLRouterCommand(command.BaseCommand):
|
||||
def __init__(self, api, name, if_exists, **columns):
|
||||
super(UpdateLRouterCommand, self).__init__(api)
|
||||
|
|
@ -340,25 +320,6 @@ class UpdateLRouterCommand(command.BaseCommand):
|
|||
return
|
||||
|
||||
|
||||
class DelLRouterCommand(command.BaseCommand):
|
||||
def __init__(self, api, name, if_exists):
|
||||
super(DelLRouterCommand, self).__init__(api)
|
||||
self.name = name
|
||||
self.if_exists = if_exists
|
||||
|
||||
def run_idl(self, txn):
|
||||
try:
|
||||
lrouter = idlutils.row_by_value(self.api.idl, 'Logical_Router',
|
||||
'name', self.name)
|
||||
except idlutils.RowNotFound:
|
||||
if self.if_exists:
|
||||
return
|
||||
msg = _("Logical Router %s does not exist") % self.name
|
||||
raise RuntimeError(msg)
|
||||
|
||||
self.api._tables['Logical_Router'].rows[lrouter.uuid].delete()
|
||||
|
||||
|
||||
class AddLRouterPortCommand(command.BaseCommand):
|
||||
def __init__(self, api, name, lrouter, may_exist, **columns):
|
||||
super(AddLRouterPortCommand, self).__init__(api)
|
||||
|
|
|
|||
|
|
@ -423,17 +423,10 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
|
|||
acl_list.append(acl_string)
|
||||
return acl_values_dict, acl_obj_dict, lswitch_ovsdb_dict
|
||||
|
||||
def create_lrouter(self, name, may_exist=True, **columns):
|
||||
return cmd.AddLRouterCommand(self, name,
|
||||
may_exist, **columns)
|
||||
|
||||
def update_lrouter(self, name, if_exists=True, **columns):
|
||||
return cmd.UpdateLRouterCommand(self, name,
|
||||
if_exists, **columns)
|
||||
|
||||
def delete_lrouter(self, name, if_exists=True):
|
||||
return cmd.DelLRouterCommand(self, name, if_exists)
|
||||
|
||||
def add_lrouter_port(self, name, lrouter, may_exist=False, **columns):
|
||||
return cmd.AddLRouterPortCommand(self, name, lrouter,
|
||||
may_exist, **columns)
|
||||
|
|
|
|||
|
|
@ -1410,10 +1410,9 @@ class OVNClient(object):
|
|||
ovn_const.LR_OPTIONS_MAC_AGE_LIMIT:
|
||||
ovn_conf.get_ovn_mac_binding_age_threshold()}
|
||||
with self._nb_idl.transaction(check_error=True) as txn:
|
||||
txn.add(self._nb_idl.create_lrouter(lrouter_name,
|
||||
external_ids=external_ids,
|
||||
enabled=enabled,
|
||||
options=options))
|
||||
txn.add(self._nb_idl.lr_add(router=lrouter_name, may_exist=True,
|
||||
external_ids=external_ids,
|
||||
enabled=enabled, options=options))
|
||||
# TODO(lucasagomes): add_external_gateway is being only used
|
||||
# by the ovn_db_sync.py script, remove it after the database
|
||||
# synchronization work
|
||||
|
|
@ -1522,7 +1521,7 @@ class OVNClient(object):
|
|||
"""Delete a logical router."""
|
||||
lrouter_name = utils.ovn_name(router_id)
|
||||
with self._nb_idl.transaction(check_error=True) as txn:
|
||||
txn.add(self._nb_idl.delete_lrouter(lrouter_name))
|
||||
txn.add(self._nb_idl.lr_del(lrouter_name, if_exists=True))
|
||||
db_rev.delete_revision(context, router_id, ovn_const.TYPE_ROUTERS)
|
||||
|
||||
def get_candidates_for_scheduling(self, physnet, cms=None,
|
||||
|
|
|
|||
|
|
@ -711,8 +711,8 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
|
|||
if self.mode == SYNC_MODE_REPAIR:
|
||||
LOG.warning("Deleting the router %s from OVN NB DB",
|
||||
lrouter['name'])
|
||||
txn.add(self.ovn_api.delete_lrouter(
|
||||
utils.ovn_name(lrouter['name'])))
|
||||
txn.add(self.ovn_api.lr_del(
|
||||
utils.ovn_name(lrouter['name']), if_exists=True))
|
||||
|
||||
for lrport_info in del_lrouter_ports_list:
|
||||
LOG.warning("Router Port found in OVN NB DB but not in "
|
||||
|
|
|
|||
|
|
@ -775,11 +775,11 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
|
|||
for lrouter_name in self.create_lrouters:
|
||||
external_ids = {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:
|
||||
lrouter_name}
|
||||
txn.add(self.nb_api.create_lrouter(lrouter_name, True,
|
||||
external_ids=external_ids))
|
||||
txn.add(self.nb_api.lr_add(router=lrouter_name, may_exist=True,
|
||||
external_ids=external_ids))
|
||||
|
||||
for lrouter_name in self.delete_lrouters:
|
||||
txn.add(self.nb_api.delete_lrouter(lrouter_name, True))
|
||||
txn.add(self.nb_api.lr_del(lrouter_name, if_exists=True))
|
||||
|
||||
for lrport, lrouter_name in self.create_lrouter_ports:
|
||||
txn.add(self.nb_api.add_lrouter_port(lrport, lrouter_name))
|
||||
|
|
|
|||
|
|
@ -60,11 +60,11 @@ class FakeOvsdbNbOvnIdl(object):
|
|||
self.set_lswitch_port = mock.Mock()
|
||||
self.delete_lswitch_port = mock.Mock()
|
||||
self.get_acls_for_lswitches = mock.Mock()
|
||||
self.create_lrouter = mock.Mock()
|
||||
self.lrp_del = mock.Mock()
|
||||
self.lrp_set_options = mock.Mock()
|
||||
self.lr_add = mock.Mock()
|
||||
self.update_lrouter = mock.Mock()
|
||||
self.delete_lrouter = mock.Mock()
|
||||
self.lr_del = mock.Mock()
|
||||
self.add_lrouter_port = mock.Mock()
|
||||
self.update_lrouter_port = mock.Mock()
|
||||
self.delete_lrouter_port = mock.Mock()
|
||||
|
|
|
|||
|
|
@ -431,52 +431,6 @@ class TestDelLSwitchPortCommand(TestBaseCommand):
|
|||
v4_ext_ids, v6_ext_ids)
|
||||
|
||||
|
||||
class TestAddLRouterCommand(TestBaseCommand):
|
||||
|
||||
def test_lrouter_exists(self):
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
return_value=mock.ANY):
|
||||
cmd = commands.AddLRouterCommand(
|
||||
self.ovn_api, 'fake-lrouter', may_exist=True,
|
||||
a='1', b='2')
|
||||
cmd.run_idl(self.transaction)
|
||||
self.transaction.insert.assert_not_called()
|
||||
|
||||
def test_lrouter_add_exists(self):
|
||||
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row()
|
||||
self.ovn_api._tables['Logical_Router'].rows[fake_lrouter.uuid] = \
|
||||
fake_lrouter
|
||||
self.transaction.insert.return_value = fake_lrouter
|
||||
cmd = commands.AddLRouterCommand(
|
||||
self.ovn_api, fake_lrouter.name, may_exist=False)
|
||||
cmd.run_idl(self.transaction)
|
||||
# NOTE(rtheis): Mocking the transaction allows this insert
|
||||
# to succeed when it normally would fail due the duplicate name.
|
||||
self.transaction.insert.assert_called_once_with(
|
||||
self.ovn_api._tables['Logical_Router'])
|
||||
|
||||
def _test_lrouter_add(self, may_exist=True):
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
return_value=None):
|
||||
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row()
|
||||
self.transaction.insert.return_value = fake_lrouter
|
||||
cmd = commands.AddLRouterCommand(
|
||||
self.ovn_api, 'fake-lrouter', may_exist=may_exist,
|
||||
a='1', b='2')
|
||||
cmd.run_idl(self.transaction)
|
||||
self.transaction.insert.assert_called_once_with(
|
||||
self.ovn_api._tables['Logical_Router'])
|
||||
self.assertEqual('fake-lrouter', fake_lrouter.name)
|
||||
self.assertEqual('1', fake_lrouter.a)
|
||||
self.assertEqual('2', fake_lrouter.b)
|
||||
|
||||
def test_lrouter_add_may_exist(self):
|
||||
self._test_lrouter_add(may_exist=True)
|
||||
|
||||
def test_lrouter_add_ignore_exists(self):
|
||||
self._test_lrouter_add(may_exist=False)
|
||||
|
||||
|
||||
class TestUpdateLRouterCommand(TestBaseCommand):
|
||||
|
||||
def _test_lrouter_update_no_exist(self, if_exists=True):
|
||||
|
|
@ -509,36 +463,6 @@ class TestUpdateLRouterCommand(TestBaseCommand):
|
|||
self.assertEqual(new_ext_ids, fake_lrouter.external_ids)
|
||||
|
||||
|
||||
class TestDelLRouterCommand(TestBaseCommand):
|
||||
|
||||
def _test_lrouter_del_no_exist(self, if_exists=True):
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
side_effect=idlutils.RowNotFound):
|
||||
cmd = commands.DelLRouterCommand(
|
||||
self.ovn_api, 'fake-lrouter', if_exists=if_exists)
|
||||
if if_exists:
|
||||
cmd.run_idl(self.transaction)
|
||||
else:
|
||||
self.assertRaises(RuntimeError, cmd.run_idl, self.transaction)
|
||||
|
||||
def test_lrouter_no_exist_ignore(self):
|
||||
self._test_lrouter_del_no_exist(if_exists=True)
|
||||
|
||||
def test_lrouter_no_exist_fail(self):
|
||||
self._test_lrouter_del_no_exist(if_exists=False)
|
||||
|
||||
def test_lrouter_del(self):
|
||||
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row()
|
||||
self.ovn_api._tables['Logical_Router'].rows[fake_lrouter.uuid] = \
|
||||
fake_lrouter
|
||||
with mock.patch.object(idlutils, 'row_by_value',
|
||||
return_value=fake_lrouter):
|
||||
cmd = commands.DelLRouterCommand(
|
||||
self.ovn_api, fake_lrouter.name, if_exists=True)
|
||||
cmd.run_idl(self.transaction)
|
||||
fake_lrouter.delete.assert_called_once_with()
|
||||
|
||||
|
||||
class TestAddLRouterPortCommand(TestBaseCommand):
|
||||
|
||||
def test_lrouter_not_found(self):
|
||||
|
|
|
|||
|
|
@ -686,8 +686,7 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase):
|
|||
create_router_port_calls,
|
||||
any_order=True)
|
||||
|
||||
self.assertEqual(len(del_router_list),
|
||||
ovn_api.delete_lrouter.call_count)
|
||||
self.assertEqual(len(del_router_list), ovn_api.lr_del.call_count)
|
||||
update_router_port_calls = [mock.call(mock.ANY, p)
|
||||
for p in update_router_port_list]
|
||||
self.assertEqual(
|
||||
|
|
@ -697,10 +696,9 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase):
|
|||
update_router_port_calls,
|
||||
any_order=True)
|
||||
|
||||
delete_lrouter_calls = [mock.call(r['router'])
|
||||
delete_lrouter_calls = [mock.call(r['router'], if_exists=True)
|
||||
for r in del_router_list]
|
||||
ovn_api.delete_lrouter.assert_has_calls(
|
||||
delete_lrouter_calls, any_order=True)
|
||||
ovn_api.lr_del.assert_has_calls(delete_lrouter_calls, any_order=True)
|
||||
|
||||
self.assertEqual(
|
||||
len(del_router_port_list),
|
||||
|
|
|
|||
|
|
@ -616,9 +616,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
|||
external_ids = {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router',
|
||||
ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1',
|
||||
ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ''}
|
||||
self.l3_inst._nb_ovn.create_lrouter.assert_called_once_with(
|
||||
'neutron-router-id', external_ids=external_ids,
|
||||
enabled=True,
|
||||
self.l3_inst._nb_ovn.lr_add.assert_called_once_with(
|
||||
router='neutron-router-id', may_exist=True,
|
||||
external_ids=external_ids, enabled=True,
|
||||
options={'always_learn_from_arp_request': 'false',
|
||||
'dynamic_neigh_routers': 'true',
|
||||
ovn_const.LR_OPTIONS_MAC_AGE_LIMIT:
|
||||
|
|
@ -663,8 +663,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
|
|||
events.AFTER_DELETE,
|
||||
self, payload)
|
||||
|
||||
self.l3_inst._nb_ovn.delete_lrouter.assert_called_once_with(
|
||||
'neutron-router-id')
|
||||
self.l3_inst._nb_ovn.lr_del.assert_called_once_with(
|
||||
'neutron-router-id', if_exists=True)
|
||||
|
||||
@mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.ovn_client'
|
||||
'.OVNClient._get_router_ports')
|
||||
|
|
|
|||
Loading…
Reference in New Issue