From 9dab00a76e499a2dedfd769278bc5b0b5d9c41dc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 9 Aug 2022 14:33:07 +0200 Subject: [PATCH 1/2] daemon/images: manifestMatchesPlatform() punch through context Signed-off-by: Sebastiaan van Stijn --- daemon/images/image.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/daemon/images/image.go b/daemon/images/image.go index ddd7e19ae3..b0069281cd 100644 --- a/daemon/images/image.go +++ b/daemon/images/image.go @@ -45,11 +45,10 @@ type manifest struct { Config specs.Descriptor `json:"config"` } -func (i *ImageService) manifestMatchesPlatform(img *image.Image, platform specs.Platform) bool { - ctx := context.TODO() +func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.Image, platform specs.Platform) bool { logger := logrus.WithField("image", img.ID).WithField("desiredPlatform", platforms.Format(platform)) - ls, leaseErr := i.leases.ListResources(context.TODO(), leases.Lease{ID: imageKey(img.ID().Digest())}) + ls, leaseErr := i.leases.ListResources(ctx, leases.Lease{ID: imageKey(img.ID().Digest())}) if leaseErr != nil { logger.WithError(leaseErr).Error("Error looking up image leases") return false @@ -150,6 +149,7 @@ func (i *ImageService) manifestMatchesPlatform(img *image.Image, platform specs. // GetImage returns an image corresponding to the image referred to by refOrID. func (i *ImageService) GetImage(refOrID string, options imagetypes.GetImageOpts) (retImg *image.Image, retErr error) { + ctx := context.TODO() defer func() { if retErr != nil || retImg == nil || options.Platform == nil { return @@ -168,7 +168,7 @@ func (i *ImageService) GetImage(refOrID string, options imagetypes.GetImageOpts) } // In some cases the image config can actually be wrong (e.g. classic `docker build` may not handle `--platform` correctly) // So we'll look up the manifest list that coresponds to this imaage to check if at least the manifest list says it is the correct image. - if i.manifestMatchesPlatform(retImg, p) { + if i.manifestMatchesPlatform(ctx, retImg, p) { return } From 779a5b3029473c6bb61c2d08a43e0a8d68a73d7f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 9 Aug 2022 14:42:50 +0200 Subject: [PATCH 2/2] ImageService.GetImage(): pass context Signed-off-by: Sebastiaan van Stijn Signed-off-by: Djordje Lukic --- api/server/router/image/backend.go | 2 +- api/server/router/image/image_routes.go | 2 +- daemon/cluster/executor/backend.go | 2 +- daemon/cluster/executor/container/adapter.go | 5 ++++- daemon/containerd/image.go | 4 +++- daemon/create.go | 11 +++++++++-- daemon/image_service.go | 2 +- daemon/images/cache.go | 5 ++++- daemon/images/image.go | 15 ++++++++------- daemon/images/image_builder.go | 8 +++++--- daemon/images/image_delete.go | 2 +- daemon/images/image_events.go | 5 ++++- daemon/images/image_history.go | 6 ++++-- daemon/images/image_list.go | 6 +++--- daemon/images/image_pull.go | 4 +++- daemon/images/image_tag.go | 5 ++++- daemon/list.go | 7 +++++-- daemon/oci_windows.go | 5 +++-- 18 files changed, 64 insertions(+), 32 deletions(-) diff --git a/api/server/router/image/backend.go b/api/server/router/image/backend.go index dcb190fb2b..71261f06b4 100644 --- a/api/server/router/image/backend.go +++ b/api/server/router/image/backend.go @@ -24,7 +24,7 @@ type imageBackend interface { ImageDelete(ctx context.Context, imageRef string, force, prune bool) ([]types.ImageDeleteResponseItem, error) ImageHistory(imageName string) ([]*image.HistoryResponseItem, error) Images(ctx context.Context, opts types.ImageListOptions) ([]*types.ImageSummary, error) - GetImage(refOrID string, options image.GetImageOpts) (*dockerimage.Image, error) + GetImage(ctx context.Context, refOrID string, options image.GetImageOpts) (*dockerimage.Image, error) TagImage(imageName, repository, tag string) (string, error) ImagesPrune(ctx context.Context, pruneFilters filters.Args) (*types.ImagesPruneReport, error) } diff --git a/api/server/router/image/image_routes.go b/api/server/router/image/image_routes.go index e2654cb279..724e4e18c6 100644 --- a/api/server/router/image/image_routes.go +++ b/api/server/router/image/image_routes.go @@ -193,7 +193,7 @@ func (ir *imageRouter) deleteImages(ctx context.Context, w http.ResponseWriter, } func (ir *imageRouter) getImagesByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - img, err := ir.backend.GetImage(vars["name"], opts.GetImageOpts{}) + img, err := ir.backend.GetImage(ctx, vars["name"], opts.GetImageOpts{}) if err != nil { return err } diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index 8aab1b13e0..d4a4bd318b 100644 --- a/daemon/cluster/executor/backend.go +++ b/daemon/cluster/executor/backend.go @@ -77,5 +77,5 @@ type VolumeBackend interface { type ImageBackend interface { PullImage(ctx context.Context, image, tag string, platform *specs.Platform, metaHeaders map[string][]string, authConfig *registry.AuthConfig, outStream io.Writer) error GetRepository(context.Context, reference.Named, *registry.AuthConfig) (distribution.Repository, error) - GetImage(refOrID string, options opts.GetImageOpts) (*image.Image, error) + GetImage(ctx context.Context, refOrID string, options opts.GetImageOpts) (*image.Image, error) } diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 8e9aa108ac..1256f49019 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -76,7 +76,10 @@ func (c *containerAdapter) pullImage(ctx context.Context) error { named, err := reference.ParseNormalizedNamed(spec.Image) if err == nil { if _, ok := named.(reference.Canonical); ok { - _, err := c.imageBackend.GetImage(spec.Image, imagetypes.GetImageOpts{}) + _, err := c.imageBackend.GetImage(ctx, spec.Image, imagetypes.GetImageOpts{}) + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return err + } if err == nil { return nil } diff --git a/daemon/containerd/image.go b/daemon/containerd/image.go index 6405ec63a7..6f139aa392 100644 --- a/daemon/containerd/image.go +++ b/daemon/containerd/image.go @@ -1,11 +1,13 @@ package containerd import ( + "context" + imagetype "github.com/docker/docker/api/types/image" "github.com/docker/docker/image" ) // GetImage returns an image corresponding to the image referred to by refOrID. -func (i *ImageService) GetImage(refOrID string, options imagetype.GetImageOpts) (retImg *image.Image, retErr error) { +func (i *ImageService) GetImage(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (retImg *image.Image, retErr error) { panic("not implemented") } diff --git a/daemon/create.go b/daemon/create.go index c843381ce0..c016db7fb2 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -1,6 +1,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( + "context" "fmt" "net" "runtime" @@ -58,6 +59,7 @@ func (daemon *Daemon) ContainerCreateIgnoreImagesArgsEscaped(params types.Contai } func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.CreateResponse, error) { + ctx := context.TODO() start := time.Now() if opts.params.Config == nil { return containertypes.CreateResponse{}, errdefs.InvalidParameter(errors.New("Config cannot be empty in order to create a container")) @@ -69,7 +71,11 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.CreateRes } if opts.params.Platform == nil && opts.params.Config.Image != "" { - if img, _ := daemon.imageService.GetImage(opts.params.Config.Image, imagetypes.GetImageOpts{Platform: opts.params.Platform}); img != nil { + img, err := daemon.imageService.GetImage(ctx, opts.params.Config.Image, imagetypes.GetImageOpts{Platform: opts.params.Platform}) + if err != nil { + return containertypes.CreateResponse{}, err + } + if img != nil { p := maximumSpec() imgPlat := v1.Platform{ OS: img.OS, @@ -111,6 +117,7 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.CreateRes // Create creates a new container from the given configuration with a given name. func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr error) { + ctx := context.TODO() var ( ctr *container.Container img *image.Image @@ -120,7 +127,7 @@ func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr ) if opts.params.Config.Image != "" { - img, err = daemon.imageService.GetImage(opts.params.Config.Image, imagetypes.GetImageOpts{Platform: opts.params.Platform}) + img, err = daemon.imageService.GetImage(ctx, opts.params.Config.Image, imagetypes.GetImageOpts{Platform: opts.params.Platform}) if err != nil { return nil, err } diff --git a/daemon/image_service.go b/daemon/image_service.go index e6be1c763f..d0969dba33 100644 --- a/daemon/image_service.go +++ b/daemon/image_service.go @@ -40,7 +40,7 @@ type ImageService interface { ImportImage(src string, repository string, platform *v1.Platform, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error TagImage(imageName, repository, tag string) (string, error) TagImageWithReference(imageID image.ID, newTag reference.Named) error - GetImage(refOrID string, options imagetype.GetImageOpts) (*image.Image, error) + GetImage(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*image.Image, error) ImageHistory(name string) ([]*imagetype.HistoryResponseItem, error) CommitImage(c backend.CommitConfig) (image.ID, error) SquashImage(id, parent string) (string, error) diff --git a/daemon/images/cache.go b/daemon/images/cache.go index d81136b8a3..65730c8a98 100644 --- a/daemon/images/cache.go +++ b/daemon/images/cache.go @@ -1,6 +1,8 @@ package images // import "github.com/docker/docker/daemon/images" import ( + "context" + imagetypes "github.com/docker/docker/api/types/image" "github.com/docker/docker/builder" "github.com/docker/docker/image/cache" @@ -9,6 +11,7 @@ import ( // MakeImageCache creates a stateful image cache. func (i *ImageService) MakeImageCache(sourceRefs []string) builder.ImageCache { + ctx := context.TODO() if len(sourceRefs) == 0 { return cache.NewLocal(i.imageStore) } @@ -16,7 +19,7 @@ func (i *ImageService) MakeImageCache(sourceRefs []string) builder.ImageCache { cache := cache.New(i.imageStore) for _, ref := range sourceRefs { - img, err := i.GetImage(ref, imagetypes.GetImageOpts{}) + img, err := i.GetImage(ctx, ref, imagetypes.GetImageOpts{}) if err != nil { logrus.Warnf("Could not look up %s for cache resolution, skipping: %+v", ref, err) continue diff --git a/daemon/images/image.go b/daemon/images/image.go index b0069281cd..12d6cd6c91 100644 --- a/daemon/images/image.go +++ b/daemon/images/image.go @@ -45,13 +45,13 @@ type manifest struct { Config specs.Descriptor `json:"config"` } -func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.Image, platform specs.Platform) bool { +func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.Image, platform specs.Platform) (bool, error) { logger := logrus.WithField("image", img.ID).WithField("desiredPlatform", platforms.Format(platform)) ls, leaseErr := i.leases.ListResources(ctx, leases.Lease{ID: imageKey(img.ID().Digest())}) if leaseErr != nil { logger.WithError(leaseErr).Error("Error looking up image leases") - return false + return false, leaseErr } // Note we are comparing against manifest lists here, which we expect to always have a CPU variant set (where applicable). @@ -137,19 +137,18 @@ func (i *ImageService) manifestMatchesPlatform(ctx context.Context, img *image.I if m.Config.Digest == img.ID().Digest() { logger.WithField("manifestDigest", md.Digest).Debug("Found matching manifest for image") - return true + return true, nil } logger.WithField("otherDigest", md.Digest).Debug("Skipping non-matching manifest") } } - return false + return false, nil } // GetImage returns an image corresponding to the image referred to by refOrID. -func (i *ImageService) GetImage(refOrID string, options imagetypes.GetImageOpts) (retImg *image.Image, retErr error) { - ctx := context.TODO() +func (i *ImageService) GetImage(ctx context.Context, refOrID string, options imagetypes.GetImageOpts) (retImg *image.Image, retErr error) { defer func() { if retErr != nil || retImg == nil || options.Platform == nil { return @@ -168,7 +167,9 @@ func (i *ImageService) GetImage(refOrID string, options imagetypes.GetImageOpts) } // In some cases the image config can actually be wrong (e.g. classic `docker build` may not handle `--platform` correctly) // So we'll look up the manifest list that coresponds to this imaage to check if at least the manifest list says it is the correct image. - if i.manifestMatchesPlatform(ctx, retImg, p) { + var matches bool + matches, retErr = i.manifestMatchesPlatform(ctx, retImg, p) + if matches || retErr != nil { return } diff --git a/daemon/images/image_builder.go b/daemon/images/image_builder.go index 5ec5442eb2..10980c6561 100644 --- a/daemon/images/image_builder.go +++ b/daemon/images/image_builder.go @@ -168,7 +168,7 @@ func (i *ImageService) pullForBuilder(ctx context.Context, name string, authConf return nil, err } - img, err := i.GetImage(name, imagetypes.GetImageOpts{Platform: platform}) + img, err := i.GetImage(ctx, name, imagetypes.GetImageOpts{Platform: platform}) if errdefs.IsNotFound(err) && img != nil && platform != nil { imgPlat := specs.Platform{ OS: img.OS, @@ -212,11 +212,13 @@ func (i *ImageService) GetImageAndReleasableLayer(ctx context.Context, refOrID s } if opts.PullOption != backend.PullOptionForcePull { - img, err := i.GetImage(refOrID, imagetypes.GetImageOpts{Platform: opts.Platform}) + img, err := i.GetImage(ctx, refOrID, imagetypes.GetImageOpts{Platform: opts.Platform}) if err != nil && opts.PullOption == backend.PullOptionNoPull { return nil, nil, err } - // TODO: shouldn't we error out if error is different from "not found" ? + if err != nil && !errdefs.IsNotFound(err) { + return nil, nil, err + } if img != nil { if !system.IsOSSupported(img.OperatingSystem()) { return nil, nil, system.ErrNotSupportedOperatingSystem diff --git a/daemon/images/image_delete.go b/daemon/images/image_delete.go index 89dab288a0..d20de8dae9 100644 --- a/daemon/images/image_delete.go +++ b/daemon/images/image_delete.go @@ -64,7 +64,7 @@ func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force, start := time.Now() records := []types.ImageDeleteResponseItem{} - img, err := i.GetImage(imageRef, imagetypes.GetImageOpts{}) + img, err := i.GetImage(ctx, imageRef, imagetypes.GetImageOpts{}) if err != nil { return nil, err } diff --git a/daemon/images/image_events.go b/daemon/images/image_events.go index e3e5952587..1a824f5c64 100644 --- a/daemon/images/image_events.go +++ b/daemon/images/image_events.go @@ -1,6 +1,8 @@ package images // import "github.com/docker/docker/daemon/images" import ( + "context" + "github.com/docker/docker/api/types/events" imagetypes "github.com/docker/docker/api/types/image" ) @@ -12,7 +14,8 @@ func (i *ImageService) LogImageEvent(imageID, refName, action string) { // LogImageEventWithAttributes generates an event related to an image with specific given attributes. func (i *ImageService) LogImageEventWithAttributes(imageID, refName, action string, attributes map[string]string) { - img, err := i.GetImage(imageID, imagetypes.GetImageOpts{}) + ctx := context.TODO() + img, err := i.GetImage(ctx, imageID, imagetypes.GetImageOpts{}) if err == nil && img.Config != nil { // image has not been removed yet. // it could be missing if the event is `delete`. diff --git a/daemon/images/image_history.go b/daemon/images/image_history.go index 135eb7d13f..d32c873925 100644 --- a/daemon/images/image_history.go +++ b/daemon/images/image_history.go @@ -1,6 +1,7 @@ package images // import "github.com/docker/docker/daemon/images" import ( + "context" "fmt" "time" @@ -12,8 +13,9 @@ import ( // ImageHistory returns a slice of ImageHistory structures for the specified image // name by walking the image lineage. func (i *ImageService) ImageHistory(name string) ([]*image.HistoryResponseItem, error) { + ctx := context.TODO() start := time.Now() - img, err := i.GetImage(name, image.GetImageOpts{}) + img, err := i.GetImage(ctx, name, image.GetImageOpts{}) if err != nil { return nil, err } @@ -69,7 +71,7 @@ func (i *ImageService) ImageHistory(name string) ([]*image.HistoryResponseItem, if id == "" { break } - histImg, err = i.GetImage(id.String(), image.GetImageOpts{}) + histImg, err = i.GetImage(ctx, id.String(), image.GetImageOpts{}) if err != nil { break } diff --git a/daemon/images/image_list.go b/daemon/images/image_list.go index 32bb366d2c..f898f7249a 100644 --- a/daemon/images/image_list.go +++ b/daemon/images/image_list.go @@ -31,7 +31,7 @@ func (r byCreated) Swap(i, j int) { r[i], r[j] = r[j], r[i] } func (r byCreated) Less(i, j int) bool { return r[i].Created < r[j].Created } // Images returns a filtered list of images. -func (i *ImageService) Images(_ context.Context, opts types.ImageListOptions) ([]*types.ImageSummary, error) { +func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions) ([]*types.ImageSummary, error) { if err := opts.Filters.Validate(acceptedImageFilterTags); err != nil { return nil, err } @@ -50,7 +50,7 @@ func (i *ImageService) Images(_ context.Context, opts types.ImageListOptions) ([ err error ) err = opts.Filters.WalkValues("before", func(value string) error { - beforeFilter, err = i.GetImage(value, imagetypes.GetImageOpts{}) + beforeFilter, err = i.GetImage(ctx, value, imagetypes.GetImageOpts{}) return err }) if err != nil { @@ -58,7 +58,7 @@ func (i *ImageService) Images(_ context.Context, opts types.ImageListOptions) ([ } err = opts.Filters.WalkValues("since", func(value string) error { - sinceFilter, err = i.GetImage(value, imagetypes.GetImageOpts{}) + sinceFilter, err = i.GetImage(ctx, value, imagetypes.GetImageOpts{}) return err }) if err != nil { diff --git a/daemon/images/image_pull.go b/daemon/images/image_pull.go index 0ad3eec99a..7b248850ac 100644 --- a/daemon/images/image_pull.go +++ b/daemon/images/image_pull.go @@ -64,7 +64,7 @@ func (i *ImageService) PullImage(ctx context.Context, image, tag string, platfor // we allow the image to have a non-matching architecture. The code // below checks for this situation, and returns a warning to the client, // as well as logging it to the daemon logs. - img, err := i.GetImage(image, imagetypes.GetImageOpts{Platform: platform}) + img, err := i.GetImage(ctx, image, imagetypes.GetImageOpts{Platform: platform}) // Note that this is a special case where GetImage returns both an image // and an error: https://github.com/docker/docker/blob/v20.10.7/daemon/images/image.go#L175-L183 @@ -72,6 +72,8 @@ func (i *ImageService) PullImage(ctx context.Context, image, tag string, platfor po := streamformatter.NewJSONProgressOutput(outStream, false) progress.Messagef(po, "", `WARNING: %s`, err.Error()) logrus.WithError(err).WithField("image", image).Warn("ignoring platform mismatch on single-arch image") + } else if err != nil { + return err } } diff --git a/daemon/images/image_tag.go b/daemon/images/image_tag.go index 16a7aaea2c..0f39bbc6f6 100644 --- a/daemon/images/image_tag.go +++ b/daemon/images/image_tag.go @@ -1,6 +1,8 @@ package images // import "github.com/docker/docker/daemon/images" import ( + "context" + "github.com/docker/distribution/reference" imagetypes "github.com/docker/docker/api/types/image" "github.com/docker/docker/image" @@ -9,7 +11,8 @@ import ( // TagImage creates the tag specified by newTag, pointing to the image named // imageName (alternatively, imageName can also be an image ID). func (i *ImageService) TagImage(imageName, repository, tag string) (string, error) { - img, err := i.GetImage(imageName, imagetypes.GetImageOpts{}) + ctx := context.TODO() + img, err := i.GetImage(ctx, imageName, imagetypes.GetImageOpts{}) if err != nil { return "", err } diff --git a/daemon/list.go b/daemon/list.go index f1083d4f62..7c796939a7 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -1,6 +1,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( + "context" "fmt" "sort" "strconv" @@ -243,6 +244,7 @@ func (daemon *Daemon) reducePsContainer(container *container.Snapshot, filter *l // foldFilter generates the container filter based on the user's filtering options. func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerListOptions) (*listContext, error) { + ctx := context.TODO() psFilters := config.Filters var filtExited []int @@ -318,7 +320,7 @@ func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerLis if psFilters.Contains("ancestor") { ancestorFilter = true psFilters.WalkValues("ancestor", func(ancestor string) error { - img, err := daemon.imageService.GetImage(ancestor, imagetypes.GetImageOpts{}) + img, err := daemon.imageService.GetImage(ctx, ancestor, imagetypes.GetImageOpts{}) if err != nil { logrus.Warnf("Error while looking up for image %v", ancestor) return nil @@ -579,10 +581,11 @@ func includeContainerInList(container *container.Snapshot, filter *listContext) // refreshImage checks if the Image ref still points to the correct ID, and updates the ref to the actual ID when it doesn't func (daemon *Daemon) refreshImage(s *container.Snapshot, filter *listContext) (*types.Container, error) { + ctx := context.TODO() 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(tmpImage, imagetypes.GetImageOpts{}) + img, err := daemon.imageService.GetImage(ctx, tmpImage, imagetypes.GetImageOpts{}) if _, isDNE := err.(images.ErrImageDoesNotExist); err != nil && !isDNE { return nil, err } diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go index 839daa2d98..502ce10102 100644 --- a/daemon/oci_windows.go +++ b/daemon/oci_windows.go @@ -1,6 +1,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( + "context" "encoding/json" "fmt" "os" @@ -26,8 +27,8 @@ const ( ) func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { - - img, err := daemon.imageService.GetImage(string(c.ImageID), imagetypes.GetImageOpts{}) + ctx := context.TODO() + img, err := daemon.imageService.GetImage(ctx, string(c.ImageID), imagetypes.GetImageOpts{}) if err != nil { return nil, err }