From f2643f27fb617a517be95138246331d8556c0cc3 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 15 Sep 2020 15:30:22 +0100 Subject: [PATCH] [OVN] Fix test_add_interface_in_use negative test While fixing https://bugs.launchpad.net/neutron/+bug/1876148 we've made the add_router_interface method idempotent and it no longer raised a Conflict if the same interface was added twice, this broke the test_add_interface_in_use from neutron-tempest-pluign. Change-Id: I76d46a4c3c8af166e022894a5693d5c78555ea50 Closes-Bug: #1895671 Signed-off-by: Lucas Alvares Gomes --- .../ovn/mech_driver/ovsdb/maintenance.py | 5 +-- neutron/services/ovn_l3/plugin.py | 4 ++- neutron/tests/unit/extensions/test_l3.py | 32 +++++++++++++++++++ .../ovn/mech_driver/ovsdb/test_maintenance.py | 6 ++-- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 19d79ee7c82..0a510c6a9f8 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -397,8 +397,9 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): def _create_lrouter_port(self, context, port): router_id = port['device_id'] - self._ovn_client._l3_plugin.add_router_interface( - context, router_id, {'port_id': port['id']}) + iface_info = self._ovn_client._l3_plugin._add_neutron_router_interface( + context, router_id, {'port_id': port['id']}, may_exist=True) + self._ovn_client.create_router_port(context, router_id, iface_info) def _check_subnet_global_dhcp_opts(self): inconsistent_subnets = [] diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index fddf66cbc5b..e9bf11a6801 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -177,12 +177,14 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, context, {'router': original_router}) def _add_neutron_router_interface(self, context, router_id, - interface_info): + interface_info, may_exist=False): try: router_interface_info = ( super(OVNL3RouterPlugin, self).add_router_interface( context, router_id, interface_info)) except n_exc.PortInUse: + if not may_exist: + raise # NOTE(lucasagomes): If the port is already being used it means # the interface has been created already, let's just fetch it from # the database. Perhaps the code below should live in Neutron diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index cf39a19d471..6f12dafd575 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -1457,6 +1457,38 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): exc.HTTPBadRequest.code) self.assertFalse(plugin.get_ports(context.get_admin_context())) + def test_router_add_interface_dup_port(self): + '''This tests that if multiple routers add one port as their + interfaces. Only the first router's interface would be added + to this port. All the later requests would return exceptions. + ''' + with self.router() as r1, self.router() as r2, self.network() as n: + with self.subnet(network=n) as s: + with self.port(subnet=s) as p: + self._router_interface_action('add', + r1['router']['id'], + None, + p['port']['id']) + # mock out the sequential check + plugin = 'neutron.db.l3_db.L3_NAT_dbonly_mixin' + check_p = mock.patch(plugin + '._check_router_port', + port_id=p['port']['id'], + device_id=r2['router']['id'], + return_value=p['port']) + checkport = check_p.start() + # do regular checkport after first skip + checkport.side_effect = check_p.stop() + self._router_interface_action('add', + r2['router']['id'], + None, + p['port']['id'], + exc.HTTPConflict.code) + # clean-up + self._router_interface_action('remove', + r1['router']['id'], + None, + p['port']['id']) + def test_update_router_interface_port_ipv6_subnet_ext_ra(self): use_cases = [{'msg': 'IPv6 Subnet Modes (none, slaac)', 'ra_mode': None, diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index a3ebe4a3b5d..e8a643babd5 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -262,9 +262,9 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, port = {'id': 'port-id', 'device_id': 'router-id'} self.periodic._create_lrouter_port(self.ctx, port) - l3_mock = self.periodic._ovn_client._l3_plugin - l3_mock.add_router_interface.assert_called_once_with( - self.ctx, port['device_id'], {'port_id': port['id']}) + ovn_client_mock = self.periodic._ovn_client + ovn_client_mock.create_router_port.assert_called_once_with( + self.ctx, port['device_id'], mock.ANY) @mock.patch.object(maintenance.LOG, 'debug') def test__log_maintenance_inconsistencies(self, mock_log):