From 31b311af57e495bb975d83a4fc1037f7c55b0d83 Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Fri, 22 Nov 2013 18:57:44 -0700 Subject: [PATCH] Return an exit code for configuration errors Red Hat's QA noticed that in case of the infamous "xattr>=0.4" error, swift-init exits with a zero error code, which confuses startup scripts (not Systemd though -- that one knows exactly if processes run or not). The easiest fix is to return the error code like Guido's blog post suggested. Change-Id: I7badd8742375a7cb2aa5606277316477b8083f8d Fixes: rhbz#1020480 --- bin/swift-account-server | 4 ++- bin/swift-container-server | 4 ++- bin/swift-object-server | 6 +++-- bin/swift-proxy-server | 3 ++- swift/common/wsgi.py | 8 +++--- test/unit/common/test_wsgi.py | 47 +++++++++++++++++++++++++++++++++++ 6 files changed, 64 insertions(+), 8 deletions(-) diff --git a/bin/swift-account-server b/bin/swift-account-server index 9b8f1e58ab..a1f48e16d3 100755 --- a/bin/swift-account-server +++ b/bin/swift-account-server @@ -14,9 +14,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys from swift.common.utils import parse_options from swift.common.wsgi import run_wsgi if __name__ == '__main__': conf_file, options = parse_options() - run_wsgi(conf_file, 'account-server', default_port=6002, **options) + sys.exit(run_wsgi(conf_file, + 'account-server', default_port=6002, **options)) diff --git a/bin/swift-container-server b/bin/swift-container-server index aef038ca65..ffd6840f94 100755 --- a/bin/swift-container-server +++ b/bin/swift-container-server @@ -14,9 +14,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys from swift.common.utils import parse_options from swift.common.wsgi import run_wsgi if __name__ == '__main__': conf_file, options = parse_options() - run_wsgi(conf_file, 'container-server', default_port=6001, **options) + sys.exit(run_wsgi(conf_file, + 'container-server', default_port=6001, **options)) diff --git a/bin/swift-object-server b/bin/swift-object-server index d879684b1c..a3306b2d8a 100755 --- a/bin/swift-object-server +++ b/bin/swift-object-server @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys from swift.common.utils import parse_options from swift.common.wsgi import run_wsgi from swift.obj import server @@ -21,5 +22,6 @@ from swift.obj import server if __name__ == '__main__': conf_file, options = parse_options() - run_wsgi(conf_file, 'object-server', default_port=6000, - global_conf_callback=server.global_conf_callback, **options) + sys.exit(run_wsgi(conf_file, 'object-server', default_port=6000, + global_conf_callback=server.global_conf_callback, + **options)) diff --git a/bin/swift-proxy-server b/bin/swift-proxy-server index 449a8d8285..c6846bcf3e 100755 --- a/bin/swift-proxy-server +++ b/bin/swift-proxy-server @@ -14,9 +14,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys from swift.common.utils import parse_options from swift.common.wsgi import run_wsgi if __name__ == '__main__': conf_file, options = parse_options() - run_wsgi(conf_file, 'proxy-server', default_port=8080, **options) + sys.exit(run_wsgi(conf_file, 'proxy-server', default_port=8080, **options)) diff --git a/swift/common/wsgi.py b/swift/common/wsgi.py index 193322f2fa..c730c1c92a 100644 --- a/swift/common/wsgi.py +++ b/swift/common/wsgi.py @@ -240,6 +240,7 @@ def run_wsgi(conf_path, app_section, *args, **kwargs): :param conf_path: Path to paste.deploy style configuration file/directory :param app_section: App name from conf file to load config from + :returns: 0 if successful, nonzero otherwise """ # Load configuration, Set logger and Load request processor try: @@ -247,7 +248,7 @@ def run_wsgi(conf_path, app_section, *args, **kwargs): _initrp(conf_path, app_section, *args, **kwargs) except ConfigFileError as e: print e - return + return 1 # bind to address and port sock = get_socket(conf, default_port=kwargs.get('default_port', 8080)) @@ -272,7 +273,7 @@ def run_wsgi(conf_path, app_section, *args, **kwargs): # Useful for profiling [no forks]. if worker_count == 0: run_server(conf, logger, sock, global_conf=global_conf) - return + return 0 def kill_children(*args): """Kills the entire process group.""" @@ -299,7 +300,7 @@ def run_wsgi(conf_path, app_section, *args, **kwargs): signal.signal(signal.SIGTERM, signal.SIG_DFL) run_server(conf, logger, sock) logger.notice('Child %d exiting normally' % os.getpid()) - return + return 0 else: logger.notice('Started child %s' % pid) children.append(pid) @@ -317,6 +318,7 @@ def run_wsgi(conf_path, app_section, *args, **kwargs): greenio.shutdown_safe(sock) sock.close() logger.notice('Exited') + return 0 class ConfigFileError(Exception): diff --git a/test/unit/common/test_wsgi.py b/test/unit/common/test_wsgi.py index 085364c267..a3e7323def 100644 --- a/test/unit/common/test_wsgi.py +++ b/test/unit/common/test_wsgi.py @@ -536,6 +536,53 @@ class TestWSGI(unittest.TestCase): self.assertEqual(calls['_global_conf_callback'], 1) self.assertEqual(calls['_loadapp'], 1) + def test_run_server_success(self): + calls = defaultdict(lambda: 0) + + def _initrp(conf_file, app_section, *args, **kwargs): + calls['_initrp'] += 1 + return ( + {'__file__': 'test', 'workers': 0}, + 'logger', + 'log_name') + + def _loadapp(uri, name=None, **kwargs): + calls['_loadapp'] += 1 + + with nested( + 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'], 1) + self.assertEqual(rc, 0) + + def test_run_server_failure1(self): + calls = defaultdict(lambda: 0) + + def _initrp(conf_file, app_section, *args, **kwargs): + calls['_initrp'] += 1 + raise wsgi.ConfigFileError('test exception') + + def _loadapp(uri, name=None, **kwargs): + calls['_loadapp'] += 1 + + with nested( + 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) + def test_pre_auth_req_with_empty_env_no_path(self): r = wsgi.make_pre_authed_request( {}, 'GET')