openstack
/
swift
Code Issues Proposed changes

Add --test-config option to WSGI servers 

Previously, seamless reloads were a little risky: when they worked, they
worked great, but if they failed (say, because you wrote out an invalid
config), you were left with no usable server processes and possible
client downtime.

Now, add the ability to do a preflight check before reloading processes
to reduce the likelihood of the reloaded process immediately dying. For
example, you might use a systemd unit that includes something like

    ExecReload=swift-proxy-server --test-config /etc/swift/proxy-server.conf
    ExecReload=kill -USR1 $MAINPID"

Change-Id: I9e5e158ce8be92535430b9cabf040063f5188bf4
This commit is contained in:
Tim Burke 2022-03-10 08:42:06 -08:00
parent 57ce156a7f
commit 0a5f0253b1
7 changed files with 120 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
bin/swift-account-server
View File

@ -19,5 +19,5 @@ from swift.common.utils import parse_options
from swift.common.wsgi import run_wsgi
if __name__ == '__main__':
conf_file, options = parse_options()
conf_file, options = parse_options(test_config=True)
sys.exit(run_wsgi(conf_file, 'account-server', **options))

2
bin/swift-container-server
View File

@ -19,5 +19,5 @@ from swift.common.utils import parse_options
from swift.common.wsgi import run_wsgi
if __name__ == '__main__':
conf_file, options = parse_options()
conf_file, options = parse_options(test_config=True)
sys.exit(run_wsgi(conf_file, 'container-server', **options))

2
bin/swift-object-server
View File

@ -21,7 +21,7 @@ from swift.obj import server
if __name__ == '__main__':
conf_file, options = parse_options()
conf_file, options = parse_options(test_config=True)
sys.exit(run_wsgi(conf_file, 'object-server',
global_conf_callback=server.global_conf_callback,
**options))

2
bin/swift-proxy-server
View File

@ -19,5 +19,5 @@ from swift.common.utils import parse_options
from swift.common.wsgi import run_wsgi
if __name__ == '__main__':
conf_file, options = parse_options()
conf_file, options = parse_options(test_config=True)
sys.exit(run_wsgi(conf_file, 'proxy-server', **options))

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

@ -2655,11 +2655,13 @@ def capture_stdio(logger, **kwargs):
sys.stderr = LoggerFileObject(logger, 'STDERR')
def parse_options(parser=None, once=False, test_args=None):
def parse_options(parser=None, once=False, test_config=False, test_args=None):
"""Parse standard swift server/daemon options with optparse.OptionParser.
:param parser: OptionParser to use. If not sent one will be created.
:param once: Boolean indicating the "once" option is available
:param test_config: Boolean indicating the "test-config" option is
available
:param test_args: Override sys.argv; used in testing
:returns: Tuple of (config, options); config is an absolute path to the
@ -2674,6 +2676,11 @@ def parse_options(parser=None, once=False, test_args=None):
if once:
parser.add_option("-o", "--once", default=False, action="store_true",
help="only run one pass of daemon")
if test_config:
parser.add_option("-t", "--test-config",
default=False, action="store_true",
help="exit after loading and validating config; "
"do not run the daemon")
# if test_args is None, optparse will use sys.argv[:1]
options, args = parser.parse_args(args=test_args)

63
swift/common/wsgi.py
View File

@ -788,27 +788,10 @@ class ServersPerPortStrategy(StrategyBase):
yield sock
def run_wsgi(conf_path, app_section, *args, **kwargs):
"""
Runs the server according to some strategy. The default strategy runs a
specified number of workers in pre-fork model. The object-server (only)
may use a servers-per-port strategy if its config has a servers_per_port
setting with a value greater than zero.
:param conf_path: Path to paste.deploy style configuration file/directory
:param app_section: App name from conf file to load config from
:param allow_modify_pipeline: Boolean for whether the server should have
an opportunity to change its own pipeline.
Defaults to True
:returns: 0 if successful, nonzero otherwise
"""
def check_config(conf_path, app_section, *args, **kwargs):
# Load configuration, Set logger and Load request processor
try:
(conf, logger, log_name) = \
_initrp(conf_path, app_section, *args, **kwargs)
except ConfigFileError as e:
print(e)
return 1
(conf, logger, log_name) = \
_initrp(conf_path, app_section, *args, **kwargs)
# optional nice/ionice priority scheduling
utils.modify_priority(conf, logger)
@ -825,13 +808,13 @@ def run_wsgi(conf_path, app_section, *args, **kwargs):
strategy = WorkersStrategy(conf, logger)
try:
# Quick sanity check
int(conf['bind_port'])
if not (1 <= int(conf['bind_port']) <= 2 ** 16 - 1):
raise ValueError
except (ValueError, KeyError, TypeError):
error_msg = 'bind_port wasn\'t properly set in the config file. ' \
'It must be explicitly set to a valid port number.'
'It must be explicitly set to a valid port number.'
logger.error(error_msg)
print(error_msg)
return 1
raise ConfigFileError(error_msg)
# patch event before loadapp
utils.eventlet_monkey_patch()
@ -842,16 +825,44 @@ def run_wsgi(conf_path, app_section, *args, **kwargs):
if 'global_conf_callback' in kwargs:
kwargs['global_conf_callback'](conf, global_conf)
allow_modify_pipeline = kwargs.get('allow_modify_pipeline', True)
# set utils.FALLOCATE_RESERVE if desired
utils.FALLOCATE_RESERVE, utils.FALLOCATE_IS_PERCENT = \
utils.config_fallocate_value(conf.get('fallocate_reserve', '1%'))
return conf, logger, global_conf, strategy
def run_wsgi(conf_path, app_section, *args, **kwargs):
"""
Runs the server according to some strategy. The default strategy runs a
specified number of workers in pre-fork model. The object-server (only)
may use a servers-per-port strategy if its config has a servers_per_port
setting with a value greater than zero.
:param conf_path: Path to paste.deploy style configuration file/directory
:param app_section: App name from conf file to load config from
:param allow_modify_pipeline: Boolean for whether the server should have
an opportunity to change its own pipeline.
Defaults to True
:param test_config: if False (the default) then load and validate the
config and if successful then continue to run the server; if True then
load and validate the config but do not run the server.
:returns: 0 if successful, nonzero otherwise
"""
try:
conf, logger, global_conf, strategy = check_config(
conf_path, app_section, *args, **kwargs)
except ConfigFileError as err:
print(err)
return 1
if kwargs.get('test_config'):
return 0
# Do some daemonization process hygene before we fork any children or run a
# server without forking.
clean_up_daemon_hygiene()
allow_modify_pipeline = kwargs.get('allow_modify_pipeline', True)
no_fork_sock = strategy.no_fork_sock()
if no_fork_sock:
run_server(conf, logger, no_fork_sock, global_conf=global_conf,

73
test/unit/common/test_wsgi.py
View File

@ -879,7 +879,7 @@ class TestWSGI(unittest.TestCase):
thread=True)
def test_run_server_success(self):
calls = defaultdict(lambda: 0)
calls = defaultdict(int)
def _initrp(conf_file, app_section, *args, **kwargs):
calls['_initrp'] += 1
@ -908,10 +908,45 @@ class TestWSGI(unittest.TestCase):
select=True,
thread=True)
# run_wsgi() no longer calls drop_privileges() in the parent process,
# just clean_up_deemon_hygene()
# just clean_up_daemon_hygene()
self.assertEqual([], _d_privs.mock_calls)
self.assertEqual([mock.call()], _c_hyg.mock_calls)
def test_run_server_test_config(self):
calls = defaultdict(int)
def _initrp(conf_file, app_section, *args, **kwargs):
calls['_initrp'] += 1
return (
{'__file__': 'test', 'workers': 0, 'bind_port': 12345},
'logger',
'log_name')
def _loadapp(uri, name=None, **kwargs):
calls['_loadapp'] += 1
with mock.patch.object(wsgi, '_initrp', _initrp), \
mock.patch.object(wsgi, 'get_socket') as _get_socket, \
mock.patch.object(wsgi, 'drop_privileges') as _d_privs, \
mock.patch.object(wsgi, 'clean_up_daemon_hygiene') as _c_hyg, \
mock.patch.object(wsgi, 'loadapp', _loadapp), \
mock.patch.object(wsgi, 'capture_stdio'), \
mock.patch.object(wsgi, 'run_server'), \
mock.patch('swift.common.utils.eventlet') as _utils_evt:
rc = wsgi.run_wsgi('conf_file', 'app_section', test_config=True)
self.assertEqual(calls['_initrp'], 1)
self.assertEqual(calls['_loadapp'], 1)
self.assertEqual(rc, 0)
_utils_evt.patcher.monkey_patch.assert_called_with(all=False,
socket=True,
select=True,
thread=True)
# run_wsgi() stops before calling clean_up_daemon_hygene() or
# creating sockets
self.assertEqual([], _d_privs.mock_calls)
self.assertEqual([], _c_hyg.mock_calls)
self.assertEqual([], _get_socket.mock_calls)
@mock.patch('swift.common.wsgi.run_server')
@mock.patch('swift.common.wsgi.WorkersStrategy')
@mock.patch('swift.common.wsgi.ServersPerPortStrategy')
@ -997,6 +1032,40 @@ class TestWSGI(unittest.TestCase):
self.assertEqual(calls['_loadapp'], 0)
self.assertEqual(rc, 1)
def test_run_server_bad_bind_port(self):
def do_test(port):
calls = defaultdict(lambda: 0)
logger = debug_logger()
def _initrp(conf_file, app_section, *args, **kwargs):
calls['_initrp'] += 1
return (
{'__file__': 'test', 'workers': 0, 'bind_port': port},
logger,
'log_name')
def _loadapp(uri, name=None, **kwargs):
calls['_loadapp'] += 1
with mock.patch.object(wsgi, '_initrp', _initrp), \
mock.patch.object(wsgi, 'get_socket'), \
mock.patch.object(wsgi, 'drop_privileges'), \
mock.patch.object(wsgi, 'loadapp', _loadapp), \
mock.patch.object(wsgi, 'capture_stdio'), \
mock.patch.object(wsgi, 'run_server'):
rc = wsgi.run_wsgi('conf_file', 'app_section')
self.assertEqual(calls['_initrp'], 1)
self.assertEqual(calls['_loadapp'], 0)
self.assertEqual(rc, 1)
self.assertEqual(
["bind_port wasn't properly set in the config file. "
"It must be explicitly set to a valid port number."],
logger.get_lines_for_level('error')
)
do_test('bad')
do_test('80000')
def test_pre_auth_req_with_empty_env_no_path(self):
r = wsgi.make_pre_authed_request(
{}, 'GET')