Merge pull request #24 from mzihlmann/add-cache-2

fix: ADD learned to cache it's output layer

integration/images.go

@@ -93,6 +93,7 @@ var additionalKanikoFlagsMap = map[string][]string{
 	"Dockerfile_test_cache_install_oci":      {"--cache-copy-layers=true"},
 	"Dockerfile_test_cache_copy":             {"--cache-copy-layers=true"},
 	"Dockerfile_test_cache_copy_oci":         {"--cache-copy-layers=true"},
+	"Dockerfile_test_issue_add":              {"--cache-copy-layers=true"},
 }
 
 // output check to do when building with kaniko
@@ -226,9 +227,8 @@ func NewDockerFileBuilder() *DockerFileBuilder {
 		"Dockerfile_test_cache_copy":    {},
 		"Dockerfile_test_issue_3429":    {},
 		"Dockerfile_test_issue_workdir": {},
-		// TODO: ADD command is uncacheable
-		//"Dockerfile_test_issue_add":    {},
-		"Dockerfile_test_issue_empty": {},
+		"Dockerfile_test_issue_add":     {},
+		"Dockerfile_test_issue_empty":   {},
 	}
 	d.TestOCICacheDockerfiles = map[string]struct{}{
 		"Dockerfile_test_cache_oci":         {},

pkg/commands/add.go

@@ -17,6 +17,7 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
 	"io/fs"
 	"path/filepath"
 
@@ -24,6 +25,7 @@ import (
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
 	"github.com/pkg/errors"
 
+	kConfig "github.com/GoogleContainerTools/kaniko/pkg/config"
 	"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
@@ -35,6 +37,7 @@ type AddCommand struct {
 	cmd           *instructions.AddCommand
 	fileContext   util.FileContext
 	snapshotFiles []string
+	shdCache      bool
 }
 
 // ExecuteCommand executes the ADD command
@@ -132,9 +135,98 @@ func (a *AddCommand) String() string {
 }
 
 func (a *AddCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs) ([]string, error) {
+	return addCmdFilesUsedFromContext(config, buildArgs, a.cmd, a.fileContext)
+}
+
+func (a *AddCommand) MetadataOnly() bool {
+	return false
+}
+
+func (a *AddCommand) RequiresUnpackedFS() bool {
+	return true
+}
+
+func (a *AddCommand) ShouldCacheOutput() bool {
+	return a.shdCache
+}
+
+// CacheCommand returns true since this command should be cached
+func (a *AddCommand) CacheCommand(img v1.Image) DockerCommand {
+	return &CachingAddCommand{
+		img:         img,
+		cmd:         a.cmd,
+		fileContext: a.fileContext,
+		extractFn:   util.ExtractFile,
+	}
+}
+
+type CachingAddCommand struct {
+	BaseCommand
+	caching
+	img            v1.Image
+	extractedFiles []string
+	cmd            *instructions.AddCommand
+	fileContext    util.FileContext
+	extractFn      util.ExtractFunction
+}
+
+func (ca *CachingAddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	logrus.Infof("Found cached layer, extracting to filesystem")
+	var err error
+
+	if ca.img == nil {
+		return errors.New(fmt.Sprintf("cached command image is nil %v", ca.String()))
+	}
+
+	layers, err := ca.img.Layers()
+	if err != nil {
+		return errors.Wrapf(err, "retrieve image layers")
+	}
+
+	if len(layers) != 1 {
+		return errors.New(fmt.Sprintf("expected %d layers but got %d", 1, len(layers)))
+	}
+
+	ca.layer = layers[0]
+	ca.extractedFiles, err = util.GetFSFromLayers(kConfig.RootDir, layers, util.ExtractFunc(ca.extractFn), util.IncludeWhiteout())
+
+	logrus.Debugf("ExtractedFiles: %s", ca.extractedFiles)
+	if err != nil {
+		return errors.Wrap(err, "extracting fs from image")
+	}
+
+	return nil
+}
+
+func (ca *CachingAddCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs) ([]string, error) {
+	return addCmdFilesUsedFromContext(config, buildArgs, ca.cmd, ca.fileContext)
+}
+
+func (ca *CachingAddCommand) FilesToSnapshot() []string {
+	f := ca.extractedFiles
+	logrus.Debugf("%d files extracted by caching copy command", len(f))
+	logrus.Tracef("Extracted files: %s", f)
+
+	return f
+}
+
+func (ca *CachingAddCommand) MetadataOnly() bool {
+	return false
+}
+
+func (ca *CachingAddCommand) String() string {
+	if ca.cmd == nil {
+		return "nil command"
+	}
+	return ca.cmd.String()
+}
+
+func addCmdFilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmd *instructions.AddCommand,
+	fileContext util.FileContext,
+) ([]string, error) {
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
 
-	srcs, _, err := util.ResolveEnvAndWildcards(a.cmd.SourcesAndDest, a.fileContext, replacementEnvs)
+	srcs, _, err := util.ResolveEnvAndWildcards(cmd.SourcesAndDest, fileContext, replacementEnvs)
 	if err != nil {
 		return nil, err
 	}
@@ -147,18 +239,10 @@ func (a *AddCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfi
 		if util.IsFileLocalTarArchive(src) {
 			continue
 		}
-		fullPath := filepath.Join(a.fileContext.Root, src)
+		fullPath := filepath.Join(fileContext.Root, src)
 		files = append(files, fullPath)
 	}
 
 	logrus.Infof("Using files from context: %v", files)
 	return files, nil
 }
-
-func (a *AddCommand) MetadataOnly() bool {
-	return false
-}
-
-func (a *AddCommand) RequiresUnpackedFS() bool {
-	return true
-}

pkg/commands/commands.go

@@ -80,7 +80,7 @@ func GetCommand(cmd instructions.Command, fileContext util.FileContext, useNewRu
 	case *instructions.WorkdirCommand:
 		return &WorkdirCommand{cmd: c, shdCache: cacheRun}, nil
 	case *instructions.AddCommand:
-		return &AddCommand{cmd: c, fileContext: fileContext}, nil
+		return &AddCommand{cmd: c, fileContext: fileContext, shdCache: cacheCopy}, nil
 	case *instructions.CmdCommand:
 		return &CmdCommand{cmd: c}, nil
 	case *instructions.EntrypointCommand:

pkg/executor/build.go

@@ -440,12 +440,6 @@ func (s *stageBuilder) build() error {
 
 				logrus.Debugf("Build: cache key for command %v %v", command.String(), ck)
 
-				// Raise Warnings for commands that are uncacheable
-				switch command.(type) {
-				case *commands.AddCommand:
-					logrus.Warn("cache-violation: ADD can't be cached - consider using COPY instead.")
-				}
-
 				// Push layer to cache (in parallel) now along with new config file
 				if command.ShouldCacheOutput() && !s.opts.NoPushCache {
 					cacheGroup.Go(func() error {