Simplify delta generation, and avoid two-pass with img backend

This changeset also drops --keep-exported-images, which is quite unused
and can be replaced with a plugin, or either by manually exporting the
resulting images.

cmd/build.go

@@ -79,8 +79,6 @@ Build packages specifying multiple definition trees:
 		viper.BindPFlag("wait", cmd.Flags().Lookup("wait"))
 		viper.BindPFlag("keep-images", cmd.Flags().Lookup("keep-images"))
 
-		LuetCfg.Viper.BindPFlag("keep-exported-images", cmd.Flags().Lookup("keep-exported-images"))
-
 		LuetCfg.Viper.BindPFlag("solver.type", cmd.Flags().Lookup("solver-type"))
 		LuetCfg.Viper.BindPFlag("solver.discount", cmd.Flags().Lookup("solver-discount"))
 		LuetCfg.Viper.BindPFlag("solver.rate", cmd.Flags().Lookup("solver-rate"))
@@ -105,7 +103,6 @@ Build packages specifying multiple definition trees:
 		keepImages := viper.GetBool("keep-images")
 		nodeps := viper.GetBool("nodeps")
 		onlydeps := viper.GetBool("onlydeps")
-		keepExportedImages := viper.GetBool("keep-exported-images")
 		onlyTarget, _ := cmd.Flags().GetBool("only-target-package")
 		full, _ := cmd.Flags().GetBool("full")
 		concurrent, _ := cmd.Flags().GetBool("solver-concurrent")
@@ -169,7 +166,6 @@ Build packages specifying multiple definition trees:
 		opts.OnlyDeps = onlydeps
 		opts.NoDeps = nodeps
 		opts.Wait = wait
-		opts.KeepImageExport = keepExportedImages
 		opts.PackageTargetOnly = onlyTarget
 		opts.BuildValuesFile = values
 		var solverOpts solver.Options
@@ -315,7 +311,6 @@ func init() {
 	buildCmd.Flags().Bool("keep-images", true, "Keep built docker images in the host")
 	buildCmd.Flags().Bool("nodeps", false, "Build only the target packages, skipping deps (it works only if you already built the deps locally, or by using --pull) ")
 	buildCmd.Flags().Bool("onlydeps", false, "Build only package dependencies")
-	buildCmd.Flags().Bool("keep-exported-images", false, "Keep exported images used during building")
 	buildCmd.Flags().Bool("only-target-package", false, "Build packages of only the required target. Otherwise builds all the necessary ones not present in the destination")
 	buildCmd.Flags().String("solver-type", "", "Solver strategy")
 	buildCmd.Flags().Float32("solver-rate", 0.7, "Solver learning rate")

pkg/compiler/backend/diff.go

@@ -54,10 +54,7 @@ import (
 // ]
 func GenerateChanges(b compiler.CompilerBackend, fromImage, toImage compiler.CompilerBackendOptions) ([]compiler.ArtifactLayer, error) {
 
-	srcImage := fromImage.Destination
-	dstImage := toImage.Destination
-
-	res := compiler.ArtifactLayer{FromImage: srcImage, ToImage: dstImage}
+	res := compiler.ArtifactLayer{FromImage: fromImage.ImageName, ToImage: toImage.ImageName}
 
 	tmpdiffs, err := config.LuetCfg.GetSystem().TempDir("extraction")
 	if err != nil {
@@ -77,62 +74,22 @@ func GenerateChanges(b compiler.CompilerBackend, fromImage, toImage compiler.Com
 	}
 	defer os.RemoveAll(dstRootFS) // clean up
 
-	// Handle both files (.tar) or images. If parameters are beginning with / , don't export the images
-	if !strings.HasPrefix(srcImage, "/") {
-		srcImageTar, err := ioutil.TempFile(tmpdiffs, "srctar")
-		if err != nil {
-			return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while creating tempdir for rootfs")
-		}
-
-		defer os.Remove(srcImageTar.Name()) // clean up
-		srcImageExport := compiler.CompilerBackendOptions{
-			ImageName:   srcImage,
-			Destination: srcImageTar.Name(),
-		}
-		err = b.ExportImage(srcImageExport)
-		if err != nil {
-			return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while exporting src image "+srcImage)
-		}
-		srcImage = srcImageTar.Name()
-	}
-
 	srcImageExtract := compiler.CompilerBackendOptions{
-		SourcePath:  srcImage,
 		ImageName:   fromImage.ImageName,
 		Destination: srcRootFS,
 	}
 	err = b.ExtractRootfs(srcImageExtract, false) // No need to keep permissions as we just collect file diffs
 	if err != nil {
-		return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while unpacking src image "+srcImage)
-	}
-
-	// Handle both files (.tar) or images. If parameters are beginning with / , don't export the images
-	if !strings.HasPrefix(dstImage, "/") {
-		dstImageTar, err := ioutil.TempFile(tmpdiffs, "dsttar")
-		if err != nil {
-			return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while creating tempdir for rootfs")
-		}
-
-		defer os.Remove(dstImageTar.Name()) // clean up
-		dstImageExport := compiler.CompilerBackendOptions{
-			ImageName:   dstImage,
-			Destination: dstImageTar.Name(),
-		}
-		err = b.ExportImage(dstImageExport)
-		if err != nil {
-			return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while exporting dst image "+dstImage)
-		}
-		dstImage = dstImageTar.Name()
+		return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while unpacking src image "+fromImage.ImageName)
 	}
 
 	dstImageExtract := compiler.CompilerBackendOptions{
-		SourcePath:  dstImage,
 		ImageName:   toImage.ImageName,
 		Destination: dstRootFS,
 	}
 	err = b.ExtractRootfs(dstImageExtract, false)
 	if err != nil {
-		return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while unpacking dst image "+dstImage)
+		return []compiler.ArtifactLayer{}, errors.Wrap(err, "Error met while unpacking dst image "+toImage.ImageName)
 	}
 
 	// Get Additions/Changes. dst -> src

pkg/compiler/backend/diff_test.go

@@ -33,8 +33,7 @@ var _ = Describe("Docker image diffs", func() {
 	Context("Generate diffs from docker images", func() {
 		It("Detect no changes", func() {
 			opts := compiler.CompilerBackendOptions{
-				ImageName:   "alpine:latest",
-				Destination: "alpine:latest",
+				ImageName: "alpine:latest",
 			}
 			err := b.DownloadImage(opts)
 			Expect(err).ToNot(HaveOccurred())
@@ -58,11 +57,9 @@ var _ = Describe("Docker image diffs", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			layers, err := GenerateChanges(b, compiler.CompilerBackendOptions{
-				ImageName:   "quay.io/mocaccino/micro",
-				Destination: "quay.io/mocaccino/micro",
+				ImageName: "quay.io/mocaccino/micro",
 			}, compiler.CompilerBackendOptions{
-				ImageName:   "quay.io/mocaccino/extra",
-				Destination: "quay.io/mocaccino/extra",
+				ImageName: "quay.io/mocaccino/extra",
 			})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(len(layers)).To(Equal(1))

pkg/compiler/backend/simpledocker.go

@@ -184,9 +184,22 @@ type ManifestEntry struct {
 }
 
 func (b *SimpleDocker) ExtractRootfs(opts compiler.CompilerBackendOptions, keepPerms bool) error {
-	src := opts.SourcePath
+	name := opts.ImageName
 	dst := opts.Destination
 
+	tempexport, err := ioutil.TempDir(dst, "tmprootfs")
+	if err != nil {
+		return errors.Wrap(err, "Error met while creating tempdir for rootfs")
+	}
+	defer os.RemoveAll(tempexport) // clean up
+
+	imageExport := filepath.Join(tempexport, "image.tar")
+	if err := b.ExportImage(compiler.CompilerBackendOptions{ImageName: name, Destination: imageExport}); err != nil {
+		return errors.Wrap(err, "failed while extracting rootfs for "+name)
+	}
+
+	src := imageExport
+
 	if src == "" && opts.ImageName != "" {
 		tempUnpack, err := ioutil.TempDir(dst, "tempUnpack")
 		if err != nil {

pkg/compiler/compiler.go

@@ -236,7 +236,20 @@ func (cs *LuetCompiler) stripFromRootfs(includes []string, rootfs string, includ
 	return nil
 }
 
-func (cs *LuetCompiler) unpackFs(rootfs string, concurrency int, p CompilationSpec) (Artifact, error) {
+func (cs *LuetCompiler) unpackFs(concurrency int, keepPermissions bool, p CompilationSpec, runnerOpts CompilerBackendOptions) (Artifact, error) {
+
+	rootfs, err := ioutil.TempDir(p.GetOutputPath(), "rootfs")
+	if err != nil {
+		return nil, errors.Wrap(err, "Could not create tempdir")
+	}
+	defer os.RemoveAll(rootfs) // clean up
+
+	err = cs.Backend.ExtractRootfs(CompilerBackendOptions{
+		ImageName: runnerOpts.ImageName, Destination: rootfs}, keepPermissions)
+	if err != nil {
+		return nil, errors.Wrap(err, "Could not extract rootfs")
+	}
+
 	if p.GetPackageDir() != "" {
 		Info(":tophat: Packing from output dir", p.GetPackageDir())
 		rootfs = filepath.Join(rootfs, p.GetPackageDir())
@@ -247,7 +260,7 @@ func (cs *LuetCompiler) unpackFs(rootfs string, concurrency int, p CompilationSp
 		cs.stripFromRootfs(p.GetIncludes(), rootfs, true)
 	}
 	if len(p.GetExcludes()) > 0 {
-		// strip from includes
+		// strip from excludes
 		cs.stripFromRootfs(p.GetExcludes(), rootfs, false)
 	}
 	artifact := NewPackageArtifact(p.Rel(p.GetPackage().GetFingerPrint() + ".package.tar"))
@@ -261,7 +274,14 @@ func (cs *LuetCompiler) unpackFs(rootfs string, concurrency int, p CompilationSp
 	return artifact, nil
 }
 
-func (cs *LuetCompiler) unpackDelta(rootfs string, concurrency int, keepPermissions bool, p CompilationSpec, builderOpts, runnerOpts CompilerBackendOptions) (Artifact, error) {
+func (cs *LuetCompiler) unpackDelta(concurrency int, keepPermissions bool, p CompilationSpec, builderOpts, runnerOpts CompilerBackendOptions) (Artifact, error) {
+
+	rootfs, err := ioutil.TempDir(p.GetOutputPath(), "rootfs")
+	if err != nil {
+		return nil, errors.Wrap(err, "Could not create tempdir")
+	}
+	defer os.RemoveAll(rootfs) // clean up
+
 	pkgTag := ":package: " + p.GetPackage().HumanReadableString()
 	if cs.Options.PullFirst && !cs.Backend.ImageExists(builderOpts.ImageName) && cs.Backend.ImageAvailable(builderOpts.ImageName) {
 		err := cs.Backend.DownloadImage(builderOpts)
@@ -271,12 +291,7 @@ func (cs *LuetCompiler) unpackDelta(rootfs string, concurrency int, keepPermissi
 	} else if !cs.Backend.ImageExists(builderOpts.ImageName) {
 		return nil, errors.New("No image found for " + builderOpts.ImageName)
 	}
-	if err := cs.Backend.ExportImage(builderOpts); err != nil {
-		return nil, errors.Wrap(err, "Could not export image"+builderOpts.ImageName)
-	}
-	if !cs.Options.KeepImageExport {
-		defer os.Remove(builderOpts.Destination)
-	}
+
 	Info(pkgTag, ":hammer: Generating delta")
 	diffs, err := cs.Backend.Changes(builderOpts, runnerOpts)
 	if err != nil {
@@ -456,38 +471,15 @@ func (cs *LuetCompiler) genArtifact(p CompilationSpec, builderOpts, runnerOpts C
 		return artifact, nil
 	}
 
-	// prepare folder content of the image with the package compiled inside
-	if err := cs.Backend.ExportImage(runnerOpts); err != nil {
-		return nil, errors.Wrap(err, "Failed exporting image")
-	}
-
-	if !cs.Options.KeepImageExport {
-		defer os.Remove(runnerOpts.Destination)
-	}
-
-	rootfs, err = ioutil.TempDir(p.GetOutputPath(), "rootfs")
-	if err != nil {
-		return nil, errors.Wrap(err, "Could not create tempdir")
-	}
-	defer os.RemoveAll(rootfs) // clean up
-
-	// TODO: Compression and such
-	err = cs.Backend.ExtractRootfs(CompilerBackendOptions{
-		ImageName:  runnerOpts.ImageName,
-		SourcePath: runnerOpts.Destination, Destination: rootfs}, keepPermissions)
-	if err != nil {
-		return nil, errors.Wrap(err, "Could not extract rootfs")
-	}
-
 	if p.UnpackedPackage() {
 		// Take content of container as a base for our package files
-		artifact, err = cs.unpackFs(rootfs, concurrency, p)
+		artifact, err = cs.unpackFs(concurrency, keepPermissions, p, runnerOpts)
 		if err != nil {
 			return nil, errors.Wrap(err, "Error met while extracting image")
 		}
 	} else {
 		// Generate delta between the two images
-		artifact, err = cs.unpackDelta(rootfs, concurrency, keepPermissions, p, builderOpts, runnerOpts)
+		artifact, err = cs.unpackDelta(concurrency, keepPermissions, p, builderOpts, runnerOpts)
 		if err != nil {
 			return nil, errors.Wrap(err, "Error met while generating delta")
 		}

pkg/compiler/interface.go

@@ -50,7 +50,6 @@ type CompilerOptions struct {
 	PullFirst, KeepImg, Push bool
 	Concurrency              int
 	CompressionType          CompressionImplementation
-	KeepImageExport          bool
 
 	Wait            bool
 	OnlyDeps        bool