From bd0244ffe63b752649ae74f65a46563e986dcb00 Mon Sep 17 00:00:00 2001 From: ZhiQiang Fan Date: Mon, 3 Nov 2014 20:57:04 +0800 Subject: [PATCH] Add timeout to all http requests Currently, we generate lots of samples by polling data from other services, but theses rest requests have no timeout limitation. We have observed that some requests (for example, keystone due to openssl problem) may stuck for over several days (maybe forever if we don't restart the service). Other pollsters in same thread will not be able to work too. The worst thing is that, when outside (keystone) service becomes normal, Ceilometer cannot recover itself automatically, cloud operator needs to restart it manually. So I strongly suggest that we should add timeout limit to **every** rest api call, this is quite important to improve Ceilometer's robust and reliability. This patch adds a new option named http_timeout, and applies it to almost all http requests in Ceilometer project. Change-Id: I76df2c0a9ffacb252e15edbb125e37ccb2aac4aa Closes-Bug: #1388778 --- ceilometer/alarm/evaluator/__init__.py | 2 ++ ceilometer/alarm/notifier/trust.py | 5 +++++ ceilometer/alarm/service.py | 3 +++ ceilometer/central/manager.py | 4 +++- ceilometer/central/plugin.py | 4 +++- ceilometer/energy/kwapi.py | 3 ++- ceilometer/image/glance.py | 3 ++- ceilometer/network/statistics/opencontrail/client.py | 4 +++- ceilometer/network/statistics/opendaylight/client.py | 4 +++- ceilometer/neutron_client.py | 4 +++- ceilometer/nova_client.py | 3 ++- ceilometer/service.py | 4 ++++ ceilometer/tests/alarm/evaluator/test_threshold.py | 1 + ceilometer/tests/alarm/test_alarm_svc.py | 1 + ceilometer/tests/alarm/test_singleton_alarm_svc.py | 1 + 15 files changed, 38 insertions(+), 8 deletions(-) diff --git a/ceilometer/alarm/evaluator/__init__.py b/ceilometer/alarm/evaluator/__init__.py index 3e6a82e77..af8160108 100644 --- a/ceilometer/alarm/evaluator/__init__.py +++ b/ceilometer/alarm/evaluator/__init__.py @@ -36,6 +36,7 @@ UNKNOWN = 'insufficient data' OK = 'ok' ALARM = 'alarm' +cfg.CONF.import_opt('http_timeout', 'ceilometer.service') cfg.CONF.import_group('service_credentials', 'ceilometer.service') @@ -61,6 +62,7 @@ class Evaluator(object): os_cacert=auth_config.os_cacert, os_endpoint_type=auth_config.os_endpoint_type, insecure=auth_config.insecure, + timeout=cfg.CONF.http_timeout, ) self.api_client = ceiloclient.get_client(2, **creds) return self.api_client diff --git a/ceilometer/alarm/notifier/trust.py b/ceilometer/alarm/notifier/trust.py index a6402d4f7..0d935386d 100644 --- a/ceilometer/alarm/notifier/trust.py +++ b/ceilometer/alarm/notifier/trust.py @@ -21,6 +21,10 @@ from six.moves.urllib import parse from ceilometer.alarm.notifier import rest +cfg.CONF.import_opt('http_timeout', 'ceilometer.service') +cfg.CONF.import_group('service_credentials', 'ceilometer.service') + + class TrustRestAlarmNotifier(rest.RestAlarmNotifier): """Notifier supporting keystone trust authentication. @@ -44,6 +48,7 @@ class TrustRestAlarmNotifier(rest.RestAlarmNotifier): auth_url=auth_url, region_name=cfg.CONF.service_credentials.os_region_name, insecure=cfg.CONF.service_credentials.insecure, + timeout=cfg.CONF.http_timeout, trust_id=trust_id) # Remove the fake user diff --git a/ceilometer/alarm/service.py b/ceilometer/alarm/service.py index 0d2576c61..5b824ccea 100644 --- a/ceilometer/alarm/service.py +++ b/ceilometer/alarm/service.py @@ -51,6 +51,8 @@ cfg.CONF.import_opt('partition_rpc_topic', 'ceilometer.alarm.rpc', group='alarm') cfg.CONF.import_opt('heartbeat', 'ceilometer.coordination', group='coordination') +cfg.CONF.import_opt('http_timeout', 'ceilometer.service') +cfg.CONF.import_group('service_credentials', 'ceilometer.service') LOG = log.getLogger(__name__) @@ -88,6 +90,7 @@ class AlarmService(object): os_cacert=auth_config.os_cacert, os_endpoint_type=auth_config.os_endpoint_type, insecure=auth_config.insecure, + timeout=cfg.CONF.http_timeout, ) self.api_client = ceiloclient.get_client(2, **creds) return self.api_client diff --git a/ceilometer/central/manager.py b/ceilometer/central/manager.py index 7d72acbae..e5ebd9fa7 100644 --- a/ceilometer/central/manager.py +++ b/ceilometer/central/manager.py @@ -31,6 +31,7 @@ OPTS = [ 'subset of pollsters should be loaded.'), ] cfg.CONF.register_opts(OPTS, group='central') +cfg.CONF.import_opt('http_timeout', 'ceilometer.service') cfg.CONF.import_group('service_credentials', 'ceilometer.service') LOG = log.getLogger(__name__) @@ -52,7 +53,8 @@ class AgentManager(agent.AgentManager): cacert=cfg.CONF.service_credentials.os_cacert, auth_url=cfg.CONF.service_credentials.os_auth_url, region_name=cfg.CONF.service_credentials.os_region_name, - insecure=cfg.CONF.service_credentials.insecure) + insecure=cfg.CONF.service_credentials.insecure, + timeout=cfg.CONF.http_timeout,) except Exception as e: self.keystone = e diff --git a/ceilometer/central/plugin.py b/ceilometer/central/plugin.py index 3ca833704..941b50630 100644 --- a/ceilometer/central/plugin.py +++ b/ceilometer/central/plugin.py @@ -23,6 +23,7 @@ from ceilometer.openstack.common.gettextutils import _ from ceilometer.openstack.common import log from ceilometer import plugin +cfg.CONF.import_opt('http_timeout', 'ceilometer.service') cfg.CONF.import_group('service_credentials', 'ceilometer.service') LOG = log.getLogger(__name__) @@ -42,7 +43,8 @@ def _get_keystone(): cacert=cfg.CONF.service_credentials.os_cacert, auth_url=cfg.CONF.service_credentials.os_auth_url, region_name=cfg.CONF.service_credentials.os_region_name, - insecure=cfg.CONF.service_credentials.insecure) + insecure=cfg.CONF.service_credentials.insecure, + timeout=cfg.CONF.http_timeout) except Exception as e: return e diff --git a/ceilometer/energy/kwapi.py b/ceilometer/energy/kwapi.py index 531f4cb5b..4a73d147f 100644 --- a/ceilometer/energy/kwapi.py +++ b/ceilometer/energy/kwapi.py @@ -52,7 +52,8 @@ class KwapiClient(object): headers = {} if self.token is not None: headers = {'X-Auth-Token': self.token} - request = requests.get(probes_url, headers=headers) + timeout = cfg.CONF.http_timeout + request = requests.get(probes_url, headers=headers, timeout=timeout) message = request.json() probes = message['probes'] for key, value in six.iteritems(probes): diff --git a/ceilometer/image/glance.py b/ceilometer/image/glance.py index 76292e771..6225d216d 100644 --- a/ceilometer/image/glance.py +++ b/ceilometer/image/glance.py @@ -62,7 +62,8 @@ class _Base(plugin.CentralPollster): return glanceclient.Client('1', endpoint, token=ksclient.auth_token, cacert=service_credentials.os_cacert, - insecure=service_credentials.insecure) + insecure=service_credentials.insecure, + timeout=cfg.CONF.http_timeout) def _get_images(self, ksclient, endpoint): client = self.get_glance_client(ksclient, endpoint) diff --git a/ceilometer/network/statistics/opencontrail/client.py b/ceilometer/network/statistics/opencontrail/client.py index 6c66f42c3..0e0b9e9ce 100644 --- a/ceilometer/network/statistics/opencontrail/client.py +++ b/ceilometer/network/statistics/opencontrail/client.py @@ -24,6 +24,7 @@ from ceilometer.openstack.common import log CONF = cfg.CONF +CONF.import_opt('http_timeout', 'ceilometer.service') LOG = log.getLogger(__name__) @@ -98,7 +99,8 @@ class AnalyticsAPIBaseClient(object): 'data': data, 'verify': self.verify_ssl, 'allow_redirects': False, - 'cookies': cookies + 'cookies': cookies, + 'timeout': CONF.http_timeout, } return req_params diff --git a/ceilometer/network/statistics/opendaylight/client.py b/ceilometer/network/statistics/opendaylight/client.py index 9bb9d70d2..6bc963fbf 100644 --- a/ceilometer/network/statistics/opendaylight/client.py +++ b/ceilometer/network/statistics/opendaylight/client.py @@ -25,6 +25,7 @@ from ceilometer.openstack.common import log CONF = cfg.CONF +CONF.import_opt('http_timeout', 'ceilometer.service') LOG = log.getLogger(__name__) @@ -170,7 +171,8 @@ class Client(): req_params = { 'headers': { 'Accept': 'application/json' - } + }, + 'timeout': CONF.http_timeout, } auth_way = params.get('auth') diff --git a/ceilometer/neutron_client.py b/ceilometer/neutron_client.py index 66d3f80eb..0d46b9aad 100644 --- a/ceilometer/neutron_client.py +++ b/ceilometer/neutron_client.py @@ -30,6 +30,7 @@ SERVICE_OPTS = [ ] cfg.CONF.register_opts(SERVICE_OPTS, group='service_types') +cfg.CONF.import_opt('http_timeout', 'ceilometer.service') cfg.CONF.import_group('service_credentials', 'ceilometer.service') LOG = log.getLogger(__name__) @@ -65,7 +66,8 @@ class Client(object): 'auth_url': conf.os_auth_url, 'region_name': conf.os_region_name, 'endpoint_type': conf.os_endpoint_type, - 'service_type': cfg.CONF.service_types.neutron + 'timeout': cfg.CONF.http_timeout, + 'service_type': cfg.CONF.service_types.neutron, } if conf.os_tenant_id: diff --git a/ceilometer/nova_client.py b/ceilometer/nova_client.py index 298a68444..378143dbe 100644 --- a/ceilometer/nova_client.py +++ b/ceilometer/nova_client.py @@ -36,7 +36,7 @@ SERVICE_OPTS = [ cfg.CONF.register_opts(OPTS) cfg.CONF.register_opts(SERVICE_OPTS, group='service_types') - +cfg.CONF.import_opt('http_timeout', 'ceilometer.service') cfg.CONF.import_group('service_credentials', 'ceilometer.service') LOG = log.getLogger(__name__) @@ -74,6 +74,7 @@ class Client(object): bypass_url=bypass_url, cacert=conf.os_cacert, insecure=conf.insecure, + timeout=cfg.CONF.http_timeout, http_log_debug=cfg.CONF.nova_http_log_debug, no_cache=True) diff --git a/ceilometer/service.py b/ceilometer/service.py index 9f7d6f927..9f75fd51f 100644 --- a/ceilometer/service.py +++ b/ceilometer/service.py @@ -43,6 +43,10 @@ OPTS = [ default=1, help='Number of workers for notification service. A single ' 'notification agent is enabled by default.'), + cfg.IntOpt('http_timeout', + default=600, + help='Timeout seconds for HTTP requests. Set it to None to ' + 'disable timeout.'), ] cfg.CONF.register_opts(OPTS) diff --git a/ceilometer/tests/alarm/evaluator/test_threshold.py b/ceilometer/tests/alarm/evaluator/test_threshold.py index 8efa7868a..b09f89c89 100644 --- a/ceilometer/tests/alarm/evaluator/test_threshold.py +++ b/ceilometer/tests/alarm/evaluator/test_threshold.py @@ -360,6 +360,7 @@ class TestEvaluate(base.TestEvaluatorBase): os_username=conf.os_username, os_cacert=conf.os_cacert, os_endpoint_type=conf.os_endpoint_type, + timeout=cfg.CONF.http_timeout, insecure=conf.insecure)] actual = client.call_args_list self.assertEqual(expected, actual) diff --git a/ceilometer/tests/alarm/test_alarm_svc.py b/ceilometer/tests/alarm/test_alarm_svc.py index acd7f069a..5c06e997c 100644 --- a/ceilometer/tests/alarm/test_alarm_svc.py +++ b/ceilometer/tests/alarm/test_alarm_svc.py @@ -138,6 +138,7 @@ class TestAlarmEvaluationService(tests_base.BaseTestCase): os_username=conf.os_username, os_cacert=conf.os_cacert, os_endpoint_type=conf.os_endpoint_type, + timeout=self.CONF.http_timeout, insecure=conf.insecure)] actual = client.call_args_list self.assertEqual(expected, actual) diff --git a/ceilometer/tests/alarm/test_singleton_alarm_svc.py b/ceilometer/tests/alarm/test_singleton_alarm_svc.py index 8f12b9b9e..58bb3d80c 100644 --- a/ceilometer/tests/alarm/test_singleton_alarm_svc.py +++ b/ceilometer/tests/alarm/test_singleton_alarm_svc.py @@ -102,6 +102,7 @@ class TestSingletonAlarmService(tests_base.BaseTestCase): os_username=conf.os_username, os_cacert=conf.os_cacert, os_endpoint_type=conf.os_endpoint_type, + timeout=self.CONF.http_timeout, insecure=conf.insecure)] actual = client.call_args_list self.assertEqual(expected, actual)