Skip to content

Conversation

trly
Copy link

@trly trly commented Aug 13, 2025

Volume ls and volume prune commands were incorrectly combining multiple
filters with OR logic instead of AND logic. This meant that specifying
multiple filters like --filter "label=a=b" --filter "label!=c=d" would
return volumes that matched ANY filter rather than ALL filters, causing
results to expand instead of narrow down as users expect.

Changed the filtering logic in libpod/runtime_volume.go to require all
filters to match (AND logic) instead of any filter matching (OR logic).
Updated the function comment to reflect the correct AND behavior.

Added integration tests covering:
- Basic AND logic with label filters (label=a=b AND label=c=d)
- Mixed positive/negative label filters (label=a=b AND label!=c=d)
- Cross-filter-type combinations (label filters AND time filters)
- Both volume ls and volume prune commands
- Verification that within-key OR logic still works correctly

Fixes: #26786

Signed-off-by: Travis Lyons <[email protected]>

Does this PR introduce a user-facing change?

Volume filtering logic has been fixed in podman volume ls and podman volume prune commands - filters now use AND logic instead of OR:

   * 'AND': multiple filters must all match to include a volume in results
   * 'OR': previous incorrect behavior where any filter match would include the volume

Copy link
Contributor

openshift-ci bot commented Aug 13, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: trly
Once this PR has been reviewed and has the lgtm label, please assign luap99 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

    Volume ls and volume prune commands were incorrectly combining multiple
    filters with OR logic instead of AND logic. This meant that specifying
    multiple filters like --filter "label=a=b" --filter "label!=c=d" would
    return volumes that matched ANY filter rather than ALL filters, causing
    results to expand instead of narrow down as users expect.

    Changed the filtering logic in libpod/runtime_volume.go to require all
    filters to match (AND logic) instead of any filter matching (OR logic).
    Updated the function comment to reflect the correct AND behavior.

    Added integration tests covering:
    - Basic AND logic with label filters (label=a=b AND label=c=d)
    - Mixed positive/negative label filters (label=a=b AND label!=c=d)
    - Cross-filter-type combinations (label filters AND time filters)
    - Both volume ls and volume prune commands
    - Verification that within-key OR logic still works correctly

    Fixes: containers#26786

    Signed-off-by: Travis Lyons <[email protected]>

Signed-off-by: Travis Lyons <[email protected]>
@trly trly force-pushed the fix_volume_filters branch from a5b73a0 to 504d2e8 Compare August 13, 2025 16:08
Copy link

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This chnage is simply a breaking change. I have not looked into the issue fully but as is this just makes filtering worse as it actively changes the semantics compared to containers for example which still use filters.MatchLabelFilters() for example.

I think the way label are should match what docker is doing, if there are specifc differences I am happy to see them and then we can look into how to fix them. But like this I think it is breaking change for all our user who depend on the current filters to work like they do

@Luap99
Copy link
Member

Luap99 commented Aug 13, 2025

podman volume ls --filter label=a=b --filter label=c=1 already works as AND so I don't see why you would need to chnage that, yes label! label might be a special case we need to consider.

@trly
Copy link
Author

trly commented Aug 13, 2025

Thanks for the feedback @Luap99.

I dug into this a bit more and will work to adjust my change accordingly. I definitely see now how the submitted change breaks expected behavior.

@trly trly closed this Aug 22, 2025
@trly trly deleted the fix_volume_filters branch August 22, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants