Data remains in staging area if 'file' store is not enabled

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

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.

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):

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)