diff --git a/types/project.go b/types/project.go index 20cc645f..66365fae 100644 --- a/types/project.go +++ b/types/project.go @@ -26,6 +26,7 @@ import ( "path/filepath" "slices" "sort" + "sync" "github.com/compose-spec/compose-go/v2/dotenv" "github.com/compose-spec/compose-go/v2/errdefs" @@ -583,15 +584,32 @@ func (p *Project) WithServicesDisabled(names ...string) *Project { // - `type: image` volume sources, unless they reference another service by name (those are // resolved to a locally built image rather than a registry digest) func (p *Project) WithImagesResolved(resolver func(named reference.Named) (godigest.Digest, error)) (*Project, error) { - // Deduplicate resolutions per raw image string across the whole call: transforms run - // concurrently (one goroutine per service), so images shared by several services (or a - // hook image equal to its service image, which the loader copies at load time) would - // otherwise trigger duplicate registry round-trips. singleflight collapses concurrent - // resolutions of the same image into a single call, sharing the result. - var group singleflight.Group + // Deduplicate resolutions per raw image string across the whole call, on two axes: + // - cache (sync.Map) memoizes results for the whole call, so images resolved at + // different times — e.g. a hook or volume source equal to service.Image, resolved + // sequentially within the same service — are resolved once. + // - singleflight collapses concurrent resolutions of the same image (transforms run + // one goroutine per service) into a single call before it reaches the cache. + // Neither alone suffices: the cache lets a concurrent burst miss simultaneously and + // all resolve, while singleflight forgets a key as soon as its call returns. + var ( + cache sync.Map // map[string]string + group singleflight.Group + ) resolve := func(image string) (string, error) { + if r, ok := cache.Load(image); ok { + return r.(string), nil + } r, err, _ := group.Do(image, func() (any, error) { - return resolveImageDigest(image, resolver) + if r, ok := cache.Load(image); ok { + return r, nil + } + r, err := resolveImageDigest(image, resolver) + if err != nil { + return image, err + } + cache.Store(image, r) + return r, nil }) if err != nil { return image, err diff --git a/types/project_test.go b/types/project_test.go index 8baee1b9..1abd5ca6 100644 --- a/types/project_test.go +++ b/types/project_test.go @@ -23,6 +23,8 @@ import ( "slices" "sort" "strings" + "sync" + "sync/atomic" "testing" "github.com/compose-spec/compose-go/v2/utils" @@ -321,6 +323,42 @@ func Test_ResolveImages_imageVolumeDisabledService(t *testing.T) { assert.Equal(t, p.Services["service_1"].Volumes[0].Source, "Builder") } +func Test_ResolveImages_deduplicated(t *testing.T) { + const digested = "sha256:1234567890123456789012345678901234567890123456789012345678901234" + var calls sync.Map // image string -> *int32 call count + resolver := func(named reference.Named) (digest.Digest, error) { + c, _ := calls.LoadOrStore(named.String(), new(int32)) + atomic.AddInt32(c.(*int32), 1) + return digested, nil + } + p := &Project{ + Services: Services{ + // service_1 references alpine:3.20 three times within a single + // transform (image, hook, volume) — the sequential lookups must + // collapse to a single resolver call. + "service_1": { + Name: "service_1", + Image: "alpine:3.20", + PreStart: []ServiceHook{{Image: "alpine:3.20", Command: ShellCommand{"echo"}}}, + Volumes: []ServiceVolumeConfig{{Type: VolumeTypeImage, Source: "alpine:3.20", Target: "/data"}}, + }, + // service_2 shares the same image, resolved from a concurrent + // transform — must also be served from the shared cache. + "service_2": { + Name: "service_2", + Image: "alpine:3.20", + }, + }, + } + + _, err := p.WithImagesResolved(resolver) + assert.NilError(t, err) + + c, ok := calls.Load("docker.io/library/alpine:3.20") + assert.Assert(t, ok, "resolver was never called for alpine:3.20") + assert.Equal(t, atomic.LoadInt32(c.(*int32)), int32(1), "alpine:3.20 should be resolved exactly once") +} + func Test_ResolveImages_concurrent(t *testing.T) { const garfield = "sha256:1234567890123456789012345678901234567890123456789012345678901234" resolver := func(_ reference.Named) (digest.Digest, error) {