From 0efe615a1e096399a77e0ebcaea4779d56001d2a Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 27 Sep 2023 11:27:22 -0700 Subject: [PATCH] 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 --- pkg/overlay/overlay.go | 21 ++-------- pkg/overlay/pack.go | 89 ++++++++++++++++++---------------------- pkg/squashfs/squashfs.go | 2 +- 3 files changed, 45 insertions(+), 67 deletions(-) diff --git a/pkg/overlay/overlay.go b/pkg/overlay/overlay.go index 289eac9c..3feec586 100644 --- a/pkg/overlay/overlay.go +++ b/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) } } diff --git a/pkg/overlay/pack.go b/pkg/overlay/pack.go index 2653b132..32555dd4 100644 --- a/pkg/overlay/pack.go +++ b/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) } diff --git a/pkg/squashfs/squashfs.go b/pkg/squashfs/squashfs.go index 667e5c21..bf7d8a12 100644 --- a/pkg/squashfs/squashfs.go +++ b/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