From 2032397cf816f49e11dad68d5beb6e0b3dd96192 Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Fri, 23 Sep 2022 20:53:09 +0200 Subject: [PATCH 1/4] Check subnet overlapping after add router interface When simultaneous attempts are made to add an interface to the same router including overlapping networks in cidrs, both attempts are successful. There is a check to avoid this overlap but is performed when creating the network interface and it is done over the ports already attached to the router, so at this moment the check is not able to detect the overlapping. Furthermore, the create_port operation over the ML2 plugin must be executed in isolated transactions, so trying to control the execution context or adding additional steps to the transaction is not feasible. This patch checks once the RouterPort is created on the neutron database if there is more than one overlapping port, triggering in that case the exception that will remove the the culprit of overlapping. Conflicts: neutron/db/l3_db.py (manually cherry picked from commit 1abb77d7a63cde2aa9640351f663870c14430919) Closes-Bug: #1987666 Change-Id: I7cec8b53e72e7abf34012906e6adfecf079525af (cherry picked from commit 1abb77d7a63cde2aa9640351f663870c14430919) --- neutron/db/l3_db.py | 108 +++++++++++++----- neutron/tests/fullstack/test_l3_agent.py | 31 +++++ neutron/tests/unit/db/test_l3_db.py | 137 +++++++++++++++++++++++ 3 files changed, 246 insertions(+), 30 deletions(-) diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 026aa440acf..96a304dae12 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -13,6 +13,7 @@ # under the License. import functools +import itertools import random import netaddr @@ -563,6 +564,23 @@ def _check_for_external_ip_change(self, context, gw_port, ext_ips): ip_address_change = not ip_addresses == new_ip_addresses return ip_address_change + def _raise_on_subnets_overlap(self, subnet_1, subnet_2): + cidr = subnet_1['cidr'] + ipnet = netaddr.IPNetwork(cidr) + new_cidr = subnet_2['cidr'] + new_ipnet = netaddr.IPNetwork(new_cidr) + match1 = netaddr.all_matching_cidrs(new_ipnet, [cidr]) + match2 = netaddr.all_matching_cidrs(ipnet, [new_cidr]) + if match1 or match2: + data = {'subnet_cidr': new_cidr, + 'subnet_id': subnet_2['id'], + 'cidr': cidr, + 'sub_id': subnet_1['id']} + msg = (_("Cidr %(subnet_cidr)s of subnet " + "%(subnet_id)s overlaps with cidr %(cidr)s " + "of subnet %(sub_id)s") % data) + raise n_exc.BadRequest(resource='router', msg=msg) + def _ensure_router_not_in_use(self, context, router_id): """Ensure that no internal network interface is attached to the router. @@ -669,22 +687,8 @@ def _check_for_dup_router_subnets(self, context, router, subnets = self._core_plugin.get_subnets(context.elevated(), filters=id_filter) for sub in subnets: - cidr = sub['cidr'] - ipnet = netaddr.IPNetwork(cidr) for s in new_subnets: - new_cidr = s['cidr'] - new_ipnet = netaddr.IPNetwork(new_cidr) - match1 = netaddr.all_matching_cidrs(new_ipnet, [cidr]) - match2 = netaddr.all_matching_cidrs(ipnet, [new_cidr]) - if match1 or match2: - data = {'subnet_cidr': new_cidr, - 'subnet_id': s['id'], - 'cidr': cidr, - 'sub_id': sub['id']} - msg = (_("Cidr %(subnet_cidr)s of subnet " - "%(subnet_id)s overlaps with cidr %(cidr)s " - "of subnet %(sub_id)s") % data) - raise n_exc.BadRequest(resource='router', msg=msg) + self._raise_on_subnets_overlap(sub, s) def _get_device_owner(self, context, router=None): """Get device_owner for the specified router.""" @@ -765,17 +769,7 @@ def _validate_router_port_info(self, context, router, port_id): port = self._check_router_port(context, port_id, router.id) # Only allow one router port with IPv6 subnets per network id - if self._port_has_ipv6_address(port): - for existing_port in (rp.port for rp in router.attached_ports): - if (existing_port['network_id'] == port['network_id'] and - self._port_has_ipv6_address(existing_port)): - msg = _("Cannot have multiple router ports with the " - "same network id if both contain IPv6 " - "subnets. Existing port %(p)s has IPv6 " - "subnet(s) and network id %(nid)s") - raise n_exc.BadRequest(resource='router', msg=msg % { - 'p': existing_port['id'], - 'nid': existing_port['network_id']}) + self._validate_one_router_ipv6_port_per_network(router, port) fixed_ips = list(port['fixed_ips']) subnets = [] @@ -841,6 +835,20 @@ def _port_has_ipv6_address(self, port): if netaddr.IPNetwork(fixed_ip['ip_address']).version == 6: return True + def _validate_one_router_ipv6_port_per_network(self, router, port): + if self._port_has_ipv6_address(port): + for existing_port in (rp.port for rp in router.attached_ports): + if (existing_port["id"] != port["id"] and + existing_port["network_id"] == port["network_id"] and + self._port_has_ipv6_address(existing_port)): + msg = _("Router already contains IPv6 port %(p)s " + "belonging to network id %(nid)s. Only one IPv6 port " + "from the same network subnet can be connected to a " + "router.") + raise n_exc.BadRequest(resource='router', msg=msg % { + 'p': existing_port['id'], + 'nid': existing_port['network_id']}) + def _find_ipv6_router_port_by_network(self, context, router, net_id): router_dev_owner = self._get_device_owner(context, router) for port in router.attached_ports: @@ -959,7 +967,7 @@ def add_router_interface(self, context, router_id, interface_info=None): port=port, interface_info=interface_info) self._add_router_port( - context, port['id'], router.id, device_owner) + context, port['id'], router, device_owner) gw_ips = [] gw_network_id = None @@ -987,18 +995,58 @@ def add_router_interface(self, context, router_id, interface_info=None): subnets[-1]['id'], [subnet['id'] for subnet in subnets]) @db_api.retry_if_session_inactive() - def _add_router_port(self, context, port_id, router_id, device_owner): + def _add_router_port(self, context, port_id, router, device_owner): l3_obj.RouterPort( context, port_id=port_id, - router_id=router_id, + router_id=router.id, port_type=device_owner ).create() + + # NOTE(froyo): Check after create the RouterPort if we have generated + # an overlapping. Like creation of port is an ML2 plugin command it + # runs in an isolated transaction so we could not control there the + # addition of ports to different subnets that collides in cidrs. So + # make the check here an trigger the overlaping that will remove all + # created items. + router_ports = l3_obj.RouterPort.get_objects( + context, router_id=router.id) + + if len(router_ports) > 1: + subnets_id = [] + for rp in router_ports: + port = port_obj.Port.get_object(context.elevated(), + id=rp.port_id) + if port: + # Only allow one router port with IPv6 subnets per network + # id + self._validate_one_router_ipv6_port_per_network( + router, port) + subnets_id.extend([fixed_ip['subnet_id'] + for fixed_ip in port['fixed_ips']]) + else: + raise l3_exc.RouterInterfaceNotFound( + router_id=router.id, port_id=rp.port_id) + + if subnets_id: + id_filter = {'id': subnets_id} + subnets = self._core_plugin.get_subnets(context.elevated(), + filters=id_filter) + + # Ignore temporary Prefix Delegation CIDRs + subnets = [ + s + for s in subnets + if s["cidr"] != constants.PROVISIONAL_IPV6_PD_PREFIX + ] + for sub1, sub2 in itertools.combinations(subnets, 2): + self._raise_on_subnets_overlap(sub1, sub2) + # Update owner after actual process again in order to # make sure the records in routerports table and ports # table are consistent. self._core_plugin.update_port( - context, port_id, {'port': {'device_id': router_id, + context, port_id, {'port': {'device_id': router.id, 'device_owner': device_owner}}) def _check_router_interface_not_in_use(self, router_id, subnet): diff --git a/neutron/tests/fullstack/test_l3_agent.py b/neutron/tests/fullstack/test_l3_agent.py index 4fe977fb299..84603266dc5 100644 --- a/neutron/tests/fullstack/test_l3_agent.py +++ b/neutron/tests/fullstack/test_l3_agent.py @@ -18,6 +18,7 @@ import netaddr from neutron_lib import constants +from neutronclient.common import exceptions from oslo_utils import uuidutils from neutron.agent.l3 import ha_router @@ -266,6 +267,29 @@ def _is_filter_set(direction): return (_is_filter_set(constants.INGRESS_DIRECTION) and _is_filter_set(constants.EGRESS_DIRECTION)) + def _test_concurrent_router_subnet_attachment_overlapping_cidr(self, + ha=False): + tenant_id = uuidutils.generate_uuid() + subnet_cidr = '10.100.0.0/24' + network1 = self.safe_client.create_network( + tenant_id, name='foo-network1') + subnet1 = self.safe_client.create_subnet( + tenant_id, network1['id'], subnet_cidr) + network2 = self.safe_client.create_network( + tenant_id, name='foo-network2') + subnet2 = self.safe_client.create_subnet( + tenant_id, network2['id'], subnet_cidr) + router = self.safe_client.create_router(tenant_id, ha=ha) + + funcs = [self.safe_client.add_router_interface, + self.safe_client.add_router_interface] + args = [(router['id'], subnet1['id']), (router['id'], subnet2['id'])] + self.assertRaises( + exceptions.BadRequest, + self._simulate_concurrent_requests_process_and_raise, + funcs, + args) + class TestLegacyL3Agent(TestL3Agent): @@ -417,6 +441,9 @@ def test_external_subnet_changed(self): def test_router_fip_qos_after_admin_state_down_up(self): self._router_fip_qos_after_admin_state_down_up() + def test_concurrent_router_subnet_attachment_overlapping_cidr_(self): + self._test_concurrent_router_subnet_attachment_overlapping_cidr() + class TestHAL3Agent(TestL3Agent): @@ -573,3 +600,7 @@ def test_external_subnet_changed(self): def test_router_fip_qos_after_admin_state_down_up(self): self._router_fip_qos_after_admin_state_down_up(ha=True) + + def test_concurrent_router_subnet_attachment_overlapping_cidr_(self): + self._test_concurrent_router_subnet_attachment_overlapping_cidr( + ha=True) diff --git a/neutron/tests/unit/db/test_l3_db.py b/neutron/tests/unit/db/test_l3_db.py index 22003c38c13..fea1d1f738f 100644 --- a/neutron/tests/unit/db/test_l3_db.py +++ b/neutron/tests/unit/db/test_l3_db.py @@ -514,6 +514,93 @@ def test__validate_gw_info_delete_gateway_no_route(self): self.assertIsNone( self.db._validate_gw_info(mock.ANY, info, [], router)) + def test__raise_on_subnets_overlap_does_not_raise(self): + subnets = [ + {'id': uuidutils.generate_uuid(), + 'cidr': '10.1.0.0/24'}, + {'id': uuidutils.generate_uuid(), + 'cidr': '10.2.0.0/24'}] + self.db._raise_on_subnets_overlap(subnets[0], subnets[1]) + + def test__raise_on_subnets_overlap_raises(self): + subnets = [ + {'id': uuidutils.generate_uuid(), + 'cidr': '10.1.0.0/20'}, + {'id': uuidutils.generate_uuid(), + 'cidr': '10.1.10.0/24'}] + self.assertRaises( + n_exc.BadRequest, self.db._raise_on_subnets_overlap, subnets[0], + subnets[1]) + + def test__validate_one_router_ipv6_port_per_network(self): + port = models_v2.Port( + id=uuidutils.generate_uuid(), + network_id='foo_network', + fixed_ips=[models_v2.IPAllocation( + ip_address=str(netaddr.IPNetwork( + '2001:db8::/32').ip + 1), + subnet_id='foo_subnet')]) + rports = [l3_models.RouterPort(router_id='foo_router', port=port)] + router = l3_models.Router( + id='foo_router', attached_ports=rports, route_list=[], + gw_port_id=None) + new_port = models_v2.Port( + id=uuidutils.generate_uuid(), + network_id='foo_network2', + fixed_ips=[models_v2.IPAllocation( + ip_address=str(netaddr.IPNetwork( + '2001:db8::/32').ip + 2), + subnet_id='foo_subnet')]) + self.db._validate_one_router_ipv6_port_per_network( + router, new_port) + + def test__validate_one_router_ipv6_port_per_network_mix_ipv4_ipv6(self): + port = models_v2.Port( + id=uuidutils.generate_uuid(), + network_id='foo_network', + fixed_ips=[models_v2.IPAllocation( + ip_address=str(netaddr.IPNetwork( + '10.1.10.0/24').ip + 1), + subnet_id='foo_subnet')]) + rports = [l3_models.RouterPort(router_id='foo_router', port=port)] + router = l3_models.Router( + id='foo_router', attached_ports=rports, route_list=[], + gw_port_id=None) + new_port = models_v2.Port( + id=uuidutils.generate_uuid(), + network_id='foo_network', + fixed_ips=[models_v2.IPAllocation( + ip_address=str(netaddr.IPNetwork( + '2001:db8::/32').ip + 2), + subnet_id='foo_subnet')]) + self.db._validate_one_router_ipv6_port_per_network( + router, new_port) + + def test__validate_one_router_ipv6_port_per_network_failed(self): + port = models_v2.Port( + id=uuidutils.generate_uuid(), + network_id='foo_network', + fixed_ips=[models_v2.IPAllocation( + ip_address=str(netaddr.IPNetwork( + '2001:db8::/32').ip + 1), + subnet_id='foo_subnet')]) + rports = [l3_models.RouterPort(router_id='foo_router', port=port)] + router = l3_models.Router( + id='foo_router', attached_ports=rports, route_list=[], + gw_port_id=None) + new_port = models_v2.Port( + id=uuidutils.generate_uuid(), + network_id='foo_network', + fixed_ips=[models_v2.IPAllocation( + ip_address=str(netaddr.IPNetwork( + '2001:db8::/32').ip + 2), + subnet_id='foo_subnet')]) + self.assertRaises( + n_exc.BadRequest, + self.db._validate_one_router_ipv6_port_per_network, + router, + new_port) + class L3_NAT_db_mixin(base.BaseTestCase): def setUp(self): @@ -697,6 +784,56 @@ def test_remove_router_interface_by_subnet(self, mock_log): mock_log.warning.not_called_once() self._check_routerports((True, False)) + @mock.patch.object(l3_db.L3_NAT_dbonly_mixin, + '_check_for_dup_router_subnets') + @mock.patch.object(l3_db.L3_NAT_dbonly_mixin, + '_raise_on_subnets_overlap') + def test_add_router_interface_by_port_overlap_detected( + self, mock_raise_on_subnets_overlap, mock_check_dup): + # NOTE(froyo): On a normal behaviour this overlapping would be detected + # by _check_for_dup_router_subnets, in order to evalue the code + # implemented to cover the race condition when two ports are added + # simultaneously using colliding cidrs we need to "fake" this method + # to overpass it and check we achieve the code part that cover the case + mock_check_dup.return_value = True + network2 = self.create_network('network2') + subnet = self.create_subnet(network2, '1.1.1.1', '1.1.1.0/24') + ipa = str(netaddr.IPNetwork(subnet['subnet']['cidr']).ip + 10) + fixed_ips = [{'subnet_id': subnet['subnet']['id'], 'ip_address': ipa}] + port = self.create_port( + network2['network']['id'], {'fixed_ips': fixed_ips}) + self.mixin.add_router_interface( + self.ctx, self.router['id'], + interface_info={'port_id': port['port']['id']}) + mock_raise_on_subnets_overlap.assert_not_called() + self.mixin.add_router_interface( + self.ctx, self.router['id'], + interface_info={'port_id': self.ports[0]['port']['id']}) + mock_raise_on_subnets_overlap.assert_called_once() + + @mock.patch.object(l3_db.L3_NAT_dbonly_mixin, + '_check_for_dup_router_subnets') + @mock.patch.object(l3_db.L3_NAT_dbonly_mixin, + '_raise_on_subnets_overlap') + def test_add_router_interface_by_subnet_overlap_detected( + self, mock_raise_on_subnets_overlap, mock_check_dup): + # NOTE(froyo): On a normal behaviour this overlapping would be detected + # by _check_for_dup_router_subnets, in order to evalue the code + # implemented to cover the race condition when two ports are added + # simultaneously using colliding cidrs we need to "fake" this method + # to overpass it and check we achieve the code part that cover the case + mock_check_dup.return_value = True + network2 = self.create_network('network2') + subnet = self.create_subnet(network2, '1.1.1.1', '1.1.1.0/24') + self.mixin.add_router_interface( + self.ctx, self.router['id'], + interface_info={'subnet_id': subnet['subnet']['id']}) + mock_raise_on_subnets_overlap.assert_not_called() + self.mixin.add_router_interface( + self.ctx, self.router['id'], + interface_info={'subnet_id': self.subnets[0]['subnet']['id']}) + mock_raise_on_subnets_overlap.assert_called_once() + @mock.patch.object(port_obj, 'LOG') def test_remove_router_interface_by_subnet_removed_rport(self, mock_log): self._add_router_interfaces() From 32fde52371a7f9fbaa3d833efa687091aa10927f Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Wed, 16 Nov 2022 14:07:00 +0000 Subject: [PATCH 2/4] OVN: Add support for DHCP option "domain-search" for IPv4 Nothing much else, what the title says... Change-Id: Ib1d41a6e4c869e108f31c1eb604f22c794d66467 Closes-Bug: #1996759 Signed-off-by: Lucas Alvares Gomes (cherry picked from commit bf44e70db6219e7f3a45bd61b7dd14a31ae33bb0) --- doc/source/ovn/dhcp_opts.rst | 2 ++ neutron/common/ovn/constants.py | 3 +++ neutron/tests/unit/common/ovn/test_utils.py | 14 ++++++++++++++ 3 files changed, 19 insertions(+) diff --git a/doc/source/ovn/dhcp_opts.rst b/doc/source/ovn/dhcp_opts.rst index d4bd459a161..f85a560c4f1 100644 --- a/doc/source/ovn/dhcp_opts.rst +++ b/doc/source/ovn/dhcp_opts.rst @@ -17,6 +17,7 @@ classless-static-route classless_static_route default-ttl default_ttl dns-server dns_server domain-name domain_name +domain-search domain_search_list ethernet-encap ethernet_encap ip-forward-enable ip_forward_enable lease-time lease_time @@ -67,6 +68,7 @@ wpad wpad 59 T2 66 tftp_server 67 bootfile_name +119 domain_search_list 121 classless_static_route 150 tftp_server_address 210 path_prefix diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 46ec0ade093..17330931c51 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -110,6 +110,7 @@ 'log-server': 'log_server', 'lpr-server': 'lpr_server', 'domain-name': 'domain_name', + 'domain-search': 'domain_search_list', 'swap-server': 'swap_server', 'policy-filter': 'policy_filter', 'router-solicitation': 'router_solicitation', @@ -160,6 +161,7 @@ '58': 'T1', '59': 'T2', '67': 'bootfile_name', + '119': 'domain_search_list', '252': 'wpad', '210': 'path_prefix', '150': 'tftp_server_address'}, @@ -176,6 +178,7 @@ # OVN string type DHCP options OVN_STR_TYPE_DHCP_OPTS = [ 'domain_name', + 'domain_search_list', 'bootfile_name', 'path_prefix', 'wpad', diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index 90a33486a4c..1336e5a746b 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -372,6 +372,20 @@ def test_get_lsp_dhcp_opts(self): 'bootfile_name': '"homer_simpson.bin"'} self.assertEqual(expected_options, options) + def test_get_lsp_dhcp_opts_for_domain_search(self): + opt = {'opt_name': 'domain-search', + 'opt_value': 'openstack.org,ovn.org', + 'ip_version': 4} + port = {portbindings.VNIC_TYPE: portbindings.VNIC_NORMAL, + edo_ext.EXTRADHCPOPTS: [opt]} + + dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4) + self.assertFalse(dhcp_disabled) + # Assert option got translated to "domain_search_list" and + # the value is a string (double-quoted) + expected_options = {'domain_search_list': '"openstack.org,ovn.org"'} + self.assertEqual(expected_options, options) + class TestGetDhcpDnsServers(base.BaseTestCase): From 44ec1f150338ec69a811e1b2cfa4214c07a9af83 Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Thu, 28 Apr 2022 17:52:55 +0200 Subject: [PATCH 3/4] Avoid register config options on imports Continue similar approach following in [1], where some project imports collide with config options. As part of the change, a wrapped decorator has been implemented to cover those functions that include any of the ovn config options as value to the decorators arguments (e.g. tenacity retry). This way we avoid requiring the options to be registered as soon as the module is imported, where they have not yet been registered by a main process. [1] https://review.opendev.org/c/openstack/neutron/+/837392 Co-authored-by: Jakub Libosvar Co-authored-by: Fernando Royo Conflicts: neutron/agent/ovn/metadata_agent.py neutron/cmd/ovn/neutron_ovn_db_sync_util.py neutron/common/ovn/utils.py neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py Change-Id: I4bccb094ee7f690cbc352c38b5b39d505e6ea460 (cherry picked from commit 227c5f8568d0b2499db0d20c36eb65a07e56a893) --- neutron/agent/ovn/metadata/agent.py | 6 +-- neutron/agent/ovn/metadata/ovsdb.py | 11 ++-- neutron/agent/ovn/metadata_agent.py | 2 + neutron/cmd/ovn/neutron_ovn_db_sync_util.py | 1 + neutron/common/ovn/utils.py | 12 +++++ .../conf/plugins/ml2/drivers/ovn/ovn_conf.py | 6 ++- .../drivers/ovn/mech_driver/mech_driver.py | 1 + neutron/tests/functional/base.py | 1 + .../ovn/mech_driver/ovsdb/test_impl_idl.py | 2 + .../ovn/mech_driver/test_mech_driver.py | 1 + .../unit/agent/ovn/metadata/test_agent.py | 2 + .../unit/agent/ovn/metadata/test_driver.py | 2 + neutron/tests/unit/common/ovn/test_utils.py | 51 +++++++++++++++++++ .../mech_driver/ovsdb/extensions/test_qos.py | 2 + .../ovn/mech_driver/ovsdb/test_maintenance.py | 1 + .../ovn/mech_driver/ovsdb/test_ovn_client.py | 2 + .../mech_driver/ovsdb/test_ovsdb_monitor.py | 5 ++ .../ovn/mech_driver/test_mech_driver.py | 2 + 18 files changed, 95 insertions(+), 15 deletions(-) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index d77ee28cdf8..accdff80622 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -24,7 +24,6 @@ from oslo_utils import netutils from ovsdbapp.backend.ovs_idl import event as row_event from ovsdbapp.backend.ovs_idl import vlog -import tenacity from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib @@ -280,10 +279,7 @@ def start(self): self._proxy.wait() - @tenacity.retry( - wait=tenacity.wait_exponential( - max=config.get_ovn_ovsdb_retry_max_interval()), - reraise=True) + @ovn_utils.retry() def register_metadata_agent(self): # NOTE(lucasagomes): db_add() will not overwrite the UUID if # it's already set. diff --git a/neutron/agent/ovn/metadata/ovsdb.py b/neutron/agent/ovn/metadata/ovsdb.py index 7817bdaf85c..d5ba2d66803 100644 --- a/neutron/agent/ovn/metadata/ovsdb.py +++ b/neutron/agent/ovn/metadata/ovsdb.py @@ -17,8 +17,8 @@ from ovsdbapp.backend.ovs_idl import connection from ovsdbapp.backend.ovs_idl import idlutils from ovsdbapp.schema.open_vswitch import impl_idl as idl_ovs -import tenacity +from neutron.common.ovn import utils as ovn_utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor @@ -54,16 +54,11 @@ def __init__(self, chassis=None, events=None, tables=None): if events: self.notify_handler.watch_events(events) - @tenacity.retry( - wait=tenacity.wait_exponential(max=180), - reraise=True) + @ovn_utils.retry(max_=180) def _get_ovsdb_helper(self, connection_string): return idlutils.get_schema_helper(connection_string, self.SCHEMA) - @tenacity.retry( - wait=tenacity.wait_exponential( - max=config.get_ovn_ovsdb_retry_max_interval()), - reraise=True) + @ovn_utils.retry() def start(self): LOG.info('Getting OvsdbSbOvnIdl for MetadataAgent with retry') conn = connection.Connection( diff --git a/neutron/agent/ovn/metadata_agent.py b/neutron/agent/ovn/metadata_agent.py index 3b0656368d0..f164fe46e17 100644 --- a/neutron/agent/ovn/metadata_agent.py +++ b/neutron/agent/ovn/metadata_agent.py @@ -21,11 +21,13 @@ from neutron.agent.ovn.metadata import agent from neutron.conf.agent.metadata import config as meta from neutron.conf.agent.ovn.metadata import config as ovn_meta +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf LOG = logging.getLogger(__name__) def main(): + ovn_conf.register_opts() ovn_meta.register_meta_conf_opts(meta.SHARED_OPTS) ovn_meta.register_meta_conf_opts(meta.UNIX_DOMAIN_METADATA_PROXY_OPTS) ovn_meta.register_meta_conf_opts(meta.METADATA_PROXY_HANDLER_OPTS) diff --git a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py index 274eb29ab34..34c838af84e 100644 --- a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py +++ b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py @@ -137,6 +137,7 @@ def security_groups_provider_updated(self, context, def setup_conf(): conf = cfg.CONF + ovn_conf.register_opts() ml2_group, ml2_opts = neutron_options.list_ml2_conf_opts()[0] cfg.CONF.register_cli_opts(ml2_opts, ml2_group) cfg.CONF.register_cli_opts(securitygroups_rpc.security_group_opts, diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 8acda0f175f..f8709414ccc 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -33,6 +33,7 @@ from oslo_utils import netutils from oslo_utils import strutils from ovsdbapp import constants as ovsdbapp_const +import tenacity from neutron._i18n import _ from neutron.common.ovn import constants @@ -685,3 +686,14 @@ def is_port_external(port): return (vnic_type in constants.EXTERNAL_PORT_TYPES and constants.PORT_CAP_SWITCHDEV not in capabilities) + + +def retry(max_=None): + def inner(func): + def wrapper(*args, **kwargs): + local_max = max_ or ovn_conf.get_ovn_ovsdb_retry_max_interval() + return tenacity.retry( + wait=tenacity.wait_exponential(max=local_max), + reraise=True)(func)(*args, **kwargs) + return wrapper + return inner diff --git a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py index e8652081904..3172f891a85 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py @@ -199,8 +199,10 @@ 'br-int | grep "Check pkt length action".')), ] -cfg.CONF.register_opts(ovn_opts, group='ovn') -ovs_conf.register_ovs_agent_opts() + +def register_opts(): + cfg.CONF.register_opts(ovn_opts, group='ovn') + ovs_conf.register_ovs_agent_opts() def list_opts(): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 690789ab50b..621b4c14263 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -117,6 +117,7 @@ def initialize(self): self.node_uuid = None self.hash_ring_group = ovn_const.HASH_RING_ML2_GROUP self.sg_enabled = ovn_acl.is_sg_enabled() + ovn_conf.register_opts() self._post_fork_event = threading.Event() if cfg.CONF.SECURITYGROUP.firewall_driver: LOG.warning('Firewall driver configuration is ignored') diff --git a/neutron/tests/functional/base.py b/neutron/tests/functional/base.py index dc1c70af7b8..ade4c7af93e 100644 --- a/neutron/tests/functional/base.py +++ b/neutron/tests/functional/base.py @@ -184,6 +184,7 @@ def setUp(self, maintenance_worker=False, service_plugins=None): # ensure viable minimum is set for OVN's Geneve ml2_config.cfg.CONF.set_override('max_header_size', 38, group='ml2_type_geneve') + ovn_conf.register_opts() ovn_conf.cfg.CONF.set_override('dns_servers', ['10.10.10.10'], group='ovn') diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py index 04dd99119ca..d139385ea34 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py @@ -22,6 +22,7 @@ from ovsdbapp.tests import utils from neutron.common.ovn import constants as ovn_const +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb \ import impl_idl_ovn as impl from neutron.services.portforwarding import constants as pf_const @@ -38,6 +39,7 @@ class BaseOvnIdlTest(n_base.BaseLoggingTestCase, def setUp(self): super(BaseOvnIdlTest, self).setUp() + ovn_conf.register_opts() self.api = impl.OvsdbSbOvnIdl(self.connection['OVN_Southbound']) self.nbapi = impl.OvsdbNbOvnIdl(self.connection['OVN_Northbound']) self.handler = ovsdb_event.RowEventHandler() diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index c3630993e2c..2bbe7f93377 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -801,6 +801,7 @@ class TestCreateDefaultDropPortGroup(base.BaseLoggingTestCase, def setUp(self): super(TestCreateDefaultDropPortGroup, self).setUp() + ovn_conf.register_opts() self.api = impl_idl_ovn.OvsdbNbOvnIdl( self.connection['OVN_Northbound']) self.addCleanup(self.api.pg_del(self.PG_NAME, if_exists=True).execute, diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 8a83e5c0ba1..cf18de05ef3 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_agent.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_agent.py @@ -30,6 +30,7 @@ from neutron.agent.ovn.metadata import driver from neutron.conf.agent.metadata import config as meta_conf from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.tests import base @@ -53,6 +54,7 @@ def setUp(self): meta_conf.METADATA_PROXY_HANDLER_OPTS, self.conf) ovn_meta_conf.register_meta_conf_opts( ovn_meta_conf.OVS_OPTS, self.conf, group='ovs') + ovn_conf.register_opts() class TestMetadataAgent(base.BaseTestCase): diff --git a/neutron/tests/unit/agent/ovn/metadata/test_driver.py b/neutron/tests/unit/agent/ovn/metadata/test_driver.py index 2a497c7aeb6..e5b2abd28d4 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_driver.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_driver.py @@ -24,6 +24,7 @@ from neutron.agent.ovn.metadata import driver as metadata_driver from neutron.conf.agent.metadata import config as meta_conf from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.tests import base from neutron.tests.unit.agent.linux import test_utils @@ -44,6 +45,7 @@ def setUp(self): mock.patch('eventlet.spawn').start() ovn_meta_conf.register_meta_conf_opts(meta_conf.SHARED_OPTS, cfg.CONF) + ovn_conf.register_opts() def test_spawn_metadata_proxy(self): datapath_id = _uuid() diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index 90a33486a4c..590b55f9b99 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -26,6 +26,7 @@ from neutron.common.ovn import constants from neutron.common.ovn import utils +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.tests import base from neutron.tests.unit import fake_resources as fakes @@ -375,6 +376,10 @@ def test_get_lsp_dhcp_opts(self): class TestGetDhcpDnsServers(base.BaseTestCase): + def setUp(self): + ovn_conf.register_opts() + super(TestGetDhcpDnsServers, self).setUp() + def test_ipv4(self): # DNS servers from subnet. dns_servers = utils.get_dhcp_dns_servers( @@ -701,3 +706,49 @@ def test_capability_only_allowed(self): utils.validate_and_get_data_from_binding_profile( {portbindings.VNIC_TYPE: self.VNIC_FAKE_OTHER, constants.OVN_PORT_BINDING_PROFILE: binding_profile})) + + +class TestRetryDecorator(base.BaseTestCase): + DEFAULT_RETRY_VALUE = 10 + + def setUp(self): + super().setUp() + mock.patch.object( + ovn_conf, "get_ovn_ovsdb_retry_max_interval", + return_value=self.DEFAULT_RETRY_VALUE).start() + + def test_default_retry_value(self): + with mock.patch('tenacity.wait_exponential') as m_wait: + @utils.retry() + def decorated_method(): + pass + + decorated_method() + m_wait.assert_called_with(max=self.DEFAULT_RETRY_VALUE) + + def test_custom_retry_value(self): + custom_value = 3 + with mock.patch('tenacity.wait_exponential') as m_wait: + @utils.retry(max_=custom_value) + def decorated_method(): + pass + + decorated_method() + m_wait.assert_called_with(max=custom_value) + + def test_positive_result(self): + number_of_exceptions = 3 + method = mock.Mock( + side_effect=[Exception() for i in range(number_of_exceptions)]) + + @utils.retry(max_=0.001) + def decorated_method(): + try: + method() + except StopIteration: + return + + decorated_method() + + # number of exceptions + one successful call + self.assertEqual(number_of_exceptions + 1, method.call_count) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py index 8514c7dbeee..55cca24022b 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py @@ -26,6 +26,7 @@ from neutron.api import extensions from neutron.common.ovn import constants as ovn_const +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.core_extensions import qos as core_qos from neutron.objects import network as network_obj from neutron.objects import ports as port_obj @@ -61,6 +62,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): 'TestFloatingIPQoSL3NatServicePlugin') def setUp(self): + ovn_conf.register_opts() cfg.CONF.set_override('extension_drivers', self._extension_drivers, group='ml2') cfg.CONF.set_override('enable_distributed_floating_ip', 'False', 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 61e06d27070..f6dabff4bbf 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 @@ -83,6 +83,7 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, test_sg.Ml2SecurityGroupsTestCase): def setUp(self): + ovn_conf.register_opts() super(TestDBInconsistenciesPeriodics, self).setUp() self.net = self._make_network( self.fmt, name='net1', admin_state_up=True)['network'] diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py index 8333d2281be..aff755e0696 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py @@ -16,6 +16,7 @@ from unittest import mock from neutron.common.ovn import constants +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client from neutron.tests import base from neutron.tests.unit import fake_resources as fakes @@ -25,6 +26,7 @@ class TestOVNClientBase(base.BaseTestCase): def setUp(self): + ovn_conf.register_opts() super(TestOVNClientBase, self).setUp() self.nb_idl = mock.MagicMock() self.sb_idl = mock.MagicMock() diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index ea875546265..e7653d8b333 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -198,6 +198,7 @@ def test_connection_sb_start(self): class TestOvnIdlDistributedLock(base.BaseTestCase): def setUp(self): + ovn_conf.register_opts() super(TestOvnIdlDistributedLock, self).setUp() self.node_uuid = uuidutils.generate_uuid() self.fake_driver = mock.Mock() @@ -724,6 +725,10 @@ def test_handle_ha_chassis_group_changes_update_new_gw(self): class TestShortLivingOvsdbApi(base.BaseTestCase): + def setUp(self): + ovn_conf.register_opts() + super().setUp() + def test_context(self): api_class = mock.Mock() idl = mock.Mock() diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 18c7fa37b2d..8d5a94fd2d9 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -140,6 +140,7 @@ def setUp(self): # ensure viable minimum is set for OVN's Geneve cfg.CONF.set_override('max_header_size', 38, group='ml2_type_geneve') + ovn_conf.register_opts() ovn_conf.cfg.CONF.set_override('ovn_metadata_enabled', False, group='ovn') ovn_conf.cfg.CONF.set_override('dns_servers', ['8.8.8.8'], @@ -2588,6 +2589,7 @@ class OVNMechanismDriverTestCase(MechDriverSetupBase, _mechanism_drivers = ['logger', 'ovn'] def setUp(self): + ovn_conf.register_opts() cfg.CONF.set_override('global_physnet_mtu', 1550) cfg.CONF.set_override('tenant_network_types', ['geneve'], From 0d0e6cd47b7a0f53d9eb1549eed4e540f76508c9 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Tue, 11 Oct 2022 13:38:27 -0400 Subject: [PATCH 4/4] ovn: Use ovsdb-client to create neutron_pg_drop Previously we used short living OVN database connection to create neutron_pg_drop Port Group before workers were spawned. The pre_fork_initialize actually happens after the api workers are spawned anyways and it blocks spawning of other workers, such as maintenance, rpc or periodic. If the OVN database was large it may take several minutes to connect to the database at scale and this blocks spawning of other workers. That means connecting to OVN in pre_fork is not a good idea. This patch replaces the mechanism by using ovsdb-client to send a transaction without connecting to the database and downloading the whole content. The command does following, everything is on the server side: 1) With timeout 0 it waits for neutron_pg_drop Port Group. If the PG is present, the transaction finishes and nothing happens. 2) If the PG is not present, it times out immediately and commits new entries that effectivelly creates neutron_pg_drop Port Group with implicit ACLs to block ingress and egress traffic. Conflicts: neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py Closes-Bug: #1991579 Co-Authored-By: Terry Wilson Change-Id: I27af495f96a3ea88dd31345dbfb55f1be8faabd6 (cherry picked from commit 50eee19723b2d762c2b5653e7a2c75e470f44cdf) (cherry picked from commit 5deea002aa4da506dc7c931c97cfc2414e66868b) --- neutron/common/ovn/utils.py | 118 ++++++++++++++++++ .../drivers/ovn/mech_driver/mech_driver.py | 88 ++----------- .../ovn/mech_driver/ovsdb/ovn_client.py | 21 ++-- .../ovn/mech_driver/ovsdb/ovsdb_monitor.py | 62 --------- .../tests/functional/common/ovn/test_utils.py | 60 +++++++++ .../ovn/mech_driver/test_mech_driver.py | 56 --------- neutron/tests/unit/common/ovn/test_utils.py | 116 +++++++++++++++++ .../mech_driver/ovsdb/test_ovsdb_monitor.py | 34 ----- .../ovn/mech_driver/test_mech_driver.py | 51 -------- 9 files changed, 315 insertions(+), 291 deletions(-) create mode 100644 neutron/tests/functional/common/ovn/test_utils.py diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index f8709414ccc..011849b07de 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -27,6 +27,7 @@ from neutron_lib import exceptions as n_exc from neutron_lib.plugins import directory from neutron_lib.utils import net as n_utils +from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log from oslo_serialization import jsonutils @@ -56,6 +57,70 @@ 'PortExtraDHCPValidation', ['failed', 'invalid_ipv4', 'invalid_ipv6']) +class OvsdbClientCommand(object): + _CONNECTION = 0 + _PRIVATE_KEY = 1 + _CERTIFICATE = 2 + _CA_AUTHORITY = 3 + + OVN_Northbound = "OVN_Northbound" + OVN_Southbound = "OVN_Southbound" + + _db_settings = { + OVN_Northbound: { + _CONNECTION: ovn_conf.get_ovn_nb_connection, + _PRIVATE_KEY: ovn_conf.get_ovn_nb_private_key, + _CERTIFICATE: ovn_conf.get_ovn_nb_certificate, + _CA_AUTHORITY: ovn_conf.get_ovn_nb_ca_cert, + }, + OVN_Southbound: { + _CONNECTION: ovn_conf.get_ovn_sb_connection, + _PRIVATE_KEY: ovn_conf.get_ovn_sb_private_key, + _CERTIFICATE: ovn_conf.get_ovn_sb_certificate, + _CA_AUTHORITY: ovn_conf.get_ovn_sb_ca_cert, + }, + } + + @classmethod + def run(cls, command): + """Run custom ovsdb protocol command. + + :param command: JSON object of ovsdb protocol command + """ + try: + db = command[0] + except IndexError: + raise KeyError( + _("%s or %s schema must be specified in the command %s" % ( + cls.OVN_Northbound, cls.OVN_Southbound, command))) + + if db not in (cls.OVN_Northbound, cls.OVN_Southbound): + raise KeyError( + _("%s or %s schema must be specified in the command %s" % ( + cls.OVN_Northbound, cls.OVN_Southbound, command))) + + cmd = ['ovsdb-client', + cls.COMMAND, + cls._db_settings[db][cls._CONNECTION](), + '--timeout', + str(ovn_conf.get_ovn_ovsdb_timeout())] + + if cls._db_settings[db][cls._PRIVATE_KEY](): + cmd += ['-p', cls._db_settings[db][cls._PRIVATE_KEY](), + '-c', cls._db_settings[db][cls._CERTIFICATE](), + '-C', cls._db_settings[db][cls._CA_AUTHORITY]()] + + cmd.append(jsonutils.dumps(command)) + + return processutils.execute( + *cmd, + log_errors=processutils.LOG_FINAL_ERROR) + + +class OvsdbClientTransactCommand(OvsdbClientCommand): + COMMAND = 'transact' + + def ovn_name(id): # The name of the OVN entry will be neutron- # This is due to the fact that the OVN application checks if the name @@ -697,3 +762,56 @@ def wrapper(*args, **kwargs): reraise=True)(func)(*args, **kwargs) return wrapper return inner + + +def create_neutron_pg_drop(): + """Create neutron_pg_drop Port Group. + + It uses ovsdb-client to send to server transact command using ovsdb + protocol that checks if the neutron_pg_drop row exists. If it exists + it times out immediatelly. If it doesn't exist then it creates the + Port_Group and default ACLs to drop all ingress and egress traffic. + """ + command = [ + "OVN_Northbound", { + "op": "wait", + "timeout": 0, + "table": "Port_Group", + "where": [ + ["name", "==", constants.OVN_DROP_PORT_GROUP_NAME] + ], + "until": "==", + "rows": [] + }, { + "op": "insert", + "table": "ACL", + "row": { + "action": "drop", + "direction": "to-lport", + "match": "outport == @neutron_pg_drop && ip", + "priority": 1001 + }, + "uuid-name": "droptoport" + }, { + "op": "insert", + "table": "ACL", + "row": { + "action": "drop", + "direction": "from-lport", + "match": "inport == @neutron_pg_drop && ip", + "priority": 1001 + }, + "uuid-name": "dropfromport" + }, { + "op": "insert", + "table": "Port_Group", + "row": { + "name": constants.OVN_DROP_PORT_GROUP_NAME, + "acls": ["set", [ + ["named-uuid", "droptoport"], + ["named-uuid", "dropfromport"] + ]] + } + }] + + OvsdbClientTransactCommand.run(command) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 621b4c14263..6f6faca40be 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -36,7 +36,6 @@ from neutron_lib.plugins.ml2 import api from neutron_lib.utils import helpers from oslo_concurrency import lockutils -from oslo_concurrency import processutils from oslo_config import cfg from oslo_db import exception as os_db_exc from oslo_log import log @@ -59,7 +58,6 @@ from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import maintenance from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import worker from neutron import service from neutron.services.logapi.drivers.ovn import driver as log_driver @@ -279,47 +277,7 @@ def pre_fork_initialize(self, resource, event, trigger, payload=None): """Pre-initialize the ML2/OVN driver.""" atexit.register(self._clean_hash_ring) signal.signal(signal.SIGTERM, self._clean_hash_ring) - self._create_neutron_pg_drop() - - def _create_neutron_pg_drop(self): - """Create neutron_pg_drop Port Group. - - The method creates a short living connection to the Northbound - database. Because of multiple controllers can attempt to create the - Port Group at the same time the transaction can fail and raise - RuntimeError. In such case, we make sure the Port Group was created, - otherwise the error is something else and it's raised to the caller. - """ - idl = ovsdb_monitor.OvnInitPGNbIdl.from_server( - ovn_conf.get_ovn_nb_connection(), self.nb_schema_helper, self) - # Only one server should try to create the port group - idl.set_lock('pg_drop_creation') - with ovsdb_monitor.short_living_ovsdb_api( - impl_idl_ovn.OvsdbNbOvnIdl, idl) as pre_ovn_nb_api: - try: - create_default_drop_port_group(pre_ovn_nb_api) - except KeyError: - # Due to a bug in python-ovs, we can send transactions before - # the initial OVSDB is populated in memory. This can break - # the AddCommand post_commit method which tries to return a - # row looked up by the newly commited row's uuid. Since we - # don't care about the return value from the PgAddCommand, we - # can just catch the KeyError and continue. This can be - # removed when the python-ovs bug is resolved. - pass - except RuntimeError as re: - # If we don't get the lock, and the port group didn't exist - # when we tried to create it, it might still have been - # created by another server and we just haven't gotten the - # update yet. - LOG.info("Waiting for Port Group %(pg)s to be created", - {'pg': ovn_const.OVN_DROP_PORT_GROUP_NAME}) - if not idl.neutron_pg_drop_event.wait(): - LOG.error("Port Group %(pg)s was not created in time", - {'pg': ovn_const.OVN_DROP_PORT_GROUP_NAME}) - raise re - LOG.info("Porg Group %(pg)s was created by another server", - {'pg': ovn_const.OVN_DROP_PORT_GROUP_NAME}) + ovn_utils.create_neutron_pg_drop() @staticmethod def should_post_fork_initialize(worker_class): @@ -1191,19 +1149,17 @@ def set_port_status_down(self, port_id): def delete_mac_binding_entries(self, external_ip): """Delete all MAC_Binding entries associated to this IP address""" - cmd = ['ovsdb-client', 'transact', ovn_conf.get_ovn_sb_connection(), - '--timeout', str(ovn_conf.get_ovn_ovsdb_timeout())] - - if ovn_conf.get_ovn_sb_private_key(): - cmd += ['-p', ovn_conf.get_ovn_sb_private_key(), '-c', - ovn_conf.get_ovn_sb_certificate(), '-C', - ovn_conf.get_ovn_sb_ca_cert()] - - cmd += ['["OVN_Southbound", {"op": "delete", "table": "MAC_Binding", ' - '"where": [["ip", "==", "%s"]]}]' % external_ip] + cmd = [ + "OVN_Southbound", { + "op": "delete", + "table": "MAC_Binding", + "where": [ + ["ip", "==", external_ip] + ] + } + ] - return processutils.execute(*cmd, - log_errors=processutils.LOG_FINAL_ERROR) + return ovn_utils.OvsdbClientTransactCommand.run(cmd) def update_segment_host_mapping(self, host, phy_nets): """Update SegmentHostMapping in DB""" @@ -1375,28 +1331,6 @@ def delete_agent(self, context, id, _driver=None): if_exists=True).execute(check_error=True) -def create_default_drop_port_group(nb_idl): - pg_name = ovn_const.OVN_DROP_PORT_GROUP_NAME - if nb_idl.get_port_group(pg_name): - LOG.debug("Port Group %s already exists", pg_name) - return - with nb_idl.transaction(check_error=True) as txn: - # If drop Port Group doesn't exist yet, create it. - txn.add(nb_idl.pg_add(pg_name, acls=[], may_exist=True)) - # Add ACLs to this Port Group so that all traffic is dropped. - acls = ovn_acl.add_acls_for_drop_port_group(pg_name) - for acl in acls: - txn.add(nb_idl.pg_acl_add(may_exist=True, **acl)) - - ports_with_pg = set() - for pg in nb_idl.get_sg_port_groups().values(): - ports_with_pg.update(pg['ports']) - - if ports_with_pg: - # Add the ports to the default Port Group - txn.add(nb_idl.pg_add_ports(pg_name, list(ports_with_pg))) - - def get_availability_zones(cls, context, _driver, filters=None, fields=None, sorts=None, limit=None, marker=None, page_reverse=False): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 3aac733a075..2b1762677e2 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -33,7 +33,6 @@ from neutron_lib.plugins import utils as p_utils from neutron_lib.utils import helpers from neutron_lib.utils import net as n_net -from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log from oslo_utils import excutils @@ -1676,16 +1675,16 @@ def delete_mac_binding_entries_by_mac(self, mac): is refer to patch: https://review.opendev.org/c/openstack/neutron/+/812805 """ - cmd = ['ovsdb-client', 'transact', ovn_conf.get_ovn_sb_connection(), - '--timeout', str(ovn_conf.get_ovn_ovsdb_timeout())] - if ovn_conf.get_ovn_sb_private_key(): - cmd += ['-p', ovn_conf.get_ovn_sb_private_key(), '-c', - ovn_conf.get_ovn_sb_certificate(), '-C', - ovn_conf.get_ovn_sb_ca_cert()] - cmd += ['["OVN_Southbound", {"op": "delete", "table": "MAC_Binding", ' - '"where": [["mac", "==", "%s"]]}]' % mac] - return processutils.execute(*cmd, - log_errors=processutils.LOG_FINAL_ERROR) + cmd = [ + "OVN_Southbound", { + "op": "delete", + "table": "MAC_Binding", + "where": [ + ["mac", "==", mac] + ] + } + ] + return utils.OvsdbClientTransactCommand.run(cmd) def _delete_lrouter_port(self, context, port_id, router_id=None, txn=None): """Delete a logical router port.""" diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index d2fd9780e31..21866b77983 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -13,7 +13,6 @@ # under the License. import abc -import contextlib import datetime from neutron_lib import context as neutron_context @@ -581,17 +580,6 @@ def run(self, event, row, old): self.driver.delete_mac_binding_entries(row.external_ip) -class NeutronPgDropPortGroupCreated(row_event.WaitEvent): - """WaitEvent for neutron_pg_drop Create event.""" - def __init__(self, timeout=None): - table = 'Port_Group' - events = (self.ROW_CREATE,) - conditions = (('name', '=', ovn_const.OVN_DROP_PORT_GROUP_NAME),) - super(NeutronPgDropPortGroupCreated, self).__init__( - events, table, conditions, timeout=timeout) - self.event_name = 'PortGroupCreated' - - class OvnDbNotifyHandler(row_event.RowEventHandler): def __init__(self, driver): self.driver = driver @@ -837,56 +825,6 @@ def post_connect(self): PortBindingChassisUpdateEvent(self.driver)]) -class OvnInitPGNbIdl(OvnIdl): - """Very limited OVN NB IDL. - - This IDL is intended to be used only in initialization phase with short - living DB connections. - """ - - tables = ['Port_Group', 'Logical_Switch_Port', 'ACL'] - - def __init__(self, driver, remote, schema): - super(OvnInitPGNbIdl, self).__init__(driver, remote, schema) - self.set_table_condition( - 'Port_Group', [['name', '==', ovn_const.OVN_DROP_PORT_GROUP_NAME]]) - self.neutron_pg_drop_event = NeutronPgDropPortGroupCreated( - timeout=ovn_conf.get_ovn_ovsdb_timeout()) - self.notify_handler.watch_event(self.neutron_pg_drop_event) - - def notify(self, event, row, updates=None): - # Go ahead and process events even if the lock is contended so we can - # know that some other server has created the drop group - self.notify_handler.notify(event, row, updates) - - @classmethod - def from_server(cls, connection_string, helper, driver, pg_only=False): - if pg_only: - helper.register_table('Port_Group') - else: - for table in cls.tables: - helper.register_table(table) - - return cls(driver, connection_string, helper) - - -@contextlib.contextmanager -def short_living_ovsdb_api(api_class, idl): - """Context manager for short living connections to the database. - - :param api_class: Class implementing the database calls - (e.g. from the impl_idl module) - :param idl: An instance of IDL class (e.g. instance of OvnNbIdl) - """ - conn = connection.Connection( - idl, timeout=ovn_conf.get_ovn_ovsdb_timeout()) - api = api_class(conn) - try: - yield api - finally: - api.ovsdb_connection.stop() - - def _check_and_set_ssl_files(schema_name): if schema_name == 'OVN_Southbound': priv_key_file = ovn_conf.get_ovn_sb_private_key() diff --git a/neutron/tests/functional/common/ovn/test_utils.py b/neutron/tests/functional/common/ovn/test_utils.py new file mode 100644 index 00000000000..95f8e889617 --- /dev/null +++ b/neutron/tests/functional/common/ovn/test_utils.py @@ -0,0 +1,60 @@ +# Copyright 2022 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron.common.ovn import constants as ovn_const +from neutron.common.ovn import utils +from neutron.tests.functional import base + + +class TestCreateNeutronPgDrop(base.TestOVNFunctionalBase): + def test_already_existing(self): + # Make sure pre-fork initialize created the table + existing_pg = self.nb_api.pg_get( + ovn_const.OVN_DROP_PORT_GROUP_NAME).execute() + self.assertIsNotNone(existing_pg) + + # make an attempt to create it again + utils.create_neutron_pg_drop() + + pg = self.nb_api.pg_get(ovn_const.OVN_DROP_PORT_GROUP_NAME).execute() + self.assertEqual(existing_pg.uuid, pg.uuid) + + def test_non_existing(self): + # Delete the neutron_pg_drop created by pre-fork initialize + self.nb_api.pg_del(ovn_const.OVN_DROP_PORT_GROUP_NAME).execute() + pg = self.nb_api.pg_get(ovn_const.OVN_DROP_PORT_GROUP_NAME).execute() + self.assertIsNone(pg) + + utils.create_neutron_pg_drop() + + pg = self.nb_api.pg_get(ovn_const.OVN_DROP_PORT_GROUP_NAME).execute() + self.assertIsNotNone(pg) + + directions = ['to-lport', 'from-lport'] + matches = ['outport == @neutron_pg_drop && ip', + 'inport == @neutron_pg_drop && ip'] + + # Make sure ACLs are correct + self.assertEqual(2, len(pg.acls)) + acl1, acl2 = pg.acls + + self.assertEqual('drop', acl1.action) + self.assertIn(acl1.direction, directions) + directions.remove(acl1.direction) + self.assertIn(acl1.match, matches) + matches.remove(acl1.match) + + self.assertEqual(directions[0], acl2.direction) + self.assertEqual('drop', acl2.action) + self.assertEqual(matches[0], acl2.match) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 2bbe7f93377..88e8e3c37ac 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -27,15 +27,12 @@ from oslo_config import cfg from oslo_utils import uuidutils from ovsdbapp.backend.ovs_idl import event -from ovsdbapp.tests.functional import base as ovs_base from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils from neutron.common import utils as n_utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.db import ovn_revision_numbers_db as db_rev -from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor from neutron.tests import base as tests_base @@ -794,59 +791,6 @@ def test_external_port_update_switchdev_vnic_macvtap(self): self._test_external_port_update_switchdev(portbindings.VNIC_MACVTAP) -class TestCreateDefaultDropPortGroup(base.BaseLoggingTestCase, - ovs_base.FunctionalTestCase): - schemas = ['OVN_Southbound', 'OVN_Northbound'] - PG_NAME = ovn_const.OVN_DROP_PORT_GROUP_NAME - - def setUp(self): - super(TestCreateDefaultDropPortGroup, self).setUp() - ovn_conf.register_opts() - self.api = impl_idl_ovn.OvsdbNbOvnIdl( - self.connection['OVN_Northbound']) - self.addCleanup(self.api.pg_del(self.PG_NAME, if_exists=True).execute, - check_error=True) - - def test_port_group_exists(self): - """Test new port group is not added or modified. - - If Port Group was not existent, acls would be added. - """ - self.api.pg_add( - self.PG_NAME, acls=[], may_exist=True).execute(check_error=True) - mech_driver.create_default_drop_port_group(self.api) - port_group = self.api.get_port_group(self.PG_NAME) - self.assertFalse(port_group.acls) - - def _test_pg_with_ports(self, expected_ports=None): - expected_ports = expected_ports or [] - mech_driver.create_default_drop_port_group(self.api) - port_group = self.api.get_port_group(self.PG_NAME) - self.assertCountEqual( - expected_ports, [port.name for port in port_group.ports]) - - def test_with_ports_available(self): - expected_ports = ['port1', 'port2'] - testing_pg = 'testing' - testing_ls = 'testing' - with self.api.transaction(check_error=True) as txn: - txn.add(self.api.pg_add( - testing_pg, - external_ids={ovn_const.OVN_SG_EXT_ID_KEY: 'foo'})) - txn.add(self.api.ls_add(testing_ls)) - port_uuids = [txn.add(self.api.lsp_add(testing_ls, port)) - for port in expected_ports] - txn.add(self.api.pg_add_ports(testing_pg, port_uuids)) - - self.addCleanup(self.api.pg_del(testing_pg, if_exists=True).execute, - check_error=True) - - self._test_pg_with_ports(expected_ports) - - def test_without_ports(self): - self._test_pg_with_ports(expected_ports=[]) - - class TestSecurityGroup(base.TestOVNFunctionalBase): def setUp(self): diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index 590b55f9b99..9a2fae159fa 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -15,6 +15,7 @@ from collections import namedtuple from os import path +import shlex from unittest import mock import fixtures @@ -22,7 +23,9 @@ from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext from neutron_lib.api.definitions import portbindings from neutron_lib import constants as n_const +from oslo_concurrency import processutils from oslo_config import cfg +import testtools from neutron.common.ovn import constants from neutron.common.ovn import utils @@ -752,3 +755,116 @@ def decorated_method(): # number of exceptions + one successful call self.assertEqual(number_of_exceptions + 1, method.call_count) + + +class TestOvsdbClientCommand(base.BaseTestCase): + class OvsdbClientTestCommand(utils.OvsdbClientCommand): + COMMAND = 'test' + + def setUp(self): + super().setUp() + self.nb_connection = 'ovn_nb_connection' + self.sb_connection = 'ovn_sb_connection' + + ovn_conf.register_opts() + ovn_conf.cfg.CONF.set_default( + 'ovn_nb_connection', + self.nb_connection, + group='ovn') + ovn_conf.cfg.CONF.set_default( + 'ovn_sb_connection', + self.sb_connection, + group='ovn') + self.m_exec = mock.patch.object(processutils, 'execute').start() + + def assert_exec_call(self, expected): + self.m_exec.assert_called_with( + *shlex.split(expected), log_errors=processutils.LOG_FINAL_ERROR) + + def test_run_northbound(self): + expected = ('ovsdb-client %s %s --timeout 180 ' + '\'["OVN_Northbound", "foo"]\'' % ( + self.OvsdbClientTestCommand.COMMAND, + self.nb_connection)) + self.OvsdbClientTestCommand.run(['OVN_Northbound', 'foo']) + self.assert_exec_call(expected) + + def test_run_southbound(self): + expected = ('ovsdb-client %s %s --timeout 180 ' + '\'["OVN_Southbound", "foo"]\'' % ( + self.OvsdbClientTestCommand.COMMAND, + self.sb_connection)) + self.OvsdbClientTestCommand.run(['OVN_Southbound', 'foo']) + self.assert_exec_call(expected) + + def test_run_northbound_with_ssl(self): + private_key = 'north_pk' + certificate = 'north_cert' + ca_auth = 'north_ca_auth' + + ovn_conf.cfg.CONF.set_default( + 'ovn_nb_private_key', + private_key, + group='ovn') + ovn_conf.cfg.CONF.set_default( + 'ovn_nb_certificate', + certificate, + group='ovn') + ovn_conf.cfg.CONF.set_default( + 'ovn_nb_ca_cert', + ca_auth, + group='ovn') + + expected = ('ovsdb-client %s %s --timeout 180 ' + '-p %s ' + '-c %s ' + '-C %s ' + '\'["OVN_Northbound", "foo"]\'' % ( + self.OvsdbClientTestCommand.COMMAND, + self.nb_connection, + private_key, + certificate, + ca_auth)) + + self.OvsdbClientTestCommand.run(['OVN_Northbound', 'foo']) + self.assert_exec_call(expected) + + def test_run_southbound_with_ssl(self): + private_key = 'north_pk' + certificate = 'north_cert' + ca_auth = 'north_ca_auth' + + ovn_conf.cfg.CONF.set_default( + 'ovn_sb_private_key', + private_key, + group='ovn') + ovn_conf.cfg.CONF.set_default( + 'ovn_sb_certificate', + certificate, + group='ovn') + ovn_conf.cfg.CONF.set_default( + 'ovn_sb_ca_cert', + ca_auth, + group='ovn') + + expected = ('ovsdb-client %s %s --timeout 180 ' + '-p %s ' + '-c %s ' + '-C %s ' + '\'["OVN_Southbound", "foo"]\'' % ( + self.OvsdbClientTestCommand.COMMAND, + self.sb_connection, + private_key, + certificate, + ca_auth)) + + self.OvsdbClientTestCommand.run(['OVN_Southbound', 'foo']) + self.assert_exec_call(expected) + + def test_run_empty_list(self): + with testtools.ExpectedException(KeyError): + self.OvsdbClientTestCommand.run([]) + + def test_run_bad_schema(self): + with testtools.ExpectedException(KeyError): + self.OvsdbClientTestCommand.run(['foo']) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index e7653d8b333..dda8cb33beb 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -26,7 +26,6 @@ from ovs.stream import Stream from ovsdbapp.backend.ovs_idl import connection from ovsdbapp.backend.ovs_idl import idlutils -import testtools from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import hash_ring_manager @@ -722,36 +721,3 @@ def test_handle_ha_chassis_group_changes_update_new_gw(self): # after it became a Gateway chassis self._test_handle_ha_chassis_group_changes_create( self.event.ROW_UPDATE) - - -class TestShortLivingOvsdbApi(base.BaseTestCase): - def setUp(self): - ovn_conf.register_opts() - super().setUp() - - def test_context(self): - api_class = mock.Mock() - idl = mock.Mock() - with ovsdb_monitor.short_living_ovsdb_api(api_class, idl) as api: - self.assertEqual(api_class.return_value, api) - api.ovsdb_connection.stop.assert_called_once_with() - - def test_context_error(self): - api_class = mock.Mock() - idl = mock.Mock() - exc = RuntimeError() - try: - with ovsdb_monitor.short_living_ovsdb_api(api_class, idl) as api: - self.assertEqual(api_class.return_value, api) - raise exc - except RuntimeError as re: - self.assertIs(exc, re) - api.ovsdb_connection.stop.assert_called_once_with() - - def test_api_class_error(self): - api_class = mock.Mock(side_effect=RuntimeError()) - idl = mock.Mock() - with testtools.ExpectedException(RuntimeError): - with ovsdb_monitor.short_living_ovsdb_api(api_class, idl): - # Make sure it never enter the api context - raise Exception("API class instantiated but it should not") diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 8d5a94fd2d9..43f0214cd16 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -58,7 +58,6 @@ from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor from neutron.plugins.ml2.drivers import type_geneve # noqa from neutron.services.revisions import revision_plugin from neutron.tests.unit.extensions import test_segment @@ -198,56 +197,6 @@ def test_delete_mac_binding_entries_ssl(self): class TestOVNMechanismDriver(TestOVNMechanismDriverBase): - - @mock.patch.object(ovsdb_monitor.OvnInitPGNbIdl, 'from_server') - @mock.patch.object(ovsdb_monitor, 'short_living_ovsdb_api') - def test__create_neutron_pg_drop_non_existing( - self, m_ovsdb_api_con, m_from_server): - m_ovsdb_api = m_ovsdb_api_con.return_value.__enter__.return_value - m_ovsdb_api.get_port_group.return_value = None - self.mech_driver._create_neutron_pg_drop() - self.assertEqual(1, m_ovsdb_api.get_port_group.call_count) - self.assertTrue(m_ovsdb_api.transaction.return_value.__enter__.called) - - @mock.patch.object(ovsdb_monitor.OvnInitPGNbIdl, 'from_server') - @mock.patch.object(ovsdb_monitor, 'short_living_ovsdb_api') - def test__create_neutron_pg_drop_existing( - self, m_ovsdb_api_con, m_from_server): - m_ovsdb_api = m_ovsdb_api_con.return_value.__enter__.return_value - m_ovsdb_api.get_port_group.return_value = 'foo' - self.mech_driver._create_neutron_pg_drop() - self.assertEqual(1, m_ovsdb_api.get_port_group.call_count) - self.assertFalse(m_ovsdb_api.transaction.return_value.__enter__.called) - - @mock.patch.object(ovsdb_monitor.OvnInitPGNbIdl, 'from_server') - @mock.patch.object(ovsdb_monitor, 'short_living_ovsdb_api') - def test__create_neutron_pg_drop_created_meanwhile( - self, m_ovsdb_api_con, m_from_server): - m_ovsdb_api = m_ovsdb_api_con.return_value.__enter__.return_value - m_ovsdb_api.get_port_group.return_value = None - m_ovsdb_api.transaction.return_value.__exit__.side_effect = ( - RuntimeError()) - idl = m_from_server.return_value - idl.neutron_pg_drop_event.wait.return_value = True - result = self.mech_driver._create_neutron_pg_drop() - idl.neutron_pg_drop_event.wait.assert_called_once() - # If sommething else creates the port group, just return - self.assertIsNone(result) - - @mock.patch.object(ovsdb_monitor.OvnInitPGNbIdl, 'from_server') - @mock.patch.object(ovsdb_monitor, 'short_living_ovsdb_api') - def test__create_neutron_pg_drop_error( - self, m_ovsdb_api_con, m_from_server): - m_ovsdb_api = m_ovsdb_api_con.return_value.__enter__.return_value - m_ovsdb_api.get_port_group.return_value = None - m_ovsdb_api.transaction.return_value.__exit__.side_effect = ( - RuntimeError()) - idl = m_from_server.return_value - idl.neutron_pg_drop_event.wait.return_value = False - self.assertRaises(RuntimeError, - self.mech_driver._create_neutron_pg_drop) - idl.neutron_pg_drop_event.wait.assert_called_once() - def test__get_max_tunid_no_key_set(self): self.mech_driver.nb_ovn.nb_global.options.get.return_value = None self.assertIsNone(self.mech_driver._get_max_tunid())