Change Swift zero-size chunk behaviour

There is a difference in how Swift and radosgw behave when a zero
length write is aborted. Swift returns 499 and doesn't create an object,
but radosgw appears to creates a zero size object. Because of this, new
behaviour which was introduced to prevent zero-size chunks being written
worked for Swift, but appears not to work for the radosgw case.

This patch changes how we detect a zero size chunk so that no request
is made to the backend. We no longer rely on the Swift specific behaviour.

Change-Id: I1a48e2eceaa6b07b8e5bbfd42fef6fd3d91f5834
Closes-bug: 1537721

3 files changed, 45 insertions, 19 deletions

diff --git a/glance_store/_drivers/swift/store.py b/glance_store/_drivers/swift/store.py
index 2f305df..94bf20d 100644
--- a/glance_store/_drivers/swift/store.py
+++ b/glance_store/_drivers/swift/store.py
@@ -546,14 +546,14 @@ class BaseStore(driver.Store):
                     chunk_name = "%s-%05d" % (location.obj, chunk_id)
                     reader = ChunkReader(image_file, checksum, chunk_size,
                                          verifier)
+                    if reader.is_zero_size is True:
+                        LOG.debug('Not writing zero-length chunk.')
+                        break
                     try:
                         chunk_etag = connection.put_object(
                             location.container, chunk_name, reader,
                             content_length=content_length)
                         written_chunks.append(chunk_name)
-                    except exceptions.ZeroSizeChunk:
-                        LOG.debug('Not writing zero-length chunk')
-                        break
                     except Exception:
                         # Delete orphaned segments from swift backend
                         with excutils.save_and_reraise_exception():
@@ -952,15 +952,23 @@ class ChunkReader(object):
         self.total = total
         self.verifier = verifier
         self.bytes_read = 0
+        self.is_zero_size = False
+        self.byteone = fd.read(1)
+        if len(self.byteone) == 0:
+            self.is_zero_size = True
+
+    def do_read(self, i):
+        if self.bytes_read == 0 and i > 0 and self.byteone is not None:
+            return self.byteone + self.fd.read(i - 1)
+        else:
+            return self.fd.read(i)
 
     def read(self, i):
         left = self.total - self.bytes_read
         if i > left:
             i = left
-        result = self.fd.read(i)
-        if len(result) == 0 and self.bytes_read == 0:
-            # fd was empty
-            raise exceptions.ZeroSizeChunk()
+
+        result = self.do_read(i)
         self.bytes_read += len(result)
         self.checksum.update(result)
         if self.verifier:
diff --git a/glance_store/exceptions.py b/glance_store/exceptions.py
index 163e3e9..fe59630 100644
--- a/glance_store/exceptions.py
+++ b/glance_store/exceptions.py
@@ -83,10 +83,6 @@ class NotFound(GlanceStoreException):
     message = _("Image %(image)s not found")
 
 
-class ZeroSizeChunk(GlanceStoreException):
-    message = _("Zero size chunk")
-
-
 class UnknownScheme(GlanceStoreException):
     message = _("Unknown scheme '%(scheme)s' found in URI")
 
diff --git a/glance_store/tests/unit/test_swift_store.py b/glance_store/tests/unit/test_swift_store.py
index 7268952..87e252a 100644
--- a/glance_store/tests/unit/test_swift_store.py
+++ b/glance_store/tests/unit/test_swift_store.py
@@ -778,11 +778,8 @@ class SwiftTests(object):
         self.assertEqual(expected_location, loc)
         self.assertEqual(expected_swift_size, size)
         self.assertEqual(expected_checksum, checksum)
-        # Expecting 7 calls to put_object -- 5 chunks, a zero chunk which is
-        # then deleted, and the manifest.  Note the difference with above
-        # where the image_size is specified in advance (there's no zero chunk
-        # in that case).
-        self.assertEqual(SWIFT_PUT_OBJECT_CALLS, 7)
+        # Expecting 6 calls to put_object -- 5 chunks, and the manifest.
+        self.assertEqual(SWIFT_PUT_OBJECT_CALLS, 6)
 
         loc = location.get_location_from_uri(expected_location, conf=self.conf)
         (new_image_swift, new_image_size) = self.store.get(loc)
@@ -1554,12 +1551,37 @@ class TestChunkReader(base.StoreBaseTest):
         bytes_read = 0
         while True:
             cr = swift.ChunkReader(infile, checksum, CHUNKSIZE)
-            try:
-                chunk = cr.read(CHUNKSIZE)
-            except exceptions.ZeroSizeChunk:
+            chunk = cr.read(CHUNKSIZE)
+            if len(chunk) == 0:
+                self.assertEqual(True, cr.is_zero_size)
                 break
             bytes_read += len(chunk)
         self.assertEqual(units.Ki, bytes_read)
+        self.assertEqual('fb10c6486390bec8414be90a93dfff3b',
+                         cr.checksum.hexdigest())
+        data_file.close()
+        infile.close()
+
+    def test_read_zero_size_data(self):
+        """
+        Replicate what goes on in the Swift driver with the
+        repeated creation of the ChunkReader object
+        """
+        CHUNKSIZE = 100
+        checksum = hashlib.md5()
+        data_file = tempfile.NamedTemporaryFile()
+        infile = open(data_file.name, 'rb')
+        bytes_read = 0
+        while True:
+            cr = swift.ChunkReader(infile, checksum, CHUNKSIZE)
+            chunk = cr.read(CHUNKSIZE)
+            if len(chunk) == 0:
+                break
+            bytes_read += len(chunk)
+        self.assertEqual(True, cr.is_zero_size)
+        self.assertEqual(0, bytes_read)
+        self.assertEqual('d41d8cd98f00b204e9800998ecf8427e',
+                         cr.checksum.hexdigest())
         data_file.close()
         infile.close()