From bfc42e4f4f1a59b69ff9f87fed9591fe6f1160d4 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 00:20:32 +0200 Subject: [PATCH] Read volume sizes only when asked and remove in bounded batches, so docker_volumes_remove_anonymous answers on a machine with over a thousand volumes (issue 1796) --- modules/docker/README.md | 2 +- modules/docker/cmd/docker-tools/docker.go | 82 ++++++++++++++----- .../docker/cmd/docker-tools/docker_test.go | 28 ++++++- modules/docker/cmd/docker-tools/main.go | 10 ++- 4 files changed, 98 insertions(+), 24 deletions(-) diff --git a/modules/docker/README.md b/modules/docker/README.md index 2233859a..2f85be97 100644 --- a/modules/docker/README.md +++ b/modules/docker/README.md @@ -160,7 +160,7 @@ A failure is an error naming how it failed, never an empty answer. | `docker_disk_usage` | r | `docker system df -v`: total, active and reclaimable per kind, with the largest of each | | `docker_networks` | r | networks, subnets, and the containers on each | | `docker_volumes` | r | volumes, who mounts each, whether the mesh holds one of them, anonymous or not, and sizes if asked | -| `docker_volumes_remove_anonymous` | a | leftover anonymous volumes no container mounts in any state (so none the mesh holds), optionally created before a time; each other volume answered with why it stays; none can be named. **A dry run unless `dry_run` is false, which needs `why`. Never forced** (issue 1796) | +| `docker_volumes_remove_anonymous` | a | leftover anonymous volumes no container mounts in any state (so none the mesh holds), optionally created before a time; each other volume answered with why it stays; none can be named. **A dry run unless `dry_run` is false, which needs `why`. Never forced** (issue 1796). Sizes only when `sizes` is true (docker is slow on very many volumes); at most `limit` (default 200) per call | | `docker_events` | r | the runtime's events over a window ending now (default 60 min, at most 24 h), without exec noise unless asked; **an exec's command line is shown with any secret it carried as `[redacted: ]`** | | `docker_daemon_config` | r | `daemon.json` as on disk, `docker info`'s essentials, and keys the daemon has not taken yet | | `docker_unlabelled` | r | the containers the mesh does not hold: the cleanup list | diff --git a/modules/docker/cmd/docker-tools/docker.go b/modules/docker/cmd/docker-tools/docker.go index 9d723a30..31e6791f 100644 --- a/modules/docker/cmd/docker-tools/docker.go +++ b/modules/docker/cmd/docker-tools/docker.go @@ -944,16 +944,9 @@ func (c *Client) Volumes(ctx context.Context, unmountedOnly, sizes bool) (map[st } size := map[string]string{} if sizes { - out, err := c.docker(ctx, "system", "df", "--verbose", "--format", "{{json .}}") - if err != nil { + if size, err = c.volumeSizes(ctx); err != nil { return nil, err } - var v struct{ Volumes []map[string]any } - if err := json.Unmarshal([]byte(out), &v); err == nil { - for _, vol := range v.Volumes { - size[fmt.Sprint(vol["Name"])] = fmt.Sprint(vol["Size"]) - } - } } vols := []map[string]any{} if len(lines(names)) > 0 { @@ -1216,11 +1209,30 @@ func (c *Client) DaemonConfig(ctx context.Context) (map[string]any, error) { return answer, nil } +// volumeSizes is every volume's size by name, from docker system df -v, which walks every volume: +// on a machine with a thousand of them it does not answer within a call's bound. +func (c *Client) volumeSizes(ctx context.Context) (map[string]string, error) { + size := map[string]string{} + out, err := c.docker(ctx, "system", "df", "--verbose", "--format", "{{json .}}") + if err != nil { + return nil, err + } + var v struct{ Volumes []map[string]any } + if err := json.Unmarshal([]byte(out), &v); err == nil { + for _, vol := range v.Volumes { + size[fmt.Sprint(vol["Name"])] = fmt.Sprint(vol["Size"]) + } + } + return size, nil +} + // RemoveVolumesAsk is what docker_volumes_remove_anonymous is asked to take. type RemoveVolumesAsk struct { DryRun bool Why string CreatedBefore *time.Time + Sizes bool + Limit int // how many to remove in one call; 0 is 200 } // RemoveAnonymousVolumes removes the leftover anonymous volumes (novox/hq issue 1796): those docker @@ -1232,11 +1244,10 @@ func (c *Client) RemoveAnonymousVolumes(ctx context.Context, a RemoveVolumesAsk) if !a.DryRun && strings.TrimSpace(a.Why) == "" { return nil, fmt.Errorf("why is required when dry_run is false") } - all, err := c.Volumes(ctx, false, true) + all, err := c.Volumes(ctx, false, false) if err != nil { return nil, err } - sizes := map[string]string{} candidates := []string{} skipped := []map[string]any{} skip := func(name, why string) { skipped = append(skipped, map[string]any{"name": name, "why": why}) } @@ -1253,7 +1264,18 @@ func (c *Client) RemoveAnonymousVolumes(ctx context.Context, a RemoveVolumesAsk) skip(name, "not created before "+a.CreatedBefore.Format(time.RFC3339)+" (created "+fmt.Sprint(v["created"])+")") default: candidates = append(candidates, name) - sizes[name] = fmt.Sprint(v["size"]) + } + } + answer := map[string]any{"dry_run": a.DryRun, "candidates": len(candidates)} + // Sizes are asked for, never needed: the listing walks every volume and may not answer, and + // then the call answers without them. Nothing below depends on it. + sizes := map[string]string{} + if a.Sizes { + if got, err := c.volumeSizes(ctx); err != nil { + answer["sizes_unknown"] = err.Error() + } else { + sizes = got + answer["sizes"] = sizes } } sum := func(names []string) int64 { @@ -1263,24 +1285,46 @@ func (c *Client) RemoveAnonymousVolumes(ctx context.Context, a RemoveVolumesAsk) } return t } - answer := map[string]any{"dry_run": a.DryRun, "skipped": skipped, "sizes": sizes} if a.DryRun { + answer["skipped"] = skipped answer["would_remove"] = candidates - answer["would_free_bytes"] = sum(candidates) - answer["note"] = "nothing was removed: call again with dry_run false and why to remove them" + if len(sizes) > 0 { + answer["would_free_bytes"] = sum(candidates) + } + answer["note"] = "nothing was removed: call again with dry_run false and why to remove them (limit volumes at most per call)" return answer, nil } + limit := a.Limit + if limit <= 0 { + limit = 200 + } + take := candidates + if len(take) > limit { + take = candidates[:limit] + } removed := []string{} - for _, name := range candidates { - if _, err := c.docker(ctx, "volume", "rm", name); err != nil { - skip(name, "docker refused: "+err.Error()) + // Chunks of a hundred in one `docker volume rm`, so a thousand volumes are ten calls; a chunk + // docker refuses any of is done again one by one, so what it refuses is named. + for i := 0; i < len(take); i += 100 { + chunk := take[i:min(i+100, len(take))] + if _, err := c.docker(ctx, append([]string{"volume", "rm"}, chunk...)...); err == nil { + removed = append(removed, chunk...) continue } - removed = append(removed, name) + for _, name := range chunk { + if _, err := c.docker(ctx, "volume", "rm", name); err != nil { + skip(name, "docker refused: "+err.Error()) + continue + } + removed = append(removed, name) + } } answer["skipped"] = skipped answer["removed"], answer["count"], answer["why"] = removed, len(removed), a.Why - answer["freed_bytes"] = sum(removed) + answer["remaining"] = len(candidates) - len(take) + if len(sizes) > 0 { + answer["freed_bytes"] = sum(removed) + } return answer, nil } diff --git a/modules/docker/cmd/docker-tools/docker_test.go b/modules/docker/cmd/docker-tools/docker_test.go index 14f974e6..c4d2cc03 100644 --- a/modules/docker/cmd/docker-tools/docker_test.go +++ b/modules/docker/cmd/docker-tools/docker_test.go @@ -445,7 +445,7 @@ func TestRemoveAnonymousVolumesTakesOnlyTheUnmountedAnonymousOnes(t *testing.T) on("docker volume rm old", Ran{Stdout: "old\n"}) c := client(f, 1000) before := time.Date(2026, 10, 3, 0, 0, 0, 0, time.UTC) - got, err := c.RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{DryRun: true, CreatedBefore: &before}) + got, err := c.RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{DryRun: true, Sizes: true, CreatedBefore: &before}) if err != nil { t.Fatal(err) } @@ -463,7 +463,7 @@ func TestRemoveAnonymousVolumesTakesOnlyTheUnmountedAnonymousOnes(t *testing.T) if len(why) != 3 || !strings.Contains(why["webdata"], "mesh holds") || why["young"] == "" || why["named"] == "" { t.Fatalf("skipped: %v", skipped) } - got, err = c.RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{Why: "leftovers", CreatedBefore: &before}) + got, err = c.RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{Why: "leftovers", Sizes: true, CreatedBefore: &before}) if err != nil { t.Fatal(err) } @@ -501,3 +501,27 @@ func TestRemoveAnonymousVolumesRefusesANamedVolumeAndKeepsWhatTheMeshHolds(t *te } t.Fatal("tool not served") } + +// 1796/AC3: on a machine with a very large set of volumes docker's size listing does not answer in +// time; the tool still answers, and removal never depends on it. +func TestRemoveAnonymousVolumesAnswersWhenTheSizeListingDoesNot(t *testing.T) { + f := (&fake{}). + on("docker ps --all --quiet --no-trunc", Ran{}). + on("docker volume ls --quiet", Ran{Stdout: "a1\na2\na3\n"}). + on("docker volume inspect", Ran{Stdout: `[{"Name":"a1","Labels":{"com.docker.volume.anonymous":""}},{"Name":"a2","Labels":{"com.docker.volume.anonymous":""}},{"Name":"a3","Labels":{"com.docker.volume.anonymous":""}}]`}). + on("docker system df", Ran{Status: 124, Err: "no answer within 20 s"}). + on("docker volume rm", Ran{Stdout: "ok\n"}) + c := client(f, 1000) + got, err := c.RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{DryRun: true}) + if err != nil || f.ran("docker system df") || !reflect.DeepEqual(got["would_remove"], []string{"a1", "a2", "a3"}) { + t.Fatalf("default dry run: %v %v %+v", got, err, f.calls) + } + got, err = c.RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{DryRun: true, Sizes: true}) + if err != nil || !reflect.DeepEqual(got["would_remove"], []string{"a1", "a2", "a3"}) || got["sizes_unknown"] == nil { + t.Fatalf("sizes asked, df silent: %v %v", got, err) + } + got, err = c.RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{Why: "w", Sizes: true, Limit: 2}) + if err != nil || got["count"] != 2 || got["remaining"] != 1 || !f.ran("docker volume rm a1 a2") { + t.Fatalf("removal: %v %v %+v", got, err, f.calls) + } +} diff --git a/modules/docker/cmd/docker-tools/main.go b/modules/docker/cmd/docker-tools/main.go index 545984b3..cc7dcbe5 100644 --- a/modules/docker/cmd/docker-tools/main.go +++ b/modules/docker/cmd/docker-tools/main.go @@ -291,12 +291,14 @@ func tools(c *Client) []stdio.Tool { Name: "docker_volumes_remove_anonymous", Description: "Remove the leftover anonymous volumes (novox/hq issue 1796): only a volume docker labels anonymous (as docker_volumes says), which no container mounts in any state " + "and so none the mesh holds, and with created_before only one created before it. Every other volume is answered with why it stays; a named volume is data and is never taken, " + - "and no volume can be named. Each is removed on its own, never forced; the answer has the count and names removed. The answer has each volume's size and the bytes freed (or, in a dry run, that would be). " + + "and no volume can be named. Each is removed on its own, never forced; the answer has the count and names removed. With sizes true the answer has each volume's size and the bytes freed; docker's size listing is slow on many volumes, so it is asked for, and when it does not answer the call answers without it. At most limit are removed per call, in batches. " + "A dry run by default; dry_run false needs why. Replaces docker volume rm and docker volume prune. (a)", Input: map[string]any{ "dry_run": map[string]any{"type": "boolean", "description": "list only (default true)"}, "why": map[string]any{"type": "string", "description": "why: required when dry_run is false"}, "created_before": map[string]any{"type": "string", "description": "RFC 3339 time: only volumes created before it (default: any age)"}, + "sizes": map[string]any{"type": "boolean", "description": "also read each volume's size and the bytes freed (default false: docker walks every volume for it and on a machine with very many may not answer; the call then answers without sizes)"}, + "limit": map[string]any{"type": "integer", "description": "how many to remove in one call (default 200, at most 2000); the answer says how many remain, call again for them"}, }, Run: func(args map[string]any) (any, error) { for _, k := range []string{"name", "names", "volume", "volumes"} { @@ -304,7 +306,11 @@ func tools(c *Client) []stdio.Tool { return nil, fmt.Errorf("%s: this tool takes no volume by name; it removes only unmounted anonymous volumes", k) } } - ask := RemoveVolumesAsk{DryRun: flag(args, "dry_run", true), Why: optional(args, "why")} + limit, err := bounded(args, "limit", 200, 2000) + if err != nil { + return nil, err + } + ask := RemoveVolumesAsk{DryRun: flag(args, "dry_run", true), Why: optional(args, "why"), Sizes: flag(args, "sizes", false), Limit: limit} if s := optional(args, "created_before"); s != "" { t, err := time.Parse(time.RFC3339, s) if err != nil {