From 4fb3d6f5a512eb6598dd282f7759d8066ac640f3 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 5 Aug 2025 19:27:21 +0000 Subject: [PATCH 01/10] [OVN] The OVS IDL connection is global for all tests In the functional tests, the OVN databases are created per test and removed at the end. That leaves no trace the local OVN databases if running in a system running OpenStack. But the OVS database is global and shared with all the tests and running processes. It is needed to use the OVS IDL ``api_factory`` method that returns a global object for all tests. Closes-Bug: #2119453 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I7b438ba76e6f95d533d8d7bc4aaeeba0007551e3 (cherry picked from commit 23994f6ee09d7a3d726bf7ba36d770dcdb85945c) --- .../agent/ovn/agent/test_ovn_neutron_agent.py | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py b/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py index d25ebd57cf6..3ff53017ed9 100644 --- a/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py +++ b/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py @@ -23,6 +23,7 @@ from neutron.agent.ovn.agent import ovsdb as agent_ovsdb from neutron.agent.ovn.metadata import agent as metadata_agent from neutron.agent.ovn.metadata import server_socket +from neutron.agent.ovsdb import impl_idl from neutron.common.ovn import constants as ovn_const from neutron.common import utils as n_utils from neutron.tests.common import net_helpers @@ -46,6 +47,7 @@ def setUp(self, extensions, **kwargs): self.mock_chassis_name = mock.patch.object( agent_ovsdb, 'get_own_chassis_name', return_value=self.chassis_name).start() + self.ovs_idl_events = [] with mock.patch.object(metadata_agent.MetadataAgent, '_get_own_chassis_name', return_value=self.chassis_name): @@ -57,6 +59,21 @@ def _check_loaded_and_started_extensions(self, ovn_agent): self.assertEqual(EXTENSION_NAMES.get(_ext), loaded_ext.name) self.assertTrue(loaded_ext.is_started) + def _create_ovs_idl(self, ovn_agent): + for extension in ovn_agent.ext_manager: + self.ovs_idl_events += extension.obj.ovs_idl_events + self.ovs_idl_events = [e(ovn_agent) for e in + set(self.ovs_idl_events)] + ovsdb = impl_idl.api_factory() + ovsdb.idl.notify_handler.watch_events(self.ovs_idl_events) + + ovn_agent.ext_manager_api.ovs_idl = ovsdb + return ovsdb + + def _clear_events_ovs_idl(self): + self.ovn_agent.ovs_idl.idl_monitor.notify_handler.unwatch_events( + self.ovs_idl_events) + def _start_ovn_neutron_agent(self): conf = self.useFixture(fixture_config.Config()).conf conf.set_override('extensions', ','.join(self.extensions), @@ -75,7 +92,10 @@ def _start_ovn_neutron_agent(self): # Once eventlet is completely removed, this mock can be deleted. with mock.patch.object(ovn_neutron_agent.OVNNeutronAgent, 'wait'), \ mock.patch.object(server_socket.UnixDomainMetadataProxy, - 'wait'): + 'wait'), \ + mock.patch.object(ovn_neutron_agent.OVNNeutronAgent, + '_load_ovs_idl') as mock_load_ovs_idl: + mock_load_ovs_idl.return_value = self._create_ovs_idl(agt) agt.start() external_ids = agt.sb_idl.db_get( 'Chassis_Private', agt.chassis, 'external_ids').execute( @@ -85,8 +105,7 @@ def _start_ovn_neutron_agent(self): '0') self._check_loaded_and_started_extensions(agt) - - self.addCleanup(agt.ext_manager_api.ovs_idl.ovsdb_connection.stop) + self.addCleanup(self._clear_events_ovs_idl) if agt.ext_manager_api.sb_idl: self.addCleanup(agt.ext_manager_api.sb_idl.ovsdb_connection.stop) if agt.ext_manager_api.nb_idl: From fc1039f8eb946825ffbd3d716af9d40c16038223 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 6 Dec 2024 18:33:57 -0500 Subject: [PATCH 02/10] Remove false-positive ACLs in OVN DB sync If there are two (or more) security group rules that have the same normalized cidr, only one of them will match the OVN ACL exactly. The other will match everything except the rule ID. Go through any items found a second time, ignoring the rule ID to remove the false-positives. Closes-bug: #2087822 Change-Id: Ia5a76973f80be1369753ebf42fba2fc19690a229 Signed-off-by: Brian Haley (cherry picked from commit 1c3eac52b724dd5f095ede0d17db432796c41516) --- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 27 +++++++++++++++ .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 34 +++++++++++++++++-- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index 9745ed183bb..a88d1c76a06 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py @@ -354,6 +354,33 @@ def sync_acls(self, ctx): add_acls.append(na) n_index += 1 + # Check any ACLs we found to add against existing ACLs, ignoring the + # SG rule ID key. This eliminates any false-positives where the + # normalized cidr for two SG rules is the same value, since there + # will only be a single ACL that matches exactly with the SG rule ID. + if add_acls: + def copy_acl_rem_id_key(acl): + acl_copy = acl.copy() + del acl_copy[ovn_const.OVN_SG_RULE_EXT_ID_KEY] + return acl_copy + + add_rem_acls = [] + # Make a list of non-default rule ACLs (they have a security group + # rule id). See ovn_default_acls code/comment above for more info. + nd_ovn_acls = [copy_acl_rem_id_key(oa) for oa in ovn_acls + if ovn_const.OVN_SG_RULE_EXT_ID_KEY in oa] + # We must copy here since we need to keep the original + # 'add_acl' intact for removal + for add_acl in add_acls: + add_acl_copy = copy_acl_rem_id_key(add_acl) + if add_acl_copy in nd_ovn_acls: + add_rem_acls.append(add_acl) + + # Remove any of the false-positive ACLs + LOG.warning('False-positive ACLs to remove: (%s)', add_rem_acls) + for add_rem in add_rem_acls: + add_acls.remove(add_rem) + if n_index < neutron_num: # We didn't find the OVN ACLs matching the Neutron ACLs # in "ovn_acls" and we are just adding the pending Neutron ACLs. diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index 5140361b099..6fb51367be3 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -1146,7 +1146,7 @@ def _build_acl_to_compare(self, acl, extra_fields=None): pass return acl_utils.filter_acl_dict(acl_to_compare, extra_fields) - def _validate_acls(self, should_match=True): + def _validate_acls(self, should_match=True, db_duplicate_port=None): # Get the neutron DB ACLs. db_acls = [] @@ -1194,6 +1194,19 @@ def _validate_acls(self, should_match=True): # Values taken out from list for comparison, since ACLs from OVN DB # have certain values on a list of just one object if should_match: + if db_duplicate_port: + # If we have a duplicate port, that indicates there are two + # DB entries that map to the same ACL. Remove the extra from + # our comparison. + dup_acl = None + for acl in db_acls: + if (str(db_duplicate_port) in acl['match'] and + acl not in plugin_acls): + dup_acl = acl + break + # There should have been a duplicate + self.assertIsNotNone(dup_acl) + db_acls.remove(dup_acl) for acl in plugin_acls: if isinstance(acl['severity'], list) and acl['severity']: acl['severity'] = acl['severity'][0] @@ -1786,13 +1799,16 @@ def test_sync_fip_qos_policies(self): nb_synchronizer.sync_fip_qos_policies(ctx) self._validate_qos_records() - def _create_security_group_rule(self, sg_id, direction, tcp_port): + def _create_security_group_rule(self, sg_id, direction, tcp_port, + remote_ip_prefix=None): data = {'security_group_rule': {'security_group_id': sg_id, 'direction': direction, 'protocol': constants.PROTO_NAME_TCP, 'ethertype': constants.IPv4, 'port_range_min': tcp_port, 'port_range_max': tcp_port}} + if remote_ip_prefix: + data['security_group_rule']['remote_ip_prefix'] = remote_ip_prefix req = self.new_create_request('security-group-rules', data, self.fmt) res = req.get_response(self.api) sgr = self.deserialize(self.fmt, res) @@ -1863,6 +1879,20 @@ def test_sync_acls_with_logging(self): self._test_sync_acls_helper(test_log=True, log_event=log_const.DROP_EVENT) + def test_sync_acls_overlapping_cidr(self): + data = {'security_group': {'name': 'sgdup'}} + sg_req = self.new_create_request('security-groups', data) + res = sg_req.get_response(self.api) + sg = self.deserialize(self.fmt, res)['security_group'] + + # Add SG rules that map to the same ACL due to normalizing the cidr + for ip_suffix in range(10, 12): + remote_ip_prefix = '192.168.0.' + str(ip_suffix) + '/24' + self._create_security_group_rule( + sg['id'], 'ingress', 9000, remote_ip_prefix=remote_ip_prefix) + + self._validate_acls(db_duplicate_port=9000) + class TestOvnSbSync(base.TestOVNFunctionalBase): From cf83a5906e26ba3dceeef2d1ce03995b14fb5b7c Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 14 Nov 2025 15:29:18 +0100 Subject: [PATCH 03/10] [OVN] Initialize OVN agent in ``start`` method The ``OVNAgentExtensionAPI`` and ``OVNAgentExtensionManager`` instances create different threads instances. Now the OVN agent is using the ``oslo.service`` threading implementation (that uses ``cotyledon``). This library creates a fork of the main process to execute the ``ServiceWrapper`` instance, where the agent is running. Any thread should be created in the child process, at the ``oslo_service.service.Service.start`` method. Closes-Bug: #2131540 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I97d52e8fd2236b7d9210174d05f01c81a2edcd0d (cherry picked from commit 5e92bbfa5080b219de21829f42e5f372001fc78f) --- neutron/agent/ovn/agent/ovn_neutron_agent.py | 20 ++++++++++++++++--- .../agent/ovn/agent/test_ovn_neutron_agent.py | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/neutron/agent/ovn/agent/ovn_neutron_agent.py b/neutron/agent/ovn/agent/ovn_neutron_agent.py index ffebdc4ba91..9be7b0060c8 100644 --- a/neutron/agent/ovn/agent/ovn_neutron_agent.py +++ b/neutron/agent/ovn/agent/ovn_neutron_agent.py @@ -58,9 +58,8 @@ def __init__(self, conf): self._chassis = None self._chassis_id = None self._ovn_bridge = None - self.ext_manager_api = ext_mgr.OVNAgentExtensionAPI() - self.ext_manager = ext_mgr.OVNAgentExtensionManager(self._conf) - self.ext_manager.initialize(None, 'ovn', self) + self.ext_manager_api = None + self.ext_manager = None def __getitem__(self, name): """Return the named extension objet from ``self.ext_manager``""" @@ -159,7 +158,22 @@ def update_neutron_sb_cfg_key(self): 'Chassis_Private', self.chassis, ('external_ids', external_ids)).execute(check_error=True) + def _initialize_ext_manager(self): + """Initialize the externsion manager and the extension manager API. + + This method must be called once, outside the ``__init__`` method and + at the beginning of the ``start`` method. + """ + if not self.ext_manager: + self.ext_manager_api = ext_mgr.OVNAgentExtensionAPI() + self.ext_manager = ext_mgr.OVNAgentExtensionManager(self._conf) + self.ext_manager.initialize(None, 'ovn', self) + def start(self): + # This must be the first operation in the `start` method. + self._initialize_ext_manager() + + # Extension manager configuration. self.ext_manager_api.ovs_idl = self._load_ovs_idl() self.load_config() # Before executing "_load_sb_idl", is is needed to execute diff --git a/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py b/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py index 3ff53017ed9..7211c8f9ee8 100644 --- a/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py +++ b/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py @@ -95,6 +95,7 @@ def _start_ovn_neutron_agent(self): 'wait'), \ mock.patch.object(ovn_neutron_agent.OVNNeutronAgent, '_load_ovs_idl') as mock_load_ovs_idl: + agt._initialize_ext_manager() mock_load_ovs_idl.return_value = self._create_ovs_idl(agt) agt.start() external_ids = agt.sb_idl.db_get( From 53c2c8d6e0cd7dd889518c34e4a495e557d74eec Mon Sep 17 00:00:00 2001 From: Elvira Garcia Date: Tue, 18 Nov 2025 17:30:04 +0100 Subject: [PATCH 04/10] [SGL] Ignore port groups that don't come from SGs Previously, OVN logapi driver only considered that Port Groups and ACLs were created by Security Groups. This is not true, since the FWaaS plugin does also manage ACLs and Port Groups to apply its rules. This means that port groups do not necessarily need to have a security group ID referenced on their external IDs. Closes-Bug: #2131209 Change-Id: I7f85077ce344d4d0e46190674fc79c42a9eeae9a Signed-off-by: Elvira Garcia (cherry picked from commit c0f6145b871de7fa750fbf09c125d6feb1829031) --- neutron/services/logapi/drivers/ovn/driver.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/neutron/services/logapi/drivers/ovn/driver.py b/neutron/services/logapi/drivers/ovn/driver.py index dc0a025b5ed..8871dda6c80 100644 --- a/neutron/services/logapi/drivers/ovn/driver.py +++ b/neutron/services/logapi/drivers/ovn/driver.py @@ -158,6 +158,11 @@ def _set_acls_log(self, pgs, context, ovn_txn, actions_enabled, log_name): for pg in pgs: meter_name = self.meter_name if pg["name"] != ovn_const.OVN_DROP_PORT_GROUP_NAME: + if ovn_const.OVN_SG_EXT_ID_KEY not in pg["external_ids"]: + LOG.info("Port Group %s is not part of any security " + "group, skipping its network log " + "setting...", pg["name"]) + continue sg = sg_obj.SecurityGroup.get_sg_by_id( context, pg["external_ids"][ovn_const.OVN_SG_EXT_ID_KEY]) if not sg: @@ -426,6 +431,11 @@ def _set_neutron_acls_log(self, pgs, context, actions_enabled, log_name, for pg in pgs: meter_name = self.meter_name if pg['name'] != ovn_const.OVN_DROP_PORT_GROUP_NAME: + if ovn_const.OVN_SG_EXT_ID_KEY not in pg["external_ids"]: + LOG.info("Port Group %s is not part of any security " + "group, skipping its network log " + "setting...", pg["name"]) + continue sg = sg_obj.SecurityGroup.get_sg_by_id(context, pg['external_ids'][ovn_const.OVN_SG_EXT_ID_KEY]) if not sg: From 72165d09aa203cdb0b204a2c33ebbee3358d1f63 Mon Sep 17 00:00:00 2001 From: Sergey Kraynev Date: Sat, 8 Nov 2025 21:19:17 +0400 Subject: [PATCH 05/10] Replace response body in after hook for 404 error Closes-Bug: 2130972 Change-Id: I662b3eb7fb8c17d0ad30b10e8537d4a142003256 Signed-off-by: Sergey Kraynev (cherry picked from commit d95e3e5eaf24ee788d65255d669671e175f25208) --- neutron/pecan_wsgi/hooks/policy_enforcement.py | 9 +++++++++ neutron/tests/functional/pecan_wsgi/test_hooks.py | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/neutron/pecan_wsgi/hooks/policy_enforcement.py b/neutron/pecan_wsgi/hooks/policy_enforcement.py index a6b469edc40..cba9d1573ad 100644 --- a/neutron/pecan_wsgi/hooks/policy_enforcement.py +++ b/neutron/pecan_wsgi/hooks/policy_enforcement.py @@ -16,6 +16,7 @@ from neutron_lib import constants as const from oslo_log import log as logging from oslo_policy import policy as oslo_policy +from oslo_serialization import jsonutils from oslo_utils import excutils from pecan import hooks import webob @@ -190,6 +191,14 @@ def after(self, state): # we have to set the status_code here to prevent the catch_errors # middleware from turning this into a 500. state.response.status_code = 404 + # replace the original body on NotFound body + error_message = { + 'type': 'HTTPNotFound', + 'message': 'The resource could not be found.', + 'detail': '' + } + state.response.text = jsonutils.dumps(error_message) + state.response.content_type = 'application/json' return if is_single: diff --git a/neutron/tests/functional/pecan_wsgi/test_hooks.py b/neutron/tests/functional/pecan_wsgi/test_hooks.py index 9f43659aaef..ec091e3c066 100644 --- a/neutron/tests/functional/pecan_wsgi/test_hooks.py +++ b/neutron/tests/functional/pecan_wsgi/test_hooks.py @@ -288,6 +288,12 @@ def test_after_on_get_not_found(self): headers={'X-Project-Id': 'tenid'}, expect_errors=True) self.assertEqual(404, response.status_int) + self.assertEqual( + { + 'type': 'HTTPNotFound', + 'message': 'The resource could not be found.', + 'detail': '' + }, jsonutils.loads(response.body)) self.assertEqual(1, self.mock_plugin.get_meh.call_count) def test_after_on_get_excludes_admin_attribute(self): From f3a399717130cb9b0451b635e3dd109bfbe8b257 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 26 Nov 2025 12:40:42 +0100 Subject: [PATCH 06/10] [OVN] Add 'qos-bw-minimum-ingress' ML2 extension Closes-Bug: #2132982 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I86ed1aa473a41ece4478e19b1e0f37aadaf93238 (cherry picked from commit 45b8dfe6a8d06ba5dc39ab28d2cb5adc4e7a8f5a) --- neutron/common/ovn/extensions.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/neutron/common/ovn/extensions.py b/neutron/common/ovn/extensions.py index afd6bf4f900..f2b15634096 100644 --- a/neutron/common/ovn/extensions.py +++ b/neutron/common/ovn/extensions.py @@ -70,6 +70,7 @@ from neutron_lib.api.definitions import qinq from neutron_lib.api.definitions import qos from neutron_lib.api.definitions import qos_bw_limit_direction +from neutron_lib.api.definitions import qos_bw_minimum_ingress from neutron_lib.api.definitions import qos_default from neutron_lib.api.definitions import qos_gateway_ip from neutron_lib.api.definitions import qos_rule_type_details @@ -171,6 +172,7 @@ qinq.ALIAS, qos.ALIAS, qos_bw_limit_direction.ALIAS, + qos_bw_minimum_ingress.ALIAS, qos_default.ALIAS, qos_rule_type_details.ALIAS, qos_rule_type_filter.ALIAS, From c1219812bf18d6babbac13f2affbf2e4eb3df826 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Thu, 20 Nov 2025 18:19:33 -0500 Subject: [PATCH 07/10] Fix incorrect log message in ovn_client String contained %s but had no argument, added port id. Closes-bug: #2132088 Change-Id: I36d3c036c9795603d93b01bfd94d2ae80dc2ec7b Signed-off-by: Brian Haley (cherry picked from commit 807e6a5cf633cf7e51534bec655b98a1281861bb) --- .../plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 0265c9768d1..f62f91ee95a 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 @@ -307,7 +307,7 @@ def update_lsp_host_info(self, context, db_port, up=True): if up: if not port_up: LOG.warning('Logical_Switch_Port %s host information not ' - 'updated, the port state is down') + 'updated, the port state is down', db_port.id) return if not db_port.port_bindings: @@ -333,7 +333,7 @@ def update_lsp_host_info(self, context, db_port, up=True): else: if port_up: LOG.warning('Logical_Switch_Port %s host information not ' - 'removed, the port state is up') + 'removed, the port state is up', db_port.id) return cmd.append( From 7360e1f1d682f3ce042dd4901164c17b1efd992d Mon Sep 17 00:00:00 2001 From: Dmitriy Rabotyagov Date: Fri, 3 Oct 2025 18:16:13 +0200 Subject: [PATCH 08/10] Append content-encoding header webob does not automatically detect and add content-encoding header if body is compressed. Instead, this header needs to be passed to webob.Response as a headerlist argument. Only in this case `decode_content()` performs decompression. However, Nova does not add 'content-encoding' in response, and even if it would - only selected headers are currenly consumed by agent. Thus, we generalize approach for handling headers and ensure that 'content-encoding' is present in case we detect magic file signature at the beginning of data. Closes-Bug: #2120723 Change-Id: I45d99e2bae3768f2258f39c9b92e7c2cbd080e7c Signed-off-by: Dmitriy Rabotyagov (cherry picked from commit 67969c147baaca18dc436b674f73c0eb7485cc6c) --- neutron/common/metadata.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/neutron/common/metadata.py b/neutron/common/metadata.py index b17765b8864..d3ade12235c 100644 --- a/neutron/common/metadata.py +++ b/neutron/common/metadata.py @@ -33,7 +33,10 @@ PROXY_SERVICE_NAME = 'haproxy' PROXY_SERVICE_CMD = 'haproxy' -CONTENT_ENCODERS = ('gzip', 'deflate') +CONTENT_ENCODERS = { + 'gzip': b'\x1f\x8b\x08', + 'deflate': b'\x1f\x8b\x08' +} class InvalidUserOrGroupException(Exception): @@ -159,15 +162,21 @@ class MetadataProxyHandlerBaseSocketServer( metaclass=abc.ABCMeta): @staticmethod def _http_response(http_response, request): + headerlist = list(http_response.headers.items()) + # We detect if content is compressed by magic signature, + # when `content-encoding` is not present. + if not http_response.headers.get('content-encoding'): + if http_response.content[:3] == CONTENT_ENCODERS['gzip']: + headerlist.append(('content-encoding', 'gzip')) + _res = webob.Response( body=http_response.content, status=http_response.status_code, - content_type=http_response.headers['content-type'], - charset=http_response.encoding) + headerlist=headerlist) # The content of the response is decoded depending on the # "Context-Enconding" header, if present. The operation is limited to # ("gzip", "deflate"), as is in the ``webob.response.Response`` class. - if _res.content_encoding in CONTENT_ENCODERS: + if _res.content_encoding in CONTENT_ENCODERS.keys(): _res.decode_content() # NOTE(ralonsoh): there should be a better way to format the HTTP From abfca726e59492a727297f1935875765f4bad577 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Thu, 27 Nov 2025 14:04:53 +0100 Subject: [PATCH 09/10] Don't prevent setting CIDRs as allowed_address_pair for port Previously with [1] we blocked possiblility to set as allowed address pair for port any IP or CIDR which contains IP address assigned to the distributed metadata IP address in same network. It was done that way because setting distributed metadata IP address as allowed address pair for any port in the network breaks metadata service for all of the ports in that network. But this restriction was too strict as it also prevented to set CIDRs bigger then /32 or /128 in the allowed_address_pair if CIDR contained distributed metadata port IP. For example: - distributed metadata port IP address 10.0.0.2 - allowed address pairs set for port in that network: - 10.0.0.3 - allowed - 10.0.0.1/26 - not allowed as 10.0.0.2 belongs to that CIDR. In such case however, when CIDR is set as allowed_address_pair, it is not set in OVN as Virtual IP so it won't break connectivity to the metadata service as was reported in [2] thus we should allow that. This patch is reducing that restriction. Now CIDRs can be set as allowed_address_pair for the port even if it includes IP assigned for the distributed metadata port. It is only forbidden to set as allowed_address_pair same, single IP address as set for the distributed metadata port. Closes-Bug: #2131928 [1] https://review.opendev.org/c/openstack/neutron/+/955757 [2] https://bugs.launchpad.net/neutron/+bug/2116249 Change-Id: Ieb98a126b6d380894456ed892c0a19787e7fbb04 Signed-off-by: Slawek Kaplonski (cherry picked from commit 71ae26e3529161bd5101bd42ade4d2f1b3b4c781) --- .../ml2/drivers/ovn/mech_driver/mech_driver.py | 10 +++++++++- .../drivers/ovn/mech_driver/test_mech_driver.py | 14 ++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) 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 ec87494a1bc..52c0ddfcca6 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -655,6 +655,14 @@ def _get_common_ips(ip_addresses, ip_networks): common_ips.add(str(ip_address)) return common_ips + # NOTE(slaweq): We can safely ignore any CIDR larger than /32 (for + # IPv4) or /128 (for IPv6) in the allowed_address_pairs, since such + # CIDRs cannot be set as a Virtual IP in OVN. + # Only /32 and /128 CIDRs are allowed to be set as Virtual IPs in OVN. + address_pairs_to_check = [ + ip_net for ip_net in port_allowed_address_pairs_ip_addresses + if ip_net.size == 1] + for distributed_port in distributed_ports: distributed_port_ip_addresses = [ netaddr.IPAddress(fixed_ip['ip_address']) for fixed_ip in @@ -662,7 +670,7 @@ def _get_common_ips(ip_addresses, ip_networks): common_ips = _get_common_ips( distributed_port_ip_addresses, - port_allowed_address_pairs_ip_addresses) + address_pairs_to_check) if common_ips: err_msg = ( 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 7bd6e1bc386..27ac90d9d2b 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 @@ -3212,17 +3212,23 @@ def test_create_port_with_allowed_address_pairs(self): 'mac_address': port2['mac_address']}], port2['allowed_address_pairs']) - # Now test the same but giving a subnet as allowed address pair - self._make_port( + # Now test the same but giving a subnet as allowed address + # pair, this should be fine as we treat only /32 and /128 IPs + # in allowed_address_pairs as Virtual IPs, there is no block + # anything when bigger CIDR is set as that don't break metadata + new_port = self._make_port( self.fmt, network['network']['id'], allowed_address_pairs=[{'ip_address': '10.0.0.2/26'}], - expected_res_status=exc.HTTPBadRequest.code, - arg_list=('allowed_address_pairs',)) + arg_list=('allowed_address_pairs',))['port'] port3 = self._show('ports', port1['id'])['port'] self.assertEqual( [{'ip_address': '10.0.0.3', 'mac_address': port3['mac_address']}], port3['allowed_address_pairs']) + self.assertEqual( + [{'ip_address': '10.0.0.2/26', + 'mac_address': new_port['mac_address']}], + new_port['allowed_address_pairs']) class OVNMechanismDriverTestCase(MechDriverSetupBase, From 607c3c4c45cdc474541621dc4b5d47bbd576f5a5 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 1 Dec 2025 16:51:43 +0100 Subject: [PATCH 10/10] [FT] Wait for the manager to be created (2) This patch reuses the `WaitEvent` created in [1], to check that the expected `Manager` register is created before checking it. [1]https://review.opendev.org/c/openstack/neutron/+/966673 Related-Bug: #2131024 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I0b6568ca6c3844bd32892a2ebb8c36e1fec144c9 (cherry picked from commit 31b78bf23e691c423ae1abcb0da8561a13383b3c) --- .../agent/ovsdb/native/test_helpers.py | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/neutron/tests/functional/agent/ovsdb/native/test_helpers.py b/neutron/tests/functional/agent/ovsdb/native/test_helpers.py index 3b365533df4..6bacde09282 100644 --- a/neutron/tests/functional/agent/ovsdb/native/test_helpers.py +++ b/neutron/tests/functional/agent/ovsdb/native/test_helpers.py @@ -14,7 +14,7 @@ # under the License. from neutron_lib import constants as const -from ovsdbapp.backend.ovs_idl import event +from ovsdbapp.backend.ovs_idl import event as idl_event from neutron.agent.common import ovs_lib from neutron.agent.ovsdb.native import helpers @@ -23,14 +23,21 @@ from neutron.tests.functional import base -class WaitOvsManagerEvent(event.WaitEvent): +class WaitOvsManagerEvent(idl_event.WaitEvent): event_name = 'WaitOvsManagerEvent' - def __init__(self, manager_target): + def __init__(self, manager_target, inactivity_probe=None, event=None): table = 'Manager' - events = (self.ROW_CREATE,) + events = (self.ROW_CREATE,) if not event else (event,) conditions = (('target', '=', manager_target),) super().__init__(events, table, conditions, timeout=10) + self.inactivity_probe = inactivity_probe + + def match_fn(self, event, row, old): + if (self.inactivity_probe is None or + self.inactivity_probe == row.inactivity_probe[0]): + return True + return False class EnableConnectionUriTestCase(base.BaseSudoTestCase): @@ -78,8 +85,13 @@ def test_add_manager_overwrites_existing_manager(self): ovsdb_cfg_connection = 'tcp:127.0.0.1:%s' % _port manager_connection = 'ptcp:%s:127.0.0.1' % _port + inactivity_probe = 10 + manager_event = WaitOvsManagerEvent( + manager_connection, inactivity_probe=inactivity_probe) + ovs.ovsdb.idl.notify_handler.watch_event(manager_event) helpers.enable_connection_uri(ovsdb_cfg_connection, - inactivity_probe=10) + inactivity_probe=inactivity_probe) + manager_event.wait() self.addCleanup(ovs.ovsdb.remove_manager(manager_connection).execute) # First call of enable_connection_uri cretes the manager # and the list returned by get_manager contains it: @@ -89,7 +101,13 @@ def test_add_manager_overwrites_existing_manager(self): # after 2nd call of enable_connection_uri with new value of # inactivity_probe will keep the original manager only the # inactivity_probe value is set: + inactivity_probe = 100 + manager_event = WaitOvsManagerEvent( + manager_connection, inactivity_probe=inactivity_probe, + event='update') + ovs.ovsdb.idl.notify_handler.watch_event(manager_event) helpers.enable_connection_uri(ovsdb_cfg_connection, - inactivity_probe=100) + inactivity_probe=inactivity_probe) + manager_event.wait() my_mans = ovs.ovsdb.get_manager().execute() self.assertIn(manager_connection, my_mans)