From: Jay S. Bryant <jsbryant@us.ibm.com>
Date: Fri, 19 Sep 2014 17:46:21 +0000 (-0500)
Subject: Fix unnecessary WSGI worker warning at API startup
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3d301346a7100e52fe0c319c1c1b5b81a3bc1660;p=openstack-build%2Fcinder-build.git

Fix unnecessary WSGI worker warning at API startup

There was a bug in WSGIService in the way that it was
checking the osapi_volume_workers option.  It was using
getattr() to see if the option was set, if not it was supposed
to set the value to processutils.get_worker_count().  This,
however, never happened because getattr interpreted the default
'None' value to be a value.  So, on any system with no value set
the self.workers < 1 check would be hit and a warning would be
output.

Nova had changed their approach to this option to avoid this
problem.  This patch pulls Nova's approach into Cinder for
consistency.  Cinder will now use processutils.get_worker_count()
if no option is set in /etc/cinder/cinder.conf and when the user sets
osapi_volume_workers to 0.  A negative value will cause an
InvalidInput exception to be thrown.

Unittests have been added for this functionality.

Change-Id: I4ec2fdd0d19195cccffd63cdd1af1b9ca9884c7d
Closes-bug: #1367454
---

diff --git a/cinder/service.py b/cinder/service.py
index 230e2834e..272ab7be4 100644
--- a/cinder/service.py
+++ b/cinder/service.py
@@ -328,16 +328,17 @@ class WSGIService(object):
         self.app = self.loader.load_app(name)
         self.host = getattr(CONF, '%s_listen' % name, "0.0.0.0")
         self.port = getattr(CONF, '%s_listen_port' % name, 0)
-        self.workers = getattr(CONF, '%s_workers' % name,
-                               processutils.get_worker_count())
+        self.workers = (getattr(CONF, '%s_workers' % name, None) or
+                        processutils.get_worker_count())
+        if self.workers and self.workers < 1:
+            worker_name = '%s_workers' % name
+            msg = (_("%(worker_name)s value of %(workers)d is invalid, "
+                     "must be greater than 0.") %
+                   {'worker_name': worker_name,
+                    'workers': self.workers})
+            raise exception.InvalidInput(msg)
         setup_profiler(name, self.host)
 
-        if self.workers < 1:
-            LOG.warn(_("Value of config option %(name)s_workers must be "
-                       "integer greater than 1.  Input value ignored.") %
-                     {'name': name})
-            # Reset workers to default
-            self.workers = processutils.get_worker_count()
         self.server = wsgi.Server(name,
                                   self.app,
                                   host=self.host,
diff --git a/cinder/tests/test_service.py b/cinder/tests/test_service.py
index bc9eb13e3..1b8109012 100644
--- a/cinder/tests/test_service.py
+++ b/cinder/tests/test_service.py
@@ -28,6 +28,7 @@ from cinder import context
 from cinder import db
 from cinder import exception
 from cinder import manager
+from cinder.openstack.common import processutils
 from cinder import service
 from cinder import test
 from cinder import wsgi
@@ -231,6 +232,27 @@ class TestWSGIService(test.TestCase):
         self.assertEqual(test_service.server._pool.size,
                          1000)
 
+    def test_workers_set_default(self):
+        test_service = service.WSGIService("osapi_volume")
+        self.assertEqual(test_service.workers, processutils.get_worker_count())
+
+    def test_workers_set_good_user_setting(self):
+        CONF.set_override('osapi_volume_workers', 8)
+        test_service = service.WSGIService("osapi_volume")
+        self.assertEqual(test_service.workers, 8)
+
+    def test_workers_set_zero_user_setting(self):
+        CONF.set_override('osapi_volume_workers', 0)
+        test_service = service.WSGIService("osapi_volume")
+        # If a value less than 1 is used, defaults to number of procs available
+        self.assertEqual(test_service.workers, processutils.get_worker_count())
+
+    def test_workers_set_negative_user_setting(self):
+        CONF.set_override('osapi_volume_workers', -1)
+        self.assertRaises(exception.InvalidInput,
+                          service.WSGIService,
+                          "osapi_volume")
+
 
 class OSCompatibilityTestCase(test.TestCase):
     def _test_service_launcher(self, fake_os):