openstack/swift
Code Issues Proposed changes

Merge "proxy: extract response error handling to single method"

Browse Source
This commit is contained in:
Zuul 2022-11-14 17:18:23 +00:00 committed by Gerrit Code Review
commit 61e7ac3304
5 changed files with 126 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
swift/proxy/controllers/base.py
View File

@ -55,7 +55,7 @@ from swift.common.header_key_dict import HeaderKeyDict
from swift.common.http import is_informational, is_success, is_redirection, \
is_server_error, HTTP_OK, HTTP_PARTIAL_CONTENT, HTTP_MULTIPLE_CHOICES, \
HTTP_BAD_REQUEST, HTTP_NOT_FOUND, HTTP_SERVICE_UNAVAILABLE, \
HTTP_INSUFFICIENT_STORAGE, HTTP_UNAUTHORIZED, HTTP_CONTINUE, HTTP_GONE
HTTP_UNAUTHORIZED, HTTP_CONTINUE, HTTP_GONE
from swift.common.swob import Request, Response, Range, \
HTTPException, HTTPRequestedRangeNotSatisfiable, HTTPServiceUnavailable, \
status_map, wsgi_to_str, str_to_wsgi, wsgi_quote, wsgi_unquote, \
@ -1473,15 +1473,9 @@ class GetOrHeadHandler(object):
ts = Timestamp(hdrs.get('X-Backend-Timestamp', 0))
if ts > self.latest_404_timestamp:
self.latest_404_timestamp = ts
if possible_source.status == HTTP_INSUFFICIENT_STORAGE:
self.app.error_limit(node, 'ERROR Insufficient Storage')
elif is_server_error(possible_source.status):
self.app.error_occurred(
node, ('ERROR %(status)d %(body)s '
'From %(type)s Server') %
{'status': possible_source.status,
'body': self.bodies[-1][:1024],
'type': self.server_type})
self.app.check_response(node, self.server_type, possible_source,
self.req_method, self.path,
self.bodies[-1])
return False
def _get_source_and_node(self):
@ -1914,21 +1908,12 @@ class Controller(object):
conn.send(body)
with Timeout(self.app.node_timeout):
resp = conn.getresponse()
if not is_informational(resp.status) and \
not is_server_error(resp.status):
if (self.app.check_response(node, self.server_type, resp,
method, path)
and not is_informational(resp.status)):
return resp.status, resp.reason, resp.getheaders(), \
resp.read()
elif resp.status == HTTP_INSUFFICIENT_STORAGE:
self.app.error_limit(node,
'ERROR Insufficient Storage')
elif is_server_error(resp.status):
msg = ('ERROR %(status)d Trying to '
'%(method)s %(path)s From %(type)s Server')
self.app.error_occurred(node, msg % {
'status': resp.status,
'method': method,
'path': path,
'type': self.server_type})
except (Exception, Timeout):
self.app.exception_occurred(
node, self.server_type,

24
swift/proxy/controllers/obj.py
View File

@ -478,18 +478,9 @@ class BaseObjectController(Controller):
else:
body = b''
bodies.append(body)
if response.status == HTTP_INSUFFICIENT_STORAGE:
if not self.app.check_response(putter.node, 'Object', response,
req.method, req.path, body):
putter.failed = True
self.app.error_limit(putter.node,
'ERROR Insufficient Storage')
elif response.status >= HTTP_INTERNAL_SERVER_ERROR:
putter.failed = True
self.app.error_occurred(
putter.node,
'ERROR %(status)d %(body)s From Object Server '
're: %(path)s' %
{'status': response.status,
'body': body[:1024], 'path': req.path})
elif is_success(response.status):
etags.add(normalize_etag(response.getheader('etag')))
@ -2791,15 +2782,8 @@ class ECFragGetter(object):
self.body = possible_source.read()
conn.close()
if possible_source.status == HTTP_INSUFFICIENT_STORAGE:
self.app.error_limit(node, 'ERROR Insufficient Storage')
elif is_server_error(possible_source.status):
self.app.error_occurred(
node, 'ERROR %(status)d %(body)s '
'From Object Server' %
{'status': possible_source.status,
'body': self.body[:1024]})
else:
if self.app.check_response(node, 'Object', possible_source, 'GET',
self.path):
self.logger.debug(
'Ignoring %s from primary' % possible_source.status)

40
swift/proxy/server.py
View File

@ -28,7 +28,7 @@ from eventlet import Timeout
from swift import __canonical_version__ as swift_version
from swift.common import constraints
from swift.common.http import is_server_error
from swift.common.http import is_server_error, HTTP_INSUFFICIENT_STORAGE
from swift.common.storage_policy import POLICIES
from swift.common.ring import Ring
from swift.common.error_limiter import ErrorLimiter
@ -688,6 +688,44 @@ class Application(object):
self.logger.error('%(msg)s %(node)s',
{'msg': msg, 'node': node_to_string(node)})
def check_response(self, node, server_type, response, method, path,
body=None):
"""
Check response for error status codes and update error limiters as
required.
:param node: a dict describing a node
:param server_type: the type of server from which the response was
received (e.g. 'Object').
:param response: an instance of HTTPResponse.
:param method: the request method.
:param path: the request path.
:param body: an optional response body. If given, up to 1024 of the
start of the body will be included in any log message.
:return True: if the response status code is less than 500, False
otherwise.
"""
ok = False
if response.status == HTTP_INSUFFICIENT_STORAGE:
self.error_limit(node, 'ERROR Insufficient Storage')
elif is_server_error(response.status):
values = {'status': response.status,
'method': method,
'path': path,
'type': server_type}
if body is None:
fmt = 'ERROR %(status)d Trying to %(method)s ' \
'%(path)s From %(type)s Server'
else:
fmt = 'ERROR %(status)d %(body)s Trying to %(method)s ' \
'%(path)s From %(type)s Server'
values['body'] = body[:1024]
self.error_occurred(node, fmt % values)
else:
ok = True
return ok
def iter_nodes(self, ring, partition, logger, node_iter=None, policy=None):
return NodeIter(self, ring, partition, logger, node_iter=node_iter,
policy=policy)

8
test/unit/proxy/controllers/test_obj.py
View File

@ -1365,10 +1365,14 @@ class TestReplicatedObjController(CommonObjectControllerMixin,
req, log_lines = do_test((201, (100, 500), 201))
if six.PY3:
# We allow the b'' in logs because we want to see bad characters.
self.assertIn("ERROR 500 b'' From Object Server", log_lines[0])
self.assertIn(
"ERROR 500 b'' Trying to PUT /v1/AUTH_kilroy/%ED%88%8E/"
"%E9%90%89 From Object Server", log_lines[0])
self.assertIn(req.path, log_lines[0])
else:
self.assertIn('ERROR 500 From Object Server', log_lines[0])
self.assertIn(
'ERROR 500 Trying to PUT /v1/AUTH_kilroy/%ED%88%8E/%E9%90%89 '
'From Object Server', log_lines[0])
self.assertIn(req.path.decode('utf-8'), log_lines[0])
def test_DELETE_errors(self):

70
test/unit/proxy/test_server.py
View File

@ -56,7 +56,7 @@ from test.debug_logger import debug_logger
from test.unit import (
connect_tcp, readuntil2crlfs, fake_http_connect, FakeRing, FakeMemcache,
patch_policies, write_fake_ring, mocked_http_conn, DEFAULT_TEST_EC_TYPE,
make_timestamp_iter, skip_if_no_xattrs)
make_timestamp_iter, skip_if_no_xattrs, FakeHTTPResponse)
from test.unit.helpers import setup_servers, teardown_servers
from swift.proxy import server as proxy_server
from swift.proxy.controllers.obj import ReplicatedObjectController
@ -1288,6 +1288,74 @@ class TestProxyServer(unittest.TestCase):
self.assertIs(log_kwargs['exc_info'][1], expected_err)
self.assertEqual(4, node_error_count(app, node))
def test_check_response_200(self):
app = proxy_server.Application({},
account_ring=FakeRing(),
container_ring=FakeRing(),
logger=debug_logger())
node = app.container_ring.get_part_nodes(1)[0]
resp = FakeHTTPResponse(Response())
ret = app.check_response(node, 'Container', resp, 'PUT', '/v1/a/c')
self.assertTrue(ret)
error_lines = app.logger.get_lines_for_level('error')
self.assertFalse(error_lines)
self.assertEqual(0, node_error_count(app, node))
def test_check_response_507(self):
app = proxy_server.Application({},
account_ring=FakeRing(),
container_ring=FakeRing(),
logger=debug_logger())
node = app.container_ring.get_part_nodes(1)[0]
resp = FakeHTTPResponse(Response(status=507))
ret = app.check_response(node, 'Container', resp, 'PUT', '/v1/a/c')
self.assertFalse(ret)
error_lines = app.logger.get_lines_for_level('error')
self.assertEqual(1, len(error_lines))
self.assertEqual('ERROR Insufficient Storage 10.0.0.0:1000/sda',
error_lines[0])
self.assertEqual(11, node_error_count(app, node))
self.assertTrue(app.error_limited(node))
app.logger.clear()
ret = app.check_response(node, 'Account', resp, 'PUT', '/v1/a/c',
body='full')
self.assertFalse(ret)
error_lines = app.logger.get_lines_for_level('error')
self.assertEqual(1, len(error_lines))
self.assertEqual('ERROR Insufficient Storage 10.0.0.0:1000/sda',
error_lines[0])
self.assertEqual(11, node_error_count(app, node))
self.assertTrue(app.error_limited(node))
def test_check_response_503(self):
app = proxy_server.Application({},
account_ring=FakeRing(),
container_ring=FakeRing(),
logger=debug_logger())
node = app.container_ring.get_part_nodes(1)[0]
resp = FakeHTTPResponse(Response(status=503))
app.logger.clear()
ret = app.check_response(node, 'Container', resp, 'PUT', '/v1/a/c')
self.assertFalse(ret)
error_lines = app.logger.get_lines_for_level('error')
self.assertEqual(1, len(error_lines))
self.assertEqual('ERROR 503 Trying to PUT /v1/a/c From Container '
'Server 10.0.0.0:1000/sda', error_lines[0])
self.assertEqual(1, node_error_count(app, node))
self.assertFalse(app.error_limited(node))
app.logger.clear()
ret = app.check_response(node, 'Object', resp, 'GET', '/v1/a/c/o',
body='full')
self.assertFalse(ret)
error_lines = app.logger.get_lines_for_level('error')
self.assertEqual(1, len(error_lines))
self.assertEqual('ERROR 503 full Trying to GET /v1/a/c/o From Object '
'Server 10.0.0.0:1000/sda', error_lines[0])
self.assertEqual(2, node_error_count(app, node))
self.assertFalse(app.error_limited(node))
def test_valid_api_version(self):
app = proxy_server.Application({},
account_ring=FakeRing(),