From 93c72ab7198fda5c8eb3436fa6d8b07b2f0f34e4 Mon Sep 17 00:00:00 2001 From: dlorenc Date: Mon, 8 Oct 2018 08:57:04 -0700 Subject: [PATCH] Refactor the build loop. This change refactors the build loop a bit to make cache optimization easier in the future. Some notable changes: The special casing around volume snapshots is removed. Every volume is added to the snapshotFiles list for every command that will snapshot anyway. Snapshot saving was extracted to a sub-function The decision on whether or not to snapshot was extracted --- pkg/commands/volume.go | 6 +- pkg/commands/volume_test.go | 1 - pkg/constants/constants.go | 3 - pkg/executor/build.go | 155 ++++++++++++++++++++---------------- 4 files changed, 87 insertions(+), 78 deletions(-) diff --git a/pkg/commands/volume.go b/pkg/commands/volume.go index af2d956752..4dd337850f 100644 --- a/pkg/commands/volume.go +++ b/pkg/commands/volume.go @@ -30,8 +30,7 @@ import ( type VolumeCommand struct { BaseCommand - cmd *instructions.VolumeCommand - snapshotFiles []string + cmd *instructions.VolumeCommand } func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { @@ -57,7 +56,6 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile. // Only create and snapshot the dir if it didn't exist already if _, err := os.Stat(volume); os.IsNotExist(err) { logrus.Infof("Creating directory %s", volume) - v.snapshotFiles = append(v.snapshotFiles, volume) if err := os.MkdirAll(volume, 0755); err != nil { return fmt.Errorf("Could not create directory for volume %s: %s", volume, err) } @@ -69,7 +67,7 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile. } func (v *VolumeCommand) FilesToSnapshot() []string { - return v.snapshotFiles + return []string{} } func (v *VolumeCommand) String() string { diff --git a/pkg/commands/volume_test.go b/pkg/commands/volume_test.go index c85ed7f6de..9616006b8a 100644 --- a/pkg/commands/volume_test.go +++ b/pkg/commands/volume_test.go @@ -43,7 +43,6 @@ func TestUpdateVolume(t *testing.T) { cmd: &instructions.VolumeCommand{ Volumes: volumes, }, - snapshotFiles: []string{}, } expectedVolumes := map[string]struct{}{ diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 1c8d774b80..d8fcc722e9 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -62,9 +62,6 @@ const ( // Docker command names Cmd = "cmd" Entrypoint = "entrypoint" - - // VolumeCmdName is the name of the volume command - VolumeCmdName = "volume" ) // KanikoBuildFiles is the list of files required to build kaniko diff --git a/pkg/executor/build.go b/pkg/executor/build.go index e517dafa09..1f90037d85 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -49,6 +49,7 @@ type stageBuilder struct { cf *v1.ConfigFile snapshotter *snapshot.Snapshotter baseImageDigest string + opts *config.KanikoOptions } // newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage @@ -81,6 +82,7 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta cf: imageConfig, snapshotter: snapshotter, baseImageDigest: digest.String(), + opts: opts, }, nil } @@ -111,7 +113,7 @@ func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) erro return err } -func (s *stageBuilder) build(opts *config.KanikoOptions) error { +func (s *stageBuilder) build() error { // Unpack file system to root if _, err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { return err @@ -120,23 +122,26 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { if err := s.snapshotter.Init(); err != nil { return err } - var volumes []string // Set the initial cache key to be the base image digest, the build args and the SrcContext. compositeKey := NewCompositeCache(s.baseImageDigest) - contextHash, err := HashDir(opts.SrcContext) + contextHash, err := HashDir(s.opts.SrcContext) if err != nil { return err } - compositeKey.AddKey(opts.BuildArgs...) + compositeKey.AddKey(s.opts.BuildArgs...) - args := dockerfile.NewBuildArgs(opts.BuildArgs) - for index, cmd := range s.stage.Commands { - finalCmd := index == len(s.stage.Commands)-1 - command, err := commands.GetCommand(cmd, opts.SrcContext) + cmds := []commands.DockerCommand{} + for _, cmd := range s.stage.Commands { + command, err := commands.GetCommand(cmd, s.opts.SrcContext) if err != nil { return err } + cmds = append(cmds, command) + } + + args := dockerfile.NewBuildArgs(s.opts.BuildArgs) + for index, command := range cmds { if command == nil { continue } @@ -153,8 +158,8 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { return err } - if command.CacheCommand() && opts.Cache { - image, err := cache.RetrieveLayer(opts, ck) + if command.CacheCommand() && s.opts.Cache { + image, err := cache.RetrieveLayer(s.opts, ck) if err == nil { if err := s.extractCachedLayer(image, command.String()); err != nil { return errors.Wrap(err, "extracting cached layer") @@ -163,84 +168,94 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { } logrus.Info("No cached layer found, executing command...") } + if err := command.ExecuteCommand(&s.cf.Config, args); err != nil { return err } files := command.FilesToSnapshot() - if cmd.Name() == constants.VolumeCmdName { - volumes = append(volumes, files...) + var contents []byte + + if !s.shouldTakeSnapshot(index, files) { continue } - var contents []byte - // If this is an intermediate stage, we only snapshot for the last command and we - // want to snapshot the entire filesystem since we aren't tracking what was changed - // by previous commands. - if !s.stage.Final { - if finalCmd { - contents, err = s.snapshotter.TakeSnapshotFS() - } + if files == nil || s.opts.SingleSnapshot { + contents, err = s.snapshotter.TakeSnapshotFS() } else { - // If we are in single snapshot mode, we only take a snapshot once, after all - // commands have completed. - if opts.SingleSnapshot { - if finalCmd { - contents, err = s.snapshotter.TakeSnapshotFS() - } - } else { - // Otherwise, in the final stage we take a snapshot at each command. If we know - // the files that were changed, we'll snapshot those explicitly, otherwise we'll - // check if anything in the filesystem changed. - if files != nil { - if len(files) > 0 { - files = append(files, volumes...) - volumes = []string{} - } - contents, err = s.snapshotter.TakeSnapshot(files) - } else { - contents, err = s.snapshotter.TakeSnapshotFS() - volumes = []string{} - } + // Volumes are very weird. They get created in their command, but snapshotted in the next one. + // Add them to the list of files to snapshot. + for v := range s.cf.Config.Volumes { + files = append(files, v) } + contents, err = s.snapshotter.TakeSnapshot(files) } - if err != nil { - return fmt.Errorf("Error taking snapshot of files for command %s: %s", command, err) - } - - if contents == nil { - logrus.Info("No files were changed, appending empty layer to config. No layer added to image.") - continue - } - // Append the layer to the image - opener := func() (io.ReadCloser, error) { - return ioutil.NopCloser(bytes.NewReader(contents)), nil - } - layer, err := tarball.LayerFromOpener(opener) if err != nil { return err } - // Push layer to cache now along with new config file - if command.CacheCommand() && opts.Cache { - if err := pushLayerToCache(opts, ck, layer, command.String()); err != nil { - return err - } - } - s.image, err = mutate.Append(s.image, - mutate.Addendum{ - Layer: layer, - History: v1.History{ - Author: constants.Author, - CreatedBy: command.String(), - }, - }, - ) - if err != nil { + if err := s.saveSnapshot(command, ck, contents); err != nil { return err } } return nil } +func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool { + isLastCommand := index == len(s.stage.Commands)-1 + + // We only snapshot the very end of intermediate stages. + if !s.stage.Final { + return isLastCommand + } + + // We only snapshot the very end with single snapshot mode on. + if s.opts.SingleSnapshot { + return isLastCommand + } + + // nil means snapshot everything. + if files == nil { + return true + } + + // Don't snapshot an empty list. + if len(files) == 0 { + return false + } + return true +} + +func (s *stageBuilder) saveSnapshot(command commands.DockerCommand, ck string, contents []byte) error { + if contents == nil { + logrus.Info("No files were changed, appending empty layer to config. No layer added to image.") + return nil + } + // Append the layer to the image + opener := func() (io.ReadCloser, error) { + return ioutil.NopCloser(bytes.NewReader(contents)), nil + } + layer, err := tarball.LayerFromOpener(opener) + if err != nil { + return err + } + // Push layer to cache now along with new config file + if command.CacheCommand() && s.opts.Cache { + if err := pushLayerToCache(s.opts, ck, layer, command.String()); err != nil { + return err + } + } + s.image, err = mutate.Append(s.image, + mutate.Addendum{ + Layer: layer, + History: v1.History{ + Author: constants.Author, + CreatedBy: command.String(), + }, + }, + ) + return err + +} + // DoBuild executes building the Dockerfile func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { // Parse dockerfile and unpack base image to root @@ -253,7 +268,7 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("getting stage builder for stage %d", index)) } - if err := sb.build(opts); err != nil { + if err := sb.build(); err != nil { return nil, errors.Wrap(err, "error building stage") } reviewConfig(stage, &sb.cf.Config)