diff --git a/neutron/agent/linux/external_process.py b/neutron/agent/linux/external_process.py index 77f634a558b..f58f186d049 100644 --- a/neutron/agent/linux/external_process.py +++ b/neutron/agent/linux/external_process.py @@ -272,7 +272,6 @@ def _check_child_processes(self): 'resource_type': self._resource_type, 'uuid': service_id.uuid}) self._execute_action(service_id) - eventlet.sleep(0) def _periodic_checking_thread(self): while self._monitor_processes: 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 1426ff39091..c3a63bab80a 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -786,6 +786,9 @@ def _ovn_update_port(self, plugin_context, port, original_port, port['revision_number'] = db_port['revision_number'] self._ovn_update_port(plugin_context, port, original_port, retry_on_revision_mismatch=False) + except ovn_revision_numbers_db.StandardAttributeIDNotFound: + LOG.debug("Standard attribute was not found for port %s. It was " + "possibly deleted concurrently.", port['id']) def create_port_postcommit(self, context): """Create a port. diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 66634923efe..2112e023f6f 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -15,6 +15,7 @@ import abc import copy +import functools import inspect import re import threading @@ -53,6 +54,20 @@ INCONSISTENCY_TYPE_DELETE = 'delete' +def has_lock_periodic(*args, **kwargs): + def wrapper(f): + @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 + if not self.has_lock: + return + return f(self, *args, **kwargs) + return decorator + return wrapper + + class MaintenanceThread(object): def __init__(self): @@ -289,15 +304,10 @@ def _fix_create_update_subnet(self, context, row): # is held by some other neutron-server instance in the cloud, we'll attempt # to perform the migration every 10 seconds until completed. # TODO(ihrachys): Remove the migration to stateful fips in Z+1. - @periodics.periodic(spacing=10, run_immediately=True) + @has_lock_periodic(spacing=10, run_immediately=True) @rerun_on_schema_updates def migrate_to_stateful_fips(self): """Perform the migration from stateless to stateful Floating IPs. """ - # Only the worker holding a valid lock within OVSDB will perform the - # migration. - if not self.has_lock: - return - admin_context = n_context.get_admin_context() nb_sync = ovn_db_sync.OvnNbSynchronizer( self._ovn_client._plugin, self._nb_idl, self._ovn_client._sb_idl, @@ -310,7 +320,7 @@ def migrate_to_stateful_fips(self): # to perform the migration every 10 seconds until completed. # TODO(jlibosva): Remove the migration to port groups at some point. It's # been around since Queens release so it is good to drop this soon. - @periodics.periodic(spacing=10, run_immediately=True) + @has_lock_periodic(spacing=10, run_immediately=True) @rerun_on_schema_updates def migrate_to_port_groups(self): """Perform the migration from Address Sets to Port Groups. """ @@ -322,11 +332,6 @@ def migrate_to_port_groups(self): if not self._nb_idl.get_address_sets(): raise periodics.NeverAgain() - # Only the worker holding a valid lock within OVSDB will perform the - # migration. - if not self.has_lock: - return - admin_context = n_context.get_admin_context() nb_sync = ovn_db_sync.OvnNbSynchronizer( self._ovn_client._plugin, self._nb_idl, self._ovn_client._sb_idl, @@ -358,14 +363,9 @@ def _log(inconsistencies, type_): _log(create_update_inconsistencies, INCONSISTENCY_TYPE_CREATE_UPDATE) _log(delete_inconsistencies, INCONSISTENCY_TYPE_DELETE) - @periodics.periodic(spacing=ovn_const.DB_CONSISTENCY_CHECK_INTERVAL, - run_immediately=True) + @has_lock_periodic(spacing=ovn_const.DB_CONSISTENCY_CHECK_INTERVAL, + run_immediately=True) def check_for_inconsistencies(self): - # Only the worker holding a valid lock within OVSDB will run - # this periodic - if not self.has_lock: - return - admin_context = n_context.get_admin_context() create_update_inconsistencies = ( revision_numbers_db.get_inconsistent_resources(admin_context)) @@ -481,13 +481,8 @@ 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(). - @periodics.periodic(spacing=600, - run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_global_dhcp_opts(self): - # This periodic task is included in DBInconsistenciesPeriodics since - # it uses the lock to ensure only one worker is executing - if not self.has_lock: - return if (not ovn_conf.get_global_dhcpv4_opts() and not ovn_conf.get_global_dhcpv6_opts()): # No need to scan the subnets if the settings are unset. @@ -516,11 +511,8 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_for_igmp_snoop_support(self): - if not self.has_lock: - return - with self._nb_idl.transaction(check_error=True) as txn: value = ('true' if ovn_conf.is_igmp_snooping_enabled() else 'false') @@ -539,11 +531,8 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_port_has_address_scope(self): - if not self.has_lock: - return - ports = self._nb_idl.db_find_rows( "Logical_Switch_Port", ("type", "!=", ovn_const.LSP_TYPE_LOCALNET) ).execute(check_error=True) @@ -585,16 +574,13 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_for_ha_chassis_group(self): # If external ports is not supported stop running # this periodic task if not self._ovn_client.is_external_ports_supported(): raise periodics.NeverAgain() - if not self.has_lock: - return - external_ports = self._nb_idl.db_find_rows( 'Logical_Switch_Port', ('type', '=', ovn_const.LSP_TYPE_EXTERNAL) ).execute(check_error=True) @@ -618,11 +604,8 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_for_mcast_flood_reports(self): - if not self.has_lock: - return - cmds = [] for port in self._nb_idl.lsp_list().execute(check_error=True): port_type = port.type.strip() @@ -667,11 +650,8 @@ def check_for_mcast_flood_reports(self): # TODO(lucasagomes): Remove this in the Z cycle # A static spacing value is used here, but this method will only run # once per lock due to the use of periodics.NeverAgain(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_router_mac_binding_options(self): - if not self.has_lock: - return - cmds = [] for router in self._nb_idl.lr_list().execute(check_error=True): if (router.options.get('always_learn_from_arp_request') and @@ -692,16 +672,13 @@ def check_router_mac_binding_options(self): # TODO(ralonsoh): Remove this in the Z+2 cycle # A static spacing value is used here, but this method will only run # once per lock due to the use of periodics.NeverAgain(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def update_port_qos_with_external_ids_reference(self): """Update all OVN QoS registers with the port ID This method will only update the OVN QoS registers related to port QoS, not FIP QoS. FIP QoS have the corresponding "external_ids" reference. """ - if not self.has_lock: - return - regex = re.compile( r'(inport|outport) == \"(?P[a-z0-9\-]{36})\"') cmds = [] @@ -726,14 +703,12 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, 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 router gateway ports. """ - if not self.has_lock: - return context = n_context.get_admin_context() cmds = [] gw_ports = self._ovn_client._plugin.get_ports( @@ -786,14 +761,12 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_vlan_distributed_ports(self): """Check VLAN distributed ports Check for the option "reside-on-redirect-chassis" value for distributed VLAN ports. """ - if not self.has_lock: - return context = n_context.get_admin_context() cmds = [] # Get router ports belonging to VLAN networks @@ -829,12 +802,9 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def update_logical_router_with_gateway_network_id(self): """Update all OVN logical router registers with the GW network ID""" - if not self.has_lock: - return - cmds = [] context = n_context.get_admin_context() for lr in self._nb_idl.lr_list().execute(check_error=True): @@ -859,7 +829,7 @@ 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(). - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_baremetal_ports_dhcp_options(self): """Update baremetal ports DHCP options @@ -871,9 +841,6 @@ def check_baremetal_ports_dhcp_options(self): if not self._ovn_client.is_external_ports_supported(): raise periodics.NeverAgain() - if not self.has_lock: - return - context = n_context.get_admin_context() ports = ports_obj.Port.get_ports_by_vnic_type_and_host( context, portbindings.VNIC_BAREMETAL) @@ -911,16 +878,13 @@ def check_baremetal_ports_dhcp_options(self): raise periodics.NeverAgain() # TODO(ralonsoh): Remove this in the Z+4 cycle - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def update_port_virtual_type(self): """Set type=virtual to those ports with parents Before LP#1973276, any virtual port with "device_owner" defined, lost its type=virtual. This task restores the type for those ports updated before the fix https://review.opendev.org/c/openstack/neutron/+/841711. """ - if not self.has_lock: - return - context = n_context.get_admin_context() cmds = [] for lsp in self._nb_idl.lsp_list().execute(check_error=True): @@ -948,7 +912,7 @@ def update_port_virtual_type(self): raise periodics.NeverAgain() # TODO(ralonsoh): Remove this in the Antelope+4 cycle - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def create_router_extra_attributes_registers(self): """Create missing ``RouterExtraAttributes`` registers. @@ -958,9 +922,6 @@ def create_router_extra_attributes_registers(self): only execution method finds those ``Routers`` registers without the child one and creates one with the default values. """ - if not self.has_lock: - return - context = n_context.get_admin_context() for router_id in router_obj.Router.\ get_router_ids_without_router_std_attrs(context): @@ -1008,13 +969,10 @@ def add_gw_port_info_to_logical_router_port(self): txn.add(cmd) raise periodics.NeverAgain() - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_router_default_route_empty_dst_ip(self): """Check routers with default route with empty dst-ip (LP: #2002993). """ - if not self.has_lock: - return - cmds = [] for router in self._nb_idl.lr_list().execute(check_error=True): if not router.external_ids.get(ovn_const.OVN_REV_NUM_EXT_ID_KEY): @@ -1036,7 +994,7 @@ def check_router_default_route_empty_dst_ip(self): raise periodics.NeverAgain() # TODO(ralonsoh): Remove this in the Antelope+4 cycle - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, 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. @@ -1047,9 +1005,6 @@ def add_vnic_type_and_pb_capabilities_to_lsp(self): been added to the LSP the VNIC type and the port binding capabilities. To implement LP#1998608, only direct ports are needed. """ - if not self.has_lock: - return - port_bindings = ports_obj.PortBinding.get_port_binding_by_vnic_type( n_context.get_admin_context(), portbindings.VNIC_DIRECT) with self._nb_idl.transaction(check_error=True) as txn: @@ -1071,7 +1026,7 @@ def add_vnic_type_and_pb_capabilities_to_lsp(self): raise periodics.NeverAgain() - @periodics.periodic(spacing=600, run_immediately=True) + @has_lock_periodic(spacing=600, run_immediately=True) def check_fair_meter_consistency(self): """Update the logging meter after neutron-server reload @@ -1080,8 +1035,6 @@ def check_fair_meter_consistency(self): driver after the OVN NB idl is loaded """ - if not self.has_lock: - return if log_driver.OVNDriver.network_logging_supported(self._nb_idl): meter_name = ( cfg.CONF.network_log.local_output_log_base or "acl_log_meter") @@ -1089,7 +1042,7 @@ def check_fair_meter_consistency(self): from_reload=True) raise periodics.NeverAgain() - @periodics.periodic(spacing=300, run_immediately=True) + @has_lock_periodic(spacing=300, run_immediately=True) def remove_duplicated_chassis_registers(self): """Remove the "Chassis" and "Chassis_Private" duplicated registers. @@ -1108,9 +1061,6 @@ def remove_duplicated_chassis_registers(self): if not self._sb_idl.is_table_present('Chassis_Private'): raise periodics.NeverAgain() - if not self.has_lock: - return - # dup_chassis_port_host = {host_name: [(ch1, ch_private1), # (ch2, ch_private2), ... ]} dup_chassis_port_host = {} @@ -1146,7 +1096,7 @@ def remove_duplicated_chassis_registers(self): for table in ('Chassis_Private', 'Chassis'): txn.add(self._sb_idl.db_destroy(table, ch.name)) - @periodics.periodic(spacing=86400, run_immediately=True) + @has_lock_periodic(spacing=86400, run_immediately=True) def cleanup_old_hash_ring_nodes(self): """Daily task to cleanup old stable Hash Ring node entries. @@ -1155,8 +1105,6 @@ def cleanup_old_hash_ring_nodes(self): information. """ - if not self.has_lock: - return context = n_context.get_admin_context() hash_ring_db.cleanup_old_nodes(context, days=5) 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 3f32152fe3b..4f81e38011b 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 @@ -2347,8 +2347,11 @@ def test_update_port_postcommit_live_migration_revision_mismatch_once( '_is_port_provisioning_required', lambda *_: True) @mock.patch.object(mech_driver.OVNMechanismDriver, '_notify_dhcp_updated') @mock.patch.object(ovn_client.OVNClient, 'update_port') - def test_update_port_postcommit_revision_mismatch_not_after_live_migration( - self, mock_update_port, mock_notify_dhcp): + def _test_update_port_postcommit_with_exception( + self, mock_update_port, mock_notify_dhcp, + raised_exc, + resource_id_name, + **exc_extra_params): self.plugin.update_port_status = mock.Mock() self.plugin.get_port = mock.Mock(return_value=mock.MagicMock()) @@ -2366,10 +2369,12 @@ def test_update_port_postcommit_revision_mismatch_not_after_live_migration( fake_ctx = mock.Mock(current=fake_port, original=original_fake_port, plugin_context=fake_context) + + exc_params = exc_extra_params.copy() + exc_params[resource_id_name] = fake_port['id'] + mock_update_port.side_effect = [ - ovn_exceptions.RevisionConflict( - resource_id=fake_port['id'], - resource_type=ovn_const.TYPE_PORTS), + raised_exc(**exc_params), None] self.mech_driver.update_port_postcommit(fake_ctx) @@ -2379,6 +2384,20 @@ def test_update_port_postcommit_revision_mismatch_not_after_live_migration( self.assertEqual(1, mock_update_port.call_count) mock_notify_dhcp.assert_called_with(fake_port['id']) + def test_update_port_postcommit_revision_mismatch_not_after_live_migration( + self): + self._test_update_port_postcommit_with_exception( + raised_exc=ovn_exceptions.RevisionConflict, + resource_id_name='resource_id', + resource_type=ovn_const.TYPE_PORTS, + ) + + def test__ovn_update_port_missing_stdattribute(self): + """Make sure exception is handled.""" + self._test_update_port_postcommit_with_exception( + raised_exc=ovn_revision_numbers_db.StandardAttributeIDNotFound, + resource_id_name='resource_uuid') + def test_agent_alive_true(self): chassis_private = self._add_chassis_private(5) for agent_type in (ovn_const.OVN_CONTROLLER_AGENT,