Minor cleanups

Signed-off-by: Natalie Arellano <[email protected]>

cnb_image.go

@@ -105,8 +105,6 @@ func (i *CNBImageCore) History() ([]v1.History, error) {
 	return configFile.History, nil
 }
 
-// Kind exposes the type of image (via the Store) that backs the imgutil.Image implementation.
-// It could be `locallayout`, `remote`, or `layout`.
 func (i *CNBImageCore) Kind() string {
 	storeType := fmt.Sprintf("%T", i.Store)
 	parts := strings.Split(storeType, ".")
@@ -351,7 +349,7 @@ func (i *CNBImageCore) Rebase(baseTopLayerDiffID string, withNewBase Image) erro
 	if i.Kind() != withNewBase.Kind() {
 		return fmt.Errorf("expected new base to be a %s image; got %s", i.Kind(), withNewBase.Kind())
 	}
-	newBase := withNewBase.UnderlyingImage()
+	newBase := withNewBase.UnderlyingImage() // FIXME: when all imgutil.Images are v1.Images, we can remove this part
 	var err error
 	i.Image, err = mutate.Rebase(i.Image, i.newV1ImageFacade(baseTopLayerDiffID), newBase)
 	if err != nil {

image.go

@@ -26,6 +26,8 @@ type Image interface {
 	GetLayer(diffID string) (io.ReadCloser, error)
 	History() ([]v1.History, error)
 	Identifier() (Identifier, error)
+	// Kind exposes the type of image that backs the imgutil.Image implementation.
+	// It could be `local`, `locallayout`, `remote`, or `layout`.
 	Kind() string
 	Label(string) (string, error)
 	Labels() (map[string]string, error)

layout/layout.go

@@ -152,7 +152,7 @@ func (i *Image) Identifier() (imgutil.Identifier, error) {
 }
 
 func (i *Image) Kind() string {
-	return fmt.Sprintf("%T", i)
+	return `layout`
 }
 
 func (i *Image) Label(key string) (string, error) {

local/local.go

@@ -116,7 +116,7 @@ func (i *Image) Identifier() (imgutil.Identifier, error) {
 }
 
 func (i *Image) Kind() string {
-	return fmt.Sprintf("%T", i)
+	return `local`
 }
 
 func (i *Image) Label(key string) (string, error) {

locallayout/image.go

@@ -41,16 +41,25 @@ func (i idStringer) String() string {
 }
 
 // GetLayer returns an io.ReadCloser with uncompressed layer data.
-// The layer will always have data, even if that means downloading all of the image layers from the daemon.
+// The layer will always have data, even if that means downloading ALL the image layers from the daemon.
 func (i *Image) GetLayer(diffID string) (io.ReadCloser, error) {
 	layerHash, err := v1.NewHash(diffID)
 	if err != nil {
 		return nil, err
 	}
+	layer, err := i.LayerByDiffID(layerHash)
+	if err == nil {
+		// this avoids downloading ALL the image layers from the daemon
+		// if the layer is available locally
+		// (e.g., it was added using AddLayer).
+		if size, err := layer.Size(); err != nil && size != -1 {
+			return layer.Uncompressed()
+		}
+	}
 	if err = i.ensureLayers(); err != nil {
 		return nil, err
 	}
-	layer, err := i.LayerByDiffID(layerHash)
+	layer, err = i.LayerByDiffID(layerHash)
 	if err != nil {
 		return nil, fmt.Errorf("image %q does not contain layer with diff ID %q", i.Name(), layerHash.String())
 	}

locallayout/new.go

@@ -13,7 +13,7 @@ import (
 )
 
 // NewImage returns a new image that can be modified and saved to a docker daemon
-// via a tarball in OCI layout format.
+// via a tarball in legacy format.
 func NewImage(repoName string, dockerClient DockerClient, ops ...func(*imgutil.ImageOptions)) (imgutil.Image, error) {
 	options := &imgutil.ImageOptions{}
 	for _, op := range ops {

locallayout/store.go

@@ -144,7 +144,7 @@ func (s *Store) doSave(image v1.Image, withName string) (types.ImageInspect, err
 	tw := tar.NewWriter(pw)
 	defer tw.Close()
 
-	if err = s.addImageToTar(tw, image, withName, false); err != nil {
+	if err = s.addImageToTar(tw, image, withName); err != nil {
 		return types.ImageInspect{}, err
 	}
 	tw.Close()
@@ -164,7 +164,7 @@ func (s *Store) doSave(image v1.Image, withName string) (types.ImageInspect, err
 	return inspect, nil
 }
 
-func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string, populateEmptyLayers bool) error {
+func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) error {
 	rawConfigFile, err := image.RawConfigFile()
 	if err != nil {
 		return err
@@ -187,7 +187,7 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string, p
 		if err != nil {
 			return err
 		}
-		if size == -1 && !populateEmptyLayers { // layer facade fronting empty layer
+		if size == -1 { // layer facade fronting empty layer
 			layerName = fmt.Sprintf("blank_%d", blankIdx)
 			blankIdx++
 			hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0}
@@ -221,24 +221,9 @@ func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer) (string, error) {
 	if err != nil {
 		return "", err
 	}
-
-	var layerToUse v1.Layer
-	compressedSize, err := layer.Size()
-	if err != nil {
-		return "", err
-	}
-	if compressedSize == -1 {
-		onDiskLayer := findLayer(layerDiffID, s.Layers())
-		if onDiskLayer == nil {
-			return "", fmt.Errorf("failed to find layer with diff ID %q", layerDiffID.String())
-		}
-		layerToUse = onDiskLayer
-	} else {
-		layerToUse = layer
-	}
-
 	withName := fmt.Sprintf("/%s.tar", layerDiffID.String())
-	uncompressedSize, err := getLayerSize(layerToUse)
+
+	uncompressedSize, err := getLayerSize(layer)
 	if err != nil {
 		return "", err
 	}
@@ -247,7 +232,7 @@ func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer) (string, error) {
 		return "", err
 	}
 
-	layerReader, err := layerToUse.Uncompressed()
+	layerReader, err := layer.Uncompressed()
 	if err != nil {
 		return "", err
 	}
@@ -350,7 +335,7 @@ func (s *Store) SaveFile(image imgutil.IdentifiableV1Image, withName string) (st
 		tw := tar.NewWriter(pw)
 		defer tw.Close()
 
-		return s.addImageToTar(tw, image, withName, true)
+		return s.addImageToTar(tw, image, withName)
 	})
 
 	err = errs.Wait()

locallayout/v1_facade.go

@@ -31,7 +31,7 @@ type v1ImageFacade struct {
 
 	// for downloading layers from the daemon as needed
 	store                  imgutil.ImageStore
-	downloadLayersOnAccess bool // set to true to download ALL image layers when LayerByDiffID is called
+	downloadLayersOnAccess bool // set to true to downloading ALL the image layers from the daemon when LayerByDiffID is called
 	downloadOnce           *sync.Once
 	identifier             string
 }

remote/remote.go

@@ -185,7 +185,7 @@ func (i *Image) Identifier() (imgutil.Identifier, error) {
 }
 
 func (i *Image) Kind() string {
-	return fmt.Sprintf("%T", i)
+	return `remote`
 }
 
 func (i *Image) Label(key string) (string, error) {