refine dangling checks

By proxy by vendoring containers/common. Previously, a "dangling" image
was an untagged image; just a described in the Docker docs. The
definition of dangling has now been refined to an untagged image without
children to be compatible with Docker.

Further update a redundant image-prune test.

Fixes: #10998
Fixes: #10832
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
This commit is contained in:
Valentin Rothberg
2021-07-19 11:41:04 +02:00
parent ec5c7c1f6a
commit 1b6423e9f1
24 changed files with 300 additions and 255 deletions

View File

@ -88,7 +88,7 @@ func (r *Runtime) compileImageFilters(ctx context.Context, filters []string) ([]
if err != nil {
return nil, errors.Wrapf(err, "non-boolean value %q for dangling filter", value)
}
filterFuncs = append(filterFuncs, filterDangling(dangling))
filterFuncs = append(filterFuncs, filterDangling(ctx, dangling))
case "id":
filterFuncs = append(filterFuncs, filterID(value))
@ -201,9 +201,13 @@ func filterContainers(value bool) filterFunc {
}
// filterDangling creates a dangling filter for matching the specified value.
func filterDangling(value bool) filterFunc {
func filterDangling(ctx context.Context, value bool) filterFunc {
return func(img *Image) (bool, error) {
return img.IsDangling() == value, nil
isDangling, err := img.IsDangling(ctx)
if err != nil {
return false, err
}
return isDangling == value, nil
}
}

View File

@ -121,24 +121,29 @@ func (i *Image) IsReadOnly() bool {
return i.storageImage.ReadOnly
}
// IsDangling returns true if the image is dangling. An image is considered
// dangling if no names are associated with it in the containers storage.
func (i *Image) IsDangling() bool {
return len(i.Names()) == 0
}
// IsIntermediate returns true if the image is an intermediate image, that is
// a dangling image without children.
func (i *Image) IsIntermediate(ctx context.Context) (bool, error) {
// If the image has tags, it's not an intermediate one.
if !i.IsDangling() {
// IsDangling returns true if the image is dangling, that is an untagged image
// without children.
func (i *Image) IsDangling(ctx context.Context) (bool, error) {
if len(i.Names()) > 0 {
return false, nil
}
children, err := i.getChildren(ctx, false)
if err != nil {
return false, err
}
return len(children) == 0, nil
}
// IsIntermediate returns true if the image is an intermediate image, that is
// an untagged image with children.
func (i *Image) IsIntermediate(ctx context.Context) (bool, error) {
if len(i.Names()) > 0 {
return false, nil
}
children, err := i.getChildren(ctx, false)
if err != nil {
return false, err
}
// No tags, no children -> intermediate!
return len(children) != 0, nil
}
@ -271,7 +276,7 @@ type RemoveImageReport struct {
// remove removes the image along with all dangling parent images that no other
// image depends on. The image must not be set read-only and not be used by
// containers.
// containers. Returns IDs of removed/untagged images in order.
//
// If the image is used by containers return storage.ErrImageUsedByContainer.
// Use force to remove these containers.
@ -282,7 +287,12 @@ type RemoveImageReport struct {
//
// This function is internal. Users of libimage should always use
// `(*Runtime).RemoveImages()`.
func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport, referencedBy string, options *RemoveImagesOptions) error {
func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport, referencedBy string, options *RemoveImagesOptions) ([]string, error) {
processedIDs := []string{}
return i.removeRecursive(ctx, rmMap, processedIDs, referencedBy, options)
}
func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveImageReport, processedIDs []string, referencedBy string, options *RemoveImagesOptions) ([]string, error) {
// If referencedBy is empty, the image is considered to be removed via
// `image remove --all` which alters the logic below.
@ -294,7 +304,7 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
logrus.Debugf("Removing image %s", i.ID())
if i.IsReadOnly() {
return errors.Errorf("cannot remove read-only image %q", i.ID())
return processedIDs, errors.Errorf("cannot remove read-only image %q", i.ID())
}
if i.runtime.eventChannel != nil {
@ -306,7 +316,7 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
if exists {
// If the image has already been removed, we're done.
if report.Removed {
return nil
return processedIDs, nil
}
} else {
report = &RemoveImageReport{ID: i.ID()}
@ -333,7 +343,7 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
if options.WithSize {
size, err := i.Size()
if handleError(err) != nil {
return err
return processedIDs, err
}
report.Size = size
}
@ -354,18 +364,18 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
byDigest := strings.HasPrefix(referencedBy, "sha256:")
if !options.Force {
if byID && numNames > 1 {
return errors.Errorf("unable to delete image %q by ID with more than one tag (%s): please force removal", i.ID(), i.Names())
return processedIDs, errors.Errorf("unable to delete image %q by ID with more than one tag (%s): please force removal", i.ID(), i.Names())
} else if byDigest && numNames > 1 {
// FIXME - Docker will remove the digest but containers storage
// does not support that yet, so our hands are tied.
return errors.Errorf("unable to delete image %q by digest with more than one tag (%s): please force removal", i.ID(), i.Names())
return processedIDs, errors.Errorf("unable to delete image %q by digest with more than one tag (%s): please force removal", i.ID(), i.Names())
}
}
// Only try to untag if we know it's not an ID or digest.
if !byID && !byDigest {
if err := i.Untag(referencedBy); handleError(err) != nil {
return err
return processedIDs, err
}
report.Untagged = append(report.Untagged, referencedBy)
@ -374,14 +384,15 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
}
}
processedIDs = append(processedIDs, i.ID())
if skipRemove {
return nil
return processedIDs, nil
}
// Perform the actual removal. First, remove containers if needed.
if options.Force {
if err := i.removeContainers(options.RemoveContainerFunc); err != nil {
return err
return processedIDs, err
}
}
@ -407,7 +418,7 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
}
if _, err := i.runtime.store.DeleteImage(i.ID(), true); handleError(err) != nil {
return err
return processedIDs, err
}
report.Untagged = append(report.Untagged, i.Names()...)
@ -417,27 +428,24 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
// Check if can remove the parent image.
if parent == nil {
return nil
return processedIDs, nil
}
if !parent.IsDangling() {
return nil
}
// If the image has siblings, we don't remove the parent.
hasSiblings, err := parent.HasChildren(ctx)
// Only remove the parent if it's dangling, that is being untagged and
// without children.
danglingParent, err := parent.IsDangling(ctx)
if err != nil {
// See Podman commit fd9dd7065d44: we need to
// be tolerant toward corrupted images.
logrus.Warnf("error determining if an image is a parent: %v, ignoring the error", err)
hasSiblings = false
danglingParent = false
}
if hasSiblings {
return nil
if !danglingParent {
return processedIDs, nil
}
// Recurse into removing the parent.
return parent.remove(ctx, rmMap, "", options)
return parent.removeRecursive(ctx, rmMap, processedIDs, "", options)
}
// Tag the image with the specified name and store it in the local containers

View File

@ -15,6 +15,9 @@ type layerTree struct {
// ociCache is a cache for Image.ID -> OCI Image. Translations are done
// on-demand.
ociCache map[string]*ociv1.Image
// emptyImages do not have any top-layer so we cannot create a
// *layerNode for them.
emptyImages []*Image
}
// node returns a layerNode for the specified layerID.
@ -105,6 +108,7 @@ func (r *Runtime) layerTree() (*layerTree, error) {
img := images[i] // do not leak loop variable outside the scope
topLayer := img.TopLayer()
if topLayer == "" {
tree.emptyImages = append(tree.emptyImages, img)
continue
}
node, exists := tree.nodes[topLayer]
@ -126,22 +130,13 @@ func (r *Runtime) layerTree() (*layerTree, error) {
// either the same top layer as parent or parent being the true parent layer.
// Furthermore, the history of the parent and child images must match with the
// parent having one history item less. If all is true, all images are
// returned. Otherwise, the first image is returned.
// returned. Otherwise, the first image is returned. Note that manifest lists
// do not have children.
func (t *layerTree) children(ctx context.Context, parent *Image, all bool) ([]*Image, error) {
if parent.TopLayer() == "" {
return nil, nil
}
var children []*Image
parentNode, exists := t.nodes[parent.TopLayer()]
if !exists {
// Note: erroring out in this case has turned out having been a
// mistake. Users may not be able to recover, so we're now
// throwing a warning to guide them to resolve the issue and
// turn the errors non-fatal.
logrus.Warnf("Layer %s not found in layer tree. The storage may be corrupted, consider running `podman system reset`.", parent.TopLayer())
return children, nil
if isManifestList, _ := parent.IsManifestList(ctx); isManifestList {
return nil, nil
}
}
parentID := parent.ID()
@ -163,6 +158,38 @@ func (t *layerTree) children(ctx context.Context, parent *Image, all bool) ([]*I
return areParentAndChild(parentOCI, childOCI), nil
}
var children []*Image
// Empty images are special in that they do not have any physical layer
// but yet can have a parent-child relation. Hence, compare the
// "parent" image to all other known empty images.
if parent.TopLayer() == "" {
for i := range t.emptyImages {
empty := t.emptyImages[i]
isParent, err := checkParent(empty)
if err != nil {
return nil, err
}
if isParent {
children = append(children, empty)
if !all {
break
}
}
}
return children, nil
}
parentNode, exists := t.nodes[parent.TopLayer()]
if !exists {
// Note: erroring out in this case has turned out having been a
// mistake. Users may not be able to recover, so we're now
// throwing a warning to guide them to resolve the issue and
// turn the errors non-fatal.
logrus.Warnf("Layer %s not found in layer tree. The storage may be corrupted, consider running `podman system reset`.", parent.TopLayer())
return children, nil
}
// addChildrenFrom adds child images of parent to children. Returns
// true if any image is a child of parent.
addChildrenFromNode := func(node *layerNode) (bool, error) {
@ -204,8 +231,37 @@ func (t *layerTree) children(ctx context.Context, parent *Image, all bool) ([]*I
}
// parent returns the parent image or nil if no parent image could be found.
// Note that manifest lists do not have parents.
func (t *layerTree) parent(ctx context.Context, child *Image) (*Image, error) {
if child.TopLayer() == "" {
if isManifestList, _ := child.IsManifestList(ctx); isManifestList {
return nil, nil
}
}
childID := child.ID()
childOCI, err := t.toOCI(ctx, child)
if err != nil {
return nil, err
}
// Empty images are special in that they do not have any physical layer
// but yet can have a parent-child relation. Hence, compare the
// "child" image to all other known empty images.
if child.TopLayer() == "" {
for _, empty := range t.emptyImages {
if childID == empty.ID() {
continue
}
emptyOCI, err := t.toOCI(ctx, empty)
if err != nil {
return nil, err
}
// History check.
if areParentAndChild(emptyOCI, childOCI) {
return empty, nil
}
}
return nil, nil
}
@ -219,14 +275,8 @@ func (t *layerTree) parent(ctx context.Context, child *Image) (*Image, error) {
return nil, nil
}
childOCI, err := t.toOCI(ctx, child)
if err != nil {
return nil, err
}
// Check images from the parent node (i.e., parent layer) and images
// with the same layer (i.e., same top layer).
childID := child.ID()
images := node.images
if node.parent != nil {
images = append(images, node.parent.images...)

View File

@ -10,6 +10,7 @@ import (
"github.com/containers/common/pkg/config"
registryTransport "github.com/containers/image/v5/docker"
dockerArchiveTransport "github.com/containers/image/v5/docker/archive"
dockerDaemonTransport "github.com/containers/image/v5/docker/daemon"
"github.com/containers/image/v5/docker/reference"
ociArchiveTransport "github.com/containers/image/v5/oci/archive"
ociTransport "github.com/containers/image/v5/oci/layout"
@ -55,6 +56,13 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
var possiblyUnqualifiedName string // used for short-name resolution
ref, err := alltransports.ParseImageName(name)
if err != nil {
// Check whether `name` points to a transport. If so, we
// return the error. Otherwise we assume that `name` refers to
// an image on a registry (e.g., "fedora").
if alltransports.TransportFromImageName(name) != nil {
return nil, err
}
// If the image clearly refers to a local one, we can look it up directly.
// In fact, we need to since they are not parseable.
if strings.HasPrefix(name, "sha256:") || (len(name) == 64 && !strings.ContainsAny(name, "/.:@")) {
@ -169,6 +177,15 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference,
var storageName, imageName string
switch ref.Transport().Name() {
case dockerDaemonTransport.Transport.Name():
// Normalize to docker.io if needed (see containers/podman/issues/10998).
named, err := reference.ParseNormalizedNamed(ref.StringWithinTransport())
if err != nil {
return nil, err
}
imageName = named.String()
storageName = imageName
case ociTransport.Transport.Name():
split := strings.SplitN(ref.StringWithinTransport(), ":", 2)
storageName = toLocalImageName(split[0])

View File

@ -3,7 +3,6 @@ package libimage
import (
"context"
"os"
"path/filepath"
"strings"
"github.com/containers/image/v5/docker/reference"
@ -94,10 +93,6 @@ func RuntimeFromStore(store storage.Store, options *RuntimeOptions) (*Runtime, e
setRegistriesConfPath(&systemContext)
if systemContext.BlobInfoCacheDir == "" {
systemContext.BlobInfoCacheDir = filepath.Join(store.GraphRoot(), "cache")
}
return &Runtime{
store: store,
systemContext: systemContext,
@ -592,11 +587,10 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem
rmErrors = append(rmErrors, err)
}
orderedIDs := []string{} // determinism and relative order
deleteMap := make(map[string]*deleteMe) // ID -> deleteMe
toDelete := []string{}
// Look up images in the local containers storage and fill out
// orderedIDs and the deleteMap.
// toDelete and the deleteMap.
switch {
case len(names) > 0:
// Look up the images one-by-one. That allows for removing
@ -610,15 +604,12 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem
}
dm, exists := deleteMap[img.ID()]
if !exists {
orderedIDs = append(orderedIDs, img.ID())
toDelete = append(toDelete, img.ID())
dm = &deleteMe{image: img}
deleteMap[img.ID()] = dm
}
dm.referencedBy = append(dm.referencedBy, resolvedName)
}
if len(orderedIDs) == 0 {
return nil, rmErrors
}
default:
filteredImages, err := r.ListImages(ctx, nil, &ListImagesOptions{Filters: options.Filters})
@ -627,14 +618,21 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem
return nil, rmErrors
}
for _, img := range filteredImages {
orderedIDs = append(orderedIDs, img.ID())
toDelete = append(toDelete, img.ID())
deleteMap[img.ID()] = &deleteMe{image: img}
}
}
// Return early if there's no image to delete.
if len(deleteMap) == 0 {
return nil, rmErrors
}
// Now remove the images in the given order.
rmMap := make(map[string]*RemoveImageReport)
for _, id := range orderedIDs {
orderedIDs := []string{}
visitedIDs := make(map[string]bool)
for _, id := range toDelete {
del, exists := deleteMap[id]
if !exists {
appendError(errors.Errorf("internal error: ID %s not in found in image-deletion map", id))
@ -644,9 +642,17 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem
del.referencedBy = []string{""}
}
for _, ref := range del.referencedBy {
if err := del.image.remove(ctx, rmMap, ref, options); err != nil {
processedIDs, err := del.image.remove(ctx, rmMap, ref, options)
if err != nil {
appendError(err)
continue
}
// NOTE: make sure to add given ID only once to orderedIDs.
for _, id := range processedIDs {
if visited := visitedIDs[id]; visited {
continue
}
orderedIDs = append(orderedIDs, id)
visitedIDs[id] = true
}
}
}