Check VMware session before uploading image

Uploading an image to vSphere backend without checking the session
results in broken pipe socket error. When this happens, glance-api
sends a 400 response because the IOError is not handled by the
store.

We address this issue by :-
1. Checking if session is authenticated before uploading the image.
2. Handle IOError and check the response code.

Closes-Bug: #1402354

Change-Id: I66b6dfddfb2ddd089488f3f79f3917fd69739fc9

2 files changed, 59 insertions, 11 deletions

diff --git a/glance_store/_drivers/vmware_datastore.py b/glance_store/_drivers/vmware_datastore.py
index 0e1b649..81297b4 100644
--- a/glance_store/_drivers/vmware_datastore.py
+++ b/glance_store/_drivers/vmware_datastore.py
@@ -294,8 +294,11 @@ class Store(glance_store.Store):
                 store_name='vmware_datastore', reason=reason)
         return result
 
-    def _build_vim_cookie_header(self, vim_cookies):
+    def _build_vim_cookie_header(self, verify_session=False):
         """Build ESX host session cookie header."""
+        if verify_session and not self.session.is_current_session_active():
+            self.reset_session(force=True)
+        vim_cookies = self.session.vim.client.options.transport.cookiejar
         if len(list(vim_cookies)) > 0:
             cookie = list(vim_cookies)[0]
             return cookie.name + '=' + cookie.value
@@ -332,18 +335,31 @@ class Store(glance_store.Store):
                              'datastore_name': self.datastore_name,
                              'image_id': image_id}, self.conf)
         # NOTE(arnaud): use a decorator when the config is not tied to self
-        cookie = self._build_vim_cookie_header(
-            self.session.vim.client.options.transport.cookiejar)
+        cookie = self._build_vim_cookie_header(True)
         headers = dict(headers.items() + {'Cookie': cookie}.items())
+        conn_class = self._get_http_conn_class()
+        conn = conn_class(loc.server_host)
+        url = urlparse.quote('%s?%s' % (loc.path, loc.query))
         try:
-            conn = self._get_http_conn('PUT', loc, headers,
-                                       content=image_file)
-            res = conn.getresponse()
+            conn.request('PUT', url, image_file, headers)
+        except IOError as e:
+            # When a session is not authenticated, the socket is closed by
+            # the server after sending the response. httplib has an open
+            # issue with https that raises Broken Pipe
+            # error instead of returning the response.
+            # See http://bugs.python.org/issue16062. Here, we log the error
+            # and continue to look into the response.
+            msg = _LE('Communication error sending http %(method)s request'
+                      'to the url %(url)s.\n'
+                      'Got IOError %(e)s') % {'method': 'PUT',
+                                              'url': url,
+                                              'e': e}
+            LOG.error(msg)
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.exception(_LE('Failed to upload content of image '
                                   '%(image)s'), {'image': image_id})
-
+        res = conn.getresponse()
         if res.status == httplib.CONFLICT:
             raise exceptions.Duplicate(_("Image file %(image_id)s already "
                                          "exists!") %
@@ -429,8 +445,7 @@ class Store(glance_store.Store):
         loc = location.store_location
         # NOTE(arnaud): use a decorator when the config is not tied to self
         for i in range(self.api_retry_count + 1):
-            cookie = self._build_vim_cookie_header(
-                self.session.vim.client.options.transport.cookiejar)
+            cookie = self._build_vim_cookie_header()
             headers = {'Cookie': cookie}
             try:
                 conn = self._get_http_conn(method, loc, headers)
diff --git a/tests/unit/test_vmware_store.py b/tests/unit/test_vmware_store.py
index 685dd30..43c62f4 100644
--- a/tests/unit/test_vmware_store.py
+++ b/tests/unit/test_vmware_store.py
@@ -80,7 +80,7 @@ class FakeHTTPConnection(object):
 
 class TestStore(base.StoreBaseTest):
 
-    @mock.patch('oslo.vmware.api.VMwareAPISession', auptospec=True)
+    @mock.patch('oslo.vmware.api.VMwareAPISession', autospec=True)
     def setUp(self, mock_session):
         """Establish a clean test environment."""
         super(TestStore, self).setUp()
@@ -102,7 +102,6 @@ class TestStore(base.StoreBaseTest):
 
         self.store.store_image_dir = (
             VMWARE_DS['vmware_store_image_dir'])
-        vm_store.Store._build_vim_cookie_header = mock.Mock()
 
     def test_get(self):
         """Test a "normal" retrieval of an image in chunks."""
@@ -349,3 +348,37 @@ class TestStore(base.StoreBaseTest):
         self.assertFalse(mock_api_session.called)
         self.store.reset_session(force=True)
         self.assertTrue(mock_api_session.called)
+
+    @mock.patch.object(api, 'VMwareAPISession')
+    def test_build_vim_cookie_header_active(self, mock_api_session):
+        self.store.session.is_current_session_active = mock.Mock()
+        self.store.session.is_current_session_active.return_value = True
+        self.store._build_vim_cookie_header(True)
+        self.assertFalse(mock_api_session.called)
+
+    @mock.patch.object(api, 'VMwareAPISession')
+    def test_build_vim_cookie_header_expired(self, mock_api_session):
+        self.store.session.is_current_session_active = mock.Mock()
+        self.store.session.is_current_session_active.return_value = False
+        self.store._build_vim_cookie_header(True)
+        self.assertTrue(mock_api_session.called)
+
+    @mock.patch.object(api, 'VMwareAPISession')
+    def test_build_vim_cookie_header_expired_noverify(self, mock_api_session):
+        self.store.session.is_current_session_active = mock.Mock()
+        self.store.session.is_current_session_active.return_value = False
+        self.store._build_vim_cookie_header()
+        self.assertFalse(mock_api_session.called)
+
+    @mock.patch.object(api, 'VMwareAPISession')
+    def test_add_ioerror(self, mock_api_session):
+        expected_image_id = str(uuid.uuid4())
+        expected_size = FIVE_KB
+        expected_contents = "*" * expected_size
+        image = six.StringIO(expected_contents)
+        self.session = mock.Mock()
+        with mock.patch('httplib.HTTPConnection') as HttpConn:
+            HttpConn.request.side_effect = IOError
+            self.assertRaises(exceptions.BackendException,
+                              self.store.add,
+                              expected_image_id, image, expected_size)