Buffer layers to disk as they are created. (#428)

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

@@ -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 {
+func (s *stageBuilder) saveSnapshotToImage(createdBy string, ck string, tarPath string) error {
+	if tarPath == "" {
+		return nil
+	}
+	fi, err := os.Stat(tarPath)
+	if err != nil {
+		return err
+	}
+	if fi.Size() <= emptyTarSize {
 		logrus.Info("No files were changed, appending empty layer to config. No layer added to image.")
 		return nil
 	}
-	// Append the layer to the image
-	opener := func() (io.ReadCloser, error) {
-		return ioutil.NopCloser(bytes.NewReader(contents)), nil
-	}
-	layer, err := tarball.LayerFromOpener(opener)
+
+	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
 }

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.

pkg/snapshot/snapshot.go

@@ -17,18 +17,21 @@ limitations under the License.
 package snapshot
 
 import (
-	"bytes"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"syscall"
 
+	"github.com/GoogleContainerTools/kaniko/pkg/constants"
+
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
 	"github.com/sirupsen/logrus"
 )
 
+// For testing
+var snapshotPathPrefix = constants.KanikoDir
+
 // Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken
 type Snapshotter struct {
 	l         *LayeredMap
@@ -42,10 +45,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 +56,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
+		return "", err
 	}
-	contents := buf.Bytes()
-	if !filesAdded {
-		return nil, nil
-	}
-	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 +90,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 +109,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 +137,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 +155,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 +168,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 +178,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
 }

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
 }

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