builder: use buildkit's GC for build cache by tiborvass · Pull Request #37846 · moby/moby

tiborvass · 2018-09-14T07:11:48Z

This allows users to configure the buildkit GC.

The following enables the default GC:

{
  "builder": {
    "gc": {
      "enabled": true
    }
  }
}

The default GC policy has a simple config:

{
  "builder": {
    "gc": {
      "enabled": true,
      "defaultKeepStorage": "30GB"
    }
  }
}

A custom GC policy can be used instead by specifying a list of cache prune rules:

{
  "builder": {
    "gc": {
      "enabled": true,
      "policy": [
        {"keepStorage": "512MB", "filter": ["unused-for=1400h"]},
        {"keepStorage": "30GB", "all": true}
      ]
    }
  }
}

Signed-off-by: Tibor Vass [email protected]

tiborvass · 2018-09-14T17:57:11Z

builder/builder-next/controller.go

TODO: remove this, not needed.

codecov · 2018-09-14T23:17:33Z

Codecov Report

❗ No coverage uploaded for pull request base (master@b111647). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #37846   +/-   ##
=========================================
  Coverage          ?   36.09%           
=========================================
  Files             ?      610           
  Lines             ?    45115           
  Branches          ?        0           
=========================================
  Hits              ?    16284           
  Misses            ?    26591           
  Partials          ?     2240

tonistiigi · 2018-09-17T22:42:05Z

builder/builder-next/controller.go

nit: wrap these parse errors

andrewhsu

LGTM

fntlnz

I love this, just a few comments but in general looks very good 👼

fntlnz · 2018-09-20T19:00:49Z

builder/builder-next/controller.go

Shouldn't this only be done only if conf.GC.Enabled is true?

fntlnz · 2018-09-20T19:02:03Z

builder/builder-next/controller.go

Why not initializing gcPolicy to this directly instead of having an else statement? then if the user wants to set it the make at 194 can override it

@fntlnz It's a small matter of preference: with what you're suggesting, in the case where user wants to set it, it would have already called DefaultGCPolicy unnecessarily. The way it is now, only one of the two codepaths is executed once. If readability is what concerns you, I could change it to if conf.GC.Policy == nil { first.

tiborvass · 2018-09-20T20:27:28Z

@fntlnz updated

fntlnz · 2018-09-20T20:32:45Z

LGTM

fntlnz · 2018-09-21T08:24:03Z

builder/builder-next/worker/gc_unix.go

CI is complaining that on s390x (z linux) Bsize is int32.

https://github.com/golang/go/blob/master/src/syscall/ztypes_linux_s390x.go#L119

07:19:55 builder/builder-next/worker/gc_unix.go:14:23: invalid operation: st.Bsize * int64(st.Blocks) (mismatched types uint32 and int64)

I think we need an int32 version of this too.

@tiborvass ^^

This allows users to configure the buildkit GC. The following enables the default GC: ``` { "builder": { "gc": { "enabled": true } } } ``` The default GC policy has a simple config: ``` { "builder": { "gc": { "enabled": true, "defaultKeepStorage": "30GB" } } } ``` A custom GC policy can be used instead by specifying a list of cache prune rules: ``` { "builder": { "gc": { "enabled": true, "policy": [ {"keepStorage": "512MB", "filter": ["unused-for=1400h"]]}, {"keepStorage": "30GB", "all": true} ] } } } ``` Signed-off-by: Tibor Vass <[email protected]>

yongtang · 2018-09-22T16:59:42Z

All tests passed so merging.

Zeks · 2019-12-25T13:25:54Z

Can you tell me exactly which config file I need to edit to set the options from the original post?

tiborvass requested a review from tonistiigi as a code owner September 14, 2018 07:11

GordonTheTurtle added the status/0-triage label Sep 14, 2018

tiborvass commented Sep 14, 2018

View reviewed changes

builder/builder-next/controller.go Outdated

Copy link
Copy Markdown

Contributor Author

tiborvass Sep 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: remove this, not needed.

tiborvass force-pushed the buildkit-gc branch from e24c7dc to 81363a2 Compare September 14, 2018 23:17

tiborvass force-pushed the buildkit-gc branch 3 times, most recently from e482e74 to e6ac871 Compare September 15, 2018 01:16

tonistiigi added status/2-code-review and removed status/0-triage labels Sep 17, 2018

tonistiigi reviewed Sep 17, 2018

View reviewed changes

builder/builder-next/controller.go Outdated

Copy link
Copy Markdown

Member

tonistiigi Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: wrap these parse errors

tiborvass force-pushed the buildkit-gc branch from e6ac871 to c82df7c Compare September 19, 2018 19:07

thaJeztah added the area/builder Build label Sep 20, 2018

tonistiigi approved these changes Sep 20, 2018

View reviewed changes

andrewhsu approved these changes Sep 20, 2018

View reviewed changes

fntlnz reviewed Sep 20, 2018

View reviewed changes

tiborvass force-pushed the buildkit-gc branch from c82df7c to 380f52e Compare September 20, 2018 20:25

fntlnz approved these changes Sep 20, 2018

View reviewed changes

tiborvass added status/4-merge and removed status/2-code-review labels Sep 20, 2018

vdemeester added rebuild/experimental labels Sep 21, 2018

GordonTheTurtle removed rebuild/experimental labels Sep 21, 2018

fntlnz reviewed Sep 21, 2018

View reviewed changes

tiborvass force-pushed the buildkit-gc branch from 380f52e to f341564 Compare September 21, 2018 20:21

tiborvass force-pushed the buildkit-gc branch from f341564 to 4a776d0 Compare September 21, 2018 22:06

tonistiigi mentioned this pull request Sep 22, 2018

[18.09] Backport Buildkit fixes for 18.09 docker-archive/engine#59

Merged

yongtang added the rebuild/experimental label Sep 22, 2018

GordonTheTurtle removed the rebuild/experimental label Sep 22, 2018

yongtang merged commit fb7883e into moby:master Sep 22, 2018

This was referenced Oct 17, 2018

Multi stage build leaves "<none>" images behind #34151

Closed

Proposal: early stage layers from multi-stage builds should not be considered dangling #38071

Open

thaJeztah added the docs/revisit label Oct 26, 2018

thaJeztah added impact/changelog impact/documentation labels Dec 31, 2018

tiborvass deleted the buildkit-gc branch July 17, 2019 00:34

KevOrr mentioned this pull request Jul 24, 2019

Example Buildkit GC configuration results in daemon error #39601

Closed

johnbizokk mentioned this pull request Oct 13, 2019

Non-working Buildkit GC policies #40086

Closed

thaJeztah mentioned this pull request Jan 15, 2020

[epic] make BuildKit the default builder on Linux #40379

Open

12 tasks

thaJeztah mentioned this pull request Feb 11, 2020

[epic] documentation on BuildKit garbage-collect policies is missing docker/cli#2325

Closed

5 tasks

thaJeztah mentioned this pull request May 20, 2020

[Docker Desktop] Desktop resource clean up docker/roadmap#13

Open

thaJeztah mentioned this pull request Nov 29, 2023

builder-next: fix timing filter for default policy #46861

Merged

thaJeztah mentioned this pull request Dec 19, 2024

builder: don't fall back to defaultKeepStorage when set to zero #49147

Merged

Conversation

tiborvass commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tiborvass Sep 14, 2018

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tonistiigi Sep 17, 2018

Choose a reason for hiding this comment

Uh oh!

andrewhsu left a comment

Choose a reason for hiding this comment

Uh oh!

fntlnz left a comment

Choose a reason for hiding this comment

Uh oh!

fntlnz Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

fntlnz Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

tiborvass Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiborvass commented Sep 20, 2018

Uh oh!

fntlnz commented Sep 20, 2018

Uh oh!

fntlnz Sep 21, 2018

Choose a reason for hiding this comment

Uh oh!

fntlnz Sep 21, 2018

Choose a reason for hiding this comment

Uh oh!

yongtang commented Sep 22, 2018

Uh oh!

Zeks commented Dec 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tiborvass commented Sep 14, 2018 •

edited

Loading

codecov bot commented Sep 14, 2018 •

edited

Loading

tiborvass Sep 20, 2018 •

edited

Loading