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

diff --git a/cinder/service.py b/cinder/service.py
index 53530e112bdb564faf4f09523d68e098e357bebc..6a6e34efa08ca6846f227da7c5c4d60277eb5b8e 100755 (executable)
--- 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 9ed0c1675b8a59a038aeb9da616770e8ae0693b0..bc9eb13e3fa044b510d2132fb71d3a3155b129a8 100755 (executable)
--- 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 269ffc8e344554654377cc07f130b4a682bf134d..40f99baf4ad6d1956d442da32e04d95a330c01f7 100644 (file)
--- 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 bcea766ee5ac27fa08ac68e62d99e89fc551df26..2798894b6ec12cff77c789c951189768800c93e5 100644 (file)
--- 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