From a4c8198afd2b92b40b95c6fe01756cf2d0076cb6 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 20 May 2020 20:46:43 -0400 Subject: Fix remote handling of podman images calls Enable three more tests Fix handling of image filters Signed-off-by: Daniel J Walsh --- cmd/podman/images/list.go | 14 ++++++++++---- libpod/image/filters.go | 3 +-- pkg/api/handlers/utils/images.go | 33 +++++++++++++++++++++++++++++---- pkg/domain/entities/images.go | 5 ++--- pkg/domain/infra/abi/images_list.go | 24 +++++++++++++----------- pkg/domain/infra/tunnel/images.go | 9 ++++++++- test/e2e/images_test.go | 36 +++++++----------------------------- 7 files changed, 70 insertions(+), 54 deletions(-) diff --git a/cmd/podman/images/list.go b/cmd/podman/images/list.go index 83c039ed3..022c90f71 100644 --- a/cmd/podman/images/list.go +++ b/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,10 +152,16 @@ func writeTemplate(imageS []*entities.ImageSummary) error { ) imgs := make([]imageReporter, 0, len(imageS)) for _, e := range imageS { - for _, tag := range e.RepoTags { - var h imageReporter + 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, h.Tag = tokenRepoTag(tag) + h.Repository = "" imgs = append(imgs, h) } listFlag.readOnly = e.IsReadOnly() diff --git a/libpod/image/filters.go b/libpod/image/filters.go index 8ca3526a0..747eba165 100644 --- a/libpod/image/filters.go +++ b/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, referenceFilter)) + filterFuncs = append(filterFuncs, ReferenceFilter(ctx, splitFilter[1])) case "id": filterFuncs = append(filterFuncs, IdFilter(splitFilter[1])) default: diff --git a/pkg/api/handlers/utils/images.go b/pkg/api/handlers/utils/images.go index 1c67de9db..7fb31a177 100644 --- a/pkg/api/handlers/utils/images.go +++ b/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) + if err != nil { + return images, err + } } else { - return runtime.ImageRuntime().GetImages() + 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) { diff --git a/pkg/domain/entities/images.go b/pkg/domain/entities/images.go index cce3001eb..9d5fd5cc9 100644 --- a/pkg/domain/entities/images.go +++ b/pkg/domain/entities/images.go @@ -221,9 +221,8 @@ type ImageSearchReport struct { // Image List Options type ImageListOptions struct { - All bool `json:"all" schema:"all"` - Filter []string `json:"Filter,omitempty"` - Filters url.Values `json:"filters" schema:"filters"` + All bool `json:"all" schema:"all"` + Filter []string `json:"Filter,omitempty"` } type ImagePruneOptions struct { diff --git a/pkg/domain/infra/abi/images_list.go b/pkg/domain/infra/abi/images_list.go index c559e250c..3034e36ec 100644 --- a/pkg/domain/infra/abi/images_list.go +++ b/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() - } + images, err = ir.Libpod.ImageRuntime().GetImagesWithFilters(opts.Filter) if err != nil { return nil, err } @@ -40,9 +33,18 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions) } } } else { - repoTags, _ = img.RepoTags() - if len(repoTags) == 0 { - continue + repoTags, err = img.RepoTags() + 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 + } } } diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 4d00d331b..a547a24d7 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/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,6 +67,7 @@ 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) if err != nil { return nil, err diff --git a/test/e2e/images_test.go b/test/e2e/images_test.go index cd281e3c7..118566ea3 100644 --- a/test/e2e/images_test.go +++ b/test/e2e/images_test.go @@ -131,7 +131,6 @@ var _ = Describe("Podman images", func() { }) It("podman images filter by image name", func() { - Skip(v2remotefail) podmanTest.RestoreAllArtifacts() session := podmanTest.PodmanNoCache([]string{"images", "-q", ALPINE}) session.WaitWithDefaultTimeout() @@ -152,9 +151,7 @@ var _ = Describe("Podman images", func() { }) It("podman images filter reference", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } + SkipIfRemote() podmanTest.RestoreAllArtifacts() result := podmanTest.PodmanNoCache([]string{"images", "-q", "-f", "reference=docker.io*"}) result.WaitWithDefaultTimeout() @@ -180,9 +177,7 @@ var _ = Describe("Podman images", func() { }) It("podman images filter before image", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } + SkipIfRemote() dockerfile := `FROM docker.io/library/alpine:latest RUN apk update && apk add man ` @@ -194,9 +189,7 @@ RUN apk update && apk add man }) It("podman images filter after image", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } + SkipIfRemote() podmanTest.RestoreAllArtifacts() rmi := podmanTest.PodmanNoCache([]string{"rmi", "busybox"}) rmi.WaitWithDefaultTimeout() @@ -212,9 +205,7 @@ RUN apk update && apk add man }) It("podman image list filter after image", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } + SkipIfRemote() podmanTest.RestoreAllArtifacts() rmi := podmanTest.PodmanNoCache([]string{"image", "rm", "busybox"}) rmi.WaitWithDefaultTimeout() @@ -230,9 +221,7 @@ RUN apk update && apk add man }) It("podman images filter dangling", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } + SkipIfRemote() dockerfile := `FROM docker.io/library/alpine:latest ` podmanTest.BuildImage(dockerfile, "foobar.com/before:latest", "false") @@ -244,9 +233,6 @@ RUN apk update && apk add man }) It("podman check for image with sha256: prefix", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } session := podmanTest.Podman([]string{"inspect", "--format=json", ALPINE}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -259,9 +245,6 @@ RUN apk update && apk add man }) It("podman check for image with sha256: prefix", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } session := podmanTest.Podman([]string{"image", "inspect", "--format=json", ALPINE}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -308,9 +291,7 @@ RUN apk update && apk add man }) It("podman images --all flag", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } + SkipIfRemote() podmanTest.RestoreAllArtifacts() dockerfile := `FROM docker.io/library/alpine:latest RUN mkdir hello @@ -343,10 +324,7 @@ LABEL "com.example.vendor"="Example Vendor" }) It("podman with images with no layers", func() { - if podmanTest.RemoteTest { - Skip("Does not work on remote client") - } - + SkipIfRemote() dockerfile := strings.Join([]string{ `FROM scratch`, `LABEL org.opencontainers.image.authors=""`, -- cgit v1.2.3-54-g00ecf