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