From 5e5559b5450bf933d4aefcbd45abbcacac405e1b Mon Sep 17 00:00:00 2001 From: Lianhao Lu Date: Thu, 5 Jun 2014 20:19:45 +0800 Subject: [PATCH] Skipping central agent pollster when keystone not available When keystone is not available, instead of skipping all central agent pollsters, we should let the pollsters decide whether to skip or not, because not all central agent pollsters requires keystone. Closes-Bug: #1316532 Change-Id: Ib34f9838bd128e1b357937f0c177d8a09f19b13f --- ceilometer/agent.py | 4 +- ceilometer/central/manager.py | 4 +- ceilometer/central/plugin.py | 19 ++++++++ ceilometer/energy/kwapi.py | 4 +- ceilometer/image/glance.py | 8 +-- ceilometer/objectstore/swift.py | 12 +++-- ceilometer/tests/agentbase.py | 41 ++++++++-------- ceilometer/tests/central/test_manager.py | 62 ++++++++++++++++++++++-- ceilometer/tests/energy/test_kwapi.py | 2 +- ceilometer/tests/image/test_glance.py | 2 +- 10 files changed, 114 insertions(+), 44 deletions(-) diff --git a/ceilometer/agent.py b/ceilometer/agent.py index a407a8bdf..414148cc6 100644 --- a/ceilometer/agent.py +++ b/ceilometer/agent.py @@ -85,8 +85,8 @@ class PollingTask(object): source_resources = list(self.resources[key].resources) try: samples = list(pollster.obj.get_samples( - self.manager, - cache, + manager=self.manager, + cache=cache, resources=source_resources or agent_resources, )) publisher(samples) diff --git a/ceilometer/central/manager.py b/ceilometer/central/manager.py index 91d9a2673..f6f6a18cf 100644 --- a/ceilometer/central/manager.py +++ b/ceilometer/central/manager.py @@ -20,7 +20,6 @@ from keystoneclient.v2_0 import client as ksclient from oslo.config import cfg from ceilometer import agent -from ceilometer.openstack.common.gettextutils import _ from ceilometer.openstack.common import log cfg.CONF.import_group('service_credentials', 'ceilometer.service') @@ -45,7 +44,6 @@ class AgentManager(agent.AgentManager): region_name=cfg.CONF.service_credentials.os_region_name, insecure=cfg.CONF.service_credentials.insecure) except Exception as e: - LOG.error(_('Skip interval_task because Keystone error: %s'), e) - return + self.keystone = e super(AgentManager, self).interval_task(task) diff --git a/ceilometer/central/plugin.py b/ceilometer/central/plugin.py index a7c5595be..328f04842 100644 --- a/ceilometer/central/plugin.py +++ b/ceilometer/central/plugin.py @@ -18,8 +18,27 @@ """Base class for plugins used by the central agent. """ +from ceilometer.openstack.common.gettextutils import _ +from ceilometer.openstack.common import log from ceilometer import plugin +LOG = log.getLogger(__name__) + class CentralPollster(plugin.PollsterBase): """Base class for plugins that support the polling API.""" + + +def check_keystone(f): + """Decorator function to check if manager has valid keystone client.""" + def func(self, *args, **kwargs): + manager = kwargs.get('manager') + if not manager and len(args) > 0: + manager = args[0] + keystone = getattr(manager, 'keystone', None) + if not keystone or isinstance(keystone, Exception): + LOG.error(_('Skip due to keystone error %s'), + str(keystone) if keystone else '') + return iter([]) + return f(self, *args, **kwargs) + return func diff --git a/ceilometer/energy/kwapi.py b/ceilometer/energy/kwapi.py index 069ccc718..d7a7c2bbe 100644 --- a/ceilometer/energy/kwapi.py +++ b/ceilometer/energy/kwapi.py @@ -81,7 +81,7 @@ class _Base(plugin.CentralPollster): class EnergyPollster(_Base): """Measures energy consumption.""" - + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): """Returns all samples.""" for probe in self._iter_probes(manager.keystone, cache): @@ -101,7 +101,7 @@ class EnergyPollster(_Base): class PowerPollster(_Base): """Measures power consumption.""" - + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): """Returns all samples.""" for probe in self._iter_probes(manager.keystone, cache): diff --git a/ceilometer/image/glance.py b/ceilometer/image/glance.py index 20c3b9a98..868303cd8 100644 --- a/ceilometer/image/glance.py +++ b/ceilometer/image/glance.py @@ -24,12 +24,12 @@ import itertools import glanceclient from oslo.config import cfg +from ceilometer.central import plugin from ceilometer.openstack.common import timeutils -from ceilometer import plugin from ceilometer import sample -class _Base(plugin.PollsterBase): +class _Base(plugin.CentralPollster): @staticmethod def get_glance_client(ksclient): @@ -103,7 +103,7 @@ class _Base(plugin.PollsterBase): class ImagePollster(_Base): - + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): for image in self._iter_images(manager.keystone, cache): yield sample.Sample( @@ -120,7 +120,7 @@ class ImagePollster(_Base): class ImageSizePollster(_Base): - + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): for image in self._iter_images(manager.keystone, cache): yield sample.Sample( diff --git a/ceilometer/objectstore/swift.py b/ceilometer/objectstore/swift.py index 99a2b1936..9056ff01a 100644 --- a/ceilometer/objectstore/swift.py +++ b/ceilometer/objectstore/swift.py @@ -25,10 +25,10 @@ from keystoneclient import exceptions from oslo.config import cfg from swiftclient import client as swift +from ceilometer.central import plugin from ceilometer.openstack.common.gettextutils import _ from ceilometer.openstack.common import log from ceilometer.openstack.common import timeutils -from ceilometer import plugin from ceilometer import sample @@ -44,7 +44,7 @@ OPTS = [ cfg.CONF.register_opts(OPTS) -class _Base(plugin.PollsterBase): +class _Base(plugin.CentralPollster): CACHE_KEY_TENANT = 'tenants' METHOD = 'head' @@ -87,7 +87,7 @@ class _Base(plugin.PollsterBase): class ObjectsPollster(_Base): """Iterate over all accounts, using keystone. """ - + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): for tenant, account in self._iter_accounts(manager.keystone, cache): yield sample.Sample( @@ -106,7 +106,7 @@ class ObjectsPollster(_Base): class ObjectsSizePollster(_Base): """Iterate over all accounts, using keystone. """ - + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): for tenant, account in self._iter_accounts(manager.keystone, cache): yield sample.Sample( @@ -125,7 +125,7 @@ class ObjectsSizePollster(_Base): class ObjectsContainersPollster(_Base): """Iterate over all accounts, using keystone. """ - + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): for tenant, account in self._iter_accounts(manager.keystone, cache): yield sample.Sample( @@ -147,6 +147,7 @@ class ContainersObjectsPollster(_Base): METHOD = 'get' + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): for project, account in self._iter_accounts(manager.keystone, cache): containers_info = account[1] @@ -170,6 +171,7 @@ class ContainersSizePollster(_Base): METHOD = 'get' + @plugin.check_keystone def get_samples(self, manager, cache, resources=None): for project, account in self._iter_accounts(manager.keystone, cache): containers_info = account[1] diff --git a/ceilometer/tests/agentbase.py b/ceilometer/tests/agentbase.py index 3af2d8897..99e97e242 100644 --- a/ceilometer/tests/agentbase.py +++ b/ceilometer/tests/agentbase.py @@ -170,30 +170,27 @@ class BaseAgentManagerTestCase(base.BaseTestCase): self.pipeline_cfg, self.transformer_manager) + def get_extention_list(self): + return [extension.Extension('test', + None, + None, + self.Pollster(), ), + extension.Extension('testanother', + None, + None, + self.PollsterAnother(), ), + extension.Extension('testexception', + None, + None, + self.PollsterException(), ), + extension.Extension('testexceptionanother', + None, + None, + self.PollsterExceptionAnother(), )] + def create_pollster_manager(self): return extension.ExtensionManager.make_test_instance( - [ - extension.Extension( - 'test', - None, - None, - self.Pollster(), ), - extension.Extension( - 'testanother', - None, - None, - self.PollsterAnother(), ), - extension.Extension( - 'testexception', - None, - None, - self.PollsterException(), ), - extension.Extension( - 'testexceptionanother', - None, - None, - self.PollsterExceptionAnother(), ), - ], + self.get_extention_list(), ) def create_discovery_manager(self): diff --git a/ceilometer/tests/central/test_manager.py b/ceilometer/tests/central/test_manager.py index 2258ce483..3d8bbcd48 100644 --- a/ceilometer/tests/central/test_manager.py +++ b/ceilometer/tests/central/test_manager.py @@ -19,10 +19,13 @@ """ import mock +from stevedore import extension from ceilometer.central import manager +from ceilometer.central import plugin from ceilometer.openstack.common.fixture import mockpatch from ceilometer.openstack.common import test +from ceilometer import pipeline from ceilometer.tests import agentbase @@ -34,7 +37,31 @@ class TestManager(test.BaseTestCase): self.assertIsNotNone(list(mgr.pollster_manager)) +class TestPollsterKeystone(agentbase.TestPollster): + @plugin.check_keystone + def get_samples(self, manager, cache, resources=None): + func = super(TestPollsterKeystone, self).get_samples + return func(manager=manager, + cache=cache, + resources=resources) + + class TestRunTasks(agentbase.BaseAgentManagerTestCase): + + class PollsterKeystone(TestPollsterKeystone): + samples = [] + resources = [] + test_data = agentbase.TestSample( + name='testkeystone', + type=agentbase.default_test_data.type, + unit=agentbase.default_test_data.unit, + volume=agentbase.default_test_data.volume, + user_id=agentbase.default_test_data.user_id, + project_id=agentbase.default_test_data.project_id, + resource_id=agentbase.default_test_data.resource_id, + timestamp=agentbase.default_test_data.timestamp, + resource_metadata=agentbase.default_test_data.resource_metadata) + @staticmethod def create_manager(): return manager.AgentManager() @@ -44,18 +71,45 @@ class TestRunTasks(agentbase.BaseAgentManagerTestCase): super(TestRunTasks, self).setUp() self.useFixture(mockpatch.Patch( 'keystoneclient.v2_0.client.Client', - return_value=None)) + return_value=mock.Mock())) + + def tearDown(self): + self.PollsterKeystone.samples = [] + self.PollsterKeystone.resources = [] + super(TestRunTasks, self).tearDown() + + def get_extention_list(self): + exts = super(TestRunTasks, self).get_extention_list() + exts.append(extension.Extension('testkeystone', + None, + None, + self.PollsterKeystone(),)) + return exts def test_get_sample_resources(self): polling_tasks = self.mgr.setup_polling_tasks() self.mgr.interval_task(polling_tasks.values()[0]) self.assertTrue(self.Pollster.resources) - def test_skip_task_when_keystone_fail(self): - """Test for https://bugs.launchpad.net/ceilometer/+bug/1287613.""" + def test_when_keystone_fail(self): + """Test for bug 1316532. + """ self.useFixture(mockpatch.Patch( 'keystoneclient.v2_0.client.Client', side_effect=Exception)) + self.pipeline_cfg = [ + { + 'name': "test_keystone", + 'interval': 10, + 'counters': ['testkeystone'], + 'resources': ['test://'] if self.source_resources else [], + 'transformers': [], + 'publishers': ["test"], + }, + ] + self.mgr.pipeline_manager = pipeline.PipelineManager( + self.pipeline_cfg, + self.transformer_manager) polling_tasks = self.mgr.setup_polling_tasks() self.mgr.interval_task(polling_tasks.values()[0]) - self.assertFalse(self.Pollster.samples) + self.assertFalse(self.PollsterKeystone.samples) diff --git a/ceilometer/tests/energy/test_kwapi.py b/ceilometer/tests/energy/test_kwapi.py index 8f25e0f24..40c4d5f97 100644 --- a/ceilometer/tests/energy/test_kwapi.py +++ b/ceilometer/tests/energy/test_kwapi.py @@ -51,7 +51,7 @@ class TestManager(manager.AgentManager): def __init__(self): super(TestManager, self).__init__() - self.keystone = None + self.keystone = mock.Mock() class TestKwapi(test.BaseTestCase): diff --git a/ceilometer/tests/image/test_glance.py b/ceilometer/tests/image/test_glance.py index d8a65f257..198a26fb4 100644 --- a/ceilometer/tests/image/test_glance.py +++ b/ceilometer/tests/image/test_glance.py @@ -113,7 +113,7 @@ class TestManager(manager.AgentManager): def __init__(self): super(TestManager, self).__init__() - self.keystone = None + self.keystone = mock.Mock() class TestImagePollster(test.BaseTestCase):