From e1ee81a37fb7504ed482225b73212082e5d7c4b1 Mon Sep 17 00:00:00 2001 From: Sabari Kumar Murugesan Date: Sun, 14 Dec 2014 01:35:21 -0800 Subject: 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 --- glance_store/_drivers/vmware_datastore.py | 33 +++++++++++++++++++-------- tests/unit/test_vmware_store.py | 37 +++++++++++++++++++++++++++++-- 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) -- cgit v1.2.1