Fix for memcache middleware configuration

The documentation rightly said to use "memcache_max_connections", but
the code was looking for "max_connections", and only looking for it in
proxy-server.conf, not in memcache.conf as a fall back.

This commit brings the code coverage for the memcache middleware to
100%.

Closes-Bug: 1252893
Change-Id: I6ea64baa2f961a09d60b977b40d5baf842449ece
Signed-off-by: Peter Portante <peter.portante@redhat.com>

etc/memcache.conf-sample

@@ -13,3 +13,6 @@
 # set to 2 and reload.
 # In the future, the ability to use pickle serialization will be removed.
 # memcache_serialization_support = 2
+#
+# Sets the maximum number of connections to each memcached server per worker
+# memcache_max_connections = 2

etc/proxy-server.conf-sample

@@ -298,6 +298,9 @@ use = egg:swift#memcache
 # set to 2 and reload.
 # In the future, the ability to use pickle serialization will be removed.
 # memcache_serialization_support = 2
+#
+# Sets the maximum number of connections to each memcached server per worker
+# memcache_max_connections = 2
 
 [filter:ratelimit]
 use = egg:swift#ratelimit

swift/common/middleware/memcache.py

@@ -28,9 +28,17 @@ class MemcacheMiddleware(object):
         self.app = app
         self.memcache_servers = conf.get('memcache_servers')
         serialization_format = conf.get('memcache_serialization_support')
-        max_conns = int(conf.get('max_connections', 2))
+        try:
+            # Originally, while we documented using memcache_max_connections
+            # we only accepted max_connections
+            max_conns = int(conf.get('memcache_max_connections',
+                                     conf.get('max_connections', 0)))
+        except ValueError:
+            max_conns = 0
 
-        if not self.memcache_servers or serialization_format is None:
+        if (not self.memcache_servers
+                or serialization_format is None
+                or max_conns <= 0):
             path = os.path.join(conf.get('swift_dir', '/etc/swift'),
                                 'memcache.conf')
             memcache_conf = ConfigParser()
@@ -48,9 +56,19 @@ class MemcacheMiddleware(object):
                                               'memcache_serialization_support')
                     except (NoSectionError, NoOptionError):
                         pass
+                if max_conns <= 0:
+                    try:
+                        new_max_conns = \
+                            memcache_conf.get('memcache',
+                                              'memcache_max_connections')
+                        max_conns = int(new_max_conns)
+                    except (NoSectionError, NoOptionError, ValueError):
+                        pass
 
         if not self.memcache_servers:
             self.memcache_servers = '127.0.0.1:11211'
+        if max_conns <= 0:
+            max_conns = 2
         if serialization_format is None:
             serialization_format = 2
         else:

test/unit/common/middleware/test_memcache.py

@@ -38,21 +38,41 @@ class EmptyConfigParser(object):
         return False
 
 
-class SetConfigParser(object):
+def get_config_parser(memcache_servers='1.2.3.4:5',
+                      memcache_serialization_support='1',
+                      memcache_max_connections='4',
+                      section='memcache'):
+    _srvs = memcache_servers
+    _sers = memcache_serialization_support
+    _maxc = memcache_max_connections
+    _section = section
 
-    def read(self, path):
+    class SetConfigParser(object):
-        return True
 
-    def get(self, section, option):
+        def read(self, path):
-        if section == 'memcache':
+            return True
-            if option == 'memcache_servers':
+
-                return '1.2.3.4:5'
+        def get(self, section, option):
-            elif option == 'memcache_serialization_support':
+            if _section == section:
-                return '1'
+                if option == 'memcache_servers':
+                    if _srvs == 'error':
+                        raise NoOptionError(option, section)
+                    return _srvs
+                elif option == 'memcache_serialization_support':
+                    if _sers == 'error':
+                        raise NoOptionError(option, section)
+                    return _sers
+                elif option in ('memcache_max_connections',
+                                'max_connections'):
+                    if _maxc == 'error':
+                        raise NoOptionError(option, section)
+                    return _maxc
+                else:
+                    raise NoOptionError(option, section)
             else:
-                raise NoOptionError(option)
+                raise NoSectionError(option)
-        else:
+
-            raise NoSectionError(option)
+    return SetConfigParser
 
 
 def start_response(*args):
@@ -73,15 +93,29 @@ class TestCacheMiddleware(unittest.TestCase):
     def test_conf_default_read(self):
         orig_parser = memcache.ConfigParser
         memcache.ConfigParser = ExcConfigParser
-        exc = None
+        count = 0
         try:
-            memcache.MemcacheMiddleware(FakeApp(), {})
+            for d in ({},
-        except Exception as err:
+                      {'memcache_servers': '6.7.8.9:10'},
-            exc = err
+                      {'memcache_serialization_support': '0'},
+                      {'memcache_max_connections': '30'},
+                      {'memcache_servers': '6.7.8.9:10',
+                       'memcache_serialization_support': '0'},
+                      {'memcache_servers': '6.7.8.9:10',
+                       'memcache_max_connections': '30'},
+                      {'memcache_serialization_support': '0',
+                       'memcache_max_connections': '30'}
+                      ):
+                try:
+                    memcache.MemcacheMiddleware(FakeApp(), d)
+                except Exception as err:
+                    self.assertEquals(
+                        str(err),
+                        "read called with '/etc/swift/memcache.conf'")
+                    count += 1
         finally:
             memcache.ConfigParser = orig_parser
-        self.assertEquals(str(exc),
+        self.assertEquals(count, 7)
-                          "read called with '/etc/swift/memcache.conf'")
 
     def test_conf_set_no_read(self):
         orig_parser = memcache.ConfigParser
@@ -90,7 +124,8 @@ class TestCacheMiddleware(unittest.TestCase):
         try:
             memcache.MemcacheMiddleware(
                 FakeApp(), {'memcache_servers': '1.2.3.4:5',
-                            'memcache_serialization_support': '2'})
+                            'memcache_serialization_support': '2',
+                            'memcache_max_connections': '30'})
         except Exception as err:
             exc = err
         finally:
@@ -107,10 +142,91 @@ class TestCacheMiddleware(unittest.TestCase):
         self.assertEquals(app.memcache_servers, '127.0.0.1:11211')
         self.assertEquals(app.memcache._allow_pickle, False)
         self.assertEquals(app.memcache._allow_unpickle, False)
+        self.assertEquals(
+            app.memcache._client_cache['127.0.0.1:11211'].max_size, 2)
+
+    def test_conf_inline(self):
+        orig_parser = memcache.ConfigParser
+        memcache.ConfigParser = get_config_parser()
+        try:
+            app = memcache.MemcacheMiddleware(
+                FakeApp(),
+                {'memcache_servers': '6.7.8.9:10',
+                 'memcache_serialization_support': '0',
+                 'memcache_max_connections': '5'})
+        finally:
+            memcache.ConfigParser = orig_parser
+        self.assertEquals(app.memcache_servers, '6.7.8.9:10')
+        self.assertEquals(app.memcache._allow_pickle, True)
+        self.assertEquals(app.memcache._allow_unpickle, True)
+        self.assertEquals(
+            app.memcache._client_cache['6.7.8.9:10'].max_size, 5)
+
+    def test_conf_extra_no_section(self):
+        orig_parser = memcache.ConfigParser
+        memcache.ConfigParser = get_config_parser(section='foobar')
+        try:
+            app = memcache.MemcacheMiddleware(FakeApp(), {})
+        finally:
+            memcache.ConfigParser = orig_parser
+        self.assertEquals(app.memcache_servers, '127.0.0.1:11211')
+        self.assertEquals(app.memcache._allow_pickle, False)
+        self.assertEquals(app.memcache._allow_unpickle, False)
+        self.assertEquals(
+            app.memcache._client_cache['127.0.0.1:11211'].max_size, 2)
+
+    def test_conf_extra_no_option(self):
+        orig_parser = memcache.ConfigParser
+        memcache.ConfigParser = get_config_parser(
+            memcache_servers='error', memcache_serialization_support='error',
+            memcache_max_connections='error')
+        try:
+            app = memcache.MemcacheMiddleware(FakeApp(), {})
+        finally:
+            memcache.ConfigParser = orig_parser
+        self.assertEquals(app.memcache_servers, '127.0.0.1:11211')
+        self.assertEquals(app.memcache._allow_pickle, False)
+        self.assertEquals(app.memcache._allow_unpickle, False)
+        self.assertEquals(
+            app.memcache._client_cache['127.0.0.1:11211'].max_size, 2)
+
+    def test_conf_inline_other_max_conn(self):
+        orig_parser = memcache.ConfigParser
+        memcache.ConfigParser = get_config_parser()
+        try:
+            app = memcache.MemcacheMiddleware(
+                FakeApp(),
+                {'memcache_servers': '6.7.8.9:10',
+                 'memcache_serialization_support': '0',
+                 'max_connections': '5'})
+        finally:
+            memcache.ConfigParser = orig_parser
+        self.assertEquals(app.memcache_servers, '6.7.8.9:10')
+        self.assertEquals(app.memcache._allow_pickle, True)
+        self.assertEquals(app.memcache._allow_unpickle, True)
+        self.assertEquals(
+            app.memcache._client_cache['6.7.8.9:10'].max_size, 5)
+
+    def test_conf_inline_bad_max_conn(self):
+        orig_parser = memcache.ConfigParser
+        memcache.ConfigParser = get_config_parser()
+        try:
+            app = memcache.MemcacheMiddleware(
+                FakeApp(),
+                {'memcache_servers': '6.7.8.9:10',
+                 'memcache_serialization_support': '0',
+                 'max_connections': 'bad42'})
+        finally:
+            memcache.ConfigParser = orig_parser
+        self.assertEquals(app.memcache_servers, '6.7.8.9:10')
+        self.assertEquals(app.memcache._allow_pickle, True)
+        self.assertEquals(app.memcache._allow_unpickle, True)
+        self.assertEquals(
+            app.memcache._client_cache['6.7.8.9:10'].max_size, 4)
 
     def test_conf_from_extra_conf(self):
         orig_parser = memcache.ConfigParser
-        memcache.ConfigParser = SetConfigParser
+        memcache.ConfigParser = get_config_parser()
         try:
             app = memcache.MemcacheMiddleware(FakeApp(), {})
         finally:
@@ -118,21 +234,66 @@ class TestCacheMiddleware(unittest.TestCase):
         self.assertEquals(app.memcache_servers, '1.2.3.4:5')
         self.assertEquals(app.memcache._allow_pickle, False)
         self.assertEquals(app.memcache._allow_unpickle, True)
+        self.assertEquals(
+            app.memcache._client_cache['1.2.3.4:5'].max_size, 4)
 
-    def test_conf_from_inline_conf(self):
+    def test_conf_from_extra_conf_bad_max_conn(self):
         orig_parser = memcache.ConfigParser
-        memcache.ConfigParser = SetConfigParser
+        memcache.ConfigParser = get_config_parser(
+            memcache_max_connections='bad42')
+        try:
+            app = memcache.MemcacheMiddleware(FakeApp(), {})
+        finally:
+            memcache.ConfigParser = orig_parser
+        self.assertEquals(app.memcache_servers, '1.2.3.4:5')
+        self.assertEquals(app.memcache._allow_pickle, False)
+        self.assertEquals(app.memcache._allow_unpickle, True)
+        self.assertEquals(
+            app.memcache._client_cache['1.2.3.4:5'].max_size, 2)
+
+    def test_conf_from_inline_and_maxc_from_extra_conf(self):
+        orig_parser = memcache.ConfigParser
+        memcache.ConfigParser = get_config_parser()
         try:
             app = memcache.MemcacheMiddleware(
                 FakeApp(),
                 {'memcache_servers': '6.7.8.9:10',
-                 'serialization_format': '0'})
+                 'memcache_serialization_support': '0'})
+        finally:
+            memcache.ConfigParser = orig_parser
+        self.assertEquals(app.memcache_servers, '6.7.8.9:10')
+        self.assertEquals(app.memcache._allow_pickle, True)
+        self.assertEquals(app.memcache._allow_unpickle, True)
+        self.assertEquals(
+            app.memcache._client_cache['6.7.8.9:10'].max_size, 4)
+
+    def test_conf_from_inline_and_sers_from_extra_conf(self):
+        orig_parser = memcache.ConfigParser
+        memcache.ConfigParser = get_config_parser()
+        try:
+            app = memcache.MemcacheMiddleware(
+                FakeApp(),
+                {'memcache_servers': '6.7.8.9:10',
+                 'memcache_max_connections': '42'})
         finally:
             memcache.ConfigParser = orig_parser
         self.assertEquals(app.memcache_servers, '6.7.8.9:10')
         self.assertEquals(app.memcache._allow_pickle, False)
         self.assertEquals(app.memcache._allow_unpickle, True)
+        self.assertEquals(
+            app.memcache._client_cache['6.7.8.9:10'].max_size, 42)
 
+    def test_filter_factory(self):
+        factory = memcache.filter_factory({'max_connections': '3'},
+                                          memcache_servers='10.10.10.10:10',
+                                          memcache_serialization_support='1')
+        thefilter = factory('myapp')
+        self.assertEquals(thefilter.app, 'myapp')
+        self.assertEquals(thefilter.memcache_servers, '10.10.10.10:10')
+        self.assertEquals(thefilter.memcache._allow_pickle, False)
+        self.assertEquals(thefilter.memcache._allow_unpickle, True)
+        self.assertEquals(
+            thefilter.memcache._client_cache['10.10.10.10:10'].max_size, 3)
 
 if __name__ == '__main__':
     unittest.main()