diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py index 227ea427b6..bdf9798cc4 100644 --- a/glance/api/v2/image_data.py +++ b/glance/api/v2/image_data.py @@ -12,6 +12,8 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import os + from cursive import exception as cursive_exception import glance_store from glance_store import backend @@ -73,14 +75,23 @@ class ImageDataController(object): :param image: The image will be restored :param staging_store: The store used for staging """ - loc = glance_store.location.get_location_from_uri(str( - CONF.node_staging_uri + '/' + image.image_id)) - try: - staging_store.delete(loc) - except glance_store.exceptions.NotFound: - pass - finally: - self._restore(image_repo, image) + # NOTE(abhishek): staging_store not being used in this function + # because of bug #1803498 + # TODO(abhishek): refactor to use the staging_store when the + # "Rethinking Filesystem Access" spec is implemented in Train + file_path = str(CONF.node_staging_uri + '/' + image.image_id)[7:] + if os.path.exists(file_path): + try: + os.unlink(file_path) + except OSError as e: + LOG.error(_("Cannot delete staged image data %(fn)s " + "[Errno %(en)d]"), {'fn': file_path, + 'en': e.errno}) + else: + LOG.warning(_("Staged image data not found " + "at %(fn)s"), {'fn': file_path}) + + self._restore(image_repo, image) def _delete(self, image_repo, image): """Delete the image. diff --git a/glance/async_/flows/api_image_import.py b/glance/async_/flows/api_image_import.py index 0511f18c18..b789db37b4 100644 --- a/glance/async_/flows/api_image_import.py +++ b/glance/async_/flows/api_image_import.py @@ -12,8 +12,8 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import os -import glance_store as store_api from glance_store import backend from oslo_config import cfg from oslo_log import log as logging @@ -86,10 +86,23 @@ class _DeleteFromFS(task.Task): :param file_path: path to the file being deleted """ - if CONF.enabled_backends: - store_api.delete(file_path, None) + # TODO(abhishekk): After removal of backend module from glance_store + # need to change this to use multi_backend module. + file_path = file_path[7:] + if os.path.exists(file_path): + try: + LOG.debug(_("After upload to the backend, deleting staged " + "image data from %(fn)s"), {'fn': file_path}) + os.unlink(file_path) + except OSError as e: + LOG.error(_("After upload to backend, deletion of staged " + "image data from %(fn)s has failed because " + "[Errno %(en)d]"), {'fn': file_path, + 'en': e.errno}) else: - store_api.delete_from_backend(file_path) + LOG.warning(_("After upload to backend, deletion of staged " + "image data has failed because " + "it cannot be found at %(fn)s"), {'fn': file_path}) class _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 704870c6b2..66c63332af 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 @@ # under the License. import mock +import os from glance_store._drivers import filesystem from glance_store import backend from oslo_config import cfg from glance.async_.flows._internal_plugins import web_download - +from glance.async_.flows import api_image_import import glance.common.exception import glance.common.scripts.utils as script_utils from glance import domain @@ -96,3 +97,40 @@ class TestWebDownloadTask(test_utils.BaseTestCase): mock_iter.side_effect = glance.common.exception.NotFound self.assertRaises(glance.common.exception.NotFound, web_download_task.execute) + + def test_web_download_delete_staging_image_not_exist(self): + staging_path = "file:///tmp/staging/temp-image" + delete_from_fs_task = api_image_import._DeleteFromFS( + self.task.task_id, self.task_type) + with mock.patch.object(os.path, "exists") as mock_exists: + mock_exists.return_value = False + with mock.patch.object(os, "unlink") as mock_unlik: + delete_from_fs_task.execute(staging_path) + + self.assertEqual(1, mock_exists.call_count) + self.assertEqual(0, mock_unlik.call_count) + + @mock.patch.object(os.path, "exists") + def test_web_download_delete_staging_image_failed(self, mock_exists): + mock_exists.return_value = True + staging_path = "file:///tmp/staging/temp-image" + delete_from_fs_task = api_image_import._DeleteFromFS( + self.task.task_id, self.task_type) + with mock.patch.object(os, "unlink") as mock_unlink: + try: + delete_from_fs_task.execute(staging_path) + except OSError: + self.assertEqual(1, mock_unlink.call_count) + + self.assertEqual(1, mock_exists.call_count) + + @mock.patch.object(os.path, "exists") + def test_web_download_delete_staging_image_succeed(self, mock_exists): + mock_exists.return_value = True + staging_path = "file:///tmp/staging/temp-image" + delete_from_fs_task = api_image_import._DeleteFromFS( + self.task.task_id, self.task_type) + with mock.patch.object(os, "unlink") as mock_unlik: + delete_from_fs_task.execute(staging_path) + self.assertEqual(1, mock_exists.call_count) + self.assertEqual(1, mock_unlik.call_count)