From c85d64c8aed27b0ce22bd298659a53feeb1e1e40 Mon Sep 17 00:00:00 2001
From: Tejal Desai <tejal29@gmail.com>
Date: Sat, 6 Jun 2020 19:49:34 -0700
Subject: [PATCH] better shdTakeSnapshot

---
 pkg/commands/base_command.go |  2 +-
 pkg/executor/build.go        | 21 ++++-----------------
 pkg/executor/build_test.go   | 20 ++++++++++----------
 pkg/snapshot/snapshot.go     |  1 -
 4 files changed, 15 insertions(+), 29 deletions(-)

diff --git a/pkg/commands/base_command.go b/pkg/commands/base_command.go
index cf19c35cb..f42e68e02 100644
--- a/pkg/commands/base_command.go
+++ b/pkg/commands/base_command.go
@@ -52,6 +52,6 @@ func (b *BaseCommand) ShouldCacheOutput() bool {
 	return false
 }
 
-func (a *BaseCommand) ShouldDetectDeletedFiles() bool {
+func (b *BaseCommand) ShouldDetectDeletedFiles() bool {
 	return false
 }
diff --git a/pkg/executor/build.go b/pkg/executor/build.go
index 38080b364..56dc80d31 100644
--- a/pkg/executor/build.go
+++ b/pkg/executor/build.go
@@ -372,8 +372,7 @@ func (s *stageBuilder) build() error {
 		files = command.FilesToSnapshot()
 		timing.DefaultRun.Stop(t)
 
-		if !s.shouldTakeSnapshot(index, files, command.ProvidesFilesToSnapshot(),
-			command.ShouldDetectDeletedFiles()) {
+		if !s.shouldTakeSnapshot(index, files, command.MetadataOnly()) {
 			continue
 		}
 		if isCacheCommand {
@@ -433,7 +432,7 @@ func (s *stageBuilder) takeSnapshot(files []string, shdDelete bool) (string, err
 	return snapshot, err
 }
 
-func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFiles bool, detectDeletion bool) bool {
+func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, isMetadatCmd bool) bool {
 	isLastCommand := index == len(s.cmds)-1
 
 	// We only snapshot the very end with single snapshot mode on.
@@ -446,20 +445,8 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFile
 		return true
 	}
 
-	// if command does not provide files, snapshot everything.
-	if !provideFiles {
-		return true
-	}
-
-	if detectDeletion {
-		return true
-	}
-	// Don't snapshot an empty list.
-	if len(files) == 0 {
-		return false
-	}
-
-	return true
+	// if command is a metadata command, do not snapshot.
+	return !isMetadatCmd
 }
 
 func (s *stageBuilder) saveSnapshotToImage(createdBy string, tarPath string) error {
diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go
index 2dd0529d6..38bec3185 100644
--- a/pkg/executor/build_test.go
+++ b/pkg/executor/build_test.go
@@ -103,9 +103,9 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 		cmds  []commands.DockerCommand
 	}
 	type args struct {
-		index    int
-		files    []string
-		hasFiles bool
+		index        int
+		files        []string
+		metadataOnly bool
 	}
 	tests := []struct {
 		name   string
@@ -158,9 +158,9 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 				stage: config.KanikoStage{},
 			},
 			args: args{
-				index:    0,
-				files:    []string{},
-				hasFiles: true,
+				index:        0,
+				files:        []string{},
+				metadataOnly: true,
 			},
 			want: false,
 		},
@@ -172,9 +172,9 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 				},
 			},
 			args: args{
-				index:    0,
-				files:    nil,
-				hasFiles: false,
+				index:        0,
+				files:        nil,
+				metadataOnly: false,
 			},
 			want: true,
 		},
@@ -204,7 +204,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 				opts:  tt.fields.opts,
 				cmds:  tt.fields.cmds,
 			}
-			if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, tt.args.hasFiles, false); got != tt.want {
+			if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, !tt.args.metadataOnly); got != tt.want {
 				t.Errorf("stageBuilder.shouldTakeSnapshot() = %v, want %v", got, tt.want)
 			}
 		})
diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go
index f61dcafea..882fa866f 100644
--- a/pkg/snapshot/snapshot.go
+++ b/pkg/snapshot/snapshot.go
@@ -111,7 +111,6 @@ func (s *Snapshotter) TakeSnapshot(files []string, shdCheckDelete bool) (string,
 			}
 		}
 	}
-	logrus.Infof("whiteouts %s", filesToWhiteout)
 	t := util.NewTar(f)
 	defer t.Close()
 	if err := writeToTar(t, filesToAdd, filesToWhiteout); err != nil {