Fix/mounting squashfs extract

pkg/overlay/overlay.go

@@ -14,8 +14,6 @@
 	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 @@
 	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

@@ -10,6 +10,7 @@
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/klauspost/pgzip"
@@ -29,6 +30,8 @@
 	"stackerbuild.io/stacker/pkg/types"
 )
 
+var tarEx sync.Mutex
+
 func safeOverlayName(d digest.Digest) string {
 	// dirs used in overlay lowerdir args can't have : in them, so lets
 	// sanitize it
@@ -57,36 +60,19 @@
 
 	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)
-			}
+	seen := map[digest.Digest]bool{}
+	for _, curLayer := range manifest.Layers {
+		// avoid calling unpackOne twice for the same digest
+		if seen[curLayer.Digest] {
+			continue
 		}
+		seen[curLayer.Digest] = true
+
+		// copy layer to avoid race on pool access.
+		l := curLayer
+		pool.Add(func(ctx context.Context) error {
+			return unpackOne(l, cacheDir, overlayPath(o.config.RootFSDir, l.Digest, "overlay"))
+		})
 	}
 
 	pool.DoneAddingJobs()
@@ -154,6 +140,9 @@
 			return err
 		}
 
+		log.Debugf("new oci layer %s [%s] created from path %s as part of %s:%s",
+			desc.Digest, layerType, overlayPath(config.RootFSDir, theLayer.Digest), name, tag)
+
 		// slight hack, but this is much faster than a cp, and the
 		// layers are the same, just in different formats
 		err = os.Symlink(overlayPath(config.RootFSDir, theLayer.Digest), overlayPath(config.RootFSDir, desc.Digest))
@@ -232,7 +221,8 @@
 						return err
 					}
 				} else {
-					log.Debugf("converting between %v and %v", sourceLayerType, layerType)
+					log.Debugf("creating oci image %s (type=%s) by converting %s (type=%s)",
+						layerType.LayerName(name), layerType, sourceLayerType.LayerName(name), sourceLayerType)
 					err = ConvertAndOutput(o.config, cacheTag, name, layerType)
 					if err != nil {
 						return err
@@ -447,6 +437,7 @@
 		return false, errors.Wrapf(err, "couldn't make new layer overlay dir")
 	}
 
+	log.Debugf("renaming %s -> %s", dir, path.Join(target, "overlay"))
 	err = os.Rename(dir, path.Join(target, "overlay"))
 	if err != nil {
 		if !os.IsExist(err) {
@@ -477,6 +468,7 @@
 	for _, desc := range descs[1:] {
 		linkPath := overlayPath(config.RootFSDir, desc.Digest)
 
+		log.Debugf("link %s -> %s", linkPath, target)
 		err = os.Symlink(target, linkPath)
 		if err != nil {
 			// as above, this symlink may already exist; if it does, we can
@@ -548,7 +540,7 @@
 		mutators = append(mutators, mutator)
 	}
 
-	log.Debugf("Generating overlay_dirs layers")
+	log.Debugf("Generating overlay_dirs layers for %s", name)
 	mutated := false
 	for i, layerType := range layerTypes {
 		ods, ok := ovl.OverlayDirLayers[layerType]
@@ -655,29 +647,57 @@
 	return ovl.write(config, name)
 }
 
-func unpackOne(ociDir string, bundlePath string, digest digest.Digest, isSquashfs bool) error {
-	if isSquashfs {
-		return squashfs.ExtractSingleSquash(
-			path.Join(ociDir, "blobs", "sha256", digest.Encoded()),
-			bundlePath, "overlay")
+// 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 {
+	// population of a dir is not atomic, at least for tar extraction.
+	// As a result, we could hasDirEntries(extractDir) at the same time that
+	// something is un-populating that dir due to a failed extraction (like
+	// os.RemoveAll below).
+	// There needs to be a lock on the extract dir (scoped to the overlay storage backend).
+	// A sync.RWMutex would work well here since it is safe to check as long
+	// as no one is populating or unpopulating.
+	if hasDirEntries(extractDir) {
+		// the directory was already populated.
+		return nil
 	}
 
-	oci, err := umoci.OpenLayout(ociDir)
-	if err != nil {
-		return err
+	if squashfs.IsSquashfsMediaType(l.MediaType) {
+		return squashfs.ExtractSingleSquash(
+			path.Join(ociDir, "blobs", "sha256", l.Digest.Encoded()), extractDir)
 	}
-	defer oci.Close()
+	switch l.MediaType {
+	case ispec.MediaTypeImageLayer, ispec.MediaTypeImageLayerGzip:
+		tarEx.Lock()
+		defer tarEx.Unlock()
 
-	compressed, err := oci.GetBlob(context.Background(), digest)
-	if err != nil {
-		return err
-	}
-	defer compressed.Close()
+		oci, err := umoci.OpenLayout(ociDir)
+		if err != nil {
+			return err
+		}
+		defer oci.Close()
 
-	uncompressed, err := pgzip.NewReader(compressed)
-	if err != nil {
+		compressed, err := oci.GetBlob(context.Background(), l.Digest)
+		if err != nil {
+			return err
+		}
+		defer compressed.Close()
+
+		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)
 }