x/swiftonhpss
Code Issues Proposed changes

PUT of a Directory failed

Browse Source 
PUT of a directory fails with gluster-swift when there exists
object of a same name. This is because the objects and directories are placed on
the glusterfs. And hence the filesystem semantics are applicable there.

Exceptions raised in such situation are needed to be handled and reported
correctly back to proxy-server with HTTPConflict as a error code. Although
swift still continues to choose 503 as the best respose in such cases. No
tracebacks reported.

Fix covers the case when there exists a directory and object of the same
name is PUT.

Code changes fixes both the failure cases mentioned in the bug.

Examples of failing PUT requests:
1) curl -v  -X PUT http://127.0.0.1:8080/v1/AUTH_test/c1/dir1/obj2/anotherobject
-d'asdasdsadA'
-- obj2 was already an object.

2)curl -v -H 'Content-Length: 0' -H 'Content-Type: application/directory' -X PUT
http://127.0.0.1:8080/v1/AUTH_test/c1/obj1
-- obj1 was already and object

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1071021

Change-Id: Id3042d920e3f99e740d4042ef5907ac8c59e04db
Signed-off-by: Chetan Risbud <crisbud@redhat.com>
Reviewed-on: http://review.gluster.org/7181
Reviewed-by: Prashanth Pai <ppai@redhat.com>
Tested-by: Prashanth Pai <ppai@redhat.com>
This commit is contained in:
Chetan Risbud 2014-03-04 19:35:24 +05:30
parent 47cbf09a22
commit 1a6b55714f
3 changed files with 45 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
gluster/swift/obj/diskfile.py
View File

@ -29,6 +29,8 @@ from hashlib import md5
from eventlet import sleep
from greenlet import getcurrent
from contextlib import contextmanager
from gluster.swift.common.exceptions import AlreadyExistsAsFile, \
AlreadyExistsAsDir
from swift.common.utils import TRUE_VALUES, ThreadPool, config_true_value
from swift.common.exceptions import DiskFileNotExist, DiskFileError, \
DiskFileNoSpace, DiskFileDeviceUnavailable, DiskFileNotOpen
@ -166,17 +168,19 @@ def _make_directory_unlocked(full_path, uid, gid, metadata=None):
if not is_dir:
# FIXME: Ideally we'd want to return an appropriate error
# message and code in the PUT Object REST API response.
raise DiskFileError("_make_directory_unlocked: os.mkdir"
" failed on path %s because it already"
" exists but not as a directory" % (
full_path))
raise AlreadyExistsAsFile("_make_directory_unlocked:"
" os.mkdir failed on path %s"
" because it already exists"
" but not as a directory"
% (full_path))
return True, metadata
elif err.errno == errno.ENOTDIR:
# FIXME: Ideally we'd want to return an appropriate error
# message and code in the PUT Object REST API response.
raise DiskFileError("_make_directory_unlocked: os.mkdir failed"
" because some part of path %s is not in fact"
" a directory" % (full_path))
raise AlreadyExistsAsFile("_make_directory_unlocked:"
" os.mkdir failed because some "
"part of path %s is not in fact"
" a directory" % (full_path))
elif err.errno == errno.EIO:
# Sometimes Fuse will return an EIO error when it does not know
# how to handle an unexpected, but transient situation. It is
@ -482,6 +486,8 @@ class DiskFileWriter(object):
written to the temp file.
:param metadata: dictionary of metadata to be written
:raises AlreadyExistsAsDir : If there exists a directory of the same
name
"""
assert self._tmppath is not None
metadata = _adjust_metadata(metadata)
@ -497,9 +503,9 @@ class DiskFileWriter(object):
# system, perhaps gratuitously created when another
# object was created, or created externally to Swift
# REST API servicing (UFO use case).
raise DiskFileError('DiskFile.put(): file creation failed since'
' the target, %s, already exists as a'
' directory' % df._data_file)
raise AlreadyExistsAsDir('DiskFile.put(): file creation failed'
' since the target, %s, already exists'
' as a directory' % df._data_file)
df._threadpool.force_run_in_thread(self._finalize_put, metadata)
@ -897,6 +903,8 @@ class DiskFile(object):
:param size: optional initial size of file to explicitly allocate on
disk
:raises DiskFileNoSpace: if a size is specified and allocation fails
:raises AlreadyExistsAsFile: if path or part of a path is not a \
directory
"""
data_file = os.path.join(self._put_datadir, self._obj)
@ -917,6 +925,12 @@ class DiskFile(object):
# Raise DiskFileNoSpace to be handled by upper layers when
# there is no space on disk OR when quota is exceeded
raise DiskFileNoSpace()
if gerr.errno == errno.ENOTDIR:
raise AlreadyExistsAsFile('do_open(): failed on %s,'
' path or part of a'
' path is not a directory'
% (tmppath))
if gerr.errno not in (errno.ENOENT, errno.EEXIST, errno.EIO):
# FIXME: Other cases we should handle?
raise

17
gluster/swift/obj/server.py
View File

@ -1,4 +1,4 @@
# Copyright (c) 2012-2013 Red Hat, Inc.
# Copyright (c) 2012-2014 Red Hat, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
@ -18,6 +18,11 @@
# Simply importing this monkey patches the constraint handling to fit our
# needs
import gluster.swift.common.constraints # noqa
from swift.common.swob import HTTPConflict
from swift.common.utils import public, timing_stats
from gluster.swift.common.exceptions import AlreadyExistsAsFile, \
AlreadyExistsAsDir
from swift.common.request_helpers import split_and_validate_path
from swift.obj import server
@ -80,6 +85,16 @@ class ObjectController(server.ObjectController):
"""
return
@public
@timing_stats()
def PUT(self, request):
try:
return server.ObjectController.PUT(self, request)
except (AlreadyExistsAsFile, AlreadyExistsAsDir):
device = \
split_and_validate_path(request, 1, 5, True)
return HTTPConflict(drive=device, request=request)
def app_factory(global_conf, **local_conf):
"""paste.deploy app factory for creating WSGI object server apps"""

9
test/unit/obj/test_diskfile.py
View File

@ -26,7 +26,8 @@ from eventlet import tpool
from mock import Mock, patch
from hashlib import md5
from copy import deepcopy
from gluster.swift.common.exceptions import AlreadyExistsAsDir, \
AlreadyExistsAsFile
from swift.common.exceptions import DiskFileNotExist, DiskFileError, \
DiskFileNoSpace, DiskFileNotOpen
from swift.common.utils import ThreadPool
@ -409,7 +410,7 @@ class TestDiskFile(unittest.TestCase):
dc = gluster.swift.obj.diskfile.do_chown
gluster.swift.obj.diskfile.do_chown = _mock_do_chown
self.assertRaises(
DiskFileError, gdf._create_dir_object, the_dir)
AlreadyExistsAsFile, gdf._create_dir_object, the_dir)
gluster.swift.obj.diskfile.do_chown = dc
self.assertFalse(os.path.isdir(the_dir))
self.assertFalse(_mapit(the_dir) in _metadata)
@ -431,7 +432,7 @@ class TestDiskFile(unittest.TestCase):
dc = gluster.swift.obj.diskfile.do_chown
gluster.swift.obj.diskfile.do_chown = _mock_do_chown
self.assertRaises(
DiskFileError, gdf._create_dir_object, the_dir)
AlreadyExistsAsFile, gdf._create_dir_object, the_dir)
gluster.swift.obj.diskfile.do_chown = dc
self.assertFalse(os.path.isdir(the_dir))
self.assertFalse(_mapit(the_dir) in _metadata)
@ -622,7 +623,7 @@ class TestDiskFile(unittest.TestCase):
# directory.
dw.write('12345\n')
dw.put(newmd)
except DiskFileError:
except AlreadyExistsAsDir:
pass
else:
self.fail("Expected to encounter"