libimage: fix manifest race during listing

I saw a flake in parallel podman testing, podman images can fail if the
manifest was removed at the right time. In general listing should never
be able to fail when another image or manifest is removed in parallel.

Change the logic to convert to manifest and only collect the digests in
the success case and ignore all other errors to make the listing more
robust.

I observed the following error from podman images:
Error: locating image "xxx" for loading instance list: locating image with ID "xxx": image not known

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Author: Luap99

libimage/history.go

@@ -25,7 +25,7 @@ func (i *Image) History(ctx context.Context) ([]ImageHistory, error) {
 		return nil, err
 	}
 
-	layerTree, err := i.runtime.newFreshLayerTree(ctx)
+	layerTree, err := i.runtime.newFreshLayerTree()
 	if err != nil {
 		return nil, err
 	}

libimage/image.go

@@ -197,7 +197,7 @@ func (i *Image) IsDangling(ctx context.Context) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	tree, err := i.runtime.newLayerTreeFromData(ctx, images, layers, true)
+	tree, err := i.runtime.newLayerTreeFromData(images, layers, true)
 	if err != nil {
 		return false, err
 	}
@@ -267,7 +267,7 @@ func (i *Image) TopLayer() string {
 
 // Parent returns the parent image or nil if there is none
 func (i *Image) Parent(ctx context.Context) (*Image, error) {
-	tree, err := i.runtime.newFreshLayerTree(ctx)
+	tree, err := i.runtime.newFreshLayerTree()
 	if err != nil {
 		return nil, err
 	}
@@ -301,7 +301,7 @@ func (i *Image) Children(ctx context.Context) ([]*Image, error) {
 // created for this invocation only.
 func (i *Image) getChildren(ctx context.Context, all bool, tree *layerTree) ([]*Image, error) {
 	if tree == nil {
-		t, err := i.runtime.newFreshLayerTree(ctx)
+		t, err := i.runtime.newFreshLayerTree()
 		if err != nil {
 			return nil, err
 		}

libimage/image_tree.go

@@ -38,7 +38,7 @@ func (i *Image) Tree(ctx context.Context, traverseChildren bool) (string, error)
 		fmt.Fprintf(sb, "No Image Layers")
 	}
 
-	layerTree, err := i.runtime.newFreshLayerTree(ctx)
+	layerTree, err := i.runtime.newFreshLayerTree()
 	if err != nil {
 		return "", err
 	}

libimage/layer_tree.go

@@ -95,17 +95,17 @@ func (l *layerNode) repoTags() ([]string, error) {
 }
 
 // newFreshLayerTree extracts a layerTree from consistent layers and images in the local storage.
-func (r *Runtime) newFreshLayerTree(ctx context.Context) (*layerTree, error) {
+func (r *Runtime) newFreshLayerTree() (*layerTree, error) {
 	images, layers, err := r.getImagesAndLayers()
 	if err != nil {
 		return nil, err
 	}
-	return r.newLayerTreeFromData(ctx, images, layers, false)
+	return r.newLayerTreeFromData(images, layers, false)
 }
 
 // newLayerTreeFromData extracts a layerTree from the given the layers and images.
 // The caller is responsible for (layers, images) being consistent.
-func (r *Runtime) newLayerTreeFromData(ctx context.Context, images []*Image, layers []storage.Layer, generateManifestDigestList bool) (*layerTree, error) {
+func (r *Runtime) newLayerTreeFromData(images []*Image, layers []storage.Layer, generateManifestDigestList bool) (*layerTree, error) {
 	tree := layerTree{
 		nodes:               make(map[string]*layerNode),
 		ociCache:            make(map[string]*ociv1.Image),
@@ -136,14 +136,23 @@ func (r *Runtime) newLayerTreeFromData(ctx context.Context, images []*Image, lay
 			if !generateManifestDigestList {
 				continue
 			}
-			if manifestList, _ := img.IsManifestList(ctx); manifestList {
-				mlist, err := img.ToManifestList()
-				if err != nil {
-					return nil, err
-				}
-				for _, digest := range mlist.list.Instances() {
-					tree.manifestListDigests[digest] = struct{}{}
+			// ignore errors, common errors are
+			//  - image is not manifest
+			//  - image has been removed from the store in the meantime
+			// In all cases we should ensure image listing still works and not error out.
+			mlist, err := img.ToManifestList()
+			if err != nil {
+				// If it is not a manifest it likely is a regular image so just ignore it.
+				// If the image is unknown that likely means there was a race where the image/manifest
+				// was removed after out MultiList() call so we ignore that as well.
+				if errors.Is(err, ErrNotAManifestList) || errors.Is(err, storageTypes.ErrImageUnknown) {
+					continue
 				}
+				return nil, err
+			}
+
+			for _, digest := range mlist.list.Instances() {
+				tree.manifestListDigests[digest] = struct{}{}
 			}
 			continue
 		}

libimage/runtime.go

@@ -634,7 +634,7 @@ func (r *Runtime) ListImages(ctx context.Context, options *ListImagesOptions) ([
 
 	var tree *layerTree
 	if needsLayerTree {
-		tree, err = r.newLayerTreeFromData(ctx, images, snapshot.Layers, true)
+		tree, err = r.newLayerTreeFromData(images, snapshot.Layers, true)
 		if err != nil {
 			return nil, err
 		}