Error if "until" filter is combined with "--volumes" on system prune#311
Error if "until" filter is combined with "--volumes" on system prune#311thaJeztah merged 1 commit intodocker:masterdocker/cli:masterfrom thaJeztah:fix-system-prune-untilthaJeztah/cli:fix-system-prune-untilCopy head branch name to clipboard
Conversation
|
ping @cpuguy83 ptal |
| // TODO make `opts.FilterOpt` more flexible to prevent awkward hacks like this | ||
| // TODO version this hack once "until" filter is supported for volumes | ||
| vf := opts.NewFilterOpt() | ||
| if filter.Value().Include("label") { |
There was a problem hiding this comment.
If anyone has suggestions for doing this in a cleaner way, let me know
I didn't want to update the package upstream (moby/moby) to keep the diff small
|
We could keep the list of supported filters in the API spec. |
25e791a to
76ae7b4
Compare
Codecov Report
@@ Coverage Diff @@
## master #311 +/- ##
==========================================
- Coverage 46.15% 46.14% -0.01%
==========================================
Files 193 193
Lines 16069 16073 +4
==========================================
+ Hits 7416 7417 +1
- Misses 8267 8269 +2
- Partials 386 387 +1 |
|
This seems strange to me. What would be the use case for removing other things older than x, but removing all volumes? I can't think of when that would ever be useful. Generally I don't think that I think the correct fix here is to remove filters from |
|
It's been there since 17.04 moby/moby#29226, so if we want to remove it, it should go through a deprecation cycle I do see value when using filtering by label (i.e. being able to prune everything, and exclude resources labeled with x.y.z), but can understand your thinking on 'until' for these. For this PR; it's not adding a feature but fixing a bug |
The "until" filter is supported by all object types, except for
volumes.
Before this patch, the "until" filter would attempted to be used for the volume
prune endpoint, resulting in an error being returned by the daemon, and
further prune endpoints (networks, images) to be skipped.
$ docker system prune --filter until=24h --filter label=label.foo=bar
WARNING! This will remove:
- all stopped containers
- all volumes not used by at least one container
- all networks not used by at least one container
- all dangling images
Are you sure you want to continue? [y/N] y
Error response from daemon: Invalid filter 'until'
Calling POST /v1.30/containers/prune?filters=%7B%22label%22%3A%7B%22label.foo%3D%3Dbar%22%3Atrue%7D%2C%22until%22%3A%7B%2224h%22%3Atrue%7D%7D
Calling POST /v1.30/volumes/prune?filters=%7B%22label%22%3A%7B%22label.foo%3D%3Dbar%22%3Atrue%7D%2C%22until%22%3A%7B%2224h%22%3Atrue%7D%7D
Handler for POST /v1.30/volumes/prune returned error: Invalid filter 'until'
Error response from daemon: Invalid filter 'until'
With this patch, an error is produced instead, preventing "partial" prune.
$ docker system prune --filter until=24h --filter label=foo==bar --volumes
ERROR: The "until" filter is not supported with "--volumes"
Note that `docker volume prune` does not have this problem, and produces an
error if the `until` filter is used;
$ docker volume prune --filter until=24h
WARNING! This will remove all volumes not used by at least one container.
Are you sure you want to continue? [y/N] y
Error response from daemon: Invalid filter 'until'
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
76ae7b4 to
3c095dc
Compare
|
Updated; I decided to simplify this PR, and instead error out if ping @dnephin @vdemeester PTAL |
dnephin
left a comment
There was a problem hiding this comment.
LGTM
I think this is a good fix.
|
LGTM |
|
I'll merge, but happy to do a follow-up if we want this tested |
fixes #310
relates to moby/moby#31122 (comment)
The "until" filter is supported by all object types, except for
volumes.
Before this patch, the "until" filter would attempted to be used for the volume
prune endpoint, resulting in an error being returned by the daemon, and
further prune endpoints (networks, images) to be skipped.
With this patch, an error is produced instead, preventing "partial" prune.
Note that
docker volume prunedoes not have this problem, and produces anerror if the
untilfilter is used;