From 6a911622f2324cb04bbd7034a6c4182522567831 Mon Sep 17 00:00:00 2001 From: dlorenc Date: Thu, 8 Nov 2018 11:14:12 -0800 Subject: [PATCH] Fix caching for multi-step builds. 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 8a6a9a58ec..ecc80cd8aa 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 baz RUN apt-get update && apt-get install -y make +COPY context/bar /context +RUN apt-get update && apt-get install -y vim diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 27cf0c968a..7bcbce810d 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 90ee3e4451..698b72821b 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 9ca6e97ae3..d6ed04b591 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 d70d98483b..1e8f196b8e 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 0290e6f7c2..72772922bb 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 12a2f44d8e..abc1f9c065 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 35a37d70ae..62ad15629b 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