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.

Author: eamonnotoole

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

README.md

@@ -99,6 +99,7 @@ export ONEVIEWSDK_AUTH_LOGIN_DOMAIN='authdomain'
 export ONEVIEWSDK_SSL_CERTIFICATE='<path_to_cert.crt_file>'
 export ONEVIEWSDK_PROXY='<proxy_host>:<proxy_port>'
 export ONEVIEWSDK_CONNECTION_TIMEOUT='<connection time-out in seconds>'
+export ONEVIEWSDK_REUSE_CONNECTION='<string>'
 ```
 
 :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='<connection time-out in seconds>'
 "timeout": <timeout in seconds>
 ```
 
+
+### 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='<any non-null string, eg Yes>'
+```
+
+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.

hpOneView/connection.py

@@ -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,23 +137,25 @@ 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:
                     try:
                         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,29 @@ 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()
+        if self._reuse_connection:
+            self._conn = None
+
     def get_connection(self):
         context = ssl.SSLContext(ssl.PROTOCOL_TLSv1_2)
         if self._sslTrustAll is False:
@@ -290,7 +313,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 +323,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 +338,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 +348,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

hpOneView/image_streamer/image_streamer_client.py

@@ -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

hpOneView/oneview_client.py

@@ -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
 

tests/unit/test_connection.py

@@ -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):
 

tests/unit/test_oneview_client.py

@@ -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': '',