Skip to content

Commit

Permalink
better improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
tejal29 committed Jun 6, 2020
1 parent 43338d4 commit 5827627
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 97 deletions.
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions integration/benchmark_fs/cloudbuild.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion integration/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 6 additions & 2 deletions integration/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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{}{}

Expand Down Expand Up @@ -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
}
41 changes: 21 additions & 20 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}
Expand Down
43 changes: 21 additions & 22 deletions pkg/commands/run_marker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand All @@ -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
Expand All @@ -101,6 +100,6 @@ func (r *RunMarkerCommand) ShouldCacheOutput() bool {
return true
}

func (b *BaseCommand) ShouldDetectDelete() bool {
func (r *RunMarkerCommand) ShouldDetectDeletedFiles() bool {
return true
}
2 changes: 1 addition & 1 deletion pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
18 changes: 9 additions & 9 deletions pkg/snapshot/layered_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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{})
}

Expand Down Expand Up @@ -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
}

Expand Down
41 changes: 7 additions & 34 deletions pkg/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,17 @@ 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"
)

// 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 {
Expand Down Expand Up @@ -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)

Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

0 comments on commit 5827627

Please sign in to comment.