diff --git a/glance/async_/flows/_internal_plugins/copy_image.py b/glance/async_/flows/_internal_plugins/copy_image.py index 2a8e7f2ec2..9ec0260de3 100644 --- a/glance/async_/flows/_internal_plugins/copy_image.py +++ b/glance/async_/flows/_internal_plugins/copy_image.py @@ -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 diff --git a/glance/tests/unit/async_/flows/test_copy_image.py b/glance/tests/unit/async_/flows/test_copy_image.py index 251531a420..fa79053a9c 100644 --- a/glance/tests/unit/async_/flows/test_copy_image.py +++ b/glance/tests/unit/async_/flows/test_copy_image.py @@ -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