x/deb-oslo.vmware
Code Issues Proposed changes

Fix race during session creation

Browse Source 
Concurrent VIM API invocations after session expiration would result
in concurrent login attempts. In such cases, if a login attempt is
made after a successful one, it would result in InvalidLogin. This
is because vCenter server won't allow login within an active session.
On the other hand, if two or more login attempts reach vCenter at
the same, it would result in session leak-- the last one returned
overwrite any of the previous sessions. This patch prevents the
race condition by making session creation synchronized and checking
for an active session before login attempt. We can also remove the
call to terminate previous session after login since a login is
attempted only if there is no active session.

Change-Id: Ib4ca3553ce14c80ab722092907d797767072741c
Closes-Bug: #1409014
This commit is contained in:
Vipin Balachandran 2015-01-09 19:20:41 +05:30
parent 2df3a6200a
commit a229faf8ba
4 changed files with 55 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
oslo_vmware/api.py
View File

@ -23,6 +23,7 @@ in case of connection problems or server API call overload.
import logging
from oslo_concurrency import lockutils
import six
from oslo.utils import excutils
@ -211,15 +212,23 @@ class VMwareAPISession(object):
return self._pbm
@RetryDecorator(exceptions=(exceptions.VimConnectionException,))
@lockutils.synchronized('oslo_vmware_api_lock')
def _create_session(self):
"""Establish session with the server."""
# Another thread might have created the session while the current one
# was waiting for the lock.
if self._session_id and self.is_current_session_active():
LOG.debug("Current session: %s is active.",
_trunc_id(self._session_id))
return
session_manager = self.vim.service_content.sessionManager
# Login and create new session with the server for making API calls.
LOG.debug("Logging in with username = %s.", self._server_username)
session = self.vim.Login(session_manager,
userName=self._server_username,
password=self._server_password)
prev_session_id, self._session_id = self._session_id, session.key
self._session_id = session.key
# We need to save the username in the session since we may need it
# later to check active session. The SessionIsActive method requires
# the username parameter to be exactly same as that in the session
@ -230,25 +239,6 @@ class VMwareAPISession(object):
"%s."),
_trunc_id(self._session_id))
# Terminate the previous session (if exists) for preserving sessions
# as there is a limit on the number of sessions we can have.
if prev_session_id:
try:
LOG.info(_LI("Terminating the previous session with ID = %s"),
_trunc_id(prev_session_id))
self.vim.TerminateSession(session_manager,
sessionId=[prev_session_id])
except Exception:
# This exception is something we can live with. It is
# just an extra caution on our side. The session might
# have been cleared already. We could have made a call to
# SessionIsActive, but that is an overhead because we
# anyway would have to call TerminateSession.
LOG.warn(_LW("Error occurred while terminating the previous "
"session with ID = %s."),
_trunc_id(prev_session_id),
exc_info=True)
# Set PBM client cookie.
if self._pbm is not None:
self._pbm.set_soap_cookie(self._vim.get_http_cookie())

25
oslo_vmware/tests/test_api.py
View File

@ -170,25 +170,44 @@ class VMwareAPISessionTest(base.TestCase):
self.assertEqual(session.key, api_session._session_id)
pbm.set_soap_cookie.assert_called_once_with(cookie)
def test_create_session_with_existing_session(self):
def test_create_session_with_existing_inactive_session(self):
old_session_key = '12345'
new_session_key = '67890'
session = mock.Mock()
session.key = new_session_key
api_session = self._create_api_session(False)
api_session._session_id = old_session_key
api_session._session_username = api_session._server_username
vim_obj = api_session.vim
vim_obj.Login.return_value = session
vim_obj.SessionIsActive.return_value = False
api_session._create_session()
session_manager = vim_obj.service_content.sessionManager
vim_obj.SessionIsActive.assert_called_once_with(
session_manager, sessionID=old_session_key,
userName=VMwareAPISessionTest.USERNAME)
vim_obj.Login.assert_called_once_with(
session_manager, userName=VMwareAPISessionTest.USERNAME,
password=VMwareAPISessionTest.PASSWORD)
vim_obj.TerminateSession.assert_called_once_with(
session_manager, sessionId=[old_session_key])
self.assertEqual(new_session_key, api_session._session_id)
def test_create_session_with_existing_active_session(self):
old_session_key = '12345'
api_session = self._create_api_session(False)
api_session._session_id = old_session_key
api_session._session_username = api_session._server_username
vim_obj = api_session.vim
vim_obj.SessionIsActive.return_value = True
api_session._create_session()
session_manager = vim_obj.service_content.sessionManager
vim_obj.SessionIsActive.assert_called_once_with(
session_manager, sessionID=old_session_key,
userName=VMwareAPISessionTest.USERNAME)
self.assertFalse(vim_obj.Login.called)
self.assertEqual(old_session_key, api_session._session_id)
def test_invoke_api(self):
api_session = self._create_api_session(True)
response = mock.Mock()

1
requirements.txt
View File

@ -24,3 +24,4 @@ suds>=0.4
eventlet>=0.15.2
requests>=2.2.0,!=2.4.0
urllib3>=1.8.3
oslo.concurrency>=0.3.0,!=0.4.0 # Apache-2.0

25
tests/test_api.py
View File

@ -169,25 +169,44 @@ class VMwareAPISessionTest(base.TestCase):
self.assertEqual(session.key, api_session._session_id)
pbm.set_soap_cookie.assert_called_once_with(cookie)
def test_create_session_with_existing_session(self):
def test_create_session_with_existing_inactive_session(self):
old_session_key = '12345'
new_session_key = '67890'
session = mock.Mock()
session.key = new_session_key
api_session = self._create_api_session(False)
api_session._session_id = old_session_key
api_session._session_username = api_session._server_username
vim_obj = api_session.vim
vim_obj.Login.return_value = session
vim_obj.SessionIsActive.return_value = False
api_session._create_session()
session_manager = vim_obj.service_content.sessionManager
vim_obj.SessionIsActive.assert_called_once_with(
session_manager, sessionID=old_session_key,
userName=VMwareAPISessionTest.USERNAME)
vim_obj.Login.assert_called_once_with(
session_manager, userName=VMwareAPISessionTest.USERNAME,
password=VMwareAPISessionTest.PASSWORD)
vim_obj.TerminateSession.assert_called_once_with(
session_manager, sessionId=[old_session_key])
self.assertEqual(new_session_key, api_session._session_id)
def test_create_session_with_existing_active_session(self):
old_session_key = '12345'
api_session = self._create_api_session(False)
api_session._session_id = old_session_key
api_session._session_username = api_session._server_username
vim_obj = api_session.vim
vim_obj.SessionIsActive.return_value = True
api_session._create_session()
session_manager = vim_obj.service_content.sessionManager
vim_obj.SessionIsActive.assert_called_once_with(
session_manager, sessionID=old_session_key,
userName=VMwareAPISessionTest.USERNAME)
self.assertFalse(vim_obj.Login.called)
self.assertEqual(old_session_key, api_session._session_id)
def test_invoke_api(self):
api_session = self._create_api_session(True)
response = mock.Mock()