From fcd1976d3b356c9f531ab79b59b50d493bcebfbc Mon Sep 17 00:00:00 2001 From: dlorenc Date: Tue, 22 Jan 2019 13:28:21 -0600 Subject: [PATCH] Make the Digest calculation faster for locally-cached images. (#534) Right now when we find a v1.Tarball in the local disk cache, we recompute the digest. This is very expensive and redundant, because we store tarballs by their digest and use that as a key to look them up. --- pkg/cache/cache.go | 27 +++++++++++++++++++--- pkg/executor/build.go | 26 ++++++++++++++++----- pkg/util/image_util.go | 46 +++++++++++++++---------------------- pkg/util/image_util_test.go | 2 +- 4 files changed, 64 insertions(+), 37 deletions(-) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index c005cd159..fa2903085 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "path" + "path/filepath" "time" "github.com/GoogleContainerTools/kaniko/pkg/config" @@ -114,11 +115,31 @@ func LocalSource(opts *config.KanikoOptions, cacheKey string) (v1.Image, error) return nil, nil } - imgTar, err := tarball.ImageFromPath(path, nil) + logrus.Infof("Found %s in local cache", cacheKey) + return cachedImageFromPath(path) +} + +// cachedImage represents a v1.Tarball that is cached locally in a CAS. +// Computing the digest for a v1.Tarball is very expensive. If the tarball +// is named with the digest we can store this and return it directly rather +// than recompute it. +type cachedImage struct { + digest string + v1.Image +} + +func (c *cachedImage) Digest() (v1.Hash, error) { + return v1.NewHash(c.digest) +} + +func cachedImageFromPath(p string) (v1.Image, error) { + imgTar, err := tarball.ImageFromPath(p, nil) if err != nil { return nil, errors.Wrap(err, "getting image from path") } - logrus.Infof("Found %s in local cache", cacheKey) - return imgTar, nil + return &cachedImage{ + digest: filepath.Base(p), + Image: imgTar, + }, nil } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 2ee22b02c..fe1bfb73f 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -23,12 +23,15 @@ import ( "strconv" "time" + "github.com/google/go-containerregistry/pkg/v1/partial" + "github.com/moby/buildkit/frontend/dockerfile/instructions" "golang.org/x/sync/errgroup" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/pkg/errors" @@ -59,21 +62,20 @@ type stageBuilder struct { // 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) { - t := timing.Start("Retrieving Source Image") sourceImage, err := util.RetrieveSourceImage(stage, opts) if err != nil { return nil, err } - timing.DefaultRun.Stop(t) - t = timing.Start("Retrieving Config File") - imageConfig, err := util.RetrieveConfigFile(sourceImage) + + imageConfig, err := initializeConfig(sourceImage) if err != nil { return nil, err } - timing.DefaultRun.Stop(t) + if err := resolveOnBuild(&stage, &imageConfig.Config); err != nil { return nil, err } + hasher, err := getHasher(opts.SnapshotMode) if err != nil { return nil, err @@ -95,6 +97,18 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta }, nil } +func initializeConfig(img partial.WithConfigFile) (*v1.ConfigFile, error) { + imageConfig, err := img.ConfigFile() + if err != nil { + return nil, err + } + + if img == empty.Image { + imageConfig.Config.Env = constants.ScratchEnvVars + } + return imageConfig, nil +} + func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config, cmds []commands.DockerCommand, args *dockerfile.BuildArgs) error { if !s.opts.Cache { return nil @@ -429,7 +443,7 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e } // This must be an image name, fetch it. logrus.Debugf("Found extra base image stage %s", c.From) - sourceImage, err := util.RetrieveRemoteImage(c.From, opts, false) + sourceImage, err := util.RetrieveRemoteImage(c.From, opts) if err != nil { return err } diff --git a/pkg/util/image_util.go b/pkg/util/image_util.go index 9a870a830..0604a1bb3 100644 --- a/pkg/util/image_util.go +++ b/pkg/util/image_util.go @@ -23,12 +23,13 @@ import ( "path/filepath" "strconv" + "github.com/GoogleContainerTools/kaniko/pkg/timing" + "github.com/GoogleContainerTools/kaniko/pkg/creds" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" - "github.com/google/go-containerregistry/pkg/v1/partial" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/sirupsen/logrus" @@ -46,6 +47,8 @@ var ( // RetrieveSourceImage returns the base image of the stage at index func RetrieveSourceImage(stage config.KanikoStage, opts *config.KanikoOptions) (v1.Image, error) { + t := timing.Start("Retrieving Source Image") + defer timing.DefaultRun.Stop(t) buildArgs := opts.BuildArgs var metaArgsString []string for _, arg := range stage.MetaArgs { @@ -67,20 +70,21 @@ func RetrieveSourceImage(stage config.KanikoStage, opts *config.KanikoOptions) ( return retrieveTarImage(stage.BaseImageIndex) } - // Otherwise, initialize image as usual - return RetrieveRemoteImage(currentBaseName, opts, false) -} + // Finally, check if local caching is enabled + // If so, look in the local cache before trying the remote registry + if opts.Cache && opts.CacheDir != "" { + cachedImage, err := cachedImage(opts, currentBaseName) + if cachedImage != nil { + return cachedImage, nil + } -// RetrieveConfigFile returns the config file for an image -func RetrieveConfigFile(sourceImage partial.WithConfigFile) (*v1.ConfigFile, error) { - imageConfig, err := sourceImage.ConfigFile() - if err != nil { - return nil, err + if err != nil { + logrus.Infof("Error while retrieving image from cache: %v", err) + } } - if sourceImage == empty.Image { - imageConfig.Config.Env = constants.ScratchEnvVars - } - return imageConfig, nil + + // Otherwise, initialize image as usual + return RetrieveRemoteImage(currentBaseName, opts) } func tarballImage(index int) (v1.Image, error) { @@ -89,20 +93,8 @@ func tarballImage(index int) (v1.Image, error) { return tarball.ImageFromPath(tarPath, nil) } -func remoteImage(image string, opts *config.KanikoOptions, forceNoCache bool) (v1.Image, error) { +func remoteImage(image string, opts *config.KanikoOptions) (v1.Image, error) { logrus.Infof("Downloading base image %s", image) - // First, check if local caching is enabled - // If so, look in the local cache before trying the remote registry - if opts.Cache && opts.CacheDir != "" && !forceNoCache { - cachedImage, err := cachedImage(opts, image) - if cachedImage != nil { - return cachedImage, nil - } - - if err != nil { - logrus.Warnf("Error while retrieving image from cache: %v", err) - } - } ref, err := name.ParseReference(image, name.WeakValidation) if err != nil { return nil, err @@ -143,7 +135,7 @@ func cachedImage(opts *config.KanikoOptions, image string) (v1.Image, error) { if d, ok := ref.(name.Digest); ok { cacheKey = d.DigestStr() } else { - img, err := remoteImage(image, opts, true) + img, err := remoteImage(image, opts) if err != nil { return nil, err } diff --git a/pkg/util/image_util_test.go b/pkg/util/image_util_test.go index e53ff4250..c44fe8a58 100644 --- a/pkg/util/image_util_test.go +++ b/pkg/util/image_util_test.go @@ -51,7 +51,7 @@ func Test_StandardImage(t *testing.T) { defer func() { RetrieveRemoteImage = original }() - mock := func(image string, opts *config.KanikoOptions, forceNoCache bool) (v1.Image, error) { + mock := func(image string, opts *config.KanikoOptions) (v1.Image, error) { return nil, nil } RetrieveRemoteImage = mock