fix a bug where file check nil is insufficient

This commit is contained in:
Tejal Desai 2020-05-01 22:35:59 -07:00
parent 32e3336d4c
commit 7d32139a13
3 changed files with 38 additions and 10 deletions

View File

@ -367,7 +367,7 @@ func (s *stageBuilder) build() error {
files = command.FilesToSnapshot() files = command.FilesToSnapshot()
timing.DefaultRun.Stop(t) timing.DefaultRun.Stop(t)
if !s.shouldTakeSnapshot(index, files) { if !s.shouldTakeSnapshot(index, files, command.ProvidesFilesToSnapshot()) {
continue continue
} }
@ -425,7 +425,7 @@ func (s *stageBuilder) takeSnapshot(files []string) (string, error) {
return snapshot, err 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 isLastCommand := index == len(s.stage.Commands)-1
// We only snapshot the very end with single snapshot mode on. // 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 return true
} }
// nil means snapshot everything. // if command does not provide files, snapshot everything.
if files == nil { if !provideFiles {
return true return true
} }

View File

@ -116,6 +116,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
type args struct { type args struct {
index int index int
files []string files []string
hasFiles bool
} }
tests := []struct { tests := []struct {
name string name string
@ -162,6 +163,36 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
}, },
want: true, 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", name: "caching enabled intermediate container",
fields: fields{ fields: fields{
@ -187,7 +218,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
stage: tt.fields.stage, stage: tt.fields.stage,
opts: tt.fields.opts, 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) t.Errorf("stageBuilder.shouldTakeSnapshot() = %v, want %v", got, tt.want)
} }
}) })

View File

@ -39,7 +39,6 @@ var snapshotPathPrefix = config.KanikoDir
// Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken // Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken
type Snapshotter struct { type Snapshotter struct {
i int
l *LayeredMap l *LayeredMap
directory string directory string
whitelist []util.WhitelistEntry whitelist []util.WhitelistEntry
@ -52,8 +51,6 @@ func NewSnapshotter(l *LayeredMap, d string) *Snapshotter {
// Init initializes a new snapshotter // Init initializes a new snapshotter
func (s *Snapshotter) Init() error { func (s *Snapshotter) Init() error {
s.i++
logrus.Infof("Taking initial snapshot %d", s.i)
_, _, err := s.scanFullFilesystem() _, _, err := s.scanFullFilesystem()
return err return err
} }