From 58b607b4d0de146319af1c4df3702e2a3b9d91b8 Mon Sep 17 00:00:00 2001 From: dlorenc Date: Fri, 9 Nov 2018 12:28:18 -0600 Subject: [PATCH] Fix caching for multi-step builds. (#441) This change fixes that by properly "replaying" the Dockerfile and mutating the config when calculating cache keys. Previously we were looking at the wrong cache key for each command when there was more than one. --- .../dockerfiles/Dockerfile_test_cache_install | 3 + pkg/commands/add.go | 4 + pkg/commands/base_command.go | 4 + pkg/commands/commands.go | 2 + pkg/commands/copy.go | 4 + pkg/commands/run.go | 4 + pkg/commands/workdir.go | 4 + pkg/executor/build.go | 88 +++++++++++++------ 8 files changed, 87 insertions(+), 26 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_cache_install b/integration/dockerfiles/Dockerfile_test_cache_install index 8a6a9a58e..b93eb4ad5 100644 --- a/integration/dockerfiles/Dockerfile_test_cache_install +++ b/integration/dockerfiles/Dockerfile_test_cache_install @@ -17,4 +17,7 @@ # if the cache is implemented correctly FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0 +WORKDIR /foo RUN apt-get update && apt-get install -y make +COPY context/bar /context +RUN echo "hey" > foo diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 27cf0c968..7bcbce810 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -131,3 +131,7 @@ func (a *AddCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfi logrus.Infof("Using files from context: %v", files) return files, nil } + +func (a *AddCommand) MetadataOnly() bool { + return false +} diff --git a/pkg/commands/base_command.go b/pkg/commands/base_command.go index 90ee3e445..698b72821 100644 --- a/pkg/commands/base_command.go +++ b/pkg/commands/base_command.go @@ -35,3 +35,7 @@ func (b *BaseCommand) FilesToSnapshot() []string { func (b *BaseCommand) FilesUsedFromContext(_ *v1.Config, _ *dockerfile.BuildArgs) ([]string, error) { return []string{}, nil } + +func (b *BaseCommand) MetadataOnly() bool { + return true +} diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 9ca6e97ae..d6ed04b59 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -41,6 +41,8 @@ type DockerCommand interface { // Return true if this command depends on the build context. FilesUsedFromContext(*v1.Config, *dockerfile.BuildArgs) ([]string, error) + + MetadataOnly() bool } func GetCommand(cmd instructions.Command, buildcontext string) (DockerCommand, error) { diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index d70d98483..1e8f196b8 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -134,3 +134,7 @@ func (c *CopyCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerf logrus.Infof("Using files from context: %v", files) return files, nil } + +func (c *CopyCommand) MetadataOnly() bool { + return false +} diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 0290e6f7c..72772922b 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -161,6 +161,10 @@ func (r *RunCommand) CacheCommand(img v1.Image) DockerCommand { } } +func (r *RunCommand) MetadataOnly() bool { + return false +} + type CachingRunCommand struct { BaseCommand img v1.Image diff --git a/pkg/commands/workdir.go b/pkg/commands/workdir.go index 12a2f44d8..abc1f9c06 100644 --- a/pkg/commands/workdir.go +++ b/pkg/commands/workdir.go @@ -67,3 +67,7 @@ func (w *WorkdirCommand) FilesToSnapshot() []string { func (w *WorkdirCommand) String() string { return w.cmd.String() } + +func (w *WorkdirCommand) MetadataOnly() bool { + return false +} diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 35a37d70a..62ad15629 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -86,6 +86,63 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta }, nil } +func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config, cmds []commands.DockerCommand) error { + if !s.opts.Cache { + return nil + } + + args := dockerfile.NewBuildArgs(s.opts.BuildArgs) + args.AddMetaArgs(s.stage.MetaArgs) + + layerCache := &cache.RegistryCache{ + Opts: s.opts, + } + + // Possibly replace commands with their cached implementations. + // We walk through all the commands, running any commands that only operate on metadata. + // We throw the metadata away after, but we need it to properly track command dependencies + // for things like COPY ${FOO} or RUN commands that use environment variables. + for i, command := range cmds { + if command == nil { + continue + } + compositeKey.AddKey(command.String()) + // If the command uses files from the context, add them. + files, err := command.FilesUsedFromContext(&cfg, args) + if err != nil { + return err + } + for _, f := range files { + if err := compositeKey.AddPath(f); err != nil { + return err + } + } + + ck, err := compositeKey.Hash() + if err != nil { + return err + } + img, err := layerCache.RetrieveLayer(ck) + if err != nil { + logrus.Infof("No cached layer found for cmd %s", command.String()) + break + } + + if cacheCmd := command.CacheCommand(img); cacheCmd != nil { + logrus.Infof("Using caching version of cmd: %s", command.String()) + cmds[i] = cacheCmd + } + + // Mutate the config for any commands that require it. + if command.MetadataOnly() { + if err := command.ExecuteCommand(&cfg, args); err != nil { + return err + } + } + } + return nil +} + func (s *stageBuilder) build() error { // Unpack file system to root if _, err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { @@ -109,34 +166,13 @@ func (s *stageBuilder) build() error { cmds = append(cmds, command) } - layerCache := &cache.RegistryCache{ - Opts: s.opts, - } - if s.opts.Cache { - // Possibly replace commands with their cached implementations. - for i, command := range cmds { - if command == nil { - continue - } - ck, err := compositeKey.Hash() - if err != nil { - return err - } - img, err := layerCache.RetrieveLayer(ck) - if err != nil { - logrus.Infof("No cached layer found for cmd %s", command.String()) - break - } - - if cacheCmd := command.CacheCommand(img); cacheCmd != nil { - logrus.Infof("Using caching version of cmd: %s", command.String()) - cmds[i] = cacheCmd - } - } - } - args := dockerfile.NewBuildArgs(s.opts.BuildArgs) args.AddMetaArgs(s.stage.MetaArgs) + + // Apply optimizations to the instructions. + if err := s.optimize(*compositeKey, s.cf.Config, cmds); err != nil { + return err + } for index, command := range cmds { if command == nil { continue