cache_utils: fixed cache misses for the new (oslo.cache) configuration

When the new (oslo.cache) way of configuring the cache is used, cache is
never hit, because self._cache.get() consistently raises exceptions:

TypeError: 'sha1() argument 1 must be string or buffer, not tuple'

It occurs because the key passed into the oslo.cache region does not
conform to oslo.cache requirements. The library enforces the key to be
compatible with sha1_mangle_key() function:

http://git.openstack.org/cgit/openstack/oslo.cache/tree/oslo_cache/core.py?id=8b8a718507b30a4a2fd36e6c14d1071bd6cca878#n140

With this patch, we transform the key to a string, to conform to the
requirements.

The bug sneaked into the tree unnoticed because of two reasons:

- there were no unit tests to validate the new way of cache
  configuration.
- the 'legacy' code path was configuring the cache in a slightly
  different way, omitting some oslo.cache code.

For the former, new unit tests were introduced that cover the cache on
par with the legacy mode.

For the latter, the legacy code path was modified to rely on the same
configuration path as for the new way.

Closes-Bug: #1593342
Change-Id: I2724aa21f66f0fb69147407bfcf3184585d7d5cd

neutron/common/cache_utils.py

@@ -65,7 +65,6 @@ def _get_cache_region_for_legacy(url):
     backend = parsed.scheme
 
     if backend == 'memory':
-        backend = 'oslo_cache.dict'
         query = parsed.query
         # NOTE(fangzhen): The following NOTE and code is from legacy
         # oslo-incubator cache module. Previously reside in neutron at
@@ -78,11 +77,17 @@ def _get_cache_region_for_legacy(url):
         if not query and '?' in parsed.path:
             query = parsed.path.split('?', 1)[-1]
         parameters = parse.parse_qs(query)
-        expiration_time = int(parameters.get('default_ttl', [0])[0])
 
-        region = cache.create_region()
-        region.configure(backend, expiration_time=expiration_time)
-        return region
+        conf = cfg.ConfigOpts()
+        register_oslo_configs(conf)
+        cache_conf_dict = {
+            'enabled': True,
+            'backend': 'oslo_cache.dict',
+            'expiration_time': int(parameters.get('default_ttl', [0])[0]),
+        }
+        for k, v in cache_conf_dict.items():
+            conf.set_override(k, v, group='cache')
+        return _get_cache_region(conf)
     else:
         raise RuntimeError(_('Old style configuration can use only memory '
                              '(dict) backend'))
@@ -108,6 +113,8 @@ class cache_method_results(object):
         key = (func_name,) + args
         if kwargs:
             key += utils.dict2tuple(kwargs)
+        # oslo.cache expects a string or a buffer
+        key = str(key)
         try:
             item = target_self._cache.get(key)
         except TypeError:

neutron/tests/unit/agent/metadata/test_agent.py

@@ -51,6 +51,16 @@ class CacheConfFixture(ConfFixture):
         self.config(cache_url='memory://?default_ttl=5')
 
 
+class NewCacheConfFixture(ConfFixture):
+    def setUp(self):
+        super(NewCacheConfFixture, self).setUp()
+        self.config(
+            group='cache',
+            enabled=True,
+            backend='oslo_cache.dict',
+            expiration_time=5)
+
+
 class TestMetadataProxyHandlerBase(base.BaseTestCase):
     fake_conf = cfg.CONF
     fake_conf_fixture = ConfFixture(fake_conf)
@@ -96,9 +106,7 @@ class TestMetadataProxyHandlerRpc(TestMetadataProxyHandlerBase):
         self.assertEqual(expected, ports)
 
 
-class TestMetadataProxyHandlerCache(TestMetadataProxyHandlerBase):
-    fake_conf = cfg.CONF
-    fake_conf_fixture = CacheConfFixture(fake_conf)
+class _TestMetadataProxyHandlerCacheMixin(object):
 
     def test_call(self):
         req = mock.Mock()
@@ -411,6 +419,18 @@ class TestMetadataProxyHandlerCache(TestMetadataProxyHandlerBase):
         )
 
 
+class TestMetadataProxyHandlerCache(TestMetadataProxyHandlerBase,
+                                    _TestMetadataProxyHandlerCacheMixin):
+    fake_conf = cfg.CONF
+    fake_conf_fixture = CacheConfFixture(fake_conf)
+
+
+class TestMetadataProxyHandlerNewCache(TestMetadataProxyHandlerBase,
+                                       _TestMetadataProxyHandlerCacheMixin):
+    fake_conf = cfg.CONF
+    fake_conf_fixture = NewCacheConfFixture(fake_conf)
+
+
 class TestMetadataProxyHandlerNoCache(TestMetadataProxyHandlerCache):
     fake_conf = cfg.CONF
     fake_conf_fixture = ConfFixture(fake_conf)

neutron/tests/unit/common/test_cache_utils.py

@@ -100,7 +100,7 @@ class TestCachingDecorator(base.BaseTestCase):
         self.decor._cache.get.return_value = self.not_cached
         retval = self.decor.func(*args, **kwargs)
         self.decor._cache.set.assert_called_once_with(
-            expected_key, self.decor.func_retval)
+            str(expected_key), self.decor.func_retval)
         self.assertEqual(self.decor.func_retval, retval)
 
     def test_cache_hit(self):
@@ -110,7 +110,7 @@ class TestCachingDecorator(base.BaseTestCase):
         retval = self.decor.func(*args, **kwargs)
         self.assertFalse(self.decor._cache.set.called)
         self.assertEqual(self.decor._cache.get.return_value, retval)
-        self.decor._cache.get.assert_called_once_with(expected_key)
+        self.decor._cache.get.assert_called_once_with(str(expected_key))
 
     def test_get_unhashable(self):
         expected_key = (self.func_name, [1], 2)
@@ -118,7 +118,7 @@ class TestCachingDecorator(base.BaseTestCase):
         retval = self.decor.func([1], 2)
         self.assertFalse(self.decor._cache.set.called)
         self.assertEqual(self.decor.func_retval, retval)
-        self.decor._cache.get.assert_called_once_with(expected_key)
+        self.decor._cache.get.assert_called_once_with(str(expected_key))
 
     def test_missing_cache(self):
         delattr(self.decor, '_cache')