Skip to content
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

allow controlling detected platforms cache timeout #4949

Merged
merged 1 commit into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cmd/buildkitd/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ type Config struct {
Dockerfile DockerfileFrontendConfig `toml:"dockerfile.v0"`
Gateway GatewayFrontendConfig `toml:"gateway.v0"`
} `toml:"frontend"`

System *SystemConfig `toml:"system"`
}

type SystemConfig struct {
// PlatformCacheMaxAge controls how often supported platforms
// are refreshed by rescanning the system.
PlatformsCacheMaxAge *Duration `toml:"platformsCacheMaxAge"`
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to always use cache by setting platformsCacheMaxAge = -1 with this custom type?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I tested this

}

type LogConfig struct {
Expand Down
6 changes: 6 additions & 0 deletions cmd/buildkitd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ func main() {
logrus.SetLevel(logrus.TraceLevel)
}

if sc := cfg.System; sc != nil {
if v := sc.PlatformsCacheMaxAge; v != nil {
archutil.CacheMaxAge = v.Duration
}
}

if cfg.GRPC.DebugAddress != "" {
if err := setupDebugHandlers(cfg.GRPC.DebugAddress); err != nil {
return err
Expand Down
26 changes: 15 additions & 11 deletions docs/buildkitd.toml.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,18 +139,22 @@ insecure-entitlements = [ "network.host", "security.insecure" ]

# Frontend control
[frontend."dockerfile.v0"]
enabled = true
enabled = true

[frontend."gateway.v0"]
enabled = true

# If allowedRepositories is empty, all gateway sources are allowed.
# Otherwise, only the listed repositories are allowed as a gateway source.
#
# NOTE: Only the repository name (without tag) is compared.
#
# Example:
# allowedRepositories = [ "docker-registry.wikimedia.org/repos/releng/blubber/buildkit" ]
allowedRepositories = []
enabled = true

# If allowedRepositories is empty, all gateway sources are allowed.
# Otherwise, only the listed repositories are allowed as a gateway source.
#
# NOTE: Only the repository name (without tag) is compared.
#
# Example:
# allowedRepositories = [ "docker-registry.wikimedia.org/repos/releng/blubber/buildkit" ]
allowedRepositories = []

[system]
# how often buildkit scans for changes in the supported emulated platforms
platformsCacheMaxAge = "1h"

```
13 changes: 10 additions & 3 deletions util/archutil/detect.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,28 @@ import (
"sort"
"strings"
"sync"
"time"

"github.com/containerd/containerd/platforms"
"github.com/moby/buildkit/util/bklog"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
)

var mu sync.Mutex
var arr []ocispecs.Platform
var CacheMaxAge = 20 * time.Second
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity. How often does this change during the lifetime of a buildkit process? I guess it would only change on an install of new binfmt handlers right?

Potentially, could we invalidate the cache based on the contents of /proc/sys/fs/binfmt_misc/? Just a thought, curious if you considered this option.

Copy link
Member Author

Choose a reason for hiding this comment

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

How often does this change during the lifetime of a buildkit process? I guess it would only change on an install of new binfmt handlers right?

Yeah, it depends on what the user is doing outside of buildkit.

Potentially, could we invalidate the cache based on the contents of /proc/sys/fs/binfmt_misc/?

binfmt_misc is usually not mounted in the environment where buildkit runs


var (
mu sync.Mutex
arr []ocispecs.Platform
lastRefresh time.Time
)

func SupportedPlatforms(noCache bool) []ocispecs.Platform {
mu.Lock()
defer mu.Unlock()
if !noCache && arr != nil {
if arr != nil && (!noCache || CacheMaxAge < 0 || time.Since(lastRefresh) < CacheMaxAge) {
return arr
}
defer func() { lastRefresh = time.Now() }()
def := nativePlatform()
arr = append([]ocispecs.Platform{}, def)

Expand Down
8 changes: 6 additions & 2 deletions worker/base/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,14 @@ func (w *Worker) Labels() map[string]string {

func (w *Worker) Platforms(noCache bool) []ocispecs.Platform {
if noCache {
matchers := make([]platforms.MatchComparer, len(w.WorkerOpt.Platforms))
for i, p := range w.WorkerOpt.Platforms {
matchers[i] = platforms.Only(p)
Copy link
Member Author

Choose a reason for hiding this comment

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

Although the effect of this isn't that significant compared to qemu detection logic, calling platforms.Only isn't completely free either as it internally allocates memory for the variant vector arrays.

}
for _, p := range archutil.SupportedPlatforms(noCache) {
exists := false
for _, pp := range w.WorkerOpt.Platforms {
if platforms.Only(pp).Match(p) {
for _, m := range matchers {
if m.Match(p) {
exists = true
break
}
Expand Down
Loading