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

Implement build cache based on history array #26839

Merged
merged 2 commits into from Sep 26, 2016

Conversation

@tonistiigi
Member

tonistiigi commented Sep 22, 2016

carry #24711
fixes #26065

Adds capability to specify images used as a cache source on build. These images do not need to have local parent chain and can be pulled from other registries. User needs to make sure to only use trusted images as sources.

Usage:

docker pull myimage:v1.0
docker build --cache-from myimage:v1.0 -t myimage:v1.1 .
@graingert

This comment has been minimized.

Contributor

graingert commented Sep 22, 2016

Is this not going to be plugin based like #24711 (comment) ?

@tonistiigi

This comment has been minimized.

Member

tonistiigi commented Sep 22, 2016

@graingert If you look at the contents of #24711 it is not about the plugin method. This was changed if favor of #26065 proposal a long time.

@tonistiigi tonistiigi force-pushed the tonistiigi:build-cache branch 2 times, most recently from ba1feda to d3222ea Sep 22, 2016

@tonistiigi tonistiigi changed the title from wip: Implement build cache based on history array to Implement build cache based on history array Sep 22, 2016

@tonistiigi

This comment has been minimized.

Member

tonistiigi commented Sep 22, 2016

@icecrime

This comment has been minimized.

Contributor

icecrime commented Sep 23, 2016

I think design was already approved, so I'm tentatively moving to code review.

Thanks for the carry @tonistiigi!

var cacheFrom = []string{}
cacheFromJSON := r.FormValue("cachefrom")
if cacheFromJSON != "" {
if err := json.NewDecoder(strings.NewReader(cacheFromJSON)).Decode(&cacheFrom); err != nil {

This comment has been minimized.

@stevvooe

stevvooe Sep 23, 2016

Contributor

json.Unmarshal(cacheFromJSON, &cacheFrom)

@tonistiigi tonistiigi force-pushed the tonistiigi:build-cache branch from d3222ea to 0a43b1d Sep 23, 2016

@stevvooe

This comment has been minimized.

Contributor

stevvooe commented Sep 23, 2016

LGTM after the small nit.

return false
}
if parent == nil || len(parent.History) == 0 && len(parent.RootFS.DiffIDs) == 0 {
return true

This comment has been minimized.

@aaronlehmann

aaronlehmann Sep 23, 2016

Contributor

nil is considered a valid parent?

This comment has been minimized.

@tonistiigi

tonistiigi Sep 23, 2016

Member

nil means FROM scratch so we ignore the parent and only check that the configuration matches

OSVersion: target.OSVersion,
})
if err != nil {
return "", errors.Wrapf(err, "failed to marshal image config")

This comment has been minimized.

@aaronlehmann

aaronlehmann Sep 23, 2016

Contributor

errors.Wrap

imgID, err := ic.daemon.imageStore.Create(config)
if err != nil {
return "", errors.Wrapf(err, "failed to create cache image")

This comment has been minimized.

@aaronlehmann

aaronlehmann Sep 23, 2016

Contributor

errors.Wrap

@tonistiigi tonistiigi force-pushed the tonistiigi:build-cache branch from 0a43b1d to fe741fc Sep 23, 2016

@@ -447,11 +447,11 @@ func (b *Builder) processImageFrom(img builder.Image) error {
// If no image is found, it returns `(false, nil)`.
// If there is any error, it returns `(false, err)`.
func (b *Builder) probeCache() (bool, error) {

This comment has been minimized.

@aaronlehmann

aaronlehmann Sep 23, 2016

Contributor

Comment should be updated. Right now it says "probeCache checks if b.docker implements builder.ImageCache and image-caching is enabled"

@aaronlehmann

This comment has been minimized.

Contributor

aaronlehmann commented Sep 23, 2016

LGTM after nits

tonistiigi added some commits Sep 22, 2016

Implement build cache based on history array
Based on work by KJ Tsanaktsidis

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: KJ Tsanaktsidis <kjtsanaktsidis@gmail.com>
Add integration test for build —cache-from
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

@tonistiigi tonistiigi force-pushed the tonistiigi:build-cache branch from fe741fc to 4b8e680 Sep 23, 2016

@tonistiigi

This comment has been minimized.

Member

tonistiigi commented Sep 23, 2016

@stevvooe

This comment has been minimized.

Contributor

stevvooe commented Sep 23, 2016

LGTM

@aaronlehmann

LGTM

@VinceMD

This comment has been minimized.

VinceMD commented Nov 22, 2016

when is that due to be released?

@gajus

This comment has been minimized.

gajus commented Nov 22, 2016

Where is documentation for this?

I'd like to better understand the implications of using a cache source.

@tonistiigi

This comment has been minimized.

Member

tonistiigi commented Nov 22, 2016

@VinceMD It is included in v1.13 that is currently in rc phase. Stable release expected in ~3 weeks.

@gajus Docs are in this PR. Using a source image should be compatible with build cache from local images.

@gajus gajus referenced this pull request Nov 23, 2016

Closed

Support Docker v1.13 #112

@hstenzel

This comment has been minimized.

hstenzel commented Dec 14, 2016

I'm excited for release!

How would I use --cache-from if my docker build is itself handled by docker-compose?

Thanks.

@tonistiigi

This comment has been minimized.

Member

tonistiigi commented Dec 14, 2016

@dnephin

This comment has been minimized.

Member

dnephin commented Dec 16, 2016

You wont be able to until Compose supports this flag, and I don't see any support for it in the upcoming release.

@tuxity tuxity referenced this pull request Jan 19, 2017

Merged

[WIP] Docker 1.13 #100

3 of 5 tasks complete

shin- added a commit to docker/docker-py that referenced this pull request Jan 26, 2017

Merge pull request #1417 from graingert/cachefrom
This adds the cache-from build option (moby/moby#26839) and fixes #1382.
@kachkaev

This comment has been minimized.

kachkaev commented Feb 3, 2017

Hi guys,

Is docker pull myimage:v1.0 strictly necessary? Won't just docker build --cache-from myimage:v1.0 -t myimage:v1.1 . do pull as well? https://docs.docker.com/engine/reference/builder/ or CLI help do not explain this unfortunately.

I also struggled to find an explanation of what happens if there are multiple --cache-from. I've got a GitLab CI script that builds an image and I'd like it to reuse previous builds to save time. The idea is to grab a previously built image with the current branch tag, then if this fails pull master and if both pulls fail, just build from scratch.

Should the following work smoothly for all possible states of the "cache"? Or is anything here missing or redundant here?

docker pull $CI_REGISTRY_IMAGE:$CI_BUILD_REF_NAME || docker pull $CI_REGISTRY_IMAGE:master || true
docker build --tag=$CI_REGISTRY_IMAGE:$CI_BUILD_REF_NAME --cache-from=$CI_REGISTRY_IMAGE:$CI_BUILD_REF_NAME --cache-from=$CI_REGISTRY_IMAGE:master .
docker push $CI_REGISTRY_IMAGE:$CI_BUILD_REF_NAME

For my-branch this resolves to:

docker pull registry.example.com/group/repo:my-branch || docker pull registry.example.com/group/repo:master || true
docker build --tag=registry.example.com/group/repo:my-branch --cache-from=registry.example.com/group/repo:my-branch --cache-from=registry.example.com/group/repo:master .
docker push registry.example.com/group/repo:my-branch

And for master the script looks like this:

docker pull registry.example.com/group/repo:master || docker pull registry.example.com/group/repo:master || true
docker build --tag=registry.example.com/group/repo:master --cache-from=registry.example.com/group/repo:my-branch --cache-from=registry.example.com/group/repo:master .
docker push registry.example.com/group/repo:master
@gajus

This comment has been minimized.

gajus commented Feb 3, 2017

As far as my testing goes, docker build --cache-from is sufficient.

However, I have written a blog post about using --cache-from in my GitLab EE CI setup (https://medium.com/@gajus/making-docker-in-docker-builds-x2-faster-using-docker-cache-from-option-c01febd8ef84#.z3rl5bahm) and someone commented that docker pull is missing.

I am not sure whats the relevance of docker pull when using --cache-from should automatically attempt to pull the image. Is that not the case?

@kachkaev

This comment has been minimized.

kachkaev commented Feb 3, 2017

Yeah, that's what is unclear to me too. If --cache-from does pull an image, what will happen if it does not exist or if there is a problem with logging in? If I use multiple --cache-from and the first image exists, will docker build attempt to pull the second and the third image and reuse as much layers as possible? If at least one of the images fails to pull, with the whole build command fail? Or will it warn? Or will it just silently build from scratch?

I know that these questions can be answered after a few tests, but it'd be great if they were documented so that people could write robust CI scripts without too much iteration.

@graingert

This comment has been minimized.

Contributor

graingert commented Feb 3, 2017

I'd like to know so I can add --cache-from to shipwright.

@tonistiigi

This comment has been minimized.

Member

tonistiigi commented Feb 3, 2017

--cache-from does not pull if an image is not found. There have been discussions about it but the way we would like to enable it is to not pull full images but only pull data that is needed to determine the cache hit and from there decide to actually pull layers.

When using multiple --cache-from they are checked for a cache hit in the order that user specified. If one of the images produces a cache hit for a command only that image is used for the rest of the build.

databus23 added a commit to databus23/docker-image-resource that referenced this pull request Feb 13, 2017

Implement cache using docker's --cache-from flag
With docker 1.13 a new build flag was added: --cache-from
This flag deprecated the buildcache tool that we used with docker 1.12

Details: moby/moby#26839

databus23 added a commit to databus23/docker-image-resource that referenced this pull request Mar 13, 2017

Implement cache using docker's --cache-from flag
With docker 1.13 a new build flag was added: --cache-from
This flag deprecated the buildcache tool that we used with docker 1.12

Details: moby/moby#26839

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Arnaud Porterie
Merge pull request moby#26839 from tonistiigi/build-cache
Implement build cache based on history array

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Arnaud Porterie
Merge pull request moby#26839 from tonistiigi/build-cache
Implement build cache based on history array

dwrensha referenced this pull request in integer32llc/rust-playground May 18, 2017

ggeorgiev added a commit to ggeorgiev/docker-image-resource that referenced this pull request Jun 8, 2017

Implement cache using docker's --cache-from flag
With docker 1.13 a new build flag was added: --cache-from
This flag deprecated the buildcache tool that we used with docker 1.12

Details: moby/moby#26839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment