From 7611ea7a1d787a5892250b04e51e06149967c3d6 Mon Sep 17 00:00:00 2001 From: dlorenc Date: Thu, 6 Dec 2018 12:44:03 -0600 Subject: [PATCH 1/4] Add support for COPY --from=. (#479) Right now kaniko only supports COPY --from=. This commit adds support for the case where the referenced image is a remote image in a registry that has not been used as a stage yet in the build. --- .../dockerfiles/Dockerfile_test_multistage | 1 + pkg/dockerfile/dockerfile.go | 1 + pkg/executor/build.go | 52 ++++++++++++++++--- pkg/util/image_util.go | 31 ++++++----- pkg/util/image_util_test.go | 6 +-- 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_multistage b/integration/dockerfiles/Dockerfile_test_multistage index 25246c815..91a130497 100644 --- a/integration/dockerfiles/Dockerfile_test_multistage +++ b/integration/dockerfiles/Dockerfile_test_multistage @@ -15,3 +15,4 @@ FROM fedora@sha256:c4cc32b09c6ae3f1353e7e33a8dda93dc41676b923d6d89afa996b421cc5a FROM base ARG file COPY --from=second /foo ${file} +COPY --from=gcr.io/google-appengine/debian9@sha256:00109fa40230a081f5ecffe0e814725042ff62a03e2d1eae0563f1f82eaeae9b /etc/os-release /new diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 2a886d0f6..6c401d829 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -126,6 +126,7 @@ func resolveStages(stages []instructions.Stage) { if val, ok := nameToIndex[c.From]; ok { c.From = val } + } } } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 88ebafe14..17cc2dd0f 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -23,6 +23,8 @@ import ( "strconv" "time" + "github.com/moby/buildkit/frontend/dockerfile/instructions" + "golang.org/x/sync/errgroup" "github.com/google/go-containerregistry/pkg/name" @@ -337,6 +339,10 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { if err != nil { return nil, err } + // Some stages may refer to other random images, not previous stages + if err := fetchExtraStages(stages, opts); err != nil { + return nil, err + } for index, stage := range stages { sb, err := newStageBuilder(opts, stage) if err != nil { @@ -380,10 +386,10 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return sourceImage, nil } if stage.SaveStage { - if err := saveStageAsTarball(index, sourceImage); err != nil { + if err := saveStageAsTarball(strconv.Itoa(index), sourceImage); err != nil { return nil, err } - if err := extractImageToDependecyDir(index, sourceImage); err != nil { + if err := extractImageToDependecyDir(strconv.Itoa(index), sourceImage); err != nil { return nil, err } } @@ -396,8 +402,38 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return nil, err } -func extractImageToDependecyDir(index int, image v1.Image) error { - dependencyDir := filepath.Join(constants.KanikoDir, strconv.Itoa(index)) +func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) error { + for _, s := range stages { + for _, cmd := range s.Commands { + c, ok := cmd.(*instructions.CopyCommand) + if !ok || c.From == "" { + continue + } + + // FROMs at this point are guaranteed to be either an integer referring to a previous stage or + // a name of a remote image. + if _, err := strconv.Atoi(c.From); err == nil { + continue + } + // This must be an image name, fetch it. + logrus.Debugf("Found extra base image stage %s", c.From) + sourceImage, err := util.RetrieveRemoteImage(c.From, opts) + if err != nil { + return err + } + if err := saveStageAsTarball(c.From, sourceImage); err != nil { + return err + } + if err := extractImageToDependecyDir(c.From, sourceImage); err != nil { + return err + } + } + } + return nil +} + +func extractImageToDependecyDir(name string, image v1.Image) error { + dependencyDir := filepath.Join(constants.KanikoDir, name) if err := os.MkdirAll(dependencyDir, 0755); err != nil { return err } @@ -406,16 +442,16 @@ func extractImageToDependecyDir(index int, image v1.Image) error { return err } -func saveStageAsTarball(stageIndex int, image v1.Image) error { +func saveStageAsTarball(path string, image v1.Image) error { destRef, err := name.NewTag("temp/tag", name.WeakValidation) if err != nil { return err } - if err := os.MkdirAll(constants.KanikoIntermediateStagesDir, 0750); err != nil { + tarPath := filepath.Join(constants.KanikoIntermediateStagesDir, path) + logrus.Infof("Storing source image from stage %d at path %s", path, tarPath) + if err := os.MkdirAll(filepath.Dir(tarPath), 0750); err != nil { return err } - tarPath := filepath.Join(constants.KanikoIntermediateStagesDir, strconv.Itoa(stageIndex)) - logrus.Infof("Storing source image from stage %d at path %s", stageIndex, tarPath) return tarball.WriteToFile(tarPath, destRef, image) } diff --git a/pkg/util/image_util.go b/pkg/util/image_util.go index 8e46ce067..cb167c9de 100644 --- a/pkg/util/image_util.go +++ b/pkg/util/image_util.go @@ -39,8 +39,8 @@ import ( ) var ( - // For testing - retrieveRemoteImage = remoteImage + // RetrieveRemoteImage downloads an image from a remote location + RetrieveRemoteImage = remoteImage retrieveTarImage = tarballImage ) @@ -67,21 +67,8 @@ func RetrieveSourceImage(stage config.KanikoStage, opts *config.KanikoOptions) ( return retrieveTarImage(stage.BaseImageIndex) } - // Next, check if local caching is enabled - // If so, look in the local cache before trying the remote registry - if opts.Cache && opts.CacheDir != "" { - cachedImage, err := cachedImage(opts, currentBaseName) - if cachedImage != nil { - return cachedImage, nil - } - - if err != nil { - logrus.Warnf("Error while retrieving image from cache: %v", err) - } - } - // Otherwise, initialize image as usual - return retrieveRemoteImage(currentBaseName, opts) + return RetrieveRemoteImage(currentBaseName, opts) } // RetrieveConfigFile returns the config file for an image @@ -104,6 +91,18 @@ func tarballImage(index int) (v1.Image, error) { func remoteImage(image string, opts *config.KanikoOptions) (v1.Image, error) { logrus.Infof("Downloading base image %s", image) + // First, check if local caching is enabled + // If so, look in the local cache before trying the remote registry + if opts.Cache && opts.CacheDir != "" { + cachedImage, err := cachedImage(opts, image) + if cachedImage != nil { + return cachedImage, nil + } + + if err != nil { + logrus.Warnf("Error while retrieving image from cache: %v", err) + } + } ref, err := name.ParseReference(image, name.WeakValidation) if err != nil { return nil, err diff --git a/pkg/util/image_util_test.go b/pkg/util/image_util_test.go index a155963bc..c44fe8a58 100644 --- a/pkg/util/image_util_test.go +++ b/pkg/util/image_util_test.go @@ -47,14 +47,14 @@ func Test_StandardImage(t *testing.T) { if err != nil { t.Error(err) } - original := retrieveRemoteImage + original := RetrieveRemoteImage defer func() { - retrieveRemoteImage = original + RetrieveRemoteImage = original }() mock := func(image string, opts *config.KanikoOptions) (v1.Image, error) { return nil, nil } - retrieveRemoteImage = mock + RetrieveRemoteImage = mock actual, err := RetrieveSourceImage(config.KanikoStage{ Stage: stages[0], }, &config.KanikoOptions{}) From 7f9ea39bf7793d3af453f6454148596609b0d88b Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Mon, 10 Dec 2018 10:11:05 -0800 Subject: [PATCH 2/4] Avoid the cachedImage/remoteImage call loop (#483) * Avoid the cachedImage/remoteImage call loop * missed one function * fix unit tests * proper bool comparison --- pkg/executor/build.go | 4 ++-- pkg/util/image_util.go | 8 ++++---- pkg/util/image_util_test.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 17cc2dd0f..bd151d2b1 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -417,7 +417,7 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e } // This must be an image name, fetch it. logrus.Debugf("Found extra base image stage %s", c.From) - sourceImage, err := util.RetrieveRemoteImage(c.From, opts) + sourceImage, err := util.RetrieveRemoteImage(c.From, opts, false) if err != nil { return err } @@ -448,7 +448,7 @@ func saveStageAsTarball(path string, image v1.Image) error { return err } tarPath := filepath.Join(constants.KanikoIntermediateStagesDir, path) - logrus.Infof("Storing source image from stage %d at path %s", path, tarPath) + logrus.Infof("Storing source image from stage %s at path %s", path, tarPath) if err := os.MkdirAll(filepath.Dir(tarPath), 0750); err != nil { return err } diff --git a/pkg/util/image_util.go b/pkg/util/image_util.go index cb167c9de..03dd1a442 100644 --- a/pkg/util/image_util.go +++ b/pkg/util/image_util.go @@ -68,7 +68,7 @@ func RetrieveSourceImage(stage config.KanikoStage, opts *config.KanikoOptions) ( } // Otherwise, initialize image as usual - return RetrieveRemoteImage(currentBaseName, opts) + return RetrieveRemoteImage(currentBaseName, opts, false) } // RetrieveConfigFile returns the config file for an image @@ -89,11 +89,11 @@ func tarballImage(index int) (v1.Image, error) { return tarball.ImageFromPath(tarPath, nil) } -func remoteImage(image string, opts *config.KanikoOptions) (v1.Image, error) { +func remoteImage(image string, opts *config.KanikoOptions, forceNoCache bool) (v1.Image, error) { logrus.Infof("Downloading base image %s", image) // First, check if local caching is enabled // If so, look in the local cache before trying the remote registry - if opts.Cache && opts.CacheDir != "" { + if opts.Cache && opts.CacheDir != "" && !forceNoCache { cachedImage, err := cachedImage(opts, image) if cachedImage != nil { return cachedImage, nil @@ -148,7 +148,7 @@ func cachedImage(opts *config.KanikoOptions, image string) (v1.Image, error) { if d, ok := ref.(name.Digest); ok { cacheKey = d.DigestStr() } else { - img, err := remoteImage(image, opts) + img, err := remoteImage(image, opts, true) if err != nil { return nil, err } diff --git a/pkg/util/image_util_test.go b/pkg/util/image_util_test.go index c44fe8a58..e53ff4250 100644 --- a/pkg/util/image_util_test.go +++ b/pkg/util/image_util_test.go @@ -51,7 +51,7 @@ func Test_StandardImage(t *testing.T) { defer func() { RetrieveRemoteImage = original }() - mock := func(image string, opts *config.KanikoOptions) (v1.Image, error) { + mock := func(image string, opts *config.KanikoOptions, forceNoCache bool) (v1.Image, error) { return nil, nil } RetrieveRemoteImage = mock From 01329d5ac16043c0f7aa45a4be3202302bf695d5 Mon Sep 17 00:00:00 2001 From: Andrew Rynhard Date: Mon, 10 Dec 2018 11:34:06 -0800 Subject: [PATCH 3/4] Fix intermediate layer caching (#474) * Fix intermediate layer caching * Move the if statement into the ShouldTakeSnapshot function. Also add some unit tests. --- pkg/executor/build.go | 14 +++-- pkg/executor/build_test.go | 106 +++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index bd151d2b1..f2dd9c78f 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -279,13 +279,18 @@ func (s *stageBuilder) takeSnapshot(files []string) (string, error) { func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool { isLastCommand := index == len(s.stage.Commands)-1 - // We only snapshot the very end of intermediate stages. - if !s.stage.Final { + // We only snapshot the very end with single snapshot mode on. + if s.opts.SingleSnapshot { return isLastCommand } - // We only snapshot the very end with single snapshot mode on. - if s.opts.SingleSnapshot { + // Always take snapshots if we're using the cache. + if s.opts.Cache { + return true + } + + // We only snapshot the very end of intermediate stages. + if !s.stage.Final { return isLastCommand } @@ -298,6 +303,7 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool { if len(files) == 0 { return false } + return true } diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 63dbd8a4b..e212096f1 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -19,6 +19,8 @@ package executor import ( "testing" + "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/testutil" @@ -74,3 +76,107 @@ func stage(t *testing.T, d string) config.KanikoStage { Stage: stages[0], } } + +type MockCommand struct { + name string +} + +func (m *MockCommand) Name() string { + return m.name +} + +func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { + commands := []instructions.Command{ + &MockCommand{name: "command1"}, + &MockCommand{name: "command2"}, + &MockCommand{name: "command3"}, + } + + stage := instructions.Stage{ + Commands: commands, + } + + type fields struct { + stage config.KanikoStage + opts *config.KanikoOptions + } + type args struct { + index int + files []string + } + tests := []struct { + name string + fields fields + args args + want bool + }{ + { + name: "final stage not last command", + fields: fields{ + stage: config.KanikoStage{ + Final: true, + Stage: stage, + }, + }, + args: args{ + index: 1, + }, + want: true, + }, + { + name: "not final stage last command", + fields: fields{ + stage: config.KanikoStage{ + Final: false, + Stage: stage, + }, + }, + args: args{ + index: len(commands) - 1, + }, + want: true, + }, + { + name: "not final stage not last command", + fields: fields{ + stage: config.KanikoStage{ + Final: false, + Stage: stage, + }, + }, + args: args{ + index: 0, + }, + want: false, + }, + { + name: "caching enabled intermediate container", + fields: fields{ + stage: config.KanikoStage{ + Final: false, + Stage: stage, + }, + opts: &config.KanikoOptions{Cache: true}, + }, + args: args{ + index: 0, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + if tt.fields.opts == nil { + tt.fields.opts = &config.KanikoOptions{} + } + s := &stageBuilder{ + stage: tt.fields.stage, + opts: tt.fields.opts, + } + if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files); got != tt.want { + t.Errorf("stageBuilder.shouldTakeSnapshot() = %v, want %v", got, tt.want) + } + }) + } +} From 009d5aa39fda3ce2939f16a6c20469cb219a34cb Mon Sep 17 00:00:00 2001 From: dlorenc Date: Mon, 10 Dec 2018 13:40:24 -0800 Subject: [PATCH 4/4] Add changelog and bump version to 0.7.0 (#485) --- CHANGELOG.md | 14 ++++++++++++++ Makefile | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d83916c5..9b5bfd5b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +# v0.7.0 Release - 12/10/2018 + +## New Features +* Add support for COPY --from an unrelated image + +## Updates +* Speed up snapshotting by using filepath.SkipDir +* Improve layer cache upload performance +* Skip unpacking the base image in certain cases + +## Bug Fixes +* Fix bug with call loop +* Fix caching for multi-step builds + # v0.6.0 Release - 11/06/2018 ## New Features diff --git a/Makefile b/Makefile index 36bd5a190..e12bb2b18 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ # Bump these on release VERSION_MAJOR ?= 0 -VERSION_MINOR ?= 6 +VERSION_MINOR ?= 7 VERSION_BUILD ?= 0 VERSION ?= v$(VERSION_MAJOR).$(VERSION_MINOR).$(VERSION_BUILD)