Honor label filters for volume prune when all is set#29038
Conversation
Honny1
left a comment
Honny1
left a comment
There was a problem hiding this comment.
I did a light review. Please rebase onto upstream main to fix the Windows failure. Also, link fixed issue if it exists. Thanks
shuaiyuanxx
force-pushed
the
fix-volume-prune-all-label-filters
branch
from
bdad3c8 to
35dff6b
Compare
Thanks for the review! Rebased onto the latest main. There's no existing issue for this, so I've left the Fixes line as None. |
danishprakash
left a comment
danishprakash
left a comment
There was a problem hiding this comment.
Thanks, code LGTM but could you please add an integration test for this?
shuaiyuanxx
force-pushed
the
fix-volume-prune-all-label-filters
branch
from
35dff6b to
4e4713f
Compare
Thanks! Added an e2e integration test |
|
Can you please rebase with main, it should fix the apiv2 failures. |
Honny1
left a comment
Honny1
left a comment
There was a problem hiding this comment.
This behavior should be documented in the docs.
shuaiyuanxx
force-pushed
the
fix-volume-prune-all-label-filters
branch
from
4e4713f to
4dc909d
Compare
NormalizeVolumePruneFilters discarded every query filter when the "all" pseudo-filter was set, deleting label/label!/until before they reached the volume filter generator. As a result `podman volume prune --all --filter label=foo` ignored the label and pruned every unused volume. "all" only widens the prune scope from anonymous-only to all unused volumes; it is orthogonal to the label filters, which must still select which of those volumes are removed. Drop only the "all" key and keep the remaining filters so they continue to apply. NormalizeVolumePruneFilters is shared by the local (abi), remote (libpod API), and Docker-compat prune paths, so all three were affected. Signed-off-by: Shuai Yuan <shuaiyuanzju@gmail.com>
shuaiyuanxx
force-pushed
the
fix-volume-prune-all-label-filters
branch
from
4dc909d to
4586dc2
Compare
Done, could you help to review it again? Thanks! |
|
I already rebased to main, but why ci still failed.. |
|
It seems that these failures are flakes. I have rerun the tests. |
Honny1
left a comment
Honny1
left a comment
There was a problem hiding this comment.
LGTM
PTAL @podman-container-tools/podman-reviewers @podman-container-tools/podman-maintainers
|
LGTM |
4 participants
What this PR does / why we need it:
NormalizeVolumePruneFiltersdiscarded every filter when theallflag was set, deletinglabel/label!/untilbefore they reached the filter generator. Sopodman volume prune --all --filter label=fooignored the label and pruned every unused volume.allonly widens the scope from anonymous-only to all unused volumes; it is orthogonal to the label filters, which must still select which volumes are pruned. The fix drops only theallkey and keeps the rest.How to verify it
go test ./pkg/util/ -run TestNormalizeVolumePruneFiltersWhich issue(s) this PR fixes:
None
Special notes for your reviewer:
The existing
all true drops all and no anonymoustest is unchanged; the fixonly affects requests that combine
allwith other filters.Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?