diff --git a/ceilometer/cache_utils.py b/ceilometer/cache_utils.py index f3fbd1873b..f7d2adad03 100644 --- a/ceilometer/cache_utils.py +++ b/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 - if not conf.cache.expiration_time: - conf.cache.expiration_time = CACHE_DURATION - + # configure caching region using params from config 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.warning("using keystone to identify names from polled samples") + LOG.error("failed to configure oslo_cache: %s", str(e)) 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) diff --git a/ceilometer/meter/notifications.py b/ceilometer/meter/notifications.py index 159f72bd93..507c70beb8 100644 --- a/ceilometer/meter/notifications.py +++ b/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 - # created from notifications - if sample['user_id']: - sample['user_name'] = \ - cache_utils.resolve_uuid_from_cache( - self.conf, 'users', sample['user_id'] - ) - if sample['project_id']: - sample['project_name'] = \ - cache_utils.resolve_uuid_from_cache( - self.conf, 'projects', sample['project_id'] - ) + if ( + self.conf.polling.tenant_name_discovery and + self._cache + ): + # populate user_name and project_name fields in the sample + # created from notifications + if sample['user_id']: + sample['user_name'] = \ + self._cache.resolve_uuid_from_cache( + 'users', sample['user_id']) + if sample['project_id']: + sample['project_name'] = \ + self._cache.resolve_uuid_from_cache( + 'projects', sample['project_id']) yield sample else: yield sample diff --git a/ceilometer/polling/manager.py b/ceilometer/polling/manager.py index e980850b62..f9c8e4582d 100644 --- a/ceilometer/polling/manager.py +++ b/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" - "By default, collecting these values is disabled due" - "to the fact that it could overwhelm keystone service" - "with lots of continuous requests depending upon the" - "number of projects, users and samples polled from" - "the environment. While using this feature, it is" - "recommended that ceilometer be configured with a" - "caching backend to reduce the number of calls" - "made to keystone"), + help='Identify project and user names from polled samples. ' + 'By default, collecting these values is disabled due ' + 'to the fact that it could overwhelm keystone service' + 'with lots of continuous requests depending upon the ' + 'number of projects, users and samples polled from ' + 'the environment. While using this feature, it is ' + 'recommended that ceilometer be configured with a ' + 'caching backend to reduce the number of calls ' + '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,14 +211,16 @@ 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 if sample.project_id: sample.project_name = \ - cache_utils.resolve_uuid_from_cache( - self.manager.conf, + self._cache.resolve_uuid_from_cache( "projects", sample.project_id ) @@ -227,8 +229,7 @@ class PollingTask(object): # and then keystone if sample.user_id: sample.user_name = \ - cache_utils.resolve_uuid_from_cache( - self.manager.conf, + self._cache.resolve_uuid_from_cache( "users", sample.user_id ) diff --git a/ceilometer/tests/unit/meter/test_notifications.py b/ceilometer/tests/unit/meter/test_notifications.py index 9a12120f44..10d572a546 100644 --- a/ceilometer/tests/unit/meter/test_notifications.py +++ b/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') - def test_multi_meter_payload_all_multi(self, fake_cached_resource_name): - - # return "test-resource" as the name of the user and project from cache - fake_cached_resource_name.return_value = "test-resource" - - # expect user_name and project_name values to be set to "test-resource" - fake_user_name = "test-resource" - fake_project_name = "test-resource" - + @mock.patch( + 'ceilometer.cache_utils.CacheClient._resolve_uuid_from_keystone' + ) + def test_multi_meter_payload_all_multi(self, resolved_uuid): + resolved_uuid.return_value = "fake-name" 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(fake_project_name, s1['project_name']) + self.assertEqual(msg[idx]['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): diff --git a/ceilometer/tests/unit/publisher/test_gnocchi.py b/ceilometer/tests/unit/publisher/test_gnocchi.py index 8a87f70a50..8fe6fb6b75 100644 --- a/ceilometer/tests/unit/publisher/test_gnocchi.py +++ b/ceilometer/tests/unit/publisher/test_gnocchi.py @@ -710,9 +710,10 @@ class PublisherWorkflowTest(base.BaseTestCase, for call in expected_calls: self.assertIn(call, fakeclient.mock_calls) + @mock.patch('ceilometer.cache_utils.get_client') @mock.patch('ceilometer.publisher.gnocchi.LOG') @mock.patch('gnocchiclient.v1.client.Client') - def test_workflow(self, fakeclient_cls, logger): + def test_workflow(self, fakeclient_cls, logger, cache_utils): url = netutils.urlsplit("gnocchi://") publisher = gnocchi.GnocchiPublisher(self.conf.conf, url) diff --git a/ceilometer/tests/unit/test_cache_utils.py b/ceilometer/tests/unit/test_cache_utils.py index 245eaa3446..a7b6ca43e5 100644 --- a/ceilometer/tests/unit/test_cache_utils.py +++ b/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.\ - prepare_service(argv=[], config_files=[]) + no_cache_fixture = CacheConfFixture(conf) + 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)) - self.assertIsNone(cache_utils.get_client(self.no_cache_conf)) - self.assertIsNone(cache_utils.get_client(self.faulty_cache_conf)) + dict_cache_client = cache_utils.get_client(self.dict_conf) + self.assertIsNotNone(dict_cache_client) + 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)