From 801f4b3e1b0f9a5f9e568659a912146ce35a47b4 Mon Sep 17 00:00:00 2001
From: ZhiQiang Fan <zhiqiang.fan@huawei.com>
Date: Tue, 31 Mar 2015 15:03:23 +0800
Subject: Add timeout for keystoneclient session

Currently, we only apply timeout on ceilometer api, but not on keystone
api as well, this is not consistent. Keystone api may be slow, or worse,
stuck, then ceilometerclient will not return timely. This issue has caused
availability problem in our environment when ceilometer alarm service
has been running for a long time.

Change-Id: I0d5f82ff9cf4132a1de2f0b649908483326e116e
Closes-Bug: #1436249
(cherry picked from commit 44af2d4087815d5d451305b549f91ab6130528c0)
---
 ceilometerclient/client.py            |  4 +++-
 ceilometerclient/tests/test_client.py | 13 +++++++++++++
 ceilometerclient/tests/test_shell.py  | 15 +++++++++++++--
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/ceilometerclient/client.py b/ceilometerclient/client.py
index 78b4059..0df6822 100644
--- a/ceilometerclient/client.py
+++ b/ceilometerclient/client.py
@@ -66,6 +66,7 @@ def _get_keystone_session(**kwargs):
     auth_url = kwargs.pop('auth_url', None)
     project_id = kwargs.pop('project_id', None)
     project_name = kwargs.pop('project_name', None)
+    timeout = kwargs.get('timeout')
 
     if insecure:
         verify = False
@@ -78,7 +79,7 @@ def _get_keystone_session(**kwargs):
         cert = (cert, key)
 
     # create the keystone client session
-    ks_session = session.Session(verify=verify, cert=cert)
+    ks_session = session.Session(verify=verify, cert=cert, timeout=timeout)
     v2_auth_url, v3_auth_url = _discover_auth_versions(ks_session, auth_url)
 
     username = kwargs.pop('username', None)
@@ -180,6 +181,7 @@ class AuthPlugin(auth.BaseAuthPlugin):
                     self.opts.get('insecure')),
                 'endpoint_type': self.opts.get('endpoint_type'),
                 'region_name': self.opts.get('region_name'),
+                'timeout': http_client.timeout,
             }
 
             # retrieve session
diff --git a/ceilometerclient/tests/test_client.py b/ceilometerclient/tests/test_client.py
index b5ec4c3..56fa298 100644
--- a/ceilometerclient/tests/test_client.py
+++ b/ceilometerclient/tests/test_client.py
@@ -12,6 +12,7 @@
 
 import types
 
+from keystoneclient import session as ks_session
 import mock
 
 from ceilometerclient import client
@@ -135,6 +136,18 @@ class ClientTest(utils.BaseTestCase):
     def test_v2_client_timeout_valid_value(self):
         self._test_v2_client_timeout_integer(30, 30)
 
+    @mock.patch.object(ks_session, 'Session')
+    def test_v2_client_timeout_keystone_seesion(self, mocked_session):
+        mocked_session.side_effect = RuntimeError('Stop!')
+        env = FAKE_ENV.copy()
+        env['timeout'] = 5
+        del env['auth_plugin']
+        del env['token']
+        client = self.create_client(env)
+        self.assertRaises(RuntimeError, client.alarms.list)
+        args, kwargs = mocked_session.call_args
+        self.assertEqual(5, kwargs['timeout'])
+
     def test_v2_client_cacert_in_verify(self):
         env = FAKE_ENV.copy()
         env['cacert'] = '/path/to/cacert'
diff --git a/ceilometerclient/tests/test_shell.py b/ceilometerclient/tests/test_shell.py
index c3a138e..078191f 100644
--- a/ceilometerclient/tests/test_shell.py
+++ b/ceilometerclient/tests/test_shell.py
@@ -176,6 +176,15 @@ class ShellTimeoutTest(ShellTestBase):
                         '0 must be greater than 0')
         self._test_timeout('0', expected_msg)
 
+    @mock.patch.object(ks_session, 'Session')
+    def test_timeout_kesytone_session(self, mocked_session):
+        mocked_session.side_effect = exc.HTTPUnauthorized("FAIL")
+        self.make_env(FAKE_V2_ENV)
+        args = ['--debug', '--timeout', '5', 'alarm-list']
+        self.assertRaises(exc.CommandError, ceilometer_shell.main, args)
+        args, kwargs = mocked_session.call_args
+        self.assertEqual(5, kwargs.get('timeout'))
+
 
 class ShellInsecureTest(ShellTestBase):
 
@@ -193,7 +202,8 @@ class ShellInsecureTest(ShellTestBase):
         self.make_env(FAKE_V2_ENV)
         args = ['--debug', '--os-insecure', 'true', 'alarm-list']
         self.assertRaises(exc.CommandError, ceilometer_shell.main, args)
-        mocked_session.assert_called_with(verify=False, cert='')
+        args, kwargs = mocked_session.call_args
+        self.assertEqual(False, kwargs.get('verify'))
 
     @mock.patch.object(api_client, 'HTTPClient')
     def test_insecure_false_ceilometer(self, mocked_client):
@@ -209,7 +219,8 @@ class ShellInsecureTest(ShellTestBase):
         self.make_env(FAKE_V2_ENV)
         args = ['--debug', '--os-insecure', 'false', 'alarm-list']
         self.assertRaises(exc.CommandError, ceilometer_shell.main, args)
-        mocked_session.assert_called_with(verify=True, cert='')
+        args, kwargs = mocked_session.call_args
+        self.assertEqual(True, kwargs.get('verify'))
 
 
 class ShellEndpointTest(ShellTestBase):
-- 
cgit v1.2.1