From f865456bfbf9da5b9c9c3b50478a19570c87e2d3 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 30 Oct 2018 17:05:58 -0700 Subject: [PATCH] Add image cache Use cache for delete and listing images Signed-off-by: Derek McGowan --- api/server/router/image/backend.go | 4 +- api/server/router/image/image_routes.go | 4 +- daemon/daemon.go | 6 + daemon/disk_usage.go | 2 +- daemon/images/cache.go | 202 +++++++++++++++++++++++- daemon/images/image.go | 72 +++++++++ daemon/images/image_delete.go | 202 ++++++++++++------------ daemon/images/image_prune.go | 4 +- daemon/images/image_pull.go | 32 +++- daemon/images/images.go | 44 ++---- daemon/images/service.go | 9 ++ 11 files changed, 437 insertions(+), 144 deletions(-) diff --git a/api/server/router/image/backend.go b/api/server/router/image/backend.go index 5837f9a9bcdfd..a8187a4b42132 100644 --- a/api/server/router/image/backend.go +++ b/api/server/router/image/backend.go @@ -20,9 +20,9 @@ type Backend interface { } type imageBackend interface { - ImageDelete(imageRef string, force, prune bool) ([]types.ImageDeleteResponseItem, error) + ImageDelete(ctx context.Context, imageRef string, force, prune bool) ([]types.ImageDeleteResponseItem, error) ImageHistory(imageName string) ([]*image.HistoryResponseItem, error) - Images(imageFilters filters.Args, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error) + Images(ctx context.Context, imageFilters filters.Args, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error) LookupImage(name string) (*types.ImageInspect, 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 1f7d81adfaf72..1440e50489dd2 100644 --- a/api/server/router/image/image_routes.go +++ b/api/server/router/image/image_routes.go @@ -204,7 +204,7 @@ func (s *imageRouter) deleteImages(ctx context.Context, w http.ResponseWriter, r force := httputils.BoolValue(r, "force") prune := !httputils.BoolValue(r, "noprune") - list, err := s.backend.ImageDelete(name, force, prune) + list, err := s.backend.ImageDelete(ctx, name, force, prune) if err != nil { return err } @@ -237,7 +237,7 @@ func (s *imageRouter) getImagesJSON(ctx context.Context, w http.ResponseWriter, imageFilters.Add("reference", filterParam) } - images, err := s.backend.Images(imageFilters, httputils.BoolValue(r, "all"), false) + images, err := s.backend.Images(ctx, imageFilters, httputils.BoolValue(r, "all"), false) if err != nil { return err } diff --git a/daemon/daemon.go b/daemon/daemon.go index 1e1b89c864928..a1dfb41475f03 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -25,6 +25,7 @@ import ( "github.com/containerd/containerd" "github.com/containerd/containerd/defaults" + "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/pkg/dialer" "github.com/containerd/containerd/remotes/docker" "github.com/docker/distribution/reference" @@ -1060,6 +1061,11 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S TrustKey: trustKey, }) + // TODO(containerd): create earlier, background, and wait at end + if err := d.imageService.LoadCache(namespaces.WithNamespace(ctx, ContainersNamespace)); err != nil { + return nil, errors.Wrap(err, "failed to load image cache from containerd") + } + go d.execCommandGC() d.containerd, err = libcontainerd.NewClient(ctx, d.containerdCli, filepath.Join(config.ExecRoot, "containerd"), config.ContainerdNamespace, d) diff --git a/daemon/disk_usage.go b/daemon/disk_usage.go index 5bec60d174142..0e1b6e1f4617d 100644 --- a/daemon/disk_usage.go +++ b/daemon/disk_usage.go @@ -26,7 +26,7 @@ func (daemon *Daemon) SystemDiskUsage(ctx context.Context) (*types.DiskUsage, er } // Get all top images with extra attributes - allImages, err := daemon.imageService.Images(filters.NewArgs(), false, true) + allImages, err := daemon.imageService.Images(ctx, filters.NewArgs(), false, true) if err != nil { return nil, fmt.Errorf("failed to retrieve image list: %v", err) } diff --git a/daemon/images/cache.go b/daemon/images/cache.go index 3b433106e8a8b..cd34eb6cf0bf1 100644 --- a/daemon/images/cache.go +++ b/daemon/images/cache.go @@ -1,18 +1,212 @@ package images // import "github.com/docker/docker/daemon/images" import ( + "context" + "sync" + + "github.com/containerd/containerd/images" + "github.com/containerd/containerd/log" + "github.com/containerd/containerd/namespaces" + "github.com/containerd/containerd/platforms" + "github.com/docker/distribution/digestset" + "github.com/docker/distribution/reference" "github.com/docker/docker/builder" - "github.com/docker/docker/image/cache" + buildcache "github.com/docker/docker/image/cache" + digest "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" ) -// MakeImageCache creates a stateful image cache. +type cachedImage struct { + id digest.Digest + parent digest.Digest + + // Mutable values + m sync.Mutex + references []reference.Named + children []digest.Digest +} + +type cache struct { + m sync.RWMutex + // idCache maps Docker identifiers + idCache map[digest.Digest]*cachedImage + // dCache maps target digests to images + tCache map[digest.Digest]*cachedImage + ids *digestset.Set +} + +func (c *cache) byID(id digest.Digest) *cachedImage { + c.m.RLock() + img, ok := c.idCache[id] + c.m.RUnlock() + if !ok { + return nil + } + return img +} + +func (c *cache) byTarget(target digest.Digest) *cachedImage { + c.m.RLock() + img, ok := c.tCache[target] + c.m.RUnlock() + if !ok { + return nil + } + return img +} + +// LoadCache loads the image cache by scanning containerd images +// and listening to containerd events +// This process can be backgrounded and will hold hold the cache +// lock until fully populated. +func (i *ImageService) LoadCache(ctx context.Context) error { + namespace, err := namespaces.NamespaceRequired(ctx) + if err != nil { + return err + } + + _, err = i.loadNSCache(ctx, namespace) + return err +} + +func (i *ImageService) loadNSCache(ctx context.Context, namespace string) (*cache, error) { + i.cacheL.Lock() + defer i.cacheL.Unlock() + + c := &cache{ + idCache: map[digest.Digest]*cachedImage{}, + tCache: map[digest.Digest]*cachedImage{}, + ids: digestset.NewSet(), + } + + is := i.client.ImageService() + + // TODO(containerd): This must use some streaming approach + imgs, err := is.List(ctx) + if err != nil { + return nil, err + } + + for _, img := range imgs { + ref, err := reference.Parse(img.Name) + if err != nil { + log.G(ctx).WithError(err).WithField("name", img.Name).Debug("skipping invalid image name") + continue + } + + named, hasName := ref.(reference.Named) + + ci := c.tCache[img.Target.Digest] + if ci == nil { + var id digest.Digest + if !hasName { + digested, ok := ref.(reference.Digested) + if ok { + id = digested.Digest() + } + } + if img.Target.MediaType == images.MediaTypeDockerSchema2Config || img.Target.MediaType == ocispec.MediaTypeImageConfig { + id = img.Target.Digest + + } + if id == "" { + idstr, ok := img.Labels[LabelImageID] + if !ok { + cs := i.client.ContentStore() + // TODO(containerd): resolve architecture from context + platform := platforms.Default() + desc, err := images.Config(ctx, cs, img.Target, platform) + if err != nil { + log.G(ctx).WithError(err).WithField("name", img.Name).Debug("TODO: no label") + continue + } + id = desc.Digest + } else { + id, err = digest.Parse(idstr) + if err != nil { + log.G(ctx).WithError(err).WithField("name", img.Name).Debug("skipping invalid image id label") + continue + } + } + } + + ci = c.idCache[id] + if ci == nil { + ci = &cachedImage{ + id: id, + } + if s := img.Labels[LabelImageParent]; s != "" { + pid, err := digest.Parse(s) + if err != nil { + log.G(ctx).WithError(err).WithField("name", img.Name).Debug("skipping invalid parent label") + } else { + ci.parent = pid + } + } + + c.idCache[id] = ci + } + c.tCache[img.Target.Digest] = ci + } + if hasName { + ci.addReference(named) + } + } + i.cache[namespace] = c + + return c, nil +} + +func (ci *cachedImage) addReference(named reference.Named) { + var ( + i int + s = named.String() + ) + + // Add references, add in sorted place + for ; i < len(ci.references); i++ { + if rs := ci.references[i].String(); s < rs { + ci.references = append(ci.references, nil) + copy(ci.references[i+1:], ci.references[i:]) + ci.references[i] = named + break + } else if rs == s { + break + } + } + if i == len(ci.references) { + ci.references = append(ci.references, named) + } +} + +func (i *ImageService) getCache(ctx context.Context) (c *cache, err error) { + namespace, ok := namespaces.Namespace(ctx) + if !ok { + // Default namespace + // TODO(containerd): define default in service + namespace = "" + } + i.cacheL.RLock() + c, ok = i.cache[namespace] + i.cacheL.RUnlock() + if !ok { + c, err = i.loadNSCache(ctx, namespace) + if err != nil { + return nil, err + } + } + + return c, nil +} + +// MakeImageCache creates a stateful image cache for build. func (i *ImageService) MakeImageCache(sourceRefs []string) builder.ImageCache { if len(sourceRefs) == 0 { - return cache.NewLocal(i.imageStore) + return buildcache.NewLocal(i.imageStore) } - cache := cache.New(i.imageStore) + cache := buildcache.New(i.imageStore) for _, ref := range sourceRefs { img, err := i.GetImage(ref) diff --git a/daemon/images/image.go b/daemon/images/image.go index 79c0e5897adbe..d86129b0f81ca 100644 --- a/daemon/images/image.go +++ b/daemon/images/image.go @@ -6,15 +6,22 @@ import ( "fmt" "github.com/containerd/containerd/content" + cerrdefs "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/platforms" "github.com/docker/distribution/reference" "github.com/docker/docker/errdefs" "github.com/docker/docker/image" + digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) +const ( + LabelImageID = "docker.io/image.id" + LabelImageParent = "docker.io/image.parent" +) + // ErrImageDoesNotExist is error returned when no image can be found for a reference. type ErrImageDoesNotExist struct { ref reference.Reference @@ -114,3 +121,68 @@ func (i *ImageService) getImage(ctx context.Context, target ocispec.Descriptor) Image: &img, }, nil } + +func (i *ImageService) getReferences(ctx context.Context, imageID digest.Digest) ([]reference.Named, error) { + c, err := i.getCache(ctx) + if err != nil { + return nil, err + } + img := c.byID(imageID) + if img == nil { + return nil, errdefs.NotFound(errors.New("no image with given id")) + } + + return img.references, nil +} + +func (i *ImageService) getCachedRef(ctx context.Context, ref string) (*cachedImage, error) { + parsed, err := reference.ParseAnyReference(ref) + if err != nil { + return nil, err + } + + c, err := i.getCache(ctx) + if err != nil { + return nil, err + } + + c.m.RLock() + defer c.m.RUnlock() + + namedRef, ok := parsed.(reference.Named) + if !ok { + digested, ok := parsed.(reference.Digested) + if !ok { + return nil, errdefs.InvalidParameter(errors.New("bad reference")) + } + + ci, ok := c.idCache[digested.Digest()] + if !ok { + return nil, errdefs.NotFound(errors.New("id not found")) + } + return ci, nil + } + + img, err := i.client.ImageService().Get(ctx, namedRef.String()) + if err != nil { + if !cerrdefs.IsNotFound(err) { + return nil, err + } + dgst, err := c.ids.Lookup(ref) + if err != nil { + return nil, errdefs.NotFound(errors.New("reference not found")) + } + ci, ok := c.idCache[dgst] + if !ok { + return nil, errdefs.NotFound(errors.New("id not found")) + } + return ci, nil + } + ci, ok := c.tCache[img.Target.Digest] + if !ok { + // TODO(containerd): Update cache and return + return nil, errdefs.NotFound(errors.New("id not found")) + } + + return ci, nil +} diff --git a/daemon/images/image_delete.go b/daemon/images/image_delete.go index fbd6c16b74a3a..3cc721e55a2c3 100644 --- a/daemon/images/image_delete.go +++ b/daemon/images/image_delete.go @@ -1,17 +1,19 @@ package images // import "github.com/docker/docker/daemon/images" import ( + "context" "fmt" "strings" "time" + "github.com/containerd/containerd/images" "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" "github.com/docker/docker/container" "github.com/docker/docker/errdefs" "github.com/docker/docker/image" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/system" + digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -60,25 +62,22 @@ const ( // meaning any delete conflicts will cause the image to not be deleted and the // conflict will not be reported. // -func (i *ImageService) ImageDelete(imageRef string, force, prune bool) ([]types.ImageDeleteResponseItem, error) { +func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force, prune bool) ([]types.ImageDeleteResponseItem, error) { start := time.Now() - records := []types.ImageDeleteResponseItem{} - img, err := i.GetImage(imageRef) + img, err := i.getCachedRef(ctx, imageRef) if err != nil { return nil, err } - if !system.IsOSSupported(img.OperatingSystem()) { - return nil, errors.Errorf("unable to delete image: %q", system.ErrNotSupportedOperatingSystem) - } - imgID := img.ID() - repoRefs := i.referenceStore.References(imgID.Digest()) + imgID := img.id + repoRefs := img.references using := func(c *container.Container) bool { - return c.ImageID == imgID + return digest.Digest(c.ImageID) == imgID } + var deletedRefs []reference.Named var removedRepositoryRef bool if !isImageIDPrefix(imgID.String(), imageRef) { // A repository reference was given and should be removed @@ -101,17 +100,8 @@ func (i *ImageService) ImageDelete(imageRef string, force, prune bool) ([]types. return nil, err } - parsedRef, err = i.removeImageRef(parsedRef) - if err != nil { - return nil, err - } - - untaggedRecord := types.ImageDeleteResponseItem{Untagged: reference.FamiliarString(parsedRef)} - + deletedRefs = append(deletedRefs, parsedRef) i.LogImageEvent(imgID.String(), imgID.String(), "untag") - records = append(records, untaggedRecord) - - repoRefs = i.referenceStore.References(imgID.Digest()) // If a tag reference was removed and the only remaining // references to the same repository are digest references, @@ -119,6 +109,9 @@ func (i *ImageService) ImageDelete(imageRef string, force, prune bool) ([]types. if _, isCanonical := parsedRef.(reference.Canonical); !isCanonical { foundRepoTagRef := false for _, repoRef := range repoRefs { + if parsedRef.String() == repoRef.String() { + continue + } if _, repoRefIsCanonical := repoRef.(reference.Canonical); !repoRefIsCanonical && parsedRef.Name() == repoRef.Name() { foundRepoTagRef = true break @@ -126,26 +119,26 @@ func (i *ImageService) ImageDelete(imageRef string, force, prune bool) ([]types. } if !foundRepoTagRef { // Remove canonical references from same repository - var remainingRefs []reference.Named for _, repoRef := range repoRefs { + if parsedRef.String() == repoRef.String() { + continue + } if _, repoRefIsCanonical := repoRef.(reference.Canonical); repoRefIsCanonical && parsedRef.Name() == repoRef.Name() { - if _, err := i.removeImageRef(repoRef); err != nil { - return records, err - } - - untaggedRecord := types.ImageDeleteResponseItem{Untagged: reference.FamiliarString(repoRef)} - records = append(records, untaggedRecord) - } else { - remainingRefs = append(remainingRefs, repoRef) - + // TODO(containerd): can repoRef be name only here? + deletedRefs = append(deletedRefs, repoRef) } } - repoRefs = remainingRefs } } // If it has remaining references then the untag finished the remove - if len(repoRefs) > 0 { + if len(repoRefs)-len(deletedRefs) > 0 { + // Remove all references in containerd + // Do not wait for containerd's garbage collection + records, err := i.removeImageRefs(ctx, deletedRefs, false) + if err != nil { + return nil, errors.Wrap(err, "failed to delete refs") + } return records, nil } @@ -164,20 +157,17 @@ func (i *ImageService) ImageDelete(imageRef string, force, prune bool) ([]types. } for _, repoRef := range repoRefs { - parsedRef, err := i.removeImageRef(repoRef) - if err != nil { - return nil, err - } - - untaggedRecord := types.ImageDeleteResponseItem{Untagged: reference.FamiliarString(parsedRef)} - + // TODO(containerd): can repoRef be name only here? + deletedRefs = append(deletedRefs, repoRef) i.LogImageEvent(imgID.String(), imgID.String(), "untag") - records = append(records, untaggedRecord) } } } - if err := i.imageDeleteHelper(imgID, &records, force, prune, removedRepositoryRef); err != nil { + // TODO(containerd): Lock, perform deletion, + // check if image exists then delete layers + records, err := i.imageDeleteHelper(ctx, img, repoRefs, force, prune, removedRepositoryRef) + if err != nil { return nil, err } @@ -225,43 +215,32 @@ func isImageIDPrefix(imageID, possiblePrefix string) bool { return false } -// removeImageRef attempts to parse and remove the given image reference from -// this daemon's store of repository tag/digest references. The given -// repositoryRef must not be an image ID but a repository name followed by an -// optional tag or digest reference. If tag or digest is omitted, the default -// tag is used. Returns the resolved image reference and an error. -func (i *ImageService) removeImageRef(ref reference.Named) (reference.Named, error) { - ref = reference.TagNameOnly(ref) - - // Ignore the boolean value returned, as far as we're concerned, this - // is an idempotent operation and it's okay if the reference didn't - // exist in the first place. - _, err := i.referenceStore.Delete(ref) - - return ref, err -} +// removeImageRefs removes a set of image references +// if the sync flag is set then garbage collection is +// is completed before returning +func (i *ImageService) removeImageRefs(ctx context.Context, refs []reference.Named, sync bool) ([]types.ImageDeleteResponseItem, error) { + records := []types.ImageDeleteResponseItem{} + // TODO(containerd): clear from cache, get cache from arguments -// removeAllReferencesToImageID attempts to remove every reference to the given -// imgID from this daemon's store of repository tag/digest references. Returns -// on the first encountered error. Removed references are logged to this -// daemon's event service. An "Untagged" types.ImageDeleteResponseItem is added to the -// given list of records. -func (i *ImageService) removeAllReferencesToImageID(imgID image.ID, records *[]types.ImageDeleteResponseItem) error { - imageRefs := i.referenceStore.References(imgID.Digest()) + is := i.client.ImageService() - for _, imageRef := range imageRefs { - parsedRef, err := i.removeImageRef(imageRef) - if err != nil { - return err + for i, ref := range refs { + opts := []images.DeleteOpt{} + if sync && i == len(refs)-1 { + opts = append(opts, images.SynchronousDelete()) + } + if err := is.Delete(ctx, ref.String(), opts...); err != nil && !errdefs.IsNotFound(err) { + return records, errors.Wrapf(err, "failed to delete ref: %s", ref.String()) } - untaggedRecord := types.ImageDeleteResponseItem{Untagged: reference.FamiliarString(parsedRef)} + // TODO(containerd): do this? + //i.LogImageEvent(imgID.String(), imgID.String(), "untag") - i.LogImageEvent(imgID.String(), imgID.String(), "untag") - *records = append(*records, untaggedRecord) + untaggedRecord := types.ImageDeleteResponseItem{Untagged: reference.FamiliarString(ref)} + records = append(records, untaggedRecord) } - return nil + return records, nil } // ImageDeleteConflict holds a soft or hard conflict and an associated error. @@ -297,49 +276,59 @@ func (idc *imageDeleteConflict) Conflict() {} // conflict is encountered, it will be returned immediately without deleting // the image. If quiet is true, any encountered conflicts will be ignored and // the function will return nil immediately without deleting the image. -func (i *ImageService) imageDeleteHelper(imgID image.ID, records *[]types.ImageDeleteResponseItem, force, prune, quiet bool) error { +func (i *ImageService) imageDeleteHelper(ctx context.Context, img *cachedImage, repoRefs []reference.Named, force, prune, quiet bool) ([]types.ImageDeleteResponseItem, error) { + // TODO(containerd): lock deletion, make reference removal and checks transactional in the cache? + // First, determine if this image has any conflicts. Ignore soft conflicts // if force is true. c := conflictHard if !force { c |= conflictSoft } - if conflict := i.checkImageDeleteConflict(imgID, c); conflict != nil { - if quiet && (!i.imageIsDangling(imgID) || conflict.used) { + if conflict := i.checkImageDeleteConflict(img.id, c); conflict != nil { + if quiet && (!i.imageIsDangling(img.id) || conflict.used) { // Ignore conflicts UNLESS the image is "dangling" or not being used in // which case we want the user to know. - return nil + return nil, nil } // There was a conflict and it's either a hard conflict OR we are not // forcing deletion on soft conflicts. - return conflict - } - - parent, err := i.imageStore.GetParent(imgID) - if err != nil { - // There may be no parent - parent = "" + return nil, conflict } // Delete all repository tag/digest references to this image. - if err := i.removeAllReferencesToImageID(imgID, records); err != nil { - return err + records, err := i.removeImageRefs(ctx, repoRefs, true) + if err != nil { + return records, err } - removedLayers, err := i.imageStore.Delete(imgID) + // NOTE(containerd): GC can do this in the future + // TODO(containerd): Move this function locally, to track and release layers + // Walk layers and remove reference + removedLayers, err := i.imageStore.Delete(image.ID(img.id)) if err != nil { - return err + return records, err } - i.LogImageEvent(imgID.String(), imgID.String(), "delete") - *records = append(*records, types.ImageDeleteResponseItem{Deleted: imgID.String()}) + i.LogImageEvent(img.id.String(), img.id.String(), "delete") + records = append(records, types.ImageDeleteResponseItem{Deleted: img.id.String()}) for _, removedLayer := range removedLayers { - *records = append(*records, types.ImageDeleteResponseItem{Deleted: removedLayer.ChainID.String()}) + records = append(records, types.ImageDeleteResponseItem{Deleted: removedLayer.ChainID.String()}) + } + + var parent *cachedImage + if img.parent != "" { + // TODO(containerd): pass cache in + c, err := i.getCache(ctx) + if err != nil { + return records, err + } + parent = c.byID(img.parent) } - if !prune || parent == "" { - return nil + if !prune || parent == nil { + return records, nil } // We need to prune the parent image. This means delete it if there are @@ -347,7 +336,8 @@ func (i *ImageService) imageDeleteHelper(imgID image.ID, records *[]types.ImageD // either running or stopped). // Do not force prunings, but do so quietly (stopping on any encountered // conflicts). - return i.imageDeleteHelper(parent, records, false, true, true) + parentRecords, err := i.imageDeleteHelper(ctx, parent, nil, false, true, true) + return append(records, parentRecords...), nil } // checkImageDeleteConflict determines whether there are any conflicts @@ -356,12 +346,13 @@ func (i *ImageService) imageDeleteHelper(imgID image.ID, records *[]types.ImageD // using the image. A soft conflict is any tags/digest referencing the given // image or any stopped container using the image. If ignoreSoftConflicts is // true, this function will not check for soft conflict conditions. -func (i *ImageService) checkImageDeleteConflict(imgID image.ID, mask conflictType) *imageDeleteConflict { +func (i *ImageService) checkImageDeleteConflict(imgID digest.Digest, mask conflictType) *imageDeleteConflict { // Check if the image has any descendant images. - if mask&conflictDependentChild != 0 && len(i.imageStore.Children(imgID)) > 0 { + // TODO(containerd): No use of image store + if mask&conflictDependentChild != 0 && len(i.imageStore.Children(image.ID(imgID))) > 0 { return &imageDeleteConflict{ hard: true, - imgID: imgID, + imgID: image.ID(imgID), message: "image has dependent child images", } } @@ -369,11 +360,11 @@ func (i *ImageService) checkImageDeleteConflict(imgID image.ID, mask conflictTyp if mask&conflictRunningContainer != 0 { // Check if any running container is using the image. running := func(c *container.Container) bool { - return c.ImageID == imgID && c.IsRunning() + return digest.Digest(c.ImageID) == imgID && c.IsRunning() } if container := i.containers.First(running); container != nil { return &imageDeleteConflict{ - imgID: imgID, + imgID: image.ID(imgID), hard: true, used: true, message: fmt.Sprintf("image is being used by running container %s", stringid.TruncateID(container.ID)), @@ -382,9 +373,10 @@ func (i *ImageService) checkImageDeleteConflict(imgID image.ID, mask conflictTyp } // Check if any repository tags/digest reference this image. - if mask&conflictActiveReference != 0 && len(i.referenceStore.References(imgID.Digest())) > 0 { + // TODO(containerd): No use of reference store + if mask&conflictActiveReference != 0 && len(i.referenceStore.References(imgID)) > 0 { return &imageDeleteConflict{ - imgID: imgID, + imgID: image.ID(imgID), message: "image is referenced in multiple repositories", } } @@ -392,11 +384,11 @@ func (i *ImageService) checkImageDeleteConflict(imgID image.ID, mask conflictTyp if mask&conflictStoppedContainer != 0 { // Check if any stopped containers reference this image. stopped := func(c *container.Container) bool { - return !c.IsRunning() && c.ImageID == imgID + return !c.IsRunning() && digest.Digest(c.ImageID) == imgID } if container := i.containers.First(stopped); container != nil { return &imageDeleteConflict{ - imgID: imgID, + imgID: image.ID(imgID), used: true, message: fmt.Sprintf("image is being used by stopped container %s", stringid.TruncateID(container.ID)), } @@ -409,6 +401,10 @@ func (i *ImageService) checkImageDeleteConflict(imgID image.ID, mask conflictTyp // imageIsDangling returns whether the given image is "dangling" which means // that there are no repository references to the given image and it has no // child images. -func (i *ImageService) imageIsDangling(imgID image.ID) bool { - return !(len(i.referenceStore.References(imgID.Digest())) > 0 || len(i.imageStore.Children(imgID)) > 0) +func (i *ImageService) imageIsDangling(imgID digest.Digest) bool { + // TODO(containerd): No use of reference store + // TODO(containerd): No use of image store + // To find children, Docker keeps a cache of images along with parents, it + // can also keep a backpointer to parents in memory + return !(len(i.referenceStore.References(imgID)) > 0 || len(i.imageStore.Children(image.ID(imgID))) > 0) } diff --git a/daemon/images/image_prune.go b/daemon/images/image_prune.go index 0f835dff973bd..1bc99178358c7 100644 --- a/daemon/images/image_prune.go +++ b/daemon/images/image_prune.go @@ -122,7 +122,7 @@ deleteImagesLoop: if shouldDelete { for _, ref := range refs { - imgDel, err := i.ImageDelete(ref.String(), false, true) + imgDel, err := i.ImageDelete(ctx, ref.String(), false, true) if imageDeleteFailed(ref.String(), err) { continue } @@ -131,7 +131,7 @@ deleteImagesLoop: } } else { hex := id.Digest().Hex() - imgDel, err := i.ImageDelete(hex, false, true) + imgDel, err := i.ImageDelete(ctx, hex, false, true) if imageDeleteFailed(hex, err) { continue } diff --git a/daemon/images/image_pull.go b/daemon/images/image_pull.go index a72d822e40647..9587a46a2302d 100644 --- a/daemon/images/image_pull.go +++ b/daemon/images/image_pull.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/errdefs" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" ) // PullImage initiates a pull operation. image is the repository name to pull, and @@ -48,6 +49,11 @@ func (i *ImageService) PullImage(ctx context.Context, image, tag string, platfor } func (i *ImageService) pullImageWithReference(ctx context.Context, ref reference.Named, platform *specs.Platform, metaHeaders map[string][]string, authConfig *types.AuthConfig, outStream io.Writer) error { + c, err := i.getCache(ctx) + if err != nil { + return err + } + // Include a buffer so that slow client connections don't affect // transfer performance. //progressChan := make(chan progress.Progress, 100) @@ -66,12 +72,34 @@ func (i *ImageService) pullImageWithReference(ctx context.Context, ref reference // TODO: progress tracking // TODO: unpack tracking, use download manager for now? - // TODO: keep image - _, err := i.client.Pull(ctx, ref.String(), opts...) + img, err := i.client.Pull(ctx, ref.String(), opts...) + + config, err := img.Config(ctx) + if err != nil { + return errors.Wrap(err, "failed to resolve configuration") + } // TODO: Unpack into layer store // TODO: only unpack image types (does containerd already do this?) + // TODO: Update image with ID label + // TODO(containerd): Create manifest reference and add image + + c.m.Lock() + ci, ok := c.idCache[config.Digest] + if ok { + ci.addReference(ref) + // TODO: Add manifest digest ref + } else { + ci = &cachedImage{ + id: config.Digest, + references: []reference.Named{ref}, + } + c.idCache[config.Digest] = ci + } + c.tCache[img.Target().Digest] = ci + c.m.Unlock() + //go func() { // progressutils.WriteDistributionProgress(cancelFunc, outStream, progressChan) // close(writesDone) diff --git a/daemon/images/images.go b/daemon/images/images.go index 5f70a4b9fb152..3aa1db75aa2ff 100644 --- a/daemon/images/images.go +++ b/daemon/images/images.go @@ -9,7 +9,7 @@ import ( "time" "github.com/containerd/containerd/images" - "github.com/containerd/containerd/platforms" + "github.com/containerd/containerd/log" digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -17,7 +17,6 @@ import ( "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" - "github.com/docker/docker/errdefs" "github.com/docker/docker/image" "github.com/docker/docker/layer" "github.com/docker/docker/pkg/system" @@ -49,8 +48,12 @@ func (i *ImageService) Map() map[image.ID]*image.Image { // filter is a shell glob string applied to repository names. The argument // named all controls whether all images in the graph are filtered, or just // the heads. -func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error) { - ctx := context.TODO() +func (i *ImageService) Images(ctx context.Context, imageFilters filters.Args, all bool, withExtraAttrs bool) ([]*types.ImageSummary, error) { + c, err := i.getCache(ctx) + if err != nil { + return nil, err + } + if err := imageFilters.Validate(acceptedImageFilterTags); err != nil { return nil, err } @@ -65,7 +68,7 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr } var beforeFilter, sinceFilter *image.Image - err := imageFilters.WalkValues("before", func(value string) error { + err = imageFilters.WalkValues("before", func(value string) error { var err error beforeFilter, err = i.GetImage(value) return err @@ -121,9 +124,9 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr return nil, err } - cs := i.client.ContentStore() m := map[digest.Digest][]images.Image{} - cache := map[digest.Digest]digest.Digest{} + + c.m.RLock() for _, img := range allImages { if beforeFilter != nil && beforeFilter.Image.Created != nil { created := img.Labels["docker.io/created"] @@ -146,30 +149,14 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr } - config, ok := cache[img.Target.Digest] - + ci, ok := c.tCache[img.Target.Digest] if !ok { - // TODO: Resolve to a config - c, err := images.Config(ctx, cs, img.Target, platforms.Default()) - if err != nil { - if errdefs.IsNotFound(err) { - // TODO: Log this unresolved config - continue - } - return nil, err - } - config = c.Digest + // TODO(containerd): Lookup config and update cache + log.G(ctx).WithField("name", img.Name).Debugf("skipping non-cached image") + continue } - m[config] = append(m[config], img) - - // TODO: WTF? - // Skip any images with an unsupported operating system to avoid a potential - // panic when indexing through the layerstore. Don't error as we want to list - // the other images. This should never happen, but here as a safety precaution. - //if !system.IsOSSupported(img.OperatingSystem()) { - // continue - //} + m[ci.id] = append(m[ci.id], img) //var size int64 // TODO: this seems pretty dumb to do @@ -240,6 +227,7 @@ func (i *ImageService) Images(imageFilters filters.Args, all bool, withExtraAttr //images = append(images, newImage) } + c.m.RUnlock() imageSums := []*types.ImageSummary{} //var layerRefs map[layer.ChainID]int diff --git a/daemon/images/service.go b/daemon/images/service.go index dd8ce7b35e212..7bdb39c06769e 100644 --- a/daemon/images/service.go +++ b/daemon/images/service.go @@ -4,6 +4,7 @@ import ( "context" "os" "runtime" + "sync" "github.com/containerd/containerd" "github.com/docker/docker/container" @@ -32,6 +33,7 @@ type containerStore interface { // ImageServiceConfig is the configuration used to create a new ImageService type ImageServiceConfig struct { + DefaultNamespace string Client *containerd.Client ContainerStore containerStore DistributionMetadataStore metadata.Store @@ -52,10 +54,12 @@ func NewImageService(config ImageServiceConfig) *ImageService { logrus.Debugf("Max Concurrent Uploads: %d", config.MaxConcurrentUploads) logrus.Debugf("Max Download Attempts: %d", config.MaxDownloadAttempts) return &ImageService{ + namespace: config.DefaultNamespace, client: config.Client, containers: config.ContainerStore, distributionMetadataStore: config.DistributionMetadataStore, downloadManager: xfer.NewLayerDownloadManager(config.LayerStores, config.MaxConcurrentDownloads, xfer.WithMaxDownloadAttempts(config.MaxDownloadAttempts)), + cache: map[string]*cache{}, eventsService: config.EventsService, imageStore: config.ImageStore, layerStores: config.LayerStores, @@ -68,12 +72,17 @@ func NewImageService(config ImageServiceConfig) *ImageService { // ImageService provides a backend for image management type ImageService struct { + namespace string client *containerd.Client containers containerStore eventsService *daemonevents.Events layerStores map[string]layer.Store // By operating system pruneRunning int32 + // namespaced cache + cache map[string]*cache + cacheL sync.RWMutex + // To be replaced by containerd client registryService registry.Service referenceStore dockerreference.Store