Skip to content

Commit

Permalink
Fix multiple filter options logic for podman volume ls
Browse files Browse the repository at this point in the history 
Fixes a bug where `podman volume ls` with multiple `label` filters would
return volumes that matched *any* of the filters, not *all* of them.

Adapts generating volume filter functions to be more in
line with how it is done for containers and pods.

Fixes: containers#19219

Signed-off-by: Jake Correnti <[email protected]>
  • Loading branch information
@jakecorrenti @ashley-cui
jakecorrenti authored and ashley-cui committed Jul 20, 2023
1 parent 5ea0194 commit 7cbe412
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 130 deletions.
5 changes: 5 additions & 0 deletions docs/source/markdown/podman-volume-ls.1.md.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ flag. Use the **--quiet** flag to print only the volume names.

#### **--filter**, **-f**=*filter*

Filter what volumes are shown in the output.
Multiple filters can be given with multiple uses of the --filter flag.
Filters with the same key work inclusive, with the only exception being `label`
which is exclusive. Filters with different keys always work exclusive.

Volumes can be filtered by the following attributes:

| **Filter** | **Description** |
Expand Down
24 changes: 16 additions & 8 deletions pkg/api/handlers/compat/volumes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,14 @@ func ListVolumes(w http.ResponseWriter, r *http.Request) {
return
}
}
volumeFilters, err := filters.GenerateVolumeFilters(*filtersMap)
if err != nil {
utils.InternalServerError(w, err)
return

volumeFilters := []libpod.VolumeFilter{}
for filter, filterValues := range *filtersMap {
filterFunc, err := filters.GenerateVolumeFilters(filter, filterValues)
if err != nil {
utils.InternalServerError(w, err)
}
volumeFilters = append(volumeFilters, filterFunc)
}

vols, err := runtime.Volumes(volumeFilters...)
Expand Down Expand Up @@ -270,10 +274,14 @@ func PruneVolumes(w http.ResponseWriter, r *http.Request) {
}

f := (url.Values)(*filterMap)
filterFuncs, err := filters.GeneratePruneVolumeFilters(f)
if err != nil {
utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to parse filters for %s: %w", f.Encode(), err))
return
filterFuncs := []libpod.VolumeFilter{}
for filter, filterValues := range f {
filterFunc, err := filters.GeneratePruneVolumeFilters(filter, filterValues)
if err != nil {
utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to parse filters for %s: %w", f.Encode(), err))
return
}
filterFuncs = append(filterFuncs, filterFunc)
}

pruned, err := runtime.PruneVolumes(r.Context(), filterFuncs)
Expand Down
22 changes: 15 additions & 7 deletions pkg/api/handlers/libpod/volumes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,14 @@ func ListVolumes(w http.ResponseWriter, r *http.Request) {
return
}

volumeFilters, err := filters.GenerateVolumeFilters(*filterMap)
if err != nil {
utils.InternalServerError(w, err)
return
volumeFilters := []libpod.VolumeFilter{}
for filter, filterValues := range *filterMap {
filterFunc, err := filters.GenerateVolumeFilters(filter, filterValues)
if err != nil {
utils.InternalServerError(w, err)
return
}
volumeFilters = append(volumeFilters, filterFunc)
}

vols, err := runtime.Volumes(volumeFilters...)
Expand Down Expand Up @@ -162,9 +166,13 @@ func pruneVolumesHelper(r *http.Request) ([]*reports.PruneReport, error) {
}

f := (url.Values)(*filterMap)
filterFuncs, err := filters.GeneratePruneVolumeFilters(f)
if err != nil {
return nil, err
filterFuncs := []libpod.VolumeFilter{}
for filter, filterValues := range f {
filterFunc, err := filters.GeneratePruneVolumeFilters(filter, filterValues)
if err != nil {
return nil, err
}
filterFuncs = append(filterFuncs, filterFunc)
}

reports, err := runtime.PruneVolumes(r.Context(), filterFuncs)
Expand Down
178 changes: 83 additions & 95 deletions pkg/domain/filters/volumes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,48 @@ package filters

import (
"fmt"
"net/url"
"regexp"
"strings"

"github.com/containers/common/pkg/filters"
"github.com/containers/podman/v4/libpod"
"github.com/containers/podman/v4/pkg/util"
)

func GenerateVolumeFilters(filtersValues url.Values) ([]libpod.VolumeFilter, error) {
var vf []libpod.VolumeFilter
for filter, v := range filtersValues {
for _, val := range v {
switch filter {
case "name":
nameRegexp, err := regexp.Compile(val)
if err != nil {
return nil, err
func GenerateVolumeFilters(filter string, filterValues []string) (libpod.VolumeFilter, error) {
switch filter {
case "name":
return func(v *libpod.Volume) bool {
return util.StringMatchRegexSlice(v.Name(), filterValues)
}, nil
case "driver":
return func(v *libpod.Volume) bool {
for _, val := range filterValues {
if v.Driver() == val {
return true
}
}
return false
}, nil
case "scope":
return func(v *libpod.Volume) bool {
for _, val := range filterValues {
if v.Scope() == val {
return true
}
vf = append(vf, func(v *libpod.Volume) bool {
return nameRegexp.MatchString(v.Name())
})
case "driver":
driverVal := val
vf = append(vf, func(v *libpod.Volume) bool {
return v.Driver() == driverVal
})
case "scope":
scopeVal := val
vf = append(vf, func(v *libpod.Volume) bool {
return v.Scope() == scopeVal
})
case "label":
filter := val
vf = append(vf, func(v *libpod.Volume) bool {
return filters.MatchLabelFilters([]string{filter}, v.Labels())
})
case "label!":
filter := val
vf = append(vf, func(v *libpod.Volume) bool {
return !filters.MatchLabelFilters([]string{filter}, v.Labels())
})
case "opt":
}
return false
}, nil
case "label":
return func(v *libpod.Volume) bool {
return filters.MatchLabelFilters(filterValues, v.Labels())
}, nil
case "label!":
return func(v *libpod.Volume) bool {
return !filters.MatchLabelFilters(filterValues, v.Labels())
}, nil
case "opt":
return func(v *libpod.Volume) bool {
for _, val := range filterValues {
filterArray := strings.SplitN(val, "=", 2)
filterKey := filterArray[0]
var filterVal string
Expand All @@ -52,82 +52,70 @@ func GenerateVolumeFilters(filtersValues url.Values) ([]libpod.VolumeFilter, err
} else {
filterVal = ""
}
vf = append(vf, func(v *libpod.Volume) bool {
for labelKey, labelValue := range v.Options() {
if labelKey == filterKey && (filterVal == "" || labelValue == filterVal) {
return true
}

for labelKey, labelValue := range v.Options() {
if labelKey == filterKey && (filterVal == "" || labelValue == filterVal) {
return true
}
return false
})
case "until":
f, err := createUntilFilterVolumeFunction(val)
}
}
return false
}, nil
case "until":
return createUntilFilterVolumeFunction(filterValues)
case "dangling":
for _, val := range filterValues {
switch strings.ToLower(val) {
case "true", "1", "false", "0":
default:
return nil, fmt.Errorf("%q is not a valid value for the \"dangling\" filter - must be true or false", val)
}
}
return func(v *libpod.Volume) bool {
for _, val := range filterValues {
dangling, err := v.IsDangling()
if err != nil {
return nil, err
return false
}
vf = append(vf, f)
case "dangling":
danglingVal := val

invert := false
switch strings.ToLower(danglingVal) {
case "true", "1":
// Do nothing
switch strings.ToLower(val) {
case "false", "0":
// Dangling=false requires that we
// invert the result of IsDangling.
invert = true
default:
return nil, fmt.Errorf("%q is not a valid value for the \"dangling\" filter - must be true or false", danglingVal)
}
vf = append(vf, func(v *libpod.Volume) bool {
dangling, err := v.IsDangling()
if err != nil {
return false
}
if invert {
return !dangling
}
return dangling
})
default:
return nil, fmt.Errorf("%q is an invalid volume filter", filter)
if invert {
dangling = !dangling
}
if dangling {
return true
}
}
}
return false
}, nil
}
return vf, nil
return nil, fmt.Errorf("%q is an invalid volume filter", filter)
}

func GeneratePruneVolumeFilters(filtersValues url.Values) ([]libpod.VolumeFilter, error) {
var vf []libpod.VolumeFilter
for filter, v := range filtersValues {
for _, val := range v {
filterVal := val
switch filter {
case "label":
vf = append(vf, func(v *libpod.Volume) bool {
return filters.MatchLabelFilters([]string{filterVal}, v.Labels())
})
case "label!":
filter := val
vf = append(vf, func(v *libpod.Volume) bool {
return !filters.MatchLabelFilters([]string{filter}, v.Labels())
})
case "until":
f, err := createUntilFilterVolumeFunction(filterVal)
if err != nil {
return nil, err
}
vf = append(vf, f)
default:
return nil, fmt.Errorf("%q is an invalid volume filter", filter)
}
}
func GeneratePruneVolumeFilters(filter string, filterValues []string) (libpod.VolumeFilter, error) {
switch filter {
case "label":
return func(v *libpod.Volume) bool {
return filters.MatchLabelFilters(filterValues, v.Labels())
}, nil
case "label!":
return func(v *libpod.Volume) bool {
return !filters.MatchLabelFilters(filterValues, v.Labels())
}, nil
case "until":
return createUntilFilterVolumeFunction(filterValues)
}
return vf, nil
return nil, fmt.Errorf("%q is an invalid volume filter", filter)
}

func createUntilFilterVolumeFunction(filter string) (libpod.VolumeFilter, error) {
until, err := filters.ComputeUntilTimestamp([]string{filter})
func createUntilFilterVolumeFunction(filterValues []string) (libpod.VolumeFilter, error) {
until, err := filters.ComputeUntilTimestamp(filterValues)
if err != nil {
return nil, err
}
Expand Down
23 changes: 16 additions & 7 deletions pkg/domain/infra/abi/volumes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,15 @@ func (ic *ContainerEngine) VolumeInspect(ctx context.Context, namesOrIds []strin
}

func (ic *ContainerEngine) VolumePrune(ctx context.Context, options entities.VolumePruneOptions) ([]*reports.PruneReport, error) {
filterFuncs, err := filters.GenerateVolumeFilters(options.Filters)
if err != nil {
return nil, err
funcs := []libpod.VolumeFilter{}
for filter, filterValues := range options.Filters {
filterFunc, err := filters.GenerateVolumeFilters(filter, filterValues)
if err != nil {
return nil, err
}
funcs = append(funcs, filterFunc)
}
return ic.pruneVolumesHelper(ctx, filterFuncs)
return ic.pruneVolumesHelper(ctx, funcs)
}

func (ic *ContainerEngine) pruneVolumesHelper(ctx context.Context, filterFuncs []libpod.VolumeFilter) ([]*reports.PruneReport, error) {
Expand All @@ -138,10 +142,15 @@ func (ic *ContainerEngine) pruneVolumesHelper(ctx context.Context, filterFuncs [
}

func (ic *ContainerEngine) VolumeList(ctx context.Context, opts entities.VolumeListOptions) ([]*entities.VolumeListReport, error) {
volumeFilters, err := filters.GenerateVolumeFilters(opts.Filter)
if err != nil {
return nil, err
volumeFilters := []libpod.VolumeFilter{}
for filter, value := range opts.Filter {
filterFunc, err := filters.GenerateVolumeFilters(filter, value)
if err != nil {
return nil, err
}
volumeFilters = append(volumeFilters, filterFunc)
}

vols, err := ic.Libpod.Volumes(volumeFilters...)
if err != nil {
return nil, err
Expand Down
29 changes: 16 additions & 13 deletions test/e2e/volume_ls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,32 +180,35 @@ var _ = Describe("Podman volume ls", func() {
})

It("podman ls volume with multiple --filter flag", func() {
session := podmanTest.Podman([]string{"volume", "create", "--label", "foo=bar", "myvol"})
volName := session.OutputToString()
session := podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "--label", "b=c", "vol1"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))

session = podmanTest.Podman([]string{"volume", "create", "--label", "foo2=bar2", "anothervol"})
anotherVol := session.OutputToString()
vol1Name := session.OutputToString()

session = podmanTest.Podman([]string{"volume", "create", "--label", "b=c", "--label", "a=b", "vol2"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))

session = podmanTest.Podman([]string{"volume", "create"})
vol2Name := session.OutputToString()

session = podmanTest.Podman([]string{"volume", "create", "--label", "b=c", "--label", "c=d", "vol3"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))

session = podmanTest.Podman([]string{"volume", "ls", "--filter", "label=foo", "--filter", "label=foo2"})
vol3Name := session.OutputToString()

session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=a=b", "--filter", "label=b=c"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expect(session.OutputToStringArray()).To(HaveLen(3))
Expect(session.OutputToStringArray()[1]).To(ContainSubstring(volName))
Expect(session.OutputToStringArray()[2]).To(ContainSubstring(anotherVol))
Expect(session.OutputToStringArray()).To(HaveLen(2))
Expect(session.OutputToStringArray()[0]).To(Equal(vol1Name))
Expect(session.OutputToStringArray()[1]).To(Equal(vol2Name))

session = podmanTest.Podman([]string{"volume", "ls", "--filter", "label=foo=bar", "--filter", "label=foo2=bar2"})
session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=c=d", "--filter", "label=b=c"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expect(session.OutputToStringArray()).To(HaveLen(3))
Expect(session.OutputToStringArray()[1]).To(ContainSubstring(volName))
Expect(session.OutputToStringArray()[2]).To(ContainSubstring(anotherVol))
Expect(session.OutputToStringArray()).To(HaveLen(1))
Expect(session.OutputToStringArray()[0]).To(Equal(vol3Name))
})
})

0 comments on commit 7cbe412

Please sign in to comment.