openstack/glance
Code Issues Proposed changes

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

Browse Source 
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
This commit is contained in:
Abhishek Kekane 2019-03-13 18:12:42 +00:00
parent d501799a6a
commit c927246085
3 changed files with 75 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
glance/api/v2/image_data.py
View File

@ -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.

21
glance/async_/flows/api_image_import.py
View File

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

40
glance/tests/unit/async_/flows/test_web_download.py
View File

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