Merge pull request #22 from mzihlmann/workdir-cache-2

WORKDIR learned to cache it's potential output layer

integration/dockerfiles/Dockerfile_test_cache_install

@@ -17,7 +17,6 @@
 # if the cache is implemented correctly
 
 FROM debian:10.13
-RUN mkdir /foo
 WORKDIR /foo
 RUN apt-get update && apt-get install -y make
 COPY context/bar /context

integration/dockerfiles/Dockerfile_test_cache_install_oci

@@ -17,7 +17,6 @@
 # if the cache is implemented correctly
 
 FROM debian:10.13
-RUN mkdir /foo
 WORKDIR /foo
 RUN apt-get update && apt-get install -y make
 COPY context/bar /context

integration/dockerfiles/Dockerfile_test_issue_workdir

@@ -6,3 +6,4 @@ FROM ubuntu
 #
 # RUN mkdir /app
 WORKDIR /app
+WORKDIR /

integration/images.go

@@ -225,8 +225,7 @@ func NewDockerFileBuilder() *DockerFileBuilder {
 		"Dockerfile_test_cache_perm":    {},
 		"Dockerfile_test_cache_copy":    {},
 		"Dockerfile_test_issue_3429":    {},
-		// TODO: WORKDIR command is uncacheable
-		//"Dockerfile_test_issue_workdir":    {},
+		"Dockerfile_test_issue_workdir": {},
 		// TODO: ADD command is uncacheable
 		//"Dockerfile_test_issue_add":    {},
 		"Dockerfile_test_issue_empty": {},

pkg/commands/commands.go

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

pkg/commands/workdir.go

@@ -17,9 +17,11 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 
+	kConfig "github.com/GoogleContainerTools/kaniko/pkg/config"
 	"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
 	"github.com/pkg/errors"
 
@@ -33,6 +35,19 @@ type WorkdirCommand struct {
 	BaseCommand
 	cmd           *instructions.WorkdirCommand
 	snapshotFiles []string
+	shdCache      bool
+}
+
+func ToAbsPath(path string, workdir string) string {
+	if filepath.IsAbs(path) {
+		return path
+	} else {
+		if workdir != "" {
+			return filepath.Join(workdir, path)
+		} else {
+			return filepath.Join("/", path)
+		}
+	}
 }
 
 // For testing
@@ -46,15 +61,7 @@ func (w *WorkdirCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile
 	if err != nil {
 		return err
 	}
-	if filepath.IsAbs(resolvedWorkingDir) {
-		config.WorkingDir = resolvedWorkingDir
-	} else {
-		if config.WorkingDir != "" {
-			config.WorkingDir = filepath.Join(config.WorkingDir, resolvedWorkingDir)
-		} else {
-			config.WorkingDir = filepath.Join("/", resolvedWorkingDir)
-		}
-	}
+	config.WorkingDir = ToAbsPath(resolvedWorkingDir, config.WorkingDir)
 	logrus.Infof("Changed working directory to %s", config.WorkingDir)
 
 	// Only create and snapshot the dir if it didn't exist already
@@ -89,6 +96,99 @@ func (w *WorkdirCommand) String() string {
 	return w.cmd.String()
 }
 
+// CacheCommand returns true since this command should be cached
+func (w *WorkdirCommand) CacheCommand(img v1.Image) DockerCommand {
+
+	return &CachingWorkdirCommand{
+		img:       img,
+		cmd:       w.cmd,
+		extractFn: util.ExtractFile,
+	}
+}
+
 func (w *WorkdirCommand) MetadataOnly() bool {
 	return false
 }
+
+func (r *WorkdirCommand) RequiresUnpackedFS() bool {
+	return true
+}
+
+func (w *WorkdirCommand) ShouldCacheOutput() bool {
+	return w.shdCache
+}
+
+type CachingWorkdirCommand struct {
+	BaseCommand
+	caching
+	img            v1.Image
+	extractedFiles []string
+	cmd            *instructions.WorkdirCommand
+	extractFn      util.ExtractFunction
+}
+
+func (wr *CachingWorkdirCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	var err error
+	logrus.Info("Cmd: workdir")
+	workdirPath := wr.cmd.Path
+	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
+	resolvedWorkingDir, err := util.ResolveEnvironmentReplacement(workdirPath, replacementEnvs, true)
+	if err != nil {
+		return err
+	}
+	config.WorkingDir = ToAbsPath(resolvedWorkingDir, config.WorkingDir)
+	logrus.Infof("Changed working directory to %s", config.WorkingDir)
+
+	logrus.Infof("Found cached layer, extracting to filesystem")
+
+	if wr.img == nil {
+		return errors.New(fmt.Sprintf("command image is nil %v", wr.String()))
+	}
+
+	layers, err := wr.img.Layers()
+	if err != nil {
+		return errors.Wrap(err, "retrieving image layers")
+	}
+
+	if len(layers) > 1 {
+		return errors.New(fmt.Sprintf("expected %d layers but got %d", 1, len(layers)))
+	} else if len(layers) == 0 {
+		// an empty image in cache indicates that no directory was created by WORKDIR
+		return nil
+	}
+
+	wr.layer = layers[0]
+
+	wr.extractedFiles, err = util.GetFSFromLayers(
+		kConfig.RootDir,
+		layers,
+		util.ExtractFunc(wr.extractFn),
+		util.IncludeWhiteout(),
+	)
+	if err != nil {
+		return errors.Wrap(err, "extracting fs from image")
+	}
+
+	return nil
+}
+
+// FilesToSnapshot returns the workingdir, which should have been created if it didn't already exist
+func (wr *CachingWorkdirCommand) FilesToSnapshot() []string {
+	f := wr.extractedFiles
+	logrus.Debugf("%d files extracted by caching run command", len(f))
+	logrus.Tracef("Extracted files: %s", f)
+
+	return f
+}
+
+// String returns some information about the command for the image config history
+func (wr *CachingWorkdirCommand) String() string {
+	if wr.cmd == nil {
+		return "nil command"
+	}
+	return wr.cmd.String()
+}
+
+func (wr *CachingWorkdirCommand) MetadataOnly() bool {
+	return false
+}

pkg/executor/build.go

@@ -414,11 +414,13 @@ func (s *stageBuilder) build() error {
 			continue
 		}
 		if isCacheCommand {
-			if files != nil && len(files) == 0 {
+			v := command.(commands.Cached)
+			layer := v.Layer()
+			if (files != nil || layer == nil) && len(files) == 0 {
+				// a cache image with a layer with no files indicates that no files were changed, ie. by 'RUN echo hello'
+				// a cache image without a layer indicates that no files were changed too, ie. by 'WORKDIR /'
 				logrus.Info("No files were changed, appending empty layer to config. No layer added to image.")
 			} else {
-				v := command.(commands.Cached)
-				layer := v.Layer()
 				if err := s.saveLayerToImage(layer, command.String()); err != nil {
 					return errors.Wrap(err, "failed to save layer")
 				}
@@ -440,10 +442,6 @@ func (s *stageBuilder) build() error {
 
 				// Raise Warnings for commands that are uncacheable
 				switch command.(type) {
-				case *commands.WorkdirCommand:
-					if len(files) > 0 {
-						logrus.Warn("cache-violation: WORKDIR implicitly created a folder that can't be cached - consider creating it explicitly with RUN instead. https://github.com/GoogleContainerTools/kaniko/issues/3340")
-					}
 				case *commands.AddCommand:
 					logrus.Warn("cache-violation: ADD can't be cached - consider using COPY instead.")
 				}

pkg/executor/push.go

@@ -369,11 +369,6 @@ func pushLayerToCache(opts *config.KanikoOptions, cacheKey string, tarPath strin
 		// layer already gzipped by default
 	}
 
-	layer, err := tarball.LayerFromFile(tarPath, layerOpts...)
-	if err != nil {
-		return err
-	}
-
 	cache, err := cache.Destination(opts, cacheKey)
 	if err != nil {
 		return errors.Wrap(err, "getting cache destination")
@@ -385,18 +380,27 @@ func pushLayerToCache(opts *config.KanikoOptions, cacheKey string, tarPath strin
 		return errors.Wrap(err, "setting empty image created time")
 	}
 
-	empty, err = mutate.Append(empty,
-		mutate.Addendum{
-			Layer: layer,
-			History: v1.History{
-				Author:    constants.Author,
-				CreatedBy: createdBy,
+	// WORKDIR can create empty layers by design, yet still we must cache them
+	// to transfer the knowledge that they are empty.
+	if tarPath != "" {
+		layer, err := tarball.LayerFromFile(tarPath, layerOpts...)
+		if err != nil {
+			return err
+		}
+		empty, err = mutate.Append(empty,
+			mutate.Addendum{
+				Layer: layer,
+				History: v1.History{
+					Author:    constants.Author,
+					CreatedBy: createdBy,
+				},
 			},
-		},
-	)
-	if err != nil {
-		return errors.Wrap(err, "appending layer onto empty image")
+		)
+		if err != nil {
+			return errors.Wrap(err, "appending layer onto empty image")
+		}
 	}
+
 	cacheOpts := *opts
 	cacheOpts.TarPath = ""              // tarPath doesn't make sense for Docker layers
 	cacheOpts.NoPush = opts.NoPushCache // we do not want to push cache if --no-push-cache is set.