volume prune: add dry-run support#28673
Conversation
github-actions
Bot
added
the
kind/api-change
Add a --dry-run option to show which volumes would be pruned without removing them. Related: containers#27838 Signed-off-by: KyounghoonJang <matkimchi_@naver.com>
kyounghunJang
force-pushed
the
feat/add-volume-prune-dry-run
branch
from
c89d8dd to
0b42fa2
Compare
| } | ||
|
|
||
| // PlanPruneVolumes returns a list of volumes that would be pruned by PruneVolumes | ||
| func (r *Runtime) PlanPruneVolumes(filterFuncs []VolumeFilter) ([]*reports.PruneReport, error) { |
There was a problem hiding this comment.
This function has divergent logic from PruneVolumes.
There was a problem hiding this comment.
Would it not be simpler to have a dryRun option to PruneVolumes? Keep the logic between the two shared, just gate the actual r.removeVolume bit off if it's set?
There was a problem hiding this comment.
@mheon
After thinking about it again, I think having PruneVolumes take a dryRun bool parameter and handling the branching logic there, as you suggested, is a much simpler approach.
Thanks for the feedback! I’ll update it accordingly.
| Expect(session.OutputToStringArray()[0]).To(Equal(vol1)) | ||
| }) | ||
|
|
||
| It("podman volume prune --all --dry-run", func() { |
There was a problem hiding this comment.
I would like to see also other tests for example:
--dry-runwithout--all(anonymous-only volumes)--dry-runcombined with--force(should work but good to verify)- Negative test for volumes in use (should not appear in dry-run output)
There was a problem hiding this comment.
@Honny1
Thank you for the feedback.
Before adding the test code, I would like to share the results I observed in my remote environment.
## case 1: --dry-run without --all
$ podman volume prune --dry-run
Volumes that would be pruned:
## case 2: --dry-run combined with --force
$ podman volume prune --force --all --dry-run
Volumes that would be pruned:
vol1
validatepr-gocache
validatepr-gomodcache
validatepr-lintcache
validatepr-precommitcache
$ podman volume ls
DRIVER VOLUME NAME
local vol1
local validatepr-gocache
local validatepr-gomodcache
local validatepr-lintcache
local validatepr-precommitcache
## case 3: Negative test for volumes in use
$ podman run -d --name testctr -v vol1:/data alpine sleep 1000
$ podman volume prune --all --dry-run
Volumes that would be pruned:
validatepr-gocache
validatepr-gomodcache
validatepr-lintcache
validatepr-precommitcacheThere was a problem hiding this comment.
Do not forget to create also anonymous volumes.
| // - `until=<timestamp>` Prune volumes created before this timestamp. The `<timestamp>` can be Unix timestamps, date formatted timestamps, or Go duration strings (e.g. `10m`, `1h30m`) computed relative to the daemon machine’s time. | ||
| // - `label` (`label=<key>`, `label=<key>=<value>`, `label!=<key>`, or `label!=<key>=<value>`) Prune volumes with (or without, in case `label!=...` is used) the specified labels. | ||
| // - in: query | ||
| // name: dryrun |
There was a problem hiding this comment.
The dryrun query parameter should include required: false and ideally a default: false.
| _ = pruneCommand.RegisterFlagCompletionFunc(filterFlagName, common.AutocompleteVolumePruneFilters) | ||
| flags.BoolP("force", "f", false, "Do not prompt for confirmation") | ||
| flags.BoolP("all", "a", false, "Remove all unused volumes, both anonymous and named") | ||
| flags.Bool("dry-run", false, "Show what would be pruned without actually pruning") |
There was a problem hiding this comment.
This should conflict with --force
3 participants
Add a --dry-run option to show which volumes would be pruned without removing them.
Example Output (remote)
Related: #27838
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?