From 6395b4fe8ed99855853587fa93cb59fd2691aed5 Mon Sep 17 00:00:00 2001 From: Anton Kurbatov Date: Mon, 25 Mar 2024 18:49:52 +0000 Subject: [PATCH] Fixing the 500 HTTP code in the metadata service if Nova is down If the Nova metadata service is unavailable, the requests.request() function may raise a ConnectionError. This results in the upper code returning a 500 HTTP status code to the user along with a traceback. Let's handle this scenario and instead return a 503 HTTP status code (service unavailable). If the Nova service is down and is behind another proxy (such as Nginx), then instead of a ConnectionError, the request may result in receiving a 502 or 503 HTTP status code. Let's also consider this situation and add support for an additional 504 code. Closes-Bug: #2059032 Change-Id: I16be18c46a6796224b0793dc385b0ddec01739c4 --- neutron/agent/metadata/agent.py | 27 ++++++++++--------- neutron/agent/ovn/metadata/server.py | 27 ++++++++++--------- .../tests/unit/agent/metadata/test_agent.py | 21 +++++++++++++++ .../unit/agent/ovn/metadata/test_server.py | 21 +++++++++++++++ ...hance-error-handling-3655404d44249097.yaml | 6 +++++ 5 files changed, 78 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/metadata-proxy-enhance-error-handling-3655404d44249097.yaml diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py index c3d94979ed9..5c0a2062e8c 100644 --- a/neutron/agent/metadata/agent.py +++ b/neutron/agent/metadata/agent.py @@ -246,12 +246,18 @@ class MetadataProxyHandler(object): client_cert = (self.conf.nova_client_cert, self.conf.nova_client_priv_key) - resp = requests.request(method=req.method, url=url, - headers=headers, - data=req.body, - cert=client_cert, - verify=verify_cert, - timeout=60) + 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.') + explanation = str(msg) + return webob.exc.HTTPServiceUnavailable(explanation=explanation) if resp.status_code == 200: req.response.content_type = resp.headers['content-type'] @@ -264,12 +270,6 @@ class MetadataProxyHandler(object): 'response usually occurs when shared secrets do not match.' ) return webob.exc.HTTPForbidden() - elif resp.status_code == 400: - return webob.exc.HTTPBadRequest() - elif resp.status_code == 404: - return webob.exc.HTTPNotFound() - elif resp.status_code == 409: - return webob.exc.HTTPConflict() elif resp.status_code == 500: msg = _( 'Remote metadata server experienced an internal server error.' @@ -277,6 +277,9 @@ class MetadataProxyHandler(object): LOG.warning(msg) explanation = str(msg) return webob.exc.HTTPInternalServerError(explanation=explanation) + elif resp.status_code in (400, 404, 409, 502, 503, 504): + webob_exc_cls = webob.exc.status_map.get(resp.status_code) + return webob_exc_cls() else: raise Exception(_('Unexpected response code: %s') % resp.status_code) diff --git a/neutron/agent/ovn/metadata/server.py b/neutron/agent/ovn/metadata/server.py index 466b70681a9..58ef4cff82e 100644 --- a/neutron/agent/ovn/metadata/server.py +++ b/neutron/agent/ovn/metadata/server.py @@ -168,12 +168,18 @@ class MetadataProxyHandler(object): client_cert = (self.conf.nova_client_cert, self.conf.nova_client_priv_key) - resp = requests.request(method=req.method, url=url, - headers=headers, - data=req.body, - cert=client_cert, - verify=verify_cert, - timeout=60) + 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.') + explanation = str(msg) + return webob.exc.HTTPServiceUnavailable(explanation=explanation) if resp.status_code == 200: req.response.content_type = resp.headers['content-type'] @@ -186,12 +192,6 @@ class MetadataProxyHandler(object): 'response usually occurs when shared secrets do not match.' ) return webob.exc.HTTPForbidden() - elif resp.status_code == 400: - return webob.exc.HTTPBadRequest() - elif resp.status_code == 404: - return webob.exc.HTTPNotFound() - elif resp.status_code == 409: - return webob.exc.HTTPConflict() elif resp.status_code == 500: msg = _( 'Remote metadata server experienced an internal server error.' @@ -199,6 +199,9 @@ class MetadataProxyHandler(object): LOG.warning(msg) explanation = str(msg) return webob.exc.HTTPInternalServerError(explanation=explanation) + elif resp.status_code in (400, 404, 409, 502, 503, 504): + webob_exc_cls = webob.exc.status_map.get(resp.status_code) + return webob_exc_cls() else: raise Exception(_('Unexpected response code: %s') % resp.status_code) diff --git a/neutron/tests/unit/agent/metadata/test_agent.py b/neutron/tests/unit/agent/metadata/test_agent.py index cc8ff95d5bc..eb03ef2e6ba 100644 --- a/neutron/tests/unit/agent/metadata/test_agent.py +++ b/neutron/tests/unit/agent/metadata/test_agent.py @@ -17,6 +17,7 @@ from unittest import mock import ddt import netaddr from neutron_lib import constants as n_const +import requests import testtools import webob @@ -469,10 +470,30 @@ class _TestMetadataProxyHandlerCacheMixin(object): self.assertIsInstance(self._proxy_request_test_helper(500), webob.exc.HTTPInternalServerError) + def test_proxy_request_502(self): + self.assertIsInstance(self._proxy_request_test_helper(502), + webob.exc.HTTPBadGateway) + + def test_proxy_request_503(self): + self.assertIsInstance(self._proxy_request_test_helper(503), + webob.exc.HTTPServiceUnavailable) + + def test_proxy_request_504(self): + self.assertIsInstance(self._proxy_request_test_helper(504), + webob.exc.HTTPGatewayTimeout) + def test_proxy_request_other_code(self): with testtools.ExpectedException(Exception): self._proxy_request_test_helper(302) + def test_proxy_request_conenction_error(self): + req = mock.Mock(path_info='/the_path', query_string='', headers={}, + method='GET', body='') + with mock.patch('requests.request') as mock_request: + mock_request.side_effect = requests.ConnectionError() + retval = self.handler._proxy_request('the_id', 'tenant_id', req) + self.assertIsInstance(retval, webob.exc.HTTPServiceUnavailable) + class TestMetadataProxyHandlerNewCache(TestMetadataProxyHandlerBase, _TestMetadataProxyHandlerCacheMixin): diff --git a/neutron/tests/unit/agent/ovn/metadata/test_server.py b/neutron/tests/unit/agent/ovn/metadata/test_server.py index c8ede299357..aa154868299 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_server.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_server.py @@ -18,6 +18,7 @@ from unittest import mock from oslo_config import cfg from oslo_config import fixture as config_fixture from oslo_utils import fileutils +import requests import testtools import webob @@ -232,10 +233,30 @@ class TestMetadataProxyHandler(base.BaseTestCase): self.assertIsInstance(self._proxy_request_test_helper(500), webob.exc.HTTPInternalServerError) + def test_proxy_request_502(self): + self.assertIsInstance(self._proxy_request_test_helper(502), + webob.exc.HTTPBadGateway) + + def test_proxy_request_503(self): + self.assertIsInstance(self._proxy_request_test_helper(503), + webob.exc.HTTPServiceUnavailable) + + def test_proxy_request_504(self): + self.assertIsInstance(self._proxy_request_test_helper(504), + webob.exc.HTTPGatewayTimeout) + def test_proxy_request_other_code(self): with testtools.ExpectedException(Exception): self._proxy_request_test_helper(302) + def test_proxy_request_conenction_error(self): + req = mock.Mock(path_info='/the_path', query_string='', headers={}, + method='GET', body='') + with mock.patch('requests.request') as mock_request: + mock_request.side_effect = requests.ConnectionError() + retval = self.handler._proxy_request('the_id', 'tenant_id', req) + self.assertIsInstance(retval, webob.exc.HTTPServiceUnavailable) + class TestUnixDomainMetadataProxy(base.BaseTestCase): def setUp(self): diff --git a/releasenotes/notes/metadata-proxy-enhance-error-handling-3655404d44249097.yaml b/releasenotes/notes/metadata-proxy-enhance-error-handling-3655404d44249097.yaml new file mode 100644 index 00000000000..7f085db8cfe --- /dev/null +++ b/releasenotes/notes/metadata-proxy-enhance-error-handling-3655404d44249097.yaml @@ -0,0 +1,6 @@ +--- +other: + - | + Enhance error handling in the Neutron metadata service for cases when the + Nova metadata service is unavailable, ensuring correct HTTP status codes + are returned.