From 1c88d2cb818cec07d12ac17be5820cdd769aea5d Mon Sep 17 00:00:00 2001 From: Samuel Merritt Date: Thu, 11 Feb 2016 15:51:45 -0800 Subject: [PATCH] Fix up get_account_info and get_container_info get_account_info used to work like this: * make an account HEAD request * ignore the response * get the account info by digging around in the request environment, where it had been deposited by elves or something Not actually elves, but the proxy's GETorHEAD_base method would take the HEAD response and cache it in the response environment, which was the same object as the request environment, thus enabling get_account_info to find it. This was extraordinarily brittle. If a WSGI middleware were to shallow-copy the request environment, then any middlewares to its left could not use get_account_info, as the left middleware's request environment would no longer be identical to the response environment down in GETorHEAD_base. Now, get_account_info works like this: * make an account HEAD request. * if the account info is in the request environment, return it. This is an optimization to avoid a double-set in memcached. * else, compute the account info from the response headers, store it in caches, and return it. This is much easier to think about; get_account_info can get and cache account info all on its own; the cache check and cache set are right next to each other. All the above is true for get_container_info as well. get_info() is still around, but it's just a shim. It was trying to unify get_account_info and get_container_info to exploit the commonalities, but the number of times that "if container:" showed up in get_info and its helpers really indicated that something was wrong. I'd rather have two functions with some duplication than one function with no duplication but a bunch of "if container:" branches. Other things of note: * a HEAD request to a deleted account returns 410, but get_account_info would return 404 since the 410 came from the account controller *after* GETorHEAD_base ran. Now get_account_info returns 410 as well. * cache validity period (recheck_account_existence and recheck_container_existence) is now communicated to get_account_info via an X-Backend header. This way, get_account_info doesn't need a reference to the swift.proxy.server.Application object. * both logged swift_source values are now correct for get_container_info calls; before, on a cold cache, get_container_info would call get_account_info but not pass along swift_source, resulting in get_account_info logging "GET_INFO" as the source. Amusingly, there was a unit test asserting this bogus behavior. * callers that modify the return value of get_account_info or of get_container_info don't modify what's stored in swift.infocache. * get_account_info on an account that *can* be autocreated but has not been will return a 200, same as a HEAD request. The old behavior was a 404 from get_account_info but a 200 from HEAD. Callers can tell the difference by looking at info['account_really_exists'] if they need to know the difference (there is one call site that needs to know, in container PUT). Note: this is for all accounts when the proxy's "account_autocreate" setting is on. Change-Id: I5167714025ec7237f7e6dd4759c2c6eb959b3fca --- swift/common/middleware/keystoneauth.py | 3 +- swift/proxy/controllers/account.py | 26 +- swift/proxy/controllers/base.py | 339 ++++++++++++------ swift/proxy/controllers/container.py | 17 +- swift/proxy/server.py | 9 +- test/unit/common/middleware/test_copy.py | 1 + test/unit/common/middleware/test_ratelimit.py | 4 +- test/unit/proxy/controllers/test_account.py | 18 + test/unit/proxy/controllers/test_base.py | 217 +++-------- test/unit/proxy/controllers/test_container.py | 9 +- test/unit/proxy/controllers/test_obj.py | 75 ++-- test/unit/proxy/test_server.py | 77 ++-- 12 files changed, 430 insertions(+), 365 deletions(-) diff --git a/swift/common/middleware/keystoneauth.py b/swift/common/middleware/keystoneauth.py index 651aeacfbb..ccdd2a8ba9 100644 --- a/swift/common/middleware/keystoneauth.py +++ b/swift/common/middleware/keystoneauth.py @@ -287,7 +287,8 @@ class KeystoneAuth(object): def _get_project_domain_id(self, environ): info = get_account_info(environ, self.app, 'KS') domain_id = info.get('sysmeta', {}).get('project-domain-id') - exists = is_success(info.get('status', 0)) + exists = (is_success(info.get('status', 0)) + and info.get('account_really_exists', True)) return exists, domain_id def _set_project_domain_id(self, req, path_parts, env_identity): diff --git a/swift/proxy/controllers/account.py b/swift/proxy/controllers/account.py index faf4ccdee6..2abb3d1f79 100644 --- a/swift/proxy/controllers/account.py +++ b/swift/proxy/controllers/account.py @@ -24,7 +24,8 @@ from swift.common.utils import public from swift.common.constraints import check_metadata from swift.common import constraints from swift.common.http import HTTP_NOT_FOUND, HTTP_GONE -from swift.proxy.controllers.base import Controller, clear_info_cache +from swift.proxy.controllers.base import Controller, clear_info_cache, \ + set_info_cache from swift.common.swob import HTTPBadRequest, HTTPMethodNotAllowed from swift.common.request_helpers import get_sys_meta_prefix @@ -57,6 +58,9 @@ class AccountController(Controller): resp.body = 'Account name length of %d longer than %d' % \ (len(self.account_name), constraints.MAX_ACCOUNT_NAME_LENGTH) + # Don't cache this. We know the account doesn't exist because + # the name is bad; we don't need to cache that because it's + # really cheap to recompute. return resp partition = self.app.account_ring.get_part(self.account_name) @@ -70,8 +74,28 @@ class AccountController(Controller): if resp.headers.get('X-Account-Status', '').lower() == 'deleted': resp.status = HTTP_GONE elif self.app.account_autocreate: + # This is kind of a lie; we pretend like the account is + # there, but it's not. We'll create it as soon as something + # tries to write to it, but we don't need databases on disk + # to tell us that nothing's there. + # + # We set a header so that certain consumers can tell it's a + # fake listing. The important one is the PUT of a container + # to an autocreate account; the proxy checks to see if the + # account exists before actually performing the PUT and + # creates the account if necessary. If we feed it a perfect + # lie, it'll just try to create the container without + # creating the account, and that'll fail. resp = account_listing_response(self.account_name, req, get_listing_content_type(req)) + resp.headers['X-Backend-Fake-Account-Listing'] = 'yes' + + # Cache this. We just made a request to a storage node and got + # up-to-date information for the account. + resp.headers['X-Backend-Recheck-Account-Existence'] = str( + self.app.recheck_account_existence) + set_info_cache(self.app, req.environ, self.account_name, None, resp) + if req.environ.get('swift_owner'): self.add_acls_from_sys_metadata(resp) else: diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index 0cd209db65..527b7ef81d 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -32,6 +32,7 @@ import functools import inspect import itertools import operator +from copy import deepcopy from sys import exc_info from swift import gettext_ as _ @@ -51,7 +52,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_INSUFFICIENT_STORAGE, HTTP_UNAUTHORIZED, HTTP_CONTINUE, HTTP_GONE from swift.common.swob import Request, Response, Range, \ HTTPException, HTTPRequestedRangeNotSatisfiable, HTTPServiceUnavailable, \ status_map @@ -61,6 +62,10 @@ from swift.common.request_helpers import strip_sys_meta_prefix, \ from swift.common.storage_policy import POLICIES +DEFAULT_RECHECK_ACCOUNT_EXISTENCE = 60 # seconds +DEFAULT_RECHECK_CONTAINER_EXISTENCE = 60 # seconds + + def update_headers(response, headers): """ Helper function to update headers in the response. @@ -140,7 +145,7 @@ def headers_to_account_info(headers, status_int=HTTP_OK): Construct a cacheable dict of account info based on response headers. """ headers, meta, sysmeta = _prep_headers_to_info(headers, 'account') - return { + account_info = { 'status': status_int, # 'container_count' anomaly: # Previous code sometimes expects an int sometimes a string @@ -150,8 +155,12 @@ def headers_to_account_info(headers, status_int=HTTP_OK): 'total_object_count': headers.get('x-account-object-count'), 'bytes': headers.get('x-account-bytes-used'), 'meta': meta, - 'sysmeta': sysmeta + 'sysmeta': sysmeta, } + if is_success(status_int): + account_info['account_really_exists'] = not config_true_value( + headers.get('x-backend-fake-account-listing')) + return account_info def headers_to_container_info(headers, status_int=HTTP_OK): @@ -174,7 +183,7 @@ def headers_to_container_info(headers, status_int=HTTP_OK): 'max_age': meta.get('access-control-max-age') }, 'meta': meta, - 'sysmeta': sysmeta + 'sysmeta': sysmeta, } @@ -188,7 +197,7 @@ def headers_to_object_info(headers, status_int=HTTP_OK): 'type': headers.get('content-type'), 'etag': headers.get('etag'), 'meta': meta, - 'sysmeta': sysmeta + 'sysmeta': sysmeta, } return info @@ -280,8 +289,17 @@ def get_object_info(env, app, path=None, swift_source=None): split_path(path or env['PATH_INFO'], 4, 4, True) info = _get_object_info(app, env, account, container, obj, swift_source=swift_source) - if not info: + if info: + info = deepcopy(info) + else: info = headers_to_object_info({}, 0) + + for field in ('length',): + if info.get(field) is None: + info[field] = 0 + else: + info[field] = int(info[field]) + return info @@ -297,11 +315,55 @@ def get_container_info(env, app, swift_source=None): """ (version, account, container, unused) = \ split_path(env['PATH_INFO'], 3, 4, True) - info = get_info(app, env, account, container, ret_not_found=True, - swift_source=swift_source) + + # Check in environment cache and in memcache (in that order) + info = _get_info_from_caches(app, env, account, container) + if not info: + # Cache miss; go HEAD the container and populate the caches + env.setdefault('swift.infocache', {}) + # Before checking the container, make sure the account exists. + # + # If it is an autocreateable account, just assume it exists; don't + # HEAD the account, as a GET or HEAD response for an autocreateable + # account is successful whether the account actually has .db files + # on disk or not. + is_autocreate_account = account.startswith( + getattr(app, 'auto_create_account_prefix', '.')) + if not is_autocreate_account: + account_info = get_account_info(env, app, swift_source) + if not account_info or not is_success(account_info['status']): + return headers_to_container_info({}, 0) + + req = _prepare_pre_auth_info_request( + env, ("/%s/%s/%s" % (version, account, container)), + (swift_source or 'GET_CONTAINER_INFO')) + resp = req.get_response(app) + # Check in infocache to see if the proxy (or anyone else) already + # populated the cache for us. If they did, just use what's there. + # + # See similar comment in get_account_info() for justification. + info = _get_info_from_infocache(env, account, container) + if info is None: + info = set_info_cache(app, env, account, container, resp) + + if info: + info = deepcopy(info) # avoid mutating what's in swift.infocache + else: info = headers_to_container_info({}, 0) + + # Old data format in memcache immediately after a Swift upgrade; clean + # it up so consumers of get_container_info() aren't exposed to it. info.setdefault('storage_policy', '0') + if 'object_count' not in info and 'container_size' in info: + info['object_count'] = info.pop('container_size') + + for field in ('bytes', 'object_count'): + if info.get(field) is None: + info[field] = 0 + else: + info[field] = int(info[field]) + return info @@ -315,18 +377,50 @@ def get_account_info(env, app, swift_source=None): This call bypasses auth. Success does not imply that the request has authorization to the account. - :raises ValueError: when path can't be split(path, 2, 4) + :raises ValueError: when path doesn't contain an account """ (version, account, _junk, _junk) = \ split_path(env['PATH_INFO'], 2, 4, True) - info = get_info(app, env, account, ret_not_found=True, - swift_source=swift_source) + + # Check in environment cache and in memcache (in that order) + info = _get_info_from_caches(app, env, account) + + # Cache miss; go HEAD the account and populate the caches if not info: - info = headers_to_account_info({}, 0) - if info.get('container_count') is None: - info['container_count'] = 0 + env.setdefault('swift.infocache', {}) + req = _prepare_pre_auth_info_request( + env, "/%s/%s" % (version, account), + (swift_source or 'GET_ACCOUNT_INFO')) + resp = req.get_response(app) + # Check in infocache to see if the proxy (or anyone else) already + # populated the cache for us. If they did, just use what's there. + # + # The point of this is to avoid setting the value in memcached + # twice. Otherwise, we're needlessly sending requests across the + # network. + # + # If the info didn't make it into the cache, we'll compute it from + # the response and populate the cache ourselves. + # + # Note that this is taking "exists in infocache" to imply "exists in + # memcache". That's because we're trying to avoid superfluous + # network traffic, and checking in memcache prior to setting in + # memcache would defeat the purpose. + info = _get_info_from_infocache(env, account) + if info is None: + info = set_info_cache(app, env, account, None, resp) + + if info: + info = info.copy() # avoid mutating what's in swift.infocache else: - info['container_count'] = int(info['container_count']) + info = headers_to_account_info({}, 0) + + for field in ('container_count', 'bytes', 'total_object_count'): + if info.get(field) is None: + info[field] = 0 + else: + info[field] = int(info[field]) + return info @@ -335,7 +429,7 @@ def _get_cache_key(account, container): Get the keys for both memcache (cache_key) and env (env_key) where info about accounts and containers is cached - :param account: The name of the account + :param account: The name of the account :param container: The name of the container (or None if account) :returns: a tuple of (cache_key, env_key) """ @@ -356,7 +450,7 @@ def get_object_env_key(account, container, obj): """ Get the keys for env (env_key) where info about object is cached - :param account: The name of the account + :param account: The name of the account :param container: The name of the container :param obj: The name of the object :returns: a string env_key @@ -366,36 +460,36 @@ def get_object_env_key(account, container, obj): return env_key -def _set_info_cache(app, env, account, container, resp): +def set_info_cache(app, env, account, container, resp): """ Cache info in both memcache and env. - Caching is used to avoid unnecessary calls to account & container servers. - This is a private function that is being called by GETorHEAD_base and - by clear_info_cache. - Any attempt to GET or HEAD from the container/account server should use - the GETorHEAD_base interface which would than set the cache. - :param app: the application object :param account: the unquoted account name :param container: the unquoted container name or None - :param resp: the response received or None if info cache should be cleared + :param resp: the response received or None if info cache should be cleared + + :returns: the info that was placed into the cache, or None if the + request status was not in (404, 410, 2xx). """ infocache = env.setdefault('swift.infocache', {}) - if container: - cache_time = app.recheck_container_existence - else: - cache_time = app.recheck_account_existence + cache_time = None + if container and resp: + cache_time = int(resp.headers.get( + 'X-Backend-Recheck-Container-Existence', + DEFAULT_RECHECK_CONTAINER_EXISTENCE)) + elif resp: + cache_time = int(resp.headers.get( + 'X-Backend-Recheck-Account-Existence', + DEFAULT_RECHECK_ACCOUNT_EXISTENCE)) cache_key, env_key = _get_cache_key(account, container) if resp: - if resp.status_int == HTTP_NOT_FOUND: + if resp.status_int in (HTTP_NOT_FOUND, HTTP_GONE): cache_time *= 0.1 elif not is_success(resp.status_int): cache_time = None - else: - cache_time = None # Next actually set both memcache and the env cache memcache = getattr(app, 'memcache', None) or env.get('swift.cache') @@ -412,24 +506,23 @@ def _set_info_cache(app, env, account, container, resp): if memcache: memcache.set(cache_key, info, time=cache_time) infocache[env_key] = info + return info -def _set_object_info_cache(app, env, account, container, obj, resp): +def set_object_info_cache(app, env, account, container, obj, resp): """ - Cache object info env. Do not cache object information in - memcache. This is an intentional omission as it would lead - to cache pressure. This is a per-request cache. - - Caching is used to avoid unnecessary calls to object servers. - This is a private function that is being called by GETorHEAD_base. - Any attempt to GET or HEAD from the object server should use - the GETorHEAD_base interface which would then set the cache. + Cache object info in the WSGI environment, but not in memcache. Caching + in memcache would lead to cache pressure and mass evictions due to the + large number of objects in a typical Swift cluster. This is a + per-request cache only. :param app: the application object :param account: the unquoted account name - :param container: the unquoted container name or None - :param object: the unquoted object name or None - :param resp: the response received or None if info cache should be cleared + :param container: the unquoted container name + :param object: the unquoted object name + :param resp: a GET or HEAD response received from an object server, or + None if info cache should be cleared + :returns: the object info """ env_key = get_object_env_key(account, container, obj) @@ -440,6 +533,7 @@ def _set_object_info_cache(app, env, account, container, obj, resp): info = headers_to_object_info(resp.headers, resp.status_int) env.setdefault('swift.infocache', {})[env_key] = info + return info def clear_info_cache(app, env, account, container=None): @@ -447,26 +541,43 @@ def clear_info_cache(app, env, account, container=None): Clear the cached info in both memcache and env :param app: the application object + :param env: the WSGI environment :param account: the account name :param container: the containr name or None if setting info for containers """ - _set_info_cache(app, env, account, container, None) + set_info_cache(app, env, account, container, None) -def _get_info_cache(app, env, account, container=None): +def _get_info_from_infocache(env, account, container=None): """ - Get the cached info from env or memcache (if used) in that order - Used for both account and container info - A private function used by get_info + Get cached account or container information from request-environment + cache (swift.infocache). + + :param env: the environment used by the current request + :param account: the account name + :param container: the container name + + :returns: a dictionary of cached info on cache hit, None on miss + """ + _junk, env_key = _get_cache_key(account, container) + if 'swift.infocache' in env and env_key in env['swift.infocache']: + return env['swift.infocache'][env_key] + return None + + +def _get_info_from_memcache(app, env, account, container=None): + """ + Get cached account or container information from memcache :param app: the application object :param env: the environment used by the current request - :returns: the cached info or None if not cached - """ + :param account: the account name + :param container: the container name + :returns: a dictionary of cached info on cache hit, None on miss. Also + returns None if memcache is not in use. + """ cache_key, env_key = _get_cache_key(account, container) - if 'swift.infocache' in env and env_key in env['swift.infocache']: - return env['swift.infocache'][env_key] memcache = getattr(app, 'memcache', None) or env.get('swift.cache') if memcache: info = memcache.get(cache_key) @@ -483,6 +594,22 @@ def _get_info_cache(app, env, account, container=None): return None +def _get_info_from_caches(app, env, account, container=None): + """ + Get the cached info from env or memcache (if used) in that order. + Used for both account and container info. + + :param app: the application object + :param env: the environment used by the current request + :returns: the cached info or None if not cached + """ + + info = _get_info_from_infocache(env, account, container) + if info is None: + info = _get_info_from_memcache(app, env, account, container) + return info + + def _prepare_pre_auth_info_request(env, path, swift_source): """ Prepares a pre authed request to obtain info using a HEAD. @@ -499,14 +626,17 @@ def _prepare_pre_auth_info_request(env, path, swift_source): # the request so the it is not treated as a CORS request. newenv.pop('HTTP_ORIGIN', None) + # ACLs are only shown to account owners, so let's make sure this request + # looks like it came from the account owner. + newenv['swift_owner'] = True + # Note that Request.blank expects quoted path return Request.blank(quote(path), environ=newenv) -def get_info(app, env, account, container=None, ret_not_found=False, - swift_source=None): +def get_info(app, env, account, container=None, swift_source=None): """ - Get the info about accounts or containers + Get info about accounts or containers Note: This call bypasses auth. Success does not imply that the request has authorization to the info. @@ -515,42 +645,25 @@ def get_info(app, env, account, container=None, ret_not_found=False, :param env: the environment used by the current request :param account: The unquoted name of the account :param container: The unquoted name of the container (or None if account) - :param ret_not_found: if True, return info dictionary on 404; - if False, return None on 404 :param swift_source: swift source logged for any subrequests made while retrieving the account or container info - :returns: the cached info or None if cannot be retrieved + :returns: information about the specified entity in a dictionary. See + get_account_info and get_container_info for details on what's in the + dictionary. """ - info = _get_info_cache(app, env, account, container) - if info: - if ret_not_found or is_success(info['status']): - return info - return None - # Not in cache, let's try the account servers - path = '/v1/%s' % account - if container: - # Stop and check if we have an account? - if not get_info(app, env, account) and not account.startswith( - getattr(app, 'auto_create_account_prefix', '.')): - return None - path += '/' + container + env.setdefault('swift.infocache', {}) - req = _prepare_pre_auth_info_request( - env, path, (swift_source or 'GET_INFO')) - # Whenever we do a GET/HEAD, the GETorHEAD_base will set the info in the - # environment under environ['swift.infocache'][env_key] and in memcache. - # We will pick the one from environ['swift.infocache'][env_key] and use - # it to set the caller env - resp = req.get_response(app) - cache_key, env_key = _get_cache_key(account, container) - try: - info = resp.environ['swift.infocache'][env_key] - env.setdefault('swift.infocache', {})[env_key] = info - if ret_not_found or is_success(info['status']): - return info - except (KeyError, AttributeError): - pass - return None + if container: + path = '/v1/%s/%s' % (account, container) + path_env = env.copy() + path_env['PATH_INFO'] = path + return get_container_info(path_env, app, swift_source=swift_source) + else: + # account info + path = '/v1/%s' % (account,) + path_env = env.copy() + path_env['PATH_INFO'] = path + return get_account_info(path_env, app, swift_source=swift_source) def _get_object_info(app, env, account, container, obj, swift_source=None): @@ -571,20 +684,18 @@ def _get_object_info(app, env, account, container, obj, swift_source=None): info = env.get('swift.infocache', {}).get(env_key) if info: return info - # Not in cached, let's try the object servers + # Not in cache, let's try the object servers path = '/v1/%s/%s/%s' % (account, container, obj) req = _prepare_pre_auth_info_request(env, path, swift_source) - # Whenever we do a GET/HEAD, the GETorHEAD_base will set the info in - # the environment under environ[env_key]. We will - # pick the one from environ[env_key] and use it to set the caller env resp = req.get_response(app) - try: - info = resp.environ['swift.infocache'][env_key] - env.setdefault('swift.infocache', {})[env_key] = info - return info - except (KeyError, AttributeError): - pass - return None + # Unlike get_account_info() and get_container_info(), we don't save + # things in memcache, so we can store the info without network traffic, + # *and* the proxy doesn't cache object info for us, so there's no chance + # that the object info would be in the environment. Thus, we just + # compute the object info based on the response and stash it in + # swift.infocache. + info = set_object_info_cache(app, env, account, container, obj, resp) + return info def close_swift_conn(src): @@ -1355,8 +1466,14 @@ class Controller(object): env = getattr(req, 'environ', {}) else: env = {} - info = get_info(self.app, env, account) - if not info: + env.setdefault('swift.infocache', {}) + path_env = env.copy() + path_env['PATH_INFO'] = "/v1/%s" % (account,) + + info = get_account_info(path_env, self.app) + if (not info + or not is_success(info['status']) + or not info.get('account_really_exists', True)): return None, None, None if info.get('container_count') is None: container_count = 0 @@ -1383,8 +1500,11 @@ class Controller(object): env = getattr(req, 'environ', {}) else: env = {} - info = get_info(self.app, env, account, container) - if not info: + env.setdefault('swift.infocache', {}) + path_env = env.copy() + path_env['PATH_INFO'] = "/v1/%s/%s" % (account, container) + info = get_container_info(path_env, self.app) + if not info or not is_success(info.get('status')): info = headers_to_container_info({}, 0) info['partition'] = None info['nodes'] = None @@ -1672,17 +1792,7 @@ class Controller(object): req, handler.statuses, handler.reasons, handler.bodies, '%s %s' % (server_type, req.method), headers=handler.source_headers) - try: - (vrs, account, container) = req.split_path(2, 3) - _set_info_cache(self.app, req.environ, account, container, res) - except ValueError: - pass - try: - (vrs, account, container, obj) = req.split_path(4, 4, True) - _set_object_info_cache(self.app, req.environ, account, - container, obj, res) - except ValueError: - pass + # if a backend policy index is present in resp headers, translate it # here with the friendly policy name if 'X-Backend-Storage-Policy-Index' in res.headers and \ @@ -1697,6 +1807,7 @@ class Controller(object): 'Could not translate %s (%r) from %r to policy', 'X-Backend-Storage-Policy-Index', res.headers['X-Backend-Storage-Policy-Index'], path) + return res def is_origin_allowed(self, cors_info, origin): diff --git a/swift/proxy/controllers/container.py b/swift/proxy/controllers/container.py index 08a51f10d6..729f957aa9 100644 --- a/swift/proxy/controllers/container.py +++ b/swift/proxy/controllers/container.py @@ -22,7 +22,7 @@ from swift.common.constraints import check_metadata from swift.common import constraints from swift.common.http import HTTP_ACCEPTED, is_success from swift.proxy.controllers.base import Controller, delay_denial, \ - cors_validation, clear_info_cache + cors_validation, set_info_cache, clear_info_cache from swift.common.storage_policy import POLICIES from swift.common.swob import HTTPBadRequest, HTTPForbidden, \ HTTPNotFound @@ -85,11 +85,16 @@ class ContainerController(Controller): def GETorHEAD(self, req): """Handler for HTTP GET/HEAD requests.""" - if not self.account_info(self.account_name, req)[1]: + ai = self.account_info(self.account_name, req) + if not ai[1]: if 'swift.authorize' in req.environ: aresp = req.environ['swift.authorize'](req) if aresp: + # Don't cache this. It doesn't reflect the state of the + # container, just that the user can't access it. return aresp + # Don't cache this. The lack of account will be cached, and that + # is sufficient. return HTTPNotFound(request=req) part = self.app.container_ring.get_part( self.account_name, self.container_name) @@ -99,10 +104,18 @@ class ContainerController(Controller): resp = self.GETorHEAD_base( req, _('Container'), node_iter, part, req.swift_entity_path, concurrency) + # Cache this. We just made a request to a storage node and got + # up-to-date information for the container. + resp.headers['X-Backend-Recheck-Container-Existence'] = str( + self.app.recheck_container_existence) + set_info_cache(self.app, req.environ, self.account_name, + self.container_name, resp) if 'swift.authorize' in req.environ: req.acl = resp.headers.get('x-container-read') aresp = req.environ['swift.authorize'](req) if aresp: + # Don't cache this. It doesn't reflect the state of the + # container, just that the user can't access it. return aresp if not req.environ.get('swift_owner', False): for key in self.app.swift_owner_headers: diff --git a/swift/proxy/server.py b/swift/proxy/server.py index 963bf34f0e..4993c90735 100644 --- a/swift/proxy/server.py +++ b/swift/proxy/server.py @@ -36,7 +36,8 @@ from swift.common.utils import cache_from_env, get_logger, \ from swift.common.constraints import check_utf8, valid_api_version from swift.proxy.controllers import AccountController, ContainerController, \ ObjectControllerRouter, InfoController -from swift.proxy.controllers.base import get_container_info, NodeIter +from swift.proxy.controllers.base import get_container_info, NodeIter, \ + DEFAULT_RECHECK_CONTAINER_EXISTENCE, DEFAULT_RECHECK_ACCOUNT_EXISTENCE from swift.common.swob import HTTPBadRequest, HTTPForbidden, \ HTTPMethodNotAllowed, HTTPNotFound, HTTPPreconditionFailed, \ HTTPServerError, HTTPException, Request, HTTPServiceUnavailable @@ -106,9 +107,11 @@ class Application(object): self.error_suppression_limit = \ int(conf.get('error_suppression_limit', 10)) self.recheck_container_existence = \ - int(conf.get('recheck_container_existence', 60)) + int(conf.get('recheck_container_existence', + DEFAULT_RECHECK_CONTAINER_EXISTENCE)) self.recheck_account_existence = \ - int(conf.get('recheck_account_existence', 60)) + int(conf.get('recheck_account_existence', + DEFAULT_RECHECK_ACCOUNT_EXISTENCE)) self.allow_account_management = \ config_true_value(conf.get('allow_account_management', 'no')) self.container_ring = container_ring or Ring(swift_dir, diff --git a/test/unit/common/middleware/test_copy.py b/test/unit/common/middleware/test_copy.py index 190d7c9084..254203e630 100644 --- a/test/unit/common/middleware/test_copy.py +++ b/test/unit/common/middleware/test_copy.py @@ -1073,6 +1073,7 @@ class TestServerSideCopyConfiguration(unittest.TestCase): @patch_policies(with_ec_default=True) class TestServerSideCopyMiddlewareWithEC(unittest.TestCase): container_info = { + 'status': 200, 'write_acl': None, 'read_acl': None, 'storage_policy': None, diff --git a/test/unit/common/middleware/test_ratelimit.py b/test/unit/common/middleware/test_ratelimit.py index 44136d2801..66ca43d033 100644 --- a/test/unit/common/middleware/test_ratelimit.py +++ b/test/unit/common/middleware/test_ratelimit.py @@ -432,7 +432,7 @@ class TestRateLimit(unittest.TestCase): req.environ['swift.cache'] = FakeMemcache() req.environ['swift.cache'].set( get_container_memcache_key('a', 'c'), - {'container_size': 1}) + {'object_count': 1}) time_override = [0, 0, 0, 0, None] # simulates 4 requests coming in at same time, then sleeping @@ -466,7 +466,7 @@ class TestRateLimit(unittest.TestCase): req.environ['swift.cache'] = FakeMemcache() req.environ['swift.cache'].set( get_container_memcache_key('a', 'c'), - {'container_size': 1}) + {'object_count': 1}) with mock.patch('swift.common.middleware.ratelimit.get_account_info', lambda *args, **kwargs: {}): diff --git a/test/unit/proxy/controllers/test_account.py b/test/unit/proxy/controllers/test_account.py index d60a017fa5..4ef77f3dab 100644 --- a/test/unit/proxy/controllers/test_account.py +++ b/test/unit/proxy/controllers/test_account.py @@ -25,6 +25,7 @@ from test.unit import fake_http_connect, FakeRing, FakeMemcache from swift.common.storage_policy import StoragePolicy from swift.common.request_helpers import get_sys_meta_prefix import swift.proxy.controllers.base +from swift.proxy.controllers.base import get_account_info from test.unit import patch_policies @@ -378,5 +379,22 @@ class TestAccountController4Replicas(TestAccountController): self._assert_responses('POST', POST_TEST_CASES) +@patch_policies([StoragePolicy(0, 'zero', True, object_ring=FakeRing())]) +class TestGetAccountInfo(unittest.TestCase): + def setUp(self): + self.app = proxy_server.Application( + None, FakeMemcache(), + account_ring=FakeRing(), container_ring=FakeRing()) + + def test_get_deleted_account_410(self): + resp_headers = {'x-account-status': 'deleted'} + + req = Request.blank('/v1/a') + with mock.patch('swift.proxy.controllers.base.http_connect', + fake_http_connect(404, headers=resp_headers)): + info = get_account_info(req.environ, self.app) + self.assertEqual(410, info.get('status')) + + if __name__ == '__main__': unittest.main() diff --git a/test/unit/proxy/controllers/test_base.py b/test/unit/proxy/controllers/test_base.py index 1ad7624386..044a3b5cf0 100644 --- a/test/unit/proxy/controllers/test_base.py +++ b/test/unit/proxy/controllers/test_base.py @@ -21,14 +21,13 @@ from swift.proxy.controllers.base import headers_to_container_info, \ headers_to_account_info, headers_to_object_info, get_container_info, \ get_container_memcache_key, get_account_info, get_account_memcache_key, \ get_object_env_key, get_info, get_object_info, \ - Controller, GetOrHeadHandler, _set_info_cache, _set_object_info_cache, \ - bytes_to_skip + Controller, GetOrHeadHandler, bytes_to_skip from swift.common.swob import Request, HTTPException, RESPONSE_REASONS from swift.common import exceptions from swift.common.utils import split_path from swift.common.header_key_dict import HeaderKeyDict from swift.common.http import is_success -from swift.common.storage_policy import StoragePolicy, POLICIES +from swift.common.storage_policy import StoragePolicy from test.unit import fake_http_connect, FakeRing, FakeMemcache from swift.proxy import server as proxy_server from swift.common.request_helpers import get_sys_meta_prefix @@ -128,15 +127,6 @@ class FakeApp(object): reason = RESPONSE_REASONS[response.status_int][0] start_response('%d %s' % (response.status_int, reason), [(k, v) for k, v in response.headers.items()]) - # It's a bit strange, but the get_info cache stuff relies on the - # app setting some keys in the environment as it makes requests - # (in particular GETorHEAD_base) - so our fake does the same - _set_info_cache(self, environ, response.account, - response.container, response) - if response.obj: - _set_object_info_cache(self, environ, response.account, - response.container, response.obj, - response) return iter(response.body) @@ -158,96 +148,6 @@ class TestFuncs(unittest.TestCase): account_ring=FakeRing(), container_ring=FakeRing()) - def test_GETorHEAD_base(self): - base = Controller(self.app) - req = Request.blank('/v1/a/c/o/with/slashes') - ring = FakeRing() - nodes = list(ring.get_part_nodes(0)) + list(ring.get_more_nodes(0)) - with patch('swift.proxy.controllers.base.' - 'http_connect', fake_http_connect(200)): - resp = base.GETorHEAD_base(req, 'object', iter(nodes), 'part', - '/a/c/o/with/slashes') - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.object/a/c/o/with/slashes' in infocache) - self.assertEqual( - infocache['swift.object/a/c/o/with/slashes']['status'], 200) - - req = Request.blank('/v1/a/c/o') - with patch('swift.proxy.controllers.base.' - 'http_connect', fake_http_connect(200)): - resp = base.GETorHEAD_base(req, 'object', iter(nodes), 'part', - '/a/c/o') - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.object/a/c/o' in infocache) - self.assertEqual(infocache['swift.object/a/c/o']['status'], 200) - - req = Request.blank('/v1/a/c') - with patch('swift.proxy.controllers.base.' - 'http_connect', fake_http_connect(200)): - resp = base.GETorHEAD_base(req, 'container', iter(nodes), 'part', - '/a/c') - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.container/a/c' in infocache) - self.assertEqual(infocache['swift.container/a/c']['status'], 200) - - req = Request.blank('/v1/a') - with patch('swift.proxy.controllers.base.' - 'http_connect', fake_http_connect(200)): - resp = base.GETorHEAD_base(req, 'account', iter(nodes), 'part', - '/a') - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.account/a' in infocache) - self.assertEqual(infocache['swift.account/a']['status'], 200) - - # Run the above tests again, but this time with concurrent_reads - # turned on - policy = next(iter(POLICIES)) - concurrent_get_threads = policy.object_ring.replica_count - for concurrency_timeout in (0, 2): - self.app.concurrency_timeout = concurrency_timeout - req = Request.blank('/v1/a/c/o/with/slashes') - # NOTE: We are using slow_connect of fake_http_connect as using - # a concurrency of 0 when mocking the connection is a little too - # fast for eventlet. Network i/o will make this fine, but mocking - # it seems is too instantaneous. - with patch('swift.proxy.controllers.base.http_connect', - fake_http_connect(200, slow_connect=True)): - resp = base.GETorHEAD_base( - req, 'object', iter(nodes), 'part', '/a/c/o/with/slashes', - concurrency=concurrent_get_threads) - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.object/a/c/o/with/slashes' in infocache) - self.assertEqual( - infocache['swift.object/a/c/o/with/slashes']['status'], 200) - req = Request.blank('/v1/a/c/o') - with patch('swift.proxy.controllers.base.http_connect', - fake_http_connect(200, slow_connect=True)): - resp = base.GETorHEAD_base( - req, 'object', iter(nodes), 'part', '/a/c/o', - concurrency=concurrent_get_threads) - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.object/a/c/o' in infocache) - self.assertEqual(infocache['swift.object/a/c/o']['status'], 200) - req = Request.blank('/v1/a/c') - with patch('swift.proxy.controllers.base.http_connect', - fake_http_connect(200, slow_connect=True)): - resp = base.GETorHEAD_base( - req, 'container', iter(nodes), 'part', '/a/c', - concurrency=concurrent_get_threads) - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.container/a/c' in infocache) - self.assertEqual(infocache['swift.container/a/c']['status'], 200) - - req = Request.blank('/v1/a') - with patch('swift.proxy.controllers.base.http_connect', - fake_http_connect(200, slow_connect=True)): - resp = base.GETorHEAD_base( - req, 'account', iter(nodes), 'part', '/a', - concurrency=concurrent_get_threads) - infocache = resp.environ['swift.infocache'] - self.assertTrue('swift.account/a' in infocache) - self.assertEqual(infocache['swift.account/a']['status'], 200) - def test_get_info(self): app = FakeApp() # Do a non cached call to account @@ -257,38 +157,44 @@ class TestFuncs(unittest.TestCase): self.assertEqual(info_a['status'], 200) self.assertEqual(info_a['bytes'], 6666) self.assertEqual(info_a['total_object_count'], 1000) - # Make sure the env cache is set - self.assertEqual(env['swift.infocache'].get('swift.account/a'), info_a) + # Make sure the app was called self.assertEqual(app.responses.stats['account'], 1) + # Make sure the return value matches get_account_info + account_info = get_account_info({'PATH_INFO': '/v1/a'}, app) + self.assertEqual(info_a, account_info) + # Do an env cached call to account + app.responses.stats['account'] = 0 + app.responses.stats['container'] = 0 + info_a = get_info(app, env, 'a') # Check that you got proper info self.assertEqual(info_a['status'], 200) self.assertEqual(info_a['bytes'], 6666) self.assertEqual(info_a['total_object_count'], 1000) - # Make sure the env cache is set - self.assertEqual(env['swift.infocache'].get('swift.account/a'), info_a) + # Make sure the app was NOT called AGAIN - self.assertEqual(app.responses.stats['account'], 1) + self.assertEqual(app.responses.stats['account'], 0) # This time do env cached call to account and non cached to container + app.responses.stats['account'] = 0 + app.responses.stats['container'] = 0 + info_c = get_info(app, env, 'a', 'c') # Check that you got proper info self.assertEqual(info_c['status'], 200) self.assertEqual(info_c['bytes'], 6666) self.assertEqual(info_c['object_count'], 1000) - # Make sure the env cache is set - self.assertEqual( - env['swift.infocache'].get('swift.account/a'), info_a) - self.assertEqual( - env['swift.infocache'].get('swift.container/a/c'), info_c) - # Make sure the app was called for container + # Make sure the app was called for container but not account + self.assertEqual(app.responses.stats['account'], 0) self.assertEqual(app.responses.stats['container'], 1) - # This time do a non cached call to account than non cached to + # This time do a non-cached call to account then non-cached to # container + app.responses.stats['account'] = 0 + app.responses.stats['container'] = 0 app = FakeApp() env = {} # abandon previous call to env info_c = get_info(app, env, 'a', 'c') @@ -296,82 +202,31 @@ class TestFuncs(unittest.TestCase): self.assertEqual(info_c['status'], 200) self.assertEqual(info_c['bytes'], 6666) self.assertEqual(info_c['object_count'], 1000) - # Make sure the env cache is set - self.assertEqual( - env['swift.infocache'].get('swift.account/a'), info_a) - self.assertEqual( - env['swift.infocache'].get('swift.container/a/c'), info_c) # check app calls both account and container self.assertEqual(app.responses.stats['account'], 1) self.assertEqual(app.responses.stats['container'], 1) - # This time do an env cached call to container while account is not + # This time do an env-cached call to container while account is not # cached + app.responses.stats['account'] = 0 + app.responses.stats['container'] = 0 del(env['swift.infocache']['swift.account/a']) info_c = get_info(app, env, 'a', 'c') # Check that you got proper info self.assertEqual(info_a['status'], 200) self.assertEqual(info_c['bytes'], 6666) self.assertEqual(info_c['object_count'], 1000) - # Make sure the env cache is set and account still not cached - self.assertEqual( - env['swift.infocache'].get('swift.container/a/c'), info_c) + # no additional calls were made - self.assertEqual(app.responses.stats['account'], 1) - self.assertEqual(app.responses.stats['container'], 1) - - # Do a non cached call to account not found with ret_not_found - app = FakeApp(statuses=(404,)) - env = {} - info_a = get_info(app, env, 'a', ret_not_found=True) - # Check that you got proper info - self.assertEqual(info_a['status'], 404) - self.assertEqual(info_a['bytes'], None) - self.assertEqual(info_a['total_object_count'], None) - # Make sure the env cache is set - self.assertEqual( - env['swift.infocache'].get('swift.account/a'), info_a) - # and account was called - self.assertEqual(app.responses.stats['account'], 1) - - # Do a cached call to account not found with ret_not_found - info_a = get_info(app, env, 'a', ret_not_found=True) - # Check that you got proper info - self.assertEqual(info_a['status'], 404) - self.assertEqual(info_a['bytes'], None) - self.assertEqual(info_a['total_object_count'], None) - # Make sure the env cache is set - self.assertEqual( - env['swift.infocache'].get('swift.account/a'), info_a) - # add account was NOT called AGAIN - self.assertEqual(app.responses.stats['account'], 1) - - # Do a non cached call to account not found without ret_not_found - app = FakeApp(statuses=(404,)) - env = {} - info_a = get_info(app, env, 'a') - # Check that you got proper info - self.assertEqual(info_a, None) - self.assertEqual( - env['swift.infocache']['swift.account/a']['status'], 404) - # and account was called - self.assertEqual(app.responses.stats['account'], 1) - - # Do a cached call to account not found without ret_not_found - info_a = get_info(None, env, 'a') - # Check that you got proper info - self.assertEqual(info_a, None) - self.assertEqual( - env['swift.infocache']['swift.account/a']['status'], 404) - # add account was NOT called AGAIN - self.assertEqual(app.responses.stats['account'], 1) + self.assertEqual(app.responses.stats['account'], 0) + self.assertEqual(app.responses.stats['container'], 0) def test_get_container_info_swift_source(self): app = FakeApp() req = Request.blank("/v1/a/c", environ={'swift.cache': FakeCache()}) get_container_info(req.environ, app, swift_source='MC') self.assertEqual([e['swift.source'] for e in app.captured_envs], - ['GET_INFO', 'MC']) + ['MC', 'MC']) def test_get_object_info_swift_source(self): app = FakeApp() @@ -397,7 +252,7 @@ class TestFuncs(unittest.TestCase): self.assertEqual(info['status'], 0) def test_get_container_info_no_auto_account(self): - responses = DynamicResponseFactory(404, 200) + responses = DynamicResponseFactory(200) app = FakeApp(responses) req = Request.blank("/v1/.system_account/cont") info = get_container_info(req.environ, app) @@ -435,6 +290,13 @@ class TestFuncs(unittest.TestCase): self.assertEqual([e['swift.source'] for e in app.captured_envs], ['MC']) + def test_get_account_info_swift_owner(self): + app = FakeApp() + req = Request.blank("/v1/a", environ={'swift.cache': FakeCache()}) + get_account_info(req.environ, app) + self.assertEqual([e['swift_owner'] for e in app.captured_envs], + [True]) + def test_get_account_info_infocache(self): app = FakeApp() ic = {} @@ -454,7 +316,7 @@ class TestFuncs(unittest.TestCase): self.assertEqual(resp['total_object_count'], 1000) def test_get_account_info_cache(self): - # The original test that we prefer to preserve + # Works with fake apps that return ints in the headers cached = {'status': 404, 'bytes': 3333, 'total_object_count': 10} @@ -465,7 +327,8 @@ class TestFuncs(unittest.TestCase): self.assertEqual(resp['total_object_count'], 10) self.assertEqual(resp['status'], 404) - # Here is a more realistic test + # Works with strings too, like you get when parsing HTTP headers + # that came in through a socket from the account server cached = {'status': 404, 'bytes': '3333', 'container_count': '234', @@ -475,10 +338,10 @@ class TestFuncs(unittest.TestCase): environ={'swift.cache': FakeCache(cached)}) resp = get_account_info(req.environ, FakeApp()) self.assertEqual(resp['status'], 404) - self.assertEqual(resp['bytes'], '3333') + self.assertEqual(resp['bytes'], 3333) self.assertEqual(resp['container_count'], 234) self.assertEqual(resp['meta'], {}) - self.assertEqual(resp['total_object_count'], '10') + self.assertEqual(resp['total_object_count'], 10) def test_get_account_info_env(self): cache_key = get_account_memcache_key("account") diff --git a/test/unit/proxy/controllers/test_container.py b/test/unit/proxy/controllers/test_container.py index 851ecc81d9..fc5692d8c5 100644 --- a/test/unit/proxy/controllers/test_container.py +++ b/test/unit/proxy/controllers/test_container.py @@ -58,7 +58,7 @@ class TestContainerController(TestRingBase): proxy_server.ContainerController): def account_info(controller, *args, **kwargs): - patch_path = 'swift.proxy.controllers.base.get_info' + patch_path = 'swift.proxy.controllers.base.get_account_info' with mock.patch(patch_path) as mock_get_info: mock_get_info.return_value = dict(self.account_info) return super(FakeAccountInfoContainerController, @@ -95,18 +95,21 @@ class TestContainerController(TestRingBase): 'Expected %s but got %s. Failed case: %s' % (expected, resp.status_int, str(responses))) - def test_container_info_in_response_env(self): + def test_container_info_got_cached(self): controller = proxy_server.ContainerController(self.app, 'a', 'c') with mock.patch('swift.proxy.controllers.base.http_connect', fake_http_connect(200, 200, body='')): req = Request.blank('/v1/a/c', {'PATH_INFO': '/v1/a/c'}) resp = controller.HEAD(req) self.assertEqual(2, resp.status_int // 100) + # Make sure it's in both swift.infocache and memcache self.assertTrue( - "swift.container/a/c" in resp.environ['swift.infocache']) + "swift.container/a/c" in req.environ['swift.infocache']) self.assertEqual( headers_to_container_info(resp.headers), resp.environ['swift.infocache']['swift.container/a/c']) + from_memcache = self.app.memcache.get('container/a/c') + self.assertTrue(from_memcache) def test_swift_owner(self): owner_headers = { diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py index 95b92b298a..a34fee34a4 100755 --- a/test/unit/proxy/controllers/test_obj.py +++ b/test/unit/proxy/controllers/test_obj.py @@ -34,7 +34,8 @@ from swift.common import utils, swob, exceptions from swift.common.header_key_dict import HeaderKeyDict from swift.proxy import server as proxy_server from swift.proxy.controllers import obj -from swift.proxy.controllers.base import get_info as _real_get_info +from swift.proxy.controllers.base import \ + get_container_info as _real_get_container_info from swift.common.storage_policy import POLICIES, ECDriverError, StoragePolicy from test.unit import FakeRing, FakeMemcache, fake_http_connect, \ @@ -76,7 +77,7 @@ def set_http_connect(*args, **kwargs): class PatchedObjControllerApp(proxy_server.Application): """ This patch is just a hook over the proxy server's __call__ to ensure - that calls to get_info will return the stubbed value for + that calls to get_container_info will return the stubbed value for container_info if it's a container info call. """ @@ -85,22 +86,45 @@ class PatchedObjControllerApp(proxy_server.Application): def __call__(self, *args, **kwargs): - def _fake_get_info(app, env, account, container=None, **kwargs): - if container: - if container in self.per_container_info: - return self.per_container_info[container] - return self.container_info - else: - return _real_get_info(app, env, account, container, **kwargs) + def _fake_get_container_info(env, app, swift_source=None): + _vrs, account, container, _junk = utils.split_path( + env['PATH_INFO'], 3, 4) - mock_path = 'swift.proxy.controllers.base.get_info' - with mock.patch(mock_path, new=_fake_get_info): + # Seed the cache with our container info so that the real + # get_container_info finds it. + ic = env.setdefault('swift.infocache', {}) + cache_key = "swift.container/%s/%s" % (account, container) + + old_value = ic.get(cache_key) + + # Copy the container info so we don't hand out a reference to a + # mutable thing that's set up only once at compile time. Nothing + # *should* mutate it, but it's better to be paranoid than wrong. + if container in self.per_container_info: + ic[cache_key] = self.per_container_info[container].copy() + else: + ic[cache_key] = self.container_info.copy() + + real_info = _real_get_container_info(env, app, swift_source) + + if old_value is None: + del ic[cache_key] + else: + ic[cache_key] = old_value + + return real_info + + with mock.patch('swift.proxy.server.get_container_info', + new=_fake_get_container_info), \ + mock.patch('swift.proxy.controllers.base.get_container_info', + new=_fake_get_container_info): return super( PatchedObjControllerApp, self).__call__(*args, **kwargs) class BaseObjectControllerMixin(object): container_info = { + 'status': 200, 'write_acl': None, 'read_acl': None, 'storage_policy': None, @@ -121,8 +145,11 @@ class BaseObjectControllerMixin(object): self.app = PatchedObjControllerApp( None, FakeMemcache(), account_ring=FakeRing(), container_ring=FakeRing(), logger=self.logger) + # you can over-ride the container_info just by setting it on the app + # (see PatchedObjControllerApp for details) self.app.container_info = dict(self.container_info) + # default policy and ring references self.policy = POLICIES.default self.obj_ring = self.policy.object_ring @@ -957,31 +984,6 @@ class TestReplicatedObjControllerVariousReplicas(BaseObjectControllerMixin, controller_cls = obj.ReplicatedObjectController -@patch_policies(legacy_only=True) -class TestObjControllerLegacyCache(TestReplicatedObjController): - """ - This test pretends like memcache returned a stored value that should - resemble whatever "old" format. It catches KeyErrors you'd get if your - code was expecting some new format during a rolling upgrade. - """ - - # in this case policy_index is missing - container_info = { - 'read_acl': None, - 'write_acl': None, - 'sync_key': None, - 'versions': None, - } - - def test_invalid_storage_policy_cache(self): - self.app.container_info['storage_policy'] = 1 - for method in ('GET', 'HEAD', 'POST', 'PUT', 'COPY'): - req = swob.Request.blank('/v1/a/c/o', method=method) - with set_http_connect(): - resp = req.get_response(self.app) - self.assertEqual(resp.status_int, 503) - - class StubResponse(object): def __init__(self, status, body='', headers=None): @@ -1055,6 +1057,7 @@ def capture_http_requests(get_response): @patch_policies(with_ec_default=True) class TestECObjController(BaseObjectControllerMixin, unittest.TestCase): container_info = { + 'status': 200, 'read_acl': None, 'write_acl': None, 'sync_key': None, diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index f2e4986650..a2757f472e 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -74,7 +74,8 @@ from swift.common.utils import mkdirs, normalize_timestamp, NullLogger from swift.common.wsgi import monkey_patch_mimetools, loadapp from swift.proxy.controllers import base as proxy_base from swift.proxy.controllers.base import get_container_memcache_key, \ - get_account_memcache_key, cors_validation, _get_info_cache + get_account_memcache_key, cors_validation, get_account_info, \ + get_container_info import swift.proxy.controllers import swift.proxy.controllers.obj from swift.common.header_key_dict import HeaderKeyDict @@ -518,6 +519,7 @@ class TestController(unittest.TestCase): # 'container_count' changed from int to str cache_key = get_account_memcache_key(self.account) container_info = {'status': 200, + 'account_really_exists': True, 'container_count': '12345', 'total_object_count': None, 'bytes': None, @@ -686,7 +688,32 @@ class TestController(unittest.TestCase): test(404, 507, 503) test(503, 503, 503) - def test_get_info_cache_returns_values_as_strings(self): + def test_get_account_info_returns_values_as_strings(self): + app = mock.MagicMock() + app.memcache = mock.MagicMock() + app.memcache.get = mock.MagicMock() + app.memcache.get.return_value = { + u'foo': u'\u2603', + u'meta': {u'bar': u'\u2603'}, + u'sysmeta': {u'baz': u'\u2603'}} + env = {'PATH_INFO': '/v1/a'} + ai = get_account_info(env, app) + + # Test info is returned as strings + self.assertEqual(ai.get('foo'), '\xe2\x98\x83') + self.assertTrue(isinstance(ai.get('foo'), str)) + + # Test info['meta'] is returned as strings + m = ai.get('meta', {}) + self.assertEqual(m.get('bar'), '\xe2\x98\x83') + self.assertTrue(isinstance(m.get('bar'), str)) + + # Test info['sysmeta'] is returned as strings + m = ai.get('sysmeta', {}) + self.assertEqual(m.get('baz'), '\xe2\x98\x83') + self.assertTrue(isinstance(m.get('baz'), str)) + + def test_get_container_info_returns_values_as_strings(self): app = mock.MagicMock() app.memcache = mock.MagicMock() app.memcache.get = mock.MagicMock() @@ -695,25 +722,25 @@ class TestController(unittest.TestCase): u'meta': {u'bar': u'\u2603'}, u'sysmeta': {u'baz': u'\u2603'}, u'cors': {u'expose_headers': u'\u2603'}} - env = {} - r = _get_info_cache(app, env, 'account', 'container') + env = {'PATH_INFO': '/v1/a/c'} + ci = get_container_info(env, app) # Test info is returned as strings - self.assertEqual(r.get('foo'), '\xe2\x98\x83') - self.assertTrue(isinstance(r.get('foo'), str)) + self.assertEqual(ci.get('foo'), '\xe2\x98\x83') + self.assertTrue(isinstance(ci.get('foo'), str)) # Test info['meta'] is returned as strings - m = r.get('meta', {}) + m = ci.get('meta', {}) self.assertEqual(m.get('bar'), '\xe2\x98\x83') self.assertTrue(isinstance(m.get('bar'), str)) # Test info['sysmeta'] is returned as strings - m = r.get('sysmeta', {}) + m = ci.get('sysmeta', {}) self.assertEqual(m.get('baz'), '\xe2\x98\x83') self.assertTrue(isinstance(m.get('baz'), str)) # Test info['cors'] is returned as strings - m = r.get('cors', {}) + m = ci.get('cors', {}) self.assertEqual(m.get('expose_headers'), '\xe2\x98\x83') self.assertTrue(isinstance(m.get('expose_headers'), str)) @@ -6362,8 +6389,8 @@ class TestContainerController(unittest.TestCase): else: self.assertNotIn('swift.account/a', infocache) # In all the following tests cache 200 for account - # return and ache vary for container - # return 200 and cache 200 for and container + # return and cache vary for container + # return 200 and cache 200 for account and container test_status_map((200, 200, 404, 404), 200, 200, 200) test_status_map((200, 200, 500, 404), 200, 200, 200) # return 304 don't cache container @@ -6375,12 +6402,13 @@ class TestContainerController(unittest.TestCase): test_status_map((200, 500, 500, 500), 503, None, 200) self.assertFalse(self.app.account_autocreate) - # In all the following tests cache 404 for account # return 404 (as account is not found) and don't cache container test_status_map((404, 404, 404), 404, None, 404) - # This should make no difference + + # cache a 204 for the account because it's sort of like it + # exists self.app.account_autocreate = True - test_status_map((404, 404, 404), 404, None, 404) + test_status_map((404, 404, 404), 404, None, 204) def test_PUT_policy_headers(self): backend_requests = [] @@ -6966,8 +6994,7 @@ class TestContainerController(unittest.TestCase): def test_GET_no_content(self): with save_globals(): set_http_connect(200, 204, 204, 204) - controller = proxy_server.ContainerController(self.app, 'account', - 'container') + controller = proxy_server.ContainerController(self.app, 'a', 'c') req = Request.blank('/v1/a/c') self.app.update_request(req) res = controller.GET(req) @@ -6985,8 +7012,7 @@ class TestContainerController(unittest.TestCase): return HTTPUnauthorized(request=req) with save_globals(): set_http_connect(200, 201, 201, 201) - controller = proxy_server.ContainerController(self.app, 'account', - 'container') + controller = proxy_server.ContainerController(self.app, 'a', 'c') req = Request.blank('/v1/a/c') req.environ['swift.authorize'] = authorize self.app.update_request(req) @@ -7004,8 +7030,7 @@ class TestContainerController(unittest.TestCase): return HTTPUnauthorized(request=req) with save_globals(): set_http_connect(200, 201, 201, 201) - controller = proxy_server.ContainerController(self.app, 'account', - 'container') + controller = proxy_server.ContainerController(self.app, 'a', 'c') req = Request.blank('/v1/a/c', {'REQUEST_METHOD': 'HEAD'}) req.environ['swift.authorize'] = authorize self.app.update_request(req) @@ -7517,7 +7542,7 @@ class TestAccountController(unittest.TestCase): def test_GET(self): with save_globals(): - controller = proxy_server.AccountController(self.app, 'account') + controller = proxy_server.AccountController(self.app, 'a') # GET returns after the first successful call to an Account Server self.assert_status_map(controller.GET, (200,), 200, 200) self.assert_status_map(controller.GET, (503, 200), 200, 200) @@ -7539,7 +7564,7 @@ class TestAccountController(unittest.TestCase): def test_GET_autocreate(self): with save_globals(): - controller = proxy_server.AccountController(self.app, 'account') + controller = proxy_server.AccountController(self.app, 'a') self.app.memcache = FakeMemcacheReturnsNone() self.assertFalse(self.app.account_autocreate) # Repeat the test for autocreate = False and 404 by all @@ -7564,7 +7589,7 @@ class TestAccountController(unittest.TestCase): def test_HEAD(self): # Same behaviour as GET with save_globals(): - controller = proxy_server.AccountController(self.app, 'account') + controller = proxy_server.AccountController(self.app, 'a') self.assert_status_map(controller.HEAD, (200,), 200, 200) self.assert_status_map(controller.HEAD, (503, 200), 200, 200) self.assert_status_map(controller.HEAD, (503, 503, 200), 200, 200) @@ -7582,7 +7607,7 @@ class TestAccountController(unittest.TestCase): def test_HEAD_autocreate(self): # Same behaviour as GET with save_globals(): - controller = proxy_server.AccountController(self.app, 'account') + controller = proxy_server.AccountController(self.app, 'a') self.app.memcache = FakeMemcacheReturnsNone() self.assertFalse(self.app.account_autocreate) self.assert_status_map(controller.HEAD, @@ -7598,7 +7623,7 @@ class TestAccountController(unittest.TestCase): def test_POST_autocreate(self): with save_globals(): - controller = proxy_server.AccountController(self.app, 'account') + controller = proxy_server.AccountController(self.app, 'a') self.app.memcache = FakeMemcacheReturnsNone() # first test with autocreate being False self.assertFalse(self.app.account_autocreate) @@ -7620,7 +7645,7 @@ class TestAccountController(unittest.TestCase): def test_POST_autocreate_with_sysmeta(self): with save_globals(): - controller = proxy_server.AccountController(self.app, 'account') + controller = proxy_server.AccountController(self.app, 'a') self.app.memcache = FakeMemcacheReturnsNone() # first test with autocreate being False self.assertFalse(self.app.account_autocreate)