From 5cef798f8dcdee0d0512e47b67ac67d5f8d6c14c Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Fri, 21 Jun 2013 16:41:50 -0400 Subject: OpenStack Swift Functional Tests for G4S This commit has the following changes: * G4S no longer accepts URLs that end in /. A HTTP code of 400 is returned when a / at the end of the object is detected. * Directories can be created as objects setting the content-type to application/directory and content-length to 0. * Functional tests have been adjusted to work with G4S constraints Change-Id: I31038a59699a8e3eeaba902db322218c6400093e Signed-off-by: Luis Pabon Reviewed-on: http://review.gluster.org/5246 Reviewed-by: Peter Portante Tested-by: Peter Portante --- gluster/swift/common/DiskDir.py | 36 ++++++++------- gluster/swift/common/DiskFile.py | 88 +++++++++++++++++++++++++------------ gluster/swift/common/constraints.py | 5 ++- gluster/swift/common/utils.py | 68 +++++++++++++++++++++++++--- 4 files changed, 145 insertions(+), 52 deletions(-) (limited to 'gluster/swift') diff --git a/gluster/swift/common/DiskDir.py b/gluster/swift/common/DiskDir.py index 364c95e..c1fb674 100644 --- a/gluster/swift/common/DiskDir.py +++ b/gluster/swift/common/DiskDir.py @@ -16,15 +16,14 @@ import os import errno -from gluster.swift.common.fs_utils import dir_empty, rmdirs, mkdirs, os_path, \ - do_chown +from gluster.swift.common.fs_utils import dir_empty, mkdirs, os_path, do_chown from gluster.swift.common.utils import validate_account, validate_container, \ get_container_details, get_account_details, create_container_metadata, \ create_account_metadata, DEFAULT_GID, get_container_metadata, \ get_account_metadata, DEFAULT_UID, validate_object, \ create_object_metadata, read_metadata, write_metadata, X_CONTENT_TYPE, \ X_CONTENT_LENGTH, X_TIMESTAMP, X_PUT_TIMESTAMP, X_ETAG, X_OBJECTS_COUNT, \ - X_BYTES_USED, X_CONTAINER_COUNT, DIR_TYPE + X_BYTES_USED, X_CONTAINER_COUNT, DIR_TYPE, rmobjdir, dir_is_object from gluster.swift.common import Glusterfs from gluster.swift.common.exceptions import FileOrDirNotFoundError @@ -333,14 +332,13 @@ class DiskDir(DiskCommon): else: return container_list - if objects and end_marker: + if end_marker: objects = filter_end_marker(objects, end_marker) - if objects: - if marker and marker >= prefix: - objects = filter_marker(objects, marker) - elif prefix: - objects = filter_prefix_as_marker(objects, prefix) + if marker and marker >= prefix: + objects = filter_marker(objects, marker) + elif prefix: + objects = filter_prefix_as_marker(objects, prefix) if prefix is None: # No prefix, we don't need to apply the other arguments, we just @@ -376,7 +374,8 @@ class DiskDir(DiskCommon): if e.errno != errno.ENOENT: raise if Glusterfs.OBJECT_ONLY and metadata \ - and metadata[X_CONTENT_TYPE] == DIR_TYPE: + and metadata[X_CONTENT_TYPE] == DIR_TYPE \ + and not dir_is_object(metadata): continue list_item = [] list_item.append(obj) @@ -484,19 +483,22 @@ class DiskDir(DiskCommon): # within a directory implicitly. return + def empty(self): + try: + return dir_empty(self.datadir) + except FileOrDirNotFoundError: + return True + def delete_db(self, timestamp): """ Delete the container (directory) if empty. :param timestamp: delete timestamp """ - try: - if not dir_empty(self.datadir): - # FIXME: This is a failure condition here, isn't it? - return - except FileOrDirNotFoundError: - return - rmdirs(self.datadir) + # Let's check and see if it has directories that + # where created by the code, but not by the + # caller as objects + rmobjdir(self.datadir) def set_x_container_sync_points(self, sync_point1, sync_point2): self.metadata['x_container_sync_point1'] = sync_point1 diff --git a/gluster/swift/common/DiskFile.py b/gluster/swift/common/DiskFile.py index c7138d4..0bc2778 100644 --- a/gluster/swift/common/DiskFile.py +++ b/gluster/swift/common/DiskFile.py @@ -19,17 +19,16 @@ import random from hashlib import md5 from contextlib import contextmanager from swift.common.utils import renamer -from swift.common.exceptions import DiskFileNotExist +from swift.common.exceptions import DiskFileNotExist, DiskFileError from gluster.swift.common.exceptions import AlreadyExistsAsDir -from gluster.swift.common.fs_utils import mkdirs, rmdirs, do_open, do_close, \ +from gluster.swift.common.fs_utils import mkdirs, do_open, do_close, \ do_unlink, do_chown, os_path, do_fsync, do_fchown from gluster.swift.common.utils import read_metadata, write_metadata, \ - validate_object, create_object_metadata + validate_object, create_object_metadata, rmobjdir, dir_is_object from gluster.swift.common.utils import X_CONTENT_LENGTH, X_CONTENT_TYPE, \ - X_TIMESTAMP, X_TYPE, X_OBJECT_TYPE, FILE, MARKER_DIR, OBJECT, DIR_TYPE, \ - FILE_TYPE, DEFAULT_UID, DEFAULT_GID + X_TIMESTAMP, X_TYPE, X_OBJECT_TYPE, FILE, OBJECT, DIR_TYPE, \ + FILE_TYPE, DEFAULT_UID, DEFAULT_GID, DIR_NON_OBJECT, DIR_OBJECT -import logging from swift.obj.server import DiskFile @@ -50,12 +49,14 @@ def _adjust_metadata(metadata): # FIXME: If the file exists, we would already know it is a # directory. So why are we assuming it is a file object? metadata[X_CONTENT_TYPE] = FILE_TYPE - x_object_type = FILE + metadata[X_OBJECT_TYPE] = FILE else: - x_object_type = MARKER_DIR if content_type.lower() == DIR_TYPE \ - else FILE + if content_type.lower() == DIR_TYPE: + metadata[X_OBJECT_TYPE] = DIR_OBJECT + else: + metadata[X_OBJECT_TYPE] = FILE + metadata[X_TYPE] = OBJECT - metadata[X_OBJECT_TYPE] = x_object_type return metadata @@ -63,6 +64,12 @@ class Gluster_DiskFile(DiskFile): """ Manage object files on disk. + Object names ending or beginning with a '/' as in /a, a/, /a/b/, + etc, or object names with multiple consecutive slahes, like a//b, + are not supported. The proxy server's contraints filter + gluster.common.constrains.gluster_check_object_creation() should + reject such requests. + :param path: path to devices on the node/mount path for UFO. :param device: device name/account_name for UFO. :param partition: partition on the device the object lives in @@ -82,9 +89,8 @@ class Gluster_DiskFile(DiskFile): uid=DEFAULT_UID, gid=DEFAULT_GID, iter_hook=None): self.disk_chunk_size = disk_chunk_size self.iter_hook = iter_hook - # Don't support obj_name ending/begining with '/', like /a, a/, /a/b/, - # etc. obj = obj.strip(os.path.sep) + if os.path.sep in obj: self._obj_path, self._obj = os.path.split(obj) else: @@ -167,14 +173,13 @@ class Gluster_DiskFile(DiskFile): return not self.data_file def _create_dir_object(self, dir_path): - #TODO: if object already exists??? - if os_path.exists(dir_path) and not os_path.isdir(dir_path): - self.logger.error("Deleting file %s", dir_path) - do_unlink(dir_path) - #If dir aleady exist just override metadata. - mkdirs(dir_path) - do_chown(dir_path, self.uid, self.gid) - create_object_metadata(dir_path) + if not os_path.exists(dir_path): + mkdirs(dir_path) + do_chown(dir_path, self.uid, self.gid) + create_object_metadata(dir_path) + elif not os_path.isdir(dir_path): + raise DiskFileError("Cannot overwrite " + "file %s with a directory" % dir_path) def put_metadata(self, metadata, tombstone=False): """ @@ -208,7 +213,7 @@ class Gluster_DiskFile(DiskFile): metadata = _adjust_metadata(metadata) - if metadata[X_OBJECT_TYPE] == MARKER_DIR: + if dir_is_object(metadata): if not self.data_file: self.data_file = os.path.join(self.datadir, self._obj) self._create_dir_object(self.data_file) @@ -217,8 +222,10 @@ class Gluster_DiskFile(DiskFile): # Check if directory already exists. if self._is_dir: - # FIXME: How can we have a directory and it not be marked as a - # MARKER_DIR (see above)? + # A pre-existing directory already exists on the file + # system, perhaps gratuitously created when another + # object was created, or created externally to Swift + # REST API servicing (UFO use case). msg = 'File object exists as a directory: %s' % self.data_file raise AlreadyExistsAsDir(msg) @@ -256,15 +263,38 @@ class Gluster_DiskFile(DiskFile): "Have metadata, %r, but no data_file" % self.metadata if self._is_dir: - # Marker directory object - if not rmdirs(self.data_file): - logging.error('Unable to delete dir object: %s', - self.data_file) - return + # Marker, or object, directory. + # + # Delete from the filesystem only if it contains + # no objects. If it does contain objects, then just + # remove the object metadata tag which will make this directory a + # fake-filesystem-only directory and will be deleted + # when the container or parent directory is deleted. + metadata = read_metadata(self.data_file) + if dir_is_object(metadata): + metadata[X_OBJECT_TYPE] = DIR_NON_OBJECT + write_metadata(self.data_file, metadata) + rmobjdir(self.data_file) + else: - # File object + # Delete file object do_unlink(self.data_file) + # Garbage collection of non-object directories. + # Now that we deleted the file, determine + # if the current directory and any parent + # directory may be deleted. + dirname = os.path.dirname(self.data_file) + while dirname and dirname != self._container_path: + # Try to remove any directories that are not + # objects. + if not rmobjdir(dirname): + # If a directory with objects has been + # found, we can stop garabe collection + break + else: + dirname = os.path.dirname(dirname) + self.metadata = {} self.data_file = None diff --git a/gluster/swift/common/constraints.py b/gluster/swift/common/constraints.py index acdd3f5..ce1df31 100644 --- a/gluster/swift/common/constraints.py +++ b/gluster/swift/common/constraints.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os try: from webob.exc import HTTPBadRequest except ImportError: @@ -45,6 +46,8 @@ def get_object_name_component_length(): def validate_obj_name_component(obj): + if not obj: + return 'cannot begin, end, or have contiguous %s\'s' % os.path.sep if len(obj) > MAX_OBJECT_NAME_COMPONENT_LENGTH: return 'too long (%d)' % len(obj) if obj == '.' or obj == '..': @@ -74,7 +77,7 @@ def gluster_check_object_creation(req, object_name): ret = __check_object_creation(req, object_name) if ret is None: - for obj in object_name.split('/'): + for obj in object_name.split(os.path.sep): reason = validate_obj_name_component(obj) if reason: bdy = 'Invalid object name "%s", component "%s" %s' \ diff --git a/gluster/swift/common/utils.py b/gluster/swift/common/utils.py index eeebf46..c943777 100644 --- a/gluster/swift/common/utils.py +++ b/gluster/swift/common/utils.py @@ -23,8 +23,9 @@ from eventlet import sleep import cPickle as pickle from swift.common.utils import normalize_timestamp from gluster.swift.common.fs_utils import do_rename, do_fsync, os_path, \ - do_stat, do_listdir, do_walk + do_stat, do_listdir, do_walk, dir_empty, rmdirs from gluster.swift.common import Glusterfs +from gluster.swift.common.exceptions import FileOrDirNotFoundError X_CONTENT_TYPE = 'Content-Type' X_CONTENT_LENGTH = 'Content-Length' @@ -41,8 +42,8 @@ ACCOUNT = 'Account' METADATA_KEY = 'user.swift.metadata' MAX_XATTR_SIZE = 65536 CONTAINER = 'container' -DIR = 'dir' -MARKER_DIR = 'marker_dir' +DIR_NON_OBJECT = 'dir' +DIR_OBJECT = 'marker_dir' TEMP_DIR = 'tmp' ASYNCDIR = 'async_pending' # Keep in sync with swift.obj.server.ASYNCDIR FILE = 'file' @@ -231,6 +232,12 @@ def _update_list(path, cont_path, src_list, reg_file=True, object_count=0, obj_path = path.replace(cont_path, '').strip(os.path.sep) for obj_name in src_list: + if not reg_file and Glusterfs.OBJECT_ONLY: + metadata = \ + read_metadata(os.path.join(cont_path, obj_path, obj_name)) + if not dir_is_object(metadata): + continue + if obj_path: obj_list.append(os.path.join(obj_path, obj_name)) else: @@ -247,11 +254,12 @@ def _update_list(path, cont_path, src_list, reg_file=True, object_count=0, def update_list(path, cont_path, dirs=[], files=[], object_count=0, bytes_used=0, obj_list=[]): + if files: object_count, bytes_used = _update_list(path, cont_path, files, True, object_count, bytes_used, obj_list) - if not Glusterfs.OBJECT_ONLY and dirs: + if dirs: object_count, bytes_used = _update_list(path, cont_path, dirs, False, object_count, bytes_used, obj_list) @@ -368,7 +376,7 @@ def get_object_metadata(obj_path): X_TYPE: OBJECT, X_TIMESTAMP: normalize_timestamp(stats.st_ctime), X_CONTENT_TYPE: DIR_TYPE if is_dir else FILE_TYPE, - X_OBJECT_TYPE: DIR if is_dir else FILE, + X_OBJECT_TYPE: DIR_NON_OBJECT if is_dir else FILE, X_CONTENT_LENGTH: 0 if is_dir else stats.st_size, X_ETAG: md5().hexdigest() if is_dir else _get_etag(obj_path)} return metadata @@ -476,6 +484,56 @@ def write_pickle(obj, dest, tmp=None, pickle_protocol=0): do_rename(tmppath, dest) +# The following dir_xxx calls should definitely be replaced +# with a Metadata class to encapsulate their implementation. +# :FIXME: For now we have them as functions, but we should +# move them to a class. +def dir_is_object(metadata): + """ + Determine if the directory with the path specified + has been identified as an object + """ + return metadata.get(X_OBJECT_TYPE, "") == DIR_OBJECT + + +def rmobjdir(dir_path): + """ + Removes the directory as long as there are + no objects stored in it. This works for containers also. + """ + try: + if dir_empty(dir_path): + rmdirs(dir_path) + return True + except FileOrDirNotFoundError: + # No such directory exists + return False + + for (path, dirs, files) in do_walk(dir_path, topdown=False): + for directory in dirs: + fullpath = os.path.join(path, directory) + metadata = read_metadata(fullpath) + + if not dir_is_object(metadata): + # Directory is not an object created by the caller + # so we can go ahead and delete it. + try: + if dir_empty(fullpath): + rmdirs(fullpath) + else: + # Non-object dir is not empty! + return False + except FileOrDirNotFoundError: + # No such dir! + return False + else: + # Wait, this is an object created by the caller + # We cannot delete + return False + rmdirs(dir_path) + return True + + # Over-ride Swift's utils.write_pickle with ours import swift.common.utils swift.common.utils.write_pickle = write_pickle -- cgit