remove build args from composite key and replace all build args before adding command to the cache

This commit is contained in:
Tejal Desai 2020-02-28 01:51:31 -08:00
parent 7aa737c297
commit 2ba7d25090
3 changed files with 152 additions and 11 deletions

View File

@ -72,7 +72,7 @@ type stageBuilder struct {
stageIdxToDigest map[string]string
snapshotter snapShotter
layerCache cache.LayerCache
pushCache cachePusher
pushLayerToCache cachePusher
}
// newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage
@ -115,7 +115,7 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, cross
layerCache: &cache.RegistryCache{
Opts: opts,
},
pushCache: pushLayerToCache,
pushLayerToCache: pushLayerToCache,
}
for _, cmd := range s.stage.Commands {
@ -146,9 +146,15 @@ func initializeConfig(img partial.WithConfigFile) (*v1.ConfigFile, error) {
return imageConfig, nil
}
func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string, compositeKey CompositeCache) (CompositeCache, error) {
func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, 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)
resolvedCmd, err := util.ResolveEnvironmentReplacement(command.String(), replacementEnvs, false)
if err != nil {
return compositeKey, err
}
// Add the next command to the cache key.
compositeKey.AddKey(command.String())
compositeKey.AddKey(resolvedCmd)
switch v := command.(type) {
case *commands.CopyCommand:
compositeKey = s.populateCopyCmdCompositeKey(command, v.From(), compositeKey)
@ -201,7 +207,7 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro
return errors.Wrap(err, "failed to get files used from context")
}
compositeKey, err = s.populateCompositeKey(command, files, compositeKey)
compositeKey, err = s.populateCompositeKey(command, files, compositeKey, s.args, cfg.Env)
if err != nil {
return err
}
@ -251,8 +257,6 @@ func (s *stageBuilder) build() error {
compositeKey = NewCompositeCache(s.baseImageDigest)
}
compositeKey.AddKey(s.opts.BuildArgs...)
// Apply optimizations to the instructions.
if err := s.optimize(*compositeKey, s.cf.Config); err != nil {
return errors.Wrap(err, "failed to optimize instructions")
@ -309,7 +313,7 @@ func (s *stageBuilder) build() error {
return errors.Wrap(err, "failed to get files used from context")
}
*compositeKey, err = s.populateCompositeKey(command, files, *compositeKey)
*compositeKey, err = s.populateCompositeKey(command, files, *compositeKey, s.args, s.cf.Config.Env)
if err != nil {
return err
}
@ -358,7 +362,7 @@ func (s *stageBuilder) build() error {
// Push layer to cache (in parallel) now along with new config file
if s.opts.Cache && command.ShouldCacheOutput() {
cacheGroup.Go(func() error {
return s.pushCache(s.opts, ck, tarPath, command.String())
return s.pushLayerToCache(s.opts, ck, tarPath, command.String())
})
}
if err := s.saveSnapshotToImage(command.String(), tarPath); err != nil {

View File

@ -517,6 +517,130 @@ func Test_stageBuilder_optimize(t *testing.T) {
}
}
type stageContext struct {
command fmt.Stringer
args *dockerfile.BuildArgs
env []string
}
func newStageContext(command string, args map[string]string, env []string) stageContext {
dockerArgs := dockerfile.NewBuildArgs([]string{})
for k, v := range args {
dockerArgs.AddArg(k, &v)
}
return stageContext{MockDockerCommand{command: command}, dockerArgs, env}
}
func Test_stageBuilder_populateCompositeKey(t *testing.T) {
testCases := []struct {
description string
cmd1 stageContext
cmd2 stageContext
shdEqual bool
}{
{
description: "cache key for same command, different buildargs, args not used in command",
cmd1: newStageContext(
"RUN echo const > test",
map[string]string{"ARG": "foo"},
[]string{"ENV=foo1"},
),
cmd2: newStageContext(
"RUN echo const > test",
map[string]string{"ARG": "bar"},
[]string{"ENV=bar1"},
),
shdEqual: true,
},
{
description: "cache key for same command with same build args",
cmd1: newStageContext(
"RUN echo $ARG > test",
map[string]string{"ARG": "foo"},
[]string{},
),
cmd2: newStageContext(
"RUN echo $ARG > test",
map[string]string{"ARG": "foo"},
[]string{},
),
shdEqual: true,
},
{
description: "cache key for same command with same env",
cmd1: newStageContext(
"RUN echo $ENV > test",
map[string]string{"ARG": "foo"},
[]string{"ENV=same"},
),
cmd2: newStageContext(
"RUN echo $ENV > test",
map[string]string{"ARG": "bar"},
[]string{"ENV=same"},
),
shdEqual: true,
},
{
description: "cache key for same command with a build arg values",
cmd1: newStageContext(
"RUN echo $ARG > test",
map[string]string{"ARG": "foo"},
[]string{},
),
cmd2: newStageContext(
"RUN echo $ARG > test",
map[string]string{"ARG": "bar"},
[]string{},
),
},
{
description: "cache key for same command with different env values",
cmd1: newStageContext(
"RUN echo $ENV > test",
map[string]string{"ARG": "foo"},
[]string{"ENV=1"},
),
cmd2: newStageContext(
"RUN echo $ENV > test",
map[string]string{"ARG": "foo"},
[]string{"ENV=2"},
),
},
{
description: "cache key for different command same context",
cmd1: newStageContext(
"RUN echo other > test",
map[string]string{"ARG": "foo"},
[]string{"ENV=1"},
),
cmd2: newStageContext(
"RUN echo another > test",
map[string]string{"ARG": "foo"},
[]string{"ENV=1"},
),
},
}
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
sb := &stageBuilder{opts: &config.KanikoOptions{SrcContext: "workspace"}}
ck := CompositeCache{}
ck1, err := sb.populateCompositeKey(tc.cmd1.command, []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)
if err != nil {
t.Errorf("Expected error to be nil but was %v", err)
}
key1, key2 := hashCompositeKeys(t, ck1, ck2)
if b := key1 == key2; b != tc.shdEqual {
t.Errorf("expected keys to be equal as %t but found %t", tc.shdEqual, !tc.shdEqual)
}
})
}
}
func Test_stageBuilder_build(t *testing.T) {
type testcase struct {
description string
@ -881,7 +1005,7 @@ COPY %s bar.txt
cf: cf,
snapshotter: snap,
layerCache: lc,
pushCache: func(_ *config.KanikoOptions, cacheKey, _, _ string) error {
pushLayerToCache: func(_ *config.KanikoOptions, cacheKey, _, _ string) error {
keys = append(keys, cacheKey)
return nil
},
@ -993,3 +1117,15 @@ func generateTar(t *testing.T, dir string, fileNames ...string) []byte {
}
return buf.Bytes()
}
func hashCompositeKeys(t *testing.T, ck1 CompositeCache, ck2 CompositeCache) (string, string) {
key1, err := ck1.Hash()
if err != nil {
t.Errorf("could not hash composite key due to %s", err)
}
key2, err := ck2.Hash()
if err != nil {
t.Errorf("could not hash composite key due to %s", err)
}
return key1, key2
}

View File

@ -43,13 +43,14 @@ func (f fakeSnapShotter) TakeSnapshot(_ []string) (string, error) {
}
type MockDockerCommand struct {
command string
contextFiles []string
cacheCommand commands.DockerCommand
}
func (m MockDockerCommand) ExecuteCommand(c *v1.Config, args *dockerfile.BuildArgs) error { return nil }
func (m MockDockerCommand) String() string {
return "meow"
return m.command
}
func (m MockDockerCommand) FilesToSnapshot() []string {
return []string{"meow-snapshot-no-cache"}