From d9022dd7de93f3fb254f2332a6475ca7ff4a5adb Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 7 Sep 2018 17:04:04 -0700 Subject: [PATCH 1/3] Refactor build into stageBuilder type Refactoring builds by stage will make it easier to generate cache keys for layers, since the stageBuilder type will contain everything required to generate the key: 1. Base image with digest 2. Config file 3. Snapshotter (which will provide a key for the filesystem) 4. The current command (which will be passed in) --- pkg/executor/build.go | 219 ++++++++++++++++++++++++++---------------- 1 file changed, 134 insertions(+), 85 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 27f58d70fd..7902c2e3a3 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -30,6 +30,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/tarball" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/kaniko/pkg/commands" @@ -40,112 +41,160 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/util" ) -func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { - // Parse dockerfile and unpack base image to root - stages, err := dockerfile.Stages(opts) +// stageBuilder contains all fields necessary to build one stage of a Dockerfile +type stageBuilder struct { + stage config.KanikoStage + v1.Image + *v1.ConfigFile + *snapshot.Snapshotter + baseImageDigest string +} + +// newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage +func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*stageBuilder, error) { + sourceImage, err := util.RetrieveSourceImage(stage, opts.BuildArgs) if err != nil { return nil, err } - + imageConfig, err := util.RetrieveConfigFile(sourceImage) + if err != nil { + return nil, err + } + if err := resolveOnBuild(&stage, &imageConfig.Config); err != nil { + return nil, err + } hasher, err := getHasher(opts.SnapshotMode) if err != nil { return nil, err } - for index, stage := range stages { - // Unpack file system to root - sourceImage, err := util.RetrieveSourceImage(stage, opts.BuildArgs) + l := snapshot.NewLayeredMap(hasher) + snapshotter := snapshot.NewSnapshotter(l, constants.RootDir) + + digest, err := sourceImage.Digest() + if err != nil { + return nil, err + } + return &stageBuilder{ + stage: stage, + Image: sourceImage, + ConfigFile: imageConfig, + Snapshotter: snapshotter, + baseImageDigest: digest.String(), + }, nil +} + +// key will return a string representation of the build at the cmd +// TODO: priyawadhwa@ to fill this out when implementing caching +func (s *stageBuilder) key(cmd string) (string, error) { + return "", nil +} + +// extractCachedLayer will extract the cached layer and append it to the config file +// TODO: priyawadhwa@ to fill this out when implementing caching +func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { + return nil +} + +func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { + // Unpack file system to root + if err := util.GetFSFromImage(constants.RootDir, s.Image); err != nil { + return err + } + // Take initial snapshot + if err := s.Snapshotter.Init(); err != nil { + return err + } + buildArgs := dockerfile.NewBuildArgs(opts.BuildArgs) + for index, cmd := range s.stage.Commands { + finalCmd := index == len(s.stage.Commands)-1 + dockerCommand, err := commands.GetCommand(cmd, opts.SrcContext) if err != nil { - return nil, err + return err } - if err := util.GetFSFromImage(constants.RootDir, sourceImage); err != nil { - return nil, err - } - l := snapshot.NewLayeredMap(hasher) - snapshotter := snapshot.NewSnapshotter(l, constants.RootDir) - // Take initial snapshot - if err := snapshotter.Init(); err != nil { - return nil, err + if dockerCommand == nil { + continue } - imageConfig, err := util.RetrieveConfigFile(sourceImage) - if err != nil { - return nil, err + logrus.Info(dockerCommand.String()) + if err := dockerCommand.ExecuteCommand(&s.ConfigFile.Config, buildArgs); err != nil { + return err } - if err := resolveOnBuild(&stage, &imageConfig.Config); err != nil { - return nil, err - } - buildArgs := dockerfile.NewBuildArgs(opts.BuildArgs) - for index, cmd := range stage.Commands { - finalCmd := index == len(stage.Commands)-1 - dockerCommand, err := commands.GetCommand(cmd, opts.SrcContext) - if err != nil { - return nil, err - } - if dockerCommand == nil { - continue - } - logrus.Info(dockerCommand.String()) - if err := dockerCommand.ExecuteCommand(&imageConfig.Config, buildArgs); err != nil { - return nil, err - } - snapshotFiles := dockerCommand.FilesToSnapshot() - var contents []byte + snapshotFiles := dockerCommand.FilesToSnapshot() + 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 !stage.FinalStage { + // 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.FinalStage { + if finalCmd { + 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 = snapshotter.TakeSnapshotFS() + 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 = snapshotter.TakeSnapshotFS() - } + // 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 snapshotFiles != nil { + contents, err = s.Snapshotter.TakeSnapshot(snapshotFiles) } 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 snapshotFiles != nil { - contents, err = snapshotter.TakeSnapshot(snapshotFiles) - } else { - contents, err = snapshotter.TakeSnapshotFS() - } + contents, err = s.Snapshotter.TakeSnapshotFS() } } - if err != nil { - return nil, fmt.Errorf("Error taking snapshot of files for command %s: %s", dockerCommand, err) - } + } + if err != nil { + return fmt.Errorf("Error taking snapshot of files for command %s: %s", dockerCommand, err) + } - util.MoveVolumeWhitelistToWhitelist() - 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 nil, err - } - sourceImage, err = mutate.Append(sourceImage, - mutate.Addendum{ - Layer: layer, - History: v1.History{ - Author: constants.Author, - CreatedBy: dockerCommand.String(), - }, + util.MoveVolumeWhitelistToWhitelist() + 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 + } + s.Image, err = mutate.Append(s.Image, + mutate.Addendum{ + Layer: layer, + History: v1.History{ + Author: constants.Author, + CreatedBy: dockerCommand.String(), }, - ) - if err != nil { - return nil, err - } + }, + ) + if err != nil { + return err + } + } + return nil +} + +// DoBuild executes building the Dockerfile +func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { + // Parse dockerfile and unpack base image to root + stages, err := dockerfile.Stages(opts) + if err != nil { + return nil, err + } + for index, stage := range stages { + stageBuilder, err := newStageBuilder(opts, stage) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("getting stage builder for stage %d", index)) + } + if err := stageBuilder.buildStage(opts); err != nil { + return nil, errors.Wrap(err, "error building stage") } - sourceImage, err = mutate.Config(sourceImage, imageConfig.Config) + sourceImage, err := mutate.Config(stageBuilder.Image, stageBuilder.ConfigFile.Config) if err != nil { return nil, err } From bf72328611cb9da6f350c64ae5b542b9dfa10f70 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 12 Sep 2018 16:36:53 -0700 Subject: [PATCH 2/3] Addressed code review comment, removed stuttering variable names --- pkg/config/stage.go | 2 +- pkg/dockerfile/dockerfile.go | 2 +- pkg/executor/build.go | 44 ++++++++++++++++++------------------ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/config/stage.go b/pkg/config/stage.go index 3acfdc4090..7d4e908771 100644 --- a/pkg/config/stage.go +++ b/pkg/config/stage.go @@ -21,7 +21,7 @@ import "github.com/moby/buildkit/frontend/dockerfile/instructions" // KanikoStage wraps a stage of the Dockerfile and provides extra information type KanikoStage struct { instructions.Stage - FinalStage bool + Final bool BaseImageStoredLocally bool BaseImageIndex int SaveStage bool diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 5f37a16cf6..e1ae516491 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -57,7 +57,7 @@ func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) { BaseImageIndex: baseImageIndex(opts, index, stages), BaseImageStoredLocally: (baseImageIndex(opts, index, stages) != -1), SaveStage: saveStage(index, stages), - FinalStage: index == targetStage, + Final: index == targetStage, }) if index == targetStage { break diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 7902c2e3a3..06a9e7f233 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -44,8 +44,8 @@ import ( // stageBuilder contains all fields necessary to build one stage of a Dockerfile type stageBuilder struct { stage config.KanikoStage - v1.Image - *v1.ConfigFile + image v1.Image + cf *v1.ConfigFile *snapshot.Snapshotter baseImageDigest string } @@ -76,8 +76,8 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta } return &stageBuilder{ stage: stage, - Image: sourceImage, - ConfigFile: imageConfig, + image: sourceImage, + cf: imageConfig, Snapshotter: snapshotter, baseImageDigest: digest.String(), }, nil @@ -95,36 +95,36 @@ func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) erro return nil } -func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { +func (s *stageBuilder) build(opts *config.KanikoOptions) error { // Unpack file system to root - if err := util.GetFSFromImage(constants.RootDir, s.Image); err != nil { + if err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { return err } // Take initial snapshot if err := s.Snapshotter.Init(); err != nil { return err } - buildArgs := dockerfile.NewBuildArgs(opts.BuildArgs) + args := dockerfile.NewBuildArgs(opts.BuildArgs) for index, cmd := range s.stage.Commands { finalCmd := index == len(s.stage.Commands)-1 - dockerCommand, err := commands.GetCommand(cmd, opts.SrcContext) + command, err := commands.GetCommand(cmd, opts.SrcContext) if err != nil { return err } - if dockerCommand == nil { + if command == nil { continue } - logrus.Info(dockerCommand.String()) - if err := dockerCommand.ExecuteCommand(&s.ConfigFile.Config, buildArgs); err != nil { + logrus.Info(command.String()) + if err := command.ExecuteCommand(&s.cf.Config, args); err != nil { return err } - snapshotFiles := dockerCommand.FilesToSnapshot() + files := command.FilesToSnapshot() 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.FinalStage { + if !s.stage.Final { if finalCmd { contents, err = s.Snapshotter.TakeSnapshotFS() } @@ -139,15 +139,15 @@ func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { // 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 snapshotFiles != nil { - contents, err = s.Snapshotter.TakeSnapshot(snapshotFiles) + if files != nil { + contents, err = s.Snapshotter.TakeSnapshot(files) } else { contents, err = s.Snapshotter.TakeSnapshotFS() } } } if err != nil { - return fmt.Errorf("Error taking snapshot of files for command %s: %s", dockerCommand, err) + return fmt.Errorf("Error taking snapshot of files for command %s: %s", command, err) } util.MoveVolumeWhitelistToWhitelist() @@ -163,12 +163,12 @@ func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { if err != nil { return err } - s.Image, err = mutate.Append(s.Image, + s.image, err = mutate.Append(s.image, mutate.Addendum{ Layer: layer, History: v1.History{ Author: constants.Author, - CreatedBy: dockerCommand.String(), + CreatedBy: command.String(), }, }, ) @@ -187,18 +187,18 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return nil, err } for index, stage := range stages { - stageBuilder, err := newStageBuilder(opts, stage) + sb, err := newStageBuilder(opts, stage) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("getting stage builder for stage %d", index)) } - if err := stageBuilder.buildStage(opts); err != nil { + if err := sb.build(opts); err != nil { return nil, errors.Wrap(err, "error building stage") } - sourceImage, err := mutate.Config(stageBuilder.Image, stageBuilder.ConfigFile.Config) + sourceImage, err := mutate.Config(sb.image, sb.cf.Config) if err != nil { return nil, err } - if stage.FinalStage { + if stage.Final { sourceImage, err = mutate.CreatedAt(sourceImage, v1.Time{Time: time.Now()}) if err != nil { return nil, err From 7a6dfb6d8b8e1995eec115e539c27166b21c5323 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 12 Sep 2018 17:10:03 -0700 Subject: [PATCH 3/3] Removed incorrect FS extraction from earlier merge with master, and fixed linting errors --- pkg/executor/build.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 9bcbfda1ef..b908a0beb9 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -85,15 +85,15 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta // key will return a string representation of the build at the cmd // TODO: priyawadhwa@ to fill this out when implementing caching -func (s *stageBuilder) key(cmd string) (string, error) { - return "", nil -} +// func (s *stageBuilder) key(cmd string) (string, error) { +// return "", nil +// } // extractCachedLayer will extract the cached layer and append it to the config file // TODO: priyawadhwa@ to fill this out when implementing caching -func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { - return nil -} +// func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { +// return nil +// } func (s *stageBuilder) build(opts *config.KanikoOptions) error { // Unpack file system to root @@ -114,9 +114,6 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { if command == nil { continue } - if err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { - return err - } logrus.Info(command.String()) if err := command.ExecuteCommand(&s.cf.Config, args); err != nil { return err