From 9f75451584cd97339a37b15f840d082face060a6 Mon Sep 17 00:00:00 2001 From: dlorenc Date: Mon, 15 Oct 2018 08:17:33 -0700 Subject: [PATCH] Buffer layers to disk as they are created. When building Docker images, layers were previously stored in memory. This caused obvious issues when manipulating large layers, which could cause Kaniko to crash. --- pkg/executor/build.go | 53 ++++++++++++--------- pkg/snapshot/layered_map.go | 6 +-- pkg/snapshot/snapshot.go | 88 ++++++++++++---------------------- pkg/snapshot/snapshot_test.go | 89 ++++++++++++++++++++++------------- pkg/util/fs_util.go | 2 +- 5 files changed, 120 insertions(+), 118 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index d0a7d932d1..ca155361c2 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -17,10 +17,7 @@ limitations under the License. package executor import ( - "bytes" "fmt" - "io" - "io/ioutil" "os" "path/filepath" "strconv" @@ -42,6 +39,9 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/util" ) +// This is the size of an empty tar in Go +const emptyTarSize = 1024 + // stageBuilder contains all fields necessary to build one stage of a Dockerfile type stageBuilder struct { stage config.KanikoStage @@ -160,36 +160,39 @@ func (s *stageBuilder) build() error { return err } files = command.FilesToSnapshot() - var contents []byte if !s.shouldTakeSnapshot(index, files) { continue } - if files == nil || s.opts.SingleSnapshot { - contents, err = s.snapshotter.TakeSnapshotFS() - } else { - // Volumes are very weird. They get created in their command, but snapshotted in the next one. - // Add them to the list of files to snapshot. - for v := range s.cf.Config.Volumes { - files = append(files, v) - } - contents, err = s.snapshotter.TakeSnapshot(files) - } + tarPath, err := s.takeSnapshot(files) if err != nil { return err } + ck, err := compositeKey.Hash() if err != nil { return err } - if err := s.saveSnapshot(command.String(), ck, contents); err != nil { + if err := s.saveSnapshotToImage(command.String(), ck, tarPath); err != nil { return err } } return nil } +func (s *stageBuilder) takeSnapshot(files []string) (string, error) { + if files == nil || s.opts.SingleSnapshot { + return s.snapshotter.TakeSnapshotFS() + } + // Volumes are very weird. They get created in their command, but snapshotted in the next one. + // Add them to the list of files to snapshot. + for v := range s.cf.Config.Volumes { + files = append(files, v) + } + return s.snapshotter.TakeSnapshot(files) +} + func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool { isLastCommand := index == len(s.stage.Commands)-1 @@ -215,16 +218,20 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool { return true } -func (s *stageBuilder) saveSnapshot(createdBy string, ck string, contents []byte) error { - if contents == nil { - logrus.Info("No files were changed, appending empty layer to config. No layer added to image.") +func (s *stageBuilder) saveSnapshotToImage(createdBy string, ck string, tarPath string) error { + if tarPath == "" { return nil } - // Append the layer to the image - opener := func() (io.ReadCloser, error) { - return ioutil.NopCloser(bytes.NewReader(contents)), nil + fi, err := os.Stat(tarPath) + if err != nil { + return err } - layer, err := tarball.LayerFromOpener(opener) + if fi.Size() <= emptyTarSize { + logrus.Info("No files were changed, appending empty layer to config. No layer added to image.") + return nil + } + + layer, err := tarball.LayerFromFile(tarPath) if err != nil { return err } @@ -306,7 +313,7 @@ func extractImageToDependecyDir(index int, image v1.Image) error { if err := os.MkdirAll(dependencyDir, 0755); err != nil { return err } - logrus.Infof("trying to extract to %s", dependencyDir) + logrus.Debugf("trying to extract to %s", dependencyDir) _, err := util.GetFSFromImage(dependencyDir, image) return err } diff --git a/pkg/snapshot/layered_map.go b/pkg/snapshot/layered_map.go index 9a356b0e22..c84340e363 100644 --- a/pkg/snapshot/layered_map.go +++ b/pkg/snapshot/layered_map.go @@ -92,13 +92,13 @@ func (l *LayeredMap) GetWhiteout(s string) (string, bool) { return "", false } -func (l *LayeredMap) MaybeAddWhiteout(s string) (bool, error) { +func (l *LayeredMap) MaybeAddWhiteout(s string) bool { whiteout, ok := l.GetWhiteout(s) if ok && whiteout == s { - return false, nil + return false } l.whiteouts[len(l.whiteouts)-1][s] = s - return true, nil + return true } // Add will add the specified file s to the layered map. diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 4175d47610..84c62316e0 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -17,9 +17,7 @@ limitations under the License. package snapshot import ( - "bytes" "fmt" - "io" "io/ioutil" "os" "path/filepath" @@ -29,6 +27,8 @@ import ( "github.com/sirupsen/logrus" ) +var snapshotPathPrefix = "/kaniko" + // Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken type Snapshotter struct { l *LayeredMap @@ -42,10 +42,8 @@ func NewSnapshotter(l *LayeredMap, d string) *Snapshotter { // Init initializes a new snapshotter func (s *Snapshotter) Init() error { - if _, err := s.snapShotFS(ioutil.Discard); err != nil { - return err - } - return nil + _, err := s.TakeSnapshotFS() + return err } // Key returns a string based on the current state of the file system @@ -55,46 +53,21 @@ func (s *Snapshotter) Key() (string, error) { // TakeSnapshot takes a snapshot of the specified files, avoiding directories in the whitelist, 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) ([]byte, error) { - buf := bytes.NewBuffer([]byte{}) - filesAdded, err := s.snapshotFiles(buf, files) +func (s *Snapshotter) TakeSnapshot(files []string) (string, error) { + f, err := ioutil.TempFile(snapshotPathPrefix, "") if err != nil { - return nil, err - } - contents := buf.Bytes() - if !filesAdded { - return nil, nil + return "", err } - return contents, err -} + defer f.Close() -// TakeSnapshotFS takes a snapshot of the filesystem, avoiding directories in the whitelist, and creates -// a tarball of the changed files. Return contents of the tarball, and whether or not any files were changed -func (s *Snapshotter) TakeSnapshotFS() ([]byte, error) { - buf := bytes.NewBuffer([]byte{}) - filesAdded, err := s.snapShotFS(buf) - if err != nil { - return nil, err - } - contents := buf.Bytes() - if !filesAdded { - return nil, nil - } - return contents, err -} - -// snapshotFiles creates a snapshot (tar) and adds the specified files. -// It will not add files which are whitelisted. -func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) { s.l.Snapshot() if len(files) == 0 { logrus.Info("No files changed in this command, skipping snapshotting.") - return false, nil + return "", nil } logrus.Info("Taking snapshot of files...") logrus.Debugf("Taking snapshot of files %v", files) snapshottedFiles := make(map[string]bool) - filesAdded := false t := util.NewTar(f) defer t.Close() @@ -114,15 +87,14 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) { fileAdded, err := s.l.MaybeAdd(file) if err != nil { - return false, fmt.Errorf("Unable to add parent dir %s to layered map: %s", file, err) + return "", fmt.Errorf("Unable to add parent dir %s to layered map: %s", file, err) } if fileAdded { err = t.AddFileToTar(file) if err != nil { - return false, fmt.Errorf("Error adding parent dir %s to tar: %s", file, err) + return "", fmt.Errorf("Error adding parent dir %s to tar: %s", file, err) } - filesAdded = true } } // Next add the files themselves to the tar @@ -134,21 +106,26 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) { snapshottedFiles[file] = true if err := s.l.Add(file); err != nil { - return false, fmt.Errorf("Unable to add file %s to layered map: %s", file, err) + return "", fmt.Errorf("Unable to add file %s to layered map: %s", file, err) } if err := t.AddFileToTar(file); err != nil { - return false, fmt.Errorf("Error adding file %s to tar: %s", file, err) + return "", fmt.Errorf("Error adding file %s to tar: %s", file, err) } - filesAdded = true } - return filesAdded, nil + return f.Name(), nil } -// shapShotFS creates a snapshot (tar) of all files in the system which are not -// whitelisted and which have changed. -func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { +// TakeSnapshotFS takes a snapshot of the filesystem, avoiding directories in the whitelist, and creates +// a tarball of the changed files. +func (s *Snapshotter) TakeSnapshotFS() (string, error) { logrus.Info("Taking snapshot of full filesystem...") + f, err := ioutil.TempFile(snapshotPathPrefix, "") + if err != nil { + return "", err + } + defer f.Close() + // Some of the operations that follow (e.g. hashing) depend on the file system being synced, // for example the hashing function that determines if files are equal uses the mtime of the files, // which can lag if sync is not called. Unfortunately there can still be lag if too much data needs @@ -157,7 +134,6 @@ func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { s.l.Snapshot() existingPaths := s.l.GetFlattenedPathsForWhiteOut() - filesAdded := false t := util.NewTar(f) defer t.Close() @@ -176,15 +152,10 @@ func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { // Only add the whiteout if the directory for the file still exists. dir := filepath.Dir(path) if _, ok := memFs[dir]; ok { - addWhiteout, err := s.l.MaybeAddWhiteout(path) - if err != nil { - return false, nil - } - if addWhiteout { + if s.l.MaybeAddWhiteout(path) { logrus.Infof("Adding whiteout for %s", path) - filesAdded = true if err := t.Whiteout(path); err != nil { - return false, err + return "", err } } } @@ -194,7 +165,7 @@ func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { for path := range memFs { whitelisted, err := util.CheckWhitelist(path) if err != nil { - return false, err + return "", err } if whitelisted { logrus.Debugf("Not adding %s to layer, as it's whitelisted", path) @@ -204,16 +175,15 @@ func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { // Only add to the tar if we add it to the layeredmap. maybeAdd, err := s.l.MaybeAdd(path) if err != nil { - return false, err + return "", err } if maybeAdd { logrus.Debugf("Adding %s to layer, because it was changed.", path) - filesAdded = true if err := t.AddFileToTar(path); err != nil { - return false, err + return "", err } } } - return filesAdded, nil + return f.Name(), nil } diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index 95daa88f63..11c63c7915 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/pkg/snapshot/snapshot_test.go @@ -17,7 +17,6 @@ package snapshot import ( "archive/tar" - "bytes" "io" "io/ioutil" "os" @@ -30,9 +29,8 @@ import ( ) func TestSnapshotFSFileChange(t *testing.T) { - - testDir, snapshotter, err := setUpTestDir() - defer os.RemoveAll(testDir) + testDir, snapshotter, cleanup, err := setUpTestDir() + defer cleanup() if err != nil { t.Fatal(err) } @@ -45,16 +43,17 @@ func TestSnapshotFSFileChange(t *testing.T) { t.Fatalf("Error setting up fs: %s", err) } // Take another snapshot - contents, err := snapshotter.TakeSnapshotFS() + tarPath, err := snapshotter.TakeSnapshotFS() if err != nil { t.Fatalf("Error taking snapshot of fs: %s", err) } - if contents == nil { - t.Fatal("No files added to snapshot.") + + f, err := os.Open(tarPath) + if err != nil { + t.Fatal(err) } // Check contents of the snapshot, make sure contents is equivalent to snapshotFiles - reader := bytes.NewReader(contents) - tr := tar.NewReader(reader) + tr := tar.NewReader(f) fooPath := filepath.Join(testDir, "foo") batPath := filepath.Join(testDir, "bar/bat") snapshotFiles := map[string]string{ @@ -82,8 +81,8 @@ func TestSnapshotFSFileChange(t *testing.T) { } func TestSnapshotFSChangePermissions(t *testing.T) { - testDir, snapshotter, err := setUpTestDir() - defer os.RemoveAll(testDir) + testDir, snapshotter, cleanup, err := setUpTestDir() + defer cleanup() if err != nil { t.Fatal(err) } @@ -93,16 +92,16 @@ func TestSnapshotFSChangePermissions(t *testing.T) { t.Fatalf("Error changing permissions on %s: %v", batPath, err) } // Take another snapshot - contents, err := snapshotter.TakeSnapshotFS() + tarPath, err := snapshotter.TakeSnapshotFS() if err != nil { t.Fatalf("Error taking snapshot of fs: %s", err) } - if contents == nil { - t.Fatal("No files added to snapshot.") + f, err := os.Open(tarPath) + if err != nil { + t.Fatal(err) } // Check contents of the snapshot, make sure contents is equivalent to snapshotFiles - reader := bytes.NewReader(contents) - tr := tar.NewReader(reader) + tr := tar.NewReader(f) snapshotFiles := map[string]string{ batPath: "baz2", } @@ -127,8 +126,8 @@ func TestSnapshotFSChangePermissions(t *testing.T) { } func TestSnapshotFiles(t *testing.T) { - testDir, snapshotter, err := setUpTestDir() - defer os.RemoveAll(testDir) + testDir, snapshotter, cleanup, err := setUpTestDir() + defer cleanup() if err != nil { t.Fatal(err) } @@ -142,15 +141,20 @@ func TestSnapshotFiles(t *testing.T) { filesToSnapshot := []string{ filepath.Join(testDir, "foo"), } - contents, err := snapshotter.TakeSnapshot(filesToSnapshot) + tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot) if err != nil { t.Fatal(err) } + defer os.Remove(tarPath) + expectedFiles := []string{"/", "/tmp", filepath.Join(testDir, "foo")} + f, err := os.Open(tarPath) + if err != nil { + t.Fatal(err) + } // Check contents of the snapshot, make sure contents is equivalent to snapshotFiles - reader := bytes.NewReader(contents) - tr := tar.NewReader(reader) + tr := tar.NewReader(f) var actualFiles []string for { hdr, err := tr.Next() @@ -166,27 +170,42 @@ func TestSnapshotFiles(t *testing.T) { } func TestEmptySnapshotFS(t *testing.T) { - testDir, snapshotter, err := setUpTestDir() - defer os.RemoveAll(testDir) + _, snapshotter, cleanup, err := setUpTestDir() if err != nil { t.Fatal(err) } + defer cleanup() + // Take snapshot with no changes - contents, err := snapshotter.TakeSnapshotFS() + tarPath, err := snapshotter.TakeSnapshotFS() if err != nil { t.Fatalf("Error taking snapshot of fs: %s", err) } - // Since we took a snapshot with no changes, contents should be nil - if contents != nil { - t.Fatal("Files added even though no changes to file system were made.") + + f, err := os.Open(tarPath) + if err != nil { + t.Fatal(err) + } + tr := tar.NewReader(f) + + if _, err := tr.Next(); err != io.EOF { + t.Fatal("no files expected in tar, found files.") } } -func setUpTestDir() (string, *Snapshotter, error) { +func setUpTestDir() (string, *Snapshotter, func(), error) { testDir, err := ioutil.TempDir("", "") if err != nil { - return testDir, nil, errors.Wrap(err, "setting up temp dir") + return "", nil, nil, errors.Wrap(err, "setting up temp dir") + } + + snapshotPath, err := ioutil.TempDir("", "") + if err != nil { + return "", nil, nil, errors.Wrap(err, "setting up temp dir") } + + snapshotPathPrefix = snapshotPath + files := map[string]string{ "foo": "baz1", "bar/bat": "baz2", @@ -194,14 +213,20 @@ func setUpTestDir() (string, *Snapshotter, error) { } // Set up initial files if err := testutil.SetupFiles(testDir, files); err != nil { - return testDir, nil, errors.Wrap(err, "setting up file system") + return "", nil, nil, errors.Wrap(err, "setting up file system") } // Take the initial snapshot l := NewLayeredMap(util.Hasher(), util.CacheHasher()) snapshotter := NewSnapshotter(l, testDir) if err := snapshotter.Init(); err != nil { - return testDir, nil, errors.Wrap(err, "initializing snapshotter") + return "", nil, nil, errors.Wrap(err, "initializing snapshotter") } - return testDir, snapshotter, nil + + cleanup := func() { + os.RemoveAll(snapshotPath) + os.RemoveAll(testDir) + } + + return testDir, snapshotter, cleanup, nil } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index ef16ceb576..fd53ee7bf1 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -74,7 +74,7 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) { extractedFiles := []string{} for i, l := range layers { - logrus.Infof("Extracting layer %d", i) + logrus.Debugf("Extracting layer %d", i) r, err := l.Uncompressed() if err != nil { return nil, err