Skip to content

Commit

Permalink
fix: Update ExtractSingleSquash, adding policy.
Browse files Browse the repository at this point in the history
Overall, there are 3 "good things" done by this change:
1. Fix bug in the current code which tries mounting with each option
   every time.  The problem with doing that is really that the kernel
   mount option didn't work very well.  It would fail with "already
   mounted", and then squashfuse would end up getting used to mount over
   the top.

2. Fix race conditions in the current code.

   overlay.Unpack starts a thread pool and tries to unpack all layers
   at once.  That is fine if there are no duplicate layers.  But
      if there are duplicate layers used by a stacker.yaml file, then
   there were races on extraction.  The end result really was that things
   would get mounted more than once.

   Example stacker that shows this:

    l1:
      from:
        type: docker
        url: docker://busybox:latest
      run: |
        echo build layer 1

    l2:
      from:
        type: docker
        url: docker://busybox:latest
      run: |
        echo build layer 1

  There, the busybox layer would get extracted multiple times.

  The code here has a single lock on ExtractSingleSquash, it would
  be better to have lock being taken per extractDir.

3. Allow the user to control the list of extractors.

   If they knew that they could not use kernel mounts (or could, but
   didn't want to) or wanted to use unsquashfs they can now do that.

   STACKER_SQUASHFS_EXTRACT_POLICY=kmount stacker build ..

   or

   STACKER_SQUASHFS_EXTRACT_POLICY="squashfuse kmount" stacker build ...

   This adds a SquashExtractor interface, with 3 implementers
   (KernelExtractor, SquashFuseExtractor, UnsquashfsExtractor).

   A ExtractPolicy is basically a list of Extractors to try.
   The first time ExtractPolicy is used it will try each of the Extractors
   in order.  It then stores the result in .Extractor and uses that
   subsequently.

Signed-off-by: Scott Moser <[email protected]>
  • Loading branch information
smoser committed Oct 4, 2023
1 parent 7b48263 commit 39984ea
Show file tree
Hide file tree
Showing 3 changed files with 344 additions and 62 deletions.
16 changes: 12 additions & 4 deletions pkg/overlay/pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"runtime"
"strings"
"sync"
"time"

"github.com/klauspost/pgzip"
Expand All @@ -29,6 +30,8 @@ import (
"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
Expand Down Expand Up @@ -58,9 +61,10 @@ func (o *overlay) Unpack(tag, name string) error {
pool := NewThreadPool(runtime.NumCPU())

for _, curLayer := range manifest.Layers {
// copy layer to avoid race on pool access.
l := curLayer
pool.Add(func(ctx context.Context) error {
return unpackOne(curLayer, cacheDir,
overlayPath(o.config.RootFSDir, curLayer.Digest, "overlay"))
return unpackOne(l, cacheDir, overlayPath(o.config.RootFSDir, l.Digest, "overlay"))
})

Check warning on line 68 in pkg/overlay/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/overlay/pack.go#L63-L68

Added lines #L63 - L68 were not covered by tests
}

Expand Down Expand Up @@ -207,7 +211,8 @@ func (o *overlay) initializeBasesInOutput(name string, layerTypes []types.LayerT
return err
}
} else {
log.Debugf("converting between %v and %v", sourceLayerType, layerType)
log.Debugf("creating layer %s (type=%s) by converting layer %s (type=%s)",
layerType.LayerName(name), layerType, sourceLayerType.LayerName(name), sourceLayerType)

Check warning on line 215 in pkg/overlay/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/overlay/pack.go#L214-L215

Added lines #L214 - L215 were not covered by tests
err = ConvertAndOutput(o.config, cacheTag, name, layerType)
if err != nil {
return err
Expand Down Expand Up @@ -642,8 +647,11 @@ func unpackOne(l ispec.Descriptor, ociDir string, extractDir string) error {
}
switch l.MediaType {
case ispec.MediaTypeImageLayer, ispec.MediaTypeImageLayerGzip:
tarEx.Lock()
defer tarEx.Unlock()

Check warning on line 651 in pkg/overlay/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/overlay/pack.go#L648-L651

Added lines #L648 - L651 were not covered by tests

if hasDirEntries(extractDir) {
// We have done an unsquashfs of this atom
// the directory was already populated.
return nil
}

Check warning on line 656 in pkg/overlay/pack.go

View check run for this annotation

Codecov / codecov/patch

pkg/overlay/pack.go#L653-L656

Added lines #L653 - L656 were not covered by tests

Expand Down
Loading

0 comments on commit 39984ea

Please sign in to comment.