Resolve pre_start hook images alongside service images#13937
Conversation
pre_start hooks run as ephemeral init containers with their own image (ServiceHook.Image), but that image was ignored by image resolution: `config --images` didn't list it, `pull` didn't fetch it, and `up` failed at runtime with "No such image" when it wasn't already present locally. Add a GetDependentImages helper that returns a service's pre_start hook images, and use it wherever service images are collected/pulled: getLocalImagesDigests, pullRequiredImages (up path), the pull command, and `config --images`. Hook images inherit the parent service pull policy. post_start/pre_stop hooks run via ExecCreate inside the service container and never use hook.Image, so they are intentionally out of scope. Digest resolution/locking (--resolve-image-digests / --lock-image-digests) is not covered: compose-go's WithImagesResolved only resolves service.Image (needs an upstream change), and the --lock-image-digests override merges pre_start by concatenation, which would duplicate hooks. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The PR correctly surfaces pre_start hook images for resolution, pulling, and listing. The core logic in GetDependentImages, the goroutine closures in pull(), and error propagation are all sound. No high or medium severity bugs were confirmed.
Summary of analysis:
GetDependentImagescorrectly filters empty images and only includespre_starthooks (notpost_start/pre_stop, which have no image to pull).pull()correctly inherits the parent service's pull policy for hook images, including skipping onPullPolicyNever/PullPolicyBuild, checking local presence forPullPolicyMissing/PullPolicyIfNotPresent, and always pulling forPullPolicyAlways.pullRequiredImages()follows the same "pull if missing" pattern as the existing volume-image block — consistent with the function's existing design.- Goroutine closures are safe:
hookServiceis a local copy per iteration. - Error propagation from hook-image pulls goes through
eg.Wait()and is correctly returned to callers.
There was a problem hiding this comment.
Pull request overview
This PR fixes image resolution/pulling for pre_start lifecycle hooks by treating hook images as dependent images of a service, ensuring they are listed by docker compose config --images and pulled by both docker compose pull and the up/create pull path.
Changes:
- Add
api.GetDependentImages(service)to exposepre_starthook images as additional service-dependent images. - Include hook images in local image digest inspection and image pulling logic.
- Extend
config --imagesoutput to includepre_starthook images.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/api/api.go | Adds GetDependentImages helper for pre_start hook images. |
| pkg/api/api_test.go | Adds unit tests for GetDependentImages. |
| pkg/compose/build.go | Includes hook images when collecting local image digests. |
| pkg/compose/build_test.go | Adds unit test validating hook image digest inspection. |
| pkg/compose/pull.go | Pulls missing pre_start hook images in both pull and up/create pull paths. |
| cmd/compose/config.go | Updates config --images to print hook images. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Address review feedback on pre_start hook image resolution: 1. GetDependentImages now skips a hook image equal to the service image (resolved via GetImageNameOrDefault), so `config --images` no longer prints a duplicate line and pullRequiredImages no longer schedules a redundant pull for it. 2. pullRequiredImages (up/create path) now dedups dependent images by reference via a `scheduled` set, so several hooks/services sharing the same missing image don't schedule concurrent redundant pulls. The hook pass moved to a helper (addPreStartHookPulls) to keep complexity in check. 3. The `pull` command no longer skips hook images under `pull_policy: build`. A hook image is a registry image that can't be built, so only `never` justifies skipping it — making `pull` consistent with the `up` path. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
This PR correctly wires pre_start hook images into docker compose config --images, docker compose pull, and the up/create pull path. The overall approach is sound. One inconsistency was found in the up/create path: hook images are not force-pulled when the parent service has pull_policy: always, while service images are. See the inline comment for details.
addPreStartHookPulls skipped any hook image already present locally, regardless of pull policy. As a result a service with pull_policy: always had its own image force-pulled on every up while its pre_start hook images were left stale — diverging from both the service image and the `pull` command path (which already re-pulls hooks under always). Skip the "already present" shortcut when the parent service is pull_policy: always, so hook images get the same force-pull treatment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
What I did
Fixes #13924.
pre_starthooks run as ephemeral init containers with their own image (ServiceHook.Image), but that image was ignored by Compose image resolution:docker compose config --imagesdidn't list itdocker compose pulldidn't fetch itdocker compose upfailed at runtime withNo such imagewhen the hook image wasn't already present locally (the hook has no on-the-fly pull —createPreStartContainerjust callsContainerCreatewithhook.Image)This PR treats
pre_starthook images like service images. A newapi.GetDependentImages(service)helper returns a service'spre_starthook images, and it is used wherever service images are collected/pulled:pkg/api/api.goGetDependentImageshelperpkg/compose/build.go—getLocalImagesDigestspkg/compose/pull.go—pullRequiredImages(up/createpath)VolumeTypeImage)pkg/compose/pull.go—pull(thepullcommand)cmd/compose/config.go—runConfigImagesScope
Only
pre_startis in scope.post_start/pre_stophooks run viaExecCreateinside the service container (pkg/compose/hook.go) and never usehook.Image, so there is nothing to pull for them. An empty hook image falls back to the service image, which is already resolved.Known limitations / follow-ups
Digest resolution and locking are not covered by this PR, deliberately:
config --resolve-image-digests:compose-go'sProject.WithImagesResolvedonly resolvesservice.Image, never hook images. Covering hooks cleanly requires extendingWithImagesResolvedupstream — done in Resolve dependent images (pre_start hooks, type: image volumes) in WithImagesResolved compose-spec/compose-go#894, to be wired here once it is released and vendored.config --lock-image-digests: this produces an override file that is re-merged, butpre_starthas no special merge rule and its list is merged by concatenation (override/merge.go), so a hook override would duplicate the hook instead of pinning it. To be addressed together with / after the compose-go change.Tests
Unit tests only:
TestGetDependentImages(pkg/api) — with/without hook image, and confirmspost_start/pre_stopare not collected.TestGetLocalImagesDigests_PreStartHook(pkg/compose) — the hook image (alpine:3.19) is inspected alongside the service image (alpine:3.20).How to test manually