Merge "Fallback to oslo_cache.dict caching backend"

ceilometer/cache_utils.py

@@ -24,7 +24,7 @@ from oslo_log import log
 from oslo_utils.secretutils import md5
 
 # Default cache expiration period
-CACHE_DURATION = 86400
+CACHE_DURATION = 600
 
 NAME_ENCODED = __name__.encode('utf-8')
 CACHE_NAMESPACE = uuid.UUID(
@@ -35,8 +35,9 @@ LOG = log.getLogger(__name__)
 
 
 class CacheClient(object):
-    def __init__(self, region):
+    def __init__(self, region, conf):
         self.region = region
+        self.conf = conf
 
     def get(self, key):
         value = self.region.get(key)
@@ -50,57 +51,59 @@ class CacheClient(object):
     def delete(self, key):
         return self.region.delete(key)
 
+    def resolve_uuid_from_cache(self, attr, uuid):
+        resource_name = self.get(uuid)
+        if resource_name:
+            return resource_name
+        else:
+            # Retrieve project and user names from Keystone only
+            # if ceilometer doesn't have a caching backend
+            resource_name = self._resolve_uuid_from_keystone(attr, uuid)
+            self.set(uuid, resource_name)
+            return resource_name
+
+    def _resolve_uuid_from_keystone(self, attr, uuid):
+        try:
+            return getattr(
+                keystone_client.get_client(self.conf), attr
+            ).get(uuid).name
+        except AttributeError as e:
+            LOG.warning("Found '%s' while resolving uuid %s to name", e, uuid)
+        except ka_exceptions.NotFound as e:
+            LOG.warning(e.message)
+
 
 def get_client(conf):
     cache.configure(conf)
-    if 'cache' in conf.keys() and conf.cache.enabled:
+    if conf.cache.enabled:
         region = get_cache_region(conf)
         if region:
-            return CacheClient(region)
+            return CacheClient(region, conf)
+    else:
+        # configure oslo_cache.dict backend if
+        # no caching backend is configured
+        region = get_dict_cache_region()
+        return CacheClient(region, conf)
+
+
+def get_dict_cache_region():
+    region = cache.create_region()
+    region.configure('oslo_cache.dict', expiration_time=CACHE_DURATION)
+    return region
 
 
 def get_cache_region(conf):
-    # Set expiration time to default CACHE_DURATION if missing in conf
+    # configure caching region using params from config
-    if not conf.cache.expiration_time:
-        conf.cache.expiration_time = CACHE_DURATION
-
     try:
         region = cache.create_region()
         cache.configure_cache_region(conf, region)
-        cache.key_mangler = cache_key_mangler
         return region
+
     except exception.ConfigurationError as e:
-        LOG.error("failed to configure oslo_cache. %s", str(e))
+        LOG.error("failed to configure oslo_cache: %s", str(e))
-        LOG.warning("using keystone to identify names from polled samples")
 
 
 def cache_key_mangler(key):
     """Construct an opaque cache key."""
 
     return uuid.uuid5(CACHE_NAMESPACE, key).hex
-
-
-def resolve_uuid_from_cache(conf, attr, uuid):
-    # empty cache_client means either caching is not enabled or
-    # there was an error configuring cache
-    cache_client = get_client(conf)
-    if cache_client:
-        resource_name = cache_client.get(uuid)
-        if resource_name:
-            return resource_name
-
-    # Retrieve project and user names from Keystone only
-    # if ceilometer doesn't have a caching backend
-    resource_name = resolve_uuid_from_keystone(conf, attr, uuid)
-    if cache_client:
-        cache_client.set(uuid, resource_name)
-    return resource_name
-
-
-def resolve_uuid_from_keystone(conf, attr, uuid):
-    try:
-        return getattr(keystone_client.get_client(conf), attr).get(uuid).name
-    except AttributeError as e:
-        LOG.warning("Found '%s' while resolving uuid %s to name", e, uuid)
-    except ka_exceptions.NotFound as e:
-        LOG.warning(e.message)

ceilometer/meter/notifications.py

@@ -52,6 +52,7 @@ class MeterDefinition(object):
     def __init__(self, definition_cfg, conf, plugin_manager):
         self.conf = conf
         self.cfg = definition_cfg
+        self._cache = cache_utils.get_client(self.conf)
         missing = [field for field in self.REQUIRED_FIELDS
                    if not self.cfg.get(field)]
         if missing:
@@ -169,18 +170,20 @@ class MeterDefinition(object):
                 sample = dict((attributes[idx], value)
                               for idx, value in enumerate(values))
 
-                # populate user_name and project_name fields in the sample
+                if (
-                # created from notifications
+                    self.conf.polling.tenant_name_discovery and
-                if sample['user_id']:
+                    self._cache
-                    sample['user_name'] = \
+                ):
-                        cache_utils.resolve_uuid_from_cache(
+                    # populate user_name and project_name fields in the sample
-                            self.conf, 'users', sample['user_id']
+                    # created from notifications
-                        )
+                    if sample['user_id']:
-                if sample['project_id']:
+                        sample['user_name'] = \
-                    sample['project_name'] = \
+                            self._cache.resolve_uuid_from_cache(
-                        cache_utils.resolve_uuid_from_cache(
+                                'users', sample['user_id'])
-                            self.conf, 'projects', sample['project_id']
+                    if sample['project_id']:
-                        )
+                        sample['project_name'] = \
+                            self._cache.resolve_uuid_from_cache(
+                                'projects', sample['project_id'])
                 yield sample
         else:
             yield sample

ceilometer/polling/manager.py

@@ -68,15 +68,15 @@ POLLING_OPTS = [
                          "to created pollsters."),
     cfg.BoolOpt('tenant_name_discovery',
                 default=False,
-                help="Identify project and user names from polled samples"
+                help='Identify project and user names from polled samples. '
-                     "By default, collecting these values is disabled due"
+                     'By default, collecting these values is disabled due '
-                     "to the fact that it could overwhelm keystone service"
+                     'to the fact that it could overwhelm keystone service'
-                     "with lots of continuous requests depending upon the"
+                     'with lots of continuous requests depending upon the '
-                     "number of projects, users and samples polled from"
+                     'number of projects, users and samples polled from '
-                     "the environment. While using this feature, it is"
+                     'the environment. While using this feature, it is '
-                     "recommended that ceilometer be configured with a"
+                     'recommended that ceilometer be configured with a '
-                     "caching backend to reduce the number of calls"
+                     'caching backend to reduce the number of calls '
-                     "made to keystone"),
+                     'made to keystone.'),
 ]
 
 
@@ -152,7 +152,7 @@ class PollingTask(object):
 
         self.ks_client = self.manager.keystone
 
-        self.cache_client = cache_utils.get_client(self.manager.conf)
+        self._cache = cache_utils.get_client(self.manager.conf)
 
     def add(self, pollster, source):
         self.pollster_matches[source.name].add(pollster)
@@ -211,7 +211,10 @@ class PollingTask(object):
                         # Note(yuywz): Unify the timestamp of polled samples
                         sample.set_timestamp(polling_timestamp)
 
-                        if self.manager.conf.tenant_name_discovery:
+                        if (
+                            self.manager.conf.polling.tenant_name_discovery and
+                            self._cache
+                        ):
 
                             # Try to resolve project UUIDs from cache first,
                             # and then keystone
@@ -222,8 +225,7 @@ class PollingTask(object):
                                       sample)
                             if sample.project_id:
                                 sample.project_name = \
-                                    cache_utils.resolve_uuid_from_cache(
+                                    self._cache.resolve_uuid_from_cache(
-                                        self.manager.conf,
                                         "projects",
                                         sample.project_id
                                     )
@@ -238,8 +240,7 @@ class PollingTask(object):
                                       sample)
                             if sample.user_id:
                                 sample.user_name = \
-                                    cache_utils.resolve_uuid_from_cache(
+                                    self._cache.resolve_uuid_from_cache(
-                                        self.manager.conf,
                                         "users",
                                         sample.user_id
                                     )

ceilometer/tests/unit/meter/test_notifications.py

@@ -15,6 +15,8 @@ import copy
 from unittest import mock
 
 import fixtures
+from oslo_cache import core as cache
+from oslo_config import fixture as config_fixture
 from oslo_utils import encodeutils
 from oslo_utils import fileutils
 import yaml
@@ -141,23 +143,23 @@ FULL_MULTI_MSG = {
     'payload': [{
                 'counter_name': 'instance1',
                 'user_id': 'user1',
-                'user_name': 'test-resource',
+                'user_name': 'fake-name',
                 'resource_id': 'res1',
                 'counter_unit': 'ns',
                 'counter_volume': 28.0,
                 'project_id': 'proj1',
-                'project_name': 'test-resource',
+                'project_name': 'fake-name',
                 'counter_type': 'gauge'
                 },
                 {
                 'counter_name': 'instance2',
                 'user_id': 'user2',
-                'user_name': 'test-resource',
+                'user_name': 'fake-name',
                 'resource_id': 'res2',
                 'counter_unit': '%',
                 'counter_volume': 1.0,
                 'project_id': 'proj2',
-                'project_name': 'test-resource',
+                'project_name': 'fake-name',
                 'counter_type': 'delta'
                 }],
     'ctxt': {'domain': None,
@@ -238,7 +240,6 @@ METRICS_UPDATE = {
 
 
 class TestMeterDefinition(test.BaseTestCase):
-
     def test_config_definition(self):
         cfg = dict(name="test",
                    event_type="test.create",
@@ -247,7 +248,8 @@ class TestMeterDefinition(test.BaseTestCase):
                    volume="$.payload.volume",
                    resource_id="$.payload.resource_id",
                    project_id="$.payload.project_id")
-        handler = notifications.MeterDefinition(cfg, mock.Mock(), mock.Mock())
+        conf = ceilometer_service.prepare_service([], [])
+        handler = notifications.MeterDefinition(cfg, conf, mock.Mock())
         self.assertTrue(handler.match_type("test.create"))
         sample = list(handler.to_samples(NOTIFICATION))[0]
         self.assertEqual(1.0, sample["volume"])
@@ -258,8 +260,9 @@ class TestMeterDefinition(test.BaseTestCase):
 
     def test_config_required_missing_fields(self):
         cfg = dict()
+        conf = ceilometer_service.prepare_service([], [])
         try:
-            notifications.MeterDefinition(cfg, mock.Mock(), mock.Mock())
+            notifications.MeterDefinition(cfg, conf, mock.Mock())
         except declarative.DefinitionException as e:
             self.assertIn("Required fields ['name', 'type', 'event_type',"
                           " 'unit', 'volume', 'resource_id']"
@@ -270,18 +273,36 @@ class TestMeterDefinition(test.BaseTestCase):
         cfg = dict(name="test", type="foo", event_type="bar.create",
                    unit="foo", volume="bar",
                    resource_id="bea70e51c7340cb9d555b15cbfcaec23")
+        conf = ceilometer_service.prepare_service([], [])
         try:
-            notifications.MeterDefinition(cfg, mock.Mock(), mock.Mock())
+            notifications.MeterDefinition(cfg, conf, mock.Mock())
         except declarative.DefinitionException as e:
             self.assertIn("Invalid type foo specified",
                           encodeutils.exception_to_unicode(e))
 
 
+class CacheConfFixture(config_fixture.Config):
+    def setUp(self):
+        super(CacheConfFixture, self).setUp()
+        self.conf = ceilometer_service.\
+            prepare_service(argv=[], config_files=[])
+        cache.configure(self.conf)
+
+
 class TestMeterProcessing(test.BaseTestCase):
 
     def setUp(self):
         super(TestMeterProcessing, self).setUp()
         self.CONF = ceilometer_service.prepare_service([], [])
+        dict_conf_fixture = CacheConfFixture(self.CONF)
+        self.useFixture(dict_conf_fixture)
+        dict_conf_fixture.config(enabled=True, group='cache')
+        dict_conf_fixture.config(expiration_time=600,
+                                 backend='oslo_cache.dict',
+                                 group='cache')
+        dict_conf_fixture.config(tenant_name_discovery=True, group='polling')
+        self.CONF = dict_conf_fixture.conf
+
         self.path = self.useFixture(fixtures.TempDir()).path
         self.handler = notifications.ProcessMeterNotifications(
             self.CONF, mock.Mock())
@@ -619,16 +640,11 @@ class TestMeterProcessing(test.BaseTestCase):
         c = list(self.handler.build_sample(event))
         self.assertEqual(0, len(c))
 
-    @mock.patch('ceilometer.cache_utils.resolve_uuid_from_cache')
+    @mock.patch(
-    def test_multi_meter_payload_all_multi(self, fake_cached_resource_name):
+        'ceilometer.cache_utils.CacheClient._resolve_uuid_from_keystone'
-
+    )
-        # return "test-resource" as the name of the user and project from cache
+    def test_multi_meter_payload_all_multi(self, resolved_uuid):
-        fake_cached_resource_name.return_value = "test-resource"
+        resolved_uuid.return_value = "fake-name"
-
-        # expect user_name and project_name values to be set to "test-resource"
-        fake_user_name = "test-resource"
-        fake_project_name = "test-resource"
-
         cfg = yaml.dump(
             {'metric': [dict(name="$.payload.[*].counter_name",
                         event_type="full.sample",
@@ -653,8 +669,8 @@ class TestMeterProcessing(test.BaseTestCase):
             self.assertEqual(msg[idx]['resource_id'], s1['resource_id'])
             self.assertEqual(msg[idx]['project_id'], s1['project_id'])
             self.assertEqual(msg[idx]['user_id'], s1['user_id'])
-            self.assertEqual(fake_user_name, s1['user_name'])
+            self.assertEqual(msg[idx]['project_name'], s1['project_name'])
-            self.assertEqual(fake_project_name, s1['project_name'])
+            self.assertEqual(msg[idx]['user_name'], s1['user_name'])
 
     @mock.patch('ceilometer.meter.notifications.LOG')
     def test_multi_meter_payload_invalid_missing(self, LOG):

ceilometer/tests/unit/publisher/test_gnocchi.py

@@ -710,6 +710,7 @@ class PublisherWorkflowTest(base.BaseTestCase,
         for call in expected_calls:
             self.assertIn(call, fakeclient.mock_calls)
 
+    @mock.patch('ceilometer.cache_utils.get_client', mock.Mock())
     @mock.patch('ceilometer.publisher.gnocchi.LOG')
     @mock.patch('gnocchiclient.v1.client.Client')
     def test_workflow(self, fakeclient_cls, logger):

ceilometer/tests/unit/test_cache_utils.py

@@ -15,6 +15,7 @@
 
 from ceilometer import cache_utils
 from ceilometer import service as ceilometer_service
+from oslo_cache.backends import dictionary
 from oslo_cache import core as cache
 from oslo_config import fixture as config_fixture
 from oslotest import base
@@ -26,7 +27,6 @@ class CacheConfFixture(config_fixture.Config):
         self.conf = ceilometer_service.\
             prepare_service(argv=[], config_files=[])
         cache.configure(self.conf)
-        self.config(enabled=True, group='cache')
 
 
 class TestOsloCache(base.BaseTestCase):
@@ -37,6 +37,7 @@ class TestOsloCache(base.BaseTestCase):
 
         dict_conf_fixture = CacheConfFixture(conf)
         self.useFixture(dict_conf_fixture)
+        dict_conf_fixture.config(enabled=True, group='cache')
         dict_conf_fixture.config(expiration_time=600,
                                  backend='oslo_cache.dict',
                                  group='cache')
@@ -47,19 +48,53 @@ class TestOsloCache(base.BaseTestCase):
         # incorrect config
         faulty_conf_fixture = CacheConfFixture(conf)
         self.useFixture(faulty_conf_fixture)
+        faulty_conf_fixture.config(enabled=True, group='cache')
         faulty_conf_fixture.config(expiration_time=600,
                                    backend='dogpile.cache.memcached',
                                    group='cache',
                                    enable_retry_client='true')
-        self.faulty_cache_conf = faulty_conf_fixture.conf
+        self.faulty_conf = faulty_conf_fixture.conf
 
-        self.no_cache_conf = ceilometer_service.\
+        no_cache_fixture = CacheConfFixture(conf)
-            prepare_service(argv=[], config_files=[])
+        self.useFixture(no_cache_fixture)
+        # no_cache_fixture.config()
+        self.no_cache_conf = no_cache_fixture.conf
 
     def test_get_cache_region(self):
         self.assertIsNotNone(cache_utils.get_cache_region(self.dict_conf))
 
+        # having invalid configurations will return None
+        with self.assertLogs('ceilometer.cache_utils', level='ERROR') as logs:
+            self.assertIsNone(
+                cache_utils.get_cache_region(self.faulty_conf)
+            )
+            cache_configure_failed = logs.output
+            self.assertIn(
+                'ERROR:ceilometer.cache_utils:'
+                'failed to configure oslo_cache: '
+                'Retry client is only supported by '
+                'the \'dogpile.cache.pymemcache\' backend.',
+                cache_configure_failed)
+
     def test_get_client(self):
-        self.assertIsNotNone(cache_utils.get_client(self.dict_conf))
+        dict_cache_client = cache_utils.get_client(self.dict_conf)
-        self.assertIsNone(cache_utils.get_client(self.no_cache_conf))
+        self.assertIsNotNone(dict_cache_client)
-        self.assertIsNone(cache_utils.get_client(self.faulty_cache_conf))
+        self.assertIsInstance(dict_cache_client.region.backend,
+                              dictionary.DictCacheBackend)
+
+        no_cache_config = cache_utils.get_client(self.no_cache_conf)
+        self.assertIsNotNone(no_cache_config)
+        self.assertIsInstance(dict_cache_client.region.backend,
+                              dictionary.DictCacheBackend)
+
+        # having invalid configurations will return None
+        with self.assertLogs('ceilometer.cache_utils', level='ERROR') as logs:
+            cache_client = cache_utils.get_client(self.faulty_conf)
+            cache_configure_failed = logs.output
+            self.assertIsNone(cache_client)
+            self.assertIn(
+                'ERROR:ceilometer.cache_utils:'
+                'failed to configure oslo_cache: '
+                'Retry client is only supported by '
+                'the \'dogpile.cache.pymemcache\' backend.',
+                cache_configure_failed)