From 4caca0dfae45dd6c61ddc1b4a2233d2ce1a11685 Mon Sep 17 00:00:00 2001 From: Ankit Agrawal Date: Fri, 4 Jul 2014 03:41:17 -0700 Subject: [PATCH] Cinder-api service throws error on SIGHUP signal Added reset method in WSGIService class. After adding reset method when SIGHUP signal is sent to wsgi service parent process,then it sends SIGHUP signal to all of its child processes. Each child process handles SIGHUP signal by first stopping the service and then calls service start method again. When it stops the service, it kills the eventlet thread, which internally closes the wsgi server socket object. This server socket object is now not usable again and it throws following error, while restarting the service: error: [Errno 9] Bad file descriptor To resolve 'Bad file descriptor' error, creating duplicate socket object, every time service starts. Closes-Bug: #1337796 Change-Id: Iab32a3fe230a11692a8cad274304214247d6c2c6 --- cinder/service.py | 8 ++ cinder/tests/test_service.py | 14 ++++ cinder/tests/test_wsgi.py | 24 +++++- cinder/wsgi.py | 138 +++++++++++++++++++---------------- 4 files changed, 116 insertions(+), 68 deletions(-) diff --git a/cinder/service.py b/cinder/service.py index 53530e112..6a6e34efa 100755 --- a/cinder/service.py +++ b/cinder/service.py @@ -357,6 +357,14 @@ class WSGIService(object): """ self.server.wait() + def reset(self): + """Reset server greenpool size to default. + + :returns: None + + """ + self.server.reset() + def process_launcher(): return service.ProcessLauncher() diff --git a/cinder/tests/test_service.py b/cinder/tests/test_service.py index 9ed0c1675..bc9eb13e3 100755 --- a/cinder/tests/test_service.py +++ b/cinder/tests/test_service.py @@ -217,6 +217,20 @@ class TestWSGIService(test.TestCase): self.assertNotEqual(0, test_service.port) test_service.stop() + def test_reset_pool_size_to_default(self): + test_service = service.WSGIService("test_service") + test_service.start() + + # Stopping the service, which in turn sets pool size to 0 + test_service.stop() + self.assertEqual(test_service.server._pool.size, 0) + + # Resetting pool size to default + test_service.reset() + test_service.start() + self.assertEqual(test_service.server._pool.size, + 1000) + class OSCompatibilityTestCase(test.TestCase): def _test_service_launcher(self, fake_os): diff --git a/cinder/tests/test_wsgi.py b/cinder/tests/test_wsgi.py index 269ffc8e3..40f99baf4 100644 --- a/cinder/tests/test_wsgi.py +++ b/cinder/tests/test_wsgi.py @@ -94,12 +94,12 @@ class TestWSGIServer(test.TestCase): return False def test_no_app(self): - server = cinder.wsgi.Server("test_app", None) + server = cinder.wsgi.Server("test_app", None, + host="127.0.0.1", port=0) self.assertEqual("test_app", server.name) def test_start_random_port(self): server = cinder.wsgi.Server("test_random_port", None, host="127.0.0.1") - self.assertEqual(0, server.port) server.start() self.assertNotEqual(0, server.port) server.stop() @@ -128,7 +128,8 @@ class TestWSGIServer(test.TestCase): start_response('200 OK', [('Content-Type', 'text/plain')]) return [greetings] - server = cinder.wsgi.Server("test_app", hello_world) + server = cinder.wsgi.Server("test_app", hello_world, + host="127.0.0.1", port=0) server.start() response = urllib2.urlopen('http://127.0.0.1:%d/' % server.port) @@ -148,7 +149,9 @@ class TestWSGIServer(test.TestCase): def hello_world(req): return greetings - server = cinder.wsgi.Server("test_app", hello_world) + server = cinder.wsgi.Server("test_app", hello_world, + host="127.0.0.1", port=0) + server.start() response = urllib2.urlopen('https://127.0.0.1:%d/' % server.port) @@ -181,6 +184,19 @@ class TestWSGIServer(test.TestCase): server.stop() + def test_reset_pool_size_to_default(self): + server = cinder.wsgi.Server("test_resize", None, host="127.0.0.1") + server.start() + + # Stopping the server, which in turn sets pool size to 0 + server.stop() + self.assertEqual(server._pool.size, 0) + + # Resetting pool size to default + server.reset() + server.start() + self.assertEqual(server._pool.size, 1000) + class ExceptionTest(test.TestCase): diff --git a/cinder/wsgi.py b/cinder/wsgi.py index bcea766ee..2798894b6 100644 --- a/cinder/wsgi.py +++ b/cinder/wsgi.py @@ -36,8 +36,8 @@ import webob.dec import webob.exc from cinder import exception +from cinder.openstack.common import excutils from cinder.openstack.common import log as logging -from cinder.openstack.common import network_utils from cinder import utils @@ -112,18 +112,15 @@ class Server(object): self._server = None self._socket = None self._protocol = protocol - self._pool = eventlet.GreenPool(pool_size or self.default_pool_size) + self.pool_size = pool_size or self.default_pool_size + self._pool = eventlet.GreenPool(self.pool_size) self._logger = logging.getLogger("eventlet.wsgi.server") self._wsgi_logger = logging.WritableLogger(self._logger) if backlog < 1: raise exception.InvalidInput( reason='The backlog must be more than 1') - self._socket = self._get_socket(self._host, - self._port, - backlog=backlog) - def _get_socket(self, host, port, backlog): bind_addr = (host, port) # TODO(dims): eventlet's green dns/socket module does not actually # support IPv6 in getaddrinfo(). We need to get around this in the @@ -141,90 +138,95 @@ class Server(object): cert_file = CONF.ssl_cert_file key_file = CONF.ssl_key_file ca_file = CONF.ssl_ca_file - use_ssl = cert_file or key_file + self._use_ssl = cert_file or key_file if cert_file and not os.path.exists(cert_file): - raise RuntimeError(_("Unable to find cert_file : %s") % cert_file) + raise RuntimeError(_("Unable to find cert_file : %s") + % cert_file) if ca_file and not os.path.exists(ca_file): raise RuntimeError(_("Unable to find ca_file : %s") % ca_file) if key_file and not os.path.exists(key_file): - raise RuntimeError(_("Unable to find key_file : %s") % key_file) + raise RuntimeError(_("Unable to find key_file : %s") + % key_file) - if use_ssl and (not cert_file or not key_file): - raise RuntimeError(_("When running server in SSL mode, you must " - "specify both a cert_file and key_file " - "option value in your configuration file")) + if self._use_ssl and (not cert_file or not key_file): + raise RuntimeError(_("When running server in SSL mode, you " + "must specify both a cert_file and " + "key_file option value in your " + "configuration file.")) - def wrap_ssl(sock): - ssl_kwargs = { - 'server_side': True, - 'certfile': cert_file, - 'keyfile': key_file, - 'cert_reqs': ssl.CERT_NONE, - } - - if CONF.ssl_ca_file: - ssl_kwargs['ca_certs'] = ca_file - ssl_kwargs['cert_reqs'] = ssl.CERT_REQUIRED - - return ssl.wrap_socket(sock, **ssl_kwargs) - - sock = None retry_until = time.time() + 30 - while not sock and time.time() < retry_until: + while not self._socket and time.time() < retry_until: try: - sock = eventlet.listen(bind_addr, - backlog=backlog, - family=family) - if use_ssl: - sock = wrap_ssl(sock) - + self._socket = eventlet.listen(bind_addr, backlog=backlog, + family=family) except socket.error as err: if err.args[0] != errno.EADDRINUSE: raise eventlet.sleep(0.1) - if not sock: + + if not self._socket: raise RuntimeError(_("Could not bind to %(host)s:%(port)s " "after trying for 30 seconds") % {'host': host, 'port': port}) - sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - # NOTE(praneshp): Call set_tcp_keepalive in oslo to set - # tcp keepalive parameters. Sockets can hang around forever - # without keepalive - network_utils.set_tcp_keepalive(sock, - CONF.tcp_keepalive, - CONF.tcp_keepidle, - CONF.tcp_keepalive_count, - CONF.tcp_keepalive_interval) - return sock - - def _start(self): - """Run the blocking eventlet WSGI server. - - :returns: None - - """ - eventlet.wsgi.server(self._socket, - self.app, - protocol=self._protocol, - custom_pool=self._pool, - log=self._wsgi_logger) + (self._host, self._port) = self._socket.getsockname()[0:2] + LOG.info(_("%(name)s listening on %(_host)s:%(_port)s") + % self.__dict__) - def start(self, backlog=128): + def start(self): """Start serving a WSGI application. - :param backlog: Maximum number of queued connections. :returns: None :raises: cinder.exception.InvalidInput """ - self._server = eventlet.spawn(self._start) - (self._host, self._port) = self._socket.getsockname()[0:2] - LOG.info(_("Started %(name)s on %(host)s:%(port)s") % - {'name': self.name, 'host': self.host, 'port': self.port}) + # The server socket object will be closed after server exits, + # but the underlying file descriptor will remain open, and will + # give bad file descriptor error. So duplicating the socket object, + # to keep file descriptor usable. + + dup_socket = self._socket.dup() + if self._use_ssl: + try: + ssl_kwargs = { + 'server_side': True, + 'certfile': CONF.ssl_cert_file, + 'keyfile': CONF.ssl_key_file, + 'cert_reqs': ssl.CERT_NONE, + } + + if CONF.ssl_ca_file: + ssl_kwargs['ca_certs'] = CONF.ssl_ca_file + ssl_kwargs['cert_reqs'] = ssl.CERT_REQUIRED + + dup_socket = ssl.wrap_socket(dup_socket, + **ssl_kwargs) + + dup_socket.setsockopt(socket.SOL_SOCKET, + socket.SO_REUSEADDR, 1) + + # sockets can hang around forever without keepalive + dup_socket.setsockopt(socket.SOL_SOCKET, + socket.SO_KEEPALIVE, 1) + + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_("Failed to start %(name)s on %(_host)s:" + "%(_port)s with SSL support.") % self.__dict__) + + wsgi_kwargs = { + 'func': eventlet.wsgi.server, + 'sock': dup_socket, + 'site': self.app, + 'protocol': self._protocol, + 'custom_pool': self._pool, + 'log': self._wsgi_logger + } + + self._server = eventlet.spawn(**wsgi_kwargs) @property def host(self): @@ -263,6 +265,14 @@ class Server(object): except greenlet.GreenletExit: LOG.info(_("WSGI server has stopped.")) + def reset(self): + """Reset server greenpool size to default. + + :returns: None + + """ + self._pool.resize(self.pool_size) + class Request(webob.Request): pass -- 2.45.2