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/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/constants.py b/neutron/common/ovn/constants.py index 92b687c3cfb..8d3ec487a40 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -111,6 +111,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', @@ -162,6 +163,7 @@ '58': 'T1', '59': 'T2', '67': 'bootfile_name', + '119': 'domain_search_list', '252': 'wpad', '210': 'path_prefix', '150': 'tftp_server_address'}, @@ -187,6 +189,7 @@ # OVN string type DHCP options OVN_STR_TYPE_DHCP_OPTS = [ 'domain_name', + 'domain_search_list', 'bootfile_name', 'bootfile_name_alt', 'path_prefix', diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 3ab816dd6ae..0f478972546 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -27,12 +27,14 @@ 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 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 @@ -55,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 @@ -693,3 +759,67 @@ 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 + + +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/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py index 96a072794d5..e59d24e554e 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py @@ -205,8 +205,10 @@ 'baremetal nodes. Defaults to False.')), ] -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/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/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 690789ab50b..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 @@ -117,6 +115,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') @@ -278,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): @@ -1190,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""" @@ -1374,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 0c2fa626198..d9865bfff83 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 @@ -1675,16 +1674,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/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/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/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/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..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,58 +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() - 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/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 591bb1a4bbe..86aef426555 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,10 +23,13 @@ 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 +from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.tests import base from neutron.tests.unit import fake_resources as fakes @@ -414,9 +418,26 @@ def test_get_lsp_dhcp_opts_dhcp_disabled_for_baremetal(self): # Assert no options were passed self.assertEqual({}, 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): + 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( @@ -743,3 +764,162 @@ 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) + + +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/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() 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 e45922bd663..c7e149fd33b 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..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 @@ -198,6 +197,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() @@ -721,32 +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 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 18c7fa37b2d..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 @@ -140,6 +139,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'], @@ -197,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()) @@ -2588,6 +2538,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'],