openstack/swift
Code Issues Proposed changes

object: Block POSTs and chunked PUTs when already past reserve

Browse Source 
Previously, clients could bypass fallocate_reserve checks by uploading
with `Transfer-Encoding: chunked` rather than sending a `Content-Length`

Now, a chunked transfer may still push a disk past the reserve threshold,
but once over the threshold, further PUTs and POSTs will 507. DELETEs
will still be allowed.

Closes-Bug: #2031049
Change-Id: I69ec7193509cd3ed0aa98aca15190468368069a5
This commit is contained in:
Tim Burke 2023-08-14 16:05:02 -07:00
parent 5555980fb5
commit d31a54a65c
5 changed files with 159 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
swift/common/utils/__init__.py
View File

@ -830,13 +830,14 @@ class FileLikeIter(object):
self.closed = True
def fs_has_free_space(fs_path, space_needed, is_percent):
def fs_has_free_space(fs_path_or_fd, space_needed, is_percent):
"""
Check to see whether or not a filesystem has the given amount of space
free. Unlike fallocate(), this does not reserve any space.
:param fs_path: path to a file or directory on the filesystem; typically
the path to the filesystem's mount point
:param fs_path_or_fd: path to a file or directory on the filesystem, or an
open file descriptor; if a directory, typically the path to the
filesystem's mount point
:param space_needed: minimum bytes or percentage of free space
@ -849,7 +850,10 @@ def fs_has_free_space(fs_path, space_needed, is_percent):
:raises OSError: if fs_path does not exist
"""
st = os.statvfs(fs_path)
if isinstance(fs_path_or_fd, int):
st = os.fstatvfs(fs_path_or_fd)
else:
st = os.statvfs(fs_path_or_fd)
free_bytes = st.f_frsize * st.f_bavail
if is_percent:
size_bytes = st.f_frsize * st.f_blocks

20
swift/obj/diskfile.py
View File

@ -65,7 +65,8 @@ from swift.common.utils import mkdirs, Timestamp, \
get_md5_socket, F_SETPIPE_SZ, decode_timestamps, encode_timestamps, \
MD5_OF_EMPTY_STRING, link_fd_to_path, \
O_TMPFILE, makedirs_count, replace_partition_in_path, remove_directory, \
md5, is_file_older, non_negative_float
md5, is_file_older, non_negative_float, config_fallocate_value, \
fs_has_free_space
from swift.common.splice import splice, tee
from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
DiskFileCollision, DiskFileNoSpace, DiskFileDeviceUnavailable, \
@ -751,6 +752,8 @@ class BaseDiskFileManager(object):
replication_concurrency_per_device)
self.replication_lock_timeout = int(conf.get(
'replication_lock_timeout', 15))
self.fallocate_reserve, self.fallocate_is_percent = \
config_fallocate_value(conf.get('fallocate_reserve', '1%'))
self.use_splice = False
self.pipe_size = None
@ -1858,13 +1861,26 @@ class BaseDiskFileWriter(object):
# No more inodes in filesystem
raise DiskFileNoSpace()
raise
if self._size is not None and self._size > 0:
if self._extension == '.ts':
# DELETEs always bypass any free-space reserve checks
pass
elif self._size:
try:
fallocate(self._fd, self._size)
except OSError as err:
if err.errno in (errno.ENOSPC, errno.EDQUOT):
raise DiskFileNoSpace()
raise
else:
# If we don't know the size (i.e. self._size is None) or the size
# is known to be zero, we still want to block writes once we're
# past the reserve threshold.
if not fs_has_free_space(
self._fd,
self.manager.fallocate_reserve,
self.manager.fallocate_is_percent
):
raise DiskFileNoSpace()
return self
def close(self):

1
swift/obj/mem_server.py
View File

@ -32,6 +32,7 @@ class ObjectController(server.ObjectController):
:param conf: WSGI configuration parameter
"""
self._filesystem = InMemoryFileSystem()
self.fallocate_reserve = 0
def get_diskfile(self, device, partition, account, container, obj,
**kwargs):

35
test/unit/common/test_utils.py
View File

@ -6958,8 +6958,8 @@ class TestHashForFileFunction(unittest.TestCase):
class TestFsHasFreeSpace(unittest.TestCase):
def test_bytes(self):
fake_result = posix.statvfs_result([
def setUp(self):
self.fake_result = posix.statvfs_result([
4096, # f_bsize
4096, # f_frsize
2854907, # f_blocks
@ -6971,28 +6971,31 @@ class TestFsHasFreeSpace(unittest.TestCase):
4096, # f_flag
255, # f_namemax
])
with mock.patch('os.statvfs', return_value=fake_result):
def test_bytes(self):
with mock.patch(
'os.statvfs', return_value=self.fake_result) as mock_statvfs:
self.assertTrue(utils.fs_has_free_space("/", 0, False))
self.assertTrue(utils.fs_has_free_space("/", 1, False))
# free space left = f_bavail * f_bsize = 7078252544
self.assertTrue(utils.fs_has_free_space("/", 7078252544, False))
self.assertFalse(utils.fs_has_free_space("/", 7078252545, False))
self.assertFalse(utils.fs_has_free_space("/", 2 ** 64, False))
mock_statvfs.assert_has_calls([mock.call("/")] * 5)
def test_bytes_using_file_descriptor(self):
with mock.patch(
'os.fstatvfs', return_value=self.fake_result) as mock_fstatvfs:
self.assertTrue(utils.fs_has_free_space(99, 0, False))
self.assertTrue(utils.fs_has_free_space(99, 1, False))
# free space left = f_bavail * f_bsize = 7078252544
self.assertTrue(utils.fs_has_free_space(99, 7078252544, False))
self.assertFalse(utils.fs_has_free_space(99, 7078252545, False))
self.assertFalse(utils.fs_has_free_space(99, 2 ** 64, False))
mock_fstatvfs.assert_has_calls([mock.call(99)] * 5)
def test_percent(self):
fake_result = posix.statvfs_result([
4096, # f_bsize
4096, # f_frsize
2854907, # f_blocks
1984802, # f_bfree (free blocks for root)
1728089, # f_bavail (free blocks for non-root)
1280000, # f_files
1266040, # f_ffree,
1266040, # f_favail,
4096, # f_flag
255, # f_namemax
])
with mock.patch('os.statvfs', return_value=fake_result):
with mock.patch('os.statvfs', return_value=self.fake_result):
self.assertTrue(utils.fs_has_free_space("/", 0, True))
self.assertTrue(utils.fs_has_free_space("/", 1, True))
# percentage of free space for the faked statvfs is 60%

118
test/unit/obj/test_server.py
View File

@ -7765,9 +7765,7 @@ class TestObjectController(BaseTestCase):
def fake_fallocate(fd, size):
raise OSError(errno.ENOSPC, os.strerror(errno.ENOSPC))
orig_fallocate = diskfile.fallocate
try:
diskfile.fallocate = fake_fallocate
with mock.patch.object(diskfile, 'fallocate', fake_fallocate):
timestamp = normalize_timestamp(time())
body_reader = IgnoredBody()
req = Request.blank(
@ -7781,8 +7779,118 @@ class TestObjectController(BaseTestCase):
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 507)
self.assertFalse(body_reader.read_called)
finally:
diskfile.fallocate = orig_fallocate
def test_chunked_PUT_with_full_drive(self):
class IgnoredBody(object):
def __init__(self):
self.read_called = False
def read(self, size=-1):
if not self.read_called:
self.read_called = True
return b'VERIFY'
return b''
with mock.patch.object(diskfile, 'fs_has_free_space',
return_value=False):
timestamp = normalize_timestamp(time())
body_reader = IgnoredBody()
req = Request.blank(
'/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'PUT',
'wsgi.input': body_reader},
headers={'X-Timestamp': timestamp,
'Transfer-Encoding': 'chunked',
'Content-Type': 'application/octet-stream',
'Expect': '100-continue'})
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 507)
self.assertFalse(body_reader.read_called)
def test_POST_with_full_drive(self):
ts_iter = make_timestamp_iter()
timestamp = next(ts_iter).internal
req = Request.blank(
'/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'PUT'},
body=b'VERIFY',
headers={'X-Timestamp': timestamp,
'Content-Type': 'application/octet-stream'})
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 201)
with mock.patch.object(diskfile, 'fs_has_free_space',
return_value=False):
timestamp = next(ts_iter).internal
req = Request.blank(
'/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'POST'},
headers={'X-Timestamp': timestamp,
'Content-Type': 'application/octet-stream'})
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 507)
def test_DELETE_with_full_drive(self):
timestamp = normalize_timestamp(time())
req = Request.blank(
'/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'PUT'},
body=b'VERIFY',
headers={'X-Timestamp': timestamp,
'Content-Type': 'application/octet-stream'})
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 201)
with mock.patch.object(diskfile, 'fs_has_free_space',
return_value=False):
timestamp = normalize_timestamp(time())
req = Request.blank(
'/sda1/p/a/c/o',
method='DELETE',
body=b'',
headers={'X-Timestamp': timestamp})
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 204)
def test_chunked_DELETE_with_full_drive(self):
timestamp = normalize_timestamp(time())
req = Request.blank(
'/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'PUT'},
body=b'VERIFY',
headers={'X-Timestamp': timestamp,
'Content-Type': 'application/octet-stream'})
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 201)
class IgnoredBody(object):
def __init__(self):
self.read_called = False
def read(self, size=-1):
if not self.read_called:
self.read_called = True
return b'VERIFY'
return b''
with mock.patch.object(diskfile, 'fs_has_free_space',
return_value=False):
timestamp = normalize_timestamp(time())
body_reader = IgnoredBody()
req = Request.blank(
'/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE',
'wsgi.input': body_reader},
headers={'X-Timestamp': timestamp,
'Transfer-Encoding': 'chunked',
'Content-Type': 'application/octet-stream',
'Expect': '100-continue'})
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 204)
self.assertFalse(body_reader.read_called)
def test_global_conf_callback_does_nothing(self):
preloaded_app_conf = {}