public_git
/
kaniko
mirror of https://github.com/GoogleContainerTools/kaniko
1
0
Fork 0
Code Issues Releases Wiki Activity

Merge pull request #914 from cvgw/u/cvgw/fix-copy-command-cache-keys 

Fix #899 cached copy results in inconsistent key
This commit is contained in:
Tejal Desai 2019-12-16 11:45:06 -08:00 committed by GitHub
parent d72a960600 9e9b8a6e71
commit a675098b45
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 141 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
pkg/commands/copy.go
View File

@ -121,24 +121,7 @@ func (c *CopyCommand) String() string {
}
func (c *CopyCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs) ([]string, error) {
// We don't use the context if we're performing a copy --from.
if c.cmd.From != "" {
return nil, nil
}
replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
srcs, _, err := util.ResolveEnvAndWildcards(c.cmd.SourcesAndDest, c.buildcontext, replacementEnvs)
if err != nil {
return nil, err
}
files := []string{}
for _, src := range srcs {
fullPath := filepath.Join(c.buildcontext, src)
files = append(files, fullPath)
}
logrus.Infof("Using files from context: %v", files)
return files, nil
return copyCmdFilesUsedFromContext(config, buildArgs, c.cmd, c.buildcontext)
}
func (c *CopyCommand) MetadataOnly() bool {
@ -157,9 +140,10 @@ func (c *CopyCommand) ShouldCacheOutput() bool {
func (c *CopyCommand) CacheCommand(img v1.Image) DockerCommand {
return &CachingCopyCommand{
img: img,
cmd: c.cmd,
extractFn: util.ExtractFile,
img: img,
cmd: c.cmd,
buildcontext: c.buildcontext,
extractFn: util.ExtractFile,
}
}
@ -172,6 +156,7 @@ type CachingCopyCommand struct {
img v1.Image
extractedFiles []string
cmd *instructions.CopyCommand
buildcontext string
extractFn util.ExtractFunction
}
@ -192,6 +177,10 @@ func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *docke
return nil
}
func (cr *CachingCopyCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs) ([]string, error) {
return copyCmdFilesUsedFromContext(config, buildArgs, cr.cmd, cr.buildcontext)
}
func (cr *CachingCopyCommand) FilesToSnapshot() []string {
return cr.extractedFiles
}
@ -224,3 +213,32 @@ func resolveIfSymlink(destPath string) (string, error) {
}
return destPath, nil
}
func copyCmdFilesUsedFromContext(
config *v1.Config, buildArgs *dockerfile.BuildArgs, cmd *instructions.CopyCommand,
buildcontext string,
) ([]string, error) {
// We don't use the context if we're performing a copy --from.
if cmd.From != "" {
return nil, nil
}
replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
srcs, _, err := util.ResolveEnvAndWildcards(
cmd.SourcesAndDest, buildcontext, replacementEnvs,
)
if err != nil {
return nil, err
}
files := []string{}
for _, src := range srcs {
fullPath := filepath.Join(buildcontext, src)
files = append(files, fullPath)
}
logrus.Debugf("Using files from context: %v", files)
return files, nil
}

20
pkg/commands/copy_test.go
View File

@ -218,6 +218,8 @@ func Test_resolveIfSymlink(t *testing.T) {
}
func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
tempDir := setupTestTemp()
tarContent, err := prepareTarFixture([]string{"foo.txt"})
if err != nil {
t.Errorf("couldn't prepare tar fixture %v", err)
@ -238,12 +240,19 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
}
testCases := []testCase{
func() testCase {
err = ioutil.WriteFile(filepath.Join(tempDir, "foo.txt"), []byte("meow"), 0644)
if err != nil {
t.Errorf("couldn't write tempfile %v", err)
t.FailNow()
}
c := &CachingCopyCommand{
img: fakeImage{
ImageLayers: []v1.Layer{
fakeLayer{TarContent: tarContent},
},
},
buildcontext: tempDir,
cmd: &instructions.CopyCommand{
SourcesAndDest: []string{
"foo.txt", "foo.txt",
@ -339,17 +348,16 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
t.Errorf("Expected extracted files to include %v but did not %v", file, cFiles)
}
}
// CachingCopyCommand does not override BaseCommand
// FilesUseFromContext so this will always return an empty slice and no error
// This seems like it might be a bug as it results in CopyCommands and CachingCopyCommands generating different cache keys - cvgw - 2019-11-27
cmdFiles, err := c.FilesUsedFromContext(
config, buildArgs,
)
if err != nil {
t.Errorf("failed to get files used from context from command")
t.Errorf("failed to get files used from context from command %v", err)
}
if len(cmdFiles) != 0 {
t.Errorf("expected files used from context to be empty but was not")
if len(cmdFiles) != len(tc.contextFiles) {
t.Errorf("expected files used from context to equal %v but was %v", tc.contextFiles, cmdFiles)
}
}

103
pkg/executor/build_test.go
View File

@ -36,7 +36,6 @@ import (
"github.com/google/go-containerregistry/pkg/v1/empty"
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
"github.com/sirupsen/logrus"
)
func Test_reviewConfig(t *testing.T) {
@ -620,7 +619,7 @@ func Test_stageBuilder_build(t *testing.T) {
ch := NewCompositeCache("", "")
ch.AddPath(filepath)
logrus.SetLevel(logrus.DebugLevel)
hash, err := ch.Hash()
if err != nil {
t.Errorf("couldn't create hash %v", err)
@ -664,7 +663,7 @@ func Test_stageBuilder_build(t *testing.T) {
filePath := filepath.Join(dir, filename)
ch := NewCompositeCache("", "")
ch.AddPath(filePath)
logrus.SetLevel(logrus.DebugLevel)
hash, err := ch.Hash()
if err != nil {
t.Errorf("couldn't create hash %v", err)
@ -698,22 +697,24 @@ func Test_stageBuilder_build(t *testing.T) {
dir, filenames := tempDirAndFile(t)
filename := filenames[0]
tarContent := generateTar(t, filename)
destDir, err := ioutil.TempDir("", "baz")
if err != nil {
t.Errorf("could not create temp dir %v", err)
}
filePath := filepath.Join(dir, filename)
ch := NewCompositeCache("", fmt.Sprintf("COPY %s foo.txt", filename))
ch.AddPath(filePath)
logrus.SetLevel(logrus.DebugLevel)
logrus.Infof("test composite key %v", ch)
ch := NewCompositeCache("", fmt.Sprintf("RUN foobar"))
hash1, err := ch.Hash()
if err != nil {
t.Errorf("couldn't create hash %v", err)
}
ch.AddKey(fmt.Sprintf("COPY %s bar.txt", filename))
ch.AddPath(filePath)
logrus.Infof("test composite key %v", ch)
hash2, err := ch.Hash()
if err != nil {
t.Errorf("couldn't create hash %v", err)
@ -721,11 +722,78 @@ func Test_stageBuilder_build(t *testing.T) {
ch = NewCompositeCache("", fmt.Sprintf("COPY %s foo.txt", filename))
ch.AddKey(fmt.Sprintf("COPY %s bar.txt", filename))
ch.AddPath(filePath)
logrus.Infof("test composite key %v", ch)
hash3, err := ch.Hash()
image := fakeImage{
ImageLayers: []v1.Layer{
fakeLayer{
TarContent: tarContent,
},
},
}
dockerFile := fmt.Sprintf(`
FROM ubuntu:16.04
RUN foobar
COPY %s bar.txt
`, filename)
f, _ := ioutil.TempFile("", "")
ioutil.WriteFile(f.Name(), []byte(dockerFile), 0755)
opts := &config.KanikoOptions{
DockerfilePath: f.Name(),
}
stages, err := dockerfile.Stages(opts)
if err != nil {
t.Errorf("could not parse test dockerfile")
}
stage := stages[0]
cmds := stage.Commands
return testcase{
description: "cached run command followed by uncached copy command result in consistent read and write hashes",
opts: &config.KanikoOptions{Cache: true},
rootDir: dir,
config: &v1.ConfigFile{Config: v1.Config{WorkingDir: destDir}},
layerCache: &fakeLayerCache{
keySequence: []string{hash1},
img: image,
},
image: image,
// hash1 is the read cachekey for the first layer
// hash2 is the read cachekey for the second layer
expectedCacheKeys: []string{hash1, hash2},
pushedCacheKeys: []string{hash2},
commands: getCommands(dir, cmds),
}
}(),
func() testcase {
dir, filenames := tempDirAndFile(t)
filename := filenames[0]
tarContent := generateTar(t, filename)
destDir, err := ioutil.TempDir("", "baz")
if err != nil {
t.Errorf("could not create temp dir %v", err)
}
filePath := filepath.Join(dir, filename)
ch := NewCompositeCache("", fmt.Sprintf("COPY %s foo.txt", filename))
ch.AddPath(filePath)
hash1, err := ch.Hash()
if err != nil {
t.Errorf("couldn't create hash %v", err)
}
ch.AddKey(fmt.Sprintf("COPY %s bar.txt", filename))
ch.AddPath(filePath)
hash2, err := ch.Hash()
if err != nil {
t.Errorf("couldn't create hash %v", err)
}
ch = NewCompositeCache("", fmt.Sprintf("COPY %s foo.txt", filename))
ch.AddKey(fmt.Sprintf("COPY %s bar.txt", filename))
ch.AddPath(filePath)
image := fakeImage{
ImageLayers: []v1.Layer{
fakeLayer{
@ -749,10 +817,12 @@ COPY %s bar.txt
if err != nil {
t.Errorf("could not parse test dockerfile")
}
stage := stages[0]
cmds := stage.Commands
return testcase{
description: "cached copy command followed by uncached copy command result in different read and write hashes",
description: "cached copy command followed by uncached copy command result in consistent read and write hashes",
opts: &config.KanikoOptions{Cache: true},
rootDir: dir,
config: &v1.ConfigFile{Config: v1.Config{WorkingDir: destDir}},
@ -764,10 +834,8 @@ COPY %s bar.txt
// hash1 is the read cachekey for the first layer
// hash2 is the read cachekey for the second layer
expectedCacheKeys: []string{hash1, hash2},
// Due to CachingCopyCommand and CopyCommand returning different values the write cache key for the second copy command will never match the read cache key
// hash3 is the cachekey used to write to the cache for layer 2
pushedCacheKeys: []string{hash3},
commands: getCommands(dir, cmds),
pushedCacheKeys: []string{hash2},
commands: getCommands(dir, cmds),
}
}(),
}
@ -848,6 +916,11 @@ func assertCacheKeys(t *testing.T, expectedCacheKeys, actualCacheKeys []string,
sort.Slice(actualCacheKeys, func(x, y int) bool {
return actualCacheKeys[x] > actualCacheKeys[y]
})
if len(expectedCacheKeys) != len(actualCacheKeys) {
t.Errorf("expected %v to equal %v", actualCacheKeys, expectedCacheKeys)
}
for i, key := range expectedCacheKeys {
if key != actualCacheKeys[i] {
t.Errorf("expected to %v keys %d to be %v but was %v %v", description, i, key, actualCacheKeys[i], actualCacheKeys)