review comments

Author: nkubala

v1/daemon/BUILD.bazel

@@ -18,6 +18,7 @@ go_library(
         "//v1/tarball:go_default_library",
         "//vendor/github.com/docker/docker/api/types:go_default_library",
         "//vendor/github.com/docker/docker/client:go_default_library",
+        "//vendor/github.com/pkg/errors:go_default_library",
     ],
 )
 

v1/daemon/write.go

@@ -46,19 +46,19 @@ type WriteOptions struct {
 }
 
 // Write saves the image into the daemon as the given reference.
-func Write(ref name.Reference, img v1.Image, wo WriteOptions) (string, error) {
+func Write(tag name.Tag, img v1.Image, wo WriteOptions) (string, error) {
 	cli, err := getImageLoader()
 	if err != nil {
 		return "", err
 	}
 
-	tag, err := name.NewTag(ref.Name(), name.WeakValidation)
-	if err != nil {
-		return "", err
-	}
+	pr, pw := io.Pipe()
+	go func() {
+		pw.CloseWithError(tarball.Write(tag, img, &tarball.WriteOptions{}, pw))
+	}()
+
 	// write the image in docker save format first, then load it
-	reader := tarball.Write(tag, img, &tarball.WriteOptions{})
-	resp, err := cli.ImageLoad(context.Background(), reader, false)
+	resp, err := cli.ImageLoad(context.Background(), pr, false)
 	defer resp.Body.Close()
 	b, readErr := ioutil.ReadAll(resp.Body)
 	response := string(b)

v1/daemon/write_test.go

@@ -27,11 +27,11 @@ func TestWriteImage(t *testing.T) {
 	if err != nil {
 		t.Errorf("Error loading image: %v", err.Error())
 	}
-	imageName, err := name.ParseReference("test_image_2:latest", name.WeakValidation)
+	tag, err := name.NewTag("test_image_2:latest", name.WeakValidation)
 	if err != nil {
 		t.Errorf(err.Error())
 	}
-	response, err := Write(imageName, image, WriteOptions{})
+	response, err := Write(tag, image, WriteOptions{})
 	if err != nil {
 		t.Errorf("Error writing image tar: %s", err.Error())
 	}

v1/tarball/write.go

@@ -32,57 +32,24 @@ type WriteOptions struct {
 	// TODO(mattmoor): Whether to store things compressed?
 }
 
-// Write writes the image at the given tag to a tarball,
-// and returns it through a reader to be consumed
-func Write(tag name.Tag, img v1.Image, wo *WriteOptions) io.ReadCloser {
-	pr, pw := io.Pipe()
-
-	go func() {
-		// Close the writer with any errors encountered.
-		// These errors will be returned by the reader end
-		// on subsequent reads. If err == nil, the reader will return
-		// EOF.
-		pw.CloseWithError(write(tag, img, wo, pw))
-	}()
-
-	return pr
-}
-
 // WriteToFile writes in the compressed format to a tarball, on disk.
+// This is just syntactic sugar wrapping tarball.Write with a new file.
 func WriteToFile(p string, tag name.Tag, img v1.Image, wo *WriteOptions) error {
 	w, err := os.Create(p)
 	if err != nil {
 		return err
 	}
 	defer w.Close()
 
-	tf := tar.NewWriter(w)
-	defer tf.Close()
-
-	tr := tar.NewReader(Write(tag, img, wo))
-	for {
-		header, err := tr.Next()
-		if err == io.EOF {
-			break
-		}
-		if err != nil {
-			return err
-		}
-		tf.WriteHeader(header)
-		_, err = io.Copy(tf, tr)
-		if err != nil {
-			return err
-		}
-	}
-	return nil
+	return Write(tag, img, wo, w)
 }
 
 // Write in the compressed format.
 // Return a reader for contents of a tarball, containing:
 // One manifest.json file at the top level containing information about several images.
 // One file for each layer, named after the layer's SHA.
 // One file for the config blob, named after its SHA.
-func write(tag name.Tag, img v1.Image, wo *WriteOptions, w io.Writer) error {
+func Write(tag name.Tag, img v1.Image, wo *WriteOptions, w io.Writer) error {
 	tf := tar.NewWriter(w)
 	defer tf.Close()