From 43338d4b2fb0cbc02190847ad8e10de584515f60 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 5 Jun 2020 16:05:04 -0700 Subject: [PATCH 01/10] add a new run command along with a new flag --- cmd/executor/cmd/root.go | 1 + .../dockerfiles/Dockerfile_test_run_new | 26 +++++ integration/images.go | 2 + pkg/commands/base_command.go | 4 + pkg/commands/commands.go | 8 +- pkg/commands/run.go | 11 +- pkg/commands/run_marker.go | 106 ++++++++++++++++++ pkg/config/options.go | 1 + pkg/executor/build.go | 10 +- pkg/executor/build_test.go | 1 + pkg/executor/fakes.go | 6 + pkg/snapshot/snapshot.go | 78 ++++++++----- pkg/snapshot/snapshot_test.go | 4 +- 13 files changed, 219 insertions(+), 39 deletions(-) create mode 100644 integration/dockerfiles/Dockerfile_test_run_new create mode 100644 pkg/commands/run_marker.go diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 8b60da706..eed04861f 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -174,6 +174,7 @@ func addKanikoOptionsFlags() { RootCmd.PersistentFlags().BoolVarP(&opts.IgnoreVarRun, "whitelist-var-run", "", true, "Ignore /var/run directory when taking image snapshot. Set it to false to preserve /var/run/ in destination image. (Default true).") RootCmd.PersistentFlags().VarP(&opts.Labels, "label", "", "Set metadata for an image. Set it repeatedly for multiple labels.") RootCmd.PersistentFlags().BoolVarP(&opts.SkipUnusedStages, "skip-unused-stages", "", false, "Build only used stages if defined to true. Otherwise it builds by default all stages, even the unnecessaries ones until it reaches the target stage / end of Dockerfile") + RootCmd.PersistentFlags().BoolVarP(&opts.RunV2, "use-new-run", "", false, "Experimental run command to detect file system changes. This new run command does no rely on snapshotting to detect changes.") } // addHiddenFlags marks certain flags as hidden from the executor help text diff --git a/integration/dockerfiles/Dockerfile_test_run_new b/integration/dockerfiles/Dockerfile_test_run_new new file mode 100644 index 000000000..9891a4381 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_run_new @@ -0,0 +1,26 @@ +# Copyright 2018 Google, Inc. All rights reserved. +# +# 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. + +FROM debian:9.11 +RUN echo "hey" > /etc/foo +RUN echo "baz" > /etc/baz +RUN cp /etc/baz /etc/bar +RUN rm /etc/baz + +# Test with ARG +ARG file +RUN echo "run" > $file + +RUN echo "test home" > $HOME/file +COPY context/foo $HOME/foo diff --git a/integration/images.go b/integration/images.go index 8865e836a..c221d6504 100644 --- a/integration/images.go +++ b/integration/images.go @@ -48,6 +48,7 @@ const ( // Arguments to build Dockerfiles with, used for both docker and kaniko builds var argsMap = map[string][]string{ "Dockerfile_test_run": {"file=/file"}, + "Dockerfile_test_run_new": {"file=/file"}, "Dockerfile_test_workdir": {"workdir=/arg/workdir"}, "Dockerfile_test_add": {"file=context/foo"}, "Dockerfile_test_arg_secret": {"SSH_PRIVATE_KEY", "SSH_PUBLIC_KEY=Pµbl1cK€Y"}, @@ -74,6 +75,7 @@ var additionalDockerFlagsMap = map[string][]string{ // Arguments to build Dockerfiles with when building with kaniko var additionalKanikoFlagsMap = map[string][]string{ "Dockerfile_test_add": {"--single-snapshot"}, + "Dockerfile_test_run_new": {"--use-new-run=true"}, "Dockerfile_test_scratch": {"--single-snapshot"}, "Dockerfile_test_maintainer": {"--single-snapshot"}, "Dockerfile_test_target": {"--target=second"}, diff --git a/pkg/commands/base_command.go b/pkg/commands/base_command.go index 94c4fe156..cf19c35cb 100644 --- a/pkg/commands/base_command.go +++ b/pkg/commands/base_command.go @@ -51,3 +51,7 @@ func (b *BaseCommand) RequiresUnpackedFS() bool { func (b *BaseCommand) ShouldCacheOutput() bool { return false } + +func (a *BaseCommand) ShouldDetectDeletedFiles() bool { + return false +} diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 0a4e57134..a5d431735 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -52,11 +52,17 @@ type DockerCommand interface { RequiresUnpackedFS() bool ShouldCacheOutput() bool + + // ShouldDetectDeletedFiles turns true if the command could delete files. + ShouldDetectDeletedFiles() bool } -func GetCommand(cmd instructions.Command, buildcontext string) (DockerCommand, error) { +func GetCommand(cmd instructions.Command, buildcontext string, useNewRun bool) (DockerCommand, error) { switch c := cmd.(type) { case *instructions.RunCommand: + if useNewRun { + return &RunMarkerCommand{cmd: c}, nil + } return &RunCommand{cmd: c}, nil case *instructions.CopyCommand: return &CopyCommand{cmd: c, buildcontext: buildcontext}, nil diff --git a/pkg/commands/run.go b/pkg/commands/run.go index e0dd1beef..b99461eaa 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -46,8 +46,12 @@ var ( ) func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { + return runCommandInExec(config, buildArgs, r.cmd) +} + +func runCommandInExec(config *v1.Config, buildArgs *dockerfile.BuildArgs, cmdRun *instructions.RunCommand) error { var newCommand []string - if r.cmd.PrependShell { + if cmdRun.PrependShell { // This is the default shell on Linux var shell []string if len(config.Shell) > 0 { @@ -56,9 +60,9 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui shell = append(shell, "/bin/sh", "-c") } - newCommand = append(shell, strings.Join(r.cmd.CmdLine, " ")) + newCommand = append(shell, strings.Join(cmdRun.CmdLine, " ")) } else { - newCommand = r.cmd.CmdLine + newCommand = cmdRun.CmdLine } logrus.Infof("cmd: %s", newCommand[0]) @@ -111,7 +115,6 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui if err := syscall.Kill(-pgid, syscall.SIGKILL); err != nil && err.Error() != "no such process" { return err } - return nil } diff --git a/pkg/commands/run_marker.go b/pkg/commands/run_marker.go new file mode 100644 index 000000000..780ba0814 --- /dev/null +++ b/pkg/commands/run_marker.go @@ -0,0 +1,106 @@ +/* +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 ( + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + "github.com/GoogleContainerTools/kaniko/pkg/util" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/moby/buildkit/frontend/dockerfile/instructions" +) + +type RunMarkerCommand struct { + BaseCommand + cmd *instructions.RunCommand + Files []string +} + +func (r *RunMarkerCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { + // run command `touch filemarker` + markerFile, err := ioutil.TempFile("", "marker") + defer func() { + os.Remove(markerFile.Name()) + }() + + if err := runCommandInExec(config, buildArgs, r.cmd); err != nil { + return err + } + + // run command find to find all new files generated + find := exec.Command("find", "/", "-newer", markerFile.Name()) + out, err := find.Output() + if err != nil { + r.Files = []string{} + return nil + } + + r.Files = []string{} + s := strings.Split(string(out), "\n") + for _, path := range s { + path = filepath.Clean(path) + if util.IsDestDir(path) || util.CheckIgnoreList(path) { + continue + } + r.Files = append(r.Files, path) + } + return nil +} + +// String returns some information about the command for the image config +func (r *RunMarkerCommand) String() string { + return r.cmd.String() +} + +func (r *RunMarkerCommand) FilesToSnapshot() []string { + return nil +} + +func (r *RunMarkerCommand) ProvidesFilesToSnapshot() bool { + return false +} + +// CacheCommand returns true since this command should be cached +func (r *RunMarkerCommand) CacheCommand(img v1.Image) DockerCommand { + + return &CachingRunCommand{ + img: img, + cmd: r.cmd, + extractFn: util.ExtractFile, + } +} + +func (r *RunMarkerCommand) MetadataOnly() bool { + return false +} + +func (r *RunMarkerCommand) RequiresUnpackedFS() bool { + return true +} + +func (r *RunMarkerCommand) ShouldCacheOutput() bool { + return true +} + +func (b *BaseCommand) ShouldDetectDelete() bool { + return true +} diff --git a/pkg/config/options.go b/pkg/config/options.go index 576d42f09..74db79b6f 100644 --- a/pkg/config/options.go +++ b/pkg/config/options.go @@ -57,6 +57,7 @@ type KanikoOptions struct { Cleanup bool IgnoreVarRun bool SkipUnusedStages bool + RunV2 bool } // WarmerOptions are options that are set by command line arguments to the cache warmer. diff --git a/pkg/executor/build.go b/pkg/executor/build.go index d05ddecaa..0ecd60962 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -62,7 +62,7 @@ type cachePusher func(*config.KanikoOptions, string, string, string) error type snapShotter interface { Init() error TakeSnapshotFS() (string, error) - TakeSnapshot([]string) (string, error) + TakeSnapshot([]string, bool) (string, error) } // stageBuilder contains all fields necessary to build one stage of a Dockerfile @@ -127,7 +127,7 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, cross } for _, cmd := range s.stage.Commands { - command, err := commands.GetCommand(cmd, opts.SrcContext) + command, err := commands.GetCommand(cmd, opts.SrcContext, opts.RunV2) if err != nil { return nil, err } @@ -382,7 +382,7 @@ func (s *stageBuilder) build() error { return errors.Wrap(err, "failed to save layer") } } else { - tarPath, err := s.takeSnapshot(files) + tarPath, err := s.takeSnapshot(files, command.ShouldDetectDeletedFiles()) if err != nil { return errors.Wrap(err, "failed to take snapshot") } @@ -416,7 +416,7 @@ func (s *stageBuilder) build() error { return nil } -func (s *stageBuilder) takeSnapshot(files []string) (string, error) { +func (s *stageBuilder) takeSnapshot(files []string, shdDelete bool) (string, error) { var snapshot string var err error @@ -426,7 +426,7 @@ func (s *stageBuilder) takeSnapshot(files []string) (string, error) { } else { // Volumes are very weird. They get snapshotted in the next command. files = append(files, util.Volumes()...) - snapshot, err = s.snapshotter.TakeSnapshot(files) + snapshot, err = s.snapshotter.TakeSnapshot(files, shdDelete) } timing.DefaultRun.Stop(t) return snapshot, err diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index dd1a377ce..5182322a1 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -1246,6 +1246,7 @@ func getCommands(dir string, cmds []instructions.Command) []commands.DockerComma cmd, err := commands.GetCommand( c, dir, + false, ) if err != nil { panic(err) diff --git a/pkg/executor/fakes.go b/pkg/executor/fakes.go index 41793af12..cc85fb17f 100644 --- a/pkg/executor/fakes.go +++ b/pkg/executor/fakes.go @@ -73,6 +73,9 @@ func (m MockDockerCommand) RequiresUnpackedFS() bool { func (m MockDockerCommand) ShouldCacheOutput() bool { return true } +func (m MockDockerCommand) ShouldDetectDeletedFiles() bool { + return false +} type MockCachedDockerCommand struct { contextFiles []string @@ -93,6 +96,9 @@ func (m MockCachedDockerCommand) ProvidesFilesToSnapshot() bool { func (m MockCachedDockerCommand) CacheCommand(image v1.Image) commands.DockerCommand { return nil } +func (m MockCachedDockerCommand) ShouldDetectDeletedFiles() bool { + return false +} func (m MockCachedDockerCommand) FilesUsedFromContext(c *v1.Config, args *dockerfile.BuildArgs) ([]string, error) { return m.contextFiles, nil } diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index f253dfbd2..06f10f7d5 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -62,7 +62,7 @@ func (s *Snapshotter) Key() (string, error) { // TakeSnapshot takes a snapshot of the specified files, avoiding directories in the ignorelist, and creates // a tarball of the changed files. Return contents of the tarball, and whether or not any files were changed -func (s *Snapshotter) TakeSnapshot(files []string) (string, error) { +func (s *Snapshotter) TakeSnapshot(files []string, shdCheckDelete bool) (string, error) { f, err := ioutil.TempFile(config.KanikoDir, "") if err != nil { return "", err @@ -92,9 +92,30 @@ func (s *Snapshotter) TakeSnapshot(files []string) (string, error) { } } + // Get whiteout paths + filesToWhiteout := []string{} + if shdCheckDelete { + existingPaths := s.l.getFlattenedPathsForWhiteOut() + foundFiles := walkFS(s.directory) + for _, file := range foundFiles { + delete(existingPaths, file) + } + // The paths left here are the ones that have been deleted in this layer. + filesToWhiteOut := []string{} + for path := range existingPaths { + // Only add the whiteout if the directory for the file still exists. + dir := filepath.Dir(path) + if _, ok := existingPaths[dir]; !ok { + if s.l.MaybeAddWhiteout(path) { + logrus.Debugf("Adding whiteout for %s", path) + filesToWhiteOut = append(filesToWhiteOut, path) + } + } + } + } t := util.NewTar(f) defer t.Close() - if err := writeToTar(t, filesToAdd, nil); err != nil { + if err := writeToTar(t, filesToAdd, filesToWhiteout); err != nil { return "", err } return f.Name(), nil @@ -133,31 +154,8 @@ func (s *Snapshotter) scanFullFilesystem() ([]string, []string, error) { s.l.Snapshot() - timer := timing.Start("Walking filesystem") - - foundPaths := make([]string, 0) - - godirwalk.Walk(s.directory, &godirwalk.Options{ - Callback: func(path string, ent *godirwalk.Dirent) error { - if util.IsInIgnoreList(path) { - if util.IsDestDir(path) { - logrus.Tracef("Skipping paths under %s, as it is a ignored directory", path) - - return filepath.SkipDir - } - - return nil - } - - foundPaths = append(foundPaths, path) - - return nil - }, - Unsorted: true, - }, - ) - timing.DefaultRun.Stop(timer) - timer = timing.Start("Resolving Paths") + foundPaths := walkFS(s.directory) + timer := timing.Start("Resolving Paths") // First handle whiteouts // Get a list of all the files that existed before this layer existingPaths := s.l.getFlattenedPathsForWhiteOut() @@ -267,3 +265,29 @@ func filesWithLinks(path string) ([]string, error) { } return []string{path, link}, nil } + +func walkFS(dir string) []string { + foundPaths := make([]string, 0) + timer := timing.Start("Walking filesystem") + godirwalk.Walk(dir, &godirwalk.Options{ + Callback: func(path string, ent *godirwalk.Dirent) error { + if util.IsInIgnoreList(path) { + if util.IsDestDir(path) { + logrus.Tracef("Skipping paths under %s, as it is a ignored directory", path) + + return filepath.SkipDir + } + + return nil + } + + foundPaths = append(foundPaths, path) + + return nil + }, + Unsorted: true, + }, + ) + timing.DefaultRun.Stop(timer) + return foundPaths +} diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index 1b03bbd4f..314ddec9f 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/pkg/snapshot/snapshot_test.go @@ -212,7 +212,7 @@ func TestSnapshotFiles(t *testing.T) { filesToSnapshot := []string{ filepath.Join(testDir, "foo"), } - tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot) + tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, false) if err != nil { t.Fatal(err) } @@ -359,7 +359,7 @@ func TestSnasphotPreservesFileOrder(t *testing.T) { } // Take a snapshot - tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot) + tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, false) if err != nil { t.Fatalf("Error taking snapshot of fs: %s", err) From 58276274e835d026e0550331bab1a08b41d5a693 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 5 Jun 2020 23:44:17 -0700 Subject: [PATCH 02/10] better improvements --- Makefile | 6 ++-- integration/benchmark_fs/cloudbuild.yaml | 4 +-- integration/benchmark_test.go | 2 +- integration/images.go | 8 +++-- integration/integration_test.go | 41 +++++++++++----------- pkg/commands/run_marker.go | 43 ++++++++++++------------ pkg/executor/build.go | 2 +- pkg/snapshot/layered_map.go | 18 +++++----- pkg/snapshot/snapshot.go | 41 ++++------------------ pkg/util/fs_util.go | 35 +++++++++++++++++-- 10 files changed, 103 insertions(+), 97 deletions(-) diff --git a/Makefile b/Makefile index 0966fe693..3296e6bd1 100644 --- a/Makefile +++ b/Makefile @@ -88,9 +88,9 @@ integration-test-misc: .PHONY: images images: - docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:latest -f deploy/Dockerfile . - docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:debug -f deploy/Dockerfile_debug . - docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/warmer:latest -f deploy/Dockerfile_warmer . + docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:perf-latest -f deploy/Dockerfile . + #docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:debug -f deploy/Dockerfile_debug . + #docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/warmer:latest -f deploy/Dockerfile_warmer . .PHONY: push push: diff --git a/integration/benchmark_fs/cloudbuild.yaml b/integration/benchmark_fs/cloudbuild.yaml index 829fb946b..e8bc93f3e 100644 --- a/integration/benchmark_fs/cloudbuild.yaml +++ b/integration/benchmark_fs/cloudbuild.yaml @@ -1,9 +1,9 @@ steps: -- name: 'gcr.io/kaniko-project/executor:latest' +- name: 'gcr.io/kaniko-project/executor:perf-latest' args: - --build-arg=NUM=${_COUNT} - --no-push - - --snapshotMode=redo + - --use-new-run=true env: - 'BENCHMARK_FILE=gs://$PROJECT_ID/gcb/benchmark_file_${_COUNT}' timeout: 2400s diff --git a/integration/benchmark_test.go b/integration/benchmark_test.go index f7364114d..af3f79eab 100644 --- a/integration/benchmark_test.go +++ b/integration/benchmark_test.go @@ -125,7 +125,7 @@ func TestSnapshotBenchmarkGcloud(t *testing.T) { } contextDir := filepath.Join(cwd, "benchmark_fs") - nums := []int{10000, 50000, 100000, 200000, 300000, 500000, 700000} + nums := []int{10000} //, 50000, 100000, 200000, 300000, 500000, 700000} var wg sync.WaitGroup fmt.Println("Number of Files,Total Build Time,Walking Filesystem, Resolving Files") diff --git a/integration/images.go b/integration/images.go index c221d6504..be2ae4546 100644 --- a/integration/images.go +++ b/integration/images.go @@ -76,6 +76,7 @@ var additionalDockerFlagsMap = map[string][]string{ var additionalKanikoFlagsMap = map[string][]string{ "Dockerfile_test_add": {"--single-snapshot"}, "Dockerfile_test_run_new": {"--use-new-run=true"}, + "Dockerfile_test_run": {"-v=debug"}, "Dockerfile_test_scratch": {"--single-snapshot"}, "Dockerfile_test_maintainer": {"--single-snapshot"}, "Dockerfile_test_target": {"--target=second"}, @@ -272,11 +273,13 @@ func (d *DockerFileBuilder) BuildImageWithContext(config *integrationTestConfig, kanikoImage := GetKanikoImage(imageRepo, dockerfile) timer = timing.Start(dockerfile + "_kaniko") - if _, err := buildKanikoImage(dockerfilesPath, dockerfile, buildArgs, additionalKanikoFlags, kanikoImage, - contextDir, gcsBucket, serviceAccount, true); err != nil { + out, err := buildKanikoImage(dockerfilesPath, dockerfile, buildArgs, additionalKanikoFlags, kanikoImage, + contextDir, gcsBucket, serviceAccount, true) + if err != nil { return err } timing.DefaultRun.Stop(timer) + fmt.Println(out) d.filesBuilt[dockerfile] = struct{}{} @@ -451,5 +454,6 @@ func buildKanikoImage( return "", fmt.Errorf("Output check failed for image %s with kaniko command : %s %s", kanikoImage, err, string(out)) } } + fmt.Println("kaniko command\n", string(out)) return benchmarkDir, nil } diff --git a/integration/integration_test.go b/integration/integration_test.go index 117ba76d7..39e4590f1 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -142,26 +142,26 @@ func buildRequiredImages() error { name: "Building kaniko image", command: []string{"docker", "build", "-t", ExecutorImage, "-f", "../deploy/Dockerfile", ".."}, }, - { - name: "Building cache warmer image", - command: []string{"docker", "build", "-t", WarmerImage, "-f", "../deploy/Dockerfile_warmer", ".."}, - }, - { - name: "Building onbuild base image", - command: []string{"docker", "build", "-t", config.onbuildBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_onbuild_base", dockerfilesPath), "."}, - }, - { - name: "Pushing onbuild base image", - command: []string{"docker", "push", config.onbuildBaseImage}, - }, - { - name: "Building hardlink base image", - command: []string{"docker", "build", "-t", config.hardlinkBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_hardlink_base", dockerfilesPath), "."}, - }, - { - name: "Pushing hardlink base image", - command: []string{"docker", "push", config.hardlinkBaseImage}, - }, + //{ + // name: "Building cache warmer image", + // command: []string{"docker", "build", "-t", WarmerImage, "-f", "../deploy/Dockerfile_warmer", ".."}, + //}, + //{ + // name: "Building onbuild base image", + // command: []string{"docker", "build", "-t", config.onbuildBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_onbuild_base", dockerfilesPath), "."}, + //}, + //{ + // name: "Pushing onbuild base image", + // command: []string{"docker", "push", config.onbuildBaseImage}, + //}, + //{ + // name: "Building hardlink base image", + // command: []string{"docker", "build", "-t", config.hardlinkBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_hardlink_base", dockerfilesPath), "."}, + //}, + //{ + // name: "Pushing hardlink base image", + // command: []string{"docker", "push", config.hardlinkBaseImage}, + //}, } for _, setupCmd := range setupCommands { @@ -294,6 +294,7 @@ func TestGitBuildcontextSubPath(t *testing.T) { kanikoCmd := exec.Command("docker", dockerRunFlags...) out, err = RunCommandWithoutTest(kanikoCmd) + fmt.Println(out) if err != nil { t.Errorf("Failed to build image %s with kaniko command %q: %v %s", dockerImage, kanikoCmd.Args, err, string(out)) } diff --git a/pkg/commands/run_marker.go b/pkg/commands/run_marker.go index 780ba0814..c468e209b 100644 --- a/pkg/commands/run_marker.go +++ b/pkg/commands/run_marker.go @@ -17,16 +17,15 @@ limitations under the License. package commands import ( + "fmt" "io/ioutil" "os" - "os/exec" - "path/filepath" - "strings" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/sirupsen/logrus" ) type RunMarkerCommand struct { @@ -37,32 +36,32 @@ type RunMarkerCommand struct { func (r *RunMarkerCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { // run command `touch filemarker` + logrus.Debugf("using new RunMarker command") markerFile, err := ioutil.TempFile("", "marker") + if err != nil { + return fmt.Errorf("could not place a marker file") + } defer func() { os.Remove(markerFile.Name()) }() - + markerInfo, err := os.Stat(markerFile.Name()) + if err != nil { + return fmt.Errorf("could not place a marker file") + } if err := runCommandInExec(config, buildArgs, r.cmd); err != nil { return err } - // run command find to find all new files generated - find := exec.Command("find", "/", "-newer", markerFile.Name()) - out, err := find.Output() - if err != nil { - r.Files = []string{} - return nil - } - - r.Files = []string{} - s := strings.Split(string(out), "\n") - for _, path := range s { - path = filepath.Clean(path) - if util.IsDestDir(path) || util.CheckIgnoreList(path) { - continue + // run command find to find all new files generate + isNewer := func(p string) (bool, error) { + fi, err := os.Stat(p) + if err != nil { + return false, err } - r.Files = append(r.Files, path) + return fi.ModTime().After(markerInfo.ModTime()), nil } + r.Files = util.WalkFS("/", isNewer) + logrus.Debugf("files changed %s", r.Files) return nil } @@ -72,11 +71,11 @@ func (r *RunMarkerCommand) String() string { } func (r *RunMarkerCommand) FilesToSnapshot() []string { - return nil + return r.Files } func (r *RunMarkerCommand) ProvidesFilesToSnapshot() bool { - return false + return true } // CacheCommand returns true since this command should be cached @@ -101,6 +100,6 @@ func (r *RunMarkerCommand) ShouldCacheOutput() bool { return true } -func (b *BaseCommand) ShouldDetectDelete() bool { +func (r *RunMarkerCommand) ShouldDetectDeletedFiles() bool { return true } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 0ecd60962..d9d00cbb0 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -319,7 +319,7 @@ func (s *stageBuilder) build() error { } initSnapshotTaken := false - if s.opts.SingleSnapshot { + if s.opts.SingleSnapshot || s.opts.RunV2 { if err := s.initSnapshotWithTimings(); err != nil { return err } diff --git a/pkg/snapshot/layered_map.go b/pkg/snapshot/layered_map.go index 0097b57a5..78377f959 100644 --- a/pkg/snapshot/layered_map.go +++ b/pkg/snapshot/layered_map.go @@ -31,7 +31,7 @@ import ( type LayeredMap struct { layers []map[string]string - whiteouts []map[string]string + whiteouts []map[string]struct{} hasher func(string) (string, error) // cacheHasher doesn't include mtime in it's hash so that filesystem cache keys are stable cacheHasher func(string) (string, error) @@ -47,7 +47,7 @@ func NewLayeredMap(h func(string) (string, error), c func(string) (string, error } func (l *LayeredMap) Snapshot() { - l.whiteouts = append(l.whiteouts, map[string]string{}) + l.whiteouts = append(l.whiteouts, map[string]struct{}{}) l.layers = append(l.layers, map[string]string{}) } @@ -82,21 +82,21 @@ func (l *LayeredMap) Get(s string) (string, bool) { return "", false } -func (l *LayeredMap) GetWhiteout(s string) (string, bool) { +func (l *LayeredMap) GetWhiteout(s string) bool { for i := len(l.whiteouts) - 1; i >= 0; i-- { - if v, ok := l.whiteouts[i][s]; ok { - return v, ok + if _, ok := l.whiteouts[i][s]; ok { + return ok } } - return "", false + return false } func (l *LayeredMap) MaybeAddWhiteout(s string) bool { - whiteout, ok := l.GetWhiteout(s) - if ok && whiteout == s { + ok := l.GetWhiteout(s) + if ok { return false } - l.whiteouts[len(l.whiteouts)-1][s] = s + l.whiteouts[len(l.whiteouts)-1][s] = struct{}{} return true } diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 06f10f7d5..c9046514e 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -24,11 +24,9 @@ import ( "sort" "syscall" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/filesystem" "github.com/GoogleContainerTools/kaniko/pkg/timing" - "github.com/karrick/godirwalk" - - "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/sirupsen/logrus" @@ -36,6 +34,7 @@ import ( // For testing var snapshotPathPrefix = config.KanikoDir +var allPass = func(s string) (bool, error) { return true, nil } // Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken type Snapshotter struct { @@ -81,7 +80,7 @@ func (s *Snapshotter) TakeSnapshot(files []string, shdCheckDelete bool) (string, } logrus.Info("Taking snapshot of files...") - logrus.Debugf("Taking snapshot of files %v", files) + logrus.Debugf("Taking snapshot of files %v", filesToAdd) sort.Strings(filesToAdd) @@ -96,23 +95,23 @@ func (s *Snapshotter) TakeSnapshot(files []string, shdCheckDelete bool) (string, filesToWhiteout := []string{} if shdCheckDelete { existingPaths := s.l.getFlattenedPathsForWhiteOut() - foundFiles := walkFS(s.directory) + foundFiles := util.WalkFS(s.directory, allPass) for _, file := range foundFiles { delete(existingPaths, file) } // The paths left here are the ones that have been deleted in this layer. - filesToWhiteOut := []string{} for path := range existingPaths { // Only add the whiteout if the directory for the file still exists. dir := filepath.Dir(path) if _, ok := existingPaths[dir]; !ok { if s.l.MaybeAddWhiteout(path) { logrus.Debugf("Adding whiteout for %s", path) - filesToWhiteOut = append(filesToWhiteOut, path) + filesToWhiteout = append(filesToWhiteout, path) } } } } + logrus.Infof("whiteouts %s", filesToWhiteout) t := util.NewTar(f) defer t.Close() if err := writeToTar(t, filesToAdd, filesToWhiteout); err != nil { @@ -154,7 +153,7 @@ func (s *Snapshotter) scanFullFilesystem() ([]string, []string, error) { s.l.Snapshot() - foundPaths := walkFS(s.directory) + foundPaths := util.WalkFS(s.directory, allPass) timer := timing.Start("Resolving Paths") // First handle whiteouts // Get a list of all the files that existed before this layer @@ -265,29 +264,3 @@ func filesWithLinks(path string) ([]string, error) { } return []string{path, link}, nil } - -func walkFS(dir string) []string { - foundPaths := make([]string, 0) - timer := timing.Start("Walking filesystem") - godirwalk.Walk(dir, &godirwalk.Options{ - Callback: func(path string, ent *godirwalk.Dirent) error { - if util.IsInIgnoreList(path) { - if util.IsDestDir(path) { - logrus.Tracef("Skipping paths under %s, as it is a ignored directory", path) - - return filepath.SkipDir - } - - return nil - } - - foundPaths = append(foundPaths, path) - - return nil - }, - Unsorted: true, - }, - ) - timing.DefaultRun.Stop(timer) - return foundPaths -} diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index d957e64be..75f1d3870 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -31,14 +31,16 @@ import ( "syscall" "time" - otiai10Cpy "github.com/otiai10/copy" - - "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/docker/docker/builder/dockerignore" "github.com/docker/docker/pkg/fileutils" v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/karrick/godirwalk" + otiai10Cpy "github.com/otiai10/copy" "github.com/pkg/errors" "github.com/sirupsen/logrus" + + "github.com/GoogleContainerTools/kaniko/pkg/config" + "github.com/GoogleContainerTools/kaniko/pkg/timing" ) const DoNotChangeUID = -1 @@ -875,3 +877,30 @@ func UpdateInitialIgnoreList(ignoreVarRun bool) { PrefixMatchOnly: false, }) } + +func WalkFS(dir string, f func(string) (bool, error)) []string { + foundPaths := make([]string, 0) + timer := timing.Start("Walking filesystem") + godirwalk.Walk(dir, &godirwalk.Options{ + Callback: func(path string, ent *godirwalk.Dirent) error { + if IsInIgnoreList(path) { + if IsDestDir(path) { + logrus.Tracef("Skipping paths under %s, as it is a ignored directory", path) + return filepath.SkipDir + } + + return nil + } + if t, err := f(path); err != nil { + return err + } else if t { + foundPaths = append(foundPaths, path) + } + return nil + }, + Unsorted: true, + }, + ) + timing.DefaultRun.Stop(timer) + return foundPaths +} From 01c3a1bcf3c9baaece812d93336f5e3cf9013934 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 5 Jun 2020 23:47:31 -0700 Subject: [PATCH 03/10] mend --- integration/images.go | 7 ++---- integration/integration_test.go | 41 ++++++++++++++++----------------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/integration/images.go b/integration/images.go index be2ae4546..42560c82d 100644 --- a/integration/images.go +++ b/integration/images.go @@ -273,13 +273,11 @@ func (d *DockerFileBuilder) BuildImageWithContext(config *integrationTestConfig, kanikoImage := GetKanikoImage(imageRepo, dockerfile) timer = timing.Start(dockerfile + "_kaniko") - out, err := buildKanikoImage(dockerfilesPath, dockerfile, buildArgs, additionalKanikoFlags, kanikoImage, - contextDir, gcsBucket, serviceAccount, true) - if err != nil { + if _, err := buildKanikoImage(dockerfilesPath, dockerfile, buildArgs, additionalKanikoFlags, kanikoImage, + contextDir, gcsBucket, serviceAccount, true); err != nil { return err } timing.DefaultRun.Stop(timer) - fmt.Println(out) d.filesBuilt[dockerfile] = struct{}{} @@ -454,6 +452,5 @@ func buildKanikoImage( return "", fmt.Errorf("Output check failed for image %s with kaniko command : %s %s", kanikoImage, err, string(out)) } } - fmt.Println("kaniko command\n", string(out)) return benchmarkDir, nil } diff --git a/integration/integration_test.go b/integration/integration_test.go index 39e4590f1..117ba76d7 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -142,26 +142,26 @@ func buildRequiredImages() error { name: "Building kaniko image", command: []string{"docker", "build", "-t", ExecutorImage, "-f", "../deploy/Dockerfile", ".."}, }, - //{ - // name: "Building cache warmer image", - // command: []string{"docker", "build", "-t", WarmerImage, "-f", "../deploy/Dockerfile_warmer", ".."}, - //}, - //{ - // name: "Building onbuild base image", - // command: []string{"docker", "build", "-t", config.onbuildBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_onbuild_base", dockerfilesPath), "."}, - //}, - //{ - // name: "Pushing onbuild base image", - // command: []string{"docker", "push", config.onbuildBaseImage}, - //}, - //{ - // name: "Building hardlink base image", - // command: []string{"docker", "build", "-t", config.hardlinkBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_hardlink_base", dockerfilesPath), "."}, - //}, - //{ - // name: "Pushing hardlink base image", - // command: []string{"docker", "push", config.hardlinkBaseImage}, - //}, + { + name: "Building cache warmer image", + command: []string{"docker", "build", "-t", WarmerImage, "-f", "../deploy/Dockerfile_warmer", ".."}, + }, + { + name: "Building onbuild base image", + command: []string{"docker", "build", "-t", config.onbuildBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_onbuild_base", dockerfilesPath), "."}, + }, + { + name: "Pushing onbuild base image", + command: []string{"docker", "push", config.onbuildBaseImage}, + }, + { + name: "Building hardlink base image", + command: []string{"docker", "build", "-t", config.hardlinkBaseImage, "-f", fmt.Sprintf("%s/Dockerfile_hardlink_base", dockerfilesPath), "."}, + }, + { + name: "Pushing hardlink base image", + command: []string{"docker", "push", config.hardlinkBaseImage}, + }, } for _, setupCmd := range setupCommands { @@ -294,7 +294,6 @@ func TestGitBuildcontextSubPath(t *testing.T) { kanikoCmd := exec.Command("docker", dockerRunFlags...) out, err = RunCommandWithoutTest(kanikoCmd) - fmt.Println(out) if err != nil { t.Errorf("Failed to build image %s with kaniko command %q: %v %s", dockerImage, kanikoCmd.Args, err, string(out)) } From 0f5387960e01447630fbc14dde7f4874a2044d86 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 6 Jun 2020 00:14:11 -0700 Subject: [PATCH 04/10] minimal changes --- Makefile | 6 +++--- integration/benchmark_fs/cloudbuild.yaml | 4 ++-- integration/benchmark_test.go | 2 +- integration/images.go | 1 - pkg/snapshot/snapshot.go | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 3296e6bd1..0966fe693 100644 --- a/Makefile +++ b/Makefile @@ -88,9 +88,9 @@ integration-test-misc: .PHONY: images images: - docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:perf-latest -f deploy/Dockerfile . - #docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:debug -f deploy/Dockerfile_debug . - #docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/warmer:latest -f deploy/Dockerfile_warmer . + docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:latest -f deploy/Dockerfile . + docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/executor:debug -f deploy/Dockerfile_debug . + docker build ${BUILD_ARG} --build-arg=GOARCH=$(GOARCH) -t $(REGISTRY)/warmer:latest -f deploy/Dockerfile_warmer . .PHONY: push push: diff --git a/integration/benchmark_fs/cloudbuild.yaml b/integration/benchmark_fs/cloudbuild.yaml index e8bc93f3e..829fb946b 100644 --- a/integration/benchmark_fs/cloudbuild.yaml +++ b/integration/benchmark_fs/cloudbuild.yaml @@ -1,9 +1,9 @@ steps: -- name: 'gcr.io/kaniko-project/executor:perf-latest' +- name: 'gcr.io/kaniko-project/executor:latest' args: - --build-arg=NUM=${_COUNT} - --no-push - - --use-new-run=true + - --snapshotMode=redo env: - 'BENCHMARK_FILE=gs://$PROJECT_ID/gcb/benchmark_file_${_COUNT}' timeout: 2400s diff --git a/integration/benchmark_test.go b/integration/benchmark_test.go index af3f79eab..f7364114d 100644 --- a/integration/benchmark_test.go +++ b/integration/benchmark_test.go @@ -125,7 +125,7 @@ func TestSnapshotBenchmarkGcloud(t *testing.T) { } contextDir := filepath.Join(cwd, "benchmark_fs") - nums := []int{10000} //, 50000, 100000, 200000, 300000, 500000, 700000} + nums := []int{10000, 50000, 100000, 200000, 300000, 500000, 700000} var wg sync.WaitGroup fmt.Println("Number of Files,Total Build Time,Walking Filesystem, Resolving Files") diff --git a/integration/images.go b/integration/images.go index 42560c82d..c221d6504 100644 --- a/integration/images.go +++ b/integration/images.go @@ -76,7 +76,6 @@ var additionalDockerFlagsMap = map[string][]string{ var additionalKanikoFlagsMap = map[string][]string{ "Dockerfile_test_add": {"--single-snapshot"}, "Dockerfile_test_run_new": {"--use-new-run=true"}, - "Dockerfile_test_run": {"-v=debug"}, "Dockerfile_test_scratch": {"--single-snapshot"}, "Dockerfile_test_maintainer": {"--single-snapshot"}, "Dockerfile_test_target": {"--target=second"}, diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index c9046514e..f61dcafea 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -27,8 +27,8 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/filesystem" "github.com/GoogleContainerTools/kaniko/pkg/timing" - "github.com/GoogleContainerTools/kaniko/pkg/util" + "github.com/sirupsen/logrus" ) From 2a7d33526be17cb0d2c299b7dcf33261e27d23aa Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 6 Jun 2020 00:30:07 -0700 Subject: [PATCH 05/10] lint --- integration/dockerfiles/Dockerfile_test_run_new | 2 +- pkg/commands/commands.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_run_new b/integration/dockerfiles/Dockerfile_test_run_new index 9891a4381..e6aa5ef31 100644 --- a/integration/dockerfiles/Dockerfile_test_run_new +++ b/integration/dockerfiles/Dockerfile_test_run_new @@ -1,4 +1,4 @@ -# Copyright 2018 Google, Inc. All rights reserved. +# Copyright 2020 Google, Inc. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index a5d431735..44139093f 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -53,7 +53,7 @@ type DockerCommand interface { ShouldCacheOutput() bool - // ShouldDetectDeletedFiles turns true if the command could delete files. + // ShouldDetectDeletedFiles returns true if the command could delete files. ShouldDetectDeletedFiles() bool } From b7462742c109a991b99bf2da9d151b3b9c7d62a1 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 6 Jun 2020 02:05:45 -0700 Subject: [PATCH 06/10] add deletion logic --- pkg/executor/build.go | 8 ++++++-- pkg/executor/build_test.go | 2 +- pkg/executor/fakes.go | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index d9d00cbb0..38080b364 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -372,7 +372,8 @@ func (s *stageBuilder) build() error { files = command.FilesToSnapshot() timing.DefaultRun.Stop(t) - if !s.shouldTakeSnapshot(index, files, command.ProvidesFilesToSnapshot()) { + if !s.shouldTakeSnapshot(index, files, command.ProvidesFilesToSnapshot(), + command.ShouldDetectDeletedFiles()) { continue } if isCacheCommand { @@ -432,7 +433,7 @@ func (s *stageBuilder) takeSnapshot(files []string, shdDelete bool) (string, err return snapshot, err } -func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFiles bool) bool { +func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFiles bool, detectDeletion bool) bool { isLastCommand := index == len(s.cmds)-1 // We only snapshot the very end with single snapshot mode on. @@ -450,6 +451,9 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFile return true } + if detectDeletion { + return true + } // Don't snapshot an empty list. if len(files) == 0 { return false diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 5182322a1..2dd0529d6 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -204,7 +204,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { opts: tt.fields.opts, cmds: tt.fields.cmds, } - if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, tt.args.hasFiles); got != tt.want { + if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, tt.args.hasFiles, false); got != tt.want { t.Errorf("stageBuilder.shouldTakeSnapshot() = %v, want %v", got, tt.want) } }) diff --git a/pkg/executor/fakes.go b/pkg/executor/fakes.go index cc85fb17f..193396f31 100644 --- a/pkg/executor/fakes.go +++ b/pkg/executor/fakes.go @@ -38,7 +38,7 @@ func (f fakeSnapShotter) Init() error { return nil } func (f fakeSnapShotter) TakeSnapshotFS() (string, error) { return f.tarPath, nil } -func (f fakeSnapShotter) TakeSnapshot(_ []string) (string, error) { +func (f fakeSnapShotter) TakeSnapshot(_ []string, _ bool) (string, error) { return f.tarPath, nil } From c85d64c8aed27b0ce22bd298659a53feeb1e1e40 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 6 Jun 2020 19:49:34 -0700 Subject: [PATCH 07/10] better shdTakeSnapshot --- pkg/commands/base_command.go | 2 +- pkg/executor/build.go | 21 ++++----------------- pkg/executor/build_test.go | 20 ++++++++++---------- pkg/snapshot/snapshot.go | 1 - 4 files changed, 15 insertions(+), 29 deletions(-) diff --git a/pkg/commands/base_command.go b/pkg/commands/base_command.go index cf19c35cb..f42e68e02 100644 --- a/pkg/commands/base_command.go +++ b/pkg/commands/base_command.go @@ -52,6 +52,6 @@ func (b *BaseCommand) ShouldCacheOutput() bool { return false } -func (a *BaseCommand) ShouldDetectDeletedFiles() bool { +func (b *BaseCommand) ShouldDetectDeletedFiles() bool { return false } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 38080b364..56dc80d31 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -372,8 +372,7 @@ func (s *stageBuilder) build() error { files = command.FilesToSnapshot() timing.DefaultRun.Stop(t) - if !s.shouldTakeSnapshot(index, files, command.ProvidesFilesToSnapshot(), - command.ShouldDetectDeletedFiles()) { + if !s.shouldTakeSnapshot(index, files, command.MetadataOnly()) { continue } if isCacheCommand { @@ -433,7 +432,7 @@ func (s *stageBuilder) takeSnapshot(files []string, shdDelete bool) (string, err return snapshot, err } -func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFiles bool, detectDeletion bool) bool { +func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, isMetadatCmd bool) bool { isLastCommand := index == len(s.cmds)-1 // We only snapshot the very end with single snapshot mode on. @@ -446,20 +445,8 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFile return true } - // if command does not provide files, snapshot everything. - if !provideFiles { - return true - } - - if detectDeletion { - return true - } - // Don't snapshot an empty list. - if len(files) == 0 { - return false - } - - return true + // if command is a metadata command, do not snapshot. + return !isMetadatCmd } func (s *stageBuilder) saveSnapshotToImage(createdBy string, tarPath string) error { diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 2dd0529d6..38bec3185 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -103,9 +103,9 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { cmds []commands.DockerCommand } type args struct { - index int - files []string - hasFiles bool + index int + files []string + metadataOnly bool } tests := []struct { name string @@ -158,9 +158,9 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { stage: config.KanikoStage{}, }, args: args{ - index: 0, - files: []string{}, - hasFiles: true, + index: 0, + files: []string{}, + metadataOnly: true, }, want: false, }, @@ -172,9 +172,9 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { }, }, args: args{ - index: 0, - files: nil, - hasFiles: false, + index: 0, + files: nil, + metadataOnly: false, }, want: true, }, @@ -204,7 +204,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { opts: tt.fields.opts, cmds: tt.fields.cmds, } - if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, tt.args.hasFiles, false); got != tt.want { + if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, !tt.args.metadataOnly); got != tt.want { t.Errorf("stageBuilder.shouldTakeSnapshot() = %v, want %v", got, tt.want) } }) diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index f61dcafea..882fa866f 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -111,7 +111,6 @@ func (s *Snapshotter) TakeSnapshot(files []string, shdCheckDelete bool) (string, } } } - logrus.Infof("whiteouts %s", filesToWhiteout) t := util.NewTar(f) defer t.Close() if err := writeToTar(t, filesToAdd, filesToWhiteout); err != nil { From 07cdfcf09163efed5c748ec35d0338143ac6bd37 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 6 Jun 2020 20:28:04 -0700 Subject: [PATCH 08/10] fix tests --- pkg/executor/build_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 38bec3185..f34d2fcc6 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -204,7 +204,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { opts: tt.fields.opts, cmds: tt.fields.cmds, } - if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, !tt.args.metadataOnly); got != tt.want { + if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, tt.args.metadataOnly); got != tt.want { t.Errorf("stageBuilder.shouldTakeSnapshot() = %v, want %v", got, tt.want) } }) From f8bf02ae3c6bb40605f9797f87a82ee7b2e27847 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 6 Jun 2020 20:43:35 -0700 Subject: [PATCH 09/10] rm benchmark --- scripts/misc-integration-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/misc-integration-test.sh b/scripts/misc-integration-test.sh index b989152b3..93f6df8d6 100755 --- a/scripts/misc-integration-test.sh +++ b/scripts/misc-integration-test.sh @@ -19,7 +19,7 @@ set -e TESTS=$(./scripts/integration-test.sh -list=Test -mod=vendor) -TESTS=$(echo $TESTS | tr ' ' '\n' | grep 'Test'| grep -v 'TestRun' | grep -v 'TestLayers' | grep -v 'TestK8s') +TESTS=$(echo $TESTS | tr ' ' '\n' | grep 'Test'| grep -v 'TestRun' | grep -v 'TestLayers' | grep -v 'TestK8s' | grep -v 'TestSnapshotBenchmark') RUN_ARG='' count=0 From 98a2ee2e34855f908e5a79274195f97811c542e0 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 6 Jun 2020 20:52:58 -0700 Subject: [PATCH 10/10] intoduce delay --- pkg/commands/run_marker.go | 3 +++ pkg/executor/build.go | 4 ++-- pkg/executor/build_test.go | 5 +---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/commands/run_marker.go b/pkg/commands/run_marker.go index a27b520a1..8d67160bf 100644 --- a/pkg/commands/run_marker.go +++ b/pkg/commands/run_marker.go @@ -20,6 +20,7 @@ import ( "fmt" "io/ioutil" "os" + "time" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" @@ -48,6 +49,8 @@ func (r *RunMarkerCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfi if err != nil { return fmt.Errorf("could not place a marker file") } + // introduce a delay + time.Sleep(time.Second) if err := runCommandInExec(config, buildArgs, r.cmd); err != nil { return err } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 799b40ee9..38d6471a5 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -372,7 +372,7 @@ func (s *stageBuilder) build() error { files = command.FilesToSnapshot() timing.DefaultRun.Stop(t) - if !s.shouldTakeSnapshot(index, files, command.MetadataOnly()) { + if !s.shouldTakeSnapshot(index, command.MetadataOnly()) { continue } if isCacheCommand { @@ -432,7 +432,7 @@ func (s *stageBuilder) takeSnapshot(files []string, shdDelete bool) (string, err return snapshot, err } -func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, isMetadatCmd bool) bool { +func (s *stageBuilder) shouldTakeSnapshot(index int, isMetadatCmd bool) bool { isLastCommand := index == len(s.cmds)-1 // We only snapshot the very end with single snapshot mode on. diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index f34d2fcc6..00631ccca 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -104,7 +104,6 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { } type args struct { index int - files []string metadataOnly bool } tests := []struct { @@ -159,7 +158,6 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { }, args: args{ index: 0, - files: []string{}, metadataOnly: true, }, want: false, @@ -173,7 +171,6 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { }, args: args{ index: 0, - files: nil, metadataOnly: false, }, want: true, @@ -204,7 +201,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) { opts: tt.fields.opts, cmds: tt.fields.cmds, } - if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, tt.args.metadataOnly); got != tt.want { + if got := s.shouldTakeSnapshot(tt.args.index, tt.args.metadataOnly); got != tt.want { t.Errorf("stageBuilder.shouldTakeSnapshot() = %v, want %v", got, tt.want) } })