Merge pull request #6312 from rhatdan/image

Fix remote handling of podman images calls

cmd/podman/images/list.go

@@ -85,7 +85,7 @@ func images(cmd *cobra.Command, args []string) error {
 		return errors.New("cannot specify an image and a filter(s)")
 	}
 
-	if len(listOptions.Filter) < 1 && len(args) > 0 {
+	if len(args) > 0 {
 		listOptions.Filter = append(listOptions.Filter, "reference="+args[0])
 	}
 
@@ -152,12 +152,18 @@ func writeTemplate(imageS []*entities.ImageSummary) error {
 	)
 	imgs := make([]imageReporter, 0, len(imageS))
 	for _, e := range imageS {
-		for _, tag := range e.RepoTags {
 		var h imageReporter
+		if len(e.RepoTags) > 0 {
+			for _, tag := range e.RepoTags {
 				h.ImageSummary = *e
 				h.Repository, h.Tag = tokenRepoTag(tag)
 				imgs = append(imgs, h)
 			}
+		} else {
+			h.ImageSummary = *e
+			h.Repository = "<none>"
+			imgs = append(imgs, h)
+		}
 		listFlag.readOnly = e.IsReadOnly()
 	}
 

cmd/podman/images/prune.go

@@ -61,12 +61,6 @@ Are you sure you want to continue? [y/N] `)
 		}
 	}
 
-	// TODO Remove once filter refactor is finished and url.Values rules :)
-	for _, f := range filter {
-		t := strings.SplitN(f, "=", 2)
-		pruneOpts.Filters.Add(t[0], t[1])
-	}
-
 	results, err := registry.ImageEngine().Prune(registry.GetContext(), pruneOpts)
 	if err != nil {
 		return err

libpod/image/filters.go

@@ -170,8 +170,7 @@ func (ir *Runtime) createFilterFuncs(filters []string, img *Image) ([]ResultFilt
 			labelFilter := strings.Join(splitFilter[1:], "=")
 			filterFuncs = append(filterFuncs, LabelFilter(ctx, labelFilter))
 		case "reference":
-			referenceFilter := strings.Join(splitFilter[1:], "=")
+			filterFuncs = append(filterFuncs, ReferenceFilter(ctx, splitFilter[1]))
-			filterFuncs = append(filterFuncs, ReferenceFilter(ctx, referenceFilter))
 		case "id":
 			filterFuncs = append(filterFuncs, IdFilter(splitFilter[1]))
 		default:

pkg/api/handlers/utils/images.go

@@ -62,7 +62,6 @@ func GetImages(w http.ResponseWriter, r *http.Request) ([]*image.Image, error) {
 	}{
 		// This is where you can override the golang default value for one of fields
 	}
-	// TODO I think all is implemented with a filter?
 
 	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
 		return nil, err
@@ -71,6 +70,10 @@ func GetImages(w http.ResponseWriter, r *http.Request) ([]*image.Image, error) {
 	if _, found := r.URL.Query()["digests"]; found && query.Digests {
 		UnSupportedParameter("digests")
 	}
+	var (
+		images []*image.Image
+		err    error
+	)
 
 	if len(query.Filters) > 0 {
 		for k, v := range query.Filters {
@@ -78,11 +81,33 @@ func GetImages(w http.ResponseWriter, r *http.Request) ([]*image.Image, error) {
 				filters = append(filters, fmt.Sprintf("%s=%s", k, val))
 			}
 		}
-		return runtime.ImageRuntime().GetImagesWithFilters(filters)
+		images, err = runtime.ImageRuntime().GetImagesWithFilters(filters)
-	} else {
+		if err != nil {
-		return runtime.ImageRuntime().GetImages()
+			return images, err
 		}
-
+	} else {
+		images, err = runtime.ImageRuntime().GetImages()
+		if err != nil {
+			return images, err
+		}
+	}
+	if query.All {
+		return images, nil
+	}
+	var returnImages []*image.Image
+	for _, img := range images {
+		if len(img.Names()) == 0 {
+			parent, err := img.IsParent(r.Context())
+			if err != nil {
+				return nil, err
+			}
+			if parent {
+				continue
+			}
+		}
+		returnImages = append(returnImages, img)
+	}
+	return returnImages, nil
 }
 
 func GetImage(r *http.Request, name string) (*image.Image, error) {

pkg/domain/entities/images.go

@@ -1,7 +1,6 @@
 package entities
 
 import (
-	"net/url"
 	"time"
 
 	"github.com/containers/image/v5/manifest"
@@ -223,13 +222,11 @@ type ImageSearchReport struct {
 type ImageListOptions struct {
 	All    bool     `json:"all" schema:"all"`
 	Filter []string `json:"Filter,omitempty"`
-	Filters url.Values `json:"filters" schema:"filters"`
 }
 
 type ImagePruneOptions struct {
 	All    bool     `json:"all" schema:"all"`
 	Filter []string `json:"filter" schema:"filter"`
-	Filters url.Values `json:"filters" schema:"filters"`
 }
 
 type ImagePruneReport struct {

pkg/domain/infra/abi/images_list.go

@@ -13,14 +13,7 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions)
 		err    error
 	)
 
-	// TODO: Future work support for domain.Filters
-	// filters := utils.ToLibpodFilters(opts.Filters)
-
-	if len(opts.Filter) > 0 {
 	images, err = ir.Libpod.ImageRuntime().GetImagesWithFilters(opts.Filter)
-	} else {
-		images, err = ir.Libpod.ImageRuntime().GetImages()
-	}
 	if err != nil {
 		return nil, err
 	}
@@ -40,11 +33,20 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions)
 				}
 			}
 		} else {
-			repoTags, _ = img.RepoTags()
+			repoTags, err = img.RepoTags()
-			if len(repoTags) == 0 {
+			if err != nil {
+				return nil, err
+			}
+			if len(img.Names()) == 0 {
+				parent, err := img.IsParent(ctx)
+				if err != nil {
+					return nil, err
+				}
+				if parent {
 					continue
 				}
 			}
+		}
 
 		digests := make([]string, len(img.Digests()))
 		for j, d := range img.Digests() {

pkg/domain/infra/tunnel/images.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"io/ioutil"
 	"os"
+	"strings"
 
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/docker/reference"
@@ -25,8 +26,13 @@ func (ir *ImageEngine) Remove(ctx context.Context, imagesArg []string, opts enti
 }
 
 func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions) ([]*entities.ImageSummary, error) {
-	images, err := images.List(ir.ClientCxt, &opts.All, opts.Filters)
 
+	filters := make(map[string][]string, len(opts.Filter))
+	for _, filter := range opts.Filter {
+		f := strings.Split(filter, "=")
+		filters[f[0]] = f[1:]
+	}
+	images, err := images.List(ir.ClientCxt, &opts.All, filters)
 	if err != nil {
 		return nil, err
 	}
@@ -61,7 +67,13 @@ func (ir *ImageEngine) History(ctx context.Context, nameOrId string, opts entiti
 }
 
 func (ir *ImageEngine) Prune(ctx context.Context, opts entities.ImagePruneOptions) (*entities.ImagePruneReport, error) {
-	results, err := images.Prune(ir.ClientCxt, &opts.All, opts.Filters)
+	filters := make(map[string][]string, len(opts.Filter))
+	for _, filter := range opts.Filter {
+		f := strings.Split(filter, "=")
+		filters[f[0]] = f[1:]
+	}
+
+	results, err := images.Prune(ir.ClientCxt, &opts.All, filters)
 	if err != nil {
 		return nil, err
 	}

test/e2e/images_test.go

@@ -152,9 +152,7 @@ var _ = Describe("Podman images", func() {
 	})
 
 	It("podman images filter reference", func() {
-		if podmanTest.RemoteTest {
+		SkipIfRemote()
-			Skip("Does not work on remote client")
-		}
 		podmanTest.RestoreAllArtifacts()
 		result := podmanTest.PodmanNoCache([]string{"images", "-q", "-f", "reference=docker.io*"})
 		result.WaitWithDefaultTimeout()
@@ -180,9 +178,7 @@ var _ = Describe("Podman images", func() {
 	})
 
 	It("podman images filter before image", func() {
-		if podmanTest.RemoteTest {
+		SkipIfRemote()
-			Skip("Does not work on remote client")
-		}
 		dockerfile := `FROM docker.io/library/alpine:latest
 RUN apk update && apk add man
 `
@@ -194,9 +190,7 @@ RUN apk update && apk add man
 	})
 
 	It("podman images filter after image", func() {
-		if podmanTest.RemoteTest {
+		SkipIfRemote()
-			Skip("Does not work on remote client")
-		}
 		podmanTest.RestoreAllArtifacts()
 		rmi := podmanTest.PodmanNoCache([]string{"rmi", "busybox"})
 		rmi.WaitWithDefaultTimeout()
@@ -212,9 +206,7 @@ RUN apk update && apk add man
 	})
 
 	It("podman image list filter after image", func() {
-		if podmanTest.RemoteTest {
+		SkipIfRemote()
-			Skip("Does not work on remote client")
-		}
 		podmanTest.RestoreAllArtifacts()
 		rmi := podmanTest.PodmanNoCache([]string{"image", "rm", "busybox"})
 		rmi.WaitWithDefaultTimeout()
@@ -230,9 +222,7 @@ RUN apk update && apk add man
 	})
 
 	It("podman images filter dangling", func() {
-		if podmanTest.RemoteTest {
+		SkipIfRemote()
-			Skip("Does not work on remote client")
-		}
 		dockerfile := `FROM docker.io/library/alpine:latest
 `
 		podmanTest.BuildImage(dockerfile, "foobar.com/before:latest", "false")
@@ -308,9 +298,7 @@ RUN apk update && apk add man
 	})
 
 	It("podman images --all flag", func() {
-		if podmanTest.RemoteTest {
+		SkipIfRemote()
-			Skip("Does not work on remote client")
-		}
 		podmanTest.RestoreAllArtifacts()
 		dockerfile := `FROM docker.io/library/alpine:latest
 RUN mkdir hello
@@ -343,10 +331,7 @@ LABEL "com.example.vendor"="Example Vendor"
 	})
 
 	It("podman with images with no layers", func() {
-		if podmanTest.RemoteTest {
+		SkipIfRemote()
-			Skip("Does not work on remote client")
-		}
-
 		dockerfile := strings.Join([]string{
 			`FROM scratch`,
 			`LABEL org.opencontainers.image.authors="<somefolks@example.org>"`,