From 50e3a7c58a9862206d92fef577540c5b144ecbf0 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Sun, 20 Sep 2015 17:01:22 +0300 Subject: [PATCH] Cleanup chunks for deleted image if token expired In patch I47229b366c25367ec1bd48aec684e0880f3dfe60 it was introduced the logic that if image was deleted during file upload when we want to update image status from 'saving' to 'active' it's expected to get Duplicate error and delete stale chunks after that. But if user's token is expired there will be Unathorized exception and chunks will stay in store and clog it. And when, the upload operation for such an image is completed the operator configured quota can be exceeded. This patch fixes the issue of left over chunks for an image which was deleted from saving status, by correcly handle auth exceptions from registry server. Change-Id: I17a66eca55bfb83107046910e69c4da01415deec (cherry picked from commit 98a8832777a0639a4031e52c69f0d565b3f500c5) Conflicts: glance/api/v1/upload_utils.py (Kilo catches NotFound instead of ImagenotFound) --- glance/api/v1/upload_utils.py | 8 ++++++++ glance/api/v2/image_data.py | 14 ++++++++++++- glance/tests/unit/v1/test_upload_utils.py | 26 ++++++++++++++++++++++++ glance/tests/unit/v2/test_image_data_resource.py | 17 ++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/glance/api/v1/upload_utils.py b/glance/api/v1/upload_utils.py index 7adb2dce12a3bad025fb61bf6e4fc91b1f1b9892..ad4f724c6285d14b79cebaa72d709aa77dd98fca 100644 --- a/glance/api/v1/upload_utils.py +++ b/glance/api/v1/upload_utils.py @@ -171,6 +171,14 @@ def upload_data_to_store(req, image_meta, image_data, store, notifier): raise exception.NotFound() else: raise + + except exception.NotAuthenticated as e: + # Delete image data due to possible token expiration. + LOG.debug("Authentication error - the token may have " + "expired during file upload. Deleting image data for " + " %s " % image_id) + initiate_deletion(req, location_data, image_id) + raise webob.exc.HTTPUnauthorized(explanation=e.msg, request=req) except exception.NotFound: msg = _LI("Image %s could not be found after upload. The image may" " have been deleted during the upload.") % image_id diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py index 4025eebde0a296d598ed2ba8f77349f968847f66..9967662b37297a9872dd519a192b9ef9eb91495f 100644 --- a/glance/api/v2/image_data.py +++ b/glance/api/v2/image_data.py @@ -88,7 +88,19 @@ class ImageDataController(object): raise webob.exc.HTTPGone(explanation=msg, request=req, content_type='text/plain') - + except exception.NotAuthenticated: + msg = (_("Authentication error - the token may have " + "expired during file upload. Deleting image data for " + "%s.") % image_id) + LOG.debug(msg) + try: + image.delete() + except exception.NotAuthenticated: + # NOTE: Ignore this exception + pass + raise webob.exc.HTTPUnauthorized(explanation=msg, + request=req, + content_type='text/plain') except ValueError as e: LOG.debug("Cannot save data for image %(id)s: %(e)s", {'id': image_id, 'e': utils.exception_to_str(e)}) diff --git a/glance/tests/unit/v1/test_upload_utils.py b/glance/tests/unit/v1/test_upload_utils.py index 1afaf001a2b832f39876c6fc11fd5cb247e7bedf..8d055159fc939749d7b208bec1997ab103a2597a 100644 --- a/glance/tests/unit/v1/test_upload_utils.py +++ b/glance/tests/unit/v1/test_upload_utils.py @@ -323,3 +323,29 @@ class TestUploadUtils(base.StoreClearingUnitTest): 'metadata': {}}, image_meta['id']) mock_safe_kill.assert_called_once_with( req, image_meta['id'], 'saving') + + @mock.patch.object(registry, 'update_image_metadata', + side_effect=exception.NotAuthenticated) + @mock.patch.object(upload_utils, 'initiate_deletion') + def test_activate_image_with_expired_token( + self, mocked_delete, mocked_update): + """Test token expiration during image upload. + + If users token expired before image was uploaded then if auth error + was caught from registry during changing image status from 'saving' + to 'active' then it's required to delete all image data. + """ + context = mock.Mock() + req = mock.Mock() + req.context = context + with self._get_store_and_notifier() as (location, checksum, image_meta, + image_data, store, notifier, + update_data): + self.assertRaises(webob.exc.HTTPUnauthorized, + upload_utils.upload_data_to_store, + req, image_meta, image_data, store, notifier) + self.assertEqual(2, mocked_update.call_count) + mocked_delete.assert_called_once_with( + req, + {'url': 'file://foo/bar', 'status': 'active', 'metadata': {}}, + 'c80a1a6c-bd1f-41c5-90ee-81afedb1d58d') diff --git a/glance/tests/unit/v2/test_image_data_resource.py b/glance/tests/unit/v2/test_image_data_resource.py index bc8891e76876fb937ada784e87e856b135c61c8c..7458eda019085f49a7ee5fe52b2e4e7fb61d8d1d 100644 --- a/glance/tests/unit/v2/test_image_data_resource.py +++ b/glance/tests/unit/v2/test_image_data_resource.py @@ -192,6 +192,23 @@ class TestImagesController(base.StoreClearingUnitTest): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.upload, request, unit_test_utils.UUID1, 'YYYY', 4) + def test_upload_with_expired_token(self): + def side_effect(image, from_state=None): + if from_state == 'saving': + raise exception.NotAuthenticated() + + mocked_save = mock.Mock(side_effect=side_effect) + mocked_delete = mock.Mock() + request = unit_test_utils.get_fake_request() + image = FakeImage('abcd') + image.delete = mocked_delete + self.image_repo.result = image + self.image_repo.save = mocked_save + self.assertRaises(webob.exc.HTTPUnauthorized, self.controller.upload, + request, unit_test_utils.UUID1, 'YYYY', 4) + self.assertEqual(3, mocked_save.call_count) + mocked_delete.assert_called_once_with() + def test_upload_non_existent_image_during_save_initiates_deletion(self): def fake_save_not_found(self): raise exception.NotFound() -- 2.5.0