Fix: Interrupted copy-image leaking data on subsequent operation
If copying existing image in other stores fails while staging the data to
staging directory due to power, network or any other reason. Then
subsequent try may lead to data leaks in stores.
To fix this, added check of the actual image size with the size
of image file present in the staging area. If it does not match
then delete the image file from staging area so that the entire
image will be staged again.
Change-Id: I44bfefb6eee421e18e5e95a0dafaef0ea4e170da
Closes-Bug: #1885003
(cherry picked from commit 22d8f1fcbf
)
This commit is contained in:
parent
75862b395c
commit
72510f9ce9
|
@ -59,7 +59,26 @@ class _CopyImage(task.Task):
|
|||
self.image_id)
|
||||
|
||||
if os.path.exists(file_path):
|
||||
return file_path, 0
|
||||
# NOTE (abhishekk): If previous copy-image operation is failed
|
||||
# due to power failure, network failure or any other reason and
|
||||
# the image data here is partial then clear the staging area and
|
||||
# re-stage the fresh image data.
|
||||
# Ref: https://bugs.launchpad.net/glance/+bug/1885003
|
||||
size_in_staging = os.path.getsize(file_path)
|
||||
if image.size == size_in_staging:
|
||||
return file_path, 0
|
||||
else:
|
||||
LOG.debug(("Found partial image data in staging "
|
||||
"%(fn)s, deleting it to re-stage "
|
||||
"again"), {'fn': file_path})
|
||||
try:
|
||||
os.unlink(file_path)
|
||||
except OSError as e:
|
||||
LOG.error(_LE("Deletion of staged "
|
||||
"image data from %(fn)s has failed because "
|
||||
"[Errno %(en)d]"), {'fn': file_path,
|
||||
'en': e.errno})
|
||||
raise
|
||||
|
||||
# At first search image in default_backend
|
||||
default_store = CONF.glance_store.default_backend
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
# under the License.
|
||||
|
||||
import datetime
|
||||
import os
|
||||
from unittest import mock
|
||||
|
||||
import glance_store as store_api
|
||||
|
@ -139,6 +140,67 @@ class TestCopyImageTask(test_utils.BaseTestCase):
|
|||
mock_store_api.assert_called_once_with(
|
||||
"os_glance_staging_store")
|
||||
|
||||
@mock.patch.object(os, 'unlink')
|
||||
@mock.patch.object(os.path, 'getsize')
|
||||
@mock.patch.object(os.path, 'exists')
|
||||
@mock.patch.object(store_api, 'get_store_from_store_identifier')
|
||||
def test_copy_image_to_staging_store_partial_data_exists(
|
||||
self, mock_store_api, mock_exists, mock_getsize, mock_unlink):
|
||||
mock_store_api.return_value = self.staging_store
|
||||
mock_exists.return_value = True
|
||||
mock_getsize.return_value = 3
|
||||
|
||||
copy_image_task = copy_image._CopyImage(
|
||||
self.task.task_id, self.task_type, self.image_repo,
|
||||
self.image_id)
|
||||
with mock.patch.object(self.image_repo, 'get') as get_mock:
|
||||
get_mock.return_value = mock.MagicMock(
|
||||
image_id=self.images[0]['id'],
|
||||
locations=self.images[0]['locations'],
|
||||
status=self.images[0]['status'],
|
||||
size=4
|
||||
)
|
||||
with mock.patch.object(store_api, 'get') as get_data:
|
||||
get_data.return_value = (b"dddd", 4)
|
||||
copy_image_task.execute()
|
||||
mock_exists.assert_called_once()
|
||||
mock_getsize.assert_called_once()
|
||||
mock_unlink.assert_called_once()
|
||||
self.staging_store.add.assert_called_once()
|
||||
mock_store_api.assert_called_once_with(
|
||||
"os_glance_staging_store")
|
||||
|
||||
@mock.patch.object(os, 'unlink')
|
||||
@mock.patch.object(os.path, 'getsize')
|
||||
@mock.patch.object(os.path, 'exists')
|
||||
@mock.patch.object(store_api, 'get_store_from_store_identifier')
|
||||
def test_copy_image_to_staging_store_data_exists(
|
||||
self, mock_store_api, mock_exists, mock_getsize, mock_unlink):
|
||||
mock_store_api.return_value = self.staging_store
|
||||
mock_exists.return_value = True
|
||||
mock_getsize.return_value = 4
|
||||
|
||||
copy_image_task = copy_image._CopyImage(
|
||||
self.task.task_id, self.task_type, self.image_repo,
|
||||
self.image_id)
|
||||
with mock.patch.object(self.image_repo, 'get') as get_mock:
|
||||
get_mock.return_value = mock.MagicMock(
|
||||
image_id=self.images[0]['id'],
|
||||
locations=self.images[0]['locations'],
|
||||
status=self.images[0]['status'],
|
||||
size=4
|
||||
)
|
||||
copy_image_task.execute()
|
||||
mock_exists.assert_called_once()
|
||||
mock_store_api.assert_called_once_with(
|
||||
"os_glance_staging_store")
|
||||
mock_getsize.assert_called_once()
|
||||
# As valid image data already exists in staging area
|
||||
# it does not remove it and also does not download
|
||||
# it again to staging area
|
||||
mock_unlink.assert_not_called()
|
||||
self.staging_store.add.assert_not_called()
|
||||
|
||||
@mock.patch.object(store_api, 'get_store_from_store_identifier')
|
||||
def test_copy_non_existing_image_to_staging_store_(self, mock_store_api):
|
||||
mock_store_api.return_value = self.staging_store
|
||||
|
|
Loading…
Reference in New Issue