From 6982284218a95d5ddc2625b53ebfc684720c0447 Mon Sep 17 00:00:00 2001 From: ankitagrawal Date: Fri, 10 Oct 2014 08:17:17 -0700 Subject: [PATCH] Glance returns HTTP 500 for image download HTTPInternalServerError 500 response is returned to the user while image server is down during downloading the image. When glance tries to download the image from the remote location (image server) which is down, Connection refused ECONNREFUSED error is raised on the glance server. Raised RemoteServiceUnavailable exception from glance_store and handle it in v1 and v2 api's to return 503 HTTPServiceUnavailable response to user. Note: Please refer below link to check glance_store related changes which addresses this issue partially. I065b9a3e8e674ea74ff8563aad99d7d022417caa Closes-Bug: #1379798 Change-Id: I45099153e75d53b028e249fad8a4d944d38adf65 --- glance/api/v1/images.py | 3 + glance/api/v2/image_data.py | 2 + glance/tests/functional/v2/test_images.py | 59 +++++++++++++++++++ glance/tests/unit/v1/test_api.py | 16 +++++ .../tests/unit/v2/test_image_data_resource.py | 15 +++++ 5 files changed, 95 insertions(+) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 292effd0db..3578960271 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -31,6 +31,7 @@ from webob.exc import HTTPForbidden from webob.exc import HTTPMethodNotAllowed from webob.exc import HTTPNotFound from webob.exc import HTTPRequestEntityTooLarge +from webob.exc import HTTPServiceUnavailable from webob import Response from glance.api import common @@ -453,6 +454,8 @@ class Controller(controller.BaseController): image_data, image_size = src_store.get(loc, context=context) + except store.RemoteServiceUnavailable as e: + raise HTTPServiceUnavailable(explanation=e.msg) except store.NotFound as e: raise HTTPNotFound(explanation=e.msg) image_size = int(image_size) if image_size else None diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py index 928b3badbe..94a2d927b8 100644 --- a/glance/api/v2/image_data.py +++ b/glance/api/v2/image_data.py @@ -218,6 +218,8 @@ class ResponseSerializer(wsgi.JSONResponseSerializer): chunk_size=chunk_size)) except glance_store.NotFound as e: raise webob.exc.HTTPNotFound(explanation=e.msg) + except glance_store.RemoteServiceUnavailable as e: + raise webob.exc.HTTPServiceUnavailable(explanation=e.msg) except exception.Forbidden as e: raise webob.exc.HTTPForbidden(explanation=e.msg) # NOTE(saschpe): "response.app_iter = ..." currently resets Content-MD5 diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index 0f3447f902..beab543cba 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -716,6 +716,65 @@ class TestImages(functional.FunctionalTest): self.stop_servers() + def test_download_image_raises_service_unavailable(self): + """Test image download returns HTTPServiceUnavailable.""" + self.start_servers(**self.__dict__.copy()) + + # Create an image + path = self._url('/v2/images') + headers = self._headers({'content-type': 'application/json'}) + data = jsonutils.dumps({'name': 'image-1', + 'disk_format': 'aki', + 'container_format': 'aki'}) + response = requests.post(path, headers=headers, data=data) + self.assertEqual(201, response.status_code) + + # Get image id + image = jsonutils.loads(response.text) + image_id = image['id'] + + # Update image locations via PATCH + path = self._url('/v2/images/%s' % image_id) + media_type = 'application/openstack-images-v2.1-json-patch' + headers = self._headers({'content-type': media_type}) + http_server_pid, http_port = test_utils.start_http_server(image_id, + "image-1") + values = [{'url': 'http://127.0.0.1:%s/image-1' % http_port, + 'metadata': {'idx': '0'}}] + doc = [{'op': 'replace', + 'path': '/locations', + 'value': values}] + data = jsonutils.dumps(doc) + response = requests.patch(path, headers=headers, data=data) + self.assertEqual(200, response.status_code) + + # Download an image should work + path = self._url('/v2/images/%s/file' % image_id) + headers = self._headers({'Content-Type': 'application/json'}) + response = requests.get(path, headers=headers) + self.assertEqual(200, response.status_code) + + # Stop http server used to update image location + os.kill(http_server_pid, signal.SIGKILL) + + # Download an image should raise HTTPServiceUnavailable + path = self._url('/v2/images/%s/file' % image_id) + headers = self._headers({'Content-Type': 'application/json'}) + response = requests.get(path, headers=headers) + self.assertEqual(503, response.status_code) + + # Image Deletion should work + path = self._url('/v2/images/%s' % image_id) + response = requests.delete(path, headers=self._headers()) + self.assertEqual(204, response.status_code) + + # This image should be no longer be directly accessible + path = self._url('/v2/images/%s' % image_id) + response = requests.get(path, headers=self._headers()) + self.assertEqual(404, response.status_code) + + self.stop_servers() + def test_image_size_cap(self): self.api_server.image_size_cap = 128 self.start_servers(**self.__dict__.copy()) diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py index 40f75ebc35..fee8765759 100644 --- a/glance/tests/unit/v1/test_api.py +++ b/glance/tests/unit/v1/test_api.py @@ -2606,6 +2606,22 @@ class TestGlanceAPI(base.IsolatedUnitTest): res = req.get_response(self.api) self.assertEqual(403, res.status_int) + def test_download_service_unavailable(self): + """Test image download returns HTTPServiceUnavailable.""" + image_fixture = self.FIXTURES[1] + image_fixture.update({'location': 'http://netloc/path/to/file.tar.gz'}) + request = webob.Request.blank("/images/%s" % UUID2) + request.context = self.context + + image_controller = glance.api.v1.images.Controller() + with mock.patch.object(image_controller, + 'get_active_image_meta_or_404' + ) as mocked_get_image: + mocked_get_image.return_value = image_fixture + self.assertRaises(webob.exc.HTTPServiceUnavailable, + image_controller.show, + request, mocked_get_image) + def test_delete_image(self): req = webob.Request.blank("/images/%s" % UUID2) req.method = 'DELETE' diff --git a/glance/tests/unit/v2/test_image_data_resource.py b/glance/tests/unit/v2/test_image_data_resource.py index 807238723f..4d961efb5f 100644 --- a/glance/tests/unit/v2/test_image_data_resource.py +++ b/glance/tests/unit/v2/test_image_data_resource.py @@ -492,6 +492,21 @@ class TestImageDataSerializer(test_utils.BaseTestCase): self.serializer.download, response, image) + def test_download_service_unavailable(self): + """Test image download returns HTTPServiceUnavailable.""" + with mock.patch.object(glance.api.policy.ImageProxy, + 'get_data') as mock_get_data: + mock_get_data.side_effect = glance_store.RemoteServiceUnavailable() + + request = wsgi.Request.blank('/') + response = webob.Response() + response.request = request + image = FakeImage(size=3, data=iter('ZZZ')) + image.get_data = mock_get_data + self.assertRaises(webob.exc.HTTPServiceUnavailable, + self.serializer.download, + response, image) + def test_upload(self): request = webob.Request.blank('/') request.environ = {}