diff --git a/pkg/commands/base_command.go b/pkg/commands/base_command.go index f42e68e02..fe6fa1805 100644 --- a/pkg/commands/base_command.go +++ b/pkg/commands/base_command.go @@ -24,6 +24,10 @@ import ( type BaseCommand struct { } +func (b *BaseCommand) IsArgsEnvsRequiredInCache() bool { + return false +} + func (b *BaseCommand) CacheCommand(v1.Image) DockerCommand { return nil } diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 416fc64ab..ae1510510 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -58,6 +58,10 @@ type DockerCommand interface { // ShouldDetectDeletedFiles returns true if the command could delete files. ShouldDetectDeletedFiles() bool + + // True if need add ARGs and EVNs to composite cache string with resolved command + // need only for RUN instruction + IsArgsEnvsRequiredInCache() bool } func GetCommand(cmd instructions.Command, fileContext util.FileContext, useNewRun bool, cacheCopy bool, cacheRun bool) (DockerCommand, error) { diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index c85b0f89e..f5736bcab 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -270,9 +270,8 @@ func copyCmdFilesUsedFromContext( config *v1.Config, buildArgs *dockerfile.BuildArgs, cmd *instructions.CopyCommand, fileContext util.FileContext, ) ([]string, error) { - // We don't use the context if we're performing a copy --from. if cmd.From != "" { - return nil, nil + fileContext = util.FileContext{Root: filepath.Join(kConfig.KanikoDir, cmd.From)} } replacementEnvs := buildArgs.ReplacementEnvs(config.Env) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index f2f372b3d..f9acc4029 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -44,6 +44,10 @@ var ( userLookup = util.LookupUser ) +func (r *RunCommand) IsArgsEnvsRequiredInCache() bool { + return true +} + func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { return runCommandInExec(config, buildArgs, r.cmd) } @@ -200,6 +204,10 @@ type CachingRunCommand struct { extractFn util.ExtractFunction } +func (cr *CachingRunCommand) IsArgsEnvsRequiredInCache() bool { + return true +} + func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { logrus.Infof("Found cached layer, extracting to filesystem") var err error diff --git a/pkg/commands/run_marker.go b/pkg/commands/run_marker.go index ab33271c0..1e37a58ae 100644 --- a/pkg/commands/run_marker.go +++ b/pkg/commands/run_marker.go @@ -59,6 +59,10 @@ func (r *RunMarkerCommand) ProvidesFilesToSnapshot() bool { return true } +func (r *RunMarkerCommand) IsArgsEnvsRequiredInCache() bool { + return true +} + // CacheCommand returns true since this command should be cached func (r *RunMarkerCommand) CacheCommand(img v1.Image) DockerCommand { diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 6b317d344..a21fbe438 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -58,10 +58,6 @@ func ParseStages(opts *config.KanikoOptions) ([]instructions.Stage, []instructio return nil, nil, errors.Wrap(err, "parsing dockerfile") } - if opts.CacheCopyLayers && len(stages) >= 2 { - return nil, nil, errors.New("kaniko does not support caching copy layers in multistage builds") - } - metaArgs, err = expandNested(metaArgs, opts.BuildArgs) if err != nil { return nil, nil, errors.Wrap(err, "expanding meta ARGs") diff --git a/pkg/dockerfile/dockerfile_test.go b/pkg/dockerfile/dockerfile_test.go index 036f8bb9b..0588636f1 100644 --- a/pkg/dockerfile/dockerfile_test.go +++ b/pkg/dockerfile/dockerfile_test.go @@ -29,39 +29,6 @@ import ( "github.com/moby/buildkit/frontend/dockerfile/instructions" ) -func Test_ParseStages_NoMultistageWithCacheCopy(t *testing.T) { - dockerfile := ` - FROM scratch as first - COPY testfile / - - FROM scratch as second - COPY --from=second testfile / - ` - tmpfile, err := ioutil.TempFile("", "Dockerfile.test") - if err != nil { - t.Fatal(err) - } - - defer os.Remove(tmpfile.Name()) - - if _, err := tmpfile.Write([]byte(dockerfile)); err != nil { - t.Fatal(err) - } - if err := tmpfile.Close(); err != nil { - t.Fatal(err) - } - - opts := &config.KanikoOptions{ - DockerfilePath: tmpfile.Name(), - CacheCopyLayers: true, - } - - _, _, err = ParseStages(opts) - if err == nil { - t.Fatal("expected ParseStages to fail on MultiStage build if CacheCopyLayers=true") - } -} - func Test_ParseStages_ArgValueWithQuotes(t *testing.T) { dockerfile := ` ARG IMAGE="ubuntu:16.04" diff --git a/pkg/executor/build.go b/pkg/executor/build.go index b65f301bf..9957079ce 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -198,7 +198,7 @@ func isOCILayout(path string) bool { return strings.HasPrefix(path, "oci:") } -func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string, compositeKey CompositeCache, args *dockerfile.BuildArgs, env []string) (CompositeCache, error) { +func (s *stageBuilder) populateCompositeKey(command commands.DockerCommand, files []string, compositeKey CompositeCache, args *dockerfile.BuildArgs, env []string) (CompositeCache, error) { // First replace all the environment variables or args in the command replacementEnvs := args.ReplacementEnvs(env) // The sort order of `replacementEnvs` is basically undefined, sort it @@ -212,15 +212,16 @@ func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string // avoid conflicts with any RUN command since commands can not // start with | (vertical bar). The "#" (number of build envs) is there to // help ensure proper cache matches. - if len(replacementEnvs) > 0 { - compositeKey.AddKey(fmt.Sprintf("|%d", len(replacementEnvs))) - compositeKey.AddKey(replacementEnvs...) + + if command.IsArgsEnvsRequiredInCache() { + if len(replacementEnvs) > 0 { + compositeKey.AddKey(fmt.Sprintf("|%d", len(replacementEnvs))) + compositeKey.AddKey(replacementEnvs...) + } } + // Add the next command to the cache key. compositeKey.AddKey(resolvedCmd) - if copyCmd, ok := commands.CastAbstractCopyCommand(command); ok == true { - compositeKey = s.populateCopyCmdCompositeKey(command, copyCmd.From(), compositeKey) - } for _, f := range files { if err := compositeKey.AddPath(f, s.fileContext); err != nil { @@ -230,22 +231,6 @@ func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string return compositeKey, nil } -func (s *stageBuilder) populateCopyCmdCompositeKey(command fmt.Stringer, from string, compositeKey CompositeCache) CompositeCache { - if from != "" { - digest, ok := s.stageIdxToDigest[from] - if ok { - ds := digest - cacheKey, ok := s.digestToCacheKey[ds] - if ok { - logrus.Debugf("Adding digest %v from previous stage to composite key for %v", ds, command.String()) - compositeKey.AddKey(cacheKey) - } - } - } - - return compositeKey -} - func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) error { if !s.opts.Cache { return nil diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 603969f20..a413379b6 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -654,14 +654,15 @@ func newStageContext(command string, args map[string]string, env []string) stage } func Test_stageBuilder_populateCompositeKey(t *testing.T) { - testCases := []struct { + type testcase struct { description string cmd1 stageContext cmd2 stageContext shdEqual bool - }{ + } + testCases := []testcase{ { - description: "cache key for same command with same build args", + description: "cache key for same command [RUN] with same build args", cmd1: newStageContext( "RUN echo $ARG > test", map[string]string{"ARG": "foo"}, @@ -675,7 +676,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { shdEqual: true, }, { - description: "cache key for same command with same env and args", + description: "cache key for same command [RUN] with same env and args", cmd1: newStageContext( "RUN echo $ENV > test", map[string]string{"ARG": "foo"}, @@ -689,7 +690,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { shdEqual: true, }, { - description: "cache key for same command with same env but different args", + description: "cache key for same command [RUN] with same env but different args", cmd1: newStageContext( "RUN echo $ENV > test", map[string]string{"ARG": "foo"}, @@ -702,7 +703,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { ), }, { - description: "cache key for same command, different buildargs, args not used in command", + description: "cache key for same command [RUN], different buildargs, args not used in command", cmd1: newStageContext( "RUN echo const > test", map[string]string{"ARG": "foo"}, @@ -715,7 +716,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { ), }, { - description: "cache key for same command, different buildargs, args used in script", + description: "cache key for same command [RUN], different buildargs, args used in script", // test.sh // #!/bin/sh // echo ${ARG} @@ -731,7 +732,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { ), }, { - description: "cache key for same command with a build arg values", + description: "cache key for same command [RUN] with a build arg values", cmd1: newStageContext( "RUN echo $ARG > test", map[string]string{"ARG": "foo"}, @@ -744,7 +745,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { ), }, { - description: "cache key for same command with different env values", + description: "cache key for same command [RUN] with different env values", cmd1: newStageContext( "RUN echo $ENV > test", map[string]string{"ARG": "foo"}, @@ -757,7 +758,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { ), }, { - description: "cache key for different command same context", + description: "cache key for different command [RUN] same context", cmd1: newStageContext( "RUN echo other > test", map[string]string{"ARG": "foo"}, @@ -769,17 +770,105 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { []string{"ENV=1"}, ), }, + func() testcase { + dir, files := tempDirAndFile(t) + file := files[0] + filePath := filepath.Join(dir, file) + return testcase{ + description: "cache key for same command [COPY] with same args", + cmd1: newStageContext( + fmt.Sprintf("COPY %s /meow", filePath), + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + cmd2: newStageContext( + fmt.Sprintf("COPY %s /meow", filePath), + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + shdEqual: true, + } + }(), + func() testcase { + dir, files := tempDirAndFile(t) + file := files[0] + filePath := filepath.Join(dir, file) + return testcase{ + description: "cache key for same command [COPY] with different args", + cmd1: newStageContext( + fmt.Sprintf("COPY %s /meow", filePath), + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + cmd2: newStageContext( + fmt.Sprintf("COPY %s /meow", filePath), + map[string]string{"ARG": "bar"}, + []string{"ENV=2"}, + ), + shdEqual: true, + } + }(), + { + description: "cache key for same command [WORKDIR] with same args", + cmd1: newStageContext( + "WORKDIR /", + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + cmd2: newStageContext( + "WORKDIR /", + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + shdEqual: true, + }, + { + description: "cache key for same command [WORKDIR] with different args", + cmd1: newStageContext( + "WORKDIR /", + map[string]string{"ARG": "foo"}, + []string{"ENV=1"}, + ), + cmd2: newStageContext( + "WORKDIR /", + map[string]string{"ARG": "bar"}, + []string{"ENV=2"}, + ), + shdEqual: true, + }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { sb := &stageBuilder{fileContext: util.FileContext{Root: "workspace"}} ck := CompositeCache{} - ck1, err := sb.populateCompositeKey(tc.cmd1.command, []string{}, ck, tc.cmd1.args, tc.cmd1.env) + instructions1, err := dockerfile.ParseCommands([]string{tc.cmd1.command.String()}) + if err != nil { + t.Fatal(err) + } + + fc1 := util.FileContext{Root: "workspace"} + dockerCommand1, err := commands.GetCommand(instructions1[0], fc1, false, true, true) + if err != nil { + t.Fatal(err) + } + + instructions, err := dockerfile.ParseCommands([]string{tc.cmd2.command.String()}) + if err != nil { + t.Fatal(err) + } + + fc2 := util.FileContext{Root: "workspace"} + dockerCommand2, err := commands.GetCommand(instructions[0], fc2, false, true, true) + if err != nil { + t.Fatal(err) + } + + ck1, err := sb.populateCompositeKey(dockerCommand1, []string{}, ck, tc.cmd1.args, tc.cmd1.env) if err != nil { t.Errorf("Expected error to be nil but was %v", err) } - ck2, err := sb.populateCompositeKey(tc.cmd2.command, []string{}, ck, tc.cmd2.args, tc.cmd2.env) + ck2, err := sb.populateCompositeKey(dockerCommand2, []string{}, ck, tc.cmd2.args, tc.cmd2.env) if err != nil { t.Errorf("Expected error to be nil but was %v", err) } @@ -1206,6 +1295,7 @@ RUN foobar cacheCommand: MockCachedDockerCommand{ contextFiles: []string{}, }, + argToCompositeCache: true, } return testcase{ @@ -1243,6 +1333,7 @@ RUN foobar cacheCommand: MockCachedDockerCommand{ contextFiles: []string{}, }, + argToCompositeCache: true, } return testcase{ @@ -1286,6 +1377,7 @@ RUN foobar cacheCommand: MockCachedDockerCommand{ contextFiles: []string{}, }, + argToCompositeCache: true, } return testcase{ @@ -1500,9 +1592,10 @@ func Test_stageBuild_populateCompositeKeyForCopyCommand(t *testing.T) { expectedCacheKey string }{ { - description: "multi-stage copy command", + description: "multi-stage copy command", + // dont use digest from previoust stage for COPY command: "COPY --from=0 foo.txt bar.txt", - expectedCacheKey: "COPY --from=0 foo.txt bar.txt-some-cache-key", + expectedCacheKey: "COPY --from=0 foo.txt bar.txt", }, { description: "copy command", @@ -1524,7 +1617,7 @@ func Test_stageBuild_populateCompositeKeyForCopyCommand(t *testing.T) { for _, useCacheCommand := range []bool{false, true} { t.Run(fmt.Sprintf("CacheCommand=%t", useCacheCommand), func(t *testing.T) { - var cmd fmt.Stringer = copyCommand + var cmd commands.DockerCommand = copyCommand if useCacheCommand { cmd = copyCommand.(*commands.CopyCommand).CacheCommand(nil) } diff --git a/pkg/executor/fakes.go b/pkg/executor/fakes.go index 4b8dec1c6..7bb7b383d 100644 --- a/pkg/executor/fakes.go +++ b/pkg/executor/fakes.go @@ -43,9 +43,10 @@ func (f fakeSnapShotter) TakeSnapshot(_ []string, _, _ bool) (string, error) { } type MockDockerCommand struct { - command string - contextFiles []string - cacheCommand commands.DockerCommand + command string + contextFiles []string + cacheCommand commands.DockerCommand + argToCompositeCache bool } func (m MockDockerCommand) ExecuteCommand(c *v1.Config, args *dockerfile.BuildArgs) error { return nil } @@ -76,9 +77,13 @@ func (m MockDockerCommand) ShouldCacheOutput() bool { func (m MockDockerCommand) ShouldDetectDeletedFiles() bool { return false } +func (m MockDockerCommand) IsArgsEnvsRequiredInCache() bool { + return m.argToCompositeCache +} type MockCachedDockerCommand struct { - contextFiles []string + contextFiles []string + argToCompositeCache bool } func (m MockCachedDockerCommand) ExecuteCommand(c *v1.Config, args *dockerfile.BuildArgs) error { @@ -111,6 +116,9 @@ func (m MockCachedDockerCommand) RequiresUnpackedFS() bool { func (m MockCachedDockerCommand) ShouldCacheOutput() bool { return false } +func (m MockCachedDockerCommand) IsArgsEnvsRequiredInCache() bool { + return m.argToCompositeCache +} type fakeLayerCache struct { retrieve bool