From aa8a183192182a81fcfe8e42c86451b23a8e1d7f Mon Sep 17 00:00:00 2001 From: melanie witt Date: Wed, 18 Nov 2015 20:58:54 +0000 Subject: [PATCH] xenapi: Add helper function and unit tests for client session This builds upon change I9bd4266186c69ff93aa6c854b98a3182a8086398 that adds a missing timeout check for a session login. Add a helper function for session login that uses the timeout context manager and call it instead of calling login_with_password directly on a session. Also use the _create_session helper function instead of directly creating a session in the exception handling. Change-Id: Ibb41702fea03b9b33a533f972a988e5111987e80 --- .../unit/virt/xenapi/client/test_session.py | 38 +++++++++++++++++++ nova/virt/xenapi/client/session.py | 26 +++++++------ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/nova/tests/unit/virt/xenapi/client/test_session.py b/nova/tests/unit/virt/xenapi/client/test_session.py index ba895aa775fa..7c7435f2442d 100644 --- a/nova/tests/unit/virt/xenapi/client/test_session.py +++ b/nova/tests/unit/virt/xenapi/client/test_session.py @@ -43,6 +43,44 @@ class SessionTestCase(stubs.XenAPITestBaseNoDB): expected_version, 'OpenStack') + @mock.patch('eventlet.timeout.Timeout') + @mock.patch.object(session.XenAPISession, '_create_session') + @mock.patch.object(session.XenAPISession, '_get_product_version_and_brand') + @mock.patch.object(session.XenAPISession, '_verify_plugin_version') + def test_session_login_with_timeout(self, mock_verify, mock_version, + create_session, mock_timeout): + self.flags(connection_concurrent=2, group='xenserver') + sess = mock.Mock() + create_session.return_value = sess + mock_version.return_value = ('version', 'brand') + + session.XenAPISession('url', 'username', 'password') + self.assertEqual(2, sess.login_with_password.call_count) + self.assertEqual(2, mock_timeout.call_count) + + @mock.patch('eventlet.timeout.Timeout') + @mock.patch.object(session.XenAPISession, '_create_session') + @mock.patch.object(session.XenAPISession, '_get_product_version_and_brand') + @mock.patch.object(session.XenAPISession, '_verify_plugin_version') + @mock.patch.object(session.XenAPISession, '_get_host_uuid') + @mock.patch.object(session.XenAPISession, '_get_host_ref') + def test_session_raises_exception(self, mock_ref, mock_uuid, + mock_verify, mock_version, + create_session, mock_timeout): + import XenAPI + self.flags(connection_concurrent=2, group='xenserver') + sess = mock.Mock() + create_session.return_value = sess + # First login fails, second login in except block succeeds, + # third login for the pool succeeds + sess.login_with_password.side_effect = [ + XenAPI.Failure(['HOST_IS_SLAVE', 'master']), None, None] + mock_version.return_value = ('version', 'brand') + + session.XenAPISession('url', 'username', 'password') + self.assertEqual(3, sess.login_with_password.call_count) + self.assertEqual(3, mock_timeout.call_count) + class ApplySessionHelpersTestCase(stubs.XenAPITestBaseNoDB): def setUp(self): diff --git a/nova/virt/xenapi/client/session.py b/nova/virt/xenapi/client/session.py index 6db55fcb174c..4c1f4f31f373 100644 --- a/nova/virt/xenapi/client/session.py +++ b/nova/virt/xenapi/client/session.py @@ -107,6 +107,11 @@ class XenAPISession(object): apply_session_helpers(self) + def _login_with_password(self, user, pw, session, exception): + with timeout.Timeout(CONF.xenserver.login_timeout, exception): + session.login_with_password(user, pw, + self.nova_version, 'OpenStack') + def _verify_plugin_version(self): requested_version = self.PLUGIN_REQUIRED_VERSION current_version = self.call_plugin_serialized( @@ -119,19 +124,14 @@ class XenAPISession(object): def _create_first_session(self, url, user, pw, exception): try: - session = self._create_session(url) - with timeout.Timeout(CONF.xenserver.login_timeout, exception): - session.login_with_password(user, pw, - self.nova_version, 'OpenStack') + session = self._create_session_and_login(url, user, pw, exception) except self.XenAPI.Failure as e: # if user and pw of the master are different, we're doomed! if e.details[0] == 'HOST_IS_SLAVE': master = e.details[1] url = pool.swap_xapi_host(url, master) - session = self.XenAPI.Session(url) - with timeout.Timeout(CONF.xenserver.login_timeout, exception): - session.login_with_password(user, pw, - self.nova_version, 'OpenStack') + session = self._create_session_and_login(url, user, pw, + exception) self.is_slave = True else: raise @@ -140,10 +140,7 @@ class XenAPISession(object): def _populate_session_pool(self, url, user, pw, exception): for i in range(CONF.xenserver.connection_concurrent - 1): - session = self._create_session(url) - with timeout.Timeout(CONF.xenserver.login_timeout, exception): - session.login_with_password(user, pw, - self.nova_version, 'OpenStack') + session = self._create_session_and_login(url, user, pw, exception) self._sessions.put(session) def _get_host_uuid(self): @@ -286,6 +283,11 @@ class XenAPISession(object): return self.XenAPI.xapi_local() return self.XenAPI.Session(url) + def _create_session_and_login(self, url, user, pw, exception): + session = self._create_session(url) + self._login_with_password(user, pw, session, exception) + return session + def _unwrap_plugin_exceptions(self, func, *args, **kwargs): """Parse exception details.""" try: