Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions neutron/agent/linux/keepalived.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ def build_config(self):
LOG.warning("keepalived_use_no_track cfg option is True but "
"keepalived on host seems to not support this "
"option")
# NOTE(mstinsky): neutron and keepalived are adding the same routes on
# primary routers. With this we ensure that both are adding the routes
# with the same procotol and prevent duplicated routes which result in
# neutron exception for ip route commands.
output += ' protocol static'
return output


Expand Down
12 changes: 10 additions & 2 deletions neutron/common/ovn/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,8 @@ def compute_address_pairs_diff(ovn_port, neutron_port):

def get_ovn_cms_options(chassis):
"""Return the list of CMS options in a Chassis."""
return [opt.strip() for opt in chassis.external_ids.get(
constants.OVN_CMS_OPTIONS, '').split(',')]
return [opt.strip() for opt in get_ovn_chassis_other_config(chassis).get(
constants.OVN_CMS_OPTIONS, '').split(',')]


def is_gateway_chassis(chassis):
Expand Down Expand Up @@ -815,3 +815,11 @@ def create_neutron_pg_drop():
}]

OvsdbClientTransactCommand.run(command)


def get_ovn_chassis_other_config(chassis):
# NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is 22.09
try:
return chassis.other_config
except AttributeError:
return chassis.external_ids
13 changes: 9 additions & 4 deletions neutron/db/l3_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -1025,10 +1025,15 @@ def _add_router_port(self, context, port_id, router, device_owner):
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:
# due to race conditions maybe the port under analysis is
# deleted, so instead returning a RouterInterfaceNotFound
# we continue the analysis avoiding that port
LOG.debug("Port %s could not be found, it might have been "
"deleted concurrently. Will not be checked for "
"an overlapping router interface.",
rp.port_id)

if len(subnets_id) > 1:
id_filter = {'id': subnets_id}
subnets = self._core_plugin.get_subnets(context.elevated(),
filters=id_filter)
Expand Down
19 changes: 11 additions & 8 deletions neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ def as_dict(self):
'configurations': {
'chassis_name': self.chassis.name,
'bridge-mappings':
self.chassis.external_ids.get('ovn-bridge-mappings', '')},
ovn_utils.get_ovn_chassis_other_config(self.chassis).get(
'ovn-bridge-mappings', '')},
'start_flag': True,
'agent_type': self.agent_type,
'id': self.agent_id,
Expand Down Expand Up @@ -141,9 +142,9 @@ class ControllerAgent(NeutronAgent):

@staticmethod # it is by default, but this makes pep8 happy
def __new__(cls, chassis_private, driver):
external_ids = cls.chassis_from_private(chassis_private).external_ids
if ('enable-chassis-as-gw' in
external_ids.get('ovn-cms-options', [])):
_chassis = cls.chassis_from_private(chassis_private)
other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []):
cls = ControllerGatewayAgent
return super().__new__(cls)

Expand All @@ -166,8 +167,9 @@ def description(self):

def update(self, chassis_private, clear_down=False):
super().update(chassis_private, clear_down)
external_ids = self.chassis_from_private(chassis_private).external_ids
if 'enable-chassis-as-gw' in external_ids.get('ovn-cms-options', []):
_chassis = self.chassis_from_private(chassis_private)
other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []):
self.__class__ = ControllerGatewayAgent


Expand All @@ -176,9 +178,10 @@ class ControllerGatewayAgent(ControllerAgent):

def update(self, chassis_private, clear_down=False):
super().update(chassis_private, clear_down)
external_ids = self.chassis_from_private(chassis_private).external_ids
_chassis = self.chassis_from_private(chassis_private)
other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
if ('enable-chassis-as-gw' not in
external_ids.get('ovn-cms-options', [])):
other_config.get('ovn-cms-options', [])):
self.__class__ = ControllerAgent


Expand Down
8 changes: 5 additions & 3 deletions neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ def sb_ovn(self, val):
def get_supported_vif_types(self):
vif_types = set()
for ch in self.sb_ovn.chassis_list().execute(check_error=True):
dp_type = ch.external_ids.get('datapath-type', '')
other_config = ovn_utils.get_ovn_chassis_other_config(ch)
dp_type = other_config.get('datapath-type', '')
if dp_type == ovn_const.CHASSIS_DATAPATH_NETDEV:
vif_types.add(portbindings.VIF_TYPE_VHOST_USER)
else:
Expand Down Expand Up @@ -961,8 +962,9 @@ def bind_port(self, context):
'agent': agent})
return
chassis = agent.chassis
datapath_type = chassis.external_ids.get('datapath-type', '')
iface_types = chassis.external_ids.get('iface-types', '')
other_config = ovn_utils.get_ovn_chassis_other_config(chassis)
datapath_type = other_config.get('datapath-type', '')
iface_types = other_config.get('iface-types', '')
iface_types = iface_types.split(',') if iface_types else []
chassis_physnets = self.sb_ovn._get_chassis_physnets(chassis)
for segment_to_bind in context.segments_to_bind:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ def _parse_ovn_cms_options(chassis):


def _parse_bridge_mappings(chassis):
bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '')
other_config = ovn_utils.get_ovn_chassis_other_config(chassis)
bridge_mappings = other_config.get('ovn-bridge-mappings', '')
bridge_mappings = helpers.parse_mappings(bridge_mappings.split(','),
unique_values=False)
return {k: [v] for k, v in bridge_mappings.items()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,8 @@ def from_worker(cls, worker_class, driver=None):
return cls(conn)

def _get_chassis_physnets(self, chassis):
bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '')
other_config = utils.get_ovn_chassis_other_config(chassis)
bridge_mappings = other_config.get('ovn-bridge-mappings', '')
mapping_dict = helpers.parse_mappings(bridge_mappings.split(','))
return list(mapping_dict.keys())

Expand All @@ -848,7 +849,8 @@ def get_gateway_chassis_from_cms_options(self, name_only=True):
return [ch.name if name_only else ch
for ch in self.chassis_list().execute(check_error=True)
if ovn_const.CMS_OPT_CHASSIS_AS_GW in
ch.external_ids.get(ovn_const.OVN_CMS_OPTIONS, '').split(',')]
utils.get_ovn_chassis_other_config(ch).get(
ovn_const.OVN_CMS_OPTIONS, '').split(',')]

def get_chassis_and_physnets(self):
chassis_info_dict = {}
Expand All @@ -867,7 +869,7 @@ def get_chassis_by_card_serial_from_cms_options(self,
if ('{}={}'
.format(ovn_const.CMS_OPT_CARD_SERIAL_NUMBER,
card_serial_number)
in ch.external_ids.get(
in utils.get_ovn_chassis_other_config(ch).get(
ovn_const.OVN_CMS_OPTIONS, '').split(',')):
return ch
msg = _('Chassis with %s %s %s does not exist'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,21 @@ def handle_ha_chassis_group_changes(self, event, row, old):
def match_fn(self, event, row, old):
if event != self.ROW_UPDATE:
return True
# NOTE(lucasgomes): If the external_ids column wasn't updated
# (meaning, Chassis "gateway" status didn't change) just returns
if not hasattr(old, 'external_ids') and event == self.ROW_UPDATE:

# NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is
# 22.09
other_config = ('other_config' if hasattr(row, 'other_config') else
'external_ids')
# NOTE(lucasgomes): If the other_config/external_ids column wasn't
# updated (meaning, Chassis "gateway" status didn't change) just
# returns
if not hasattr(old, other_config) and event == self.ROW_UPDATE:
return False
if (old.external_ids.get('ovn-bridge-mappings') !=
row.external_ids.get('ovn-bridge-mappings')):
old_br_mappings = utils.get_ovn_chassis_other_config(old).get(
'ovn-bridge-mappings')
new_br_mappings = utils.get_ovn_chassis_other_config(row).get(
'ovn-bridge-mappings')
if old_br_mappings != new_br_mappings:
return True
# Check if either the Gateway status or Availability Zones has
# changed in the Chassis
Expand All @@ -192,8 +201,9 @@ def match_fn(self, event, row, old):
def run(self, event, row, old):
host = row.hostname
phy_nets = []
new_other_config = utils.get_ovn_chassis_other_config(row)
if event != self.ROW_DELETE:
bridge_mappings = row.external_ids.get('ovn-bridge-mappings', '')
bridge_mappings = new_other_config.get('ovn-bridge-mappings', '')
mapping_dict = helpers.parse_mappings(bridge_mappings.split(','))
phy_nets = list(mapping_dict)

Expand All @@ -208,9 +218,10 @@ def run(self, event, row, old):
if event == self.ROW_DELETE:
kwargs['event_from_chassis'] = row.name
elif event == self.ROW_UPDATE:
old_mappings = old.external_ids.get('ovn-bridge-mappings',
old_other_config = utils.get_ovn_chassis_other_config(old)
old_mappings = old_other_config.get('ovn-bridge-mappings',
set()) or set()
new_mappings = row.external_ids.get('ovn-bridge-mappings',
new_mappings = new_other_config.get('ovn-bridge-mappings',
set()) or set()
if old_mappings:
old_mappings = set(old_mappings.split(','))
Expand Down Expand Up @@ -339,11 +350,17 @@ class ChassisAgentTypeChangeEvent(ChassisEvent):
events = (BaseEvent.ROW_UPDATE,)

def match_fn(self, event, row, old=None):
if not getattr(old, 'external_ids', False):
# NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is
# 22.09
other_config = ('other_config' if hasattr(row, 'other_config') else
'external_ids')
if not getattr(old, other_config, False):
return False
agent_type_change = n_agent.NeutronAgent.chassis_from_private(
row).external_ids.get('ovn-cms-options', []) != (
old.external_ids.get('ovn-cms-options', []))
chassis = n_agent.NeutronAgent.chassis_from_private(row)
new_other_config = utils.get_ovn_chassis_other_config(chassis)
old_other_config = utils.get_ovn_chassis_other_config(old)
agent_type_change = new_other_config.get('ovn-cms-options', []) != (
old_other_config.get('ovn-cms-options', []))
return agent_type_change

def run(self, event, row, old):
Expand Down
12 changes: 11 additions & 1 deletion neutron/tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,11 @@ def setup_rootwrap(self):
root_helper_daemon=get_rootwrap_daemon_cmd())

def _simulate_concurrent_requests_process_and_raise(self, calls, args):
self._simulate_concurrent_requests_process(calls, args,
raise_on_exception=True)

def _simulate_concurrent_requests_process(self, calls, args,
raise_on_exception=False):
class SimpleThread(threading.Thread):
def __init__(self, q):
super(SimpleThread, self).__init__()
Expand Down Expand Up @@ -529,10 +533,16 @@ def get_exception(self):
t.start()
q.join()

threads_exceptions = []
for t in threads:
e = t.get_exception()
if e:
raise e
if raise_on_exception:
raise e
else:
threads_exceptions.append(e)

return threads_exceptions


class PluginFixture(fixtures.Fixture):
Expand Down
50 changes: 31 additions & 19 deletions neutron/tests/fullstack/test_l3_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,25 +270,37 @@ def _is_filter_set(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)
subnet_cidr = '10.200.0.0/24'
# to have many port interactions where race conditions would happen
# deleting ports meanwhile find operations to evaluate the overlapping
subnets = 10

funcs = []
args = []
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)
for i in range(subnets):
network_tmp = self.safe_client.create_network(
tenant_id, name='foo-network' + str(i))
subnet_tmp = self.safe_client.create_subnet(
tenant_id, network_tmp['id'], subnet_cidr)
funcs.append(self.safe_client.add_router_interface)
args.append((router['id'], subnet_tmp['id']))

exception_requests = self._simulate_concurrent_requests_process(
funcs, args)

if not all(type(e) == exceptions.BadRequest
for e in exception_requests):
self.fail('Unexpected exception adding interfaces to router from '
'different subnets overlapping')

if not len(exception_requests) >= (subnets - 1):
self.fail('If we have tried to associate %s subnets overlapping '
'cidr to the router, we should have received at least '
'%s or %s rejected requests, but we have only received '
'%s', (str(subnets), str(subnets - 1), str(subnets),
str(len(exception_requests))))


class TestLegacyL3Agent(TestL3Agent):
Expand Down Expand Up @@ -441,7 +453,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()

def test_concurrent_router_subnet_attachment_overlapping_cidr_(self):
def test_concurrent_router_subnet_attachment_overlapping_cidr(self):
self._test_concurrent_router_subnet_attachment_overlapping_cidr()


Expand Down Expand Up @@ -601,6 +613,6 @@ 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):
def test_concurrent_router_subnet_attachment_overlapping_cidr(self):
self._test_concurrent_router_subnet_attachment_overlapping_cidr(
ha=True)
8 changes: 4 additions & 4 deletions neutron/tests/functional/agent/l3/framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@
%(int_port_ipv6)s dev %(internal_device_name)s scope link no_track
}
virtual_routes {
0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s no_track
8.8.8.0/24 via 19.4.4.4 no_track
%(extra_subnet_cidr)s dev %(ex_device_name)s scope link no_track
0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s no_track protocol static
8.8.8.0/24 via 19.4.4.4 no_track protocol static
%(extra_subnet_cidr)s dev %(ex_device_name)s scope link no_track protocol static
}
}"""
}""" # noqa: E501 # pylint: disable=line-too-long


def get_ovs_bridge(br_name):
Expand Down
11 changes: 7 additions & 4 deletions neutron/tests/functional/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ def restart(self):
self._start_ovn_northd()

def add_fake_chassis(self, host, physical_nets=None, external_ids=None,
name=None, enable_chassis_as_gw=False):
name=None, enable_chassis_as_gw=False,
other_config=None):
def append_cms_options(ext_ids, value):
if 'ovn-cms-options' not in ext_ids:
ext_ids['ovn-cms-options'] = value
Expand All @@ -432,14 +433,15 @@ def append_cms_options(ext_ids, value):

physical_nets = physical_nets or []
external_ids = external_ids or {}
other_config = other_config or {}
if enable_chassis_as_gw:
append_cms_options(external_ids, 'enable-chassis-as-gw')
append_cms_options(other_config, 'enable-chassis-as-gw')

bridge_mapping = ",".join(["%s:br-provider%s" % (phys_net, i)
for i, phys_net in enumerate(physical_nets)])
if name is None:
name = uuidutils.generate_uuid()
external_ids['ovn-bridge-mappings'] = bridge_mapping
other_config['ovn-bridge-mappings'] = bridge_mapping
# We'll be using different IP addresses every time for the Encap of
# the fake chassis as the SB schema doesn't allow to have two entries
# with same (ip,type) pairs as of OVS 2.11. This shouldn't have any
Expand All @@ -450,7 +452,8 @@ def append_cms_options(ext_ids, value):
self._counter += 1
chassis = self.sb_api.chassis_add(
name, ['geneve'], '172.24.4.%d' % self._counter,
external_ids=external_ids, hostname=host).execute(check_error=True)
external_ids=external_ids, hostname=host,
other_config=other_config).execute(check_error=True)
if self.sb_api.is_table_present('Chassis_Private'):
nb_cfg_timestamp = timeutils.utcnow_ts() * 1000
self.sb_api.db_create(
Expand Down
Loading