Merge pull request #2400 from Luap99/list-manifest

libimage: fix manifest race during listing

Author: openshift-merge-bot[bot]

Diff for: 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
 	}

Diff for: 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
 		}

Diff for: 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
 	}

Diff for: 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
 		}

Diff for: libimage/manifests/manifests.go

@@ -149,7 +149,7 @@ func LoadFromImage(store storage.Store, image string) (string, List, error) {
 	}
 	manifestList, err := manifests.FromBlob(manifestBytes)
 	if err != nil {
-		return "", nil, err
+		return "", nil, fmt.Errorf("decoding manifest blob for image %q: %w", image, err)
 	}
 	list := &list{
 		List:      manifestList,

Diff for: 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
 		}