Merge pull request #118 from tri-adam/close-on-unload

Add OptCloseOnUnload

pkg/sif/create.go

@@ -135,9 +135,10 @@ func (f *FileImage) writeHeader() error {
 
 // createOpts accumulates container creation options.
 type createOpts struct {
-	id  uuid.UUID
-	dis []DescriptorInput
-	t   time.Time
+	id            uuid.UUID
+	dis           []DescriptorInput
+	t             time.Time
+	closeOnUnload bool
 }
 
 // CreateOpt are used to specify container creation options.
@@ -168,6 +169,15 @@ func OptCreateWithTime(t time.Time) CreateOpt {
 	}
 }
 
+// OptCreateWithCloseOnUnload specifies whether the ReadWriter should be closed by UnloadContainer.
+// By default, the ReadWriter will be closed if it implements the io.Closer interface.
+func OptCreateWithCloseOnUnload(b bool) CreateOpt {
+	return func(co *createOpts) error {
+		co.closeOnUnload = b
+		return nil
+	}
+}
+
 // createContainer creates a new SIF container file in rw, according to opts.
 func createContainer(rw ReadWriter, co createOpts) (*FileImage, error) {
 	h := header{
@@ -215,16 +225,18 @@ func createContainer(rw ReadWriter, co createOpts) (*FileImage, error) {
 // CreateContainer creates a new SIF container in rw, according to opts.
 //
 // On success, a FileImage is returned. The caller must call UnloadContainer to ensure resources
-// are released.
+// are released. By default, UnloadContainer will close rw if it implements the io.Closer
+// interface. To change this behavior, consider using OptCreateWithCloseOnUnload.
 func CreateContainer(rw ReadWriter, opts ...CreateOpt) (*FileImage, error) {
 	id, err := uuid.NewRandom()
 	if err != nil {
 		return nil, err
 	}
 
 	co := createOpts{
-		id: id,
-		t:  time.Now(),
+		id:            id,
+		t:             time.Now(),
+		closeOnUnload: true,
 	}
 
 	for _, opt := range opts {
@@ -237,6 +249,8 @@ func CreateContainer(rw ReadWriter, opts ...CreateOpt) (*FileImage, error) {
 	if err != nil {
 		return nil, fmt.Errorf("%w", err)
 	}
+
+	f.closeOnUnload = co.closeOnUnload
 	return f, nil
 }
 
@@ -255,6 +269,8 @@ func CreateContainerAtPath(path string, opts ...CreateOpt) (*FileImage, error) {
 		fp.Close()
 		os.Remove(fp.Name())
 	}
+
+	f.closeOnUnload = true
 	return f, err
 }
 

pkg/sif/create_test.go

@@ -65,7 +65,7 @@ func TestCreateContainer(t *testing.T) {
 	defer tf.Close()
 
 	// test container creation without any input descriptors
-	f, err := CreateContainer(tf)
+	f, err := CreateContainer(tf, OptCreateWithCloseOnUnload(true))
 	if err != nil {
 		t.Fatalf("failed to create container: %v", err)
 	}

pkg/sif/load.go

@@ -75,7 +75,8 @@ func loadContainer(rw ReadWriter) (*FileImage, error) {
 
 // loadOpts accumulates container loading options.
 type loadOpts struct {
-	flag int
+	flag          int
+	closeOnUnload bool
 }
 
 // LoadOpt are used to specify container loading options.
@@ -89,6 +90,15 @@ func OptLoadWithFlag(flag int) LoadOpt {
 	}
 }
 
+// OptLoadWithCloseOnUnload specifies whether the ReadWriter should be closed by UnloadContainer.
+// By default, the ReadWriter will be closed if it implements the io.Closer interface.
+func OptLoadWithCloseOnUnload(b bool) LoadOpt {
+	return func(lo *loadOpts) error {
+		lo.closeOnUnload = b
+		return nil
+	}
+}
+
 // LoadContainerFromPath loads a new SIF container from path, according to opts.
 //
 // On success, a FileImage is returned. The caller must call UnloadContainer to ensure resources
@@ -118,15 +128,20 @@ func LoadContainerFromPath(path string, opts ...LoadOpt) (*FileImage, error) {
 
 		return nil, fmt.Errorf("%w", err)
 	}
+
+	f.closeOnUnload = true
 	return f, nil
 }
 
 // LoadContainer loads a new SIF container from rw, according to opts.
 //
 // On success, a FileImage is returned. The caller must call UnloadContainer to ensure resources
-// are released.
+// are released. By default, UnloadContainer will close rw if it implements the io.Closer
+// interface. To change this behavior, consider using OptLoadWithCloseOnUnload.
 func LoadContainer(rw ReadWriter, opts ...LoadOpt) (*FileImage, error) {
-	lo := loadOpts{}
+	lo := loadOpts{
+		closeOnUnload: true,
+	}
 
 	for _, opt := range opts {
 		if err := opt(&lo); err != nil {
@@ -138,12 +153,14 @@ func LoadContainer(rw ReadWriter, opts ...LoadOpt) (*FileImage, error) {
 	if err != nil {
 		return nil, fmt.Errorf("%w", err)
 	}
+
+	f.closeOnUnload = lo.closeOnUnload
 	return f, nil
 }
 
 // UnloadContainer unloads f, releasing associated resources.
 func (f *FileImage) UnloadContainer() error {
-	if c, ok := f.rw.(io.Closer); ok {
+	if c, ok := f.rw.(io.Closer); ok && f.closeOnUnload {
 		if err := c.Close(); err != nil {
 			return err
 		}

pkg/sif/load_test.go

@@ -6,58 +6,83 @@
 package sif
 
 import (
-	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
 )
 
-func TestLoadContainer(t *testing.T) {
-	fimg, err := LoadContainerFromPath(
-		filepath.Join("testdata", "testcontainer2.sif"),
-		OptLoadWithFlag(os.O_RDONLY),
-	)
-	if err != nil {
-		t.Error("LoadContainer(testdata/testcontainer2.sif, true):", err)
+func TestLoadContainerFromPath(t *testing.T) {
+	tests := []struct {
+		name string
+		path string
+		opts []LoadOpt
+	}{
+		{
+			name: "NoOpts",
+			path: filepath.Join("testdata", "testcontainer2.sif"),
+		},
+		{
+			name: "ReadOnly",
+			path: filepath.Join("testdata", "testcontainer2.sif"),
+			opts: []LoadOpt{OptLoadWithFlag(os.O_RDONLY)},
+		},
+		{
+			name: "ReadWrite",
+			path: filepath.Join("testdata", "testcontainer2.sif"),
+			opts: []LoadOpt{OptLoadWithFlag(os.O_RDWR)},
+		},
 	}
+	for _, tt := range tests {
+		tt := tt
 
-	if err = fimg.UnloadContainer(); err != nil {
-		t.Error("fimg.UnloadContainer():", err)
+		t.Run(tt.name, func(t *testing.T) {
+			f, err := LoadContainerFromPath(tt.path, tt.opts...)
+			if err != nil {
+				t.Fatalf("failed to load container: %v", err)
+			}
+
+			if err := f.UnloadContainer(); err != nil {
+				t.Errorf("failed to unload container: %v", err)
+			}
+		})
 	}
 }
 
-func TestLoadContainerFp(t *testing.T) {
+func TestLoadContainer(t *testing.T) {
 	tests := []struct {
-		name   string
-		offset int64
+		name string
+		opts []LoadOpt
 	}{
 		{
-			name: "NoSeek",
+			name: "NoOpts",
+		},
+		{
+			name: "CloseOnUnload",
+			opts: []LoadOpt{OptLoadWithCloseOnUnload(true)},
 		},
 		{
-			name:   "Seek",
-			offset: 1,
+			name: "NoCloseOnUnload",
+			opts: []LoadOpt{OptLoadWithCloseOnUnload(false)},
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
-			fp, err := os.Open("testdata/testcontainer2.sif")
+			rw, err := os.Open(filepath.Join("testdata", "testcontainer2.sif"))
 			if err != nil {
-				t.Fatal("error opening testdata/testcontainer2.sif:", err)
-			}
-
-			if _, err := fp.Seek(tt.offset, io.SeekStart); err != nil {
 				t.Fatal(err)
 			}
+			defer rw.Close()
 
-			fimg, err := LoadContainer(fp, OptLoadWithFlag(os.O_RDONLY))
+			f, err := LoadContainer(rw, tt.opts...)
 			if err != nil {
-				t.Error("LoadContainerFp(fp, true):", err)
+				t.Fatalf("failed to load container: %v", err)
 			}
 
-			if err = fimg.UnloadContainer(); err != nil {
-				t.Error("fimg.UnloadContainer():", err)
+			if err := f.UnloadContainer(); err != nil {
+				t.Errorf("failed to unload container: %v", err)
 			}
 		})
 	}

pkg/sif/sif.go

@@ -311,7 +311,8 @@ type FileImage struct {
 	h   header          // Raw global header from image.
 	rds []rawDescriptor // Raw descriptors from image.
 
-	minIDs map[uint32]uint32 // Minimum object IDs for each group ID.
+	closeOnUnload bool              // Close rw on Unload.
+	minIDs        map[uint32]uint32 // Minimum object IDs for each group ID.
 }
 
 // LaunchScript returns the image launch script.