From c46f29278d6a916416948d4120489b429f8bf460 Mon Sep 17 00:00:00 2001
From: Morgan Fainberg <morgan.fainberg@gmail.com>
Date: Wed, 18 Jul 2018 11:26:18 -0700
Subject: [PATCH] Fix KeystoneMiddleware memcachepool abstraction

Keystonemiddleware's abstraction for the memcache pool was broken
when converting to use a queue.Queue. The logic that placed the
connection back into the pool was moved to .acquire and the reserve
method was not using acquire.

Change-Id: I0eda5981cbb661f63790258cf8e70c7340615159
Closes-Bug: #1782404
---
 keystonemiddleware/auth_token/_cache.py       |  6 ++++-
 .../tests/unit/auth_token/test_cache.py       | 23 +++++++++++++++++++
 .../notes/bug-1782404-c4e37bbc83756a89.yaml   |  8 +++++++
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/bug-1782404-c4e37bbc83756a89.yaml

diff --git a/keystonemiddleware/auth_token/_cache.py b/keystonemiddleware/auth_token/_cache.py
index c148349a..8951b375 100644
--- a/keystonemiddleware/auth_token/_cache.py
+++ b/keystonemiddleware/auth_token/_cache.py
@@ -98,7 +98,11 @@ class _MemcacheClientPool(object):
 
     @contextlib.contextmanager
     def reserve(self):
-        yield self._pool.get()
+        # NOTE(morgan): We must use "acquire" if we want all the added context
+        # manager logic that places the connection back into the pool at the
+        # end of it's use.
+        with self._pool.acquire() as client:
+            yield client
 
 
 class TokenCache(object):
diff --git a/keystonemiddleware/tests/unit/auth_token/test_cache.py b/keystonemiddleware/tests/unit/auth_token/test_cache.py
index 52e657cb..eb537a63 100644
--- a/keystonemiddleware/tests/unit/auth_token/test_cache.py
+++ b/keystonemiddleware/tests/unit/auth_token/test_cache.py
@@ -12,8 +12,10 @@
 
 import uuid
 
+import fixtures
 import six
 
+from keystonemiddleware.auth_token import _cache
 from keystonemiddleware.auth_token import _exceptions as exc
 from keystonemiddleware.tests.unit.auth_token import base
 from keystonemiddleware.tests.unit import utils
@@ -165,3 +167,24 @@ class TestLiveMemcache(base.BaseAuthTokenTestCase):
 
         token_cache.set(token, data)
         self.assertEqual(token_cache.get(token), data)
+
+
+class TestMemcachePoolAbstraction(utils.TestCase):
+    def setUp(self):
+        super(TestMemcachePoolAbstraction, self).setUp()
+        self.useFixture(fixtures.MockPatch(
+            'oslo_cache._memcache_pool._MemcacheClient'))
+
+    def test_abstraction_layer_reserve_places_connection_back_in_pool(self):
+        cache_pool = _cache._MemcacheClientPool(
+            memcache_servers=[], arguments={}, maxsize=1, unused_timeout=10)
+        conn = None
+        with cache_pool.reserve() as client:
+            self.assertEqual(cache_pool._pool._acquired, 1)
+            conn = client
+
+        self.assertEqual(cache_pool._pool._acquired, 0)
+        with cache_pool.reserve() as client:
+            # Make sure the connection we got before is in-fact the one we
+            # get again.
+            self.assertEqual(conn, client)
diff --git a/releasenotes/notes/bug-1782404-c4e37bbc83756a89.yaml b/releasenotes/notes/bug-1782404-c4e37bbc83756a89.yaml
new file mode 100644
index 00000000..852685c8
--- /dev/null
+++ b/releasenotes/notes/bug-1782404-c4e37bbc83756a89.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - >
+    [`bug 1782404 <https://bugs.launchpad.net/keystonemiddleware/+bug/1782404>`_]
+    Keystonemiddleware incorrectly implemented an abstraction for the memcache
+    client pool that utilized a `queue.Queue` `get` method instead of the
+    supplied `acquire()` context manager. The `acquire()` context manager
+    properly places the client connection back into the pool after `__exit__`.