fix: getFlattenedPaths

- Take only `whiteouts` into account.
- Fix ugly delete behavior of `layerHashCache`.
  Delete it when crerating a new snapshot.
- Slight cleanup in `snapshot.go`.

pkg/snapshot/layered_map.go

@@ -20,14 +20,12 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/timing"
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/sirupsen/logrus"
 )
 
 type LayeredMap struct {
@@ -52,6 +50,7 @@ func NewLayeredMap(h func(string) (string, error), c func(string) (string, error
 func (l *LayeredMap) Snapshot() {
 	l.whiteouts = append(l.whiteouts, map[string]struct{}{})
 	l.layers = append(l.layers, map[string]string{})
+	l.layerHashCache = map[string]string{} // Erase the hash cache for this new layer.
 }
 
 // Key returns a hash for added files
@@ -65,26 +64,28 @@ func (l *LayeredMap) Key() (string, error) {
 // getFlattenedPaths returns all existing paths in the current FS
 func (l *LayeredMap) getFlattenedPaths() map[string]struct{} {
 	paths := map[string]struct{}{}
-	for _, l := range l.layers {
 
-		// Keep track of deleted files.
-		deleted := map[string]struct{}{}
+	// Adding and deleting files over all layers.
+	for i := 0; i < len(l.layers); i++ {
 
-		for p := range l {
-			basename := filepath.Base(p)
+		addedFiles := l.layers[i]
+		deletedFiles := l.whiteouts[i]
+
+		for add := range addedFiles {
+			// No whiteouts should be here!!!
+			basename := filepath.Base(add)
 			if strings.HasPrefix(basename, archive.WhiteoutPrefix) {
-				deletedFile := filepath.Join(filepath.Dir(p), strings.TrimPrefix(basename, archive.WhiteoutPrefix))
-
-				delete(paths, deletedFile)
-				deleted[deletedFile] = struct{}{}
-			} else {
-				// Only add files which are not deleted already.
-				if _, isDeleted := deleted[p]; !isDeleted {
-					paths[p] = struct{}{}
-				}
+				panic("WTF")
 			}
+
+			paths[add] = struct{}{}
+		}
+
+		for del := range deletedFiles {
+			delete(paths, del)
 		}
 	}
+
 	return paths
 }
 
@@ -97,58 +98,45 @@ func (l *LayeredMap) Get(s string) (string, bool) {
 	return "", false
 }
 
-func (l *LayeredMap) GetWhiteout(s string) bool {
-	for i := len(l.whiteouts) - 1; i >= 0; i-- {
-		if _, ok := l.whiteouts[i][s]; ok {
-			return ok
-		}
-	}
-	return false
-}
-
-func (l *LayeredMap) MaybeAddWhiteout(s string) bool {
-	ok := l.GetWhiteout(s)
-	if ok {
-		return false
-	}
+// AddWhiteout will delete the specific files in the current layer.
+func (l *LayeredMap) AddWhiteout(s string) error {
 	l.whiteouts[len(l.whiteouts)-1][s] = struct{}{}
-	return true
+	return nil
 }
 
-// Add will add the specified file s to the layered map.
+// Add will add the specified file s to the current layer.
 func (l *LayeredMap) Add(s string) error {
 	// Use hash function and add to layers
 	newV, err := func(s string) (string, error) {
 		if v, ok := l.layerHashCache[s]; ok {
-			// clear it cache for next layer.
-			delete(l.layerHashCache, s)
 			return v, nil
 		}
 		return l.hasher(s)
 	}(s)
+
 	if err != nil {
 		return fmt.Errorf("error creating hash for %s: %v", s, err)
 	}
+
 	l.layers[len(l.layers)-1][s] = newV
 	return nil
 }
 
 // CheckFileChange checks whether a given file changed
 // from the current layered map by its hashing function.
+// If the file does not exist, an error is returned.
 // Returns true if the file is changed.
 func (l *LayeredMap) CheckFileChange(s string) (bool, error) {
 	t := timing.Start("Hashing files")
 	defer timing.DefaultRun.Stop(t)
+
 	newV, err := l.hasher(s)
 	if err != nil {
-		// if this file does not exist in the new layer return.
-		if os.IsNotExist(err) {
-			logrus.Tracef("%s detected as changed but does not exist", s)
-			return false, nil
-		}
 		return false, err
 	}
+
 	l.layerHashCache[s] = newV
+
 	oldV, ok := l.Get(s)
 	if ok && newV == oldV {
 		return false, nil

pkg/snapshot/layered_map_test.go

@@ -83,22 +83,33 @@ func Test_CacheKey(t *testing.T) {
 func Test_FlattenPaths(t *testing.T) {
 	layers := []map[string]string{
 		{
-			archive.WhiteoutPrefix + "a": "1", // delete a
-			"a":                          "2",
-			"b":                          "3",
+			"a": "2",
+			"b": "3",
 		},
 		{
-			archive.WhiteoutPrefix + "b": "4", // delete b
-			"b":                          "5",
-			"c":                          "6",
+			"b": "5",
+			"c": "6",
 		},
 		{
-			archive.WhiteoutPrefix + "c": "7", // delete c
-			"a":                          "8",
+			"a": "8",
 		},
 	}
 
-	lm := LayeredMap{layers: []map[string]string{layers[0]}}
+	whiteouts := []map[string]struct{}{
+		{
+			"a": {}, // delete a
+		},
+		{
+			"b": {}, // delete b
+		},
+		{
+			"c": {}, // delete c
+		},
+	}
+
+	lm := LayeredMap{
+		layers:    []map[string]string{layers[0]},
+		whiteouts: []map[string]struct{}{whiteouts[0]}}
 	paths := lm.getFlattenedPaths()
 
 	assertPath := func(f string, exists bool) {
@@ -114,14 +125,18 @@ func Test_FlattenPaths(t *testing.T) {
 	assertPath(archive.WhiteoutPrefix+"a", false)
 	assertPath("b", true)
 
-	lm = LayeredMap{layers: []map[string]string{layers[0], layers[1]}}
+	lm = LayeredMap{
+		layers:    []map[string]string{layers[0], layers[1]},
+		whiteouts: []map[string]struct{}{whiteouts[0], whiteouts[1]}}
 	paths = lm.getFlattenedPaths()
 
 	assertPath("a", false)
 	assertPath("b", false)
 	assertPath("c", true)
 
-	lm = LayeredMap{layers: []map[string]string{layers[0], layers[1], layers[2]}}
+	lm = LayeredMap{
+		layers:    []map[string]string{layers[0], layers[1], layers[2]},
+		whiteouts: []map[string]struct{}{whiteouts[0], whiteouts[1], whiteouts[2]}}
 	paths = lm.getFlattenedPaths()
 
 	assertPath("a", true)

pkg/snapshot/snapshot.go

@@ -79,38 +79,36 @@ func (s *Snapshotter) TakeSnapshot(files []string, shdCheckDelete bool, forceBui
 	}
 
 	logrus.Info("Taking snapshot of files...")
-	logrus.Debugf("Taking snapshot of files %v", filesToAdd)
 
 	sort.Strings(filesToAdd)
+	logrus.Debugf("Adding to layer: %v", filesToAdd)
 
-	// Add files to the layered map
+	// Add files to current layer.
 	for _, file := range filesToAdd {
 		if err := s.l.Add(file); err != nil {
 			return "", fmt.Errorf("unable to add file %s to layered map: %s", file, err)
 		}
 	}
 
 	// Get whiteout paths
-	filesToWhiteout := []string{}
+	var filesToWhiteout []string
 	if shdCheckDelete {
 		_, deletedFiles := util.WalkFS(s.directory, s.l.getFlattenedPaths(), func(s string) (bool, error) {
 			return true, nil
 		})
-		// The paths left here are the ones that have been deleted in this layer.
-		for path := range deletedFiles {
-			// Only add the whiteout if the directory for the file still exists.
-			dir := filepath.Dir(path)
-			if _, ok := deletedFiles[dir]; !ok {
-				if s.l.MaybeAddWhiteout(path) {
-					logrus.Debugf("Adding whiteout for %s", path)
-					filesToWhiteout = append(filesToWhiteout, path)
-				}
+
+		filesToWhiteout = filterWhiteouts(deletedFiles)
+		sort.Strings(filesToWhiteout)
+
+		logrus.Debugf("Deleting in layer: %v", filesToWhiteout)
+		// Whiteout files in current layer.
+		for _, file := range filesToWhiteout {
+			if err := s.l.AddWhiteout(file); err != nil {
+				return "", fmt.Errorf("unable to whiteout file %s in layered map: %s", file, err)
 			}
 		}
 	}
 
-	sort.Strings(filesToWhiteout)
-
 	t := util.NewTar(f)
 	defer t.Close()
 	if err := writeToTar(t, filesToAdd, filesToWhiteout); err != nil {
@@ -175,35 +173,49 @@ func (s *Snapshotter) scanFullFilesystem() ([]string, []string, error) {
 		filesToAdd = append(filesToAdd, path)
 	}
 
-	// The paths left here are the ones that have been deleted in this layer.
-	filesToWhiteOut := []string{}
-	for path := range deletedPaths {
-		// Only add the whiteout if the directory for the file still exists.
-		dir := filepath.Dir(path)
-		if _, ok := deletedPaths[dir]; !ok {
-			if s.l.MaybeAddWhiteout(path) {
-				logrus.Debugf("Adding whiteout for %s", path)
-				filesToWhiteOut = append(filesToWhiteOut, path)
-			}
-		}
-	}
+	filesToWhiteout := filterWhiteouts(deletedPaths)
 	timing.DefaultRun.Stop(timer)
 
 	sort.Strings(filesToAdd)
-	sort.Strings(filesToWhiteOut)
+	sort.Strings(filesToWhiteout)
+
+	logrus.Debugf("Adding to layer: %v", filesToAdd)
+	logrus.Debugf("Deleting in layer: %v", filesToWhiteout)
 
 	// Add files to the layered map
 	for _, file := range filesToAdd {
 		if err := s.l.Add(file); err != nil {
 			return nil, nil, fmt.Errorf("unable to add file %s to layered map: %s", file, err)
 		}
 	}
-	return filesToAdd, filesToWhiteOut, nil
+	for _, file := range filesToWhiteout {
+		if err := s.l.AddWhiteout(file); err != nil {
+			return nil, nil, fmt.Errorf("unable to whiteout file %s in layered map: %s", file, err)
+		}
+	}
+
+	return filesToAdd, filesToWhiteout, nil
+}
+
+// filterWhiteouts filters deleted files according to their parents delete status.
+func filterWhiteouts(deletedFiles map[string]struct{}) (filesToWhiteout []string) {
+
+	for path := range deletedFiles {
+		// Only add the whiteout if the directory for the file still exists.
+		dir := filepath.Dir(path)
+		if _, ok := deletedFiles[dir]; !ok {
+			logrus.Debugf("Adding whiteout for %s", path)
+			filesToWhiteout = append(filesToWhiteout, path)
+		}
+	}
+
+	return filesToWhiteout
 }
 
 func writeToTar(t util.Tar, files, whiteouts []string) error {
 	timer := timing.Start("Writing tar file")
 	defer timing.DefaultRun.Stop(timer)
+
 	// Now create the tar.
 	for _, path := range whiteouts {
 		if err := t.Whiteout(path); err != nil {