From 3029f554cc26bff91504d6c4528179e0bc01eb63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 26 Jul 2023 13:28:18 +0200 Subject: [PATCH 1/4] c8d/readConfig: Translate c8d NotFound to errdefs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Gronowski (cherry picked from commit 7379d18018255069b03273589ad481b9a54d010b) Signed-off-by: Paweł Gronowski --- daemon/containerd/image_list.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/daemon/containerd/image_list.go b/daemon/containerd/image_list.go index e8afa43945..319467a449 100644 --- a/daemon/containerd/image_list.go +++ b/daemon/containerd/image_list.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" timetypes "github.com/docker/docker/api/types/time" + "github.com/docker/docker/errdefs" "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -479,11 +480,20 @@ func computeSharedSize(chainIDs []digest.Digest, layers map[digest.Digest]int, s func readConfig(ctx context.Context, store content.Provider, desc ocispec.Descriptor, out interface{}) error { data, err := content.ReadBlob(ctx, store, desc) if err != nil { - return errors.Wrapf(err, "failed to read config content") + err = errors.Wrapf(err, "failed to read config content") + if cerrdefs.IsNotFound(err) { + return errdefs.NotFound(err) + } + return err } + err = json.Unmarshal(data, out) if err != nil { - return errors.Wrapf(err, "could not deserialize image config") + err = errors.Wrapf(err, "could not deserialize image config") + if cerrdefs.IsNotFound(err) { + return errdefs.NotFound(err) + } + return err } return nil From fcb68e55fa4ff6503d222483af41097f0c513595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 26 Jul 2023 13:22:18 +0200 Subject: [PATCH 2/4] daemon/list: Replace ErrImageDoesNotExist check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check for generic `errdefs.NotFound` rather than specific error helper struct when checking if the error is caused by the image not being present. It still works for `ErrImageDoesNotExist` because it implements the NotFound errdefs interface too. Signed-off-by: Paweł Gronowski (cherry picked from commit 5a39bee63562490ba6f95c13fcac5fe1d46e330a) Signed-off-by: Paweł Gronowski --- daemon/list.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/daemon/list.go b/daemon/list.go index 676de6f222..003503ea96 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -11,7 +11,6 @@ import ( "github.com/docker/docker/api/types/filters" imagetypes "github.com/docker/docker/api/types/image" "github.com/docker/docker/container" - "github.com/docker/docker/daemon/images" "github.com/docker/docker/errdefs" "github.com/docker/docker/image" "github.com/docker/go-connections/nat" @@ -585,7 +584,7 @@ func (daemon *Daemon) refreshImage(ctx context.Context, s *container.Snapshot, f tmpImage := s.Image // keep the original ref if still valid (hasn't changed) if tmpImage != s.ImageID { img, err := daemon.imageService.GetImage(ctx, tmpImage, imagetypes.GetImageOpts{}) - if _, isDNE := err.(images.ErrImageDoesNotExist); err != nil && !isDNE { + if err != nil && !errdefs.IsNotFound(err) { return nil, err } if err != nil || img.ImageID() != s.ImageID { From 6c4121a943eed9c42d051ae94d21419c9021a0ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 26 Jul 2023 13:20:20 +0200 Subject: [PATCH 3/4] daemon/list: Refactor refreshImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add context comments and make it a bit more readable. Signed-off-by: Paweł Gronowski (cherry picked from commit 68991ae240ce56e302cc9297fcc4fa04d37f7c34) Signed-off-by: Paweł Gronowski --- daemon/list.go | 73 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 12 deletions(-) diff --git a/daemon/list.go b/daemon/list.go index 003503ea96..5b90fbbede 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -578,21 +578,70 @@ func includeContainerInList(container *container.Snapshot, filter *listContext) return includeContainer } -// refreshImage checks if the Image ref still points to the correct ID, and updates the ref to the actual ID when it doesn't +// refreshImage checks if the Image ref still points to the correct ID, and +// updates the ref to the actual ID when it doesn't. +// This happens when the image with a reference that was used to create +// container was deleted or updated and now resolves to a different ID. +// +// For example: +// $ docker run -d busybox:latest +// $ docker ps -a +// CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES +// b0318bca5aef busybox "sh" 4 seconds ago Exited (0) 3 seconds ago ecstatic_beaver +// +// After some time, busybox image got updated on the Docker Hub: +// $ docker pull busybox:latest +// +// So now busybox:latest points to a different digest, but that doesn't impact +// the ecstatic_beaver container which was still created under an older +// version. In this case, it should still point to the original image ID it was +// created from. +// +// $ docker ps -a +// CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES +// b0318bca5aef 3fbc63216742 "sh" 3 years ago Exited (0) 3 years ago ecstatic_beaver func (daemon *Daemon) refreshImage(ctx context.Context, s *container.Snapshot, filter *listContext) (*types.Container, error) { c := s.Container - tmpImage := s.Image // keep the original ref if still valid (hasn't changed) - if tmpImage != s.ImageID { - img, err := daemon.imageService.GetImage(ctx, tmpImage, imagetypes.GetImageOpts{}) - if err != nil && !errdefs.IsNotFound(err) { - return nil, err - } - if err != nil || img.ImageID() != s.ImageID { - // ref changed, we need to use original ID - tmpImage = s.ImageID - } + + // s.Image is the image reference passed by the user to create an image + // can be a: + // - name (like nginx, ubuntu:latest, docker.io/library/busybox:latest), + // - truncated ID (abcdef), + // - full digest (sha256:abcdef...) + // + // s.ImageID is the ID of the image that s.Image resolved to at the time + // of the container creation. It's always a full digest. + + // If these match, there's nothing to refresh. + if s.Image == s.ImageID { + return &c, nil } - c.Image = tmpImage + + // Check if the image reference still resolves to the same digest. + img, err := daemon.imageService.GetImage(ctx, s.Image, imagetypes.GetImageOpts{}) + + // If the image is no longer found or can't be resolved for some other + // reason. Update the Image to the specific ID of the original image it + // resolved to when the container was created. + if err != nil { + if !errdefs.IsNotFound(err) { + logrus.WithFields(logrus.Fields{ + logrus.ErrorKey: err, + "containerID": c.ID, + "image": s.Image, + "imageID": s.ImageID, + }).Warn("failed to resolve container image") + } + c.Image = s.ImageID + return &c, nil + } + + // Also update the image to the specific image ID, if the Image now + // resolves to a different ID. + if img.ImageID() != s.ImageID { + c.Image = s.ImageID + } + return &c, nil } From 45ba926c6d1d26992971e4b215719e3ead369b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Wed, 26 Jul 2023 14:52:11 +0200 Subject: [PATCH 4/4] daemon/list: Drop unused arg from containerReducer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refreshImage is the only function used as a reducer and it doesn't use the `filter *listContext`. Signed-off-by: Paweł Gronowski (cherry picked from commit 13180c1c4924ac2c8529507cf2405e3afe0a2413) Signed-off-by: Paweł Gronowski --- daemon/list.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/daemon/list.go b/daemon/list.go index 5b90fbbede..5057e188d8 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -41,7 +41,7 @@ type iterationAction int // containerReducer represents a reducer for a container. // Returns the object to serialize by the api. -type containerReducer func(context.Context, *container.Snapshot, *listContext) (*types.Container, error) +type containerReducer func(context.Context, *container.Snapshot) (*types.Container, error) const ( // includeContainer is the action to include a container in the reducer. @@ -230,7 +230,7 @@ func (daemon *Daemon) reducePsContainer(ctx context.Context, container *containe } // transform internal container struct into api structs - newC, err := reducer(ctx, container, filter) + newC, err := reducer(ctx, container) if err != nil { return nil, err } @@ -600,7 +600,7 @@ func includeContainerInList(container *container.Snapshot, filter *listContext) // $ docker ps -a // CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES // b0318bca5aef 3fbc63216742 "sh" 3 years ago Exited (0) 3 years ago ecstatic_beaver -func (daemon *Daemon) refreshImage(ctx context.Context, s *container.Snapshot, filter *listContext) (*types.Container, error) { +func (daemon *Daemon) refreshImage(ctx context.Context, s *container.Snapshot) (*types.Container, error) { c := s.Container // s.Image is the image reference passed by the user to create an image