diff options
| -rw-r--r-- | .zuul.yaml | 2 | ||||
| -rw-r--r-- | swift/common/daemon.py | 4 | ||||
| -rw-r--r-- | swift/proxy/controllers/obj.py | 40 | ||||
| -rw-r--r-- | test/unit/__init__.py | 33 | ||||
| -rw-r--r-- | test/unit/common/test_daemon.py | 94 | ||||
| -rw-r--r-- | test/unit/common/test_wsgi.py | 102 | ||||
| -rw-r--r-- | test/unit/proxy/controllers/test_obj.py | 69 | ||||
| -rw-r--r-- | tox.ini | 5 |
8 files changed, 311 insertions, 38 deletions
diff --git a/.zuul.yaml b/.zuul.yaml index b0bf91be7..16e5fd2e2 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -527,7 +527,7 @@ name: swift-tox-lower-constraints parent: openstack-tox-lower-constraints # This seems defensible for a l-c job - nodeset: ubuntu-bionic + nodeset: ubuntu-jammy vars: bindep_profile: test py27 python_version: 2.7 diff --git a/swift/common/daemon.py b/swift/common/daemon.py index 59a661189..773ca9424 100644 --- a/swift/common/daemon.py +++ b/swift/common/daemon.py @@ -315,7 +315,9 @@ def run_daemon(klass, conf_file, section_name='', once=False, **kwargs): logger.notice('Starting %s', os.getpid()) try: - DaemonStrategy(klass(conf), logger).run(once=once, **kwargs) + d = klass(conf) + DaemonStrategy(d, logger).run(once=once, **kwargs) except KeyboardInterrupt: logger.info('User quit') logger.notice('Exited %s', os.getpid()) + return d diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index fe0471191..fc489b79b 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -2500,7 +2500,7 @@ class ECFragGetter(object): self.client_chunk_size = policy.fragment_size self.skip_bytes = 0 self.bytes_used_from_backend = 0 - self.source = None + self.source = self.node = None self.logger_thread_locals = logger_thread_locals self.logger = logger @@ -2660,14 +2660,13 @@ class ECFragGetter(object): read_chunk_size=self.app.object_chunk_size) def iter_bytes_from_response_part(self, part_file, nbytes): - client_chunk_size = self.client_chunk_size - node_timeout = self.app.recoverable_node_timeout nchunks = 0 buf = b'' part_file = ByteCountEnforcer(part_file, nbytes) while True: try: - with WatchdogTimeout(self.app.watchdog, node_timeout, + with WatchdogTimeout(self.app.watchdog, + self.app.recoverable_node_timeout, ChunkReadTimeout): chunk = part_file.read(self.app.object_chunk_size) nchunks += 1 @@ -2726,33 +2725,18 @@ class ECFragGetter(object): self.bytes_used_from_backend += len(buf) buf = b'' - if not chunk: - if buf: - with WatchdogTimeout(self.app.watchdog, - self.app.client_timeout, - ChunkWriteTimeout): - self.bytes_used_from_backend += len(buf) - yield buf - buf = b'' - break - - if client_chunk_size is not None: - while len(buf) >= client_chunk_size: - client_chunk = buf[:client_chunk_size] - buf = buf[client_chunk_size:] - with WatchdogTimeout(self.app.watchdog, - self.app.client_timeout, - ChunkWriteTimeout): - self.bytes_used_from_backend += \ - len(client_chunk) - yield client_chunk - else: + client_chunk_size = self.client_chunk_size or len(buf) + while buf and (len(buf) >= client_chunk_size or not chunk): + client_chunk = buf[:client_chunk_size] + buf = buf[client_chunk_size:] with WatchdogTimeout(self.app.watchdog, self.app.client_timeout, ChunkWriteTimeout): - self.bytes_used_from_backend += len(buf) - yield buf - buf = b'' + self.bytes_used_from_backend += len(client_chunk) + yield client_chunk + + if not chunk: + break # This is for fairness; if the network is outpacing # the CPU, we'll always be able to read and write diff --git a/test/unit/__init__.py b/test/unit/__init__.py index 6f731b70a..f9847a10a 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -1408,3 +1408,36 @@ def generate_db_path(tempdir, server_type): return os.path.join( tempdir, '%ss' % server_type, 'part', 'suffix', 'hash', '%s-%s.db' % (server_type, uuid4())) + + +class ConfigAssertMixin(object): + """ + Use this with a TestCase to get py2/3 compatible assert for DuplicateOption + """ + def assertDuplicateOption(self, app_config, option_name, option_value): + """ + PY3 added a DuplicateOptionError, PY2 didn't seem to care + """ + if six.PY3: + self.assertDuplicateOptionError(app_config, option_name) + else: + self.assertDuplicateOptionOK(app_config, option_name, option_value) + + def assertDuplicateOptionError(self, app_config, option_name): + with self.assertRaises( + utils.configparser.DuplicateOptionError) as ctx: + app_config() + msg = str(ctx.exception) + self.assertIn(option_name, msg) + self.assertIn('already exists', msg) + + def assertDuplicateOptionOK(self, app_config, option_name, option_value): + app = app_config() + if hasattr(app, 'conf'): + found_value = app.conf[option_name] + else: + if hasattr(app, '_pipeline_final_app'): + # special case for proxy app! + app = app._pipeline_final_app + found_value = getattr(app, option_name) + self.assertEqual(found_value, option_value) diff --git a/test/unit/common/test_daemon.py b/test/unit/common/test_daemon.py index d53d15f10..94c7917bc 100644 --- a/test/unit/common/test_daemon.py +++ b/test/unit/common/test_daemon.py @@ -14,18 +14,19 @@ # limitations under the License. import os -from six import StringIO +import six import time import unittest from getpass import getuser import logging -from test.unit import tmpfile +from test.unit import tmpfile, with_tempdir, ConfigAssertMixin import mock import signal from contextlib import contextmanager import itertools from collections import defaultdict import errno +from textwrap import dedent from swift.common import daemon, utils from test.debug_logger import debug_logger @@ -106,7 +107,7 @@ class TestWorkerDaemon(unittest.TestCase): self.assertTrue(d.is_healthy()) -class TestRunDaemon(unittest.TestCase): +class TestRunDaemon(unittest.TestCase, ConfigAssertMixin): def setUp(self): for patcher in [ @@ -167,7 +168,7 @@ class TestRunDaemon(unittest.TestCase): conf_file, once=True) # test user quit - sio = StringIO() + sio = six.StringIO() logger = logging.getLogger('server') logger.addHandler(logging.StreamHandler(sio)) logger = utils.get_logger(None, 'server', log_route='server') @@ -207,6 +208,91 @@ class TestRunDaemon(unittest.TestCase): os.environ['TZ'] = old_tz time.tzset() + @with_tempdir + def test_run_deamon_from_conf_file(self, tempdir): + conf_path = os.path.join(tempdir, 'test-daemon.conf') + conf_body = """ + [DEFAULT] + conn_timeout = 5 + client_timeout = 1 + [my-daemon] + CONN_timeout = 10 + client_timeout = 2 + """ + contents = dedent(conf_body) + with open(conf_path, 'w') as f: + f.write(contents) + with mock.patch('swift.common.daemon.use_hub'): + d = daemon.run_daemon(MyDaemon, conf_path) + # my-daemon section takes priority (!?) + self.assertEqual('2', d.conf['client_timeout']) + self.assertEqual('10', d.conf['conn_timeout']) + + @with_tempdir + def test_run_daemon_from_conf_file_with_duplicate_var(self, tempdir): + conf_path = os.path.join(tempdir, 'test-daemon.conf') + conf_body = """ + [DEFAULT] + client_timeout = 3 + [my-daemon] + CLIENT_TIMEOUT = 2 + client_timeout = 1 + """ + contents = dedent(conf_body) + with open(conf_path, 'w') as f: + f.write(contents) + with mock.patch('swift.common.daemon.use_hub'): + app_config = lambda: daemon.run_daemon(MyDaemon, tempdir) + self.assertDuplicateOption(app_config, 'client_timeout', '1') + + @with_tempdir + def test_run_deamon_from_conf_dir(self, tempdir): + conf_files = { + 'default': """ + [DEFAULT] + conn_timeout = 5 + client_timeout = 1 + """, + 'daemon': """ + [DEFAULT] + CONN_timeout = 3 + CLIENT_TIMEOUT = 4 + [my-daemon] + CONN_timeout = 10 + client_timeout = 2 + """, + } + for filename, conf_body in conf_files.items(): + path = os.path.join(tempdir, filename + '.conf') + with open(path, 'wt') as fd: + fd.write(dedent(conf_body)) + with mock.patch('swift.common.daemon.use_hub'): + d = daemon.run_daemon(MyDaemon, tempdir) + # my-daemon section takes priority (!?) + self.assertEqual('2', d.conf['client_timeout']) + self.assertEqual('10', d.conf['conn_timeout']) + + @with_tempdir + def test_run_daemon_from_conf_dir_with_duplicate_var(self, tempdir): + conf_files = { + 'default': """ + [DEFAULT] + client_timeout = 3 + """, + 'daemon': """ + [my-daemon] + client_timeout = 2 + CLIENT_TIMEOUT = 4 + """, + } + for filename, conf_body in conf_files.items(): + path = os.path.join(tempdir, filename + '.conf') + with open(path, 'wt') as fd: + fd.write(dedent(conf_body)) + with mock.patch('swift.common.daemon.use_hub'): + app_config = lambda: daemon.run_daemon(MyDaemon, tempdir) + self.assertDuplicateOption(app_config, 'client_timeout', '4') + @contextmanager def mock_os(self, child_worker_cycles=3): self.waitpid_calls = defaultdict(int) diff --git a/test/unit/common/test_wsgi.py b/test/unit/common/test_wsgi.py index 5cddc7164..39349ffa3 100644 --- a/test/unit/common/test_wsgi.py +++ b/test/unit/common/test_wsgi.py @@ -43,7 +43,7 @@ from swift.common.storage_policy import POLICIES from test import listen_zero from test.debug_logger import debug_logger from test.unit import ( - temptree, with_tempdir, write_fake_ring, patch_policies) + temptree, with_tempdir, write_fake_ring, patch_policies, ConfigAssertMixin) from paste.deploy import loadwsgi @@ -60,7 +60,7 @@ def _fake_rings(tmpdir): @patch_policies -class TestWSGI(unittest.TestCase): +class TestWSGI(unittest.TestCase, ConfigAssertMixin): """Tests for swift.common.wsgi""" def test_init_request_processor(self): @@ -133,14 +133,38 @@ class TestWSGI(unittest.TestCase): def test_loadapp_from_file(self, tempdir): conf_path = os.path.join(tempdir, 'object-server.conf') conf_body = """ + [DEFAULT] + CONN_timeout = 10 + client_timeout = 1 [app:main] use = egg:swift#object + conn_timeout = 5 + client_timeout = 2 + CLIENT_TIMEOUT = 3 """ contents = dedent(conf_body) with open(conf_path, 'w') as f: f.write(contents) app = wsgi.loadapp(conf_path) self.assertIsInstance(app, obj_server.ObjectController) + self.assertTrue(isinstance(app, obj_server.ObjectController)) + self.assertEqual(1, app.client_timeout) + self.assertEqual(5, app.conn_timeout) + + @with_tempdir + def test_loadapp_from_file_with_duplicate_var(self, tempdir): + conf_path = os.path.join(tempdir, 'object-server.conf') + conf_body = """ + [app:main] + use = egg:swift#object + client_timeout = 2 + client_timeout = 3 + """ + contents = dedent(conf_body) + with open(conf_path, 'w') as f: + f.write(contents) + app_config = lambda: wsgi.loadapp(conf_path) + self.assertDuplicateOption(app_config, 'client_timeout', 3.0) @with_tempdir def test_loadapp_from_file_with_global_conf(self, tempdir): @@ -204,11 +228,85 @@ class TestWSGI(unittest.TestCase): def test_loadapp_from_string(self): conf_body = """ + [DEFAULT] + CONN_timeout = 10 + client_timeout = 1 [app:main] use = egg:swift#object + conn_timeout = 5 + client_timeout = 2 """ app = wsgi.loadapp(wsgi.ConfigString(conf_body)) self.assertTrue(isinstance(app, obj_server.ObjectController)) + self.assertEqual(1, app.client_timeout) + self.assertEqual(5, app.conn_timeout) + + @with_tempdir + def test_loadapp_from_dir(self, tempdir): + conf_files = { + 'pipeline': """ + [pipeline:main] + pipeline = tempauth proxy-server + """, + 'tempauth': """ + [DEFAULT] + swift_dir = %s + random_VAR = foo + [filter:tempauth] + use = egg:swift#tempauth + random_var = bar + """ % tempdir, + 'proxy': """ + [DEFAULT] + conn_timeout = 5 + client_timeout = 1 + [app:proxy-server] + use = egg:swift#proxy + CONN_timeout = 10 + client_timeout = 2 + """, + } + _fake_rings(tempdir) + for filename, conf_body in conf_files.items(): + path = os.path.join(tempdir, filename + '.conf') + with open(path, 'wt') as fd: + fd.write(dedent(conf_body)) + app = wsgi.loadapp(tempdir) + # DEFAULT takes priority (!?) + self.assertEqual(5, app._pipeline_final_app.conn_timeout) + self.assertEqual(1, app._pipeline_final_app.client_timeout) + self.assertEqual('foo', app.app.app.app.conf['random_VAR']) + self.assertEqual('bar', app.app.app.app.conf['random_var']) + + @with_tempdir + def test_loadapp_from_dir_with_duplicate_var(self, tempdir): + conf_files = { + 'pipeline': """ + [pipeline:main] + pipeline = tempauth proxy-server + """, + 'tempauth': """ + [DEFAULT] + swift_dir = %s + random_VAR = foo + [filter:tempauth] + use = egg:swift#tempauth + random_var = bar + """ % tempdir, + 'proxy': """ + [app:proxy-server] + use = egg:swift#proxy + client_timeout = 2 + CLIENT_TIMEOUT = 1 + """, + } + _fake_rings(tempdir) + for filename, conf_body in conf_files.items(): + path = os.path.join(tempdir, filename + '.conf') + with open(path, 'wt') as fd: + fd.write(dedent(conf_body)) + app_config = lambda: wsgi.loadapp(tempdir) + self.assertDuplicateOption(app_config, 'client_timeout', 2.0) @with_tempdir def test_load_app_config(self, tempdir): diff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py index 6c2c2bdab..0b8bf1568 100644 --- a/test/unit/proxy/controllers/test_obj.py +++ b/test/unit/proxy/controllers/test_obj.py @@ -39,8 +39,9 @@ else: import swift from swift.common import utils, swob, exceptions -from swift.common.exceptions import ChunkWriteTimeout -from swift.common.utils import Timestamp, list_from_csv, md5 +from swift.common.exceptions import ChunkWriteTimeout, ShortReadError, \ + ChunkReadTimeout +from swift.common.utils import Timestamp, list_from_csv, md5, FileLikeIter from swift.proxy import server as proxy_server from swift.proxy.controllers import obj from swift.proxy.controllers.base import \ @@ -6676,5 +6677,69 @@ class TestNumContainerUpdates(unittest.TestCase): c_replica, o_replica, o_quorum)) +@patch_policies(with_ec_default=True) +class TestECFragGetter(BaseObjectControllerMixin, unittest.TestCase): + def setUp(self): + super(TestECFragGetter, self).setUp() + req = Request.blank(path='/a/c/o') + self.getter = obj.ECFragGetter( + self.app, req, None, None, self.policy, 'a/c/o', + {}, None, self.logger.thread_locals, + self.logger) + + def test_iter_bytes_from_response_part(self): + part = FileLikeIter([b'some', b'thing']) + it = self.getter.iter_bytes_from_response_part(part, nbytes=None) + self.assertEqual(b'something', b''.join(it)) + + def test_iter_bytes_from_response_part_insufficient_bytes(self): + part = FileLikeIter([b'some', b'thing']) + it = self.getter.iter_bytes_from_response_part(part, nbytes=100) + with mock.patch.object(self.getter, '_dig_for_source_and_node', + return_value=(None, None)): + with self.assertRaises(ShortReadError) as cm: + b''.join(it) + self.assertEqual('Too few bytes; read 9, expecting 100', + str(cm.exception)) + + def test_iter_bytes_from_response_part_read_timeout(self): + part = FileLikeIter([b'some', b'thing']) + self.app.recoverable_node_timeout = 0.05 + self.app.client_timeout = 0.8 + it = self.getter.iter_bytes_from_response_part(part, nbytes=9) + with mock.patch.object(self.getter, '_dig_for_source_and_node', + return_value=(None, None)): + with mock.patch.object(part, 'read', + side_effect=[b'some', ChunkReadTimeout(9)]): + with self.assertRaises(ChunkReadTimeout) as cm: + b''.join(it) + self.assertEqual('9 seconds', str(cm.exception)) + + def test_iter_bytes_from_response_part_null_chunk_size(self): + # we don't expect a policy to have fragment_size None or zero but + # verify that the getter is defensive + self.getter.client_chunk_size = None + part = FileLikeIter([b'some', b'thing', b'']) + it = self.getter.iter_bytes_from_response_part(part, nbytes=None) + self.assertEqual(b'something', b''.join(it)) + + self.getter.client_chunk_size = 0 + part = FileLikeIter([b'some', b'thing', b'']) + it = self.getter.iter_bytes_from_response_part(part, nbytes=None) + self.assertEqual(b'something', b''.join(it)) + + def test_iter_bytes_from_response_part_small_chunk_size(self): + # we don't expect a policy to have fragment_size None or zero but + # verify that the getter is defensive + self.getter.client_chunk_size = 4 + part = FileLikeIter([b'some', b'thing', b'']) + it = self.getter.iter_bytes_from_response_part(part, nbytes=None) + self.assertEqual([b'some', b'thin', b'g'], [ch for ch in it]) + self.getter.client_chunk_size = 1 + part = FileLikeIter([b'some', b'thing', b'']) + it = self.getter.iter_bytes_from_response_part(part, nbytes=None) + self.assertEqual([c.encode() for c in 'something'], [ch for ch in it]) + + if __name__ == '__main__': unittest.main() @@ -1,6 +1,11 @@ [tox] envlist = py37,py27,pep8 minversion = 3.18.0 +requires = + # required to support py27/py36 envs + virtualenv<20.22 + # project-wide requirement; see .zuul.yaml + tox<4 [pytest] addopts = --verbose |