Walk destination only once when computing delta

Avoid the double pass by constructing the list on the fly

pkg/api/core/image/delta_test.go

@@ -51,14 +51,12 @@ var _ = Describe("Delta", func() {
 			var tmpfile *os.File
 			var ref, ref2 name.Reference
 			var img, img2 v1.Image
-			var diff ImageDiff
 			var err error
 
 			ref, _ = name.ParseReference("alpine")
 			ref2, _ = name.ParseReference("golang:alpine")
 			img, _ = daemon.Image(ref)
 			img2, _ = daemon.Image(ref2)
-			diff, err = Delta(img, img2)
 
 			BeforeEach(func() {
 				ctx = types.NewContext()
@@ -69,11 +67,15 @@ var _ = Describe("Delta", func() {
 			})
 
 			It("Extract all deltas", func() {
+
+				f, err := ExtractDeltaAdditionsFiles(ctx, img, []string{}, []string{})
+				Expect(err).ToNot(HaveOccurred())
+
 				_, tmpdir, err := Extract(
 					ctx,
 					img2,
 					true,
-					ExtractDeltaFiles(ctx, diff, []string{}, []string{}),
+					f,
 				)
 				Expect(err).ToNot(HaveOccurred())
 				defer os.RemoveAll(tmpdir) // clean up
@@ -85,11 +87,15 @@ var _ = Describe("Delta", func() {
 			})
 
 			It("Extract deltas and excludes /usr/local/go", func() {
+				f, err := ExtractDeltaAdditionsFiles(ctx, img, []string{}, []string{"usr/local/go"})
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(err).ToNot(HaveOccurred())
 				_, tmpdir, err := Extract(
 					ctx,
 					img2,
 					true,
-					ExtractDeltaFiles(ctx, diff, []string{}, []string{"usr/local/go"}),
+					f,
 				)
 				Expect(err).ToNot(HaveOccurred())
 				defer os.RemoveAll(tmpdir) // clean up
@@ -97,11 +103,14 @@ var _ = Describe("Delta", func() {
 			})
 
 			It("Extract deltas and excludes /usr/local/go/bin, but includes /usr/local/go", func() {
+				f, err := ExtractDeltaAdditionsFiles(ctx, img, []string{"usr/local/go"}, []string{"usr/local/go/bin"})
+				Expect(err).ToNot(HaveOccurred())
+
 				_, tmpdir, err := Extract(
 					ctx,
 					img2,
 					true,
-					ExtractDeltaFiles(ctx, diff, []string{"usr/local/go"}, []string{"usr/local/go/bin"}),
+					f,
 				)
 				Expect(err).ToNot(HaveOccurred())
 				defer os.RemoveAll(tmpdir) // clean up
@@ -110,11 +119,13 @@ var _ = Describe("Delta", func() {
 			})
 
 			It("Extract deltas and includes /usr/local/go", func() {
+				f, err := ExtractDeltaAdditionsFiles(ctx, img, []string{"usr/local/go"}, []string{})
+				Expect(err).ToNot(HaveOccurred())
 				_, tmpdir, err := Extract(
 					ctx,
 					img2,
 					true,
-					ExtractDeltaFiles(ctx, diff, []string{"usr/local/go"}, []string{}),
+					f,
 				)
 				Expect(err).ToNot(HaveOccurred())
 				defer os.RemoveAll(tmpdir) // clean up

pkg/api/core/image/extract.go

@@ -37,23 +37,38 @@ import (
 // Afterward create artifact pointing to the dir
 
 // ExtractDeltaFiles returns an handler to extract files in a list
-func ExtractDeltaFiles(
+func ExtractDeltaAdditionsFiles(
 	ctx *types.Context,
-	d ImageDiff,
+	srcimg v1.Image,
 	includes []string, excludes []string,
-) func(h *tar.Header) (bool, error) {
+) (func(h *tar.Header) (bool, error), error) {
 
 	includeRegexp := compileRegexes(includes)
 	excludeRegexp := compileRegexes(excludes)
+	filesSrc := map[string]interface{}{}
 
-	additions := map[string]interface{}{}
+	srcReader := mutate.Extract(srcimg)
-	for _, a := range d.Additions {
+	defer srcReader.Close()
-		additions[a.Name] = nil
+
+	srcTar := tar.NewReader(srcReader)
+
+	for {
+		var hdr *tar.Header
+		hdr, err := srcTar.Next()
+		if err == io.EOF {
+			// end of tar archive
+			break
+		}
+		if err != nil {
+			return nil, err
+		}
+		filesSrc[hdr.Name] = nil
 	}
+
 	return func(h *tar.Header) (bool, error) {
 		fileName := filepath.Join(string(os.PathSeparator), h.Name)
-		_, exists := additions[h.Name]
+		_, exists := filesSrc[h.Name]
-		if !exists {
+		if exists {
 			return false, nil
 		}
 
@@ -97,7 +112,7 @@ func ExtractDeltaFiles(
 			return true, nil
 		}
 
-	}
+	}, nil
 }
 
 func ExtractFiles(
@@ -166,51 +181,63 @@ func ExtractFiles(
 func ExtractReader(ctx *types.Context, reader io.ReadCloser, output string, keepPerms bool, filter func(h *tar.Header) (bool, error), opts ...containerdarchive.ApplyOpt) (int64, string, error) {
 	defer reader.Close()
 
-	perms := map[string][]int{}
+	// If no filter is specified, grab all.
-	xattrs := map[string]map[string]string{}
+	if filter == nil {
-	paxrecords := map[string]map[string]string{}
+		filter = func(h *tar.Header) (bool, error) { return true, nil }
+	}
+
+	// Keep records of permissions as we walk the tar
+	type permData struct {
+		PAX, Xattrs map[string]string
+		Uid, Gid    int
+		Name        string
+	}
+
+	perms := []permData{}
 
 	f := func(h *tar.Header) (bool, error) {
-		perms[h.Name] = []int{h.Gid, h.Uid}
+		res, err := filter(h)
-		xattrs[h.Name] = h.Xattrs
+		if res {
-		paxrecords[h.Name] = h.PAXRecords
+			perms = append(perms, permData{
-		if filter != nil {
+				PAX: h.PAXRecords,
-			return filter(h)
+				Uid: h.Uid, Gid: h.Gid,
+				Xattrs: h.Xattrs,
+				Name:   h.Name,
+			})
 		}
-		return true, nil
+		return res, err
 	}
 
 	opts = append(opts, containerdarchive.WithFilter(f))
 
+	// Handle the extraction
 	c, err := containerdarchive.Apply(context.Background(), output, reader, opts...)
 	if err != nil {
 		return 0, "", err
 	}
 
-	// TODO: Parametrize this
+	// Reconstruct permissions
 	if keepPerms {
-		for f, p := range perms {
+		ctx.Info("Reconstructing permissions")
-			ff := filepath.Join(output, f)
+		for _, p := range perms {
+			ff := filepath.Join(output, p.Name)
 			if _, err := os.Lstat(ff); err == nil {
-				if err := os.Lchown(ff, p[1], p[0]); err != nil {
+				if err := os.Lchown(ff, p.Uid, p.Gid); err != nil {
 					ctx.Warning(err, "failed chowning file")
 				}
 			}
-		}
+			for _, attrs := range []map[string]string{p.Xattrs, p.PAX} {
-
-		for _, m := range []map[string]map[string]string{xattrs, paxrecords} {
-			for key, attrs := range m {
-				ff := filepath.Join(output, key)
 				for k, attr := range attrs {
 					if err := system.Lsetxattr(ff, k, []byte(attr), 0); err != nil {
 						if errors.Is(err, syscall.ENOTSUP) {
-							ctx.Debug("ignored xattr %s in archive", key)
+							ctx.Debug("ignored xattr %s in archive", ff)
 						}
 					}
 				}
 			}
 		}
 	}
+
 	return c, output, nil
 }
 

pkg/compiler/compiler.go

@@ -311,29 +311,35 @@ func (cs *LuetCompiler) unpackDelta(concurrency int, keepPermissions bool, p *co
 
 	cs.Options.Context.Info(pkgTag, ":hammer: Generating delta")
 
+	cs.Options.Context.Info(pkgTag, ":hammer: Retrieving reference for", builderOpts.ImageName)
+
 	ref, err := cs.Backend.ImageReference(builderOpts.ImageName)
 	if err != nil {
 		return nil, err
 	}
 
+	cs.Options.Context.Info(pkgTag, ":hammer: Retrieving reference for", runnerOpts.ImageName)
+
 	ref2, err := cs.Backend.ImageReference(runnerOpts.ImageName)
 	if err != nil {
 		return nil, err
 	}
 
-	diff, err := image.Delta(ref, ref2)
+	cs.Options.Context.Info(pkgTag, ":hammer: Generating filters for extraction")
+
+	filter, err := image.ExtractDeltaAdditionsFiles(cs.Options.Context, ref, p.GetIncludes(), p.GetExcludes())
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "failed generating filter for extraction")
 	}
 
-	// TODO: includes/excludes might need to get "/" stripped from prefix
+	cs.Options.Context.Info(pkgTag, ":hammer: Extracting artifact from image", runnerOpts.ImageName)
 	a, err := artifact.ImageToArtifact(
 		cs.Options.Context,
 		ref2,
 		cs.Options.CompressionType,
 		p.Rel(fmt.Sprintf("%s%s", p.GetPackage().GetFingerPrint(), ".package.tar")),
 		keepPermissions,
-		image.ExtractDeltaFiles(cs.Options.Context, diff, p.GetIncludes(), p.GetExcludes()),
+		filter,
 	)
 	if err != nil {
 		return nil, err