Remzona54/compose
1
0
Fork 0
mirror of https://github.com/docker/compose.git synced 2026-09-27 18:00:08 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix(pull): interpret pull_policy like up does

Browse source 
compose pull switched on the raw pull_policy string: daily/weekly/every_N
never matched a case and fell through to an unconditional re-pull, and
the hook-image loop was a second interpreter that ignored the refresh
window entirely. Delegate the decision to the exact interpreter the up
path uses (mustPull), with hook images routed through the same decision
(build mapped to missing — a hook image can't be built as a fallback).

Two deliberate differences with up are kept and documented in
shouldPullImage: a service without an explicit pull_policy is always
refreshed (skipping it would turn an explicit compose pull into a no-op
once images exist), and a present latest tag is still refreshed under
missing/if_not_present — the tag is expected to move, and triggering the
pull lets the daemon negotiate with the registry, a manifest check with
no download when the local image is already current.

User-visible change (changelog): compose pull now honors
daily/weekly/every_N refresh windows instead of always re-pulling.

Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
This commit is contained in:
Guillaume Lours 2026-08-06 15:45:33 +02:00 • committed by Guillaume Lours
parent 4b7f6149eb
commit d5e275dfb8
2 changed files with 138 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

104
pkg/compose/pull.go
View file

@ -77,24 +77,20 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts
continue
}
switch service.PullPolicy {
case types.PullPolicyNever, types.PullPolicyBuild:
pullRequired, skipReason, err := shouldPullImage(service, images)
if err != nil {
// join already-scheduled pulls before returning: bailing out with
// goroutines still in flight would leak them past pull()'s return
return errors.Join(err, eg.Wait())
}
if !pullRequired {
s.events.On(api.Resource{
ID: "Image " + service.Image,
Status: api.Done,
Text: "Skipped",
ID: "Image " + service.Image,
Status: api.Done,
Text: "Skipped",
Details: skipReason,
})
continue
case types.PullPolicyMissing, types.PullPolicyIfNotPresent:
if imageAlreadyPresent(service.Image, images) {
s.events.On(api.Resource{
ID: "Image " + service.Image,
Status: api.Done,
Text: "Skipped",
Details: "Image is already present locally",
})
continue
}
}
if service.Build != nil && opts.IgnoreBuildable {
@ -137,25 +133,31 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts
// pre_start hook images run as ephemeral init containers with their own
// registry image. They have no pull policy of their own, so we inherit the
// parent service's policy for skip decisions. Unlike the service image, a
// hook image can't be built, so `build` does not exempt it from pulling —
// only `never` does (consistent with the `up`/create path).
// parent service's policy for skip decisions — through the same
// shouldPullImage decision as the service image. Unlike the service
// image, a hook image can't be built, so `build` falls back to
// pull-if-missing instead of exempting it from pulling.
for name, service := range project.Services {
if service.PullPolicy == types.PullPolicyNever {
continue
hookPolicy := service.PullPolicy
if hookPolicy == types.PullPolicyBuild {
hookPolicy = types.PullPolicyMissing
}
for _, img := range api.GetDependentImages(service, project.Name) {
switch service.PullPolicy {
case types.PullPolicyMissing, types.PullPolicyIfNotPresent, types.PullPolicyBuild:
if imageAlreadyPresent(img, images) {
pullRequired, skipReason, err := shouldPullImage(types.ServiceConfig{Name: name, Image: img, PullPolicy: hookPolicy}, images)
if err != nil {
// same as the service loop: never leave scheduled pulls unjoined
return errors.Join(err, eg.Wait())
}
if !pullRequired {
if skipReason != "" {
s.events.On(api.Resource{
ID: "Image " + img,
Status: api.Done,
Text: "Skipped",
Details: "Image is already present locally",
Details: skipReason,
})
continue
}
continue
}
if _, ok := imagesBeingPulled[img]; ok {
continue
@ -188,19 +190,49 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts
return errors.Join(pullErrors...)
}
func imageAlreadyPresent(serviceImage string, localImages map[string]api.ImageSummary) bool {
normalizedImage, err := reference.ParseDockerRef(serviceImage)
// shouldPullImage decides whether `compose pull` refreshes a service's image,
// delegating to the exact pull_policy interpreter the up path uses (mustPull)
// so both commands honor never/build, skip-if-present and the
// daily/weekly/every_N refresh window identically. The command keeps two
// deliberate differences:
// - a service without an explicit pull_policy is always refreshed — an
// unset policy resolves to "missing" for `up`, but skipping it would turn
// an explicit `compose pull` into a no-op once images exist;
// - a present `latest` tag is still refreshed under missing/if_not_present:
// the tag is expected to move, and triggering the pull lets the daemon
// negotiate with the registry — a manifest check, no download, when the
// local image is already up to date.
func shouldPullImage(service types.ServiceConfig, images map[string]api.ImageSummary) (bool, string, error) {
if service.PullPolicy == "" {
return true, "", nil
}
pull, err := mustPull(service, images)
if err != nil || pull {
return pull, "", err
}
policy, _, _ := service.GetPullPolicy()
switch policy {
case types.PullPolicyRefresh:
return false, "Image is not due for refresh", nil
case types.PullPolicyMissing, types.PullPolicyIfNotPresent:
if isLatestTag(service.Image) {
return true, "", nil
}
return false, "Image is already present locally", nil
default: // never, build — and provider services short-circuited by mustPull
return false, "", nil
}
}
// isLatestTag reports whether ref points at a `latest` tag, including bare
// references that normalize to it.
func isLatestTag(ref string) bool {
named, err := reference.ParseDockerRef(ref)
if err != nil {
return false
}
switch refType := normalizedImage.(type) {
case reference.NamedTagged:
_, ok := localImages[serviceImage]
return ok && refType.Tag() != "latest"
default:
_, ok := localImages[serviceImage]
return ok
}
tagged, ok := named.(reference.Tagged)
return ok && tagged.Tag() == "latest"
}
func getUnwrappedErrorMessage(err error) string {
@ -286,7 +318,7 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser
}
s.events.On(newEvent(resource, api.Done, api.StatusPulled))
// Resolve the pulled image's identity exactly the way getImageSummaries
// Resolve the pulled image's identity exactly the way getLocalImagesDigests
// does for already-local images: both values feed the
// com.docker.compose.image label used to detect stale containers, so they
// must be computed identically. Returning the raw inspect ID here (the

81
pkg/compose/pull_test.go
View file

@ -22,9 +22,9 @@ import (
"iter"
"sort"
"testing"
"time"
"github.com/compose-spec/compose-go/v2/types"
"github.com/docker/cli/cli/config/configfile"
"github.com/moby/moby/api/types/image"
"github.com/moby/moby/api/types/jsonstream"
"github.com/moby/moby/client"
@ -133,14 +133,7 @@ func (fakePullResponse) JSONMessages(context.Context) iter.Seq2[jsonstream.Messa
func TestPullServiceImageUsesContentDigest(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mockAPI, cli := prepareMocks(mockCtrl)
cli.EXPECT().ConfigFile().Return(configfile.New("")).AnyTimes()
tested, err := NewComposeService(cli)
assert.NilError(t, err)
mockAPI.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}).
Return(client.PingResult{APIVersion: "1.48"}, nil).AnyTimes()
mockAPI.EXPECT().ClientVersion().Return("1.48").AnyTimes()
mockAPI, tested := newTestComposeService(t, mockCtrl, "1.48")
ref := "foo:1@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
mockAPI.EXPECT().
@ -157,8 +150,7 @@ func TestPullServiceImageUsesContentDigest(t *testing.T) {
ImageInspect(anyCancellableContext(), ref, gomock.Any()).
Return(client.ImageInspectResult{InspectResponse: inspect}, nil)
id, err := tested.(*composeService).
pullServiceImage(t.Context(), types.ServiceConfig{Name: "web", Image: ref}, true, "")
id, err := tested.pullServiceImage(t.Context(), types.ServiceConfig{Name: "web", Image: ref}, true, "")
assert.NilError(t, err)
assert.Equal(t, id, "sha256:image")
}
@ -176,3 +168,70 @@ func TestAddPreStartHookPulls_DedupsSharedHookImage(t *testing.T) {
assert.DeepEqual(t, scheduledHookImages(t, project, map[string]api.ImageSummary{}), []string{"init:latest"})
}
func TestShouldPullImage(t *testing.T) {
present := map[string]api.ImageSummary{
"web:1": {LastTagTime: time.Now()},
"web:latest": {LastTagTime: time.Now()},
"old:1": {LastTagTime: time.Now().Add(-48 * time.Hour)},
}
svc := func(image, policy string) types.ServiceConfig {
return types.ServiceConfig{Name: "web", Image: image, PullPolicy: policy}
}
t.Run("no explicit policy always refreshes", func(t *testing.T) {
pull, _, err := shouldPullImage(svc("web:1", ""), present)
assert.NilError(t, err)
assert.Assert(t, pull)
})
t.Run("never and build skip", func(t *testing.T) {
for _, policy := range []string{types.PullPolicyNever, types.PullPolicyBuild} {
pull, _, err := shouldPullImage(svc("web:1", policy), present)
assert.NilError(t, err)
assert.Assert(t, !pull)
}
})
t.Run("missing skips a present image", func(t *testing.T) {
pull, _, err := shouldPullImage(svc("web:1", types.PullPolicyMissing), present)
assert.NilError(t, err)
assert.Assert(t, !pull)
pull, _, err = shouldPullImage(svc("absent:1", types.PullPolicyMissing), present)
assert.NilError(t, err)
assert.Assert(t, pull)
})
t.Run("missing still refreshes a present latest tag", func(t *testing.T) {
// deliberate exception: `latest` is expected to move, so the pull is
// triggered anyway and the daemon's registry negotiation decides
// (a no-op when the local image is already up to date)
for _, image := range []string{"web:latest", "web"} {
pull, _, err := shouldPullImage(svc(image, types.PullPolicyMissing), map[string]api.ImageSummary{
image: {LastTagTime: time.Now()},
})
assert.NilError(t, err)
assert.Assert(t, pull, "present %s must still be refreshed", image)
}
})
t.Run("refresh policies honor the same window as up", func(t *testing.T) {
pull, _, err := shouldPullImage(svc("web:1", "daily"), present)
assert.NilError(t, err)
assert.Assert(t, !pull, "recently tagged image is not due for refresh")
pull, _, err = shouldPullImage(svc("old:1", "daily"), present)
assert.NilError(t, err)
assert.Assert(t, pull, "image older than the window must be refreshed")
pull, _, err = shouldPullImage(svc("absent:1", "weekly"), present)
assert.NilError(t, err)
assert.Assert(t, pull, "absent image must be pulled")
})
t.Run("invalid refresh spec errors", func(t *testing.T) {
_, _, err := shouldPullImage(svc("web:1", "every_bogus"), present)
assert.Assert(t, err != nil)
})
}