Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support force-building metadata layers into snapshot #1731

Merged
merged 2 commits into from
Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/executor/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ func addKanikoOptionsFlags() {
RootCmd.PersistentFlags().Var(&opts.Git, "git", "Branch to clone if build context is a git repository")
RootCmd.PersistentFlags().BoolVarP(&opts.CacheCopyLayers, "cache-copy-layers", "", false, "Caches copy layers")
RootCmd.PersistentFlags().VarP(&opts.IgnorePaths, "ignore-path", "", "Ignore these paths when taking a snapshot. Set it repeatedly for multiple paths.")
RootCmd.PersistentFlags().BoolVarP(&opts.ForceBuildMetadata, "force-build-metadata", "", false, "Force add metadata layers to build image")
}

// addHiddenFlags marks certain flags as hidden from the executor help text
Expand Down
1 change: 1 addition & 0 deletions pkg/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type KanikoOptions struct {
Git KanikoGitOptions
IgnorePaths multiArg
ImageFSExtractRetry int
ForceBuildMetadata bool
}

type KanikoGitOptions struct {
Expand Down
9 changes: 5 additions & 4 deletions pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type cachePusher func(*config.KanikoOptions, string, string, string) error
type snapShotter interface {
Init() error
TakeSnapshotFS() (string, error)
TakeSnapshot([]string, bool) (string, error)
TakeSnapshot([]string, bool, bool) (string, error)
}

// stageBuilder contains all fields necessary to build one stage of a Dockerfile
Expand Down Expand Up @@ -375,7 +375,8 @@ func (s *stageBuilder) build() error {
files = command.FilesToSnapshot()
timing.DefaultRun.Stop(t)

if !s.shouldTakeSnapshot(index, command.MetadataOnly()) {
if !s.shouldTakeSnapshot(index, command.MetadataOnly()) && !s.opts.ForceBuildMetadata{
logrus.Debugf("build: skipping snapshot for [%v]" , command.String())
continue
}
if isCacheCommand {
Expand Down Expand Up @@ -429,7 +430,7 @@ func (s *stageBuilder) takeSnapshot(files []string, shdDelete bool) (string, err
} else {
// Volumes are very weird. They get snapshotted in the next command.
files = append(files, util.Volumes()...)
snapshot, err = s.snapshotter.TakeSnapshot(files, shdDelete)
snapshot, err = s.snapshotter.TakeSnapshot(files, shdDelete, s.opts.ForceBuildMetadata)
}
timing.DefaultRun.Stop(t)
return snapshot, err
Expand Down Expand Up @@ -473,7 +474,7 @@ func (s *stageBuilder) saveSnapshotToLayer(tarPath string) (v1.Layer, error) {
if err != nil {
return nil, errors.Wrap(err, "tar file path does not exist")
}
if fi.Size() <= emptyTarSize {
if fi.Size() <= emptyTarSize && !s.opts.ForceBuildMetadata{
logrus.Info("No files were changed, appending empty layer to config. No layer added to image.")
return nil, nil
}
Expand Down
7 changes: 5 additions & 2 deletions pkg/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ import (
// For testing
var snapshotPathPrefix = ""

// for user layer flag
var addUserLayer bool = true

// Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken
type Snapshotter struct {
l *LayeredMap
Expand All @@ -60,15 +63,15 @@ func (s *Snapshotter) Key() (string, error) {

// TakeSnapshot takes a snapshot of the specified files, avoiding directories in the ignorelist, 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, shdCheckDelete bool) (string, error) {
func (s *Snapshotter) TakeSnapshot(files []string, shdCheckDelete bool, forceBuildMetadata bool) (string, error) {
f, err := ioutil.TempFile(config.KanikoDir, "")
if err != nil {
return "", err
}
defer f.Close()

s.l.Snapshot()
if len(files) == 0 {
if len(files) == 0 && !forceBuildMetadata {
logrus.Info("No files changed in this command, skipping snapshotting.")
return "", nil
}
Expand Down
48 changes: 44 additions & 4 deletions pkg/snapshot/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func TestSnapshotFiles(t *testing.T) {
filesToSnapshot := []string{
filepath.Join(testDir, "foo"),
}
tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, false)
tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, false, false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -361,7 +361,7 @@ func TestSnasphotPreservesFileOrder(t *testing.T) {
}

// Take a snapshot
tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, false)
tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, false, false)

if err != nil {
t.Fatalf("Error taking snapshot of fs: %s", err)
Expand Down Expand Up @@ -391,6 +391,46 @@ func TestSnasphotPreservesFileOrder(t *testing.T) {
}
}

func TestSnapshotWithForceBuildMetadataSet(t *testing.T) {
_, snapshotter, cleanup, err := setUpTest()
defer cleanup()

if err != nil {
t.Fatal(err)
}

filesToSnapshot := []string{}

// snapshot should be taken regardless, if forceBuildMetadata flag is set
filename, err := snapshotter.TakeSnapshot(filesToSnapshot, false, true)
if err != nil {
t.Fatalf("Error taking snapshot of fs: %s", err)
}
if filename == "" {
t.Fatalf("Filename returned from snapshot is empty.")
}
}

func TestSnapshotWithForceBuildMetadataIsNotSet(t *testing.T) {
_, snapshotter, cleanup, err := setUpTest()
defer cleanup()

if err != nil {
t.Fatal(err)
}

filesToSnapshot := []string{}

// snapshot should not be taken
filename, err := snapshotter.TakeSnapshot(filesToSnapshot, false, false)
if err != nil {
t.Fatalf("Error taking snapshot of fs: %s", err)
}
if filename != "" {
t.Fatalf("Filename returned is expected to be empty.")
}
}

func TestSnasphotPreservesWhiteoutOrder(t *testing.T) {
newFiles := map[string]string{
"foo": "newbaz1",
Expand Down Expand Up @@ -430,7 +470,7 @@ func TestSnasphotPreservesWhiteoutOrder(t *testing.T) {
}

// Take a snapshot
_, err = snapshotter.TakeSnapshot(filesToSnapshot, false)
_, err = snapshotter.TakeSnapshot(filesToSnapshot, false, false)
if err != nil {
t.Fatalf("Error taking snapshot of fs: %s", err)
}
Expand All @@ -444,7 +484,7 @@ func TestSnasphotPreservesWhiteoutOrder(t *testing.T) {
}

// Take a snapshot again
tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, true)
tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot, true, false)
if err != nil {
t.Fatalf("Error taking snapshot of fs: %s", err)
}
Expand Down