containerImageRef.NewImageSource: merge the tar filters

Merge the two tar filters, if we need two, that we use when committing
an image.  Try to improve passing of error information from the writing
end of a pipe to the reader, so that it can be reported better.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

digester.go

@@ -141,9 +141,12 @@ func newTarFilterer(writeCloser io.WriteCloser, filter func(hdr *tar.Header) (sk
 			closed = filterer.closed
 			filterer.closedLock.Unlock()
 		}
-		pipeReader.Close()
-		tarWriter.Close()
-		writeCloser.Close()
+		err1 := tarWriter.Close()
+		err := writeCloser.Close()
+		if err == nil {
+			err = err1
+		}
+		pipeReader.CloseWithError(err)
 		filterer.wg.Done()
 	}()
 	return filterer

image.go

@@ -585,50 +585,54 @@ func (i *containerImageRef) NewImageSource(ctx context.Context, sc *types.System
 			return nil, fmt.Errorf("compressing %s: %w", what, err)
 		}
 		writer := io.MultiWriter(writeCloser, srcHasher.Hash())
-		// Scrub any local user names that might correspond to UIDs or GIDs of
-		// files in this layer.
 		{
+			// Tweak the contents of layers we're creating.
 			nestedWriteCloser := ioutils.NewWriteCloserWrapper(writer, writeCloser.Close)
 			writeCloser = newTarFilterer(nestedWriteCloser, func(hdr *tar.Header) (bool, bool, io.Reader) {
+				// Scrub any local user names that might correspond to UIDs or GIDs of
+				// files in this layer.
 				hdr.Uname, hdr.Gname = "", ""
-				return false, false, nil
-			})
-			writer = io.Writer(writeCloser)
-		}
-		// Use specified timestamps in the layer, if we're doing that for
-		// history entries.
-		if i.created != nil {
-			nestedWriteCloser := ioutils.NewWriteCloserWrapper(writer, writeCloser.Close)
-			writeCloser = newTarFilterer(nestedWriteCloser, func(hdr *tar.Header) (bool, bool, io.Reader) {
-				// Changing a zeroed field to a non-zero field
-				// can affect the format that the library uses
-				// for writing the header, so only change
-				// fields that are already set to avoid
-				// changing the format (and as a result,
-				// changing the length) of the header that we
-				// write.
-				if !hdr.ModTime.IsZero() {
-					hdr.ModTime = *i.created
-				}
-				if !hdr.AccessTime.IsZero() {
-					hdr.AccessTime = *i.created
-				}
-				if !hdr.ChangeTime.IsZero() {
-					hdr.ChangeTime = *i.created
+				// Use specified timestamps in the layer, if we're doing that for history
+				// entries.
+				if i.created != nil {
+					// Changing a zeroed field to a non-zero field can affect the
+					// format that the library uses for writing the header, so only
+					// change fields that are already set to avoid changing the
+					// format (and as a result, changing the length) of the header
+					// that we write.
+					if !hdr.ModTime.IsZero() {
+						hdr.ModTime = *i.created
+					}
+					if !hdr.AccessTime.IsZero() {
+						hdr.AccessTime = *i.created
+					}
+					if !hdr.ChangeTime.IsZero() {
+						hdr.ChangeTime = *i.created
+					}
+					return false, false, nil
 				}
 				return false, false, nil
 			})
 			writer = io.Writer(writeCloser)
 		}
+		// Okay, copy from the raw diff through the filter, compressor, and counter and
+		// digesters.
 		size, err := io.Copy(writer, rc)
-		writeCloser.Close()
-		layerFile.Close()
+		if err := writeCloser.Close(); err != nil {
+			layerFile.Close()
+			rc.Close()
+			return nil, fmt.Errorf("storing %s to file: %w on pipe close", what, err)
+		}
+		if err := layerFile.Close(); err != nil {
+			rc.Close()
+			return nil, fmt.Errorf("storing %s to file: %w on file close", what, err)
+		}
 		rc.Close()
 
 		if errChan != nil {
 			err = <-errChan
 			if err != nil {
-				return nil, err
+				return nil, fmt.Errorf("extracting container rootfs: %w", err)
 			}
 		}
 

tests/commit.bats

@@ -388,3 +388,27 @@ load helpers
     assert ${rootowner}:-rw-r--r--
   done
 }
+
+@test "commit with insufficient disk space" {
+  skip_if_rootless_environment
+  _prefetch busybox
+  local tmp=$TEST_SCRATCH_DIR/buildah-test
+  mkdir -p $tmp
+  mount -t tmpfs -o size=4M tmpfs $tmp
+  # Create a temporary file which should not be easy to compress,
+  # which we'll add to our container for committing, but which is
+  # larger than the filesystem where the layer blob that would
+  # contain it, compressed or not, would be written during commit.
+  run dd if=/dev/urandom of=$TEST_SCRATCH_DIR/8M bs=1M count=8
+  # Create a working container.
+  run_buildah from --pull=never $WITH_POLICY_JSON busybox
+  ctrID="$output"
+  # Copy the file into the working container.
+  run_buildah copy $ctrID $TEST_SCRATCH_DIR/8M /8M
+  # Try to commit the image.  The temporary copy of the layer diff should
+  # require more space than is available where we're telling it to store
+  # temporary things.
+  TMPDIR=$tmp run_buildah '?' commit $ctrID
+  umount $tmp
+  expect_output --substring "no space left on device"
+}