builder: use buildkit's GC for build cache #37846
Conversation
builder/builder-next/controller.go
Outdated
| wopt := mobyworker.Opt{ | ||
| ID: "moby", | ||
| Root: root, |
There was a problem hiding this comment.
TODO: remove this, not needed.
e24c7dc to
81363a2
Compare
Codecov Report
@@ Coverage Diff @@
## master #37846 +/- ##
=========================================
Coverage ? 36.09%
=========================================
Files ? 610
Lines ? 45115
Branches ? 0
=========================================
Hits ? 16284
Misses ? 26591
Partials ? 2240 |
e482e74 to
e6ac871
Compare
builder/builder-next/controller.go
Outdated
| if conf.GC.DefaultKeepStorage != "" { | ||
| defaultKeepStorage, err = units.RAMInBytes(conf.GC.DefaultKeepStorage) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
nit: wrap these parse errors
e6ac871 to
c82df7c
Compare
builder/builder-next/controller.go
Outdated
| err error | ||
| ) | ||
|
|
||
| if conf.GC.DefaultKeepStorage != "" { |
There was a problem hiding this comment.
Shouldn't this only be done only if conf.GC.Enabled is true?
builder/builder-next/controller.go
Outdated
| } | ||
| } | ||
| } else { | ||
| gcPolicy = mobyworker.DefaultGCPolicy(root, defaultKeepStorage) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
@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.
c82df7c to
380f52e
Compare
|
@fntlnz updated |
|
LGTM |
| if err := syscall.Statfs(root, &st); err != nil { | ||
| return defaultCap | ||
| } | ||
| diskSize := st.Bsize * int64(st.Blocks) |
There was a problem hiding this comment.
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.
380f52e to
f341564
Compare
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 <tibor@docker.com>
f341564 to
4a776d0
Compare
|
All tests passed so merging. |
|
Can you tell me exactly which config file I need to edit to set the options from the original post? |
This allows users to configure the buildkit GC.
The following enables the default GC:
The default GC policy has a simple config:
A custom GC policy can be used instead by specifying a list of cache prune rules:
Signed-off-by: Tibor Vass tibor@docker.com