refactor: simplify unpackOne, drop unused param to ExtractSingleSquash

unpackOne required the caller to provide it with a boolean 'isSquashfs'
which then made each caller have to consider the layer type.

Update unpackOne to take a Descriptor and let it do the
determination of how to unpack the layer in a single place.

The result of calling unpackLayer is either error, or the contents
available at the provided path.  The caller does not have to check
if the content is already present.

Also here, drop the 'storage' parameter to ExtractSingleSquash
that had become unused.

Signed-off-by: Scott Moser <smoser@brickies.net>

pkg/overlay/overlay.go

@@ -14,8 +14,6 @@ import (
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"golang.org/x/sys/unix"
-	"stackerbuild.io/stacker/pkg/mount"
-	"stackerbuild.io/stacker/pkg/squashfs"
 	"stackerbuild.io/stacker/pkg/types"
 )
 
@@ -168,23 +166,10 @@ func (o *overlay) snapshot(source string, target string) error {
 	for _, m := range ovl.Manifests {
 		manifest = m
 	}
-	cacheDir := path.Join(o.config.StackerDir, "layer-bases", "oci")
+	ociDir := path.Join(o.config.StackerDir, "layer-bases", "oci")
 	for _, layer := range manifest.Layers {
-		if !squashfs.IsSquashfsMediaType(layer.MediaType) {
-			continue
-		}
-		digest := layer.Digest
-		contents := overlayPath(o.config.RootFSDir, digest, "overlay")
-		mounted, err := mount.IsMountpoint(contents)
-		if err == nil && mounted {
-			// We have already mounted this atom
-			continue
-		}
-		if hasDirEntries(contents) {
-			// We have done an unsquashfs of this atom
-			continue
-		}
-		if err := unpackOne(cacheDir, contents, digest, true); err != nil {
+		err := unpackOne(layer, ociDir, overlayPath(o.config.RootFSDir, layer.Digest, "overlay"))
+		if err != nil {
 			return errors.Wrapf(err, "Failed mounting %#v", layer)
 		}
 	}

pkg/overlay/pack.go

@@ -57,36 +57,11 @@ func (o *overlay) Unpack(tag, name string) error {
 
 	pool := NewThreadPool(runtime.NumCPU())
 
-	for _, layer := range manifest.Layers {
-		digest := layer.Digest
-		contents := overlayPath(o.config.RootFSDir, digest, "overlay")
-		if squashfs.IsSquashfsMediaType(layer.MediaType) {
-			// don't really need to do this in parallel, but what
-			// the hell.
-			pool.Add(func(ctx context.Context) error {
-				return unpackOne(cacheDir, contents, digest, true)
-			})
-		} else {
-			switch layer.MediaType {
-			case ispec.MediaTypeImageLayer:
-				fallthrough
-			case ispec.MediaTypeImageLayerGzip:
-				// don't extract things that have already been
-				// extracted
-				if _, err := os.Stat(contents); err == nil {
-					continue
-				}
-
-				// TODO: when the umoci API grows support for uid
-				// shifting, we can use the fancier features of context
-				// cancelling in the thread pool...
-				pool.Add(func(ctx context.Context) error {
-					return unpackOne(cacheDir, contents, digest, false)
-				})
-			default:
-				return errors.Errorf("unknown media type %s", layer.MediaType)
-			}
-		}
+	for _, curLayer := range manifest.Layers {
+		pool.Add(func(ctx context.Context) error {
+			return unpackOne(curLayer, cacheDir,
+				overlayPath(o.config.RootFSDir, curLayer.Digest, "overlay"))
+		})
 	}
 
 	pool.DoneAddingJobs()
@@ -655,29 +630,47 @@ func repackOverlay(config types.StackerConfig, name string, layerTypes []types.L
 	return ovl.write(config, name)
 }
 
-func unpackOne(ociDir string, bundlePath string, digest digest.Digest, isSquashfs bool) error {
-	if isSquashfs {
+// unpackOne - unpack a single layer (Descriptor) found in ociDir to extractDir
+//
+//	The result of calling unpackOne is either error or the contents available
+//	at the provided extractDir.  The extractDir should be either empty or
+//	fully populated with this layer.
+func unpackOne(l ispec.Descriptor, ociDir string, extractDir string) error {
+	if squashfs.IsSquashfsMediaType(l.MediaType) {
 		return squashfs.ExtractSingleSquash(
-			path.Join(ociDir, "blobs", "sha256", digest.Encoded()),
-			bundlePath, "overlay")
+			path.Join(ociDir, "blobs", "sha256", l.Digest.Encoded()), extractDir)
 	}
+	switch l.MediaType {
+	case ispec.MediaTypeImageLayer, ispec.MediaTypeImageLayerGzip:
+		if hasDirEntries(extractDir) {
+			// We have done an unsquashfs of this atom
+			return nil
+		}
 
-	oci, err := umoci.OpenLayout(ociDir)
-	if err != nil {
-		return err
-	}
-	defer oci.Close()
+		oci, err := umoci.OpenLayout(ociDir)
+		if err != nil {
+			return err
+		}
+		defer oci.Close()
 
-	compressed, err := oci.GetBlob(context.Background(), digest)
-	if err != nil {
-		return err
-	}
-	defer compressed.Close()
+		compressed, err := oci.GetBlob(context.Background(), l.Digest)
+		if err != nil {
+			return err
+		}
+		defer compressed.Close()
 
-	uncompressed, err := pgzip.NewReader(compressed)
-	if err != nil {
+		uncompressed, err := pgzip.NewReader(compressed)
+		if err != nil {
+			return err
+		}
+
+		err = layer.UnpackLayer(extractDir, uncompressed, nil)
+		if err != nil {
+			if rmErr := os.RemoveAll(extractDir); rmErr != nil {
+				log.Errorf("Failed to remove dir '%s' after failed extraction: %v", extractDir, rmErr)
+			}
+		}
 		return err
 	}
-
-	return layer.UnpackLayer(bundlePath, uncompressed, nil)
+	return errors.Errorf("unknown media type %s", l.MediaType)
 }

pkg/squashfs/squashfs.go

@@ -300,7 +300,7 @@ func squashFuse(squashFile, extractDir string) (*exec.Cmd, error) {
 	return cmd, nil
 }
 
-func ExtractSingleSquash(squashFile string, extractDir string, storageType string) error {
+func ExtractSingleSquash(squashFile string, extractDir string) error {
 	err := os.MkdirAll(extractDir, 0755)
 	if err != nil {
 		return err