S3: Fix multipart upload of 0 byte image
When uploading a 0 byte image using multipart, we error out with MalformedXML. A common case where this happens is when creating a snapshot of a BFV server which in-turn creates a 0 byte image in glance. Multipart request is divided into 3 steps[1]: 1. CreateMultipartUpload 2. UploadPart 3. CompleteMultipartUpload After completing the UploadPart step, we can see that the "Parts" section of the request is empty eventually leading to MalformedXML. 'MultipartUpload': {'Parts': []} To fix this, we need to handle the case when no Parts exist and perform the following actions: 1. Abort the multipart upload 2. Initiate a single part upload with the 0 byte file This doesn't affect the code path when image file is > 0 bytes as we still execute the CompleteMultipartUpload step in that case. Following are the success logs for the following operations: 1. Nova VM snapshot (Edge case handled in this patch): INFO glance_store._drivers.s3 [None req-8f7e44f4-5f53-48f0-944d-5c6d5cdcb874 admin admin] Not doing multipart upload for zero-byte image key=bce3eb46-5542-42e6-a178-dbb44bc0587a, UploadId=MDA3MDNiNTUtZTIxZC00NDY2LTg2ZjQtZmFjYWMxNjkwMTMx DEBUG glance_store._drivers.s3 [None req-8f7e44f4-5f53-48f0-944d-5c6d5cdcb874 admin admin] Creating zero-byte object directly using singlepart for key=bce3eb46-5542-42e6-a178-dbb44bc0587a {{(pid=2107463) _add_multipart /opt/stack/data/venv/lib/python3.10/site-packages/glance_store/_drivers/s3.py:856}} INFO glance_store._drivers.s3 [None req-8f7e44f4-5f53-48f0-944d-5c6d5cdcb874 admin admin] Singlepart upload completed. Wrote 0 bytes to S3 key named bce3eb46-5542-42e6-a178-dbb44bc0587a with checksum d41d8cd98f00b204e9800998ecf8427e INFO glance.location [None req-8f7e44f4-5f53-48f0-944d-5c6d5cdcb874 admin admin] Image format matched and virtual size computed: 0 2. Upload volume to image (Image file > 0 bytes): INFO glance_store._drivers.s3 [None req-c4209dd7-3213-4590-a14e-d57c36803638 admin admin] Multipart complete key=14bf4232-9401-42bc-a7dd-aa811581ae55 UploadId=N2JhYmU3ZDAtNjI5My00MjZjLThhMGYtODlkODQzOTM0MTk5 Wrote 1073741824 bytes to S3 key named 14bf4232-9401-42bc-a7dd-aa811581ae55 with checksum cd573cfaace07e7949bc0c46028904ff [1] https://docs.aws.amazon.com/AmazonS3/latest/userguide/mpuoverview.html#mpu-process Closes-Bug: #2124829 Change-Id: I77d255d7c868779e9ea700ab83c7e77373fdffb5 Signed-off-by: Rajat Dhasmana <rajatdhasmana@gmail.com>
This commit is contained in:
@@ -835,28 +835,49 @@ class Store(glance_store.driver.Store):
|
||||
total_size = sum(p.size for p in plist)
|
||||
|
||||
if success:
|
||||
# Complete
|
||||
pedict = {p.partnum: p.etag[p.partnum] for p in plist}
|
||||
mpu_list = self._get_mpu_list(pedict)
|
||||
s3_client.complete_multipart_upload(Bucket=bucket,
|
||||
Key=key,
|
||||
MultipartUpload=mpu_list,
|
||||
UploadId=upload_id)
|
||||
hash_hex = os_hash_value.hexdigest()
|
||||
checksum_hex = checksum.hexdigest()
|
||||
|
||||
# Add store backend information to location metadata
|
||||
metadata = {}
|
||||
if self.backend_group:
|
||||
metadata['store'] = self.backend_group
|
||||
# If no parts were uploaded, this was a zero-byte image. This is
|
||||
# the case when a BFV server creates a snapshot resulting in a 0
|
||||
# byte image file. Since we cannot complete a multipart upload
|
||||
# with zero parts, so we must abort it and create the empty
|
||||
# object directly.
|
||||
if not plist:
|
||||
LOG.info("Not doing multipart upload for zero-byte image "
|
||||
"key=%(key)s, UploadId=%(UploadId)s",
|
||||
{'key': key, 'UploadId': upload_id})
|
||||
s3_client.abort_multipart_upload(Bucket=bucket, Key=key,
|
||||
UploadId=upload_id)
|
||||
|
||||
LOG.info("Multipart complete key=%(key)s "
|
||||
"UploadId=%(UploadId)s "
|
||||
"Wrote %(total_size)d bytes to S3 key "
|
||||
"named %(key)s "
|
||||
"with checksum %(checksum)s",
|
||||
{'key': key, 'UploadId': upload_id,
|
||||
'total_size': total_size, 'checksum': checksum_hex})
|
||||
LOG.debug("Creating zero-byte object directly using "
|
||||
"singlepart for key=%s", key)
|
||||
s3_client.put_object(Body=b'', Bucket=bucket, Key=key)
|
||||
LOG.info("Singlepart upload completed. "
|
||||
"Wrote %(total_size)d bytes to S3 key "
|
||||
"named %(key)s "
|
||||
"with checksum %(checksum)s",
|
||||
{'key': key, 'total_size': total_size,
|
||||
'checksum': checksum_hex})
|
||||
else:
|
||||
# Complete the multipart upload
|
||||
pedict = {p.partnum: p.etag[p.partnum] for p in plist}
|
||||
mpu_list = self._get_mpu_list(pedict)
|
||||
s3_client.complete_multipart_upload(Bucket=bucket,
|
||||
Key=key,
|
||||
MultipartUpload=mpu_list,
|
||||
UploadId=upload_id)
|
||||
|
||||
LOG.info("Multipart complete key=%(key)s "
|
||||
"UploadId=%(UploadId)s "
|
||||
"Wrote %(total_size)d bytes to S3 key "
|
||||
"named %(key)s "
|
||||
"with checksum %(checksum)s",
|
||||
{'key': key, 'UploadId': upload_id,
|
||||
'total_size': total_size, 'checksum': checksum_hex})
|
||||
return loc.get_uri(), total_size, checksum_hex, hash_hex, metadata
|
||||
|
||||
# Abort
|
||||
|
@@ -757,6 +757,78 @@ class TestMultiS3Store(base.MultiStoreBaseTest,
|
||||
UploadId='UploadId'
|
||||
)
|
||||
|
||||
@mock.patch.object(boto3.session.Session, "client")
|
||||
def test_add_multipart_zero_byte_image(self, mock_client):
|
||||
"""Bug #2124829: add a zero-byte image using multipart upload."""
|
||||
expected_image_id = str(uuid.uuid4())
|
||||
expected_s3_size = 0
|
||||
expected_s3_contents = b""
|
||||
expected_checksum = hashlib.md5(expected_s3_contents,
|
||||
usedforsecurity=False).hexdigest()
|
||||
expected_multihash = hashlib.sha256(expected_s3_contents).hexdigest()
|
||||
expected_location = format_s3_location(
|
||||
S3_CONF['s3_store_access_key'],
|
||||
S3_CONF['s3_store_secret_key'],
|
||||
S3_CONF['s3_store_host'],
|
||||
S3_CONF['s3_store_bucket'],
|
||||
expected_image_id)
|
||||
image_s3 = io.BytesIO(expected_s3_contents)
|
||||
|
||||
self.config(group='s3_region1', s3_store_large_object_size=0)
|
||||
self.store.configure()
|
||||
fake_s3_client = botocore.session.get_session().create_client('s3')
|
||||
|
||||
with stub.Stubber(fake_s3_client) as stubber:
|
||||
stubber.add_response(method='head_bucket',
|
||||
service_response={},
|
||||
expected_params={
|
||||
'Bucket': S3_CONF['s3_store_bucket']
|
||||
})
|
||||
stubber.add_client_error(method='head_object',
|
||||
service_error_code='404',
|
||||
service_message='',
|
||||
expected_params={
|
||||
'Bucket': S3_CONF['s3_store_bucket'],
|
||||
'Key': expected_image_id
|
||||
})
|
||||
stubber.add_response(method='create_multipart_upload',
|
||||
service_response={
|
||||
"Bucket": S3_CONF['s3_store_bucket'],
|
||||
"Key": expected_image_id,
|
||||
"UploadId": 'UploadId'
|
||||
},
|
||||
expected_params={
|
||||
"Bucket": S3_CONF['s3_store_bucket'],
|
||||
"Key": expected_image_id,
|
||||
})
|
||||
|
||||
stubber.add_response(method='abort_multipart_upload',
|
||||
service_response={},
|
||||
expected_params={
|
||||
'Bucket': S3_CONF['s3_store_bucket'],
|
||||
'Key': expected_image_id,
|
||||
'UploadId': 'UploadId'
|
||||
})
|
||||
stubber.add_response(method='put_object',
|
||||
service_response={},
|
||||
expected_params={
|
||||
'Bucket': S3_CONF['s3_store_bucket'],
|
||||
'Key': expected_image_id,
|
||||
'Body': b''
|
||||
})
|
||||
|
||||
mock_client.return_value = fake_s3_client
|
||||
loc, size, checksum, multihash, metadata = \
|
||||
self.store.add(expected_image_id, image_s3, expected_s3_size,
|
||||
self.hash_algo)
|
||||
|
||||
stubber.assert_no_pending_responses()
|
||||
self.assertEqual("s3_region1", metadata["store"])
|
||||
self.assertEqual(expected_location, loc)
|
||||
self.assertEqual(expected_s3_size, size)
|
||||
self.assertEqual(expected_checksum, checksum)
|
||||
self.assertEqual(expected_multihash, multihash)
|
||||
|
||||
@mock.patch.object(boto3.session.Session, "client")
|
||||
def test_add_already_existing(self, mock_client):
|
||||
"""Tests that adding an image with an existing identifier raises an
|
||||
|
@@ -0,0 +1,5 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
S3: Fixed issue when uploading a zero byte image using
|
||||
multipart upload.
|
Reference in New Issue
Block a user