From e003bae87de000d92b70cdb14f4bfeaa9822ae34 Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Mon, 9 Sep 2019 13:57:31 -0700 Subject: [PATCH 01/11] first commit of copy_test --- pkg/commands/copy_test.go | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 pkg/commands/copy_test.go diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go new file mode 100644 index 000000000..d99e756aa --- /dev/null +++ b/pkg/commands/copy_test.go @@ -0,0 +1,56 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package commands + +import ( + "testing" + + "github.com/GoogleContainerTools/kaniko/testutil" + "github.com/google/go-containerregistry/pkg/v1" + "github.com/moby/buildkit/frontend/dockerfile/instructions" +) + +var copyTests = []struct { + SourcesAndDest []string + expectedDest []string +}{ + { + SourcesAndDest: []string{"/usr/bin/bash", "/tmp"}, + expectedDest: []string{"/tmp/bash"}, + }, + { + SourcesAndDest: []string{"/usr/bin/bash", "/tmp/"}, + expectedDest: []string{"/tmp/bash"}, + }, +} + +func TestCopyExecuteCmd(t *testing.T) { + + cfg := &v1.Config{ + Cmd: nil, + } + + for _, test := range copyTests { + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: instructions.SourcesAndDest{ + SourcesAndDest: test.SourcesAndDest, + }, + } + err := cmd.ExecuteCommand(cfg, nil) + testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedDest, cfg.WorkingDir) + } +} From 1bb5a41d7d66b9775deab575e809d470563f126e Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Mon, 16 Sep 2019 10:41:59 -0700 Subject: [PATCH 02/11] Fixing lint issues. Adding tests for COPY command. Fixing issue with copying files out of snapshots --- pkg/commands/add.go | 3 +- pkg/commands/arg.go | 2 +- pkg/commands/base_command.go | 2 +- pkg/commands/cmd.go | 2 +- pkg/commands/cmd_test.go | 2 +- pkg/commands/commands.go | 2 +- pkg/commands/copy.go | 2 +- pkg/commands/copy_test.go | 96 +++++++++++++++++++++++++++------ pkg/commands/entrypoint.go | 2 +- pkg/commands/entrypoint_test.go | 2 +- pkg/commands/env.go | 2 +- pkg/commands/env_test.go | 2 +- pkg/commands/expose.go | 2 +- pkg/commands/expose_test.go | 2 +- pkg/commands/healthcheck.go | 2 +- pkg/commands/label.go | 2 +- pkg/commands/label_test.go | 2 +- pkg/commands/onbuild.go | 2 +- pkg/commands/onbuild_test.go | 2 +- pkg/util/fs_util.go | 9 ++-- pkg/util/image_util.go | 2 +- 21 files changed, 104 insertions(+), 40 deletions(-) diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 769d050dd..7a3d6164b 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -19,12 +19,11 @@ package commands import ( "path/filepath" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" - "github.com/google/go-containerregistry/pkg/v1" - "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/sirupsen/logrus" ) diff --git a/pkg/commands/arg.go b/pkg/commands/arg.go index e17767338..598e59a50 100644 --- a/pkg/commands/arg.go +++ b/pkg/commands/arg.go @@ -19,7 +19,7 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/base_command.go b/pkg/commands/base_command.go index 3d2cd8fad..ddbfd650b 100644 --- a/pkg/commands/base_command.go +++ b/pkg/commands/base_command.go @@ -18,7 +18,7 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" ) type BaseCommand struct { diff --git a/pkg/commands/cmd.go b/pkg/commands/cmd.go index 87714430b..ff94de509 100644 --- a/pkg/commands/cmd.go +++ b/pkg/commands/cmd.go @@ -20,8 +20,8 @@ import ( "strings" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/cmd_test.go b/pkg/commands/cmd_test.go index b56786ad1..182e9707c 100644 --- a/pkg/commands/cmd_test.go +++ b/pkg/commands/cmd_test.go @@ -19,7 +19,7 @@ import ( "testing" "github.com/GoogleContainerTools/kaniko/testutil" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 2615138be..a2ea7788d 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -18,7 +18,7 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/pkg/errors" "github.com/sirupsen/logrus" diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 9af5ce3fd..c8ec11427 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -27,7 +27,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" ) type CopyCommand struct { diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index d99e756aa..0519f6682 100644 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -16,41 +16,103 @@ limitations under the License. package commands import ( + "io/ioutil" + "os" + "path/filepath" "testing" + "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/testutil" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/sirupsen/logrus" ) var copyTests = []struct { - SourcesAndDest []string - expectedDest []string + name string + SourcesAndDest []string + expectedDest []string }{ { - SourcesAndDest: []string{"/usr/bin/bash", "/tmp"}, - expectedDest: []string{"/tmp/bash"}, + name: "copy foo into tempCopyExecuteTest/", + SourcesAndDest: []string{"foo", "tempCopyExecuteTest/"}, + expectedDest: []string{"foo"}, }, { - SourcesAndDest: []string{"/usr/bin/bash", "/tmp/"}, - expectedDest: []string{"/tmp/bash"}, + name: "copy foo into tempCopyExecuteTest", + SourcesAndDest: []string{"foo", "tempCopyExecuteTest"}, + expectedDest: []string{"tempCopyExecuteTest"}, }, } func TestCopyExecuteCmd(t *testing.T) { - cfg := &v1.Config{ - Cmd: nil, + Cmd: nil, + Env: []string{}, + WorkingDir: "../../integration/context/", } for _, test := range copyTests { - cmd := CopyCommand{ - cmd: &instructions.CopyCommand{ - SourcesAndDest: instructions.SourcesAndDest{ - SourcesAndDest: test.SourcesAndDest, - }, - } - err := cmd.ExecuteCommand(cfg, nil) - testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedDest, cfg.WorkingDir) + t.Run(test.name, func(t *testing.T) { + dirList := []string{} + + logrus.Infof("Running test: %v", test.name) + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: test.SourcesAndDest, + }, + buildcontext: "../../integration/context/", + } + + buildArgs := copySetUpBuildArgs() + dest := cfg.WorkingDir + test.SourcesAndDest[len(test.SourcesAndDest)-1] + + os.RemoveAll(dest) + + err := cmd.ExecuteCommand(cfg, buildArgs) + + fi, ferr := os.Open(dest) + if ferr != nil { + t.Error() + } + defer fi.Close() + fstat, ferr := fi.Stat() + if ferr != nil { + t.Error() + } + if fstat.IsDir() { + files, ferr := ioutil.ReadDir(dest) + if ferr != nil { + t.Error() + } + for _, file := range files { + logrus.Infof("file: %v", file.Name()) + dirList = append(dirList, file.Name()) + } + } else { + dirList = append(dirList, filepath.Base(dest)) + } + //dir, err := os.Getwd() + // logrus.Infof("CWD: %v", dir) + // logrus.Infof("test.SourcesAndDest: %v", test.SourcesAndDest) + logrus.Infof("dest: %v", dest) + logrus.Infof("test.expectedDest: %v", test.expectedDest) + logrus.Infof("dirList: %v", dirList) + + testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedDest, dirList) + os.RemoveAll(dest) + }) } } + +func copySetUpBuildArgs() *dockerfile.BuildArgs { + buildArgs := dockerfile.NewBuildArgs([]string{ + "buildArg1=foo", + "buildArg2=foo2", + }) + buildArgs.AddArg("buildArg1", nil) + d := "default" + buildArgs.AddArg("buildArg2", &d) + return buildArgs +} diff --git a/pkg/commands/entrypoint.go b/pkg/commands/entrypoint.go index 23f349347..83964d4fa 100644 --- a/pkg/commands/entrypoint.go +++ b/pkg/commands/entrypoint.go @@ -20,8 +20,8 @@ import ( "strings" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/entrypoint_test.go b/pkg/commands/entrypoint_test.go index 5343d67fe..190c7055c 100644 --- a/pkg/commands/entrypoint_test.go +++ b/pkg/commands/entrypoint_test.go @@ -19,7 +19,7 @@ import ( "testing" "github.com/GoogleContainerTools/kaniko/testutil" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/env.go b/pkg/commands/env.go index 5286f7260..2be271490 100644 --- a/pkg/commands/env.go +++ b/pkg/commands/env.go @@ -18,9 +18,9 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/GoogleContainerTools/kaniko/pkg/util" - "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/env_test.go b/pkg/commands/env_test.go index d4870bb1e..ccada538c 100644 --- a/pkg/commands/env_test.go +++ b/pkg/commands/env_test.go @@ -20,7 +20,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/testutil" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go index 9d56ee3ea..0aacbb6cf 100644 --- a/pkg/commands/expose.go +++ b/pkg/commands/expose.go @@ -21,9 +21,9 @@ import ( "strings" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/GoogleContainerTools/kaniko/pkg/util" - "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/sirupsen/logrus" ) diff --git a/pkg/commands/expose_test.go b/pkg/commands/expose_test.go index 02ad17576..fa1f9520a 100644 --- a/pkg/commands/expose_test.go +++ b/pkg/commands/expose_test.go @@ -20,9 +20,9 @@ import ( "testing" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/GoogleContainerTools/kaniko/testutil" - "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/healthcheck.go b/pkg/commands/healthcheck.go index 610d9c935..af40ba652 100644 --- a/pkg/commands/healthcheck.go +++ b/pkg/commands/healthcheck.go @@ -18,7 +18,7 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/label.go b/pkg/commands/label.go index c8790824b..7568886c0 100644 --- a/pkg/commands/label.go +++ b/pkg/commands/label.go @@ -18,9 +18,9 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/GoogleContainerTools/kaniko/pkg/util" - "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/sirupsen/logrus" ) diff --git a/pkg/commands/label_test.go b/pkg/commands/label_test.go index da2cfbaea..644746311 100644 --- a/pkg/commands/label_test.go +++ b/pkg/commands/label_test.go @@ -21,7 +21,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/testutil" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/commands/onbuild.go b/pkg/commands/onbuild.go index 0d36afe4b..74c6ad01e 100644 --- a/pkg/commands/onbuild.go +++ b/pkg/commands/onbuild.go @@ -18,7 +18,7 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/sirupsen/logrus" ) diff --git a/pkg/commands/onbuild_test.go b/pkg/commands/onbuild_test.go index 10ee9ba26..e2d5d940d 100644 --- a/pkg/commands/onbuild_test.go +++ b/pkg/commands/onbuild_test.go @@ -20,9 +20,9 @@ import ( "testing" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/GoogleContainerTools/kaniko/testutil" - "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 4abfcd4a4..b9dffacc4 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -32,7 +32,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/docker/docker/builder/dockerignore" "github.com/docker/docker/pkg/fileutils" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -188,9 +188,12 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { switch hdr.Typeflag { case tar.TypeReg: logrus.Debugf("creating file %s", path) - // It's possible a file is in the tar before its directory. - if _, err := os.Stat(dir); os.IsNotExist(err) { + // It's possible a file is in the tar before its directory, + // or a file was copied over a directory prior to now + fi, err := os.Stat(dir) + if os.IsNotExist(err) || !fi.IsDir() { logrus.Debugf("base %s for file %s does not exist. Creating.", base, path) + os.RemoveAll(dir) if err := os.MkdirAll(dir, 0755); err != nil { return err } diff --git a/pkg/util/image_util.go b/pkg/util/image_util.go index 0978a6fef..01dc12fae 100644 --- a/pkg/util/image_util.go +++ b/pkg/util/image_util.go @@ -28,7 +28,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/creds" "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/tarball" From e58ee0967a1995ad0aa330efd6cbc7e369c4b4bc Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Thu, 26 Sep 2019 11:29:53 -0700 Subject: [PATCH 03/11] Fixing logic for Copy command. The problem was not that tar files were being unpacked in wrong order. The problem was that the COPY command requires the FS to be unpacked before it does its work. --- pkg/commands/copy.go | 6 ++++++ pkg/util/fs_util.go | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index c8ec11427..1f62b4311 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -70,6 +70,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu // we need to add '/' to the end to indicate the destination is a directory dest = filepath.Join(cwd, dest) + "/" } + logrus.Debugf("Calling CopyDir fullPath:%s dest:%s", fullPath, dest) copiedFiles, err := util.CopyDir(fullPath, dest, c.buildcontext) if err != nil { return err @@ -87,6 +88,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu c.snapshotFiles = append(c.snapshotFiles, destPath) } else { // ... Else, we want to copy over a file + logrus.Debugf("Calling CopyFile fullPath:%s destPath:%s", fullPath, destPath) exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext) if err != nil { return err @@ -134,3 +136,7 @@ func (c *CopyCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerf func (c *CopyCommand) MetadataOnly() bool { return false } + +func (c *CopyCommand) RequiresUnpackedFS() bool { + return true +} diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index b9dffacc4..a4d7996f8 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -192,8 +192,8 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { // or a file was copied over a directory prior to now fi, err := os.Stat(dir) if os.IsNotExist(err) || !fi.IsDir() { - logrus.Debugf("base %s for file %s does not exist. Creating.", base, path) - os.RemoveAll(dir) + logrus.Infof("base %s for file %s does not exist. Creating.", base, path) + //os.RemoveAll(dir) if err := os.MkdirAll(dir, 0755); err != nil { return err } From dbabcb1f5fc101c2735c9438cf326e8a8a51aac4 Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Thu, 26 Sep 2019 15:32:40 -0700 Subject: [PATCH 04/11] Adding CachingCopy command --- pkg/commands/copy.go | 42 ++++++++++++++++++++++++++++++++++++++++-- pkg/commands/run.go | 2 +- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 1f62b4311..0dd427239 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -21,6 +21,7 @@ import ( "path/filepath" "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/kaniko/pkg/constants" @@ -70,7 +71,6 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu // we need to add '/' to the end to indicate the destination is a directory dest = filepath.Join(cwd, dest) + "/" } - logrus.Debugf("Calling CopyDir fullPath:%s dest:%s", fullPath, dest) copiedFiles, err := util.CopyDir(fullPath, dest, c.buildcontext) if err != nil { return err @@ -88,7 +88,6 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu c.snapshotFiles = append(c.snapshotFiles, destPath) } else { // ... Else, we want to copy over a file - logrus.Debugf("Calling CopyFile fullPath:%s destPath:%s", fullPath, destPath) exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext) if err != nil { return err @@ -140,3 +139,42 @@ func (c *CopyCommand) MetadataOnly() bool { func (c *CopyCommand) RequiresUnpackedFS() bool { return true } + +func (r *CopyCommand) ShouldCacheOutput() bool { + return true +} + +// CacheCommand returns true since this command should be cached +func (r *CopyCommand) CacheCommand(img v1.Image) DockerCommand { + + return &CachingCopyCommand{ + img: img, + cmd: r.cmd, + } +} + +type CachingCopyCommand struct { + BaseCommand + img v1.Image + extractedFiles []string + cmd *instructions.CopyCommand +} + +func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { + logrus.Infof("Found cached layer, extracting to filesystem") + var err error + cr.extractedFiles, err = util.GetFSFromImage(constants.RootDir, cr.img) + logrus.Infof("extractedFiles: %s", cr.extractedFiles) + if err != nil { + return errors.Wrap(err, "extracting fs from image") + } + return nil +} + +func (cr *CachingCopyCommand) FilesToSnapshot() []string { + return cr.extractedFiles +} + +func (cr *CachingCopyCommand) String() string { + return cr.cmd.String() +} diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 7a9bf03dc..0122f37d5 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -28,7 +28,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/pkg/errors" "github.com/sirupsen/logrus" From 4e1639c0305ff3d934af6c676af2df8c90ac5bf1 Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Mon, 7 Oct 2019 10:46:28 -0700 Subject: [PATCH 05/11] Addressing review comments --- pkg/commands/copy_test.go | 10 +++++----- pkg/util/fs_util.go | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 0519f6682..b60fb951d 100644 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -30,17 +30,17 @@ import ( var copyTests = []struct { name string - SourcesAndDest []string + sourcesAndDest []string expectedDest []string }{ { name: "copy foo into tempCopyExecuteTest/", - SourcesAndDest: []string{"foo", "tempCopyExecuteTest/"}, + sourcesAndDest: []string{"foo", "tempCopyExecuteTest/"}, expectedDest: []string{"foo"}, }, { name: "copy foo into tempCopyExecuteTest", - SourcesAndDest: []string{"foo", "tempCopyExecuteTest"}, + sourcesAndDest: []string{"foo", "tempCopyExecuteTest"}, expectedDest: []string{"tempCopyExecuteTest"}, }, } @@ -60,13 +60,13 @@ func TestCopyExecuteCmd(t *testing.T) { cmd := CopyCommand{ cmd: &instructions.CopyCommand{ - SourcesAndDest: test.SourcesAndDest, + SourcesAndDest: test.sourcesAndDest, }, buildcontext: "../../integration/context/", } buildArgs := copySetUpBuildArgs() - dest := cfg.WorkingDir + test.SourcesAndDest[len(test.SourcesAndDest)-1] + dest := cfg.WorkingDir + test.sourcesAndDest[len(test.sourcesAndDest)-1] os.RemoveAll(dest) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index a4d7996f8..aa4289315 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -193,7 +193,6 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { fi, err := os.Stat(dir) if os.IsNotExist(err) || !fi.IsDir() { logrus.Infof("base %s for file %s does not exist. Creating.", base, path) - //os.RemoveAll(dir) if err := os.MkdirAll(dir, 0755); err != nil { return err } From 2eace1d511a68ae4f54cd427d8cc1b6ebb379b1b Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Mon, 7 Oct 2019 12:56:34 -0700 Subject: [PATCH 06/11] Refactoring to add use of a tempdir --- pkg/commands/copy_test.go | 86 ++++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index b60fb951d..d1d33306f 100644 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -16,9 +16,11 @@ limitations under the License. package commands import ( + "io" "io/ioutil" "os" "path/filepath" + "strings" "testing" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" @@ -45,45 +47,97 @@ var copyTests = []struct { }, } +func setupTestTemp() string { + tempDir, err := ioutil.TempDir("", "") + if err != nil { + logrus.Fatalf("error creating temp dir %s", err) + } + //defer os.RemoveAll(tempDir) + logrus.Infof("Tempdir: %s", tempDir) + + srcPath, err := filepath.Abs("../../integration/context") + cperr := filepath.Walk(srcPath, + func(path string, info os.FileInfo, err error) error { + if path != srcPath { + if err != nil { + return err + } + tempPath := strings.TrimPrefix(path, srcPath) + fileInfo, err := os.Stat(path) + if err != nil { + return err + } + if fileInfo.IsDir() { + os.MkdirAll(tempDir+"/"+tempPath, 0777) + } else { + out, err := os.Create(tempDir + "/" + tempPath) + defer out.Close() + if err != nil { + return err + } + + in, err := os.Open(path) + defer in.Close() + if err != nil { + return err + } + + _, err = io.Copy(out, in) + if err != nil { + return err + } + } + } + return nil + }) + if cperr != nil { + logrus.Fatalf("error populating temp dir %s", cperr) + } + + return tempDir +} func TestCopyExecuteCmd(t *testing.T) { + tempDir := setupTestTemp() + cfg := &v1.Config{ Cmd: nil, Env: []string{}, - WorkingDir: "../../integration/context/", + WorkingDir: tempDir, } for _, test := range copyTests { t.Run(test.name, func(t *testing.T) { dirList := []string{} - logrus.Infof("Running test: %v", test.name) - cmd := CopyCommand{ cmd: &instructions.CopyCommand{ SourcesAndDest: test.sourcesAndDest, }, - buildcontext: "../../integration/context/", + buildcontext: tempDir, } buildArgs := copySetUpBuildArgs() - dest := cfg.WorkingDir + test.sourcesAndDest[len(test.sourcesAndDest)-1] - - os.RemoveAll(dest) + dest := cfg.WorkingDir + "/" + test.sourcesAndDest[len(test.sourcesAndDest)-1] + logrus.Infof("dest dir: %s", dest) + //os.RemoveAll(dest) err := cmd.ExecuteCommand(cfg, buildArgs) + if err != nil { + t.Error() + } - fi, ferr := os.Open(dest) - if ferr != nil { + fi, err := os.Open(dest) + if err != nil { t.Error() } defer fi.Close() - fstat, ferr := fi.Stat() - if ferr != nil { + fstat, err := fi.Stat() + if err != nil { t.Error() } if fstat.IsDir() { - files, ferr := ioutil.ReadDir(dest) - if ferr != nil { + files, err := ioutil.ReadDir(dest) + if err != nil { t.Error() } for _, file := range files { @@ -93,12 +147,6 @@ func TestCopyExecuteCmd(t *testing.T) { } else { dirList = append(dirList, filepath.Base(dest)) } - //dir, err := os.Getwd() - // logrus.Infof("CWD: %v", dir) - // logrus.Infof("test.SourcesAndDest: %v", test.SourcesAndDest) - logrus.Infof("dest: %v", dest) - logrus.Infof("test.expectedDest: %v", test.expectedDest) - logrus.Infof("dirList: %v", dirList) testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedDest, dirList) os.RemoveAll(dest) From 2d73c2d996efd8b116418cc24e76a98ac228e45d Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Mon, 7 Oct 2019 13:06:40 -0700 Subject: [PATCH 07/11] addressing lint issues --- pkg/commands/copy.go | 4 ++-- pkg/commands/copy_test.go | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 0dd427239..55b635ea0 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -140,12 +140,12 @@ func (c *CopyCommand) RequiresUnpackedFS() bool { return true } -func (r *CopyCommand) ShouldCacheOutput() bool { +func (c *CopyCommand) ShouldCacheOutput() bool { return true } // CacheCommand returns true since this command should be cached -func (r *CopyCommand) CacheCommand(img v1.Image) DockerCommand { +func (c *CopyCommand) CacheCommand(img v1.Image) DockerCommand { return &CachingCopyCommand{ img: img, diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index d1d33306f..fcdbf517f 100644 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -56,6 +56,9 @@ func setupTestTemp() string { logrus.Infof("Tempdir: %s", tempDir) srcPath, err := filepath.Abs("../../integration/context") + if err != nil { + logrus.Fatalf("error getting abs path %s", srcPath) + } cperr := filepath.Walk(srcPath, func(path string, info os.FileInfo, err error) error { if path != srcPath { @@ -71,16 +74,16 @@ func setupTestTemp() string { os.MkdirAll(tempDir+"/"+tempPath, 0777) } else { out, err := os.Create(tempDir + "/" + tempPath) - defer out.Close() if err != nil { return err } + defer out.Close() in, err := os.Open(path) - defer in.Close() if err != nil { return err } + defer in.Close() _, err = io.Copy(out, in) if err != nil { @@ -119,7 +122,6 @@ func TestCopyExecuteCmd(t *testing.T) { buildArgs := copySetUpBuildArgs() dest := cfg.WorkingDir + "/" + test.sourcesAndDest[len(test.sourcesAndDest)-1] logrus.Infof("dest dir: %s", dest) - //os.RemoveAll(dest) err := cmd.ExecuteCommand(cfg, buildArgs) if err != nil { From 2b6965b294b5bca4e73ac9fed3f5a0569dea389f Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Mon, 7 Oct 2019 13:11:09 -0700 Subject: [PATCH 08/11] addressing lint issues --- pkg/commands/copy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 55b635ea0..15f778e8d 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -149,7 +149,7 @@ func (c *CopyCommand) CacheCommand(img v1.Image) DockerCommand { return &CachingCopyCommand{ img: img, - cmd: r.cmd, + cmd: c.cmd, } } From e5da55d4cd141753c0cfa8d32cd8740d29ba9ac7 Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Tue, 8 Oct 2019 07:42:09 -0700 Subject: [PATCH 09/11] Adding integration test --- integration/images.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integration/images.go b/integration/images.go index 4f7f3b8bc..f39ed0ecd 100644 --- a/integration/images.go +++ b/integration/images.go @@ -135,6 +135,7 @@ func NewDockerFileBuilder(dockerfiles []string) *DockerFileBuilder { "Dockerfile_test_cache": {}, "Dockerfile_test_cache_install": {}, "Dockerfile_test_cache_perm": {}, + "Dockerfile_test_cache_copy": {}, } return &d } From 27020f23b92ae0bed0c62e33cdc5eaa0351dd1cf Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Tue, 8 Oct 2019 12:19:20 -0700 Subject: [PATCH 10/11] Resolving comments --- pkg/commands/copy_test.go | 7 +++---- pkg/util/fs_util.go | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index fcdbf517f..9b16e9291 100644 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -52,8 +52,7 @@ func setupTestTemp() string { if err != nil { logrus.Fatalf("error creating temp dir %s", err) } - //defer os.RemoveAll(tempDir) - logrus.Infof("Tempdir: %s", tempDir) + logrus.Debugf("Tempdir: %s", tempDir) srcPath, err := filepath.Abs("../../integration/context") if err != nil { @@ -101,6 +100,7 @@ func setupTestTemp() string { } func TestCopyExecuteCmd(t *testing.T) { tempDir := setupTestTemp() + defer os.RemoveAll(tempDir) cfg := &v1.Config{ Cmd: nil, @@ -121,7 +121,6 @@ func TestCopyExecuteCmd(t *testing.T) { buildArgs := copySetUpBuildArgs() dest := cfg.WorkingDir + "/" + test.sourcesAndDest[len(test.sourcesAndDest)-1] - logrus.Infof("dest dir: %s", dest) err := cmd.ExecuteCommand(cfg, buildArgs) if err != nil { @@ -143,7 +142,7 @@ func TestCopyExecuteCmd(t *testing.T) { t.Error() } for _, file := range files { - logrus.Infof("file: %v", file.Name()) + logrus.Debugf("file: %v", file.Name()) dirList = append(dirList, file.Name()) } } else { diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index aa4289315..7a7f15b34 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -192,7 +192,7 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { // or a file was copied over a directory prior to now fi, err := os.Stat(dir) if os.IsNotExist(err) || !fi.IsDir() { - logrus.Infof("base %s for file %s does not exist. Creating.", base, path) + logrus.Debugf("base %s for file %s does not exist. Creating.", base, path) if err := os.MkdirAll(dir, 0755); err != nil { return err } From ab7b210ac110d4154011ff62972f60bf747d0148 Mon Sep 17 00:00:00 2001 From: Don McCasland Date: Mon, 4 Nov 2019 09:30:58 -0800 Subject: [PATCH 11/11] Adding missing integration test file --- integration/dockerfiles/Dockerfile_test_cache_copy | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 integration/dockerfiles/Dockerfile_test_cache_copy diff --git a/integration/dockerfiles/Dockerfile_test_cache_copy b/integration/dockerfiles/Dockerfile_test_cache_copy new file mode 100644 index 000000000..661397eae --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_cache_copy @@ -0,0 +1,3 @@ +FROM google/cloud-sdk:256.0.0-alpine + +COPY context/foo /usr/bin