Error if "until" filter is combined with "--volumes" on system prune #311
Conversation
|
ping @cpuguy83 ptal |
cli/command/prune/prune.go
Outdated
| // 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. |
thaJeztah
force-pushed
the
fix-system-prune-until
branch
from
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>
thaJeztah
force-pushed
the
fix-system-prune-until
branch
from
76ae7b4
to
3c095dc
Compare
thaJeztah
changed the title
|
Updated; I decided to simplify this PR, and instead error out if ping @dnephin @vdemeester PTAL |
|
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;