diff --git a/integration/dockerfiles/Dockerfile_test_cache_install b/integration/dockerfiles/Dockerfile_test_cache_install index 5a79fe1b4..ae2feb195 100644 --- a/integration/dockerfiles/Dockerfile_test_cache_install +++ b/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 diff --git a/integration/dockerfiles/Dockerfile_test_cache_install_oci b/integration/dockerfiles/Dockerfile_test_cache_install_oci index 5a79fe1b4..ae2feb195 100644 --- a/integration/dockerfiles/Dockerfile_test_cache_install_oci +++ b/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 diff --git a/integration/dockerfiles/Dockerfile_test_issue_workdir b/integration/dockerfiles/Dockerfile_test_issue_workdir index 536942c39..d78f32feb 100644 --- a/integration/dockerfiles/Dockerfile_test_issue_workdir +++ b/integration/dockerfiles/Dockerfile_test_issue_workdir @@ -6,3 +6,4 @@ FROM ubuntu # # RUN mkdir /app WORKDIR /app +WORKDIR / \ No newline at end of file diff --git a/integration/images.go b/integration/images.go index b606132d7..43898dbd5 100644 --- a/integration/images.go +++ b/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": {}, diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index ae1510510..65b2f1f5e 100644 --- a/pkg/commands/commands.go +++ b/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: diff --git a/pkg/commands/workdir.go b/pkg/commands/workdir.go index eefa978b8..4e93d6f70 100644 --- a/pkg/commands/workdir.go +++ b/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 +} diff --git a/pkg/executor/build.go b/pkg/executor/build.go index e693aa901..8e1743267 100644 --- a/pkg/executor/build.go +++ b/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.") } diff --git a/pkg/executor/push.go b/pkg/executor/push.go index 2580e314c..3d98b220e 100644 --- a/pkg/executor/push.go +++ b/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.