summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAbhishek Kekane <akekane@redhat.com>2019-03-13 18:12:42 +0000
committerAbhishek Kekane <akekane@redhat.com>2019-03-15 12:32:00 +0000
commitc92724608512d42b56152d199cc0fea049e4d4a7 (patch)
tree40afa113117f56a52d9429d695e4c906dc6595ab
parentd501799a6a13e36b8160af497af13aaabea2c38f (diff)
Data remains in staging area if 'file' store is not enabledHEADmaster
When operator has not enabled 'file' store and using other stores like ceph, swift etc. the uploading to staging area works as we explicitly build 'file' store during this operation, while cleaning up we directly use 'glance_store.delete_from_backend' which only works if 'file' store is enabled. Modified '_DeleteFromFS' task and _unstage call which will use os module to unlink the file present in staging area explicitly to delete the data from staging area. Closes-Bug: #1803498 Change-Id: If0b3b0af9300301291758c67267890e0959ebb3c
Notes
Notes (review): Code-Review+2: Brian Rosmaita <rosmaita.fossdev@gmail.com> Review-Priority+1: Brian Rosmaita <rosmaita.fossdev@gmail.com> Code-Review+1: limeng <limeng@awcloud.com> Code-Review+2: Sean McGinnis <sean.mcginnis@gmail.com> Workflow+1: Sean McGinnis <sean.mcginnis@gmail.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Tue, 19 Mar 2019 15:33:18 +0000 Reviewed-on: https://review.openstack.org/618468 Project: openstack/glance Branch: refs/heads/master
-rw-r--r--glance/api/v2/image_data.py27
-rw-r--r--glance/async_/flows/api_image_import.py21
-rw-r--r--glance/tests/unit/async_/flows/test_web_download.py40
3 files changed, 75 insertions, 13 deletions
diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py
index 227ea42..bdf9798 100644
--- a/glance/api/v2/image_data.py
+++ b/glance/api/v2/image_data.py
@@ -12,6 +12,8 @@
12# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the 12# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13# License for the specific language governing permissions and limitations 13# License for the specific language governing permissions and limitations
14# under the License. 14# under the License.
15import os
16
15from cursive import exception as cursive_exception 17from cursive import exception as cursive_exception
16import glance_store 18import glance_store
17from glance_store import backend 19from glance_store import backend
@@ -73,14 +75,23 @@ class ImageDataController(object):
73 :param image: The image will be restored 75 :param image: The image will be restored
74 :param staging_store: The store used for staging 76 :param staging_store: The store used for staging
75 """ 77 """
76 loc = glance_store.location.get_location_from_uri(str( 78 # NOTE(abhishek): staging_store not being used in this function
77 CONF.node_staging_uri + '/' + image.image_id)) 79 # because of bug #1803498
78 try: 80 # TODO(abhishek): refactor to use the staging_store when the
79 staging_store.delete(loc) 81 # "Rethinking Filesystem Access" spec is implemented in Train
80 except glance_store.exceptions.NotFound: 82 file_path = str(CONF.node_staging_uri + '/' + image.image_id)[7:]
81 pass 83 if os.path.exists(file_path):
82 finally: 84 try:
83 self._restore(image_repo, image) 85 os.unlink(file_path)
86 except OSError as e:
87 LOG.error(_("Cannot delete staged image data %(fn)s "
88 "[Errno %(en)d]"), {'fn': file_path,
89 'en': e.errno})
90 else:
91 LOG.warning(_("Staged image data not found "
92 "at %(fn)s"), {'fn': file_path})
93
94 self._restore(image_repo, image)
84 95
85 def _delete(self, image_repo, image): 96 def _delete(self, image_repo, image):
86 """Delete the image. 97 """Delete the image.
diff --git a/glance/async_/flows/api_image_import.py b/glance/async_/flows/api_image_import.py
index 0511f18..b789db3 100644
--- a/glance/async_/flows/api_image_import.py
+++ b/glance/async_/flows/api_image_import.py
@@ -12,8 +12,8 @@
12# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the 12# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13# License for the specific language governing permissions and limitations 13# License for the specific language governing permissions and limitations
14# under the License. 14# under the License.
15import os
15 16
16import glance_store as store_api
17from glance_store import backend 17from glance_store import backend
18from oslo_config import cfg 18from oslo_config import cfg
19from oslo_log import log as logging 19from oslo_log import log as logging
@@ -86,10 +86,23 @@ class _DeleteFromFS(task.Task):
86 86
87 :param file_path: path to the file being deleted 87 :param file_path: path to the file being deleted
88 """ 88 """
89 if CONF.enabled_backends: 89 # TODO(abhishekk): After removal of backend module from glance_store
90 store_api.delete(file_path, None) 90 # need to change this to use multi_backend module.
91 file_path = file_path[7:]
92 if os.path.exists(file_path):
93 try:
94 LOG.debug(_("After upload to the backend, deleting staged "
95 "image data from %(fn)s"), {'fn': file_path})
96 os.unlink(file_path)
97 except OSError as e:
98 LOG.error(_("After upload to backend, deletion of staged "
99 "image data from %(fn)s has failed because "
100 "[Errno %(en)d]"), {'fn': file_path,
101 'en': e.errno})
91 else: 102 else:
92 store_api.delete_from_backend(file_path) 103 LOG.warning(_("After upload to backend, deletion of staged "
104 "image data has failed because "
105 "it cannot be found at %(fn)s"), {'fn': file_path})
93 106
94 107
95class _VerifyStaging(task.Task): 108class _VerifyStaging(task.Task):
diff --git a/glance/tests/unit/async_/flows/test_web_download.py b/glance/tests/unit/async_/flows/test_web_download.py
index 704870c..66c6333 100644
--- a/glance/tests/unit/async_/flows/test_web_download.py
+++ b/glance/tests/unit/async_/flows/test_web_download.py
@@ -14,13 +14,14 @@
14# under the License. 14# under the License.
15 15
16import mock 16import mock
17import os
17 18
18from glance_store._drivers import filesystem 19from glance_store._drivers import filesystem
19from glance_store import backend 20from glance_store import backend
20from oslo_config import cfg 21from oslo_config import cfg
21 22
22from glance.async_.flows._internal_plugins import web_download 23from glance.async_.flows._internal_plugins import web_download
23 24from glance.async_.flows import api_image_import
24import glance.common.exception 25import glance.common.exception
25import glance.common.scripts.utils as script_utils 26import glance.common.scripts.utils as script_utils
26from glance import domain 27from glance import domain
@@ -96,3 +97,40 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
96 mock_iter.side_effect = glance.common.exception.NotFound 97 mock_iter.side_effect = glance.common.exception.NotFound
97 self.assertRaises(glance.common.exception.NotFound, 98 self.assertRaises(glance.common.exception.NotFound,
98 web_download_task.execute) 99 web_download_task.execute)
100
101 def test_web_download_delete_staging_image_not_exist(self):
102 staging_path = "file:///tmp/staging/temp-image"
103 delete_from_fs_task = api_image_import._DeleteFromFS(
104 self.task.task_id, self.task_type)
105 with mock.patch.object(os.path, "exists") as mock_exists:
106 mock_exists.return_value = False
107 with mock.patch.object(os, "unlink") as mock_unlik:
108 delete_from_fs_task.execute(staging_path)
109
110 self.assertEqual(1, mock_exists.call_count)
111 self.assertEqual(0, mock_unlik.call_count)
112
113 @mock.patch.object(os.path, "exists")
114 def test_web_download_delete_staging_image_failed(self, mock_exists):
115 mock_exists.return_value = True
116 staging_path = "file:///tmp/staging/temp-image"
117 delete_from_fs_task = api_image_import._DeleteFromFS(
118 self.task.task_id, self.task_type)
119 with mock.patch.object(os, "unlink") as mock_unlink:
120 try:
121 delete_from_fs_task.execute(staging_path)
122 except OSError:
123 self.assertEqual(1, mock_unlink.call_count)
124
125 self.assertEqual(1, mock_exists.call_count)
126
127 @mock.patch.object(os.path, "exists")
128 def test_web_download_delete_staging_image_succeed(self, mock_exists):
129 mock_exists.return_value = True
130 staging_path = "file:///tmp/staging/temp-image"
131 delete_from_fs_task = api_image_import._DeleteFromFS(
132 self.task.task_id, self.task_type)
133 with mock.patch.object(os, "unlink") as mock_unlik:
134 delete_from_fs_task.execute(staging_path)
135 self.assertEqual(1, mock_exists.call_count)
136 self.assertEqual(1, mock_unlik.call_count)