diff --git a/pkg/machine/compression/decompress.go b/pkg/machine/compression/decompress.go index fb37d99b13..4d362c3a5d 100644 --- a/pkg/machine/compression/decompress.go +++ b/pkg/machine/compression/decompress.go @@ -1,7 +1,6 @@ package compression import ( - "errors" "io" "os" "path/filepath" @@ -25,7 +24,7 @@ type decompressor interface { compressedFileSize() int64 compressedFileMode() os.FileMode compressedFileReader() (io.ReadCloser, error) - decompress(w WriteSeekCloser, r io.Reader) error + decompress(w io.WriteSeeker, r io.Reader) error close() } @@ -72,7 +71,7 @@ func newDecompressor(compressedFilePath string, compressedFileContent []byte) (d } } -func runDecompression(d decompressor, decompressedFilePath string) error { +func runDecompression(d decompressor, decompressedFilePath string) (retErr error) { compressedFileReader, err := d.compressedFileReader() if err != nil { return err @@ -99,8 +98,11 @@ func runDecompression(d decompressor, decompressedFilePath string) error { return err } defer func() { - if err := decompressedFileWriter.Close(); err != nil && !errors.Is(err, os.ErrClosed) { + if err := decompressedFileWriter.Close(); err != nil { logrus.Warnf("Unable to to close destination file %s: %q", decompressedFilePath, err) + if retErr == nil { + retErr = err + } } }() diff --git a/pkg/machine/compression/generic.go b/pkg/machine/compression/generic.go index 236a18ea33..f3995bd92b 100644 --- a/pkg/machine/compression/generic.go +++ b/pkg/machine/compression/generic.go @@ -43,7 +43,7 @@ func (d *genericDecompressor) compressedFileReader() (io.ReadCloser, error) { return compressedFile, nil } -func (d *genericDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { +func (d *genericDecompressor) decompress(w io.WriteSeeker, r io.Reader) error { decompressedFileReader, _, err := compression.AutoDecompress(r) if err != nil { return err @@ -64,7 +64,7 @@ func (d *genericDecompressor) close() { } } -func (d *genericDecompressor) sparseOptimizedCopy(w WriteSeekCloser, r io.Reader) error { +func (d *genericDecompressor) sparseOptimizedCopy(w io.WriteSeeker, r io.Reader) error { var err error sparseWriter := NewSparseWriter(w) defer func() { diff --git a/pkg/machine/compression/gzip.go b/pkg/machine/compression/gzip.go index 1df7a5f471..7bbe164d0c 100644 --- a/pkg/machine/compression/gzip.go +++ b/pkg/machine/compression/gzip.go @@ -16,7 +16,7 @@ func newGzipDecompressor(compressedFilePath string) (*gzipDecompressor, error) { return &gzipDecompressor{*d}, err } -func (d *gzipDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { +func (d *gzipDecompressor) decompress(w io.WriteSeeker, r io.Reader) error { gzReader, err := image.GzipDecompressor(r) if err != nil { return err diff --git a/pkg/machine/compression/sparse_file_writer.go b/pkg/machine/compression/sparse_file_writer.go index c79941587e..d97ac9f6d0 100644 --- a/pkg/machine/compression/sparse_file_writer.go +++ b/pkg/machine/compression/sparse_file_writer.go @@ -14,14 +14,18 @@ type WriteSeekCloser interface { } type sparseWriter struct { - file WriteSeekCloser + file io.WriteSeeker // Invariant between method calls: // The contents of the file match the contents passed to Write, except that pendingZeroes trailing zeroes have not been written. // Also, the data that _has_ been written does not end with a zero byte (i.e. pendingZeroes is the largest possible value. pendingZeroes int64 } -func NewSparseWriter(file WriteSeekCloser) *sparseWriter { +// NewSparseWriter returns a WriteCloser for underlying file which creates +// holes where appropriate. +// NOTE: The caller must .Close() both the returned sparseWriter AND the underlying file, +// in that order. +func NewSparseWriter(file io.WriteSeeker) *sparseWriter { return &sparseWriter{ file: file, pendingZeroes: 0, @@ -121,18 +125,15 @@ func (sw *sparseWriter) Close() error { if sw.pendingZeroes != 0 { if holeSize := sw.pendingZeroes - 1; holeSize >= zerosThreshold { if err := sw.createHole(holeSize); err != nil { - sw.file.Close() return err } sw.pendingZeroes -= holeSize } var zeroArray [zerosThreshold]byte if _, err := sw.file.Write(zeroArray[:sw.pendingZeroes]); err != nil { - sw.file.Close() return err } } - err := sw.file.Close() sw.file = nil - return err + return nil } diff --git a/pkg/machine/compression/sparse_file_writer_test.go b/pkg/machine/compression/sparse_file_writer_test.go index c14291ac34..d4a06625b4 100644 --- a/pkg/machine/compression/sparse_file_writer_test.go +++ b/pkg/machine/compression/sparse_file_writer_test.go @@ -58,10 +58,6 @@ func (m *memorySparseFile) Write(b []byte) (n int, err error) { return n, err } -func (m *memorySparseFile) Close() error { - return nil -} - func testInputWithWriteLen(t *testing.T, input []byte, minSparse int64, chunkSize int) { m := &memorySparseFile{} sparseWriter := NewSparseWriter(m) diff --git a/pkg/machine/compression/uncompressed.go b/pkg/machine/compression/uncompressed.go index dd1a17b66d..251a3a9ccc 100644 --- a/pkg/machine/compression/uncompressed.go +++ b/pkg/machine/compression/uncompressed.go @@ -13,6 +13,6 @@ func newUncompressedDecompressor(compressedFilePath string) (*uncompressedDecomp return &uncompressedDecompressor{*d}, err } -func (d *uncompressedDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { +func (d *uncompressedDecompressor) decompress(w io.WriteSeeker, r io.Reader) error { return d.sparseOptimizedCopy(w, r) } diff --git a/pkg/machine/compression/xz.go b/pkg/machine/compression/xz.go index 867181a55e..d4f492d88e 100644 --- a/pkg/machine/compression/xz.go +++ b/pkg/machine/compression/xz.go @@ -22,7 +22,7 @@ func newXzDecompressor(compressedFilePath string) (*xzDecompressor, error) { // Will error out if file without .Xz already exists // Maybe extracting then renaming is a good idea here.. // depends on Xz: not pre-installed on mac, so it becomes a brew dependency -func (*xzDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { +func (*xzDecompressor) decompress(w io.WriteSeeker, r io.Reader) error { var cmd *exec.Cmd var read io.Reader diff --git a/pkg/machine/compression/zip.go b/pkg/machine/compression/zip.go index bd251b5d88..7de1f4813c 100644 --- a/pkg/machine/compression/zip.go +++ b/pkg/machine/compression/zip.go @@ -40,7 +40,7 @@ func (d *zipDecompressor) compressedFileReader() (io.ReadCloser, error) { return z, nil } -func (*zipDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { +func (*zipDecompressor) decompress(w io.WriteSeeker, r io.Reader) error { _, err := io.Copy(w, r) return err } diff --git a/pkg/machine/compression/zstd.go b/pkg/machine/compression/zstd.go index 33718a8035..7b78ab3cc1 100644 --- a/pkg/machine/compression/zstd.go +++ b/pkg/machine/compression/zstd.go @@ -15,7 +15,7 @@ func newZstdDecompressor(compressedFilePath string) (*zstdDecompressor, error) { return &zstdDecompressor{*d}, err } -func (d *zstdDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { +func (d *zstdDecompressor) decompress(w io.WriteSeeker, r io.Reader) error { zstdReader, err := zstd.NewReader(r) if err != nil { return err diff --git a/pkg/machine/e2e/machine_test.go b/pkg/machine/e2e/machine_test.go index 4ef85a2eec..d8962660c0 100644 --- a/pkg/machine/e2e/machine_test.go +++ b/pkg/machine/e2e/machine_test.go @@ -1,10 +1,8 @@ package e2e_test import ( - "errors" "fmt" "io" - "io/fs" "os" "path/filepath" "runtime" @@ -121,9 +119,8 @@ func setup() (string, *machineTestBuilder) { Fail(fmt.Sprintf("failed to create file %s: %q", mb.imagePath, err)) } defer func() { - closeErr := dest.Close() - if err != nil || !errors.Is(closeErr, fs.ErrClosed) { - fmt.Printf("failed to close destination file %q: %q\n", dest.Name(), err) + if err := dest.Close(); err != nil { + Fail(fmt.Sprintf("failed to close destination file %q: %q\n", dest.Name(), err)) } }() fmt.Printf("--> copying %q to %q\n", src.Name(), dest.Name()) @@ -161,26 +158,14 @@ func teardown(origHomeDir string, testDir string, mb *machineTestBuilder) { } } -// copySparseFile is a helper method for tests only. copies a file sparsely -// between two string inputs -func copySparseFile(src, dst string) error { //nolint:unused - dstWriter, err := os.OpenFile(dst, os.O_CREATE|os.O_RDWR, 0600) - if err != nil { - return err - } - dstWriter.Close() - fSrc, err := os.Open(src) - if err != nil { - return err - } - defer fSrc.Close() - return copySparse(dstWriter, fSrc) -} - // copySparse is a helper method for tests only; caller is responsible for closures -func copySparse(dst compression.WriteSeekCloser, src io.Reader) error { +func copySparse(dst io.WriteSeeker, src io.Reader) (retErr error) { spWriter := compression.NewSparseWriter(dst) - defer spWriter.Close() + defer func() { + if err := spWriter.Close(); err != nil && retErr == nil { + retErr = err + } + }() _, err := io.Copy(spWriter, src) return err }