From 4f427a89cbc0814c25dd4b562ddf4ff4e568a005 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 17 Nov 2020 16:43:17 +0100 Subject: [PATCH] Align the podman ps --filter behavior with docker All of our filters worked exclusive resulting in `--filter status=created --filter status=exited` to return nothing. In docker filters with the same key work inclusive with the only exception being `label` which is exclusive. Filters with different keys always work exclusive. This PR aims to match the docker behavior with podman. Signed-off-by: Paul Holzinger --- cmd/podman/containers/ps.go | 8 - libpod/filters/containers.go | 163 +++++++++++++------- pkg/api/handlers/compat/containers_prune.go | 14 +- pkg/domain/infra/abi/containers.go | 12 +- pkg/ps/ps.go | 16 +- pkg/util/utils.go | 12 ++ test/e2e/ps_test.go | 122 +++++++++++++++ 7 files changed, 258 insertions(+), 89 deletions(-) diff --git a/cmd/podman/containers/ps.go b/cmd/podman/containers/ps.go index a9e2d2e35407..642feb5e0af8 100644 --- a/cmd/podman/containers/ps.go +++ b/cmd/podman/containers/ps.go @@ -98,14 +98,6 @@ func checkFlags(c *cobra.Command) error { if listOpts.Last >= 0 && listOpts.Latest { return errors.Errorf("last and latest are mutually exclusive") } - // Filter on status forces all - for _, filter := range filters { - splitFilter := strings.SplitN(filter, "=", 2) - if strings.ToLower(splitFilter[0]) == "status" { - listOpts.All = true - break - } - } // Quiet conflicts with size and namespace and is overridden by a Go // template. if listOpts.Quiet { diff --git a/libpod/filters/containers.go b/libpod/filters/containers.go index da1b5b263e53..2520c4f30807 100644 --- a/libpod/filters/containers.go +++ b/libpod/filters/containers.go @@ -1,7 +1,6 @@ package lpfilters import ( - "regexp" "strconv" "strings" "time" @@ -11,101 +10,133 @@ import ( "github.com/containers/podman/v2/pkg/timetype" "github.com/containers/podman/v2/pkg/util" "github.com/pkg/errors" - "github.com/sirupsen/logrus" ) // GenerateContainerFilterFuncs return ContainerFilter functions based of filter. -func GenerateContainerFilterFuncs(filter, filterValue string, r *libpod.Runtime) (func(container *libpod.Container) bool, error) { +func GenerateContainerFilterFuncs(filter string, filterValues []string, r *libpod.Runtime) (func(container *libpod.Container) bool, error) { switch filter { case "id": + // we only have to match one ID return func(c *libpod.Container) bool { - return strings.Contains(c.ID(), filterValue) + return util.StringMatchRegexSlice(c.ID(), filterValues) }, nil case "label": - var filterArray = strings.SplitN(filterValue, "=", 2) - var filterKey = filterArray[0] - if len(filterArray) > 1 { - filterValue = filterArray[1] - } else { - filterValue = "" - } + // we have to match that all given labels exits on that container return func(c *libpod.Container) bool { - for labelKey, labelValue := range c.Labels() { - if labelKey == filterKey && ("" == filterValue || labelValue == filterValue) { - return true + labels := c.Labels() + for _, filterValue := range filterValues { + matched := false + filterArray := strings.SplitN(filterValue, "=", 2) + filterKey := filterArray[0] + if len(filterArray) > 1 { + filterValue = filterArray[1] + } else { + filterValue = "" + } + for labelKey, labelValue := range labels { + if labelKey == filterKey && ("" == filterValue || labelValue == filterValue) { + matched = true + break + } + } + if !matched { + return false } } - return false + return true }, nil case "name": + // we only have to match one name return func(c *libpod.Container) bool { - match, err := regexp.MatchString(filterValue, c.Name()) - if err != nil { - logrus.Errorf("Failed to compile regex for 'name' filter: %v", err) - return false - } - return match + return util.StringMatchRegexSlice(c.Name(), filterValues) }, nil case "exited": - exitCode, err := strconv.ParseInt(filterValue, 10, 32) - if err != nil { - return nil, errors.Wrapf(err, "exited code out of range %q", filterValue) + var exitCodes []int32 + for _, exitCode := range filterValues { + ec, err := strconv.ParseInt(exitCode, 10, 32) + if err != nil { + return nil, errors.Wrapf(err, "exited code out of range %q", ec) + } + exitCodes = append(exitCodes, int32(ec)) } return func(c *libpod.Container) bool { ec, exited, err := c.ExitCode() - if ec == int32(exitCode) && err == nil && exited { - return true + if err == nil && exited { + for _, exitCode := range exitCodes { + if ec == exitCode { + return true + } + } } return false }, nil case "status": - if !util.StringInSlice(filterValue, []string{"created", "running", "paused", "stopped", "exited", "unknown"}) { - return nil, errors.Errorf("%s is not a valid status", filterValue) + for _, filterValue := range filterValues { + if !util.StringInSlice(filterValue, []string{"created", "running", "paused", "stopped", "exited", "unknown"}) { + return nil, errors.Errorf("%s is not a valid status", filterValue) + } } return func(c *libpod.Container) bool { status, err := c.State() if err != nil { return false } - if filterValue == "stopped" { - filterValue = "exited" - } state := status.String() if status == define.ContainerStateConfigured { state = "created" } else if status == define.ContainerStateStopped { state = "exited" } - return state == filterValue + for _, filterValue := range filterValues { + if filterValue == "stopped" { + filterValue = "exited" + } + if state == filterValue { + return true + } + } + return false }, nil case "ancestor": // This needs to refine to match docker // - ancestor=([:tag]|| ⟨image@digest⟩) - containers created from an image or a descendant. return func(c *libpod.Container) bool { - containerConfig := c.Config() - if strings.Contains(containerConfig.RootfsImageID, filterValue) || strings.Contains(containerConfig.RootfsImageName, filterValue) { - return true + for _, filterValue := range filterValues { + containerConfig := c.Config() + if strings.Contains(containerConfig.RootfsImageID, filterValue) || strings.Contains(containerConfig.RootfsImageName, filterValue) { + return true + } } return false }, nil case "before": - ctr, err := r.LookupContainer(filterValue) - if err != nil { - return nil, errors.Errorf("unable to find container by name or id of %s", filterValue) + var createTime time.Time + for _, filterValue := range filterValues { + ctr, err := r.LookupContainer(filterValue) + if err != nil { + return nil, err + } + containerConfig := ctr.Config() + if createTime.IsZero() || createTime.After(containerConfig.CreatedTime) { + createTime = containerConfig.CreatedTime + } } - containerConfig := ctr.Config() - createTime := containerConfig.CreatedTime return func(c *libpod.Container) bool { cc := c.Config() return createTime.After(cc.CreatedTime) }, nil case "since": - ctr, err := r.LookupContainer(filterValue) - if err != nil { - return nil, errors.Errorf("unable to find container by name or id of %s", filterValue) + var createTime time.Time + for _, filterValue := range filterValues { + ctr, err := r.LookupContainer(filterValue) + if err != nil { + return nil, err + } + containerConfig := ctr.Config() + if createTime.IsZero() || createTime.After(containerConfig.CreatedTime) { + createTime = containerConfig.CreatedTime + } } - containerConfig := ctr.Config() - createTime := containerConfig.CreatedTime return func(c *libpod.Container) bool { cc := c.Config() return createTime.Before(cc.CreatedTime) @@ -115,17 +146,27 @@ func GenerateContainerFilterFuncs(filter, filterValue string, r *libpod.Runtime) return func(c *libpod.Container) bool { containerConfig := c.Config() var dest string - arr := strings.Split(filterValue, ":") - source := arr[0] - if len(arr) == 2 { - dest = arr[1] - } - for _, mount := range containerConfig.Spec.Mounts { - if dest != "" && (mount.Source == source && mount.Destination == dest) { - return true + for _, filterValue := range filterValues { + arr := strings.SplitN(filterValue, ":", 2) + source := arr[0] + if len(arr) == 2 { + dest = arr[1] } - if dest == "" && mount.Source == source { - return true + for _, mount := range containerConfig.Spec.Mounts { + if dest != "" && (mount.Source == source && mount.Destination == dest) { + return true + } + if dest == "" && mount.Source == source { + return true + } + } + for _, vname := range containerConfig.NamedVolumes { + if dest != "" && (vname.Name == source && vname.Dest == dest) { + return true + } + if dest == "" && vname.Name == source { + return true + } } } return false @@ -136,10 +177,18 @@ func GenerateContainerFilterFuncs(filter, filterValue string, r *libpod.Runtime) if err != nil { return false } - return hcStatus == filterValue + for _, filterValue := range filterValues { + if hcStatus == filterValue { + return true + } + } + return false }, nil case "until": - ts, err := timetype.GetTimestamp(filterValue, time.Now()) + if len(filterValues) != 1 { + return nil, errors.Errorf("specify exactly one timestamp for %s", filter) + } + ts, err := timetype.GetTimestamp(filterValues[0], time.Now()) if err != nil { return nil, err } diff --git a/pkg/api/handlers/compat/containers_prune.go b/pkg/api/handlers/compat/containers_prune.go index 397feac9a362..2cfeebcce3f4 100644 --- a/pkg/api/handlers/compat/containers_prune.go +++ b/pkg/api/handlers/compat/containers_prune.go @@ -16,7 +16,6 @@ func PruneContainers(w http.ResponseWriter, r *http.Request) { var ( delContainers []string space int64 - filterFuncs []libpod.ContainerFilter ) runtime := r.Context().Value("runtime").(*libpod.Runtime) decoder := r.Context().Value("decoder").(*schema.Decoder) @@ -28,15 +27,14 @@ func PruneContainers(w http.ResponseWriter, r *http.Request) { utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String())) return } + filterFuncs := make([]libpod.ContainerFilter, 0, len(query.Filters)) for k, v := range query.Filters { - for _, val := range v { - generatedFunc, err := lpfilters.GenerateContainerFilterFuncs(k, val, runtime) - if err != nil { - utils.InternalServerError(w, err) - return - } - filterFuncs = append(filterFuncs, generatedFunc) + generatedFunc, err := lpfilters.GenerateContainerFilterFuncs(k, v, runtime) + if err != nil { + utils.InternalServerError(w, err) + return } + filterFuncs = append(filterFuncs, generatedFunc) } // Libpod response differs diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 855f9ece82a4..4b69ac74eec6 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -205,15 +205,13 @@ func (ic *ContainerEngine) ContainerStop(ctx context.Context, namesOrIds []strin } func (ic *ContainerEngine) ContainerPrune(ctx context.Context, options entities.ContainerPruneOptions) (*entities.ContainerPruneReport, error) { - var filterFuncs []libpod.ContainerFilter + filterFuncs := make([]libpod.ContainerFilter, 0, len(options.Filters)) for k, v := range options.Filters { - for _, val := range v { - generatedFunc, err := lpfilters.GenerateContainerFilterFuncs(k, val, ic.Libpod) - if err != nil { - return nil, err - } - filterFuncs = append(filterFuncs, generatedFunc) + generatedFunc, err := lpfilters.GenerateContainerFilterFuncs(k, v, ic.Libpod) + if err != nil { + return nil, err } + filterFuncs = append(filterFuncs, generatedFunc) } return ic.pruneContainersHelper(filterFuncs) } diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go index 96b2d754f2e4..3dd7eb0c67df 100644 --- a/pkg/ps/ps.go +++ b/pkg/ps/ps.go @@ -21,19 +21,17 @@ import ( func GetContainerLists(runtime *libpod.Runtime, options entities.ContainerListOptions) ([]entities.ListContainer, error) { var ( - filterFuncs []libpod.ContainerFilter - pss = []entities.ListContainer{} + pss = []entities.ListContainer{} ) + filterFuncs := make([]libpod.ContainerFilter, 0, len(options.Filters)) all := options.All || options.Last > 0 if len(options.Filters) > 0 { for k, v := range options.Filters { - for _, val := range v { - generatedFunc, err := lpfilters.GenerateContainerFilterFuncs(k, val, runtime) - if err != nil { - return nil, err - } - filterFuncs = append(filterFuncs, generatedFunc) + generatedFunc, err := lpfilters.GenerateContainerFilterFuncs(k, v, runtime) + if err != nil { + return nil, err } + filterFuncs = append(filterFuncs, generatedFunc) } } @@ -43,7 +41,7 @@ func GetContainerLists(runtime *libpod.Runtime, options entities.ContainerListOp all = true } if !all { - runningOnly, err := lpfilters.GenerateContainerFilterFuncs("status", define.ContainerStateRunning.String(), runtime) + runningOnly, err := lpfilters.GenerateContainerFilterFuncs("status", []string{define.ContainerStateRunning.String()}, runtime) if err != nil { return nil, err } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 415fd169bae4..f6a084c007de 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -6,6 +6,7 @@ import ( "os" "os/user" "path/filepath" + "regexp" "strconv" "strings" "sync" @@ -84,6 +85,17 @@ func StringInSlice(s string, sl []string) bool { return false } +// StringMatchRegexSlice determines if a given string matches one of the given regexes, returns bool +func StringMatchRegexSlice(s string, re []string) bool { + for _, r := range re { + m, err := regexp.MatchString(r, s) + if err == nil && m { + return true + } + } + return false +} + // ImageConfig is a wrapper around the OCIv1 Image Configuration struct exported // by containers/image, but containing additional fields that are not supported // by OCIv1 (but are by Docker v2) - notably OnBuild. diff --git a/test/e2e/ps_test.go b/test/e2e/ps_test.go index f3a66e58a65b..fd08d4308759 100644 --- a/test/e2e/ps_test.go +++ b/test/e2e/ps_test.go @@ -545,4 +545,126 @@ var _ = Describe("Podman ps", func() { Expect(result.ExitCode()).To(Equal(0)) Expect(result.OutputToString()).To(ContainSubstring("ago")) }) + + It("podman ps filter test", func() { + session := podmanTest.Podman([]string{"run", "-d", "--name", "test1", "--label", "foo=1", + "--label", "bar=2", "--volume", "volume1:/test", ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + cid1 := session.OutputToString() + + session = podmanTest.Podman([]string{"run", "--name", "test2", "--label", "foo=1", + ALPINE, "ls", "/fail"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(1)) + + session = podmanTest.Podman([]string{"create", "--name", "test3", ALPINE, cid1}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"run", "--name", "test4", "--volume", "volume1:/test1", + "--volume", "/:/test2", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "name=test"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(5)) + Expect(session.LineInOutputContains("test1")).To(BeTrue()) + Expect(session.LineInOutputContains("test2")).To(BeTrue()) + Expect(session.LineInOutputContains("test3")).To(BeTrue()) + Expect(session.LineInOutputContains("test4")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "name=test1", "--filter", "name=test2"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(3)) + Expect(session.LineInOutputContains("test1")).To(BeTrue()) + Expect(session.LineInOutputContains("test2")).To(BeTrue()) + + // check container id matches with regex + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "id=" + cid1[:40], "--filter", "id=" + cid1 + "$"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(2)) + Expect(session.LineInOutputContains("test1")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--filter", "status=created"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(2)) + Expect(session.LineInOutputContains("test3")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--filter", "status=created", "--filter", "status=exited"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(4)) + Expect(session.LineInOutputContains("test2")).To(BeTrue()) + Expect(session.LineInOutputContains("test3")).To(BeTrue()) + Expect(session.LineInOutputContains("test4")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "label=foo=1"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(3)) + Expect(session.LineInOutputContains("test1")).To(BeTrue()) + Expect(session.LineInOutputContains("test2")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--filter", "label=foo=1", "--filter", "status=exited"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(2)) + Expect(session.LineInOutputContains("test2")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "label=foo=1", "--filter", "label=non=1"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(1)) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "label=foo=1", "--filter", "label=bar=2"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(2)) + Expect(session.LineInOutputContains("test1")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "exited=1"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(2)) + Expect(session.LineInOutputContains("test2")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "exited=1", "--filter", "exited=0"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(3)) + Expect(session.LineInOutputContains("test2")).To(BeTrue()) + Expect(session.LineInOutputContains("test4")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "volume=volume1"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(3)) + Expect(session.LineInOutputContains("test1")).To(BeTrue()) + Expect(session.LineInOutputContains("test4")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "volume=/:/test2"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(2)) + Expect(session.LineInOutputContains("test4")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "before=test2"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(2)) + Expect(session.LineInOutputContains("test1")).To(BeTrue()) + + session = podmanTest.Podman([]string{"ps", "--all", "--filter", "since=test2"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(len(session.OutputToStringArray())).To(Equal(3)) + Expect(session.LineInOutputContains("test3")).To(BeTrue()) + Expect(session.LineInOutputContains("test4")).To(BeTrue()) + }) })