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
2 changes: 2 additions & 0 deletions neutron/common/ovn/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,8 @@
TYPE_SECURITY_GROUP_RULES)

DB_CONSISTENCY_CHECK_INTERVAL = 300 # 5 minutes
MAINTENANCE_TASK_RETRY_LIMIT = 100 # times
MAINTENANCE_ONE_RUN_TASK_SPACING = 5 # seconds

# The order in which the resources should be created or updated by the
# maintenance task: Root ones first and leafs at the end.
Expand Down
2 changes: 2 additions & 0 deletions neutron/db/external_net_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ def _network_filter_hook(context, original_model, conditions):
(rbac_model.target_project == context.tenant_id) |
(rbac_model.target_project == '*'))
conditions = expr.or_(tenant_allowed, *conditions)
conditions = expr.or_(original_model.tenant_id == context.tenant_id,
*conditions)
return conditions


Expand Down
96 changes: 76 additions & 20 deletions neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,28 @@
INCONSISTENCY_TYPE_DELETE = 'delete'


def has_lock_periodic(*args, **kwargs):
def has_lock_periodic(*args, periodic_run_limit=0, **kwargs):
def wrapper(f):
_retries = 0

@functools.wraps(f)
@periodics.periodic(*args, **kwargs)
def decorator(self, *args, **kwargs):
# This periodic task is included in DBInconsistenciesPeriodics
# since it uses the lock to ensure only one worker is executing
# additonally, if periodic_run_limit parameter with value > 0 is
# provided and lock is not acquired for periodic_run_limit
# times, task will not be run anymore by this maintenance worker
nonlocal _retries
if not self.has_lock:
if periodic_run_limit > 0:
if _retries >= periodic_run_limit:
LOG.debug("Have not been able to acquire lock to run "
"task '%s' after %s tries, limit reached. "
"No more attempts will be made.",
f, _retries)
raise periodics.NeverAgain()
_retries += 1
return
return f(self, *args, **kwargs)
return decorator
Expand Down Expand Up @@ -481,7 +495,10 @@ def _delete_floatingip_and_pf(self, context, fip_id):

# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_global_dhcp_opts(self):
if (not ovn_conf.get_global_dhcpv4_opts() and
not ovn_conf.get_global_dhcpv6_opts()):
Expand Down Expand Up @@ -511,7 +528,10 @@ def check_global_dhcp_opts(self):

# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_for_igmp_snoop_support(self):
with self._nb_idl.transaction(check_error=True) as txn:
value = ('true' if ovn_conf.is_igmp_snooping_enabled()
Expand All @@ -531,7 +551,10 @@ def check_for_igmp_snoop_support(self):
# TODO(czesla): Remove this in the A+4 cycle
# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_port_has_address_scope(self):
ports = self._nb_idl.db_find_rows(
"Logical_Switch_Port", ("type", "!=", ovn_const.LSP_TYPE_LOCALNET)
Expand Down Expand Up @@ -574,7 +597,10 @@ def _delete_default_ha_chassis_group(self, txn):

# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_for_ha_chassis_group(self):
# If external ports is not supported stop running
# this periodic task
Expand Down Expand Up @@ -604,7 +630,10 @@ def check_for_ha_chassis_group(self):
# TODO(lucasagomes): Remove this in the B+3 cycle
# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_for_mcast_flood_reports(self):
cmds = []
for port in self._nb_idl.lsp_list().execute(check_error=True):
Expand Down Expand Up @@ -703,7 +732,10 @@ def update_port_qos_with_external_ids_reference(self):

# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_redirect_type_router_gateway_ports(self):
"""Check OVN router gateway ports
Check for the option "redirect-type=bridged" value for
Expand Down Expand Up @@ -761,7 +793,10 @@ def check_redirect_type_router_gateway_ports(self):

# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_vlan_distributed_ports(self):
"""Check VLAN distributed ports
Check for the option "reside-on-redirect-chassis" value for
Expand All @@ -772,10 +807,7 @@ def check_vlan_distributed_ports(self):
# Get router ports belonging to VLAN networks
vlan_nets = self._ovn_client._plugin.get_networks(
context, {pnet.NETWORK_TYPE: [n_const.TYPE_VLAN]})
# FIXME(ltomasbo): Once Bugzilla 2162756 is fixed the
# is_provider_network check should be removed
vlan_net_ids = [vn['id'] for vn in vlan_nets
if not utils.is_provider_network(vn)]
vlan_net_ids = [vn['id'] for vn in vlan_nets]
router_ports = self._ovn_client._plugin.get_ports(
context, {'network_id': vlan_net_ids,
'device_owner': n_const.ROUTER_PORT_OWNERS})
Expand All @@ -802,7 +834,10 @@ def check_vlan_distributed_ports(self):
# a gateway (that means, that has "external_ids:OVN_GW_PORT_EXT_ID_KEY").
# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def update_logical_router_with_gateway_network_id(self):
"""Update all OVN logical router registers with the GW network ID"""
cmds = []
Expand All @@ -829,7 +864,10 @@ def update_logical_router_with_gateway_network_id(self):

# A static spacing value is used here, but this method will only run
# once per lock due to the use of periodics.NeverAgain().
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_baremetal_ports_dhcp_options(self):
"""Update baremetal ports DHCP options

Expand Down Expand Up @@ -912,7 +950,10 @@ def update_port_virtual_type(self):
raise periodics.NeverAgain()

# TODO(ralonsoh): Remove this in the Antelope+4 cycle
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def create_router_extra_attributes_registers(self):
"""Create missing ``RouterExtraAttributes`` registers.

Expand All @@ -933,7 +974,10 @@ def create_router_extra_attributes_registers(self):
raise periodics.NeverAgain()

# TODO(slaweq): Remove this in the E cycle (C+2 as it will be next SLURP)
@periodics.periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def add_gw_port_info_to_logical_router_port(self):
"""Add info if LRP is connecting internal subnet or ext gateway."""
cmds = []
Expand Down Expand Up @@ -969,7 +1013,10 @@ def add_gw_port_info_to_logical_router_port(self):
txn.add(cmd)
raise periodics.NeverAgain()

@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_router_default_route_empty_dst_ip(self):
"""Check routers with default route with empty dst-ip (LP: #2002993).
"""
Expand All @@ -994,7 +1041,10 @@ def check_router_default_route_empty_dst_ip(self):
raise periodics.NeverAgain()

# TODO(ralonsoh): Remove this in the Antelope+4 cycle
@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def add_vnic_type_and_pb_capabilities_to_lsp(self):
"""Add the port VNIC type and port binding capabilities to the LSP.

Expand Down Expand Up @@ -1026,7 +1076,10 @@ def add_vnic_type_and_pb_capabilities_to_lsp(self):

raise periodics.NeverAgain()

@has_lock_periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def check_fair_meter_consistency(self):
"""Update the logging meter after neutron-server reload

Expand Down Expand Up @@ -1108,7 +1161,10 @@ def cleanup_old_hash_ring_nodes(self):
context = n_context.get_admin_context()
hash_ring_db.cleanup_old_nodes(context, days=5)

@periodics.periodic(spacing=600, run_immediately=True)
@has_lock_periodic(
periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT,
spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING,
run_immediately=True)
def update_nat_floating_ip_with_gateway_port_reference(self):
"""Set NAT rule gateway_port column to any floating IP without
router gateway port uuid reference - LP#2035281.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1610,8 +1610,6 @@ def _gen_router_port_options(self, port, network=None):
# logical router port is centralized in the chassis hosting the
# distributed gateway port.
# https://github.com/openvswitch/ovs/commit/85706c34d53d4810f54bec1de662392a3c06a996
# FIXME(ltomasbo): Once Bugzilla 2162756 is fixed the
# is_provider_network check should be removed
if network.get(pnet.NETWORK_TYPE) == const.TYPE_VLAN:
reside_redir_ch = self._get_reside_redir_for_gateway_port(
port['device_id'])
Expand Down
6 changes: 4 additions & 2 deletions neutron/tests/unit/extensions/test_external_net.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,17 @@ def test_network_filter_hook_admin_context(self):
def test_network_filter_hook_nonadmin_context(self):
ctx = context.Context('edinson', 'cavani')
model = models_v2.Network
txt = ("networkrbacs.action = :action_1 AND "
txt = ("networks.project_id = :project_id_1 OR "
"networkrbacs.action = :action_1 AND "
"networkrbacs.target_project = :target_project_1 OR "
"networkrbacs.target_project = :target_project_2")
conditions = external_net_db._network_filter_hook(ctx, model, [])
self.assertEqual(conditions.__str__(), txt)
# Try to concatenate conditions
txt2 = (txt.replace('project_1', 'project_3').
replace('project_2', 'project_4').
replace('action_1', 'action_2'))
replace('action_1', 'action_2').
replace('project_id_1', 'project_id_2'))
conditions = external_net_db._network_filter_hook(ctx, model,
conditions)
self.assertEqual(conditions.__str__(), "%s OR %s" % (txt, txt2))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,69 @@
from neutron.objects import ports as ports_obj
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import maintenance
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync
from neutron.tests import base
from neutron.tests.unit import fake_resources as fakes
from neutron.tests.unit.plugins.ml2 import test_security_group as test_sg
from neutron.tests.unit import testlib_api
from neutron_lib import exceptions as n_exc


class TestHasLockPeriodicDecorator(base.BaseTestCase):

def test_decorator_no_limit_have_lock(self):
run_counter = 0

@maintenance.has_lock_periodic(
periodic_run_limit=0, spacing=30)
def test_maintenance_task(worker):
nonlocal run_counter
run_counter += 1

worker_mock = mock.MagicMock()
worker_mock.has_lock = True

for _ in range(3):
test_maintenance_task(worker_mock)
self.assertEqual(3, run_counter)

def test_decorator_no_lock_no_limit(self):
run_counter = 0

@maintenance.has_lock_periodic(
periodic_run_limit=0, spacing=30)
def test_maintenance_task(worker):
nonlocal run_counter
run_counter += 1

worker_mock = mock.MagicMock()
has_lock_values = [False, False, True]

for has_lock in has_lock_values:
worker_mock.has_lock = has_lock
test_maintenance_task(worker_mock)
self.assertEqual(1, run_counter)

def test_decorator_no_lock_with_limit(self):
run_counter = 0

@maintenance.has_lock_periodic(
periodic_run_limit=1, spacing=30)
def test_maintenance_task(worker):
nonlocal run_counter
run_counter += 1

worker_mock = mock.MagicMock()

worker_mock.has_lock = False
test_maintenance_task(worker_mock)
self.assertEqual(0, run_counter)

worker_mock.has_lock = False
self.assertRaises(periodics.NeverAgain,
test_maintenance_task, worker_mock)
self.assertEqual(0, run_counter)


class TestSchemaAwarePeriodicsBase(testlib_api.SqlTestCaseLight):

def test__set_schema_aware_periodics(self):
Expand Down
16 changes: 14 additions & 2 deletions zuul.d/base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@
- ^roles/add_mariadb_repo/.*$
- ^roles/nftables/.*$
- ^rally-jobs/.*$
- ^zuul.d/(?!(project)).*\.yaml
# Ignore everything except for zuul.d/project.yaml
- ^zuul.d/base.yaml
- ^zuul.d/grenade.yaml
- ^zuul.d/job-templates.yaml
- ^zuul.d/rally.yaml
- ^zuul.d/tempest-multinode.yaml
- ^zuul.d/tempest-singlenode.yaml
vars:
configure_swap_size: 8192
Q_BUILD_OVS_FROM_GIT: True
Expand Down Expand Up @@ -96,7 +102,13 @@
- ^roles/add_mariadb_repo/.*$
- ^roles/nftables/.*$
- ^rally-jobs/.*$
- ^zuul.d/(?!(project)).*\.yaml
# Ignore everything except for zuul.d/project.yaml
- ^zuul.d/base.yaml
- ^zuul.d/grenade.yaml
- ^zuul.d/job-templates.yaml
- ^zuul.d/rally.yaml
- ^zuul.d/tempest-multinode.yaml
- ^zuul.d/tempest-singlenode.yaml

- job:
name: neutron-fullstack-with-uwsgi
Expand Down
16 changes: 14 additions & 2 deletions zuul.d/grenade.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@
- ^roles/.*functional.*$
- ^playbooks/.*functional.*$
- ^vagrant/.*$
- ^zuul.d/(?!(project)).*\.yaml
# Ignore everything except for zuul.d/project.yaml
- ^zuul.d/base.yaml
- ^zuul.d/grenade.yaml
- ^zuul.d/job-templates.yaml
- ^zuul.d/rally.yaml
- ^zuul.d/tempest-multinode.yaml
- ^zuul.d/tempest-singlenode.yaml
vars:
grenade_devstack_localrc:
shared:
Expand Down Expand Up @@ -249,7 +255,13 @@
- ^roles/.*functional.*$
- ^playbooks/.*functional.*$
- ^vagrant/.*$
- ^zuul.d/(?!(project)).*\.yaml
# Ignore everything except for zuul.d/project.yaml
- ^zuul.d/base.yaml
- ^zuul.d/grenade.yaml
- ^zuul.d/job-templates.yaml
- ^zuul.d/rally.yaml
- ^zuul.d/tempest-multinode.yaml
- ^zuul.d/tempest-singlenode.yaml
roles:
- zuul: openstack/neutron-tempest-plugin
required-projects:
Expand Down
Loading