From e81474d4ecb385001e65b2220acb54fc43ece35a Mon Sep 17 00:00:00 2001 From: Eamonn O'Toole Date: Mon, 5 Feb 2018 11:38:04 +0000 Subject: [PATCH] Allow connection reuse We have to use the SDK in environments where the total number of connections is restricted, where repeatedly opening and closing connections can cause instability, and with fairly large latencies in opening a new connection. To aid stability and SDK performance in these environments we allow for the reuse of https connections as returned by get_connection(). Connections will be reused when 'reuse_connection' is set to True or when the ONEVIEWSDK_REUSE_CONNECTION is anything other than an empty string. We've added a unit-test for this new capability. --- CHANGELOG.md | 4 +- README.md | 17 ++++++ examples/config-rename.json | 3 +- hpOneView/connection.py | 59 ++++++++++++++----- .../image_streamer/image_streamer_client.py | 7 +-- hpOneView/oneview_client.py | 13 ++-- tests/unit/test_connection.py | 21 ++++++- tests/unit/test_oneview_client.py | 12 ++-- 8 files changed, 104 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa470726..f1cc32a6 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,11 @@ # 4.5.0 (Unreleased) #### Notes -Added the capability to set a connection timeout when connecting to the HPE OneView Appliance +Added the capability to set a connection timeout when connecting to the HPE OneView Appliance. Extends support of the SDK to OneView Rest API version 600 (OneView v4.0). +Added the capability to reuse https connections to the HPE OneView Appliance. + #### Features supported with current release: - Connection template - Enclosure diff --git a/README.md b/README.md index 557b37ce..0f38cb13 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,7 @@ export ONEVIEWSDK_AUTH_LOGIN_DOMAIN='authdomain' export ONEVIEWSDK_SSL_CERTIFICATE='' export ONEVIEWSDK_PROXY=':' export ONEVIEWSDK_CONNECTION_TIMEOUT='' +export ONEVIEWSDK_REUSE_CONNECTION='' ``` :lock: Tip: Make sure no unauthorized person has access to the environment variables, since the password is stored in clear-text. @@ -259,6 +260,22 @@ export ONEVIEWSDK_CONNECTION_TIMEOUT='' "timeout": ``` + +### Reuse https connections +By default a new https connection is made and subsequently closed for each SDK transaction. To +change this so that the https_connection is reused then either: + +1. Set the appropriate environment variable: +```bash +export ONEVIEWSDK_REUSE_CONNECTION='' +``` + +2. Set the reuse_connection flag in the JSON configuration file: +```bash +"reuse_connection": true +``` + + ## Exception handling All exceptions raised by the OneView Python SDK inherit from HPOneViewException. diff --git a/examples/config-rename.json b/examples/config-rename.json index 28a8ebee..61ff6f08 100644 --- a/examples/config-rename.json +++ b/examples/config-rename.json @@ -21,5 +21,6 @@ "storage_system_password": "", "power_device_hostname": "", "power_device_username": "", - "power_device_password": "" + "power_device_password": "", + "reuse_connection": true } diff --git a/hpOneView/connection.py b/hpOneView/connection.py index 99c420cb..7d0217d3 100644 --- a/hpOneView/connection.py +++ b/hpOneView/connection.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -* ### -# (C) Copyright (2012-2017) Hewlett Packard Enterprise Development LP +# (C) Copyright (2012-2018) Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -54,7 +54,7 @@ class connection(object): - def __init__(self, applianceIp, api_version=300, sslBundle=False, timeout=None): + def __init__(self, applianceIp, api_version=300, sslBundle=False, timeout=None, reuse_connection=False): self._session = None self._host = applianceIp self._cred = None @@ -75,6 +75,10 @@ def __init__(self, applianceIp, api_version=300, sslBundle=False, timeout=None): self._numDisplayedRecords = 0 self._validateVersion = False self._timeout = timeout + self._reuse_connection = reuse_connection + if self._reuse_connection: + self._headers['Connection'] = 'keep-alive' + self._conn = None def validateVersion(self): version = self.get(uri['version']) @@ -120,11 +124,11 @@ def do_http(self, method, path, body, custom_headers=None): if custom_headers: http_headers.update(custom_headers) - bConnected = False conn = None + bConnected = False while bConnected is False: try: - conn = self.get_connection() + conn = self.get_reusable_connection() conn.request(method, path, body, http_headers) resp = conn.getresponse() tempbytes = '' @@ -133,7 +137,8 @@ def do_http(self, method, path, body, custom_headers=None): tempbody = tempbytes.decode('utf-8') except UnicodeDecodeError: # Might be binary data tempbody = tempbytes - conn.close() + if not self._reuse_connection: + self.close_reusable_connection(conn) bConnected = True return resp, tempbody if tempbody: @@ -141,15 +146,16 @@ def do_http(self, method, path, body, custom_headers=None): body = json.loads(tempbody) except ValueError: body = tempbody - conn.close() + if not self._reuse_connection: + self.close_reusable_connection(conn) bConnected = True except http.client.BadStatusLine: logger.warning('Bad Status Line. Trying again...') - if conn: - conn.close() + self.close_reusable_connection(conn) time.sleep(1) continue except http.client.HTTPException: + self.close_reusable_connection(conn) raise HPOneViewException('Failure during login attempt.\n %s' % traceback.format_exc()) return resp, body @@ -165,7 +171,7 @@ def download_to_stream(self, stream_writer, url, body='', method='GET', custom_h successful_connected = False while not successful_connected: try: - conn = self.get_connection() + conn = self.get_reusable_connection() conn.request(method, url, body, http_headers) resp = conn.getresponse() @@ -178,15 +184,16 @@ def download_to_stream(self, stream_writer, url, body='', method='GET', custom_h if tempbytes: # filter out keep-alive new chunks stream_writer.write(tempbytes) - conn.close() + if not self._reuse_connection: + self.close_reusable_connection(conn) successful_connected = True except http.client.BadStatusLine: logger.warning('Bad Status Line. Trying again...') - if conn: - conn.close() + self.close_reusable_connection(conn) time.sleep(1) continue except http.client.HTTPException: + self.close_reusable_connection(conn) raise HPOneViewException('Failure during login attempt.\n %s' % traceback.format_exc()) return successful_connected @@ -201,13 +208,28 @@ def __handle_download_error(self, resp, conn): body = tempbody except UnicodeDecodeError: # Might be binary data body = tempbytes - conn.close() + self.close_reusable_connection(conn) if not body: body = "Error " + str(resp.status) - conn.close() + self.close_reusable_connection(conn) raise HPOneViewException(body) + def get_reusable_connection(self): + if self._reuse_connection: + if not self._conn: + logger.debug('Creating new connection') + self._conn = self.get_connection() + conn = self._conn + else: + conn = self.get_connection() + return conn + + def close_reusable_connection(self, conn): + if conn: + conn.close() + self._conn = None + def get_connection(self): context = ssl.SSLContext(ssl.PROTOCOL_TLSv1_2) if self._sslTrustAll is False: @@ -290,7 +312,7 @@ def post_multipart(self, uri, fields, files, baseName, verbose=False): mappedfile = mmap.mmap(inputfile.fileno(), 0, access=mmap.ACCESS_READ) if verbose is True: print(('Uploading ' + files + '...')) - conn = self.get_connection() + conn = self.get_reusable_connection() # conn.set_debuglevel(1) conn.connect() conn.putrequest('POST', uri) @@ -300,6 +322,8 @@ def post_multipart(self, uri, fields, files, baseName, verbose=False): totalSize = os.path.getsize(files + '.b64') conn.putheader('Content-Length', totalSize) conn.putheader('X-API-Version', self._apiVersion) + if self._reuse_connection: + conn.putheader('Connection', 'keep-alive') conn.endheaders() while mappedfile.tell() < mappedfile.size(): @@ -313,6 +337,7 @@ def post_multipart(self, uri, fields, files, baseName, verbose=False): mappedfile.close() inputfile.close() os.remove(files + '.b64') + response = conn.getresponse() body = response.read().decode('utf-8') @@ -322,9 +347,11 @@ def post_multipart(self, uri, fields, files, baseName, verbose=False): except ValueError: body = response.read().decode('utf-8') - conn.close() + if not self._reuse_connection: + self.close_reusable_connection(conn) if response.status >= 400: + self.close_reusable_connection(conn) raise HPOneViewException(body) return response, body diff --git a/hpOneView/image_streamer/image_streamer_client.py b/hpOneView/image_streamer/image_streamer_client.py index e15d777a..eb5d6b4b 100644 --- a/hpOneView/image_streamer/image_streamer_client.py +++ b/hpOneView/image_streamer/image_streamer_client.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- ### -# (C) Copyright (2012-2017) Hewlett Packard Enterprise Development LP +# (C) Copyright (2012-2018) Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -32,7 +32,6 @@ standard_library.install_aliases() - from hpOneView.connection import connection from hpOneView.image_streamer.resources.golden_images import GoldenImages from hpOneView.image_streamer.resources.plan_scripts import PlanScripts @@ -44,8 +43,8 @@ class ImageStreamerClient(object): - def __init__(self, ip, session_id, api_version, sslBundle=False): - self.__connection = connection(ip, api_version, sslBundle) + def __init__(self, ip, session_id, api_version, sslBundle=False, timeout=None, reuse_connection=False): + self.__connection = connection(ip, api_version, sslBundle, timeout, reuse_connection) self.__connection.set_session_id(session_id) self.__golden_images = None self.__plan_scripts = None diff --git a/hpOneView/oneview_client.py b/hpOneView/oneview_client.py index 70e811fe..0daaeaef 100755 --- a/hpOneView/oneview_client.py +++ b/hpOneView/oneview_client.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- ### -# (C) Copyright (2012-2017) Hewlett Packard Enterprise Development LP +# (C) Copyright (2012-2018) Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -117,7 +117,7 @@ class OneViewClient(object): def __init__(self, config): self.__connection = connection(config["ip"], config.get('api_version', self.DEFAULT_API_VERSION), config.get('ssl_certificate', False), - config.get('timeout')) + config.get('timeout'), config.get('reuse_connection', False)) self.__image_streamer_ip = config.get("image_streamer_ip") self.__set_proxy(config) self.__connection.login(config["credentials"]) @@ -218,7 +218,7 @@ def from_environment_variables(cls): Allowed variables: ONEVIEWSDK_IP (required), ONEVIEWSDK_USERNAME (required), ONEVIEWSDK_PASSWORD (required), ONEVIEWSDK_AUTH_LOGIN_DOMAIN, ONEVIEWSDK_API_VERSION, ONEVIEWSDK_IMAGE_STREAMER_IP, ONEVIEWSDK_SESSIONID, ONEVIEWSDK_SSL_CERTIFICATE, - ONEVIEWSDK_CONNECTION_TIMEOUT and ONEVIEWSDK_PROXY. + ONEVIEWSDK_CONNECTION_TIMEOUT, ONEVIEWSDK_PROXY and ONEVIEWSDK_REUSE_CONNECTION. Returns: OneViewClient: @@ -233,13 +233,14 @@ def from_environment_variables(cls): proxy = os.environ.get('ONEVIEWSDK_PROXY', '') sessionID = os.environ.get('ONEVIEWSDK_SESSIONID', '') timeout = os.environ.get('ONEVIEWSDK_CONNECTION_TIMEOUT') + reuse_connection = bool(os.environ.get('ONEVIEWSDK_REUSE_CONNECTION', '')) config = dict(ip=ip, image_streamer_ip=image_streamer_ip, api_version=api_version, ssl_certificate=ssl_certificate, credentials=dict(userName=username, authLoginDomain=auth_login_domain, password=password, sessionID=sessionID), - proxy=proxy, timeout=timeout) + proxy=proxy, timeout=timeout, reuse_connection=reuse_connection) return cls(config) @@ -289,7 +290,9 @@ def create_image_streamer_client(self): image_streamer = ImageStreamerClient(self.__image_streamer_ip, self.__connection.get_session_id(), self.__connection._apiVersion, - self.__connection._sslBundle) + self.__connection._sslBundle, + self.__connection._timeout, + self.__connection._reuse_connection) return image_streamer diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py index 17877d8a..347be36f 100644 --- a/tests/unit/test_connection.py +++ b/tests/unit/test_connection.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- ### -# (C) Copyright (2016-2017) Hewlett Packard Enterprise Development LP +# (C) Copyright (2016-2018) Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -604,6 +604,25 @@ def test_download_to_stream_when_error_status_with_empty_body(self, mock_get_con else: self.fail() + @patch.object(connection, 'get_connection') + def test_reuse_connection(self, mock_get_conn): + mock_conn = Mock() + mock_get_conn.return_value = mock_conn + + mock_response = mock_conn.getresponse.return_value + mock_response.read.return_value = json.dumps('').encode('utf-8') + mock_response.status = 202 + + try: + self.connection._reuse_connection = True + self.connection.do_http('GET', '/rest', None) + self.assertEqual(self.connection._conn, mock_conn) + self.connection.close_reusable_connection(mock_conn) + self.assertEqual(self.connection._conn, None) + finally: + self.connection._reuse_connection = False + self.connection._conn = None + @patch.object(connection, 'get_connection') def test_download_to_stream_with_timeout_error(self, mock_get_connection): diff --git a/tests/unit/test_oneview_client.py b/tests/unit/test_oneview_client.py index a5898aad..5a96f400 100755 --- a/tests/unit/test_oneview_client.py +++ b/tests/unit/test_oneview_client.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- ### -# (C) Copyright (2012-2017) Hewlett Packard Enterprise Development LP +# (C) Copyright (2012-2018) Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -100,7 +100,8 @@ 'ONEVIEWSDK_API_VERSION': '201', 'ONEVIEWSDK_AUTH_LOGIN_DOMAIN': 'authdomain', 'ONEVIEWSDK_PROXY': '172.16.100.195:9999', - 'ONEVIEWSDK_CONNECTION_TIMEOUT': '20' + 'ONEVIEWSDK_CONNECTION_TIMEOUT': '20', + 'ONEVIEWSDK_REUSE_CONNECTION': 'Yes' } OS_ENVIRON_CONFIG_FULL_WITH_SESSIONID = { @@ -111,8 +112,8 @@ 'ONEVIEWSDK_SESSIONID': '123', 'ONEVIEWSDK_API_VERSION': '201', 'ONEVIEWSDK_PROXY': '172.16.100.195:9999', - 'ONEVIEWSDK_CONNECTION_TIMEOUT': '20' - + 'ONEVIEWSDK_CONNECTION_TIMEOUT': '20', + 'ONEVIEWSDK_REUSE_CONNECTION': 'Yes' } @@ -300,6 +301,7 @@ def test_from_environment_variables_is_passing_right_arguments_to_the_constructo mock_cls.assert_called_once_with({'api_version': 201, 'proxy': '172.16.100.195:9999', 'timeout': '20', + 'reuse_connection': True, 'ip': '172.16.100.199', 'ssl_certificate': '', 'image_streamer_ip': '172.172.172.172', @@ -317,6 +319,7 @@ def test_from_environment_variables_is_passing_right_arguments_to_the_constructo mock_cls.assert_called_once_with({'api_version': 201, 'proxy': '172.16.100.195:9999', 'timeout': '20', + 'reuse_connection': True, 'ip': '172.16.100.199', 'image_streamer_ip': '172.172.172.172', 'ssl_certificate': '', @@ -334,6 +337,7 @@ def test_from_environment_variables_is_passing_right_arguments_to_the_constructo mock_cls.assert_called_once_with({'api_version': 300, 'proxy': '', 'timeout': None, + 'reuse_connection': False, 'ip': '172.16.100.199', 'image_streamer_ip': '', 'ssl_certificate': '',