|
1 From 5447e8419d92f0cbb14de53b207e772ce9067933 Mon Sep 17 00:00:00 2001 |
|
2 From: Mike Fedosin <[email protected]> |
|
3 Date: Sun, 20 Sep 2015 17:01:22 +0300 |
|
4 Subject: [PATCH] Cleanup chunks for deleted image if token expired |
|
5 |
|
6 In patch I47229b366c25367ec1bd48aec684e0880f3dfe60 it was |
|
7 introduced the logic that if image was deleted during file |
|
8 upload when we want to update image status from 'saving' |
|
9 to 'active' it's expected to get Duplicate error and delete |
|
10 stale chunks after that. But if user's token is expired |
|
11 there will be Unathorized exception and chunks will stay |
|
12 in store and clog it. |
|
13 And when, the upload operation for such an image is |
|
14 completed the operator configured quota can be exceeded. |
|
15 |
|
16 This patch fixes the issue of left over chunks for an image |
|
17 which was deleted from saving status, by correcly handle |
|
18 auth exceptions from registry server. |
|
19 |
|
20 This patch fixes the issue of left over chunks for an image |
|
21 which was deleted from saving status, by correctly handle |
|
22 auth exceptions from registry server. |
|
23 |
|
24 Partial-bug: #1498163 |
|
25 |
|
26 Conflicts: |
|
27 glance/api/v1/upload_utils.py |
|
28 (Kilo catches NotFound instead of ImagenotFound) |
|
29 |
|
30 Change-Id: I17a66eca55bfb83107046910e69c4da01415deec |
|
31 (cherry picked from commit 50e3a7c58a9862206d92fef577540c5b144ecbf0) |
|
32 --- |
|
33 glance/api/v1/upload_utils.py | 8 ++++++++ |
|
34 glance/api/v2/image_data.py | 14 ++++++++++++- |
|
35 glance/tests/unit/v1/test_upload_utils.py | 26 ++++++++++++++++++++++++ |
|
36 glance/tests/unit/v2/test_image_data_resource.py | 17 ++++++++++++++++ |
|
37 4 files changed, 64 insertions(+), 1 deletion(-) |
|
38 |
|
39 diff --git a/glance/api/v1/upload_utils.py b/glance/api/v1/upload_utils.py |
|
40 index e587319..f9bdc29 100644 |
|
41 --- a/glance/api/v1/upload_utils.py |
|
42 +++ b/glance/api/v1/upload_utils.py |
|
43 @@ -171,6 +171,14 @@ def upload_data_to_store(req, image_meta, image_data, store, notifier): |
|
44 raise exception.NotFound() |
|
45 else: |
|
46 raise |
|
47 + |
|
48 + except exception.NotAuthenticated as e: |
|
49 + # Delete image data due to possible token expiration. |
|
50 + LOG.debug("Authentication error - the token may have " |
|
51 + "expired during file upload. Deleting image data for " |
|
52 + " %s " % image_id) |
|
53 + initiate_deletion(req, location_data, image_id) |
|
54 + raise webob.exc.HTTPUnauthorized(explanation=e.msg, request=req) |
|
55 except exception.NotFound: |
|
56 msg = _LI("Image %s could not be found after upload. The image may" |
|
57 " have been deleted during the upload.") % image_id |
|
58 diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py |
|
59 index cdfa34b..ee9d0ba 100644 |
|
60 --- a/glance/api/v2/image_data.py |
|
61 +++ b/glance/api/v2/image_data.py |
|
62 @@ -90,7 +90,19 @@ class ImageDataController(object): |
|
63 raise webob.exc.HTTPGone(explanation=msg, |
|
64 request=req, |
|
65 content_type='text/plain') |
|
66 - |
|
67 + except exception.NotAuthenticated: |
|
68 + msg = (_("Authentication error - the token may have " |
|
69 + "expired during file upload. Deleting image data for " |
|
70 + "%s.") % image_id) |
|
71 + LOG.debug(msg) |
|
72 + try: |
|
73 + image.delete() |
|
74 + except exception.NotAuthenticated: |
|
75 + # NOTE: Ignore this exception |
|
76 + pass |
|
77 + raise webob.exc.HTTPUnauthorized(explanation=msg, |
|
78 + request=req, |
|
79 + content_type='text/plain') |
|
80 except ValueError as e: |
|
81 LOG.debug("Cannot save data for image %(id)s: %(e)s", |
|
82 {'id': image_id, 'e': utils.exception_to_str(e)}) |
|
83 diff --git a/glance/tests/unit/v1/test_upload_utils.py b/glance/tests/unit/v1/test_upload_utils.py |
|
84 index 083cda3..f561dbe 100644 |
|
85 --- a/glance/tests/unit/v1/test_upload_utils.py |
|
86 +++ b/glance/tests/unit/v1/test_upload_utils.py |
|
87 @@ -323,3 +323,29 @@ class TestUploadUtils(base.StoreClearingUnitTest): |
|
88 'metadata': {}}, image_meta['id']) |
|
89 mock_safe_kill.assert_called_once_with( |
|
90 req, image_meta['id'], 'saving') |
|
91 + |
|
92 + @mock.patch.object(registry, 'update_image_metadata', |
|
93 + side_effect=exception.NotAuthenticated) |
|
94 + @mock.patch.object(upload_utils, 'initiate_deletion') |
|
95 + def test_activate_image_with_expired_token( |
|
96 + self, mocked_delete, mocked_update): |
|
97 + """Test token expiration during image upload. |
|
98 + |
|
99 + If users token expired before image was uploaded then if auth error |
|
100 + was caught from registry during changing image status from 'saving' |
|
101 + to 'active' then it's required to delete all image data. |
|
102 + """ |
|
103 + context = mock.Mock() |
|
104 + req = mock.Mock() |
|
105 + req.context = context |
|
106 + with self._get_store_and_notifier() as (location, checksum, image_meta, |
|
107 + image_data, store, notifier, |
|
108 + update_data): |
|
109 + self.assertRaises(webob.exc.HTTPUnauthorized, |
|
110 + upload_utils.upload_data_to_store, |
|
111 + req, image_meta, image_data, store, notifier) |
|
112 + self.assertEqual(2, mocked_update.call_count) |
|
113 + mocked_delete.assert_called_once_with( |
|
114 + req, |
|
115 + {'url': 'file://foo/bar', 'status': 'active', 'metadata': {}}, |
|
116 + 'c80a1a6c-bd1f-41c5-90ee-81afedb1d58d') |
|
117 diff --git a/glance/tests/unit/v2/test_image_data_resource.py b/glance/tests/unit/v2/test_image_data_resource.py |
|
118 index a121e82..b7bacab 100644 |
|
119 --- a/glance/tests/unit/v2/test_image_data_resource.py |
|
120 +++ b/glance/tests/unit/v2/test_image_data_resource.py |
|
121 @@ -183,6 +183,23 @@ class TestImagesController(base.StoreClearingUnitTest): |
|
122 self.assertRaises(webob.exc.HTTPBadRequest, self.controller.upload, |
|
123 request, unit_test_utils.UUID1, 'YYYY', 4) |
|
124 |
|
125 + def test_upload_with_expired_token(self): |
|
126 + def side_effect(image, from_state=None): |
|
127 + if from_state == 'saving': |
|
128 + raise exception.NotAuthenticated() |
|
129 + |
|
130 + mocked_save = mock.Mock(side_effect=side_effect) |
|
131 + mocked_delete = mock.Mock() |
|
132 + request = unit_test_utils.get_fake_request() |
|
133 + image = FakeImage('abcd') |
|
134 + image.delete = mocked_delete |
|
135 + self.image_repo.result = image |
|
136 + self.image_repo.save = mocked_save |
|
137 + self.assertRaises(webob.exc.HTTPUnauthorized, self.controller.upload, |
|
138 + request, unit_test_utils.UUID1, 'YYYY', 4) |
|
139 + self.assertEqual(3, mocked_save.call_count) |
|
140 + mocked_delete.assert_called_once_with() |
|
141 + |
|
142 def test_upload_non_existent_image_during_save_initiates_deletion(self): |
|
143 def fake_save_not_found(self): |
|
144 raise exception.NotFound() |
|
145 -- |
|
146 1.9.1 |
|
147 |