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.

Conflicts:
      glance/tests/unit/async/flows/test_web_download.py

NOTE: Renamed import statement from above test files as async_
module is not available in stable/rocky.

Closes-Bug: #1803498
Change-Id: If0b3b0af9300301291758c67267890e0959ebb3c
(cherry picked from commit c927246085)
This commit is contained in:
Abhishek Kekane 2019-03-13 18:12:42 +00:00
parent 893d337c01
commit 8c3751db21
3 changed files with 86 additions and 14 deletions

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.

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
@ -28,7 +28,7 @@ import glance.async.flows.plugins as import_plugins
from glance.common import exception
from glance.common.scripts.image_import import main as image_import
from glance.common.scripts import utils as script_utils
from glance.i18n import _, _LE, _LI
from glance.i18n import _, _LE, _LI, _LW
LOG = logging.getLogger(__name__)
@ -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(_LW("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):

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
@ -86,3 +87,50 @@ class TestWebDownloadTask(test_utils.BaseTestCase):
self.task_type, self.task_repo, self.image_id,
self.uri)
mock_override.assert_called()
def test_web_download_failed(self):
web_download_task = web_download._WebDownload(
self.task.task_id, self.task_type, self.task_repo,
self.image_id, self.uri)
with mock.patch.object(script_utils,
"get_image_data_iter") as mock_iter:
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)