server: avoid logging an error when deploying plugins

chunker/writer.go

@@ -18,6 +18,7 @@ package chunker
 import (
 	"fmt"
 	"os"
+	"sync"
 )
 
 // Writer is an implementation of a chunked writer.
@@ -26,6 +27,11 @@ type Writer struct {
 	chunkSize  int
 	chunkIndex int
 
+	// cleanupOnce and cleanupErr are for use
+	// within the Cleanup function only.
+	cleanupOnce sync.Once
+	cleanupErr  error
+
 	bytesWritten uint64
 }
 
@@ -51,11 +57,13 @@ func (w *Writer) Write(p []byte) (int, error) {
 }
 
 func (w *Writer) Cleanup() error {
-	if err := os.Remove(w.tmp.Name()); err != nil {
-		return fmt.Errorf("failed to remove temporary file %s with error: %s",
-			w.tmp.Name(), err)
-	}
-	return nil
+	w.cleanupOnce.Do(func() {
+		if err := os.Remove(w.tmp.Name()); err != nil {
+			w.cleanupErr = fmt.Errorf("failed to remove temporary file %s with error: %s",
+				w.tmp.Name(), err)
+		}
+	})
+	return w.cleanupErr
 }
 
 // Size returns the number of bytes written so far.

chunker/writer_test.go

@@ -15,6 +15,7 @@
 package chunker
 
 import (
+	"fmt"
 	"io"
 	"math"
 	"os"
@@ -91,3 +92,41 @@ func TestWrite(t *testing.T) {
 		})
 	}
 }
+
+// TestCleanupOnce tests that Cleanup can be called multiple times,
+// and all calls to Cleanup will return the same error.
+func TestCleanupOnce(t *testing.T) {
+	for _, tc := range []struct {
+		name                    string
+		removeFileBeforeCleanup bool
+	}{{
+		name: "multiple_cleanups_no_err",
+	}, {
+		name:                    "multiple_cleanups_with_err",
+		removeFileBeforeCleanup: true,
+	}} {
+		t.Run(tc.name, func(t *testing.T) {
+			const inChunkSize = 100
+			w, err := NewWriter(os.TempDir(), inChunkSize)
+			if err != nil {
+				t.Fatalf("NewWriter(%q, %v) returned an error: %v",
+					os.TempDir(), inChunkSize, err)
+			}
+			var expectedErr error
+			if tc.removeFileBeforeCleanup {
+				if err := os.Remove(w.File().Name()); err != nil {
+					t.Fatal(err)
+				}
+				expectedErr = fmt.Errorf(
+					"failed to remove temporary file %s with error: %s",
+					w.File().Name(), os.Remove(w.File().Name()))
+			}
+			for i := 0; i < 3; i++ {
+				if err := w.Cleanup(); (err == nil) != (expectedErr == nil) ||
+					err != nil && err.Error() != expectedErr.Error() {
+					t.Fatal(err)
+				}
+			}
+		})
+	}
+}

server/deploy.go

@@ -138,7 +138,7 @@ func (s *Server) handleImageTransfer(ctx context.Context, srv cpb.Containerz_Dep
 
 		case *cpb.DeployRequest_ImageTransferEnd:
 			if transfer.IsPlugin {
-				if err := moveFile(chunkWriter.File().Name(), filepath.Join(pluginLocation, fmt.Sprintf("%s.tar", transfer.GetName()))); err != nil {
+				if err := moveFile(chunkWriter, filepath.Join(pluginLocation, fmt.Sprintf("%s.tar", transfer.GetName()))); err != nil {
 					return status.Errorf(codes.Internal, "unable to move plugin: %v", err)
 				}
 
@@ -207,7 +207,8 @@ func diskSpace(loc string) (uint64, error) {
 // actual location may be on different devices.
 // To ensure that the completed file is created atomically, the copy is done to a temp location
 // within the destination directory, and then the file is renamed within that destination directory.
-func moveFile(sourcePath, destPath string) error {
+func moveFile(writer *chunker.Writer, destPath string) error {
+	sourcePath := writer.File().Name()
 	// idempotent create of the destPath directory
 	if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil {
 		return fmt.Errorf("failed to create %s with error %s",
@@ -238,8 +239,7 @@ func moveFile(sourcePath, destPath string) error {
 			outputTmp.Name(), destPath, err)
 	}
 	// Success, now delete the original file.
-	err = os.Remove(sourcePath)
-	if err != nil {
+	if err := writer.Cleanup(); err != nil {
 		return fmt.Errorf("failed removing original file: %s", err)
 	}
 	return nil