fix a bug where file check nil is insufficient

GoogleContainerTools · May 2, 2020 · 7d32139 · 7d32139
1 parent 32e3336
commit 7d32139
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 10 deletions.
diff --git a/pkg/executor/build.go b/pkg/executor/build.go
@@ -367,7 +367,7 @@ func (s *stageBuilder) build() error {
 		files = command.FilesToSnapshot()
 		timing.DefaultRun.Stop(t)
 
-		if !s.shouldTakeSnapshot(index, files) {
+		if !s.shouldTakeSnapshot(index, files, command.ProvidesFilesToSnapshot()) {
 			continue
 		}
 
@@ -425,7 +425,7 @@ func (s *stageBuilder) takeSnapshot(files []string) (string, error) {
 	return snapshot, err
 }
 
-func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool {
+func (s *stageBuilder) shouldTakeSnapshot(index int, files []string, provideFiles bool) bool {
 	isLastCommand := index == len(s.stage.Commands)-1
 
 	// We only snapshot the very end with single snapshot mode on.
@@ -438,8 +438,8 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool {
 		return true
 	}
 
-	// nil means snapshot everything.
-	if files == nil {
+	// if command does not provide files, snapshot everything.
+	if !provideFiles {
 		return true
 	}
 

diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go
@@ -114,8 +114,9 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 		opts  *config.KanikoOptions
 	}
 	type args struct {
-		index int
-		files []string
+		index    int
+		files    []string
+		hasFiles bool
 	}
 	tests := []struct {
 		name   string
@@ -162,6 +163,36 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 			},
 			want: true,
 		},
+		{
+			name: "not final stage not last command but empty list of files",
+			fields: fields{
+				stage: config.KanikoStage{
+					Final: false,
+					Stage: stage,
+				},
+			},
+			args: args{
+				index:    0,
+				files:    []string{},
+				hasFiles: true,
+			},
+			want: false,
+		},
+		{
+			name: "not final stage not last command no files provided",
+			fields: fields{
+				stage: config.KanikoStage{
+					Final: false,
+					Stage: stage,
+				},
+			},
+			args: args{
+				index:    0,
+				files:    nil,
+				hasFiles: false,
+			},
+			want: true,
+		},
 		{
 			name: "caching enabled intermediate container",
 			fields: fields{
@@ -187,7 +218,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 				stage: tt.fields.stage,
 				opts:  tt.fields.opts,
 			}
-			if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files); got != tt.want {
+			if got := s.shouldTakeSnapshot(tt.args.index, tt.args.files, tt.args.hasFiles); 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
@@ -39,7 +39,6 @@ var snapshotPathPrefix = config.KanikoDir
 
 // Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken
 type Snapshotter struct {
-	i         int
 	l         *LayeredMap
 	directory string
 	whitelist []util.WhitelistEntry
@@ -52,8 +51,6 @@ func NewSnapshotter(l *LayeredMap, d string) *Snapshotter {
 
 // Init initializes a new snapshotter
 func (s *Snapshotter) Init() error {
-	s.i++
-	logrus.Infof("Taking initial snapshot %d", s.i)
 	_, _, err := s.scanFullFilesystem()
 	return err
 }