compose: recreate container when mounted image digest changes #13549
Conversation
ndeloof
left a comment
ndeloof
left a comment
There was a problem hiding this comment.
I'm not a fan about using (yet another) labels for this.
As an alternative, I suggest compose could resolve digest for image used as volume source, and update service definition accordingly (comparable to docker compose config --resolve-image-digests). Doing so we get a service definition that can be compared with actual container state.
ibrahimypr
force-pushed
the
fix/image-mount-recreate
branch
from
38a9962 to
12e2cd1
Compare
|
Thanks for the feedback! I've updated the implementation to resolve the image digest directly in the volume source. |
pkg/compose/build.go
Outdated
| } | ||
| if img, ok := images[imgName]; ok { | ||
| // Append digest to source so config hash changes when image is rebuilt | ||
| service.Volumes[i].Source = fmt.Sprintf("%s@%s", vol.Source, img.ID) |
There was a problem hiding this comment.
better use of reference.ParseDockerRef() then reference.WithDigest() to enforce format follows expectations
| "gotest.tools/v3/assert" | ||
| ) | ||
|
|
||
| func TestMustRecreateImageMounts(t *testing.T) { |
There was a problem hiding this comment.
we prefer use of e2e test on docker/compose as those allows to detect actual issues with the engine API.
ibrahimypr
force-pushed
the
fix/image-mount-recreate
branch
from
12e2cd1 to
ac49d00
Compare
|
Sorry for the amateur mistakes, this is my first open source contribution! |
ibrahimypr
force-pushed
the
fix/image-mount-recreate
branch
from
ac49d00 to
193247d
Compare
ibrahimypr
force-pushed
the
fix/image-mount-recreate
branch
from
193247d to
822f608
Compare
|
Fixed subpath in e2e test fixture (must be relative path) causing daemon error. Also addressed linter feedback. |
pkg/compose/build.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func resolveImageVolumes(service types.ServiceConfig, images map[string]api.ImageSummary, projectName string) { |
There was a problem hiding this comment.
I wonder this works as expected as you don't pass a *types.ServiceConfig so struct is copied in function scope, leaving original unmodified.
Until now, mustRecreate logic only checked for divergence in TypeVolume mounts but ignored TypeImage mounts. This inconsistency caused containers to erroneously retain stale images even after the source image was rebuilt. This commit updates ensureImagesExists to resolve image volume sources to their digests using the official reference package. This enables ServiceHash (config hash) to naturally detect underlying image digest changes, triggering recreation via the standard convergence logic. An E2E test case is added to verify this behavior. Fixes docker#13547 Signed-off-by: ibrahim yapar <74625807+ibrahimypr@users.noreply.github.com>
ibrahimypr
force-pushed
the
fix/image-mount-recreate
branch
from
822f608 to
3463651
Compare
|
Thanks again for the review!
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
2 participants
What I did
Until now,
mustRecreatelogic only checked for divergence inTypeVolumemounts but ignoredTypeImagemounts. This inconsistency caused containers to erroneously retain stale images even after the source image was rebuilt.This commit introduces a new label
com.docker.compose.image.mountsthat tracks the digests of images used as mounts. The convergence logic is updated to verify this label, ensuring that containers are correctly recreated whenever the underlying image mount digest changes.Related issue
Fixes #13547