Skip to content

Commit e9db1b0

Browse files
authored
Merge pull request #1176 from jonjohnsonjr/lock-bases
2 parents 93d195e + c1c4af0 commit e9db1b0

File tree

2 files changed

+32
-10
lines changed

2 files changed

+32
-10
lines changed

pkg/commands/cache.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,12 @@ import (
3838
type imageCache struct {
3939
// In memory
4040
cache sync.Map
41-
p *layout.Path
4241

42+
// On disk
43+
mu sync.Mutex
44+
p *layout.Path
45+
46+
// Over the network
4347
puller *remote.Puller
4448
}
4549

@@ -101,10 +105,15 @@ func (i *imageCache) get(ctx context.Context, ref name.Reference, missFunc baseF
101105
key = dig.String()
102106
}
103107

108+
// Use a pretty broad lock on the on-disk cache to avoid races.
109+
i.mu.Lock()
110+
defer i.mu.Unlock()
111+
104112
ii, err := i.p.ImageIndex()
105113
if err != nil {
106-
return nil, err
114+
return nil, fmt.Errorf("loading cache index: %w", err)
107115
}
116+
108117
h, err := v1.NewHash(key)
109118
if err != nil {
110119
return nil, err

pkg/commands/config.go

+21-8
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,6 @@ func getBaseImage(bo *options.BuildOptions) build.GetBase {
7575
}
7676

7777
fetch := func(ctx context.Context, ref name.Reference) (build.Result, error) {
78-
// For ko.local, look in the daemon.
79-
if ref.Context().RegistryStr() == publish.LocalDomain {
80-
return daemon.Image(ref)
81-
}
82-
8378
ropt = append(ropt, remote.WithContext(ctx))
8479

8580
desc, err := remote.Get(ref, ropt...)
@@ -91,6 +86,7 @@ func getBaseImage(bo *options.BuildOptions) build.GetBase {
9186
}
9287
return desc.Image()
9388
}
89+
9490
return func(ctx context.Context, s string) (name.Reference, build.Result, error) {
9591
s = strings.TrimPrefix(s, build.StrictScheme)
9692
// Viper configuration file keys are case insensitive, and are
@@ -112,9 +108,26 @@ func getBaseImage(bo *options.BuildOptions) build.GetBase {
112108
return nil, nil, fmt.Errorf("parsing base image (%q): %w", baseImage, err)
113109
}
114110

115-
result, err := cache.get(ctx, ref, fetch)
116-
if err != nil {
117-
return ref, result, err
111+
var result build.Result
112+
113+
// For ko.local, look in the daemon.
114+
if ref.Context().RegistryStr() == publish.LocalDomain {
115+
result, err = daemon.Image(ref)
116+
if err != nil {
117+
return nil, nil, fmt.Errorf("loading %s from daemon: %w", ref, err)
118+
}
119+
} else {
120+
result, err = cache.get(ctx, ref, fetch)
121+
if err != nil {
122+
// We don't expect this to fail, usually, but the cache should also not be fatal.
123+
// Log it so people can complain about it and we can fix the cache.
124+
log.Printf("cache.get(%q) failed with %v", ref.String(), err)
125+
126+
result, err = fetch(ctx, ref)
127+
if err != nil {
128+
return nil, nil, fmt.Errorf("pulling %s: %w", ref, err)
129+
}
130+
}
118131
}
119132

120133
if _, ok := ref.(name.Digest); ok {

0 commit comments

Comments
 (0)