From 3efe8dc9954367a957e8a492ee3c391ac768622a Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 22 Apr 2025 08:27:09 +0000 Subject: [PATCH 1/4] Unify ``MetadataProxyHandlerBaseSocketServer`` class The class ``MetadataProxyHandlerBaseSocketServer`` is implemented only once in ``neutron.common.metadata``. Conflicts: neutron/agent/metadata/agent.py Partial-Bug: #2100585 Change-Id: I17f80ec62398eeb0135a7ab2ff59aa692d2fa086 (cherry picked from commit af3cab07372cc9685418b57ff12fecf98cbb96ef) Signed-off-by: Rodolfo Alonso Hernandez --- neutron/agent/metadata/agent.py | 96 +------------------- neutron/agent/ovn/metadata/server_socket.py | 97 +------------------- neutron/common/metadata.py | 98 +++++++++++++++++++++ 3 files changed, 104 insertions(+), 187 deletions(-) diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py index fa6c4e9b973..de3953b23ec 100644 --- a/neutron/agent/metadata/agent.py +++ b/neutron/agent/metadata/agent.py @@ -14,14 +14,12 @@ import io import socketserver -import urllib from neutron_lib.agent import topics from neutron_lib import constants from neutron_lib import context from oslo_config import cfg from oslo_log import log as logging -import requests import webob from neutron._i18n import _ @@ -29,10 +27,8 @@ from neutron.agent.linux import utils as agent_utils from neutron.agent.metadata import proxy_base from neutron.agent import rpc as agent_rpc -from neutron.common import ipv6_utils from neutron.common import loopingcall from neutron.common import metadata as common_metadata -from neutron.common import utils as common_utils LOG = logging.getLogger(__name__) @@ -58,95 +54,9 @@ def __init__(self, topic): version='1.0') -class MetadataProxyHandlerBaseSocketServer( - proxy_base.MetadataProxyHandlerBase): - @staticmethod - def _http_response(http_response, request): - _res = webob.Response( - body=http_response.content, - status=http_response.status_code, - content_type=http_response.headers['content-type'], - charset=http_response.encoding) - # NOTE(ralonsoh): there should be a better way to format the HTTP - # response, adding the HTTP version to the ``webob.Response`` - # output string. - out = request.http_version + ' ' + str(_res) - if (int(_res.headers['content-length']) == 0 and - _res.status_code == 200): - # Add 2 extra \r\n to the result. HAProxy is also expecting - # it even when the body is empty. - out += '\r\n\r\n' - return out.encode(http_response.encoding) - - def _proxy_request(self, instance_id, project_id, req): - headers = { - 'X-Forwarded-For': req.headers.get('X-Forwarded-For'), - 'X-Instance-ID': instance_id, - 'X-Tenant-ID': project_id, - 'X-Instance-ID-Signature': common_utils.sign_instance_id( - self.conf, instance_id) - } - - nova_host_port = ipv6_utils.valid_ipv6_url( - self.conf.nova_metadata_host, - self.conf.nova_metadata_port) - - url = urllib.parse.urlunsplit(( - self.conf.nova_metadata_protocol, - nova_host_port, - req.path_info, - req.query_string, - '')) - - disable_ssl_certificate_validation = self.conf.nova_metadata_insecure - if self.conf.auth_ca_cert and not disable_ssl_certificate_validation: - verify_cert = self.conf.auth_ca_cert - else: - verify_cert = not disable_ssl_certificate_validation - - client_cert = None - if self.conf.nova_client_cert and self.conf.nova_client_priv_key: - client_cert = (self.conf.nova_client_cert, - self.conf.nova_client_priv_key) - - try: - resp = requests.request(method=req.method, url=url, - headers=headers, - data=req.body, - cert=client_cert, - verify=verify_cert, - timeout=60) - except requests.ConnectionError: - msg = _('The remote metadata server is temporarily unavailable. ' - 'Please try again later.') - LOG.warning(msg) - title = '503 Service Unavailable' - return common_metadata.encode_http_reponse(title, title, msg) - - if resp.status_code == 200: - return self._http_response(resp, req) - if resp.status_code == 403: - LOG.warning( - 'The remote metadata server responded with Forbidden. This ' - 'response usually occurs when shared secrets do not match.' - ) - # TODO(ralonsoh): add info in the returned HTTP message to the VM. - return self._http_response(resp, req) - if resp.status_code == 500: - msg = _( - 'Remote metadata server experienced an internal server error.' - ) - LOG.warning(msg) - # TODO(ralonsoh): add info in the returned HTTP message to the VM. - return self._http_response(resp, req) - if resp.status_code in (400, 404, 409, 502, 503, 504): - # TODO(ralonsoh): add info in the returned HTTP message to the VM. - return self._http_response(resp, req) - raise Exception(_('Unexpected response code: %s') % resp.status_code) - - -class MetadataProxyHandler(MetadataProxyHandlerBaseSocketServer, - socketserver.StreamRequestHandler): +class MetadataProxyHandler( + common_metadata.MetadataProxyHandlerBaseSocketServer, + socketserver.StreamRequestHandler): NETWORK_ID_HEADER = 'X-Neutron-Network-ID' ROUTER_ID_HEADER = 'X-Neutron-Router-ID' _conf = None diff --git a/neutron/agent/ovn/metadata/server_socket.py b/neutron/agent/ovn/metadata/server_socket.py index 2c0fb110f5a..d5aa6275f9f 100644 --- a/neutron/agent/ovn/metadata/server_socket.py +++ b/neutron/agent/ovn/metadata/server_socket.py @@ -12,117 +12,26 @@ # See the License for the specific language governing permissions and # limitations under the License. - import io import socketserver -import urllib from oslo_config import cfg from oslo_log import log as logging -import requests import webob from neutron._i18n import _ from neutron.agent.linux import utils as agent_utils from neutron.agent.metadata import proxy_base -from neutron.common import ipv6_utils from neutron.common import metadata as common_metadata from neutron.common.ovn import constants as ovn_const -from neutron.common import utils as common_utils LOG = logging.getLogger(__name__) -class MetadataProxyHandlerBaseSocketServer( - proxy_base.MetadataProxyHandlerBase): - @staticmethod - def _http_response(http_response, request): - _res = webob.Response( - body=http_response.content, - status=http_response.status_code, - content_type=http_response.headers['content-type'], - charset=http_response.encoding) - # NOTE(ralonsoh): there should be a better way to format the HTTP - # response, adding the HTTP version to the ``webob.Response`` - # output string. - out = request.http_version + ' ' + str(_res) - if (int(_res.headers['content-length']) == 0 and - _res.status_code == 200): - # Add 2 extra \r\n to the result. HAProxy is also expecting - # it even when the body is empty. - out += '\r\n\r\n' - return out.encode(http_response.encoding) - - def _proxy_request(self, instance_id, project_id, req): - headers = { - 'X-Forwarded-For': req.headers.get('X-Forwarded-For'), - 'X-Instance-ID': instance_id, - 'X-Tenant-ID': project_id, - 'X-Instance-ID-Signature': common_utils.sign_instance_id( - self.conf, instance_id) - } - - nova_host_port = ipv6_utils.valid_ipv6_url( - self.conf.nova_metadata_host, - self.conf.nova_metadata_port) - - url = urllib.parse.urlunsplit(( - self.conf.nova_metadata_protocol, - nova_host_port, - req.path_info, - req.query_string, - '')) - - disable_ssl_certificate_validation = self.conf.nova_metadata_insecure - if self.conf.auth_ca_cert and not disable_ssl_certificate_validation: - verify_cert = self.conf.auth_ca_cert - else: - verify_cert = not disable_ssl_certificate_validation - - client_cert = None - if self.conf.nova_client_cert and self.conf.nova_client_priv_key: - client_cert = (self.conf.nova_client_cert, - self.conf.nova_client_priv_key) - - try: - resp = requests.request(method=req.method, url=url, - headers=headers, - data=req.body, - cert=client_cert, - verify=verify_cert, - timeout=60) - except requests.ConnectionError: - msg = _('The remote metadata server is temporarily unavailable. ' - 'Please try again later.') - LOG.warning(msg) - title = '503 Service Unavailable' - return common_metadata.encode_http_reponse(title, title, msg) - - if resp.status_code == 200: - return self._http_response(resp, req) - if resp.status_code == 403: - LOG.warning( - 'The remote metadata server responded with Forbidden. This ' - 'response usually occurs when shared secrets do not match.' - ) - # TODO(ralonsoh): add info in the returned HTTP message to the VM. - return self._http_response(resp, req) - if resp.status_code == 500: - msg = _( - 'Remote metadata server experienced an internal server error.' - ) - LOG.warning(msg) - # TODO(ralonsoh): add info in the returned HTTP message to the VM. - return self._http_response(resp, req) - if resp.status_code in (400, 404, 409, 502, 503, 504): - # TODO(ralonsoh): add info in the returned HTTP message to the VM. - return self._http_response(resp, req) - raise Exception(_('Unexpected response code: %s') % resp.status_code) - - -class MetadataProxyHandler(MetadataProxyHandlerBaseSocketServer, - socketserver.StreamRequestHandler): +class MetadataProxyHandler( + common_metadata.MetadataProxyHandlerBaseSocketServer, + socketserver.StreamRequestHandler): NETWORK_ID_HEADER = 'X-OVN-Network-ID' ROUTER_ID_HEADER = '' _conf = None diff --git a/neutron/common/metadata.py b/neutron/common/metadata.py index 334265bd38b..7fe08180718 100644 --- a/neutron/common/metadata.py +++ b/neutron/common/metadata.py @@ -11,10 +11,20 @@ # License for the specific language governing permissions and limitations # under the License. +import abc +from urllib import parse + import jinja2 from neutron_lib import constants from oslo_log import log as logging from oslo_utils import encodeutils +import requests +import webob + +from neutron._i18n import _ +from neutron.agent.metadata import proxy_base +from neutron.common import ipv6_utils +from neutron.common import utils as common_utils LOG = logging.getLogger(__name__) @@ -140,3 +150,91 @@ def encode_http_reponse(http_code, title, message): reponse = RESPONSE.render(http_code=http_code, title=title, body_title=title, body=message, len=length) return encodeutils.to_utf8(reponse) + + +class MetadataProxyHandlerBaseSocketServer( + proxy_base.MetadataProxyHandlerBase, + metaclass=abc.ABCMeta): + @staticmethod + def _http_response(http_response, request): + _res = webob.Response( + body=http_response.content, + status=http_response.status_code, + content_type=http_response.headers['content-type'], + charset=http_response.encoding) + # NOTE(ralonsoh): there should be a better way to format the HTTP + # response, adding the HTTP version to the ``webob.Response`` + # output string. + out = request.http_version + ' ' + str(_res) + if (int(_res.headers['content-length']) == 0 and + _res.status_code == 200): + # Add 2 extra \r\n to the result. HAProxy is also expecting + # it even when the body is empty. + out += '\r\n\r\n' + return out.encode(http_response.encoding) + + def _proxy_request(self, instance_id, project_id, req): + headers = { + 'X-Forwarded-For': req.headers.get('X-Forwarded-For'), + 'X-Instance-ID': instance_id, + 'X-Tenant-ID': project_id, + 'X-Instance-ID-Signature': common_utils.sign_instance_id( + self.conf, instance_id) + } + + nova_host_port = ipv6_utils.valid_ipv6_url( + self.conf.nova_metadata_host, + self.conf.nova_metadata_port) + + url = parse.urlunsplit(( + self.conf.nova_metadata_protocol, + nova_host_port, + req.path_info, + req.query_string, + '')) + + disable_ssl_certificate_validation = self.conf.nova_metadata_insecure + if self.conf.auth_ca_cert and not disable_ssl_certificate_validation: + verify_cert = self.conf.auth_ca_cert + else: + verify_cert = not disable_ssl_certificate_validation + + client_cert = None + if self.conf.nova_client_cert and self.conf.nova_client_priv_key: + client_cert = (self.conf.nova_client_cert, + self.conf.nova_client_priv_key) + + try: + resp = requests.request(method=req.method, url=url, + headers=headers, + data=req.body, + cert=client_cert, + verify=verify_cert, + timeout=60) + except requests.ConnectionError: + msg = _('The remote metadata server is temporarily unavailable. ' + 'Please try again later.') + LOG.warning(msg) + title = '503 Service Unavailable' + return encode_http_reponse(title, title, msg) + + if resp.status_code == 200: + return self._http_response(resp, req) + if resp.status_code == 403: + LOG.warning( + 'The remote metadata server responded with Forbidden. This ' + 'response usually occurs when shared secrets do not match.' + ) + # TODO(ralonsoh): add info in the returned HTTP message to the VM. + return self._http_response(resp, req) + if resp.status_code == 500: + msg = _( + 'Remote metadata server experienced an internal server error.' + ) + LOG.warning(msg) + # TODO(ralonsoh): add info in the returned HTTP message to the VM. + return self._http_response(resp, req) + if resp.status_code in (400, 404, 409, 502, 503, 504): + # TODO(ralonsoh): add info in the returned HTTP message to the VM. + return self._http_response(resp, req) + raise Exception(_('Unexpected response code: %s') % resp.status_code) From d0349fe613918acf616f2ef2a537bb7a9cf6bd3c Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 1 Aug 2025 06:51:32 +0000 Subject: [PATCH 2/4] [OVN] Check LSP is subport before removing it from the trunk In the test ``TestOVNTrunkDriver.test_subport_delete``, a new wait event is added. Before removing the subport from the trunk, the test checks that the OVN NB database has correctly stored the device owner in the external_ids dictionary. More information in [1]. [1]https://bugs.launchpad.net/neutron/+bug/2117405/comments/2 Closes-Bug: #2117405 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I3aa677c1e75415d3ad2ca77ef578494526a44969 (cherry picked from commit fc2a53ccceb37635ae53500d86a1b9e92e90d428) --- .../trunk/drivers/ovn/test_trunk_driver.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py b/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py index 72090c495dd..dc91c37af23 100644 --- a/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py +++ b/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py @@ -36,6 +36,24 @@ def __init__(self, port_id): super().__init__(events, table, conditions, timeout=10) +class WaitForLSPSubportEvent(event.WaitEvent): + event_name = 'WaitForLSPSubportEvent' + + def __init__(self, port_id): + table = 'Logical_Switch_Port' + events = (self.ROW_UPDATE, self.ROW_CREATE) + conditions = (('name', '=', port_id), ) + super().__init__(events, table, conditions, timeout=10) + + def matches(self, event, row, old=None): + # Check the "conditions" defined (name=port_id) + if not super().matches(event, row, old): + return False + device_owner = row.external_ids.get( + ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY) + return device_owner == trunk_consts.TRUNK_SUBPORT_OWNER + + class TestOVNTrunkDriver(base.TestOVNFunctionalBase): def setUp(self): @@ -127,7 +145,12 @@ def test_subport_add(self): def test_subport_delete(self): with self.subport() as subport: + lsp_subport = WaitForLSPSubportEvent(subport['port_id']) + self.mech_driver.nb_ovn.idl.notify_handler.watch_event( + lsp_subport) with self.trunk([subport]) as trunk: + # Wait for the subport LSP to be assigned as a subport. + self.assertTrue(lsp_subport.wait()) pb_event = WaitForPortBindingDeleteEvent(subport['port_id']) self.mech_driver.sb_ovn.idl.notify_handler.watch_event( pb_event) @@ -135,6 +158,7 @@ def test_subport_delete(self): {'sub_ports': [subport]}) new_trunk = self.trunk_plugin.get_trunk(self.context, trunk['id']) + # Wait for the subport LSP to be unbound. self.assertTrue(pb_event.wait()) self._verify_trunk_info(new_trunk, has_items=False) From 4fa5543309bb514c09f5b4b1d829ea9de3f297da Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 9 Sep 2025 10:30:27 +0000 Subject: [PATCH 3/4] [OVN][FT] Check the subport Port_Binding is created before deletion When a trunk subport is created and assigned to a trunk, a "Port_Binding" register is created. If the subport is then deleted from the trunk, this "Port_Binding" register is deleted. This patch, before removing the subport from the trunk, adds a check for this "Port_Binding" register creation. Closes-Bug: #2117405 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I68a8c972ed88e4c62dc4de59fe3904cb85278b1c (cherry picked from commit 40692e922ac35a36e9039d2f46008c53fdd8098a) --- .../trunk/drivers/ovn/test_trunk_driver.py | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py b/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py index dc91c37af23..754ea7b19cc 100644 --- a/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py +++ b/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py @@ -27,8 +27,6 @@ class WaitForPortBindingDeleteEvent(event.WaitEvent): - event_name = 'WaitForPortBindingDeleteEvent' - def __init__(self, port_id): table = 'Port_Binding' events = (self.ROW_DELETE, ) @@ -36,6 +34,14 @@ def __init__(self, port_id): super().__init__(events, table, conditions, timeout=10) +class WaitForPortBindingCreateEvent(event.WaitEvent): + def __init__(self, port_id): + table = 'Port_Binding' + events = (self.ROW_CREATE, ) + conditions = (('logical_port', '=', port_id), ) + super().__init__(events, table, conditions, timeout=10) + + class WaitForLSPSubportEvent(event.WaitEvent): event_name = 'WaitForLSPSubportEvent' @@ -148,18 +154,23 @@ def test_subport_delete(self): lsp_subport = WaitForLSPSubportEvent(subport['port_id']) self.mech_driver.nb_ovn.idl.notify_handler.watch_event( lsp_subport) + pb_create = WaitForPortBindingCreateEvent(subport['port_id']) + self.mech_driver.sb_ovn.idl.notify_handler.watch_event( + pb_create) with self.trunk([subport]) as trunk: # Wait for the subport LSP to be assigned as a subport. self.assertTrue(lsp_subport.wait()) - pb_event = WaitForPortBindingDeleteEvent(subport['port_id']) + self.assertTrue(pb_create.wait()) + + pb_delete = WaitForPortBindingDeleteEvent(subport['port_id']) self.mech_driver.sb_ovn.idl.notify_handler.watch_event( - pb_event) + pb_delete) self.trunk_plugin.remove_subports(self.context, trunk['id'], {'sub_ports': [subport]}) new_trunk = self.trunk_plugin.get_trunk(self.context, trunk['id']) # Wait for the subport LSP to be unbound. - self.assertTrue(pb_event.wait()) + self.assertTrue(pb_delete.wait()) self._verify_trunk_info(new_trunk, has_items=False) def test_trunk_delete(self): From d84c51fd2fe60563e7c7a44256c608c03548a3d9 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 4 Sep 2025 16:52:41 +0000 Subject: [PATCH 4/4] Decode the metadata response before sending it to the VM The metadata server response can be encoded. The metadata proxy decodes it before crafting the HTTP message that will be delivered to the virtual machine. It requires that the HTTP message sent by the metadata server contains the proper "Context-Encoding" header, defining the encoding type used. This header is not provided by default (the content is not encoded) or is empty. NOTE: unless we provide a method to encode the Nova metadata server content, it would not be possible to properly test this patch. Closes-Bug: #2120723 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I747872f031cc5a1a87ced69bb0af645c088143f3 (cherry picked from commit c031b59ec116db7c9d4c5e102e5cb7d274ae1975) --- neutron/common/metadata.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/neutron/common/metadata.py b/neutron/common/metadata.py index 7fe08180718..b17765b8864 100644 --- a/neutron/common/metadata.py +++ b/neutron/common/metadata.py @@ -33,6 +33,8 @@ PROXY_SERVICE_NAME = 'haproxy' PROXY_SERVICE_CMD = 'haproxy' +CONTENT_ENCODERS = ('gzip', 'deflate') + class InvalidUserOrGroupException(Exception): pass @@ -162,6 +164,12 @@ def _http_response(http_response, request): status=http_response.status_code, content_type=http_response.headers['content-type'], charset=http_response.encoding) + # 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: + _res.decode_content() + # NOTE(ralonsoh): there should be a better way to format the HTTP # response, adding the HTTP version to the ``webob.Response`` # output string.