make image listing more resilient

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/containernetworking/cni v1.1.2
 	github.com/containernetworking/plugins v1.3.0
 	github.com/containers/buildah v1.30.1-0.20230504052500-e925b5852e07
-	github.com/containers/common v0.53.1-0.20230621174116-586a3be4e1fc
+	github.com/containers/common v0.53.1-0.20230626115555-370c89881624
 	github.com/containers/conmon v2.0.20+incompatible
 	github.com/containers/image/v5 v5.25.1-0.20230613183705-07ced6137083
 	github.com/containers/libhvee v0.0.5

go.sum

@@ -239,8 +239,8 @@ github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q
 github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0=
 github.com/containers/buildah v1.30.1-0.20230504052500-e925b5852e07 h1:Bs2sNFh/fSYr4J6JJLFqzyn3dp6HhlA6ewFwRYUpeIE=
 github.com/containers/buildah v1.30.1-0.20230504052500-e925b5852e07/go.mod h1:6A/BK0YJLXL8+AqlbceKJrhUT+NtEgsvAc51F7TAllc=
-github.com/containers/common v0.53.1-0.20230621174116-586a3be4e1fc h1:6yxDNgJGrddAWKeeAH7m0GUzCFRuvc2BqXund52Ui7k=
-github.com/containers/common v0.53.1-0.20230621174116-586a3be4e1fc/go.mod h1:qE1MzGl69IoK7ZNCCH51+aLVjyQtnH0LiZe0wG32Jy0=
+github.com/containers/common v0.53.1-0.20230626115555-370c89881624 h1:YBgjfoo0G3tR8vm225ghJnqOZOVv3tH1L1GbyRM9320=
+github.com/containers/common v0.53.1-0.20230626115555-370c89881624/go.mod h1:qE1MzGl69IoK7ZNCCH51+aLVjyQtnH0LiZe0wG32Jy0=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/image/v5 v5.25.1-0.20230613183705-07ced6137083 h1:6Pbnll97ls6G0U3DSxaTqp7Sd8Fykc4gd7BUJm7Bpn8=

libpod/container_internal.go

@@ -2094,7 +2094,7 @@ func (c *Container) postDeleteHooks(ctx context.Context) error {
 				hook := hook
 				logrus.Debugf("container %s: invoke poststop hook %d, path %s", c.ID(), i, hook.Path)
 				var stderr, stdout bytes.Buffer
-				hookErr, err := exec.Run(ctx, &hook, state, &stdout, &stderr, exec.DefaultPostKillTimeout)
+				hookErr, err := exec.Run(ctx, &hook, state, &stdout, &stderr, exec.DefaultPostKillTimeout) //nolint:staticcheck
 				if err != nil {
 					logrus.Warnf("Container %s: poststop hook %d: %v", c.ID(), i, err)
 					if hookErr != err {
@@ -2223,7 +2223,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[s
 		}
 	}
 
-	hookErr, err := exec.RuntimeConfigFilter(ctx, allHooks["precreate"], config, exec.DefaultPostKillTimeout)
+	hookErr, err := exec.RuntimeConfigFilter(ctx, allHooks["precreate"], config, exec.DefaultPostKillTimeout) //nolint:staticcheck
 	if err != nil {
 		logrus.Warnf("Container %s: precreate hook: %v", c.ID(), err)
 		if hookErr != nil && hookErr != err {

pkg/domain/infra/abi/images_list.go

@@ -28,54 +28,63 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions)
 
 	summaries := []*entities.ImageSummary{}
 	for _, img := range images {
-		repoDigests, err := img.RepoDigests()
-		if err != nil {
-			return nil, fmt.Errorf("getting repoDigests from image %q: %w", img.ID(), err)
-		}
+		summary, err := func() (*entities.ImageSummary, error) {
+			repoDigests, err := img.RepoDigests()
+			if err != nil {
+				return nil, fmt.Errorf("getting repoDigests from image %q: %w", img.ID(), err)
+			}
 
-		if img.ListData.IsDangling == nil { // Sanity check
-			return nil, fmt.Errorf("%w: ListData.IsDangling is nil but should not", define.ErrInternal)
-		}
-		isDangling := *img.ListData.IsDangling
-		parentID := ""
-		if img.ListData.Parent != nil {
-			parentID = img.ListData.Parent.ID()
-		}
+			if img.ListData.IsDangling == nil { // Sanity check
+				return nil, fmt.Errorf("%w: ListData.IsDangling is nil but should not be", define.ErrInternal)
+			}
+			isDangling := *img.ListData.IsDangling
+			parentID := ""
+			if img.ListData.Parent != nil {
+				parentID = img.ListData.Parent.ID()
+			}
 
-		e := entities.ImageSummary{
-			ID:          img.ID(),
-			Created:     img.Created().Unix(),
-			Dangling:    isDangling,
-			Digest:      string(img.Digest()),
-			RepoDigests: repoDigests,
-			History:     img.NamesHistory(),
-			Names:       img.Names(),
-			ReadOnly:    img.IsReadOnly(),
-			SharedSize:  0,
-			RepoTags:    img.Names(), // may include tags and digests
-			ParentId:    parentID,
-		}
-		e.Labels, err = img.Labels(ctx)
-		if err != nil {
-			return nil, fmt.Errorf("retrieving label for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err)
-		}
+			s := &entities.ImageSummary{
+				ID:          img.ID(),
+				Created:     img.Created().Unix(),
+				Dangling:    isDangling,
+				Digest:      string(img.Digest()),
+				RepoDigests: repoDigests,
+				History:     img.NamesHistory(),
+				Names:       img.Names(),
+				ReadOnly:    img.IsReadOnly(),
+				SharedSize:  0,
+				RepoTags:    img.Names(), // may include tags and digests
+				ParentId:    parentID,
+			}
+			s.Labels, err = img.Labels(ctx)
+			if err != nil {
+				return nil, fmt.Errorf("retrieving label for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err)
+			}
 
-		ctnrs, err := img.Containers()
-		if err != nil {
-			return nil, fmt.Errorf("retrieving containers for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err)
-		}
-		e.Containers = len(ctnrs)
+			ctnrs, err := img.Containers()
+			if err != nil {
+				return nil, fmt.Errorf("retrieving containers for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err)
+			}
+			s.Containers = len(ctnrs)
 
-		sz, err := img.Size()
+			sz, err := img.Size()
+			if err != nil {
+				return nil, fmt.Errorf("retrieving size of image %q: you may need to remove the image to resolve the error: %w", img.ID(), err)
+			}
+			s.Size = sz
+			// This is good enough for now, but has to be
+			// replaced later with correct calculation logic
+			s.VirtualSize = sz
+			return s, nil
+		}()
 		if err != nil {
-			return nil, fmt.Errorf("retrieving size of image %q: you may need to remove the image to resolve the error: %w", img.ID(), err)
+			if libimage.ErrorIsImageUnknown(err) {
+				// The image may have been (partially) removed in the meantime
+				continue
+			}
+			return nil, err
 		}
-		e.Size = sz
-		// This is good enough for now, but has to be
-		// replaced later with correct calculation logic
-		e.VirtualSize = sz
-
-		summaries = append(summaries, &e)
+		summaries = append(summaries, summary)
 	}
 	return summaries, nil
 }

test/system/010-images.bats

@@ -359,4 +359,40 @@ EOF
     run_podman --root $imstore/root rmi --all
 }
 
+@test "podman images with concurrent removal" {
+    skip_if_remote "following test is not supported for remote clients"
+    local count=5
+
+    # First build $count images
+    for i in $(seq --format '%02g' 1 $count); do
+        cat >$PODMAN_TMPDIR/Containerfile <<EOF
+FROM $IMAGE
+RUN echo $i
+EOF
+        run_podman build -q -t i$i $PODMAN_TMPDIR
+    done
+
+    run_podman images
+    # Now remove all images in parallel and in the background and make sure
+    # that listing all images does not fail (see BZ 2216700).
+    for i in $(seq --format '%02g' 1 $count); do
+        timeout --foreground -v --kill=10 60 \
+                $PODMAN rmi i$i &
+    done
+
+    tries=100
+    while [[ ${#lines[*]} -gt 1 ]] && [[ $tries -gt 0 ]]; do
+        # Prior to #18980, 'podman images' during rmi could fail with 'image not known'
+        run_podman images --format "{{.ID}} {{.Names}}"
+        tries=$((tries - 1))
+    done
+
+    if [[ $tries -eq 0 ]]; then
+        die "Timed out waiting for images to be removed"
+    fi
+
+    wait
+}
+
+
 # vim: filetype=sh